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/04/03 13:38:43 UTC

[sling-whiteboard] branch master updated: [feature-diff] added generic map comparator, design adjusted

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


The following commit(s) were added to refs/heads/master by this push:
     new 7a95862  [feature-diff] added generic map comparator, design adjusted
7a95862 is described below

commit 7a95862b4321fb468b0c2e368fdf7b0aecea0a12
Author: stripodi <st...@192.168.1.111>
AuthorDate: Wed Apr 3 15:38:36 2019 +0200

    [feature-diff] added generic map comparator, design adjusted
---
 .../diff/AbstractFeatureElementComparator.java     |  4 +-
 ...mparator.java => ComplexElementComparator.java} |  3 +-
 ...ementComparator.java => ElementComparator.java} |  4 +-
 .../sling/feature/diff/GenericMapComparator.java   | 62 +++++++++++++++
 .../feature/diff/ArtifactsComparatorTest.java      |  4 +-
 .../feature/diff/ConfigurationsComparatorTest.java |  7 +-
 .../feature/diff/GenericMapComparatorTest.java     | 92 ++++++++++++++++++++++
 7 files changed, 165 insertions(+), 11 deletions(-)

diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/AbstractFeatureElementComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/AbstractFeatureElementComparator.java
index 5de0ca2..e05a06a 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/AbstractFeatureElementComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/AbstractFeatureElementComparator.java
@@ -18,7 +18,7 @@ package org.apache.sling.feature.diff;
 
 import static java.util.Objects.requireNonNull;
 
-abstract class AbstractFeatureElementComparator<T, I extends Iterable<T>> implements FeatureElementComparator<T, I> {
+abstract class AbstractFeatureElementComparator<T, I extends Iterable<T>> implements ComplexElementComparator<T, I> {
 
     private final String id;
 
@@ -30,8 +30,6 @@ abstract class AbstractFeatureElementComparator<T, I extends Iterable<T>> implem
 
     protected abstract T find(T item, I collection);
 
-    protected abstract DiffSection compare(T previous, T current);
-
     @Override
     public final DiffSection apply(I previouses, I currents) {
         final DiffSection diffDsection = new DiffSection(id);
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureElementComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/ComplexElementComparator.java
similarity index 86%
copy from feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureElementComparator.java
copy to feature-diff/src/main/java/org/apache/sling/feature/diff/ComplexElementComparator.java
index 6042d7f..840e364 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureElementComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/ComplexElementComparator.java
@@ -18,6 +18,7 @@ package org.apache.sling.feature.diff;
 
 import java.util.function.BiFunction;
 
-public interface FeatureElementComparator<T, I extends Iterable<T>> extends BiFunction<I, I, DiffSection> {
+public interface ComplexElementComparator<T, I extends Iterable<T>>
+        extends BiFunction<I, I, DiffSection>, ElementComparator<T> {
 
 }
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureElementComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/ElementComparator.java
similarity index 85%
rename from feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureElementComparator.java
rename to feature-diff/src/main/java/org/apache/sling/feature/diff/ElementComparator.java
index 6042d7f..dc36699 100644
--- a/feature-diff/src/main/java/org/apache/sling/feature/diff/FeatureElementComparator.java
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/ElementComparator.java
@@ -16,8 +16,8 @@
  */
 package org.apache.sling.feature.diff;
 
-import java.util.function.BiFunction;
+public interface ElementComparator<T> {
 
-public interface FeatureElementComparator<T, I extends Iterable<T>> extends BiFunction<I, I, DiffSection> {
+    DiffSection compare(T previous, T current);
 
 }
diff --git a/feature-diff/src/main/java/org/apache/sling/feature/diff/GenericMapComparator.java b/feature-diff/src/main/java/org/apache/sling/feature/diff/GenericMapComparator.java
new file mode 100644
index 0000000..8c0ea24
--- /dev/null
+++ b/feature-diff/src/main/java/org/apache/sling/feature/diff/GenericMapComparator.java
@@ -0,0 +1,62 @@
+/*
+ * 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 java.util.Objects.requireNonNull;
+
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.apache.commons.lang3.builder.EqualsBuilder;
+
+final class GenericMapComparator {
+
+    private final String id;
+
+    public GenericMapComparator(String id) {
+        this.id = requireNonNull(id, "Impossible to instantiate a generic Map comparator with null id");
+    }
+
+    public <V> DiffSection compare(Map<String, V> previous, Map<String, V> current) {
+        DiffSection diffSection = new DiffSection(id);
+
+        for (Entry<String, V> previousEntry : previous.entrySet()) {
+            String previousKey = previousEntry.getKey();
+
+            if (!current.containsKey(previousKey)) {
+                diffSection.markRemoved(previousKey);
+            } else {
+                V previousValue = previousEntry.getValue();
+                V currentValue = current.get(previousKey);
+
+                if (!new EqualsBuilder().reflectionAppend(previousValue, currentValue).isEquals()) {
+                    diffSection.markItemUpdated(previousKey, previousValue, currentValue);
+                }
+            }
+        }
+
+        for (String currentKey : current.keySet()) {
+            if (!previous.containsKey(currentKey)) {
+                diffSection.markAdded(currentKey);
+            }
+        }
+
+        return diffSection;
+    }
+
+
+}
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/ArtifactsComparatorTest.java
index 91cb379..673551d 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/ArtifactsComparatorTest.java
@@ -54,8 +54,8 @@ public class ArtifactsComparatorTest {
         Artifacts currentArtifacts = new Artifacts();
 
         DiffSection artifactsDiff = comparator.apply(previousArtifacts, currentArtifacts);
-        assertFalse(artifactsDiff.isEmpty());
 
+        assertFalse(artifactsDiff.isEmpty());
         assertEquals(previousArtifact.getId().toMvnId(), artifactsDiff.getRemoved().iterator().next());
     }
 
@@ -68,8 +68,8 @@ public class ArtifactsComparatorTest {
         currentArtifacts.add(currentArtifact);
 
         DiffSection artifactsDiff = comparator.apply(previousArtifacts, currentArtifacts);
-        assertFalse(artifactsDiff.isEmpty());
 
+        assertFalse(artifactsDiff.isEmpty());
         assertEquals(currentArtifact.getId().toMvnId(), artifactsDiff.getAdded().iterator().next());
     }
 
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 5fba06b..77376c3 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
@@ -92,10 +92,11 @@ public class ConfigurationsComparatorTest {
         assertEquals("removed", configurationDiff.getRemoved().iterator().next());
         assertEquals("added", configurationDiff.getAdded().iterator().next());
 
-        UpdatedItem<?> updated = configurationDiff.getUpdatedItems().iterator().next();
+        @SuppressWarnings("unchecked") // type known by design
+        UpdatedItem<String[]> updated = (UpdatedItem<String[]>) configurationDiff.getUpdatedItems().iterator().next();
         assertEquals("updated", updated.getId());
-        assertArrayEquals(new String[] { "/log" }, (String[]) updated.getPrevious());
-        assertArrayEquals(new String[] { "/log", "/etc" }, (String[]) updated.getCurrent());
+        assertArrayEquals(new String[] { "/log" }, updated.getPrevious());
+        assertArrayEquals(new String[] { "/log", "/etc" }, updated.getCurrent());
     }
 
 }
diff --git a/feature-diff/src/test/java/org/apache/sling/feature/diff/GenericMapComparatorTest.java b/feature-diff/src/test/java/org/apache/sling/feature/diff/GenericMapComparatorTest.java
new file mode 100644
index 0000000..a26b93a
--- /dev/null
+++ b/feature-diff/src/test/java/org/apache/sling/feature/diff/GenericMapComparatorTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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 java.util.HashMap;
+import java.util.Map;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public final class GenericMapComparatorTest {
+
+    private GenericMapComparator comparator;
+    @Before
+    public void setUp() {
+        comparator = new GenericMapComparator("map");
+    }
+
+    @After
+    public void tearDown() {
+        comparator = null;
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void constructoreRequiresValidId() {
+        new GenericMapComparator(null);
+    }
+
+    @Test
+    public void checkRemoved() {
+        Map<String, String> previous = new HashMap<>();
+        previous.put("removed", "removed entry");
+
+        Map<String, String> current = new HashMap<>();
+
+        DiffSection mapsDiff = comparator.compare(previous, current);
+
+        assertFalse(mapsDiff.isEmpty());
+        assertEquals("removed", mapsDiff.getRemoved().iterator().next());
+    }
+
+    @Test
+    public void checkAdded() {
+        Map<String, String> previous = new HashMap<>();
+
+        Map<String, String> current = new HashMap<>();
+        current.put("added", "added entry");
+
+        DiffSection mapsDiff = comparator.compare(previous, current);
+
+        assertFalse(mapsDiff.isEmpty());
+        assertEquals("added", mapsDiff.getAdded().iterator().next());
+    }
+
+    @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");
+
+        DiffSection mapsDiff = comparator.compare(previous, current);
+
+        assertFalse(mapsDiff.isEmpty());
+
+        @SuppressWarnings("unchecked") // type known by design
+        UpdatedItem<String> updated = (UpdatedItem<String>) mapsDiff.getUpdatedItems().iterator().next();
+        assertEquals("updated", updated.getId());
+        assertEquals("regular entry", updated.getPrevious());
+        assertEquals("updated entry", updated.getCurrent());
+    }
+
+}