diff --git a/react-front-end/__mocks__/ACLExpressionModule.mock.ts b/react-front-end/__mocks__/ACLExpressionModule.mock.ts index ff7be873ec..608d343e83 100644 --- a/react-front-end/__mocks__/ACLExpressionModule.mock.ts +++ b/react-front-end/__mocks__/ACLExpressionModule.mock.ts @@ -82,6 +82,15 @@ export const aclWithComplexSubExpression = export const aclWithComplexSubExpressionInfix = "Yasmin Day [user300] AND From 127.0.0.1%2F24 AND From *google* AND ( Owner OR Guest User Role OR Ronny Southgate [user400] OR ( NOT ( Fabienne Hobson [user100] OR Racheal Carlyle [user200] ) ) OR ( From aa AND ( NOT ( From bb OR From cc ) ) ) ) AND ( NOT ( Engineering & Computer Science Students OR Engineering & Computer Science Staff ) )"; +export const aclWithEmptyRecipientExpression = "I:1 I:2 OR I:3 NOT AND"; +export const aclWithEmptyRecipientExpressionInfix = + "( From 1 OR From 2 ) AND ( NOT From 3 )"; + +export const aclChildWithEmptyRecipientExpression = + "I:1 I:2 OR I:3 I:4 OR I:5 NOT AND OR"; +export const aclChildWithEmptyRecipientExpressionInfix = + "From 1 OR From 2 OR ( ( From 3 OR From 4 ) AND ( NOT From 5 ) )"; + export const everyoneACLExpression: ACLExpression = createACLExpression( "OR", [everyoneRecipient], @@ -256,6 +265,46 @@ export const complexExpressionACLExpression: ACLExpression = ] ); +/** + * ``` + * AND + * OR 1 2 + * NOT 3 + * ``` + */ +export const withEmptyRecipientExpression: ACLExpression = createACLExpression( + "AND", + [], + [ + createACLExpression("OR", [ipRecipient("1"), ipRecipient("2")], []), + createACLExpression("NOT", [ipRecipient("3")], []), + ] +); + +/** + * ``` + * OR 1 2 + * AND + * OR 3 4 + * NOT 4 + * ``` + */ +export const childWithEmptyRecipientExpression: ACLExpression = + createACLExpression( + "OR", + [ipRecipient("1"), ipRecipient("2")], + [ + createACLExpression( + "AND", + [], + [ + createACLExpression("OR", [ipRecipient("3"), ipRecipient("4")], []), + createACLExpression("NOT", [ipRecipient("5")], []), + ] + ), + ] + ); + export const childSameOperatorExpression: ACLExpression = createACLExpression( "AND", [userContentAdminRecipient], diff --git a/react-front-end/__tests__/tsrc/modules/ACLExpressionModule.test.ts b/react-front-end/__tests__/tsrc/modules/ACLExpressionModule.test.ts index e4a62f52ee..fd620483e5 100644 --- a/react-front-end/__tests__/tsrc/modules/ACLExpressionModule.test.ts +++ b/react-front-end/__tests__/tsrc/modules/ACLExpressionModule.test.ts @@ -54,6 +54,8 @@ import { aclUserInfix, aclWithComplexSubExpression, aclWithComplexSubExpressionInfix, + aclChildWithEmptyRecipientExpression, + aclChildWithEmptyRecipientExpressionInfix, aclWithMultipleSubExpression, aclWithMultipleSubExpressionInfix, aclWithNestedSubExpression, @@ -65,6 +67,7 @@ import { childSameOperatorExpression, complexExpressionACLExpression, complexRedundantExpression, + childWithEmptyRecipientExpression, emptyRecipientWithOneChildExpression, everyoneACLExpression, fourItemsACLExpression, @@ -93,6 +96,9 @@ import { withMultipleSubExpression, withNestedSubExpressionACLExpression, withSubExpressionACLExpression, + aclWithEmptyRecipientExpressionInfix, + aclWithEmptyRecipientExpression, + withEmptyRecipientExpression, } from "../../../__mocks__/ACLExpressionModule.mock"; import { ignoreId } from "./ACLExpressionModuleTestHelper"; @@ -114,38 +120,71 @@ describe("ACLExpressionModule", () => { aclExpression: ACLExpression ): E.Either => pipe(aclExpression, ignoreId, E.right); + const commonACLExpressionForParametricTests: [ + string, + string, + string, + ACLExpression + ][] = [ + ["everyone", aclEveryone, aclEveryoneInfix, everyoneACLExpression], + ["aclOwner", aclOwner, aclOwnerInfix, ownerACLExpression], + ["aclUser", aclUser, aclUserInfix, userACLExpression], + ["aclNotUser", aclNotUser, aclNotUserInfix, notUserACLExpression], + ["aclTwoItems", aclTwoItems, aclTwoItemsInfix, twoItemsACLExpression], + [ + "aclThreeItems", + aclThreeItems, + aclThreeItemsInfix, + threeItemsACLExpression, + ], + [ + "fourItemsACLExpression", + aclFourItems, + aclFourItemsInfix, + fourItemsACLExpression, + ], + [ + "aclWithSubExpression", + aclWithSubExpression, + aclWithSubExpressionInfix, + withSubExpressionACLExpression, + ], + [ + "aclWithMultipleSubExpression", + aclWithMultipleSubExpression, + aclWithMultipleSubExpressionInfix, + withMultipleSubExpression, + ], + [ + "aclWithNestedSubExpression", + aclWithNestedSubExpression, + aclWithNestedSubExpressionInfix, + withNestedSubExpressionACLExpression, + ], + [ + "aclWithComplexSubExpression", + aclWithComplexSubExpression, + aclWithComplexSubExpressionInfix, + complexExpressionACLExpression, + ], + [ + "aclWithEmptyRecipientExpression", + aclWithEmptyRecipientExpression, + aclWithEmptyRecipientExpressionInfix, + withEmptyRecipientExpression, + ], + [ + "aclChildWithEmptyRecipientExpression", + aclChildWithEmptyRecipientExpression, + aclChildWithEmptyRecipientExpressionInfix, + childWithEmptyRecipientExpression, + ], + ]; + describe("parse", () => { - it.each([ - ["everyone", aclEveryone, everyoneACLExpression], - ["aclOwner", aclOwner, ownerACLExpression], - ["aclUser", aclUser, userACLExpression], - ["aclNotUser", aclNotUser, notUserACLExpression], - ["aclTwoItems", aclTwoItems, twoItemsACLExpression], - ["aclThreeItems", aclThreeItems, threeItemsACLExpression], - ["fourItemsACLExpression", aclFourItems, fourItemsACLExpression], - [ - "aclWithSubExpression", - aclWithSubExpression, - withSubExpressionACLExpression, - ], - [ - "aclWithMultipleSubExpression", - aclWithMultipleSubExpression, - withMultipleSubExpression, - ], - [ - "aclWithNestedSubExpression", - aclWithNestedSubExpression, - withNestedSubExpressionACLExpression, - ], - [ - "aclWithComplexSubExpression", - aclWithComplexSubExpression, - complexExpressionACLExpression, - ], - ])( + it.each(commonACLExpressionForParametricTests)( "parse %s as ACL Expressions", - (_, aclExpression, expectedExpression) => { + (_, aclExpression, _infix, expectedExpression) => { expect(parse(aclExpression)).toEqual( expectedRightResult(expectedExpression) ); @@ -225,37 +264,9 @@ describe("ACLExpressionModule", () => { }); describe("generate", () => { - it.each([ - ["everyoneACLExpression", everyoneACLExpression, aclEveryone], - ["ownerACLExpression", ownerACLExpression, aclOwner], - ["userACLExpression", userACLExpression, aclUser], - ["notUserACLExpression", notUserACLExpression, aclNotUser], - ["twoItemsACLExpression", twoItemsACLExpression, aclTwoItems], - ["threeItemsACLExpression", threeItemsACLExpression, aclThreeItems], - ["fourItemsACLExpression", fourItemsACLExpression, aclFourItems], - [ - "withSubExpressionACLExpression", - withSubExpressionACLExpression, - aclWithSubExpression, - ], - [ - "withMultipleSubExpression", - withMultipleSubExpression, - aclWithMultipleSubExpression, - ], - [ - "withNestedSubExpressionACLExpression", - withNestedSubExpressionACLExpression, - aclWithNestedSubExpression, - ], - [ - "complexExpressionACLExpression", - complexExpressionACLExpression, - aclWithComplexSubExpression, - ], - ])( + it.each(commonACLExpressionForParametricTests)( "generate postfix ACL Expression text from %s", - (_, aclExpression, expectedExpression) => { + (_, expectedExpression, _infix, aclExpression) => { expect(generate(aclExpression)).toEqual(expectedExpression); } ); @@ -271,40 +282,12 @@ describe("ACLExpressionModule", () => { resolveRoleProvider: findRoleById, }); - it.each([ - ["everyoneACLExpression", everyoneACLExpression, aclEveryoneInfix], - ["ownerACLExpression", ownerACLExpression, aclOwnerInfix], - ["userACLExpression", userACLExpression, aclUserInfix], - ["notUserACLExpression", notUserACLExpression, aclNotUserInfix], - ["twoItemsACLExpression", twoItemsACLExpression, aclTwoItemsInfix], - ["threeItemsACLExpression", threeItemsACLExpression, aclThreeItemsInfix], - ["fourItemsACLExpression", fourItemsACLExpression, aclFourItemsInfix], - [ - "withSubExpressionACLExpression", - withSubExpressionACLExpression, - aclWithSubExpressionInfix, - ], - [ - "withMultipleSubExpression", - withMultipleSubExpression, - aclWithMultipleSubExpressionInfix, - ], - [ - "withNestedSubExpressionACLExpression", - withNestedSubExpressionACLExpression, - aclWithNestedSubExpressionInfix, - ], - [ - "complexExpressionACLExpression", - complexExpressionACLExpression, - aclWithComplexSubExpressionInfix, - ], - ])( + it.each(commonACLExpressionForParametricTests)( "generate infix ACL Expression text (human readable text) from %s", - async (_, aclExpression, expectedExpression) => { + async (_, _acl, expectedInfixText, aclExpression) => { await expect( generateHumanReadableWithMocks(aclExpression)() - ).resolves.toEqual(expectedExpression); + ).resolves.toEqual(expectedInfixText); } ); }); @@ -426,71 +409,18 @@ describe("ACLExpressionModule", () => { }); describe("values produced by the module, should also work with the module", () => { - it.each([ - ["everyone", aclEveryone, everyoneACLExpression], - ["aclOwner", aclOwner, ownerACLExpression], - ["aclUser", aclUser, userACLExpression], - ["aclNotUser", aclNotUser, notUserACLExpression], - ["aclTwoItems", aclTwoItems, twoItemsACLExpression], - ["aclThreeItems", aclThreeItems, threeItemsACLExpression], - ["fourItemsACLExpression", aclFourItems, fourItemsACLExpression], - [ - "aclWithSubExpression", - aclWithSubExpression, - withSubExpressionACLExpression, - ], - [ - "aclWithMultipleSubExpression", - aclWithMultipleSubExpression, - withMultipleSubExpression, - ], - [ - "aclWithNestedSubExpression", - aclWithNestedSubExpression, - withNestedSubExpressionACLExpression, - ], - [ - "aclWithComplexSubExpression", - aclWithComplexSubExpression, - complexExpressionACLExpression, - ], - ])("parse -> generate -> parse: %s", (_, acl, expectedACLExpression) => { - expect(pipe(acl, parse, handleParse, generate, parse)).toEqual( - expectedRightResult(expectedACLExpression) - ); - }); + it.each(commonACLExpressionForParametricTests)( + "parse -> generate -> parse: %s", + (_, acl, _infix, expectedACLExpression) => { + expect(pipe(acl, parse, handleParse, generate, parse)).toEqual( + expectedRightResult(expectedACLExpression) + ); + } + ); - it.each([ - ["everyone", everyoneACLExpression, aclEveryone], - ["aclOwner", ownerACLExpression, aclOwner], - ["aclUser", userACLExpression, aclUser], - ["aclNotUser", notUserACLExpression, aclNotUser], - ["aclTwoItems", twoItemsACLExpression, aclTwoItems], - ["aclThreeItems", threeItemsACLExpression, aclThreeItems], - ["fourItemsACLExpression", fourItemsACLExpression, aclFourItems], - [ - "aclWithSubExpression", - withSubExpressionACLExpression, - aclWithSubExpression, - ], - [ - "aclWithMultipleSubExpression", - withMultipleSubExpression, - aclWithMultipleSubExpression, - ], - [ - "aclWithNestedSubExpression", - withNestedSubExpressionACLExpression, - aclWithNestedSubExpression, - ], - [ - "aclWithComplexSubExpression", - complexExpressionACLExpression, - aclWithComplexSubExpression, - ], - ])( + it.each(commonACLExpressionForParametricTests)( "generate -> parse -> generate: %s", - (_, aclExpression, expectedText) => { + (_, expectedText, _infix, aclExpression) => { expect( pipe(aclExpression, generate, parse, handleParse, generate) ).toEqual(expectedText); diff --git a/react-front-end/tsrc/modules/ACLExpressionModule.ts b/react-front-end/tsrc/modules/ACLExpressionModule.ts index 70cf1b501a..240e0f9253 100644 --- a/react-front-end/tsrc/modules/ACLExpressionModule.ts +++ b/react-front-end/tsrc/modules/ACLExpressionModule.ts @@ -982,28 +982,48 @@ const generatePostfixResults = (aclExpression: ACLExpression): string[] => { ? [showRecipient(recipient), operator] : [showRecipient(recipient)]; - const reduceRecipients = ( + // This function helps to insert the operator between elements and flatten the array. + const insertOperators = + (operator: string) => + (results: string[][]): string[] => + pipe( + results, + A.reduceWithIndex([], (index, acc, childResult) => + // Insert an operator between each elements, except for the first element. + index > 1 + ? [...acc, aclExpression.operator, ...childResult] + : [...acc, ...childResult] + ), + // Make sure the operator is always added on the end + pfTernary(A.isNonEmpty, A.append(operator), identity) + ); + + const handleRecipients = ( operator: ACLOperatorType, recipients: ACLRecipient[] ): string[] => pipe( recipients, - A.reduceWithIndex( - [], - (index, acc, currentRecipient) => - // Insert an operator between each elements, except for the first element." - index > 1 - ? [...acc, operator, showRecipient(currentRecipient)] - : [...acc, showRecipient(currentRecipient)] - ), - // Make sure the operator is always added on the end - pfTernary( - (a) => A.isNonEmpty(a) && NEA.last(a) !== operator, - A.append(operator as string), - identity - ) + A.map((r) => [showRecipient(r)]), + insertOperators(operator) ); + // generate results from children and combine it with recipients result + const handleWithChildren = + (recipientResult: string[]) => + (children: ACLExpression[]): string[] => + pipe( + children, + A.map(generatePostfixResults), + // if recipientResult is non-empty, prepend it + (results) => + A.isNonEmpty(recipientResult) + ? [recipientResult, ...results] + : results, + // Insert operators between results + insertOperators(aclExpression.operator) + ); + return pipe( // Process the recipients aclExpression, @@ -1011,19 +1031,14 @@ const generatePostfixResults = (aclExpression: ACLExpression): string[] => { ({ operator, recipients }: ACLExpression) => A.isNonEmpty(recipients) && A.size(recipients) === 1 ? handleSingleRecipient(operator, NEA.head(recipients)) - : reduceRecipients(operator, recipients), - // ... then process the children - (currentResult) => + : handleRecipients(operator, recipients), + // ... then process the children if it's not empty + (recipientResult) => pipe( aclExpression.children, - A.reduce( - currentResult, - (result, childExpression) => [ - ...result, - ...generatePostfixResults(childExpression), - aclExpression.operator, - ] - ) + O.fromPredicate(A.isNonEmpty), + O.map(handleWithChildren(recipientResult)), + O.getOrElse(() => recipientResult) ) ); }; @@ -1131,6 +1146,47 @@ const generateInfixResults = ) ); + /** + * Insert an operator between recipient result and children result if they are not empty. + * For `NOT` operator always add a `NOT` prefix, even if the recipient result is empty. + * + * ( It is due to the structure of `NOT` ACLExpression. + * For example, this is a raw (un-compacted) structure of ACLExpression `NOT` node with multiple recipients, + * and all those recipients will be stored in a child `OR` node: + * ``` + * NOT + * OR A B + * ``` + * In this case, the `childrenResult` is ["(", "A", "OR", "B", ")"] while `recipientsResult` is an empty array. + * Thus, here the operator `NOT` will be added. + * But for `NOT` ACLExpression with only one recipient, the structure is different: + * ``` + * NOT A + * ``` + * In this case, `recipientsResult` would be ["NOT", "A"], and no need to add the operator `NOT` here. + * ) + */ + const combineRecipientAndChildrenResult = ( + recipientsResult: string[], + childrenResult: string[] + ) => { + if (A.isEmpty(recipientsResult) && A.isEmpty(childrenResult)) { + return []; + } + + if (A.isEmpty(recipientsResult)) { + return aclExpression.operator !== "NOT" + ? childrenResult + : [aclExpression.operator, ...childrenResult]; + } + + if (A.isEmpty(childrenResult)) { + return recipientsResult; + } + + return [...recipientsResult, aclExpression.operator, ...childrenResult]; + }; + return pipe( aclExpression, // Make sure it doesn't use the reverted form of ACLExpression. Because the following logic assume @@ -1143,9 +1199,7 @@ const generateInfixResults = aclExpression.children, processChildren(aclExpression.operator), T.map((childrenResult: string[]) => - A.isNonEmpty(childrenResult) - ? [...recipientsResult, aclExpression.operator, ...childrenResult] - : recipientsResult + combineRecipientAndChildrenResult(recipientsResult, childrenResult) ) ) )