You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2020/09/21 15:17:37 UTC

[sling-org-apache-sling-feature] branch master updated: SLING-9752 Concurrent Modification Exception when PostProcessor changes extensions

This is an automated email from the ASF dual-hosted git repository.

davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature.git


The following commit(s) were added to refs/heads/master by this push:
     new f01b9b6  SLING-9752 Concurrent Modification Exception when PostProcessor changes extensions
     new a17ee4c  Merge pull request #21 from bosschaert/postprocess-fix
f01b9b6 is described below

commit f01b9b668466d98deb64883fd06d580897d504a8
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Mon Sep 21 14:08:54 2020 +0100

    SLING-9752 Concurrent Modification Exception when PostProcessor changes extensions
---
 .../apache/sling/feature/builder/BuilderUtil.java  |  4 ++-
 .../sling/feature/builder/BuilderUtilTest.java     | 35 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
index c79cfd8..4d02111 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -668,8 +668,10 @@ class BuilderUtil {
                 }
             }
         }
+
         // post processing
-        for(final Extension ext : target.getExtensions()) {
+        // Make a defensive copy of the extensions, as the handlers may modify the extensions on the target
+        for(final Extension ext : new ArrayList<>(target.getExtensions())) {
             for(final PostProcessHandler ppe : context.getPostProcessExtensions()) {
                 ppe.postProcess(new HandlerContextImpl(context, ppe), target, ext);
             }
diff --git a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
index c4abc44..dd86386 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -27,9 +27,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.stream.Collectors;
 
 import javax.json.Json;
 import javax.json.JsonArray;
@@ -261,6 +263,39 @@ public class BuilderUtilTest {
 
     }
 
+    @Test public void testPostProcessor() {
+        Feature f = new Feature(ArtifactId.fromMvnId("g:a:1"));
+
+        Extension e1 = new Extension(ExtensionType.TEXT, "foo", ExtensionState.OPTIONAL);
+        e1.setText("test");
+        f.getExtensions().add(e1);
+
+        Extension e2 = new Extension(ExtensionType.JSON, "bar", ExtensionState.REQUIRED);
+        e2.setJSON("[\"xyz\"]");
+        f.getExtensions().add(e2);
+
+        Extension e3 = new Extension(ExtensionType.ARTIFACTS, "lala", ExtensionState.TRANSIENT);
+        f.getExtensions().add(e3);
+
+        Feature f2 = new Feature(ArtifactId.fromMvnId("g:b:1"));
+
+        PostProcessHandler pph = (co, fe, ex) -> {
+            if ("lala".equals(ex.getName())) {
+                Extension barEx = fe.getExtensions().getByName("bar");
+                fe.getExtensions().remove(barEx);
+
+                fe.getExtensions().remove(ex);
+            }
+        };
+
+        BuilderContext bc = Mockito.mock(BuilderContext.class);
+        Mockito.when(bc.getPostProcessExtensions()).thenReturn(Collections.singletonList(pph));
+        BuilderUtil.mergeExtensions(f2, f, bc, Collections.emptyList(), "abc");
+
+        assertEquals(Collections.singleton("foo"),
+                f2.getExtensions().stream().map(Extension::getName).collect(Collectors.toSet()));
+    }
+
     @Test public void testMergeVariables() {
         Map<String,String> target = new HashMap<>();
         target.put("x", "327");