sling-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] bosschaert commented on a change in pull request #2: SLING-8077 - refactor ContentorderMergeProcessor to MergeHandler adju…
Date Tue, 06 Nov 2018 13:15:00 GMT
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

Mime
View raw message