sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dav...@apache.org
Subject [sling-org-apache-sling-feature] 09/22: Clarify update handling
Date Fri, 27 Apr 2018 09:51:29 GMT
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

commit 5582860f8e9297b329c427423056e125f5a4d80e
Author: Carsten Ziegeler <cziegeler@apache.org>
AuthorDate: Fri Feb 23 08:19:02 2018 +0100

    Clarify update handling
---
 .../java/org/apache/sling/feature/Feature.java     |  31 ------
 .../sling/feature/process/ApplicationBuilder.java  |  58 +++++------
 .../sling/feature/process/FeatureBuilder.java      | 111 +--------------------
 .../sling/feature/process/FeatureBuilderTest.java  |   2 -
 4 files changed, 24 insertions(+), 178 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/Feature.java b/src/main/java/org/apache/sling/feature/Feature.java
index f0f2cc5..64c2488 100644
--- a/src/main/java/org/apache/sling/feature/Feature.java
+++ b/src/main/java/org/apache/sling/feature/Feature.java
@@ -65,15 +65,9 @@ public class Feature implements Comparable<Feature> {
     /** The optional license. */
     private volatile String license;
 
-    /** Is this an upgrade of another feature? */
-    private volatile ArtifactId upgradeOf;
-
     /** Flag indicating whether this is an assembled feature */
     private volatile boolean assembled = false;
 
-    /** Contained upgrades (this is usually only set for assembled features*/
-    private final List<ArtifactId> upgrades = new ArrayList<>();
-
     /**
      * Construct a new feature.
      * @param id The id of the feature.
@@ -239,31 +233,6 @@ public class Feature implements Comparable<Feature> {
     }
 
     /**
-     * Set the upgrade of information
-     * @param id The artifact id
-     */
-    public void setUpgradeOf(final ArtifactId id) {
-        this.upgradeOf = id;
-    }
-
-    /**
-     * Get the artifact id of the upgrade of information
-     * @return The artifact id or {@code null}
-     */
-    public ArtifactId getUpgradeOf() {
-        return this.upgradeOf;
-    }
-
-    /**
-     * Get the list of upgrades applied to this feature
-     * The returned object is modifiable.
-     * @return The list of upgrades
-     */
-    public List<ArtifactId> getUpgrades() {
-        return this.upgrades;
-    }
-
-    /**
      * Check whether the feature is already assembled
      * @return {@code true} if it is assembled, {@code false} if it needs to be assembled
      */
diff --git a/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java b/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java
index cc0f235..1a555e1 100644
--- a/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java
+++ b/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java
@@ -18,9 +18,7 @@ package org.apache.sling.feature.process;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 import org.apache.sling.feature.Application;
 import org.apache.sling.feature.ArtifactId;
@@ -67,9 +65,8 @@ public class ApplicationBuilder {
     /**
      * Assemble an application based on the provided features.
      *
-     * Upgrade features are only applied if the provided feature list
-     * contains the feature to be upgraded. Otherwise the upgrade feature
-     * is ignored.
+     * If the same feature is included more than once only the feature with
+     * the highest version is used. The others are ignored.
      *
      * @param app The optional application to use as a base.
      * @param context The builder context
@@ -90,39 +87,35 @@ public class ApplicationBuilder {
             app = new Application();
         }
 
-        // detect upgrades and created sorted feature list
-        final Map<Feature, List<Feature>> upgrades = new HashMap<>();
+        // Created sorted feature list
+        // Remove duplicate features by selecting the one with the highest version
         final List<Feature> sortedFeatureList = new ArrayList<>();
         for(final Feature f : features) {
-            if ( f.getUpgradeOf() != null ) {
-                for(final Feature i : features) {
-                    if ( i.getId().equals(f.getUpgradeOf()) ) {
-                        List<Feature> u = upgrades.get(i);
-                        if ( u == null ) {
-                            u = new ArrayList<>();
-                            upgrades.put(i, u);
-                        }
-                        u.add(f);
-                        app.getFeatureIds().add(f.getId());
-                        break;
-                    }
+            Feature found = null;
+            for(final Feature s : sortedFeatureList) {
+                if ( s.getId().isSame(f.getId()) ) {
+                    found = s;
+                    break;
+                }
+            }
+            boolean add = true;
+            // feature with different version found
+            if ( found != null ) {
+                if ( f.getId().getOSGiVersion().compareTo(found.getId().getOSGiVersion())
<= 0 ) {
+                    // higher version already included
+                    add = false;
+                } else {
+                    // remove lower version, higher version will be added
+                    app.getFeatureIds().remove(found.getId());
+                    sortedFeatureList.remove(found);
                 }
-            } else {
+            }
+            if ( add ) {
                 app.getFeatureIds().add(f.getId());
                 sortedFeatureList.add(f);
             }
         }
 
-        // process upgrades first
-        for(final Map.Entry<Feature, List<Feature>> entry : upgrades.entrySet())
{
-            final Feature assembled = FeatureBuilder.assemble(entry.getKey(),
-                    entry.getValue(),
-                    context);
-            // update feature to assembled feature
-            sortedFeatureList.remove(entry.getKey());
-            sortedFeatureList.add(assembled);
-        }
-
         // sort
         Collections.sort(sortedFeatureList);
 
@@ -132,11 +125,6 @@ public class ApplicationBuilder {
 
                 @Override
                 public Feature provide(final ArtifactId id) {
-                    for(final Feature f : upgrades.keySet()) {
-                        if ( f.getId().equals(id) ) {
-                            return f;
-                        }
-                    }
                     for(final Feature f : features) {
                         if ( f.getId().equals(id) ) {
                             return f;
diff --git a/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java b/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java
index 7208f00..6d57dc6 100644
--- a/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java
@@ -17,7 +17,6 @@
 package org.apache.sling.feature.process;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -48,91 +47,6 @@ public class FeatureBuilder {
         return internalAssemble(new ArrayList<>(), feature, context);
     }
 
-    /**
-     * Assemble the final feature and apply upgrades
-     *
-     * If the list of upgrades contains upgrade features not intended for the
-     * provided feature, this is not considered an error situation. But the
-     * provided upgrade is ignored.
-     *
-     * @param feature The feature to start
-     * @param upgrades The list of upgrades. If this is {@code null} or empty, this method
-     *     behaves like {@link #assemble(Feature, FeatureProvider)}.
-     * @param context The builder context
-     * @return The assembled feature.
-     * @throws IllegalArgumentException If feature or context is {@code null}
-     * @throws IllegalStateException If an included feature can't be provided
-     */
-    public static Feature assemble(final Feature feature,
-            final List<Feature> upgrades,
-            final BuilderContext context) {
-        if ( feature == null || context == null ) {
-            throw new IllegalArgumentException("Feature and/or context must not be null");
-        }
-
-        // check upgrades
-        List<Feature> useUpdates = null;
-        if ( upgrades != null && !upgrades.isEmpty() ) {
-            useUpdates = new ArrayList<>();
-            for(final Feature uf : upgrades) {
-                if ( !feature.getId().equals(uf.getUpgradeOf()) ) {
-                    continue;
-                }
-                boolean found = false;
-                for(final Feature i : useUpdates) {
-                    if ( i.getId().isSame(uf.getId()) ) {
-                        if ( uf.getId().getOSGiVersion().compareTo(i.getId().getOSGiVersion())
> 0 ) {
-                            useUpdates.remove(i);
-                        } else {
-                            found = true;
-                        }
-                        break;
-                    }
-                }
-                if ( !found ) {
-                    // we add a copy as we manipulate the upgrade below
-                    useUpdates.add(uf.copy());
-                }
-            }
-            Collections.sort(useUpdates);
-            if ( useUpdates.isEmpty() ) {
-                useUpdates = null;
-            }
-        }
-
-        // assemble feature without upgrades
-        final Feature assembledFeature = internalAssemble(new ArrayList<>(), feature,
context);
-
-        // handle upgrades
-        if ( useUpdates != null ) {
-            for(final Feature uf : useUpdates) {
-                Include found = null;
-                for(final Include inc : uf.getIncludes() ) {
-                    if ( inc.getId().equals(assembledFeature.getId()) ) {
-                        found = inc;
-                        break;
-                    }
-                }
-                if ( found != null ) {
-                    uf.getIncludes().remove(found);
-
-                    // process include instructions
-                    include(assembledFeature, found);
-                }
-
-                // now assemble upgrade, but without considering the base
-                uf.setUpgradeOf(null);
-                assembledFeature.getUpgrades().add(uf.getId());
-                final Feature auf = assemble(uf, context);
-
-                // merge
-                merge(assembledFeature, auf, context);
-            }
-        }
-
-        return assembledFeature;
-    }
-
     private static Feature internalAssemble(final List<String> processedFeatures,
             final Feature feature,
             final BuilderContext context) {
@@ -145,30 +59,7 @@ public class FeatureBuilder {
         processedFeatures.add(feature.getId().toMvnId());
 
         // we copy the feature as we set the assembled flag on the result
-        final Feature result;
-
-        if ( feature.getUpgradeOf() != null ) {
-            Include found = null;
-            for(final Include inc : feature.getIncludes()) {
-                if ( inc.getId().equals(feature.getUpgradeOf()) ) {
-                    found = inc;
-                    break;
-                }
-            }
-
-            result = feature.copy(feature.getUpgradeOf());
-
-            // add base as the first include
-            if ( found == null ) {
-                result.getIncludes().add(0, new Include(feature.getUpgradeOf()));
-            } else {
-                result.getIncludes().remove(found);
-                result.getIncludes().add(0, found);
-            }
-            result.getUpgrades().add(feature.getId());
-        } else {
-            result = feature.copy();
-        }
+        final Feature result = feature.copy();
 
         if ( !result.getIncludes().isEmpty() ) {
 
diff --git a/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java b/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java
index 4fc31b3..f990aec 100644
--- a/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java
@@ -104,8 +104,6 @@ public class FeatureBuilderTest {
         assertEquals(expected.getDescription(), actuals.getDescription());
         assertEquals(expected.getVendor(), actuals.getVendor());
         assertEquals(expected.getLicense(), actuals.getLicense());
-        assertEquals(expected.getUpgradeOf(), actuals.getUpgradeOf());
-        assertEquals(expected.getUpgrades(), actuals.getUpgrades());
 
         // bundles
         final List<Map.Entry<Integer, Artifact>> expectedBundles = getBundles(expected);

-- 
To stop receiving notification emails like this one, please contact
davidb@apache.org.

Mime
View raw message