This is an automated email from the ASF dual-hosted git repository.
cziegeler 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 9b43030 Clarify update handling
9b43030 is described below
commit 9b43030ec12e6d36db8ee3e9705ce08be4e1d909
Author: Carsten Ziegeler <cziege...@apache.org>
AuthorDate: Fri Feb 23 08:19:02 2018 +0100
Clarify update handling
---
.../feature/support/json/FeatureJSONReader.java | 16 ---
.../feature/support/json/FeatureJSONWriter.java | 35 ++-----
.../sling/feature/support/json/JSONConstants.java | 12 +--
.../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 -
featuremodel/requirements.md | 14 ++-
8 files changed, 49 insertions(+), 230 deletions(-)
diff --git
a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java
b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java
index cffd875..d865398 100644
---
a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java
+++
b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java
@@ -135,22 +135,6 @@ public class FeatureJSONReader extends JSONReaderBase {
this.readRequirements(map);
this.readIncludes(map);
- if ( map.containsKey(JSONConstants.FEATURE_UPGRADEOF) ) {
- final Object idObj = map.get(JSONConstants.FEATURE_UPGRADEOF);
- checkType(JSONConstants.FEATURE_UPGRADEOF, idObj, String.class);
- this.feature.setUpgradeOf(ArtifactId.parse(idObj.toString()));
- }
- if ( map.containsKey(JSONConstants.FEATURE_UPGRADES) ) {
- final Object listObj = map.get(JSONConstants.FEATURE_UPGRADES);
- checkType(JSONConstants.FEATURE_UPGRADES, listObj, List.class);
- @SuppressWarnings("unchecked")
- final List<Object> list = (List<Object>) listObj;
- for(final Object element : list) {
- checkType(JSONConstants.FEATURE_UPGRADES, element,
String.class);
-
feature.getUpgrades().add(ArtifactId.parse(element.toString()));
- }
-
- }
this.readExtensions(map,
JSONConstants.FEATURE_KNOWN_PROPERTIES,
this.feature.getExtensions(),
this.feature.getConfigurations());
diff --git
a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONWriter.java
b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONWriter.java
index 1c299c9..13c82f1 100644
---
a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONWriter.java
+++
b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONWriter.java
@@ -16,6 +16,17 @@
*/
package org.apache.sling.feature.support.json;
+import static
org.apache.sling.feature.support.util.ManifestUtil.marshalAttribute;
+import static
org.apache.sling.feature.support.util.ManifestUtil.marshalDirective;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.util.List;
+import java.util.Map;
+
+import javax.json.Json;
+import javax.json.stream.JsonGenerator;
+
import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Capability;
import org.apache.sling.feature.Configuration;
@@ -24,16 +35,6 @@ import org.apache.sling.feature.Feature;
import org.apache.sling.feature.Include;
import org.apache.sling.feature.Requirement;
-import javax.json.Json;
-import javax.json.stream.JsonGenerator;
-import java.io.IOException;
-import java.io.Writer;
-import java.util.List;
-import java.util.Map;
-
-import static
org.apache.sling.feature.support.util.ManifestUtil.marshalAttribute;
-import static
org.apache.sling.feature.support.util.ManifestUtil.marshalDirective;
-
/**
* Simple JSON writer for a feature
@@ -72,20 +73,6 @@ public class FeatureJSONWriter extends JSONWriterBase {
writeProperty(w, JSONConstants.FEATURE_VENDOR, feature.getVendor());
writeProperty(w, JSONConstants.FEATURE_LICENSE, feature.getLicense());
- // upgradeOf
- if ( feature.getUpgradeOf() != null ) {
- writeProperty(w, JSONConstants.FEATURE_UPGRADEOF,
feature.getUpgradeOf().toMvnId());
- }
-
- // upgrades
- if ( !feature.getUpgrades().isEmpty() ) {
- w.writeStartArray(JSONConstants.FEATURE_UPGRADES);
- for(final ArtifactId id : feature.getUpgrades()) {
- w.write(id.toMvnId());
- }
- w.writeEnd();
- }
-
// includes
if ( !feature.getIncludes().isEmpty() ) {
w.writeStartArray(JSONConstants.FEATURE_INCLUDES);
diff --git
a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java
b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java
index d34bd80..8f1fce5 100644
---
a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java
+++
b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java
@@ -16,11 +16,11 @@
*/
package org.apache.sling.feature.support.json;
-import org.apache.sling.feature.Configuration;
-
import java.util.Arrays;
import java.util.List;
+import org.apache.sling.feature.Configuration;
+
public abstract class JSONConstants {
public static final String FEATURE_ID = "id";
@@ -45,10 +45,6 @@ public abstract class JSONConstants {
public static final String FEATURE_LICENSE = "license";
- public static final String FEATURE_UPGRADES = "upgrades";
-
- public static final String FEATURE_UPGRADEOF = "upgradeOf";
-
public static final List<String> FEATURE_KNOWN_PROPERTIES =
Arrays.asList(FEATURE_ID,
FEATURE_BUNDLES,
FEATURE_FRAMEWORK_PROPERTIES,
@@ -59,9 +55,7 @@ public abstract class JSONConstants {
FEATURE_TITLE,
FEATURE_DESCRIPTION,
FEATURE_VENDOR,
- FEATURE_LICENSE,
- FEATURE_UPGRADES,
- FEATURE_UPGRADEOF);
+ FEATURE_LICENSE);
public static final String ARTIFACT_ID = "id";
diff --git
a/featuremodel/feature/src/main/java/org/apache/sling/feature/Feature.java
b/featuremodel/feature/src/main/java/org/apache/sling/feature/Feature.java
index f0f2cc5..64c2488 100644
--- a/featuremodel/feature/src/main/java/org/apache/sling/feature/Feature.java
+++ b/featuremodel/feature/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/featuremodel/feature/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java
b/featuremodel/feature/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java
index cc0f235..1a555e1 100644
---
a/featuremodel/feature/src/main/java/org/apache/sling/feature/process/ApplicationBuilder.java
+++
b/featuremodel/feature/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/featuremodel/feature/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java
b/featuremodel/feature/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java
index 7208f00..6d57dc6 100644
---
a/featuremodel/feature/src/main/java/org/apache/sling/feature/process/FeatureBuilder.java
+++
b/featuremodel/feature/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/featuremodel/feature/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java
b/featuremodel/feature/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java
index 4fc31b3..f990aec 100644
---
a/featuremodel/feature/src/test/java/org/apache/sling/feature/process/FeatureBuilderTest.java
+++
b/featuremodel/feature/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);
diff --git a/featuremodel/requirements.md b/featuremodel/requirements.md
index cb74def..b5d61bf 100644
--- a/featuremodel/requirements.md
+++ b/featuremodel/requirements.md
@@ -60,19 +60,25 @@ The feature model is about describing a feature,
aggregating features to either
* SFM410 - It must be possible to specify the framework to launch an
application as part of the application model.
* SFM420 - When features are aggregated to either a higher level feature or an
application, the resulting feature or application must still contain the
variables.
* SFM430 - The startup order of features and bundles must be part of the
resulting aggregated application model.
-* SFM440 - The feature module should support the definition of updates to an
existing feature - that is, it only describes the delta to a given feature.
-* SFM450 - The feature model must support additional, optional information
about the feature like a human readable title, a description, vendor and
licensing information.
+* SFM440 - The feature model must support additional, optional information
about the feature like a human readable title, a description, vendor and
licensing information.
## Analysis Requirements
* SFA010 - Tooling must be able to compute the effective requirements of a
feature by inspecting the feature's content and combining this with
requirements specified on the feature itself.
* SFA020 - Tooling must be able to compute the capabilities of a feature by
inspecting the feature's content and directly specified capabilities.
-* SFA030 - The feature model should support to store the results of SFA010 and
SFA020 as port of the model, avoiding duplicate calculations.
+* SFA030 - The feature model should support to store the results of SFA010 and
SFA020 as part of the model, avoiding duplicate calculations.
## Resolving Requirements
* SFR010 - Tooling must be able to find all features that provide the
capabilities required by a given feature, from a set of available features.
+## Packaging requirements
+
+* SFP010 - Tooling must be able to convert a feature to an Apache Karaf Feature
+* SFP020 - Tooling must be able to convert a feature to an Apache Sling
Provisioning Model Archive
+* SFP030 - Tooling must be able to convert a feature to an OSGi Subsystem
+* SFP040 - Tooling should be able to create diff packages between two versions
of a features
+
## Launching Requirements
* SFL010 - Tooling must support creating an application model out of one or
more features.
@@ -80,6 +86,8 @@ The feature model is about describing a feature, aggregating
features to either
* SFL030 - Tooling must be able to introspect and potentially override the
startup order of bundles for an application.
* SFL040 - Tooling must support substitution of variable values at launch time.
* SFL050 - When an application is started, the install and the startup order
of bundles should be the same, ensuring that the bundles are shutdown in
reverse order and started in the same order on next startup of the framework.
+* SFL060 - Tooling must support multiple versions of a feature and only select
the highest version of a feature to launch the application.
+* SFL070 - Tooling must be able to differentiate between a feature and an
update of a feature. An update should only be included in the application of
the base feature is included.
## Runtime Requirements
--
To stop receiving notification emails like this one, please contact
cziege...@apache.org.
