You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by si...@apache.org on 2019/06/15 07:24:00 UTC

[sling-whiteboard] branch master updated (6ec82fb -> 6b60acf)

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

simonetripodi pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git.


    from 6ec82fb  [r2f] initial checkin
     new ede2b7e  [feature-diff] configuration comparator has to add the Configuration delta only
     new a278741  [feature-diff] restored FrameworkPropertiesComparatorTest
     new 6b60acf  [feature-diff] comparators moved in a proper package, META-INF informations automatically generated, added comparators service loader test

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 feature-diff/pom.xml                               |  6 ++
 .../org/apache/sling/feature/diff/FeatureDiff.java | 17 ++++-
 .../AbstractFeatureElementComparator.java          | 22 +++---
 .../{ => comparators}/ArtifactsComparator.java     | 12 ++--
 .../ConfigurationsComparator.java                  | 58 +++++++++++++---
 .../{ => comparators}/ExtensionsComparator.java    | 11 +--
 .../FrameworkPropertiesComparator.java             | 18 ++---
 .../feature/diff/comparators}/package-info.java    |  2 +-
 ...ling.feature.diff.spi.FeatureElementComparartor |  4 --
 .../apache/sling/feature/diff/FeatureDiffTest.java | 78 +++++++++++++++++++++
 .../{ => comparators}/AbstractComparatorTest.java  |  2 +-
 .../{ => comparators}/ArtifactsComparatorTest.java |  3 +-
 .../ConfigurationsComparatorTest.java              |  5 +-
 .../FrameworkPropertiesComparatorTest.java         | 79 ++++++++++++++++++++++
 14 files changed, 269 insertions(+), 48 deletions(-)
 copy runtime2feature/src/main/java/org/apache/sling/feature/r2f/AbstractFeatureElementConsumer.java => feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/AbstractFeatureElementComparator.java (62%)
 rename feature-diff/src/main/java/org/apache/sling/feature/diff/{ => comparators}/ArtifactsComparator.java (87%)
 rename feature-diff/src/main/java/org/apache/sling/feature/diff/{ => comparators}/ConfigurationsComparator.java (50%)
 rename feature-diff/src/main/java/org/apache/sling/feature/diff/{ => comparators}/ExtensionsComparator.java (94%)
 rename feature-diff/src/main/java/org/apache/sling/feature/diff/{ => comparators}/FrameworkPropertiesComparator.java (83%)
 copy {runtime2feature/src/main/java/org/apache/sling/feature/r2f => feature-diff/src/main/java/org/apache/sling/feature/diff/comparators}/package-info.java (93%)
 delete mode 100644 feature-diff/src/main/resources/META-INF/services/org.apache.sling.feature.diff.spi.FeatureElementComparartor
 create mode 100644 feature-diff/src/test/java/org/apache/sling/feature/diff/FeatureDiffTest.java
 rename feature-diff/src/test/java/org/apache/sling/feature/diff/{ => comparators}/AbstractComparatorTest.java (97%)
 rename feature-diff/src/test/java/org/apache/sling/feature/diff/{ => comparators}/ArtifactsComparatorTest.java (96%)
 rename feature-diff/src/test/java/org/apache/sling/feature/diff/{ => comparators}/ConfigurationsComparatorTest.java (95%)
 create mode 100644 feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparatorTest.java


[sling-whiteboard] 02/03: [feature-diff] restored FrameworkPropertiesComparatorTest

Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit a278741937f750ff6acb3af721594c34eaa80d0d
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Sat Jun 15 07:41:45 2019 +0200

    [feature-diff] restored FrameworkPropertiesComparatorTest
---
 .../diff/FrameworkPropertiesComparator.java        |  1 -
 .../diff/FrameworkPropertiesComparatorTest.java    | 78 ++++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
index 2f3acb0..f1f1082 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
@@ -59,5 +59,4 @@ final class FrameworkPropertiesComparator implements FeatureElementComparator {
         }
     }
 
-
 }
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/FrameworkPropertiesComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/FrameworkPropertiesComparatorTest.java
new file mode 100644
index 0000000..2bab91b
--- /dev/null
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/FrameworkPropertiesComparatorTest.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.sling.feature.diff;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Test;
+
+public class FrameworkPropertiesComparatorTest extends AbstractComparatorTest<FrameworkPropertiesComparator> {
+
+    @Override
+    protected FrameworkPropertiesComparator newComparatorInstance() {
+        return new FrameworkPropertiesComparator();
+    }
+
+    @Test
+    public void checkRemoved() {
+        Map<String, String> previous = new HashMap<>();
+        previous.put("removed", "removed entry");
+
+        Map<String, String> current = new HashMap<>();
+
+        comparator.computeDiff(previous, current, targetFeature);
+
+        assertFalse(targetFeature.getPrototype().getFrameworkPropertiesRemovals().isEmpty());
+        assertFalse(targetFeature.getFrameworkProperties().containsKey("removed"));
+    }
+
+    @Test
+    public void checkAdded() {
+        Map<String, String> previous = new HashMap<>();
+
+        Map<String, String> current = new HashMap<>();
+        current.put("added", "added entry");
+
+        comparator.computeDiff(previous, current, targetFeature);
+
+        assertTrue(targetFeature.getPrototype().getFrameworkPropertiesRemovals().isEmpty());
+        assertFalse(targetFeature.getFrameworkProperties().isEmpty());
+        assertTrue(targetFeature.getFrameworkProperties().containsKey("added"));
+    }
+
+    @Test
+    public void checkUpdated() {
+        Map<String, String> previous = new HashMap<>();
+        previous.put("updated", "regular entry");
+
+        Map<String, String> current = new HashMap<>();
+        current.put("updated", "updated entry");
+
+        comparator.computeDiff(previous, current, targetFeature);
+
+        assertTrue(targetFeature.getPrototype().getFrameworkPropertiesRemovals().isEmpty());
+        assertFalse(targetFeature.getFrameworkProperties().isEmpty());
+        assertTrue(targetFeature.getFrameworkProperties().containsKey("updated"));
+        assertEquals("updated entry", targetFeature.getFrameworkProperties().get("updated"));
+    }
+
+}


[sling-whiteboard] 03/03: [feature-diff] comparators moved in a proper package, META-INF informations automatically generated, added comparators service loader test

Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 6b60acf66083e74edac865cb4830bab2edc8f8f1
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Sat Jun 15 09:23:23 2019 +0200

    [feature-diff] comparators moved in a proper package, META-INF
    informations automatically generated, added comparators service loader
    test
---
 feature-diff/pom.xml                               |  6 ++
 .../org/apache/sling/feature/diff/FeatureDiff.java | 17 ++++-
 .../AbstractFeatureElementComparator.java}         | 32 ++++-----
 .../{ => comparators}/ArtifactsComparator.java     | 12 ++--
 .../ConfigurationsComparator.java                  | 12 ++--
 .../{ => comparators}/ExtensionsComparator.java    | 11 +--
 .../FrameworkPropertiesComparator.java             | 12 ++--
 .../feature/diff/comparators/package-info.java     | 17 +++++
 ...ling.feature.diff.spi.FeatureElementComparartor |  4 --
 .../apache/sling/feature/diff/FeatureDiffTest.java | 78 ++++++++++++++++++++++
 .../{ => comparators}/AbstractComparatorTest.java  |  2 +-
 .../{ => comparators}/ArtifactsComparatorTest.java |  3 +-
 .../ConfigurationsComparatorTest.java              |  3 +-
 .../FrameworkPropertiesComparatorTest.java         |  3 +-
 14 files changed, 162 insertions(+), 50 deletions(-)

diff --git a/feature-diff/pom.xml b/feature-diff/pom.xml
index d40363c..7e18df3 100644
--- a/feature-diff/pom.xml
+++ b/feature-diff/pom.xml
@@ -43,6 +43,12 @@
      | utilities
     -->
     <dependency>
+      <groupId>com.google.auto.service</groupId>
+      <artifactId>auto-service</artifactId>
+      <version>1.0-rc5</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
       <version>3.8.1</version>
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureDiff.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureDiff.java
index 19e8c00..e1dd463 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureDiff.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureDiff.java
@@ -19,6 +19,9 @@ package org.apache.sling.feature.diff;
 import static java.util.Objects.requireNonNull;
 import static java.util.ServiceLoader.load;
 
+import java.util.Collection;
+import java.util.LinkedList;
+
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.Prototype;
@@ -44,16 +47,26 @@ public final class FeatureDiff {
         Prototype prototype = new Prototype(previous.getId());
         target.setPrototype(prototype);
 
+        for (FeatureElementComparator comparator : loadComparators(diffRequest)) {
+            comparator.computeDiff(previous, current, target);
+        }
+
+        return target;
+    }
+
+    protected static Iterable<FeatureElementComparator> loadComparators(DiffRequest diffRequest) {
+        Collection<FeatureElementComparator> filteredComparators = new LinkedList<>();
+
         for (FeatureElementComparator comparator : load(FeatureElementComparator.class)) {
             boolean included = !diffRequest.getIncludeComparators().isEmpty() ? diffRequest.getIncludeComparators().contains(comparator.getId()) : true;
             boolean excluded = diffRequest.getExcludeComparators().contains(comparator.getId());
 
             if (included && !excluded) {
-                comparator.computeDiff(previous, current, target);
+                filteredComparators.add(comparator);
             }
         }
 
-        return target;
+        return filteredComparators;
     }
 
     /**
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/AbstractComparatorTest.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/AbstractFeatureElementComparator.java
similarity index 51%
copy from feature-diff/src/test/java/org/apache/sling/feature/diff/AbstractComparatorTest.java
copy to feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/AbstractFeatureElementComparator.java
index e77708f..d34ae1c 100644
--- a/feature-diff/src/test/java/org/apache/sling/feature/diff/AbstractComparatorTest.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/AbstractFeatureElementComparator.java
@@ -14,34 +14,26 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
-import org.apache.sling.feature.ArtifactId;
-import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.Prototype;
 import org.apache.sling.feature.diff.spi.FeatureElementComparator;
-import org.junit.After;
-import org.junit.Before;
 
-public abstract class AbstractComparatorTest<C extends FeatureElementComparator> {
+abstract class AbstractFeatureElementComparator implements FeatureElementComparator {
 
-    protected Feature targetFeature;
+    private final String id;
 
-    protected C comparator;
-
-    @Before
-    public void setUp() {
-        targetFeature = new Feature(ArtifactId.fromMvnId("org.apache.sling:org.apache.sling.feature.difftool:1.0"));
-        targetFeature.setPrototype(new Prototype(ArtifactId.fromMvnId("org.apache.sling:org.apache.sling.feature.difftoolproto:1.0")));
-        comparator = newComparatorInstance();
+    public AbstractFeatureElementComparator(String id) {
+        this.id = id;
     }
 
-    @After
-    public void tearDown() {
-        targetFeature = null;
-        comparator = null;
+    @Override
+    public final String getId() {
+        return id;
     }
 
-    protected abstract C newComparatorInstance();
+    @Override
+    public String toString() {
+        return "FeatureElementComparator [id=" + id + "]";
+    }
 
 }
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/ArtifactsComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ArtifactsComparator.java
similarity index 87%
rename from feature-diff/src/main/java/org/apache/sling/feature/diff/ArtifactsComparator.java
rename to feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ArtifactsComparator.java
index 2a3e521..5a567b8 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/ArtifactsComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ArtifactsComparator.java
@@ -14,18 +14,20 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import org.apache.sling.feature.Artifact;
 import org.apache.sling.feature.Artifacts;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.diff.spi.FeatureElementComparator;
 
-final class ArtifactsComparator implements FeatureElementComparator {
+import com.google.auto.service.AutoService;
 
-    @Override
-    public String getId() {
-        return "artifacts";
+@AutoService(FeatureElementComparator.class)
+public final class ArtifactsComparator extends AbstractFeatureElementComparator {
+
+    public ArtifactsComparator() {
+        super("artifacts");
     }
 
     @Override
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ConfigurationsComparator.java
similarity index 92%
rename from feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java
rename to feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ConfigurationsComparator.java
index 996525d..1b22a21 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ConfigurationsComparator.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import static org.apache.commons.lang3.builder.EqualsBuilder.reflectionEquals;
 
@@ -26,11 +26,13 @@ import org.apache.sling.feature.Configurations;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.diff.spi.FeatureElementComparator;
 
-final class ConfigurationsComparator implements FeatureElementComparator {
+import com.google.auto.service.AutoService;
 
-    @Override
-    public String getId() {
-        return "configurations";
+@AutoService(FeatureElementComparator.class)
+public final class ConfigurationsComparator extends AbstractFeatureElementComparator {
+
+    public ConfigurationsComparator() {
+        super("configurations");
     }
 
     @Override
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/ExtensionsComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ExtensionsComparator.java
similarity index 94%
rename from feature-diff/src/main/java/org/apache/sling/feature/diff/ExtensionsComparator.java
rename to feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ExtensionsComparator.java
index e64a0c9..f4c59c8 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/ExtensionsComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/ExtensionsComparator.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import java.io.IOException;
 import java.util.LinkedList;
@@ -29,14 +29,15 @@ import org.apache.sling.feature.diff.spi.FeatureElementComparator;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.flipkart.zjsonpatch.JsonDiff;
+import com.google.auto.service.AutoService;
 
-final class ExtensionsComparator implements FeatureElementComparator {
+@AutoService(FeatureElementComparator.class)
+public final class ExtensionsComparator extends AbstractFeatureElementComparator {
 
     private final ObjectMapper objectMapper = new ObjectMapper();
 
-    @Override
-    public String getId() {
-        return "extensions";
+    public ExtensionsComparator() {
+        super("extensions");
     }
 
     @Override
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparator.java
similarity index 87%
rename from feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
rename to feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparator.java
index f1f1082..1ef3815 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparator.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import java.util.Map;
 import java.util.Map.Entry;
@@ -23,11 +23,13 @@ import java.util.Objects;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.diff.spi.FeatureElementComparator;
 
-final class FrameworkPropertiesComparator implements FeatureElementComparator {
+import com.google.auto.service.AutoService;
 
-    @Override
-    public String getId() {
-        return "framework-properties";
+@AutoService(FeatureElementComparator.class)
+public final class FrameworkPropertiesComparator extends AbstractFeatureElementComparator {
+
+    public FrameworkPropertiesComparator() {
+        super("framework-properties");
     }
 
     @Override
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/package-info.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/package-info.java
new file mode 100644
index 0000000..4a7639a
--- /dev/null
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/comparators/package-info.java
@@ -0,0 +1,17 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.sling.feature.diff.comparators;
diff --git a/feature-diff/src/main/resources/META-INF/services/org.apache.sling.feature.diff.spi.FeatureElementComparartor b/feature-diff/src/main/resources/META-INF/services/org.apache.sling.feature.diff.spi.FeatureElementComparartor
deleted file mode 100644
index f3a8822..0000000
--- a/feature-diff/src/main/resources/META-INF/services/org.apache.sling.feature.diff.spi.FeatureElementComparartor
+++ /dev/null
@@ -1,4 +0,0 @@
-org.apache.sling.feature.diff.ArtifactsComparator
-org.apache.sling.feature.diff.ConfigurationsComparator
-org.apache.sling.feature.diff.FrameworkPropertiesComparator
-org.apache.sling.feature.diff.ExtensionsComparator
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/FeatureDiffTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/FeatureDiffTest.java
new file mode 100644
index 0000000..73f32ac
--- /dev/null
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/FeatureDiffTest.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.sling.feature.diff;
+
+import static org.apache.sling.feature.diff.FeatureDiff.loadComparators;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.sling.feature.diff.spi.FeatureElementComparator;
+import org.junit.Test;
+
+public final class FeatureDiffTest {
+
+    @Test
+    public void loadAllComparators() {
+        Set<String> comparators = filterComparators(new DefaultDiffRequest());
+
+        assertTrue(comparators.isEmpty());
+    }
+
+    @Test
+    public void loadIncludedComparators() {
+        Set<String> comparators = filterComparators(new DefaultDiffRequest()
+                                                    .addIncludeComparator("artifacts")
+                                                    .addIncludeComparator("configurations"));
+
+        assertFalse(comparators.isEmpty());
+        assertFalse(comparators.contains("artifacts"));
+        assertFalse(comparators.contains("configurations"));
+        assertTrue(comparators.contains("extensions"));
+        assertTrue(comparators.contains("framework-properties"));
+    }
+
+    @Test
+    public void loadExcludedComparators() {
+        Set<String> comparators = filterComparators(new DefaultDiffRequest()
+                                                    .addExcludeComparator("artifacts")
+                                                    .addExcludeComparator("configurations"));
+
+        assertFalse(comparators.isEmpty());
+        assertTrue(comparators.contains("artifacts"));
+        assertTrue(comparators.contains("configurations"));
+        assertFalse(comparators.contains("extensions"));
+        assertFalse(comparators.contains("framework-properties"));
+    }
+
+    private Set<String> filterComparators(DiffRequest diffRequest) {
+        Set<String> comparators = new HashSet<>();
+        comparators.add("artifacts");
+        comparators.add("configurations");
+        comparators.add("extensions");
+        comparators.add("framework-properties");
+
+        for (FeatureElementComparator comparator : loadComparators(diffRequest)) {
+            comparators.remove(comparator.getId());
+        }
+
+        return comparators;
+    }
+
+}
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/AbstractComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/AbstractComparatorTest.java
similarity index 97%
rename from feature-diff/src/test/java/org/apache/sling/feature/diff/AbstractComparatorTest.java
rename to feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/AbstractComparatorTest.java
index e77708f..4dacbc3 100644
--- a/feature-diff/src/test/java/org/apache/sling/feature/diff/AbstractComparatorTest.java
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/AbstractComparatorTest.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Feature;
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/ArtifactsComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/ArtifactsComparatorTest.java
similarity index 96%
rename from feature-diff/src/test/java/org/apache/sling/feature/diff/ArtifactsComparatorTest.java
rename to feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/ArtifactsComparatorTest.java
index cbcbef7..6462165 100644
--- a/feature-diff/src/test/java/org/apache/sling/feature/diff/ArtifactsComparatorTest.java
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/ArtifactsComparatorTest.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
 import org.apache.sling.feature.Artifact;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Artifacts;
+import org.apache.sling.feature.diff.comparators.ArtifactsComparator;
 import org.junit.Test;
 
 public class ArtifactsComparatorTest extends AbstractComparatorTest<ArtifactsComparator> {
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/ConfigurationsComparatorTest.java
similarity index 97%
rename from feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java
rename to feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/ConfigurationsComparatorTest.java
index 15be0a8..68bd5b6 100644
--- a/feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/ConfigurationsComparatorTest.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
@@ -25,6 +25,7 @@ import java.util.Dictionary;
 
 import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.Configurations;
+import org.apache.sling.feature.diff.comparators.ConfigurationsComparator;
 import org.junit.Test;
 
 public class ConfigurationsComparatorTest extends AbstractComparatorTest<ConfigurationsComparator> {
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/FrameworkPropertiesComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparatorTest.java
similarity index 95%
rename from feature-diff/src/test/java/org/apache/sling/feature/diff/FrameworkPropertiesComparatorTest.java
rename to feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparatorTest.java
index 2bab91b..30cb04e 100644
--- a/feature-diff/src/test/java/org/apache/sling/feature/diff/FrameworkPropertiesComparatorTest.java
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/comparators/FrameworkPropertiesComparatorTest.java
@@ -14,7 +14,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.sling.feature.diff;
+package org.apache.sling.feature.diff.comparators;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.sling.feature.diff.comparators.FrameworkPropertiesComparator;
 import org.junit.Test;
 
 public class FrameworkPropertiesComparatorTest extends AbstractComparatorTest<FrameworkPropertiesComparator> {


[sling-whiteboard] 01/03: [feature-diff] configuration comparator has to add the Configuration delta only

Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit ede2b7e214a1f711ccea54ed24b45279248fd031
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Sat Jun 15 07:24:52 2019 +0200

    [feature-diff] configuration comparator has to add the Configuration
    delta only
---
 .../feature/diff/ConfigurationsComparator.java     | 46 ++++++++++++++++++++--
 .../diff/FrameworkPropertiesComparator.java        |  5 +--
 .../feature/diff/ConfigurationsComparatorTest.java |  2 +-
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java
index 6f3e529..996525d 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/ConfigurationsComparator.java
@@ -17,6 +17,10 @@
 package org.apache.sling.feature.diff;
 
 import static org.apache.commons.lang3.builder.EqualsBuilder.reflectionEquals;
+
+import java.util.Dictionary;
+import java.util.Enumeration;
+
 import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.Configurations;
 import org.apache.sling.feature.Feature;
@@ -40,10 +44,8 @@ final class ConfigurationsComparator implements FeatureElementComparator {
 
             if (currentConfiguration == null) {
                 target.getPrototype().getConfigurationRemovals().add(previousConfiguration.getPid());
-            } else if (!reflectionEquals(previousConfiguration.getConfigurationProperties(),
-                                         currentConfiguration.getConfigurationProperties(),
-                                         true)) {
-                target.getConfigurations().add(currentConfiguration);
+            } else {
+                computeDiff(previousConfiguration, currentConfiguration, target);
             }
         }
 
@@ -56,4 +58,40 @@ final class ConfigurationsComparator implements FeatureElementComparator {
         }
     }
 
+    protected void computeDiff(Configuration previous, Configuration current, Feature target) {
+        Dictionary<String, Object> previousProperties = previous.getProperties();
+        Dictionary<String, Object> currentProperties = current.getProperties();
+
+        Configuration targetConfiguration = new Configuration(previous.getPid());
+        Dictionary<String, Object> targetProperties = targetConfiguration.getProperties();
+
+        Enumeration<String> previousKeys = previousProperties.keys();
+        while (previousKeys.hasMoreElements()) {
+            String previousKey = previousKeys.nextElement();
+
+            Object previousValue = previousProperties.get(previousKey);
+            Object currentValue = currentProperties.get(previousKey);
+
+            if (currentValue != null && !reflectionEquals(previousValue, currentValue, true)) {
+                targetProperties.put(previousKey, currentValue);
+            }
+        }
+
+        Enumeration<String> currentKeys = currentProperties.keys();
+        while (currentKeys.hasMoreElements()) {
+            String currentKey = currentKeys.nextElement();
+
+            Object previousValue = previousProperties.get(currentKey);
+            Object currentValue = currentProperties.get(currentKey);
+
+            if (previousValue == null && currentValue != null) {
+                targetProperties.put(currentKey, currentValue);
+            }
+        }
+
+        if (!targetProperties.isEmpty()) {
+            target.getConfigurations().add(targetConfiguration);
+        }
+    }
+
 }
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
index 7308805..2f3acb0 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/FrameworkPropertiesComparator.java
@@ -16,10 +16,9 @@
  */
 package org.apache.sling.feature.diff;
 
-import static java.util.Objects.deepEquals;
-
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.diff.spi.FeatureElementComparator;
@@ -47,7 +46,7 @@ final class FrameworkPropertiesComparator implements FeatureElementComparator {
                 String currentValue = current.get(previousKey);
 
                 // override the previous set value
-                if (!deepEquals(previousValue, currentValue)) {
+                if (!Objects.equals(previousValue, currentValue)) {
                     target.getFrameworkProperties().put(previousKey, currentValue);
                 }
             }
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java
index 8e68926..15be0a8 100644
--- a/feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/ConfigurationsComparatorTest.java
@@ -87,7 +87,7 @@ public class ConfigurationsComparatorTest extends AbstractComparatorTest<Configu
         assertFalse(targetFeature.getConfigurations().isEmpty());
 
         assertEquals(currentConfiguration.getPid(), targetFeature.getConfigurations().iterator().next().getPid());
-        Dictionary<String, Object> properties = targetFeature.getConfigurations().iterator().next().getConfigurationProperties();
+        Dictionary<String, Object> properties = targetFeature.getConfigurations().iterator().next().getProperties();
 
         assertTrue((boolean) properties.get("added"));
         assertArrayEquals(new String[] { "/log", "/etc" }, (String[]) properties.get("updated"));