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:03 UTC

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

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> {