[
https://issues.apache.org/jira/browse/SLING-8028?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16668809#comment-16668809
]
ASF GitHub Bot commented on SLING-8028:
---------------------------------------
bosschaert closed pull request #7: SLING-8028 Split FeatureExtensionHandler
into MergeHandler and PostProcessor
URL: https://github.com/apache/sling-org-apache-sling-feature/pull/7
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
index 79cc557..696ea8b 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
@@ -16,22 +16,23 @@
*/
package org.apache.sling.feature.builder;
+import org.apache.sling.feature.KeyValueMap;
+
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.sling.feature.KeyValueMap;
-
/**
* Builder context holds services used by {@link FeatureBuilder}.
*/
public class BuilderContext {
+ private final ArtifactProvider artifactProvider;
private final FeatureProvider provider;
-
- private final List<FeatureExtensionHandler> featureExtensionHandlers = new
CopyOnWriteArrayList<>();
+ private final List<MergeHandler> mergeExtensions = new
CopyOnWriteArrayList<>();
+ private final List<PostProcessHandler> postProcessExtensions = new
CopyOnWriteArrayList<>();
private final KeyValueMap variables = new KeyValueMap();
private final Map<String, String> properties = new LinkedHashMap<>();
@@ -41,19 +42,20 @@
* @param provider A provider providing the included features
* @throws IllegalArgumentException If feature provider is {@code null}
*/
- public BuilderContext(final FeatureProvider provider) {
- this(provider, null, null);
+ public BuilderContext(final FeatureProvider provider, final
ArtifactProvider ap) {
+ this(provider, ap, null, null);
}
/**
* Create a new context
*
* @param provider A provider providing the included features
+ * @param ap
* @param variables A map of variables to override on feature merge
* @param properties A map of framework properties to override on feature
merge
* @throws IllegalArgumentException If feature provider is {@code null}
*/
- public BuilderContext(final FeatureProvider provider, KeyValueMap
variables, Map<String, String> properties) {
+ public BuilderContext(final FeatureProvider provider, ArtifactProvider ap,
KeyValueMap variables, Map<String, String> properties) {
if (variables != null) {
this.variables.putAll(variables);
}
@@ -63,19 +65,24 @@ public BuilderContext(final FeatureProvider provider,
KeyValueMap variables, Map
if ( provider == null ) {
throw new IllegalArgumentException("Provider must not be null");
}
+ this.artifactProvider = ap;
this.provider = provider;
}
- /**
- * Add a feature extension handlers
- * @param handlers Handler(s) to add
- * @return This instance
- */
- public BuilderContext add(final FeatureExtensionHandler... handlers) {
- featureExtensionHandlers.addAll(Arrays.asList(handlers));
+ public BuilderContext addMergeExtensions(final MergeHandler... extensions)
{
+ mergeExtensions.addAll(Arrays.asList(extensions));
return this;
}
+ public BuilderContext addPostProcessExtensions(final PostProcessHandler...
extensions) {
+ postProcessExtensions.addAll(Arrays.asList(extensions));
+ return this;
+ }
+
+ ArtifactProvider getArtifactProvider() {
+ return this.artifactProvider;
+ }
+
KeyValueMap getVariables() {
return this.variables;
}
@@ -92,11 +99,15 @@ FeatureProvider getFeatureProvider() {
}
/**
- * Get the list of extension handlers
- * @return The list of handlers
+ * Get the list of merge extensions
+ * @return The list of extenion
*/
- List<FeatureExtensionHandler> getFeatureExtensionHandlers() {
- return this.featureExtensionHandlers;
+ List<MergeHandler> getMergeExtensions() {
+ return this.mergeExtensions;
+ }
+
+ List<PostProcessHandler> getPostProcessExtensions() {
+ return this.postProcessExtensions;
}
/**
@@ -105,8 +116,9 @@ FeatureProvider getFeatureProvider() {
* @return Cloned context
*/
BuilderContext clone(final FeatureProvider featureProvider) {
- final BuilderContext ctx = new BuilderContext(featureProvider,
this.variables, this.properties);
- ctx.featureExtensionHandlers.addAll(featureExtensionHandlers);
+ final BuilderContext ctx = new BuilderContext(featureProvider,
this.artifactProvider, this.variables, this.properties);
+ ctx.mergeExtensions.addAll(mergeExtensions);
+ ctx.postProcessExtensions.addAll(postProcessExtensions);
return ctx;
}
}
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
index 02bccdf..aafc2f9 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -260,9 +260,9 @@ static void mergeExtensions(final Feature target,
+ " : " + current.getType() + " and " +
ext.getType());
}
boolean handled = false;
- for(final FeatureExtensionHandler fem :
context.getFeatureExtensionHandlers()) {
- if ( fem.canMerge(current) ) {
- fem.merge(target, source, current);
+ for(final MergeHandler me : context.getMergeExtensions()) {
+ if ( me.canMerge(current) ) {
+ me.merge(() -> context.getArtifactProvider(),
target, source, current, ext);
handled = true;
break;
}
@@ -279,8 +279,8 @@ static void mergeExtensions(final Feature target,
}
// post processing
for(final Extension ext : target.getExtensions()) {
- for(final FeatureExtensionHandler fem :
context.getFeatureExtensionHandlers()) {
- fem.postProcess(target, ext);
+ for(final PostProcessHandler ppe :
context.getPostProcessExtensions()) {
+ ppe.postProcess(() -> context.getArtifactProvider(), target,
ext);
}
}
}
diff --git a/src/main/java/org/apache/sling/feature/builder/HandlerContext.java
b/src/main/java/org/apache/sling/feature/builder/HandlerContext.java
new file mode 100644
index 0000000..fc18057
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/builder/HandlerContext.java
@@ -0,0 +1,29 @@
+/*
+
+ * 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.builder;
+
+/**
+ * Context for an extension handler.
+ */
+public interface HandlerContext {
+ /**
+ * Provide the artifact provider.
+ * @return The artifact provider.
+ */
+ ArtifactProvider getArtifactProvider();
+}
diff --git
a/src/main/java/org/apache/sling/feature/builder/FeatureExtensionHandler.java
b/src/main/java/org/apache/sling/feature/builder/MergeHandler.java
similarity index 70%
rename from
src/main/java/org/apache/sling/feature/builder/FeatureExtensionHandler.java
rename to src/main/java/org/apache/sling/feature/builder/MergeHandler.java
index 8434b5a..39cf543 100644
---
a/src/main/java/org/apache/sling/feature/builder/FeatureExtensionHandler.java
+++ b/src/main/java/org/apache/sling/feature/builder/MergeHandler.java
@@ -21,12 +21,10 @@
import org.osgi.annotation.versioning.ConsumerType;
/**
- * A feature extension handler can merge a feature of a particular type
- * and also post process the final assembled feature.
+ * A feature merge handler can merge a feature of a particular type.
*/
@ConsumerType
-public interface FeatureExtensionHandler {
-
+public interface MergeHandler {
/**
* Checks whether this merger can merge extensions with that name
* @param extension The extension
@@ -41,20 +39,11 @@
* extensions share the same name and type and that
* {@link #canMerge(Extension)} returned {@code true}.
*
+ * @param context Context for the handler
* @param target The target feature
* @param source The source feature
* @param extension The extension
* @throws IllegalStateException If the extensions can't be merged
*/
- void merge(Feature target, Feature source, Extension extension);
-
- /**
- * Post process the feature with respect to the extension.
- * Post processing is invoked after all extensions have been merged.
- * This method is called regardless whether {@link #canMerge(Extension)}
returned {@code true} or not.
- * @param feature The feature
- * @param extension The extension
- * @throws IllegalStateException If post processing failed
- */
- void postProcess(Feature feature, Extension extension);
+ void merge(HandlerContext context, Feature target, Feature source,
Extension targetEx, Extension sourceEx);
}
diff --git
a/src/main/java/org/apache/sling/feature/builder/PostProcessHandler.java
b/src/main/java/org/apache/sling/feature/builder/PostProcessHandler.java
new file mode 100644
index 0000000..7717ff0
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/builder/PostProcessHandler.java
@@ -0,0 +1,38 @@
+/*
+ * 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.builder;
+
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.Feature;
+import org.osgi.annotation.versioning.ConsumerType;
+
+/**
+ * A Post Process Handler processes features after a merge operation.
+ */
+@ConsumerType
+public interface PostProcessHandler {
+ /**
+ * Post process the feature with respect to the extension.
+ * Post processing is invoked after all extensions have been merged.
+ *
+ * @param context Context for the handler
+ * @param feature The feature
+ * @param extension The extension
+ * @throws IllegalStateException If post processing failed
+ */
+ void postProcess(HandlerContext context, Feature feature, Extension
extension);
+}
diff --git
a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
index bfd5ec4..73c5f64 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -16,23 +16,6 @@
*/
package org.apache.sling.feature.builder;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.AbstractMap;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
import org.apache.felix.utils.resource.CapabilityImpl;
import org.apache.felix.utils.resource.RequirementImpl;
import org.apache.sling.feature.Artifact;
@@ -48,6 +31,23 @@
import org.osgi.resource.Capability;
import org.osgi.resource.Requirement;
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
public class FeatureBuilderTest {
private static final Map<String, Feature> FEATURES = new HashMap<>();
@@ -248,7 +248,7 @@ private void equals(final Feature expected, final Feature
actuals) {
assertFalse(base.isAssembled());
- final Feature assembled = FeatureBuilder.assemble(base, new
BuilderContext(provider));
+ final Feature assembled = FeatureBuilder.assemble(base, new
BuilderContext(provider, null));
equals(base, assembled);
}
@@ -313,7 +313,7 @@ private void equals(final Feature expected, final Feature
actuals) {
result.getConfigurations().add(co3);
// assemble
- final Feature assembled = FeatureBuilder.assemble(base, new
BuilderContext(provider));
+ final Feature assembled = FeatureBuilder.assemble(base, new
BuilderContext(provider, null));
// and test
equals(result, assembled);
@@ -331,12 +331,11 @@ private void equals(final Feature expected, final Feature
actuals) {
// assemble application, it should only contain feature b as a is
included by b
Feature[] features = FeatureBuilder.deduplicate(new BuilderContext(new
FeatureProvider() {
-
@Override
public Feature provide(ArtifactId id) {
return null;
}
- }), a, b);
+ }, null), a, b);
assertEquals(1, features.length);
assertEquals(idB, features[0].getId());
}
@@ -355,7 +354,7 @@ public Feature provide(ArtifactId id) {
public Feature provide(ArtifactId id) {
return null;
}
- }), a, b);
+ }, null), a, b);
assertEquals(1, features.length);
assertEquals(idB, features[0].getId());
}
@@ -384,7 +383,7 @@ public Feature provide(ArtifactId id) {
}
return null;
}
- }));
+ }, null));
final Set<ArtifactId> set = new HashSet<>();
for(final Artifact c : feature.getBundles()) {
set.add(c.getId());
@@ -418,7 +417,7 @@ public Feature provide(ArtifactId id) {
}
return null;
}
- }));
+ }, null));
final Set<ArtifactId> set = new HashSet<>();
for(final Artifact c : feature.getBundles()) {
set.add(c.getId());
@@ -450,7 +449,7 @@ public Feature provide(ArtifactId id) {
}
return null;
}
- }));
+ }, null));
fail();
} catch ( final IllegalStateException ise) {
// expected
@@ -483,7 +482,7 @@ public Feature provide(ArtifactId id) {
}
return null;
}
- }));
+ }, null));
final Set<ArtifactId> set = new HashSet<>();
for(final Artifact c :
feature.getExtensions().getByName("foo").getArtifacts()) {
set.add(c.getId());
@@ -519,7 +518,7 @@ public Feature provide(ArtifactId id) {
}
return null;
}
- }));
+ }, null));
final Set<ArtifactId> set = new HashSet<>();
for(final Artifact c :
feature.getExtensions().getByName("foo").getArtifacts()) {
set.add(c.getId());
@@ -553,7 +552,7 @@ public Feature provide(ArtifactId id) {
}
return null;
}
- }));
+ }, null));
fail();
} catch ( final IllegalStateException ise) {
// expected
@@ -577,7 +576,7 @@ public Feature provide(ArtifactId id) {
public Feature provide(ArtifactId id) {
return null;
}
- }), a, b);
+ }, null), a, b);
final Extension list =
target.getExtensions().getByName(FeatureConstants.EXTENSION_NAME_ASSEMBLED_FEATURES);
assertNotNull(list);
assertEquals(1, list.getArtifacts().size());
@@ -626,7 +625,7 @@ public Feature provide(ArtifactId id)
{
return null;
}
- },override, null), aFeature, bFeature);
+ }, null, override, null), aFeature, bFeature);
KeyValueMap vars = new KeyValueMap();
vars.putAll(kvMapA);
@@ -647,7 +646,7 @@ public Feature provide(ArtifactId id)
{
return null;
}
- }, override, null), aFeature, bFeature);
+ }, null, override, null), aFeature, bFeature);
fail("Excepted merge exception");
} catch (IllegalStateException expected) {}
@@ -660,7 +659,7 @@ public Feature provide(ArtifactId id)
{
return null;
}
- }, override, null), aFeature, bFeature);
+ }, null, override, null), aFeature, bFeature);
vars = new KeyValueMap();
vars.putAll(kvMapA);
@@ -679,7 +678,7 @@ public Feature provide(ArtifactId id)
{
return null;
}
- }, override, null), aFeature, bFeature);
+ }, null, override, null), aFeature, bFeature);
vars.put("var2", null);
assertTrue(cFeature.getVariables().equals(vars));
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Split FeatureExtensionHandler into MergeHandler and PostProcessor
> -----------------------------------------------------------------
>
> Key: SLING-8028
> URL: https://issues.apache.org/jira/browse/SLING-8028
> Project: Sling
> Issue Type: New Feature
> Components: Feature Model
> Affects Versions: Feature Model 0.1.2
> Reporter: David Bosschaert
> Assignee: David Bosschaert
> Priority: Major
> Fix For: Feature Model 0.1.4
>
>
> The FeatureExtensionHandler class currently performs two tasks: merging and
> postprocessing. This should be split into two interfaces each with a clear
> purpose.
> The APIs should also be provided with a context object so that it will be
> possible to perform certain tasks that require plugins. For example mapping
> an Artifact ID to a file.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
