You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2018/04/27 09:51:29 UTC
[sling-org-apache-sling-feature] 09/22: Clarify update handling
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 <cz...@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.