This is an automated email from the ASF dual-hosted git repository. diru pushed a commit to branch issue/SLING-11383 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-extension-apiregions.git
commit 07661fb59f9cef303e4262b5d04a93463dde86e2 Author: Dirk Rudolph <[email protected]> AuthorDate: Wed Jun 8 12:49:47 2022 +0200 SLING-11383: use per configuration mode for factoyconfig validation --- .../api/config/validation/FeatureValidator.java | 10 ++-- .../config/validation/FeatureValidatorTest.java | 60 ++++++++++++++++++++-- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java index ac89b6b..4609b64 100644 --- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java +++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java @@ -37,6 +37,7 @@ import org.apache.sling.feature.extension.apiregions.api.config.ConfigurationApi import org.apache.sling.feature.extension.apiregions.api.config.ConfigurationDescription; import org.apache.sling.feature.extension.apiregions.api.config.FactoryConfigurationDescription; import org.apache.sling.feature.extension.apiregions.api.config.FrameworkPropertyDescription; +import org.apache.sling.feature.extension.apiregions.api.config.Mode; import org.apache.sling.feature.extension.apiregions.api.config.Operation; import org.apache.sling.feature.extension.apiregions.api.config.Region; import org.osgi.util.converter.Converter; @@ -137,23 +138,24 @@ public class FeatureValidator { if ( config.isFactoryConfiguration() ) { final FactoryConfigurationDescription desc = api.getFactoryConfigurationDescriptions().get(config.getFactoryPid()); if ( desc != null ) { + final Mode validationMode = desc.getMode() != null ? desc.getMode() : api.getMode(); final ConfigurationValidationResult r = configurationValidator.validate(config, desc, regionInfo.region, api.getMode()); result.getConfigurationResults().put(config.getPid(), r); if ( regionInfo.region != Region.INTERNAL ) { if ( desc.getOperations().isEmpty() ) { - ConfigurationValidator.setResult(r, api.getMode(), desc, "No operations allowed for " + + ConfigurationValidator.setResult(r, validationMode, desc, "No operations allowed for " + "factory configuration"); } else { if ( regionInfo.isUpdate && !desc.getOperations().contains(Operation.UPDATE)) { - ConfigurationValidator.setResult(r, api.getMode(), desc, "Updating of factory " + + ConfigurationValidator.setResult(r, validationMode, desc, "Updating of factory " + "configuration is not allowed"); } else if ( !regionInfo.isUpdate && !desc.getOperations().contains(Operation.CREATE)) { - ConfigurationValidator.setResult(r, api.getMode(), desc, "Creation of factory " + + ConfigurationValidator.setResult(r, validationMode, desc, "Creation of factory " + "configuration is not allowed"); } } if ( desc.getInternalNames().contains(config.getName())) { - ConfigurationValidator.setResult(r, api.getMode(), desc, "Factory configuration with " + + ConfigurationValidator.setResult(r, validationMode, desc, "Factory configuration with " + "name is not allowed"); } } diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java index 1f96eb2..216328e 100644 --- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java +++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java @@ -493,17 +493,29 @@ public class FeatureValidatorTest { api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getInternalNames().add("b"); ConfigurationApi.setConfigurationApi(f1, api); + // global region -> fail FeatureValidationResult result = validator.validate(f1, api); assertFalse(result.isValid()); assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~a")).isValid()); assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~b")).isValid()); assertTrue(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid()); - // internal region - api.setRegion(Region.INTERNAL); - ConfigurationApi.setConfigurationApi(f1, api); - result = validator.validate(f1, api); - assertTrue(result.isValid()); + // global region, lenient -> warn + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(Mode.LENIENT); + ConfigurationApi.setConfigurationApi(f1, api); + result = validator.validate(f1, api); + assertTrue(result.isValid()); + assertEquals(1, result.getConfigurationResults().get(FACTORY_PID.concat("~a")).getWarnings().size()); + assertEquals(1, result.getConfigurationResults().get(FACTORY_PID.concat("~b")).getWarnings().size()); + assertEquals(0, result.getConfigurationResults().get(FACTORY_PID.concat("~print")).getWarnings().size()); + + // internal region + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(null); + ConfigurationApi.setConfigurationApi(f1, api); + api.setRegion(Region.INTERNAL); + ConfigurationApi.setConfigurationApi(f1, api); + result = validator.validate(f1, api); + assertTrue(result.isValid()); } @Test public void testFactoryConfigurationOperationsWithCreate() { @@ -517,13 +529,30 @@ public class FeatureValidatorTest { assertFalse(result.isValid()); assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid()); + // no operation, lenient -> warn + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(Mode.LENIENT); + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); + ConfigurationApi.setConfigurationApi(f1, api); + result = validator.validate(f1, api); + assertTrue(result.isValid()); + assertEquals(1, result.getConfigurationResults().get(FACTORY_PID.concat("~print")).getWarnings().size()); + // only update -> fail + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(null); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE); ConfigurationApi.setConfigurationApi(f1, api); result = validator.validate(f1, api); assertFalse(result.isValid()); assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid()); + // only update, lenient -> warn + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(Mode.LENIENT); + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE); + ConfigurationApi.setConfigurationApi(f1, api); + result = validator.validate(f1, api); + assertTrue(result.isValid()); + assertEquals(1, result.getConfigurationResults().get(FACTORY_PID.concat("~print")).getWarnings().size()); + // only create -> success api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE); @@ -592,19 +621,30 @@ public class FeatureValidatorTest { this.validator.setFeatureProvider(provider); // no operation -> fail + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(null); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); ConfigurationApi.setConfigurationApi(f1, api); FeatureValidationResult result = validator.validate(f1, api); assertFalse(result.isValid()); assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid()); + // no operation, lenient -> warn + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(Mode.LENIENT); + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); + ConfigurationApi.setConfigurationApi(f1, api); + result = validator.validate(f1, api); + assertTrue(result.isValid()); + assertEquals(1, result.getConfigurationResults().get(FACTORY_PID.concat("~print")).getWarnings().size()); + // only update -> success + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(null); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE); ConfigurationApi.setConfigurationApi(f1, api); result = validator.validate(f1, api); assertTrue(result.isValid()); // only create -> fail + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(null); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE); ConfigurationApi.setConfigurationApi(f1, api); @@ -612,7 +652,17 @@ public class FeatureValidatorTest { assertFalse(result.isValid()); assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid()); + // only create, lenient -> warn + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(Mode.LENIENT); + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE); + ConfigurationApi.setConfigurationApi(f1, api); + result = validator.validate(f1, api); + assertTrue(result.isValid()); + assertEquals(1, result.getConfigurationResults().get(FACTORY_PID.concat("~print")).getWarnings().size()); + // update, create -> success + api.getFactoryConfigurationDescriptions().get(FACTORY_PID).setMode(null); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear(); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE); api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
