From commits-return-66721-archive-asf-public=cust-asf.ponee.io@sling.apache.org Fri Apr 27 11:51:25 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 6B598180679 for ; Fri, 27 Apr 2018 11:51:23 +0200 (CEST) Received: (qmail 61050 invoked by uid 500); 27 Apr 2018 09:51:22 -0000 Mailing-List: contact commits-help@sling.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sling.apache.org Delivered-To: mailing list commits@sling.apache.org Received: (qmail 60941 invoked by uid 99); 27 Apr 2018 09:51:22 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 27 Apr 2018 09:51:22 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id DBA378087C; Fri, 27 Apr 2018 09:51:20 +0000 (UTC) Date: Fri, 27 Apr 2018 09:51:29 +0000 To: "commits@sling.apache.org" Subject: [sling-org-apache-sling-feature] 09/22: Clarify update handling MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: davidb@apache.org In-Reply-To: <152482268039.31703.2932656958755458488@gitbox.apache.org> References: <152482268039.31703.2932656958755458488@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: sling-org-apache-sling-feature X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Rev: 5582860f8e9297b329c427423056e125f5a4d80e X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20180427095120.DBA378087C@gitbox.apache.org> 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 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 { /** 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 upgrades = new ArrayList<>(); - /** * Construct a new feature. * @param id The id of the feature. @@ -239,31 +233,6 @@ public class Feature implements Comparable { } /** - * 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 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> upgrades = new HashMap<>(); + // Created sorted feature list + // Remove duplicate features by selecting the one with the highest version final List sortedFeatureList = new ArrayList<>(); for(final Feature f : features) { - if ( f.getUpgradeOf() != null ) { - for(final Feature i : features) { - if ( i.getId().equals(f.getUpgradeOf()) ) { - List 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> 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 upgrades, - final BuilderContext context) { - if ( feature == null || context == null ) { - throw new IllegalArgumentException("Feature and/or context must not be null"); - } - - // check upgrades - List 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 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> expectedBundles = getBundles(expected); -- To stop receiving notification emails like this one, please contact davidb@apache.org.