[ 
https://issues.apache.org/jira/browse/SLING-8077?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16676737#comment-16676737
 ] 

ASF GitHub Bot commented on SLING-8077:
---------------------------------------

bosschaert commented on a change in pull request #2: SLING-8077 - refactor 
ContentorderMergeProcessor to MergeHandler adju…
URL: 
https://github.com/apache/sling-org-apache-sling-feature-extension-content/pull/2#discussion_r231116555
 
 

 ##########
 File path: 
src/main/java/org/apache/sling/feature/extension/content/ContentOrderMergeProcessor.java
 ##########
 @@ -23,25 +23,46 @@
 import org.apache.sling.feature.FeatureConstants;
 import org.apache.sling.feature.KeyValueMap;
 import org.apache.sling.feature.builder.HandlerContext;
+import org.apache.sling.feature.builder.MergeHandler;
 import org.apache.sling.feature.builder.PostProcessHandler;
 
-public class ContentOrderMergeProcessor implements PostProcessHandler {
+public class ContentOrderMergeProcessor implements MergeHandler {
 
     private static final String DEFAULT_CONTENT_START_ORDER = 
"default.content.startorder";
 
-    @Override
-    public void postProcess(HandlerContext context, Feature feature, Extension 
extension) {
-        if (extension.getType() == ExtensionType.ARTIFACTS
-                && 
extension.getName().equals(FeatureConstants.EXTENSION_NAME_CONTENT_PACKAGES)) {
-            String defaultOrder = 
feature.getVariables().get(DEFAULT_CONTENT_START_ORDER);
-            if (defaultOrder != null) {
-                for (Artifact a : extension.getArtifacts()) {
-                    KeyValueMap kvm = a.getMetadata();
-                    if(kvm.get(Artifact.KEY_START_ORDER) == null) {
-                        kvm.put(Artifact.KEY_START_ORDER, defaultOrder);
-                    }
+    private void processFeature(HandlerContext context, Feature feature, 
Extension extension) {
+        String defaultOrder = 
feature.getVariables().get(DEFAULT_CONTENT_START_ORDER);
+        if (defaultOrder != null) {
+            for (Artifact a : extension.getArtifacts()) {
+                KeyValueMap kvm = a.getMetadata();
+                if(kvm.get(Artifact.KEY_START_ORDER) == null) {
+                    kvm.put(Artifact.KEY_START_ORDER, defaultOrder);
                 }
-                feature.getVariables().remove(DEFAULT_CONTENT_START_ORDER);
+            }
+            feature.getVariables().remove(DEFAULT_CONTENT_START_ORDER);
+        }
+    }
+
+    @Override
+    public boolean canMerge(Extension extension) {
+        return extension.getType() == ExtensionType.ARTIFACTS
+                && 
extension.getName().equals(FeatureConstants.EXTENSION_NAME_CONTENT_PACKAGES);
+    }
+
+    @Override
+    public void merge(HandlerContext context, Feature target, Feature source, 
Extension targetEx, Extension sourceEx) {
+        processFeature(context, target, targetEx);
 
 Review comment:
   Hi @DominikSuess with this commit 
https://github.com/apache/sling-org-apache-sling-feature/commit/f7e74ebe1532c13ebfa96a935fa6fdcc544b6b43
 the `target` argument can also be `null`. Basically the handler receives an 
extra call when the first extension is applied to the aggregate. Some handlers 
also need to be able to process those. 
   I think in your case you don't need to do something in that case, so you 
could just add
   ```
   if (target == null) return
   ```
   at the top of this method.

----------------------------------------------------------------
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:
us...@infra.apache.org


> Transform ContentOrderMergeProcessor into MergeHandler
> ------------------------------------------------------
>
>                 Key: SLING-8077
>                 URL: https://issues.apache.org/jira/browse/SLING-8077
>             Project: Sling
>          Issue Type: Improvement
>          Components: Feature Model
>            Reporter: Dominik Süß
>            Priority: Major
>
> ContentOrderMergeProcessor as implemented in "Featuremodel - Content 
> Deployment Exension" currently postprocesses features after merge to set the 
> default start-order. this only works for cases where features with same order 
> are getting merged and mandates a sequential merge of features of different 
> order. 
> To tackle this issue the extension should take over the actual merge which 
> allows to preprocess the features before merging and setting the defaults 
> according to the defaults declared in the particular features before doing 
> the merge.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to