You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/07/30 12:07:37 UTC

[GitHub] [sling-org-apache-sling-feature-cpconverter] anchela commented on a change in pull request #97: Add failing unit test for mode attribute dropping

anchela commented on a change in pull request #97:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/97#discussion_r679872309



##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
##########
@@ -161,15 +163,23 @@ public void convertContentPackage() throws Exception {
                     .setEmitter(DefaultPackagesEventsEmitter.open(outputDirectory))
                     .convert(packageFile);
 
+            List<ContentPackageExtensionVerifier> verifiers = new ArrayList<>();
+            verifiers.add(new ContentPackageExtensionVerifier("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1"));
+            verifiers.add(new ContentPackageExtensionVerifier("asd.sample:Asd.Retail.ui.content:zip:cp2fm-converted:0.0.1"));
+
+            ContentPackageExtensionVerifier configPackageVerifier = new ContentPackageExtensionVerifier("asd:Asd.Retail.config:zip:cp2fm-converted:0.0.1");
+
+            configPackageVerifier.setExpectedFilterXml(expectedConfigPackageFilter);
+            verifiers.add(configPackageVerifier);
+            verifiers.add(new ContentPackageExtensionVerifier("asd.sample:asd.retail.all:zip:cp2fm-converted:0.0.1"));
+
             verifyFeatureFile(outputDirectory,
                             "asd.retail.all.json",
                             "asd.sample:asd.retail.all:slingosgifeature:0.0.1",
-                            Collections.singletonList("org.apache.felix:org.apache.felix.framework:6.0.1"),
-                            Collections.singletonList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),
-                            Arrays.asList("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1",
-                                            "asd.sample:Asd.Retail.ui.content:zip:cp2fm-converted:0.0.1",
-                                            "asd:Asd.Retail.config:zip:cp2fm-converted:0.0.1",
-                                            "asd.sample:asd.retail.all:zip:cp2fm-converted:0.0.1"));
+                            Arrays.asList("org.apache.felix:org.apache.felix.framework:6.0.1"),
+                            Arrays.asList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),

Review comment:
       changing from Collections.singletonList to Arrays.asList seems to be an unrelated change. please revert if it is not required for the test to pass

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
##########
@@ -161,15 +163,23 @@ public void convertContentPackage() throws Exception {
                     .setEmitter(DefaultPackagesEventsEmitter.open(outputDirectory))
                     .convert(packageFile);
 
+            List<ContentPackageExtensionVerifier> verifiers = new ArrayList<>();
+            verifiers.add(new ContentPackageExtensionVerifier("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1"));
+            verifiers.add(new ContentPackageExtensionVerifier("asd.sample:Asd.Retail.ui.content:zip:cp2fm-converted:0.0.1"));
+
+            ContentPackageExtensionVerifier configPackageVerifier = new ContentPackageExtensionVerifier("asd:Asd.Retail.config:zip:cp2fm-converted:0.0.1");
+
+            configPackageVerifier.setExpectedFilterXml(expectedConfigPackageFilter);
+            verifiers.add(configPackageVerifier);
+            verifiers.add(new ContentPackageExtensionVerifier("asd.sample:asd.retail.all:zip:cp2fm-converted:0.0.1"));
+
             verifyFeatureFile(outputDirectory,
                             "asd.retail.all.json",
                             "asd.sample:asd.retail.all:slingosgifeature:0.0.1",
-                            Collections.singletonList("org.apache.felix:org.apache.felix.framework:6.0.1"),
-                            Collections.singletonList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),
-                            Arrays.asList("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1",
-                                            "asd.sample:Asd.Retail.ui.content:zip:cp2fm-converted:0.0.1",
-                                            "asd:Asd.Retail.config:zip:cp2fm-converted:0.0.1",
-                                            "asd.sample:asd.retail.all:zip:cp2fm-converted:0.0.1"));
+                            Arrays.asList("org.apache.felix:org.apache.felix.framework:6.0.1"),

Review comment:
       changing from Collections.singletonList to Arrays.asList seems to be an unrelated change. please revert if it is not required for the test to pass

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -127,7 +130,7 @@
 
     public ConverterUserAndPermissionTest(@NotNull String systemUserRelPath, @Nullable String enforcePrincipalBasedSupportedPath, @NotNull String name) {
         this.aclManager = new DefaultAclManager(enforcePrincipalBasedSupportedPath, systemUserRelPath);
-        this.handlersManager = new DefaultEntryHandlersManager(Collections.emptyMap(), false, 
+        this.handlersManager = new DefaultEntryHandlersManager(Collections.emptyMap(), false,

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -174,12 +177,12 @@ public void testConvertPackageWithUsersGroupsAndServiceUsers() throws Exception
     }
 
     /**
-     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before 
+     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before
      * the corresponding system-user, whose principal is referenced in the ACE.
-     * 
-     * This test would fail if user/group information was not collected during the first pass (i.e. corresponding 
+     *
+     * This test would fail if user/group information was not collected during the first pass (i.e. corresponding
      * handlers listed in {@link org.apache.sling.feature.cpconverter.vltpkg.RecollectorVaultPackageScanner}.
-     * 
+     *

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -288,7 +320,7 @@ private void verifyRepoInit() throws RepoInitParsingException {
             assertEquals(1, setAclPrincipalBased.size());
         }
         assertTrue(ops.stream().noneMatch(op -> op instanceof SetAclPaths));
-        
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
##########
@@ -245,9 +255,11 @@ public void convertContentPackageDropContentTypePackagePolicy() throws Exception
             verifyFeatureFile(outputDirectory,
                             "asd.retail.all.json",
                             "asd.sample:asd.retail.all:slingosgifeature:0.0.1",
-                            Collections.singletonList("org.apache.felix:org.apache.felix.framework:6.0.1"),
-                            Collections.singletonList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),
-                            Arrays.asList("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1", "asd:Asd.Retail.config:zip:cp2fm-converted:0.0.1"));
+                            Arrays.asList("org.apache.felix:org.apache.felix.framework:6.0.1"),

Review comment:
       changing from Collections.singletonList to Arrays.asList seems to be an unrelated change. please revert if it is not required for the test to pass

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
##########
@@ -316,9 +328,9 @@ public void convertContentPackagePutInDedicatedFolderContentTypePackagePolicy()
             verifyFeatureFile(outputDirectory,
                             "asd.retail.all.json",
                             "asd.sample:asd.retail.all:slingosgifeature:0.0.1",
-                            Collections.singletonList("org.apache.felix:org.apache.felix.framework:6.0.1"),
-                            Collections.singletonList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),
-                            Arrays.asList("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1",
+                            Arrays.asList("org.apache.felix:org.apache.felix.framework:6.0.1"),
+                            Arrays.asList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),

Review comment:
       changing from Collections.singletonList to Arrays.asList seems to be an unrelated change. please revert if it is not required for the test to pass

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
##########
@@ -316,9 +328,9 @@ public void convertContentPackagePutInDedicatedFolderContentTypePackagePolicy()
             verifyFeatureFile(outputDirectory,
                             "asd.retail.all.json",
                             "asd.sample:asd.retail.all:slingosgifeature:0.0.1",
-                            Collections.singletonList("org.apache.felix:org.apache.felix.framework:6.0.1"),
-                            Collections.singletonList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),
-                            Arrays.asList("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1",
+                            Arrays.asList("org.apache.felix:org.apache.felix.framework:6.0.1"),

Review comment:
       changing from Collections.singletonList to Arrays.asList seems to be an unrelated change. please revert if it is not required for the test to pass

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -141,7 +144,7 @@ public void setUp() throws IOException {
 
         outputDirectory = new File(System.getProperty("java.io.tmpdir"), getClass().getName() + '_' + System.currentTimeMillis());
         featuresManager = new DefaultFeaturesManager(true, 5, outputDirectory, null, null, null, aclManager);
-        
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -254,18 +286,18 @@ private static void assertPolicy(@NotNull File contentPackage, @NotNull String p
             }
         }
     }
-    
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -225,14 +228,43 @@ public void testConvertCONTENTPackageWithUsersGroupsAndServiceUsers() throws Exc
         verifyRepoInit();
     }
 
+    @Test
+    public void testConvertPackageWithImportModes() throws Exception {
+        URL packageUrl = getClass().getResource("demo-cp-with-importmode.zip");
+        File packageFile = FileUtils.toFile(packageUrl);
+        converter.convert(packageFile);
+
+        File converted = new File(outputDirectory, "my_packages/demo-cp/0.0.0/demo-cp-0.0.0-cp2fm-converted.zip");
+
+        Set<String> notExpected = new HashSet<>(COMMON_NOT_EXPECTED_PATHS);
+        notExpected.add("jcr_root/apps/demo-cp/.content.xml");
+
+        Set<String> expected = new HashSet<>(COMMON_EXPECTED_PATHS);
+        expected.add("jcr_root/apps/.content.xml");
+
+        verifyContentPackage(converted, notExpected, expected);
+        assertExpectedPolicies(converted);
+        WorkspaceFilter filter = verifyWorkspaceFilter(converted, false);
+        verifyRepoInit();
+
+        assertEquals(ImportMode.MERGE, filter.getImportMode("/demo-cp"));
+        assertEquals(ImportMode.UPDATE, filter.getImportMode("/home/groups/demo-cp"));
+        assertEquals(ImportMode.REPLACE, filter.getImportMode("/home/users/demo-cp"));
+
+        PathFilterSet filterSet = filter.getCoveringFilterSet("/home/users/demo-cp");
+        assertNotNull(filterSet);
+        List<FilterSet.Entry<PathFilter>> entries = filterSet.getEntries();
+        assertEquals(3, entries.size());
+    }
+
     private static void assertExpectedPolicies(@NotNull File converted ) throws IOException {
         assertPolicy(converted, "jcr_root/demo-cp/_rep_policy.xml", "cp-serviceuser-1", "cp-user1", "cp-group1");
         assertPolicy(converted, "jcr_root/home/groups/demo-cp/EsYrXeBdSRkna2kqbxjl/_rep_policy.xml", null, "cp-group1");
         assertPolicy(converted,  "jcr_root/home/users/demo-cp/XPXhA_RKMFRKNO8ViIhn/_rep_policy.xml", null, "cp-user1");
         assertPolicy(converted, "jcr_root/home/groups/demo-cp/_rep_policy.xml", "cp-serviceuser-3");
         assertPolicy(converted, "jcr_root/home/users/demo-cp/_rep_policy.xml", "cp-serviceuser-3");
     }
-    
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -254,18 +286,18 @@ private static void assertPolicy(@NotNull File contentPackage, @NotNull String p
             }
         }
     }
-    
+
     private void verifyRepoInit() throws RepoInitParsingException {
         Feature f = featuresManager.getTargetFeature();
         Extension ext = f.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
         assertNotNull(ext);
-        
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -254,18 +286,18 @@ private static void assertPolicy(@NotNull File contentPackage, @NotNull String p
             }
         }
     }
-    
+
     private void verifyRepoInit() throws RepoInitParsingException {
         Feature f = featuresManager.getTargetFeature();
         Extension ext = f.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
         assertNotNull(ext);
-        
+
         List<Operation> ops = new RepoInitParserService().parse(new StringReader(ext.getText()));
         int size = (enforcePrincipalBased) ? 7 : 8;
         assertEquals(size, ops.size());
-        
+
         assertEquals(1, ops.stream().filter(operation -> operation instanceof RegisterNodetypes).count());
-        
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
##########
@@ -245,9 +255,11 @@ public void convertContentPackageDropContentTypePackagePolicy() throws Exception
             verifyFeatureFile(outputDirectory,
                             "asd.retail.all.json",
                             "asd.sample:asd.retail.all:slingosgifeature:0.0.1",
-                            Collections.singletonList("org.apache.felix:org.apache.felix.framework:6.0.1"),
-                            Collections.singletonList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),
-                            Arrays.asList("asd.sample:asd.retail.apps:zip:cp2fm-converted:0.0.1", "asd:Asd.Retail.config:zip:cp2fm-converted:0.0.1"));
+                            Arrays.asList("org.apache.felix:org.apache.felix.framework:6.0.1"),
+                            Arrays.asList("org.apache.sling.commons.log.LogManager.factory.config~asd-retail"),

Review comment:
       changing from Collections.singletonList to Arrays.asList seems to be an unrelated change. please revert if it is not required for the test to pass

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -174,12 +177,12 @@ public void testConvertPackageWithUsersGroupsAndServiceUsers() throws Exception
     }
 
     /**
-     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before 
+     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before
      * the corresponding system-user, whose principal is referenced in the ACE.
-     * 
-     * This test would fail if user/group information was not collected during the first pass (i.e. corresponding 
+     *
+     * This test would fail if user/group information was not collected during the first pass (i.e. corresponding

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -254,18 +286,18 @@ private static void assertPolicy(@NotNull File contentPackage, @NotNull String p
             }
         }
     }
-    
+
     private void verifyRepoInit() throws RepoInitParsingException {
         Feature f = featuresManager.getTargetFeature();
         Extension ext = f.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
         assertNotNull(ext);
-        
+
         List<Operation> ops = new RepoInitParserService().parse(new StringReader(ext.getText()));
         int size = (enforcePrincipalBased) ? 7 : 8;
         assertEquals(size, ops.size());
-        
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -174,12 +177,12 @@ public void testConvertPackageWithUsersGroupsAndServiceUsers() throws Exception
     }
 
     /**
-     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before 
+     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before

Review comment:
       unrelated change. can be omitted to simplify reading the diff.

##########
File path: src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
##########
@@ -277,7 +309,7 @@ private void verifyRepoInit() throws RepoInitParsingException {
             assertFalse(createServiceUsers.stream().anyMatch(operation -> ((CreateServiceUser) operation).isForcedPath()));
             assertEquals(2, createServiceUsers.stream().filter(operation -> ((CreateServiceUser) operation).getPath().startsWith(withRelPath+"/demo-cp")).count());
         }
-        
+

Review comment:
       unrelated change. can be omitted to simplify reading the diff.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org