You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2018/11/06 13:15:00 UTC

[GitHub] bosschaert commented on a change in pull request #2: SLING-8077 - refactor ContentorderMergeProcessor to MergeHandler adju…

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


With regards,
Apache Git Services