From 9c8f98c12fa360423b40bfa220ec8e7bb8934ada Mon Sep 17 00:00:00 2001 From: Maciej Pichura <41297682+mpichura@users.noreply.github.com> Date: Thu, 22 Sep 2022 08:10:59 +0200 Subject: [PATCH] ACS-3509 Rule mappers refactor pt2 (#1428) * ACS-3509: Fixes and refactors for rule mappers pt 2. * ACS-3509: Rule Condition mappings refactor + tests. --- .../alfresco/rest/rules/CreateRulesTests.java | 3 - .../alfresco/rest/rules/GetRulesTests.java | 2 - .../alfresco/rest/rules/UpdateRulesTests.java | 30 ++- ...RestRuleCompositeConditionModelMapper.java | 151 +++++++++++ .../RestRuleSimpleConditionModelMapper.java | 35 ++- .../rest/api/impl/rules/RuleLoader.java | 12 +- .../rest/api/impl/rules/RulesImpl.java | 11 +- .../api/model/mapper/RestModelMapper.java | 14 +- .../api/model/rules/CompositeCondition.java | 91 ------- .../alfresco/rest/api/model/rules/Rule.java | 8 +- .../rest/api/model/rules/SimpleCondition.java | 39 --- .../alfresco/public-rest-context.xml | 7 +- .../org/alfresco/rest/api/RulesUnitTests.java | 8 +- ...RuleCompositeConditionModelMapperTest.java | 238 ++++++++++++++++++ ...estRuleSimpleConditionModelMapperTest.java | 79 +++++- .../rest/api/impl/rules/RulesImplTest.java | 20 +- .../model/rules/CompositeConditionTest.java | 193 -------------- .../rest/api/model/rules/RuleTest.java | 14 +- .../api/model/rules/SimpleConditionTest.java | 169 ------------- 19 files changed, 555 insertions(+), 569 deletions(-) create mode 100644 remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapper.java create mode 100644 remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapperTest.java delete mode 100644 remote-api/src/test/java/org/alfresco/rest/api/model/rules/CompositeConditionTest.java delete mode 100644 remote-api/src/test/java/org/alfresco/rest/api/model/rules/SimpleConditionTest.java diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java index 28759405d1..170819ed18 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java @@ -93,7 +93,6 @@ public class CreateRulesTests extends RestTest .createSingleRule(ruleModel); RestRuleModel expectedRuleModel = createRuleModelWithModifiedValues(); - expectedRuleModel.setConditions(createEmptyConditionModel()); restClient.assertStatusCodeIs(CREATED); rule.assertThat().isEqualTo(expectedRuleModel, ID, IS_SHARED) .assertThat().field(ID).isNotNull() @@ -383,7 +382,6 @@ public class CreateRulesTests extends RestTest final RestRuleModel expectedRuleModel = createRuleModelWithDefaultValues(); expectedRuleModel.setActions(Arrays.asList(copyAction, checkOutAction, scriptAction)); - expectedRuleModel.setConditions(createEmptyConditionModel()); expectedRuleModel.setTriggers(List.of("inbound")); restClient.assertStatusCodeIs(CREATED); @@ -459,7 +457,6 @@ public class CreateRulesTests extends RestTest .createSingleRule(ruleModel); RestRuleModel expectedRuleModel = createRuleModelWithDefaultValues(); - expectedRuleModel.setConditions(createCompositeCondition(null)); expectedRuleModel.setTriggers(List.of("inbound")); restClient.assertStatusCodeIs(CREATED); rule.assertThat().isEqualTo(expectedRuleModel, ID, IS_SHARED); diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRulesTests.java index be799b5849..4092c27d5f 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRulesTests.java @@ -190,7 +190,6 @@ public class GetRulesTests extends RestTest RestRuleModel expectedRuleModel = createRuleModelWithModifiedValues(); expectedRuleModel.setTriggers(List.of("update")); - expectedRuleModel.setConditions(createEmptyConditionModel()); restClient.assertStatusCodeIs(CREATED); rule.assertThat().isEqualTo(expectedRuleModel, IGNORE_ID, IGNORE_IS_SHARED) @@ -212,7 +211,6 @@ public class GetRulesTests extends RestTest RestRuleModel expectedRuleModel = createRuleModelWithDefaultValues(); expectedRuleModel.setTriggers(List.of("inbound")); - expectedRuleModel.setConditions(createEmptyConditionModel()); restClient.assertStatusCodeIs(CREATED); diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java index c841ca0966..0934ca0e3f 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java @@ -324,7 +324,7 @@ public class UpdateRulesTests extends RestTest final RestRuleModel rule = createAndSaveRule(createRuleModelWithModifiedValues()); STEP("Try to update the rule and add null conditions."); - rule.setConditions(createCompositeCondition(null)); + rule.setConditions(null); final RestRuleModel updatedRule = restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() .updateRule(rule.getId(), rule); @@ -369,8 +369,6 @@ public class UpdateRulesTests extends RestTest final RestRuleModel updatedRule = restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() .updateRule(rule.getId(), rule); - //set expected object - rule.setConditions(createCompositeCondition(null)); restClient.assertStatusCodeIs(OK); updatedRule.assertThat().isEqualTo(rule, ID) .assertThat().field(ID).isNotNull(); @@ -397,7 +395,7 @@ public class UpdateRulesTests extends RestTest restClient.assertLastError().containsSummary("Category in condition is invalid"); } - /** Check we get a 500 error when using the POST response and update rule by adding condition without comparator. */ + /** Check we get a 400 error when using the POST response and update rule by adding condition without comparator when it is required. */ @Test (groups = { TestGroup.REST_API, TestGroup.RULES, TestGroup.SANITY }) public void updateRuleWithConditionWithoutComparatorAndFail() { @@ -405,7 +403,7 @@ public class UpdateRulesTests extends RestTest ruleModelWithInitialValues.setConditions(createVariousConditions()); final RestRuleModel rule = createAndSaveRule(ruleModelWithInitialValues); - STEP("Try to update the rule with invalid condition."); + STEP("Try to update the rule with invalid condition (null comparator when required non-null)."); final RestCompositeConditionDefinitionModel conditions = createCompositeCondition( List.of(createCompositeCondition(!INVERTED, List.of(createSimpleCondition("size", null, "65500"))))); rule.setConditions(conditions); @@ -414,11 +412,11 @@ public class UpdateRulesTests extends RestTest .include(IS_SHARED) .updateRule(rule.getId(), rule); - //TODO: in next iteration of mapper refactoring this error code will change to 400 - restClient.assertStatusCodeIs(INTERNAL_SERVER_ERROR); + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary("Comparator in condition must not be blank"); } - /** Check we get a 500 error when using the POST response and update rule by adding condition without field. */ + /** Check we get a 400 error when using the POST response and update rule by adding condition without field. */ @Test (groups = { TestGroup.REST_API, TestGroup.RULES, TestGroup.SANITY }) public void updateRuleWithConditionWithoutFieldAndFail() { @@ -426,7 +424,7 @@ public class UpdateRulesTests extends RestTest ruleModelWithInitialValues.setConditions(createVariousConditions()); final RestRuleModel rule = createAndSaveRule(ruleModelWithInitialValues); - STEP("Try to update the rule with invalid condition."); + STEP("Try to update the rule with invalid condition (null field)."); final RestCompositeConditionDefinitionModel conditions = createCompositeCondition( List.of(createCompositeCondition(!INVERTED, List.of(createSimpleCondition(null, "greater_than", "65500"))))); rule.setConditions(conditions); @@ -435,11 +433,11 @@ public class UpdateRulesTests extends RestTest .include(IS_SHARED) .updateRule(rule.getId(), rule); - //TODO: in next iteration of mapper refactoring this error code will change to 400 - restClient.assertStatusCodeIs(INTERNAL_SERVER_ERROR); + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary("Field in condition must not be blank"); } - /** Check we get a 500 error when using the POST response and update rule by adding condition without parameter value. */ + /** Check we get a 400 error when using the POST response and update rule by adding condition without parameter value. */ @Test (groups = { TestGroup.REST_API, TestGroup.RULES, TestGroup.SANITY }) public void updateRuleWithConditionWithoutParamValueAndFail() { @@ -447,17 +445,17 @@ public class UpdateRulesTests extends RestTest ruleModelWithInitialValues.setConditions(createVariousConditions()); final RestRuleModel rule = createAndSaveRule(ruleModelWithInitialValues); - STEP("Try to update the rule with invalid condition."); + STEP("Try to update the rule with invalid condition (null parameter)."); final RestCompositeConditionDefinitionModel conditions = createCompositeCondition( - List.of(createCompositeCondition(!INVERTED, List.of(createSimpleCondition("size", "greater_than", null))))); + List.of(createCompositeCondition(!INVERTED, List.of(createSimpleCondition("size", "greater_than", ""))))); rule.setConditions(conditions); restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() .include(IS_SHARED) .updateRule(rule.getId(), rule); - //TODO: in next iteration of mapper refactoring this error code will change to 400 - restClient.assertStatusCodeIs(INTERNAL_SERVER_ERROR); + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary("Parameter in condition must not be blank"); } private RestRuleModel createAndSaveRule(String name) diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapper.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapper.java new file mode 100644 index 0000000000..2c7bdb1aab --- /dev/null +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapper.java @@ -0,0 +1,151 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2022 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.rest.api.impl.mapper.rules; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.alfresco.rest.api.model.mapper.RestModelMapper; +import org.alfresco.rest.api.model.rules.CompositeCondition; +import org.alfresco.rest.api.model.rules.ConditionOperator; +import org.alfresco.rest.api.model.rules.SimpleCondition; +import org.alfresco.service.Experimental; +import org.alfresco.service.cmr.action.ActionCondition; +import org.apache.commons.collections.CollectionUtils; + +@Experimental +public class RestRuleCompositeConditionModelMapper implements RestModelMapper +{ + private final RestModelMapper simpleConditionMapper; + + public RestRuleCompositeConditionModelMapper( + RestModelMapper simpleConditionMapper) + { + this.simpleConditionMapper = simpleConditionMapper; + } + + /** + * Converts Action conditions (service POJO) list to composite condition (REST model). + * + * @param actionConditions - list of {@link ActionCondition} service POJOs + * @return {@link CompositeCondition} REST model + */ + @Override + public CompositeCondition toRestModel(final Collection actionConditions) + { + if (CollectionUtils.isEmpty(actionConditions)) + { + return null; + } + final CompositeCondition conditions = new CompositeCondition(); + conditions.setCompositeConditions(new ArrayList<>()); + // group action conditions by inversion flag + actionConditions.stream().filter(Objects::nonNull).collect(Collectors.groupingBy(ActionCondition::getInvertCondition)) + // map action condition sub lists + .forEach((inverted, actionConditionsPart) -> Optional + .ofNullable(ofActionConditions(actionConditionsPart, inverted, ConditionOperator.AND)) + // if composite condition present add to final list + .ifPresent(compositeCondition -> conditions.getCompositeConditions().add(compositeCondition))); + + if (CollectionUtils.isEmpty(conditions.getCompositeConditions())) + { + conditions.setCompositeConditions(null); + } + return conditions; + } + + @Override + public List toServiceModels(final CompositeCondition compositeCondition) + { + final List actionConditions = new ArrayList<>(); + if (compositeCondition == null) + { + return actionConditions; + } + if (CollectionUtils.isNotEmpty(compositeCondition.getSimpleConditions())) + { + compositeCondition.getSimpleConditions() + .forEach(simpleCondition -> actionConditions.add(mapSimpleCondition(simpleCondition, compositeCondition.isInverted()))); + } + if (CollectionUtils.isNotEmpty(compositeCondition.getCompositeConditions())) + { + compositeCondition.getCompositeConditions().forEach(condition -> actionConditions.addAll(toServiceModels(condition))); + } + + return actionConditions; + } + + private ActionCondition mapSimpleCondition(final SimpleCondition simpleCondition, final boolean inverted) + { + final ActionCondition actionCondition = simpleConditionMapper.toServiceModel(simpleCondition); + actionCondition.setInvertCondition(inverted); + return actionCondition; + } + + private CompositeCondition ofActionConditions(final List actionConditions, final boolean inverted, + final ConditionOperator conditionOperator) + { + if (actionConditions == null) + { + return null; + } + return ofSimpleConditions(simpleConditionMapper.toRestModels(actionConditions), inverted, conditionOperator); + } + + /** + * Creates a composite condition instance of simple conditions. + * + * @param simpleConditions - list of {@link SimpleCondition} + * @param inverted - determines if condition should be inverted + * @param conditionOperator - determines the operation, see {@link ConditionOperator} + * @return {@link CompositeCondition} + */ + private CompositeCondition ofSimpleConditions(final List simpleConditions, final boolean inverted, + final ConditionOperator conditionOperator) + { + return of(simpleConditions, null, inverted, conditionOperator); + } + + private CompositeCondition of(final List simpleConditions, final List compositeConditions, + final boolean inverted, final ConditionOperator conditionOperator) + { + if (CollectionUtils.isEmpty(simpleConditions) && CollectionUtils.isEmpty(compositeConditions)) + { + return null; + } + return CompositeCondition.builder() + .inverted(inverted) + .booleanMode(conditionOperator) + .simpleConditions(simpleConditions) + .compositeConditions(compositeConditions) + .create(); + } +} diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java index 1f356e140f..8f16fcb15c 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.Map; import java.util.UUID; +import com.rometools.utils.Strings; import org.alfresco.model.ContentModel; import org.alfresco.repo.action.ActionConditionImpl; import org.alfresco.repo.action.evaluator.CompareMimeTypeEvaluator; @@ -57,6 +58,12 @@ import org.apache.commons.collections.MapUtils; @Experimental public class RestRuleSimpleConditionModelMapper implements RestModelMapper { + static final String CATEGORY_INVALID_MSG = "Category in condition is invalid"; + static final String PARAM_CATEGORY = "category"; + static final String PARAM_MIMETYPE = "mimetype"; + static final String FIELD_NOT_NULL = "Field in condition must not be blank"; + static final String PARAMETER_NOT_NULL = "Parameter in condition must not be blank"; + static final String COMPARATOR_NOT_NULL = "Comparator in condition must not be blank"; private final NamespaceService namespaceService; private final Nodes nodes; @@ -99,14 +106,12 @@ public class RestRuleSimpleConditionModelMapper implements RestModelMapper parameterValues = new HashMap<>(); + final Map parameterValues = new HashMap<>(); String conditionDefinitionId; - String parameter = restModel.getParameter(); + final String parameter = restModel.getParameter(); + checkStringNotBlank(parameter, PARAMETER_NOT_NULL); switch (field) { @@ -118,21 +123,21 @@ public class RestRuleSimpleConditionModelMapper implements RestModelMapper simpleConditionMapper; + private RestModelMapper compositeConditionMapper; public Rule loadRule(org.alfresco.service.cmr.rule.Rule ruleModel, List includes) { - Rule rule = Rule.from(ruleModel, simpleConditionMapper); + Rule rule = Rule.from(ruleModel, compositeConditionMapper); if (includes != null && includes.contains(IS_SHARED)) { NodeRef ruleSet = ruleService.getRuleSetNode(ruleModel.getNodeRef()); @@ -67,9 +67,9 @@ public class RuleLoader this.nodeValidator = nodeValidator; } - public void setSimpleConditionMapper( - RestModelMapper simpleConditionMapper) + public void setCompositeConditionMapper( + RestModelMapper compositeConditionMapper) { - this.simpleConditionMapper = simpleConditionMapper; + this.compositeConditionMapper = compositeConditionMapper; } } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java index df35db897c..56cde7d4a6 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java @@ -33,6 +33,7 @@ import java.util.stream.Collectors; import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.Rules; import org.alfresco.rest.api.model.mapper.RestModelMapper; +import org.alfresco.rest.api.model.rules.CompositeCondition; import org.alfresco.rest.api.model.rules.Rule; import org.alfresco.rest.api.model.rules.RuleSet; import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; @@ -63,7 +64,7 @@ public class RulesImpl implements Rules private RuleLoader ruleLoader; private ActionParameterConverter actionParameterConverter; private ActionPermissionValidator actionPermissionValidator; - private RestModelMapper simpleConditionMapper; + private RestModelMapper compositeConditionMapper; @Override public CollectionWithPagingInfo getRules(final String folderNodeId, @@ -137,7 +138,7 @@ public class RulesImpl implements Rules { throw new InvalidArgumentException(MUST_HAVE_AT_LEAST_ONE_ACTION); } - final org.alfresco.service.cmr.rule.Rule serviceModelRule = rule.toServiceModel(nodes, simpleConditionMapper); + final org.alfresco.service.cmr.rule.Rule serviceModelRule = rule.toServiceModel(nodes, compositeConditionMapper); final CompositeAction compositeAction = (CompositeAction) serviceModelRule.getAction(); compositeAction.getActions().forEach(action -> action.setParameterValues( actionParameterConverter.getConvertedParams(action.getParameterValues(), action.getActionDefinitionName()))); @@ -188,9 +189,9 @@ public class RulesImpl implements Rules this.actionPermissionValidator = actionPermissionValidator; } - public void setSimpleConditionMapper( - RestModelMapper simpleConditionMapper) + public void setCompositeConditionMapper( + RestModelMapper compositeConditionMapper) { - this.simpleConditionMapper = simpleConditionMapper; + this.compositeConditionMapper = compositeConditionMapper; } } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/model/mapper/RestModelMapper.java b/remote-api/src/main/java/org/alfresco/rest/api/model/mapper/RestModelMapper.java index 1a76ab13a5..6972f3061c 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/model/mapper/RestModelMapper.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/model/mapper/RestModelMapper.java @@ -36,8 +36,12 @@ import org.apache.commons.lang3.NotImplementedException; @Experimental public interface RestModelMapper { - R toRestModel(S serviceModel); - S toServiceModel(R restModel); + default R toRestModel(S serviceModel) { + throw new NotImplementedException(); + } + default S toServiceModel(R restModel) { + throw new NotImplementedException(); + } default R toRestModel(Collection serviceModels) { throw new NotImplementedException(); } @@ -50,4 +54,10 @@ public interface RestModelMapper default List toServiceModels(Collection restModels) { return restModels.stream().map(this::toServiceModel).collect(Collectors.toList()); } + default List toRestModels(S serviceModel) { + throw new NotImplementedException(); + } + default List toServiceModels(R restModel) { + throw new NotImplementedException(); + } } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/model/rules/CompositeCondition.java b/remote-api/src/main/java/org/alfresco/rest/api/model/rules/CompositeCondition.java index 30c808fc3f..b773fba7b8 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/model/rules/CompositeCondition.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/model/rules/CompositeCondition.java @@ -26,16 +26,10 @@ package org.alfresco.rest.api.model.rules; -import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Optional; -import java.util.stream.Collectors; -import org.alfresco.rest.api.model.mapper.RestModelMapper; import org.alfresco.service.Experimental; -import org.alfresco.service.cmr.action.ActionCondition; -import org.apache.commons.collections.CollectionUtils; @Experimental public class CompositeCondition @@ -45,91 +39,6 @@ public class CompositeCondition private List compositeConditions; private List simpleConditions; - /** - * Converts Action conditions (service POJO) list to composite condition (REST model). - * - * @param actionConditions - list of {@link ActionCondition} service POJOs - * @return {@link CompositeCondition} REST model - */ - public static CompositeCondition from(final List actionConditions, final RestModelMapper simpleConditionMapper) - { - if (actionConditions == null) - { - return null; - } - - final CompositeCondition conditions = new CompositeCondition(); - conditions.compositeConditions = new ArrayList<>(); - // group action conditions by inversion flag - actionConditions.stream().filter(Objects::nonNull).collect(Collectors.groupingBy(ActionCondition::getInvertCondition)) - // map action condition sub lists - .forEach((inverted, actionConditionsPart) -> Optional.ofNullable(CompositeCondition.ofActionConditions(actionConditionsPart, simpleConditionMapper, inverted, ConditionOperator.AND)) - // if composite condition present add to final list - .ifPresent(compositeCondition -> conditions.compositeConditions.add(compositeCondition))); - - if (conditions.compositeConditions.isEmpty()) { - conditions.compositeConditions = null; - } - - return conditions; - } - - private static CompositeCondition ofActionConditions(final List actionConditions, - final RestModelMapper simpleConditionMapper, - final boolean inverted, final ConditionOperator conditionOperator) - { - if (actionConditions == null) - { - return null; - } - - return ofSimpleConditions(SimpleCondition.listOf(actionConditions, simpleConditionMapper), inverted, conditionOperator); - } - - /** - * Creates a composite condition instance of simple conditions. - * - * @param simpleConditions - list of {@link SimpleCondition} - * @param inverted - determines if condition should be inverted - * @param conditionOperator - determines the operation, see {@link ConditionOperator} - * @return {@link CompositeCondition} - */ - public static CompositeCondition ofSimpleConditions(final List simpleConditions, final boolean inverted, final ConditionOperator conditionOperator) - { - return of(simpleConditions, null, inverted, conditionOperator); - } - - private static CompositeCondition of(final List simpleConditions, final List compositeConditions, - final boolean inverted, final ConditionOperator conditionOperator) - { - if (CollectionUtils.isEmpty(simpleConditions) && CollectionUtils.isEmpty(compositeConditions)) - { - return null; - } - - return builder() - .inverted(inverted) - .booleanMode(conditionOperator) - .simpleConditions(simpleConditions) - .compositeConditions(compositeConditions) - .create(); - } - - public List toServiceModels(final RestModelMapper simpleConditionMapper) - { - final List actionConditions = new ArrayList<>(); - if (CollectionUtils.isNotEmpty(simpleConditions)) - { - simpleConditions.forEach(simpleCondition -> actionConditions.add(simpleCondition.toServiceModel(inverted, simpleConditionMapper))); - } - if (CollectionUtils.isNotEmpty(compositeConditions)) - { - compositeConditions.forEach(compositeCondition -> actionConditions.addAll(compositeCondition.toServiceModels(simpleConditionMapper))); - } - - return actionConditions; - } - public boolean isInverted() { return inverted; diff --git a/remote-api/src/main/java/org/alfresco/rest/api/model/rules/Rule.java b/remote-api/src/main/java/org/alfresco/rest/api/model/rules/Rule.java index 69ee8fd385..b5c05e0c63 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/model/rules/Rule.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/model/rules/Rule.java @@ -63,7 +63,7 @@ public class Rule * @param ruleModel - {@link org.alfresco.service.cmr.rule.Rule} service POJO * @return {@link Rule} REST model */ - public static Rule from(final org.alfresco.service.cmr.rule.Rule ruleModel, final RestModelMapper simpleConditionMapper) + public static Rule from(final org.alfresco.service.cmr.rule.Rule ruleModel, final RestModelMapper compositeConditionMapper) { if (ruleModel == null) { @@ -86,7 +86,7 @@ public class Rule } if (ruleModel.getAction() != null) { - builder.conditions(CompositeCondition.from(ruleModel.getAction().getActionConditions(), simpleConditionMapper)); + builder.conditions(compositeConditionMapper.toRestModel(ruleModel.getAction().getActionConditions())); if (ruleModel.getAction().getCompensatingAction() != null && ruleModel.getAction().getCompensatingAction().getParameterValue(ScriptActionExecuter.PARAM_SCRIPTREF) != null) { builder.errorScript(ruleModel.getAction().getCompensatingAction().getParameterValue(ScriptActionExecuter.PARAM_SCRIPTREF).toString()); @@ -106,7 +106,7 @@ public class Rule * @param nodes The nodes API. * @return The rule service POJO. */ - public org.alfresco.service.cmr.rule.Rule toServiceModel(final Nodes nodes, final RestModelMapper simpleConditionMapper) + public org.alfresco.service.cmr.rule.Rule toServiceModel(final Nodes nodes, final RestModelMapper compositeConditionMapper) { final org.alfresco.service.cmr.rule.Rule ruleModel = new org.alfresco.service.cmr.rule.Rule(); final NodeRef nodeRef = (id != null) ? nodes.validateOrLookupNode(id, null) : null; @@ -129,7 +129,7 @@ public class Rule } if (conditions != null) { - conditions.toServiceModels(simpleConditionMapper).forEach(condition -> ruleModel.getAction().addActionCondition(condition)); + compositeConditionMapper.toServiceModels(conditions).forEach(condition -> ruleModel.getAction().addActionCondition(condition)); } return ruleModel; diff --git a/remote-api/src/main/java/org/alfresco/rest/api/model/rules/SimpleCondition.java b/remote-api/src/main/java/org/alfresco/rest/api/model/rules/SimpleCondition.java index 1037f0f9af..b1a33eaa6f 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/model/rules/SimpleCondition.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/model/rules/SimpleCondition.java @@ -59,49 +59,10 @@ import org.apache.commons.collections.CollectionUtils; @Experimental public class SimpleCondition { - public static final String CATEGORY_INVALID_MSG = "Category in condition is invalid"; - public static final String PARAM_CATEGORY = "category"; - public static final String PARAM_MIMETYPE = "mimetype"; - private String field; private String comparator; private String parameter; - /** - * Converts list of service POJO action conditions to list of REST model simple conditions. - * - * @param actionConditions - list of {@link ActionCondition} service POJOs - * @return list of {@link SimpleCondition} REST models - */ - public static List listOf(final List actionConditions, - final RestModelMapper simpleConditionMapper) - { - if (CollectionUtils.isEmpty(actionConditions)) - { - return null; - } - return simpleConditionMapper.toRestModels(actionConditions); - } - - /** - * Creates simple condition REST model instance from service POJO action condition. - * - * @param actionCondition - {@link ActionCondition} service POJO - * @return {@link SimpleCondition} REST model - */ - public static SimpleCondition from(final ActionCondition actionCondition, - final RestModelMapper simpleConditionMapper) - { - return simpleConditionMapper.toRestModel(actionCondition); - } - - public ActionCondition toServiceModel(final boolean inverted, final RestModelMapper mapper) - { - final ActionCondition actionCondition = mapper.toServiceModel(this); - actionCondition.setInvertCondition(inverted); - return actionCondition; - } - public String getField() { return field; diff --git a/remote-api/src/main/resources/alfresco/public-rest-context.xml b/remote-api/src/main/resources/alfresco/public-rest-context.xml index feb432b8f9..bfe4809e9b 100644 --- a/remote-api/src/main/resources/alfresco/public-rest-context.xml +++ b/remote-api/src/main/resources/alfresco/public-rest-context.xml @@ -887,7 +887,7 @@ - + @@ -911,7 +911,7 @@ - + @@ -951,6 +951,9 @@ + + + diff --git a/remote-api/src/test/java/org/alfresco/rest/api/RulesUnitTests.java b/remote-api/src/test/java/org/alfresco/rest/api/RulesUnitTests.java index ab63a58c53..fa55c4e9d4 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/RulesUnitTests.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/RulesUnitTests.java @@ -26,6 +26,7 @@ package org.alfresco.rest.api; +import org.alfresco.rest.api.impl.mapper.rules.RestRuleCompositeConditionModelMapperTest; import org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapperTest; import org.alfresco.rest.api.impl.rules.ActionParameterConverterTest; import org.alfresco.rest.api.impl.rules.ActionPermissionValidatorTest; @@ -33,10 +34,8 @@ import org.alfresco.rest.api.impl.rules.NodeValidatorTest; import org.alfresco.rest.api.impl.rules.RuleLoaderTest; import org.alfresco.rest.api.impl.rules.RuleSetsImplTest; import org.alfresco.rest.api.model.rules.ActionTest; -import org.alfresco.rest.api.model.rules.CompositeConditionTest; import org.alfresco.rest.api.impl.rules.RulesImplTest; import org.alfresco.rest.api.model.rules.RuleTest; -import org.alfresco.rest.api.model.rules.SimpleConditionTest; import org.alfresco.rest.api.nodes.NodeRulesRelationTest; import org.alfresco.service.Experimental; import org.junit.runner.RunWith; @@ -51,12 +50,11 @@ import org.junit.runners.Suite; NodeValidatorTest.class, RuleTest.class, ActionTest.class, - SimpleConditionTest.class, - CompositeConditionTest.class, RuleLoaderTest.class, ActionParameterConverterTest.class, ActionPermissionValidatorTest.class, - RestRuleSimpleConditionModelMapperTest.class + RestRuleSimpleConditionModelMapperTest.class, + RestRuleCompositeConditionModelMapperTest.class }) public class RulesUnitTests { diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapperTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapperTest.java new file mode 100644 index 0000000000..7ffc69d5aa --- /dev/null +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleCompositeConditionModelMapperTest.java @@ -0,0 +1,238 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2022 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.rest.api.impl.mapper.rules; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.IntStream; + +import org.alfresco.repo.action.ActionConditionImpl; +import org.alfresco.repo.action.evaluator.ComparePropertyValueEvaluator; +import org.alfresco.rest.api.model.mapper.RestModelMapper; +import org.alfresco.rest.api.model.rules.CompositeCondition; +import org.alfresco.rest.api.model.rules.ConditionOperator; +import org.alfresco.rest.api.model.rules.SimpleCondition; +import org.alfresco.service.Experimental; +import org.alfresco.service.cmr.action.ActionCondition; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@Experimental +@RunWith(MockitoJUnitRunner.class) +public class RestRuleCompositeConditionModelMapperTest +{ + + @Mock + private RestModelMapper simpleConditionMapperMock; + + @InjectMocks + RestRuleCompositeConditionModelMapper objectUnderTest; + + @Test + public void testToRestModel() + { + final List actionConditions = List.of( + createActionCondition("value1"), + createActionCondition("value3"), + createActionCondition("value2", true) + ); + final List simpleConditions = List.of( + createSimpleCondition("value1"), + createSimpleCondition("value3"), + createSimpleCondition("value2") + ); + + final CompositeCondition expectedCompositeCondition = createCompositeCondition(List.of( + createCompositeCondition(false, simpleConditions.subList(0,2)), + createCompositeCondition(true, simpleConditions.subList(2,3)) + )); + + when(simpleConditionMapperMock.toRestModels(actionConditions.subList(0,2))).thenReturn(simpleConditions.subList(0,2)); + when(simpleConditionMapperMock.toRestModels(actionConditions.subList(2,3))).thenReturn(simpleConditions.subList(2,3)); + + // when + final CompositeCondition actualCompositeCondition = objectUnderTest.toRestModel(actionConditions); + + assertThat(actualCompositeCondition).isNotNull().usingRecursiveComparison().isEqualTo(expectedCompositeCondition); + } + + @Test + public void testToRestModel_fromEmptyList() + { + final List actionConditions = Collections.emptyList(); + + // when + final CompositeCondition actualCompositeCondition = objectUnderTest.toRestModel(actionConditions); + + assertThat(actualCompositeCondition).isNull(); + } + + @Test + public void testToRestModel_fromNullValue() + { + // when + final CompositeCondition actualCompositeCondition = objectUnderTest.toRestModel((Collection) null); + + assertThat(actualCompositeCondition).isNull(); + } + + @Test + public void testToRestModel_fromListContainingNull() + { + final List actionConditions = new ArrayList<>(); + actionConditions.add(null); + final CompositeCondition expectedCompositeCondition = CompositeCondition.builder().create(); + + // when + final CompositeCondition actualCompositeCondition = objectUnderTest.toRestModel(actionConditions); + + assertThat(actualCompositeCondition).isNotNull().usingRecursiveComparison().isEqualTo(expectedCompositeCondition); + } + + @Test + public void testToServiceModels() { + final List simpleConditions = List.of( + createSimpleCondition("value1"), + createSimpleCondition("value3"), + createSimpleCondition("value2") + ); + final CompositeCondition compositeCondition = createCompositeCondition(List.of( + createCompositeCondition(false, simpleConditions.subList(0,2)), + createCompositeCondition(true, simpleConditions.subList(2,3)) + )); + final List actionConditions = List.of( + createActionCondition("value1"), + createActionCondition("value3"), + createActionCondition("value2", true) + ); + + IntStream.rangeClosed(0, 2) + .forEach(i -> when(simpleConditionMapperMock.toServiceModel(simpleConditions.get(i))).thenReturn(actionConditions.get(i))); + + final List actualActionConditions = objectUnderTest.toServiceModels(compositeCondition); + assertThat(actualActionConditions).isEqualTo(actionConditions); + } + + @Test + public void testToServiceModels_simpleNonInvertedConditionsOnly() { + final List simpleConditions = List.of( + createSimpleCondition("value1"), + createSimpleCondition("value2"), + createSimpleCondition("value3") + ); + final CompositeCondition compositeCondition = createCompositeCondition(false, simpleConditions); + final List actionConditions = List.of( + createActionCondition("value1"), + createActionCondition("value2"), + createActionCondition("value3") + ); + + IntStream.rangeClosed(0, 2) + .forEach(i -> when(simpleConditionMapperMock.toServiceModel(simpleConditions.get(i))).thenReturn(actionConditions.get(i))); + + final List actualActionConditions = objectUnderTest.toServiceModels(compositeCondition); + assertThat(actualActionConditions).isEqualTo(actionConditions); + } + + @Test + public void testToServiceModels_nullSimpleConditions() { + final CompositeCondition compositeCondition = createCompositeCondition(false, null); + + final List actualActionConditions = objectUnderTest.toServiceModels(compositeCondition); + assertThat(actualActionConditions).isNotNull().isEmpty(); + } + + @Test + public void testToServiceModels_emptyCompositeCondition() { + final CompositeCondition compositeCondition = CompositeCondition.builder().create(); + + final List actualActionConditions = objectUnderTest.toServiceModels(compositeCondition); + assertThat(actualActionConditions).isNotNull().isEmpty(); + } + + @Test + public void testToServiceModels_nullCompositeCondition() { + final CompositeCondition compositeCondition = null; + + final List actualActionConditions = objectUnderTest.toServiceModels(compositeCondition); + assertThat(actualActionConditions).isNotNull().isEmpty(); + } + + private static ActionCondition createActionCondition(final String value) + { + return createActionCondition(value, false); + } + + private static ActionCondition createActionCondition(final String value, final boolean inverted) + { + final ActionCondition actionCondition = new ActionConditionImpl("fake-id", ComparePropertyValueEvaluator.NAME); + actionCondition.setInvertCondition(inverted); + final Map parameterValues = new HashMap<>(); + parameterValues.put(ComparePropertyValueEvaluator.PARAM_CONTENT_PROPERTY, "content-property"); + parameterValues.put(ComparePropertyValueEvaluator.PARAM_OPERATION, "operation"); + parameterValues.put(ComparePropertyValueEvaluator.PARAM_VALUE, value); + actionCondition.setParameterValues(parameterValues); + return actionCondition; + } + + private static SimpleCondition createSimpleCondition(final String value) { + return SimpleCondition.builder() + .field("content-property") + .comparator("operation") + .parameter(value) + .create(); + } + + private static CompositeCondition createCompositeCondition(final List compositeConditions) { + return createCompositeCondition(false, ConditionOperator.AND, compositeConditions, null); + } + + private static CompositeCondition createCompositeCondition(final boolean inverted, final List simpleConditions) { + return createCompositeCondition(inverted, ConditionOperator.AND, null, simpleConditions); + } + + private static CompositeCondition createCompositeCondition(final boolean inverted, final ConditionOperator conditionOperator, + final List compositeConditions, final List simpleConditions) { + return CompositeCondition.builder() + .inverted(inverted) + .booleanMode(conditionOperator) + .compositeConditions(compositeConditions) + .simpleConditions(simpleConditions) + .create(); + } +} diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java index 60f4409846..ee2b4194a4 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java @@ -26,19 +26,25 @@ package org.alfresco.rest.api.impl.mapper.rules; +import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.COMPARATOR_NOT_NULL; +import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.FIELD_NOT_NULL; +import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.PARAMETER_NOT_NULL; +import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.PARAM_CATEGORY; +import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.PARAM_MIMETYPE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.BDDMockito.given; import java.io.Serializable; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import junit.framework.TestCase; import org.alfresco.model.ContentModel; import org.alfresco.repo.action.ActionConditionImpl; import org.alfresco.repo.action.evaluator.CompareMimeTypeEvaluator; @@ -54,6 +60,7 @@ import org.alfresco.repo.action.evaluator.compare.ComparePropertyValueOperation; import org.alfresco.repo.action.evaluator.compare.ContentPropertyName; import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.model.rules.SimpleCondition; +import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; import org.alfresco.service.Experimental; import org.alfresco.service.cmr.action.ActionCondition; import org.alfresco.service.cmr.repository.NodeRef; @@ -69,7 +76,7 @@ import org.mockito.junit.MockitoJUnitRunner; @Experimental @RunWith(MockitoJUnitRunner.class) -public class RestRuleSimpleConditionModelMapperTest extends TestCase +public class RestRuleSimpleConditionModelMapperTest { private static final boolean NULL_RESULT = true; private static final String PARAMETER_DEFAULT = "value"; @@ -156,7 +163,8 @@ public class RestRuleSimpleConditionModelMapperTest extends TestCase public void testToRestModelListOfNullActionConditions() { // when - assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> objectUnderTest.toRestModels(null)); + assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> objectUnderTest.toRestModels( + (Collection) null)); } @Test @@ -212,7 +220,7 @@ public class RestRuleSimpleConditionModelMapperTest extends TestCase @Test public void testToServiceModel_compareMimetype() { - final SimpleCondition simpleCondition = createSimpleCondition(SimpleCondition.PARAM_MIMETYPE); + final SimpleCondition simpleCondition = createSimpleCondition(PARAM_MIMETYPE); // when final ActionCondition actualActionCondition = objectUnderTest.toServiceModel(simpleCondition); @@ -264,7 +272,7 @@ public class RestRuleSimpleConditionModelMapperTest extends TestCase @Test public void testToServiceModel_inCategory() { - final SimpleCondition simpleCondition = createSimpleCondition(SimpleCondition.PARAM_CATEGORY); + final SimpleCondition simpleCondition = createSimpleCondition(PARAM_CATEGORY); final NodeRef defaultNodeRef = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, PARAMETER_DEFAULT); given(nodesMock.validateOrLookupNode(PARAMETER_DEFAULT, null)).willReturn(defaultNodeRef); @@ -297,6 +305,67 @@ public class RestRuleSimpleConditionModelMapperTest extends TestCase .isEqualTo(expectedActionCondition); } + @Test + public void testToServiceModel_nullOrBlankParameter() + { + final SimpleCondition simpleConditionNullParam = createSimpleCondition(IsSubTypeEvaluator.PARAM_TYPE, null); + + // when + assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionNullParam)) + .isInstanceOf(InvalidArgumentException.class) + .hasMessageContaining(PARAMETER_NOT_NULL); + + final SimpleCondition simpleConditionEmptyParam = createSimpleCondition(IsSubTypeEvaluator.PARAM_TYPE, " "); + + assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionEmptyParam)) + .isInstanceOf(InvalidArgumentException.class) + .hasMessageContaining(PARAMETER_NOT_NULL); + } + + @Test + public void testToServiceModel_nullOrEmptyField() + { + final SimpleCondition simpleConditionNullField = createSimpleCondition(null); + + // when + assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionNullField)) + .isInstanceOf(InvalidArgumentException.class) + .hasMessageContaining(FIELD_NOT_NULL); + + final SimpleCondition simpleConditionEmptyField = createSimpleCondition(""); + + // when + assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionEmptyField)) + .isInstanceOf(InvalidArgumentException.class) + .hasMessageContaining(FIELD_NOT_NULL); + } + + @Test + public void testToServiceModel_nullOrEmptyComparatorWhenRequired() + { + final SimpleCondition simpleConditionNullComparator = SimpleCondition.builder() + .field("size") + .comparator(null) + .parameter("65000") + .create(); + + // when + assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionNullComparator)) + .isInstanceOf(InvalidArgumentException.class) + .hasMessageContaining(COMPARATOR_NOT_NULL); + + final SimpleCondition simpleConditionEmptyComparator = SimpleCondition.builder() + .field("size") + .comparator(" ") + .parameter("65000") + .create(); + + // when + assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionEmptyComparator)) + .isInstanceOf(InvalidArgumentException.class) + .hasMessageContaining(COMPARATOR_NOT_NULL); + } + private static ActionCondition createActionCondition(final String actionDefinitionName) { return new ActionConditionImpl("fake-id", actionDefinitionName, createParameterValues()); diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java index 5b64a1bdcd..68126b01aa 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java @@ -51,6 +51,7 @@ import org.alfresco.repo.action.CompositeActionImpl; import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.model.rules.Action; import org.alfresco.rest.api.model.mapper.RestModelMapper; +import org.alfresco.rest.api.model.rules.CompositeCondition; import org.alfresco.rest.api.model.rules.Rule; import org.alfresco.rest.api.model.rules.SimpleCondition; import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; @@ -91,7 +92,7 @@ public class RulesImplTest extends TestCase @Mock private Nodes nodesMock; @Mock - private RestModelMapper simpleConditionMapperMock; + private RestModelMapper compositeConditionMapperMock; @Mock private NodeValidator nodeValidatorMock; @Mock @@ -297,7 +298,7 @@ public class RulesImplTest extends TestCase public void testCreateRules() { List ruleList = List.of(ruleMock); - given(ruleMock.toServiceModel(nodesMock, simpleConditionMapperMock)).willReturn(serviceRuleMock); + given(ruleMock.toServiceModel(nodesMock, compositeConditionMapperMock)).willReturn(serviceRuleMock); given(ruleMock.getActions()).willReturn(List.of(actionMock)); given(serviceRuleMock.getAction()).willReturn(compositeAction); given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(arg -> arg.getArguments()[1]); @@ -314,7 +315,7 @@ public class RulesImplTest extends TestCase then(actionParameterConverterMock).shouldHaveNoMoreInteractions(); then(actionPermissionValidatorMock).should().validateRulePermissions(serviceRuleMock); then(actionPermissionValidatorMock).shouldHaveNoMoreInteractions(); - then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, ruleMock.toServiceModel(nodesMock, simpleConditionMapperMock)); + then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, ruleMock.toServiceModel(nodesMock, compositeConditionMapperMock)); then(ruleServiceMock).shouldHaveNoMoreInteractions(); List expected = List.of(ruleMock); assertThat(actual).isEqualTo(expected); @@ -327,7 +328,7 @@ public class RulesImplTest extends TestCase public void testCreateRules_defaultRuleSet() { List ruleList = List.of(ruleMock); - given(ruleMock.toServiceModel(nodesMock, simpleConditionMapperMock)).willReturn(serviceRuleMock); + given(ruleMock.toServiceModel(nodesMock, compositeConditionMapperMock)).willReturn(serviceRuleMock); given(ruleMock.getActions()).willReturn(List.of(actionMock)); given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(arg -> arg.getArguments()[1]); given(ruleLoaderMock.loadRule(serviceRuleMock, INCLUDE)).willReturn(ruleMock); @@ -343,7 +344,7 @@ public class RulesImplTest extends TestCase then(actionParameterConverterMock).shouldHaveNoMoreInteractions(); then(actionPermissionValidatorMock).should().validateRulePermissions(serviceRuleMock); then(actionPermissionValidatorMock).shouldHaveNoMoreInteractions(); - then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, ruleMock.toServiceModel(nodesMock, simpleConditionMapperMock)); + then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, ruleMock.toServiceModel(nodesMock, compositeConditionMapperMock)); then(ruleServiceMock).shouldHaveNoMoreInteractions(); List expected = List.of(ruleMock); assertThat(actual).isEqualTo(expected); @@ -374,7 +375,7 @@ public class RulesImplTest extends TestCase given(ruleBodyMock.getActions()).willReturn(List.of(actionMock)); ruleBodyList.add(ruleBodyMock); org.alfresco.service.cmr.rule.Rule serviceRuleMockInner = mock(org.alfresco.service.cmr.rule.Rule.class); - given(ruleBodyMock.toServiceModel(nodesMock, simpleConditionMapperMock)).willReturn(serviceRuleMockInner); + given(ruleBodyMock.toServiceModel(nodesMock, compositeConditionMapperMock)).willReturn(serviceRuleMockInner); final CompositeAction compositeActionInner = new CompositeActionImpl(RULE_NODE_REF, "compositeActionInnerId"); compositeActionInner.addAction(new ActionImpl(FOLDER_NODE_REF, "actionInnerId", ACTION_DEFINITION_NAME, DUMMY_PARAMS)); given(serviceRuleMockInner.getAction()).willReturn(compositeActionInner); @@ -393,8 +394,9 @@ public class RulesImplTest extends TestCase then(nodeValidatorMock).shouldHaveNoMoreInteractions(); for (Rule ruleBody : ruleBodyList) { - then(actionPermissionValidatorMock).should().validateRulePermissions(ruleBody.toServiceModel(nodesMock, simpleConditionMapperMock)); - then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, ruleBody.toServiceModel(nodesMock, simpleConditionMapperMock)); + then(actionPermissionValidatorMock).should().validateRulePermissions(ruleBody.toServiceModel(nodesMock, + compositeConditionMapperMock)); + then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, ruleBody.toServiceModel(nodesMock, compositeConditionMapperMock)); } then(actionParameterConverterMock).should(times(3)).getConvertedParams(DUMMY_PARAMS, ACTION_DEFINITION_NAME); then(actionParameterConverterMock).shouldHaveNoMoreInteractions(); @@ -468,7 +470,7 @@ public class RulesImplTest extends TestCase @Test public void testUpdateRuleById() { - given(ruleMock.toServiceModel(nodesMock, simpleConditionMapperMock)).willReturn(serviceRuleMock); + given(ruleMock.toServiceModel(nodesMock, compositeConditionMapperMock)).willReturn(serviceRuleMock); given(ruleMock.getActions()).willReturn(List.of(actionMock)); given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(a -> a.getArguments()[1]); given(serviceRuleMock.getAction()).willReturn(compositeAction); diff --git a/remote-api/src/test/java/org/alfresco/rest/api/model/rules/CompositeConditionTest.java b/remote-api/src/test/java/org/alfresco/rest/api/model/rules/CompositeConditionTest.java deleted file mode 100644 index c1b3c11e41..0000000000 --- a/remote-api/src/test/java/org/alfresco/rest/api/model/rules/CompositeConditionTest.java +++ /dev/null @@ -1,193 +0,0 @@ -/* - * #%L - * Alfresco Remote API - * %% - * Copyright (C) 2005 - 2022 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ - -package org.alfresco.rest.api.model.rules; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; - -import java.io.Serializable; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import org.alfresco.repo.action.ActionConditionImpl; -import org.alfresco.repo.action.evaluator.ComparePropertyValueEvaluator; -import org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper; -import org.alfresco.rest.api.model.mapper.RestModelMapper; -import org.alfresco.service.Experimental; -import org.alfresco.service.cmr.action.ActionCondition; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; - -@Experimental -@RunWith(MockitoJUnitRunner.class) -public class CompositeConditionTest -{ - - private final RestModelMapper simpleConditionMapper = mock(RestRuleSimpleConditionModelMapper.class); - - @Test - public void testFrom() - { - final List actionConditions = List.of( - createActionCondition("value1"), - createActionCondition("value3"), - createActionCondition("value2", true) - ); - final List simpleConditions = List.of( - createSimpleCondition("value1"), - createSimpleCondition("value3"), - createSimpleCondition("value2") - ); - - final CompositeCondition expectedCompositeCondition = createCompositeCondition(List.of( - createCompositeCondition(false, simpleConditions.subList(0,2)), - createCompositeCondition(true, simpleConditions.subList(2,3)) - )); - - Mockito.when(simpleConditionMapper.toRestModels(actionConditions.subList(0,2))).thenReturn(simpleConditions.subList(0,2)); - Mockito.when(simpleConditionMapper.toRestModels(actionConditions.subList(2,3))).thenReturn(simpleConditions.subList(2,3)); - - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.from(actionConditions, simpleConditionMapper); - - assertThat(actualCompositeCondition).isNotNull().usingRecursiveComparison().isEqualTo(expectedCompositeCondition); - } - - @Test - public void testFromEmptyList() - { - final List actionConditions = Collections.emptyList(); - final CompositeCondition expectedCompositeCondition = CompositeCondition.builder().create(); - - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.from(actionConditions, simpleConditionMapper); - - assertThat(actualCompositeCondition).isNotNull().usingRecursiveComparison().isEqualTo(expectedCompositeCondition); - } - - @Test - public void testFromNullValue() - { - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.from(null, simpleConditionMapper); - - assertThat(actualCompositeCondition).isNull(); - } - - @Test - public void testFromListContainingNull() - { - final List actionConditions = new ArrayList<>(); - actionConditions.add(null); - final CompositeCondition expectedCompositeCondition = CompositeCondition.builder().create(); - - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.from(actionConditions, simpleConditionMapper); - - assertThat(actualCompositeCondition).isNotNull().usingRecursiveComparison().isEqualTo(expectedCompositeCondition); - } - - @Test - public void testOfSimpleConditions() - { - final List simpleConditions = List.of(SimpleCondition.builder().field("field").comparator("comparator").parameter("param").create()); - final boolean inverted = true; - final ConditionOperator conditionOperator = ConditionOperator.OR; - final CompositeCondition expectedCondition = createCompositeCondition(inverted, conditionOperator, null, simpleConditions); - - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.ofSimpleConditions(simpleConditions, inverted, conditionOperator); - - assertThat(actualCompositeCondition).isNotNull().usingRecursiveComparison().isEqualTo(expectedCondition); - } - - @Test - public void testOfEmptySimpleConditions() - { - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.ofSimpleConditions(Collections.emptyList(), false, ConditionOperator.AND); - - assertThat(actualCompositeCondition).isNull(); - } - - @Test - public void testOfNullSimpleConditions() - { - // when - final CompositeCondition actualCompositeCondition = CompositeCondition.ofSimpleConditions(null, false, ConditionOperator.AND); - - assertThat(actualCompositeCondition).isNull(); - } - - private static ActionCondition createActionCondition(final String value) - { - return createActionCondition(value, false); - } - - private static ActionCondition createActionCondition(final String value, final boolean inverted) - { - final ActionCondition actionCondition = new ActionConditionImpl("fake-id", ComparePropertyValueEvaluator.NAME); - actionCondition.setInvertCondition(inverted); - final Map parameterValues = new HashMap<>(); - parameterValues.put(ComparePropertyValueEvaluator.PARAM_CONTENT_PROPERTY, "content-property"); - parameterValues.put(ComparePropertyValueEvaluator.PARAM_OPERATION, "operation"); - parameterValues.put(ComparePropertyValueEvaluator.PARAM_VALUE, value); - actionCondition.setParameterValues(parameterValues); - return actionCondition; - } - - private static SimpleCondition createSimpleCondition(final String value) { - return SimpleCondition.builder() - .field("content-property") - .comparator("operation") - .parameter(value) - .create(); - } - - private static CompositeCondition createCompositeCondition(final List compositeConditions) { - return createCompositeCondition(false, ConditionOperator.AND, compositeConditions, null); - } - - private static CompositeCondition createCompositeCondition(final boolean inverted, final List simpleConditions) { - return createCompositeCondition(inverted, ConditionOperator.AND, null, simpleConditions); - } - - private static CompositeCondition createCompositeCondition(final boolean inverted, final ConditionOperator conditionOperator, - final List compositeConditions, final List simpleConditions) { - return CompositeCondition.builder() - .inverted(inverted) - .booleanMode(conditionOperator) - .compositeConditions(compositeConditions) - .simpleConditions(simpleConditions) - .create(); - } -} diff --git a/remote-api/src/test/java/org/alfresco/rest/api/model/rules/RuleTest.java b/remote-api/src/test/java/org/alfresco/rest/api/model/rules/RuleTest.java index 9a94c22924..12959d6af6 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/model/rules/RuleTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/model/rules/RuleTest.java @@ -37,6 +37,7 @@ import org.alfresco.repo.action.ActionConditionImpl; import org.alfresco.repo.action.ActionImpl; import org.alfresco.repo.action.executer.ScriptActionExecuter; import org.alfresco.rest.api.Nodes; +import org.alfresco.rest.api.impl.mapper.rules.RestRuleCompositeConditionModelMapper; import org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper; import org.alfresco.rest.api.model.mapper.RestModelMapper; import org.alfresco.service.Experimental; @@ -44,7 +45,6 @@ import org.alfresco.service.cmr.action.ActionCondition; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.rule.RuleType; -import org.alfresco.service.namespace.NamespaceService; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; @@ -63,7 +63,7 @@ public class RuleTest private static final String ACTION_DEFINITION_NAME = "action-def-name"; private static final String ERROR_SCRIPT = "error-script-ref"; - private final RestModelMapper simpleConditionMapper = mock(RestRuleSimpleConditionModelMapper.class); + private final RestModelMapper compositeConditionMapper = mock(RestRuleCompositeConditionModelMapper.class); @Test public void testFrom() @@ -72,7 +72,7 @@ public class RuleTest final Rule expectedRule = createRuleWithDefaultValues(); // when - final Rule actualRule = Rule.from(ruleModel, simpleConditionMapper); + final Rule actualRule = Rule.from(ruleModel, compositeConditionMapper); assertThat(actualRule).isNotNull().usingRecursiveComparison().isEqualTo(expectedRule); @@ -85,7 +85,7 @@ public class RuleTest final Rule expectedRule = Rule.builder().enabled(true).create(); // when - final Rule actualRule = Rule.from(ruleModel, simpleConditionMapper); + final Rule actualRule = Rule.from(ruleModel, compositeConditionMapper); assertThat(actualRule).isNotNull().usingRecursiveComparison().isEqualTo(expectedRule); @@ -101,7 +101,7 @@ public class RuleTest final org.alfresco.service.cmr.action.Action expectedCompensatingActionModel = createCompensatingActionModel(); // when - final org.alfresco.service.cmr.rule.Rule actualRuleModel = rule.toServiceModel(nodesMock, simpleConditionMapper); + final org.alfresco.service.cmr.rule.Rule actualRuleModel = rule.toServiceModel(nodesMock, compositeConditionMapper); then(nodesMock).should().validateOrLookupNode(RULE_ID, null); then(nodesMock).shouldHaveNoMoreInteractions(); @@ -126,7 +126,7 @@ public class RuleTest expectedRuleModel.setRuleDisabled(true); // when - final org.alfresco.service.cmr.rule.Rule actualRuleModel = rule.toServiceModel(nodesMock, simpleConditionMapper); + final org.alfresco.service.cmr.rule.Rule actualRuleModel = rule.toServiceModel(nodesMock, compositeConditionMapper); then(nodesMock).shouldHaveNoInteractions(); assertThat(actualRuleModel) @@ -146,7 +146,7 @@ public class RuleTest .asynchronous(RULE_ASYNC) .triggers(List.of(RuleTrigger.INBOUND, RuleTrigger.UPDATE)) .errorScript(ERROR_SCRIPT) - .conditions(CompositeCondition.from(Collections.emptyList(), simpleConditionMapper)) + .conditions(compositeConditionMapper.toRestModel(Collections.emptyList())) .create(); } diff --git a/remote-api/src/test/java/org/alfresco/rest/api/model/rules/SimpleConditionTest.java b/remote-api/src/test/java/org/alfresco/rest/api/model/rules/SimpleConditionTest.java deleted file mode 100644 index 2f75032dfc..0000000000 --- a/remote-api/src/test/java/org/alfresco/rest/api/model/rules/SimpleConditionTest.java +++ /dev/null @@ -1,169 +0,0 @@ -/* - * #%L - * Alfresco Remote API - * %% - * Copyright (C) 2005 - 2022 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ - -package org.alfresco.rest.api.model.rules; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; -import static org.mockito.BDDMockito.then; -import static org.mockito.Mockito.mock; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; - -import org.alfresco.repo.action.ActionConditionImpl; -import org.alfresco.repo.action.evaluator.ComparePropertyValueEvaluator; -import org.alfresco.repo.action.evaluator.compare.ComparePropertyValueOperation; -import org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper; -import org.alfresco.rest.api.model.mapper.RestModelMapper; -import org.alfresco.service.Experimental; -import org.alfresco.service.cmr.action.ActionCondition; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@Experimental -@RunWith(MockitoJUnitRunner.class) -public class SimpleConditionTest -{ - private static final boolean INVERTED = true; - private static final String VALUE = "value"; - private static final String KEY = "key"; - - private final RestModelMapper simpleConditionMapperMock = mock(RestRuleSimpleConditionModelMapper.class); - - @Test - public void testFrom() - { - final ActionCondition actionCondition = createActionCondition(ComparePropertyValueEvaluator.NAME); - final SimpleCondition simpleConditionMock = mock(SimpleCondition.class); - given(simpleConditionMapperMock.toRestModel(actionCondition)).willReturn(simpleConditionMock); - - //when - final SimpleCondition actualSimpleCondition = SimpleCondition.from(actionCondition, simpleConditionMapperMock); - - then(simpleConditionMapperMock).should().toRestModel(actionCondition); - then(simpleConditionMapperMock).shouldHaveNoMoreInteractions(); - assertThat(actualSimpleCondition).isEqualTo(simpleConditionMock); - } - - @Test - public void testListOf() - { - final List actionConditionsMock = mock(List.class); - final List simpleConditionsMock = mock(List.class); - given(simpleConditionMapperMock.toRestModels(actionConditionsMock)).willReturn(simpleConditionsMock); - - // when - final List actualSimpleConditions = SimpleCondition.listOf(actionConditionsMock, simpleConditionMapperMock); - - then(simpleConditionMapperMock).should().toRestModels(actionConditionsMock); - then(simpleConditionMapperMock).shouldHaveNoMoreInteractions(); - assertThat(actualSimpleConditions) - .isNotNull() - .containsExactlyElementsOf(simpleConditionsMock); - } - - @Test - public void testListOfEmptyActionConditions() - { - // when - final List actualSimpleConditions = SimpleCondition.listOf(Collections.emptyList(), simpleConditionMapperMock); - - assertThat(actualSimpleConditions).isNull(); - } - - @Test - public void testListOfNullActionConditions() - { - // when - final List actualSimpleConditions = SimpleCondition.listOf(null, simpleConditionMapperMock); - - then(simpleConditionMapperMock).shouldHaveNoInteractions(); - assertThat(actualSimpleConditions).isNull(); - } - - @Test - public void testListOfActionConditionsContainingNull() - { - final List actionConditions = new ArrayList<>(); - actionConditions.add(null); - - // when - final List actualSimpleConditions = SimpleCondition.listOf(actionConditions, simpleConditionMapperMock); - - then(simpleConditionMapperMock).should().toRestModels(actionConditions); - then(simpleConditionMapperMock).shouldHaveNoMoreInteractions(); - assertThat(actualSimpleConditions).isNotNull().isEmpty(); - } - - @Test - public void testToServiceModel_notInverted() - { - final SimpleCondition simpleCondition = createSimpleCondition("field"); - final ActionCondition actionCondition = createActionCondition(ComparePropertyValueEvaluator.NAME); - given(simpleConditionMapperMock.toServiceModel(simpleCondition)).willReturn(actionCondition); - - // when - final ActionCondition actualActionCondition = simpleCondition.toServiceModel(!INVERTED, simpleConditionMapperMock); - - assertThat(actualActionCondition).isEqualTo(actionCondition); - } - - @Test - public void testToServiceModel_inverted() - { - final SimpleCondition simpleCondition = createSimpleCondition("field"); - final ActionCondition actionCondition = createActionCondition(ComparePropertyValueEvaluator.NAME); - given(simpleConditionMapperMock.toServiceModel(simpleCondition)).willReturn(actionCondition); - - // when - final ActionCondition actualActionCondition = simpleCondition.toServiceModel(INVERTED, simpleConditionMapperMock); - - assertThat(actualActionCondition).isEqualTo(actionCondition); - } - - private static ActionCondition createActionCondition(final String actionDefinitionName) - { - return new ActionConditionImpl("fake-id", actionDefinitionName, Map.of(KEY, VALUE)); - } - - private static SimpleCondition createSimpleCondition(final String field) - { - return createSimpleCondition(field, VALUE); - } - - private static SimpleCondition createSimpleCondition(final String field, final String parameter) - { - return SimpleCondition.builder() - .field(field) - .comparator(ComparePropertyValueOperation.EQUALS.toString().toLowerCase()) - .parameter(parameter) - .create(); - } -}