You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2020/09/29 16:35:04 UTC
[sling-slingfeature-maven-plugin] branch master updated: SLING-9656
gracefully handle scenarios where the AggregateFeaturesMojo gets invoked
more than once during the build (#58)
This is an automated email from the ASF dual-hosted git repository.
enorman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-slingfeature-maven-plugin.git
The following commit(s) were added to refs/heads/master by this push:
new ad85d8b SLING-9656 gracefully handle scenarios where the AggregateFeaturesMojo gets invoked more than once during the build (#58)
ad85d8b is described below
commit ad85d8b9ea5c26e52c5df7156da6be41d5277d18
Author: Eric Norman <er...@gmail.com>
AuthorDate: Tue Sep 29 09:34:58 2020 -0700
SLING-9656 gracefully handle scenarios where the AggregateFeaturesMojo gets invoked more than once during the build (#58)
* SLING-9656 gracefully handle scenarios where the AggregateFeaturesMojo
gets invoked more than once during the build
---
.../sling/feature/maven/mojos/Aggregate.java | 32 ++++
.../feature/maven/mojos/AggregateFeaturesMojo.java | 35 +++-
.../feature/maven/mojos/FeatureLauncherMojo.java | 10 +-
.../maven/mojos/FeatureSelectionConfig.java | 47 +++++
.../maven/mojos/AggregateFeaturesMojoTest.java | 210 ++++++++++++++++++++-
5 files changed, 327 insertions(+), 7 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/maven/mojos/Aggregate.java b/src/main/java/org/apache/sling/feature/maven/mojos/Aggregate.java
index d2efdae..ef770fc 100644
--- a/src/main/java/org/apache/sling/feature/maven/mojos/Aggregate.java
+++ b/src/main/java/org/apache/sling/feature/maven/mojos/Aggregate.java
@@ -20,6 +20,7 @@ import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.stream.Collectors;
import org.apache.sling.feature.ArtifactId;
@@ -71,6 +72,37 @@ public class Aggregate extends FeatureSelectionConfig {
public Map<String, String> frameworkPropertiesOverrides;
+ /* (non-Javadoc)
+ * @see java.lang.Object#hashCode()
+ */
+ @Override
+ public int hashCode() {
+ return Objects.hash(super.hashCode(), artifactsOverrides, attach, classifier, configurationOverrides, description,
+ frameworkPropertiesOverrides, markAsComplete, markAsFinal, title, variablesOverrides, vendor);
+ }
+
+ /* (non-Javadoc)
+ * @see java.lang.Object#equals(java.lang.Object)
+ */
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (!super.equals(obj))
+ return false;
+ if (getClass() != obj.getClass())
+ return false;
+ Aggregate other = (Aggregate) obj;
+ return Objects.equals(artifactsOverrides, other.artifactsOverrides) && attach == other.attach
+ && Objects.equals(classifier, other.classifier)
+ && Objects.equals(configurationOverrides, other.configurationOverrides)
+ && Objects.equals(description, other.description)
+ && Objects.equals(frameworkPropertiesOverrides, other.frameworkPropertiesOverrides)
+ && markAsComplete == other.markAsComplete && markAsFinal == other.markAsFinal
+ && Objects.equals(title, other.title) && Objects.equals(variablesOverrides, other.variablesOverrides)
+ && Objects.equals(vendor, other.vendor);
+ }
+
@Override
public String toString() {
return "Aggregate [selection=" + getSelections() + ", filesExcludes=" + getFilesExcludes()
diff --git a/src/main/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojo.java b/src/main/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojo.java
index b987975..5eae489 100644
--- a/src/main/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojo.java
+++ b/src/main/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojo.java
@@ -53,6 +53,9 @@ public class AggregateFeaturesMojo extends AbstractIncludingFeatureMojo {
private static final String FILE_STORAGE_CONFIG_KEY = "fileStorage";
private static final String HANDLER_CONFIG_WILDCARD = "all";
+ /* A context flag to track if we have already been processed */
+ private static final String PROPERTY_HANDLED_AGGREGATE_FEATURES = AggregateFeaturesMojo.class.getName() + "/generated";
+
/**
* The definition of the features used to create the new feature.
*/
@@ -65,7 +68,31 @@ public class AggregateFeaturesMojo extends AbstractIncludingFeatureMojo {
@Override
public void execute() throws MojoExecutionException {
checkPreconditions();
+
+ // SLING-9656 - make sure to process each aggregate feature only once
+ @SuppressWarnings("unchecked")
+ Map<Aggregate, Feature> handledAggregates = (Map<Aggregate, Feature>)this.project.getContextValue(PROPERTY_HANDLED_AGGREGATE_FEATURES);
+ if (handledAggregates == null) {
+ handledAggregates = new HashMap<>();
+ this.project.setContextValue(PROPERTY_HANDLED_AGGREGATE_FEATURES, handledAggregates);
+ }
+
for (final Aggregate aggregate : aggregates) {
+ final String aggregateFeatureKey = ProjectHelper.generateAggregateFeatureKey(aggregate.classifier, aggregate.attach);
+
+ // SLING-9656 - check if we have already processed this one
+ Feature processedFeature = handledAggregates.get(aggregate);
+ if (processedFeature != null) {
+ getLog().debug("Found previously processed aggregate-feature " + aggregateFeatureKey);
+ if (ProjectHelper.getAssembledFeatures(project).remove(aggregateFeatureKey, processedFeature)) {
+ getLog().debug(" Removed previous aggregate feature '" + aggregateFeatureKey + "' from the project assembled features map");
+ }
+
+ if (ProjectHelper.getFeatures(this.project).remove(aggregateFeatureKey, processedFeature)) {
+ getLog().debug(" Removed previous aggregate feature '" + aggregateFeatureKey + "' from the project features map");
+ }
+ }
+
// check classifier
ProjectHelper.validateFeatureClassifiers(this.project, aggregate.classifier, aggregate.attach);
@@ -152,9 +179,11 @@ public class AggregateFeaturesMojo extends AbstractIncludingFeatureMojo {
ProjectHelper.setFeatureInfo(project, result);
// Add feature to map of features
- final String key = ProjectHelper.generateAggregateFeatureKey(aggregate.classifier, aggregate.attach);
- ProjectHelper.getAssembledFeatures(project).put(key, result);
- ProjectHelper.getFeatures(this.project).put(key, result);
+ ProjectHelper.getAssembledFeatures(project).put(aggregateFeatureKey, result);
+ ProjectHelper.getFeatures(this.project).put(aggregateFeatureKey, result);
+
+ // SLING-9656 - remember that we have already processed this one
+ handledAggregates.put(aggregate, result);
}
}
diff --git a/src/main/java/org/apache/sling/feature/maven/mojos/FeatureLauncherMojo.java b/src/main/java/org/apache/sling/feature/maven/mojos/FeatureLauncherMojo.java
index be10dbf..5441c78 100644
--- a/src/main/java/org/apache/sling/feature/maven/mojos/FeatureLauncherMojo.java
+++ b/src/main/java/org/apache/sling/feature/maven/mojos/FeatureLauncherMojo.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.io.Writer;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
+import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -36,8 +37,6 @@ import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Feature;
import org.apache.sling.feature.io.json.FeatureJSONWriter;
-import com.google.common.io.Files;
-
/**
* Launches the given Feature File
*/
@@ -184,7 +183,12 @@ public class FeatureLauncherMojo extends AbstractIncludingFeatureMojo {
getLog().info("Features from Selection: " + features);
for (Feature feature : features) {
// Loop over all features found, create a temporary file, write the features there and add them to the launcher's file list
- File folder = Files.createTempDir();
+ File folder;
+ try {
+ folder = Files.createTempDirectory("features").toFile();
+ } catch (IOException e1) {
+ throw new MojoExecutionException("Failed to create temp directory", e1);
+ }
ArtifactId id = feature.getId();
File featureFile = new File(folder, id.toMvnId().replaceAll(":", "-") + ".json");
// TODO: Do we need to support Prototypes etc?
diff --git a/src/main/java/org/apache/sling/feature/maven/mojos/FeatureSelectionConfig.java b/src/main/java/org/apache/sling/feature/maven/mojos/FeatureSelectionConfig.java
index 0108279..0711a5f 100644
--- a/src/main/java/org/apache/sling/feature/maven/mojos/FeatureSelectionConfig.java
+++ b/src/main/java/org/apache/sling/feature/maven/mojos/FeatureSelectionConfig.java
@@ -18,6 +18,7 @@ package org.apache.sling.feature.maven.mojos;
import java.util.ArrayList;
import java.util.List;
+import java.util.Objects;
import org.apache.maven.model.Dependency;
import org.apache.sling.feature.maven.ProjectHelper;
@@ -37,6 +38,29 @@ public class FeatureSelectionConfig {
this.instruction = instruction;
}
+ /* (non-Javadoc)
+ * @see java.lang.Object#hashCode()
+ */
+ @Override
+ public int hashCode() {
+ return Objects.hash(instruction, type);
+ }
+
+ /* (non-Javadoc)
+ * @see java.lang.Object#equals(java.lang.Object)
+ */
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (obj == null)
+ return false;
+ if (getClass() != obj.getClass())
+ return false;
+ Selection other = (Selection) obj;
+ return Objects.equals(instruction, other.instruction) && type == other.type;
+ }
+
@Override
public String toString() {
return "Selection [type=" + type + ", instruction=" + instruction + "]";
@@ -83,6 +107,29 @@ public class FeatureSelectionConfig {
return this.selections;
}
+ /* (non-Javadoc)
+ * @see java.lang.Object#hashCode()
+ */
+ @Override
+ public int hashCode() {
+ return Objects.hash(filesExcludes, selections);
+ }
+
+ /* (non-Javadoc)
+ * @see java.lang.Object#equals(java.lang.Object)
+ */
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (obj == null)
+ return false;
+ if (getClass() != obj.getClass())
+ return false;
+ FeatureSelectionConfig other = (FeatureSelectionConfig) obj;
+ return Objects.equals(filesExcludes, other.filesExcludes) && Objects.equals(selections, other.selections);
+ }
+
@Override
public String toString() {
return "FeatureSelectionConfig [selections=" + selections + ", filesExcludes=" + filesExcludes + "]";
diff --git a/src/test/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojoTest.java b/src/test/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojoTest.java
index f1d0ce4..69cea64 100644
--- a/src/test/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojoTest.java
+++ b/src/test/java/org/apache/sling/feature/maven/mojos/AggregateFeaturesMojoTest.java
@@ -17,12 +17,14 @@
package org.apache.sling.feature.maven.mojos;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.File;
import java.io.FileReader;
+import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -62,7 +64,6 @@ import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
-@SuppressWarnings("deprecation")
public class AggregateFeaturesMojoTest {
private Path tempDir;
private static Map<String, ArtifactId> pluginCallbacks;
@@ -866,6 +867,213 @@ public class AggregateFeaturesMojoTest {
ArtifactId.fromMvnId("org.apache.sling:somebundle:1.0.0"))));
}
+
+ /**
+ * Sling-9656 - verify that equals works for two equivalent Aggregate objects
+ */
+ @Test
+ public void testAggregateEquals() throws Exception {
+ Aggregate ag = new Aggregate();
+ ag.classifier = "myagg";
+ ag.attach = true;
+ ag.markAsFinal = true;
+ ag.markAsComplete = true;
+ ag.title = "title";
+ ag.description = "description";
+ ag.vendor = "vendor";
+ ag.artifactsOverrides = Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:1.1.0", "org.apache.sling:somebundle:2.0.0");
+ ag.configurationOverrides = Arrays.asList("one");
+ ag.variablesOverrides = Collections.singletonMap("key", "value");
+ ag.frameworkPropertiesOverrides = Collections.singletonMap("key", "value");
+ ag.setFilesExclude("file1.json");
+ ag.setFilesInclude("file2.json");
+
+ Aggregate ag2 = new Aggregate();
+ ag2.classifier = "myagg";
+ ag2.attach = true;
+ ag2.markAsFinal = true;
+ ag2.markAsComplete = true;
+ ag2.title = "title";
+ ag2.description = "description";
+ ag2.vendor = "vendor";
+ ag2.artifactsOverrides = Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:1.1.0", "org.apache.sling:somebundle:2.0.0");
+ ag2.configurationOverrides = Arrays.asList("one");
+ ag2.variablesOverrides = Collections.singletonMap("key", "value");
+ ag2.frameworkPropertiesOverrides = Collections.singletonMap("key", "value");
+ ag2.setFilesExclude("file1.json");
+ ag2.setFilesInclude("file2.json");
+
+ assertEquals(ag, ag2);
+
+ // hashCode should be equal too
+ assertEquals(ag.hashCode(), ag2.hashCode());
+
+ Object [][] fieldChanges = new Object[][] {
+ {"classifier", "myagg2"},
+ {"attach", false},
+ {"markAsFinal", false},
+ {"markAsComplete", false},
+ {"title", "title2"},
+ {"description", "description2"},
+ {"vendor", "vendor2"},
+ {"artifactsOverrides", Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:2.2.0", "org.apache.sling:somebundle:3.0.0")},
+ {"configurationOverrides", Arrays.asList("two")},
+ {"variablesOverrides", Collections.singletonMap("key2", "value2")},
+ {"frameworkPropertiesOverrides", Collections.singletonMap("key2", "value2")}
+ };
+
+ // change something in each field to make them not equal
+ for (Object[] objects : fieldChanges) {
+ String fieldName = (String)objects[0];
+ Field field = ag2.getClass().getField(fieldName);
+ Object originalValue = field.get(ag2);
+ try {
+ field.set(ag2, objects[1]);
+
+ // now the two object should no longer be equal
+ assertNotEquals("expected not equal after changing field: " + fieldName, ag, ag2);
+ assertNotEquals("expected hashCode not equal after changing field: " + fieldName, ag.hashCode(), ag2.hashCode());
+ } finally {
+ // put the old value back
+ field.set(ag2, originalValue);
+ }
+ }
+
+ // also check equals afterchanges to non-field data
+ ag2.setFilesExclude("file3.json");
+ ag2.setFilesInclude("file4.json");
+
+ // now the two object should no longer be equal
+ assertNotEquals("expected not equal after changing included/excluded files", ag, ag2);
+ assertNotEquals("expected hashCode not equal after changing included/excluded files", ag.hashCode(), ag2.hashCode());
+ }
+
+ /**
+ * Sling-9656 - verify gracefully handling of scenarios where the AggregateFeaturesMojo gets invoked
+ * more than once with equivalent configuration during the build
+ */
+ @Test
+ public void testAggregateFeaturesInvokedMultipleTimes() throws Exception {
+ File featuresDir = new File(
+ getClass().getResource("/aggregate-features/dir5").getFile());
+
+ Map<String, Feature> featureMap = new HashMap<>();
+ for (File f : featuresDir.listFiles((d,f) -> f.endsWith(".json"))) {
+ Feature feat = FeatureJSONReader.read(new FileReader(f), null);
+ featureMap.put(f.getAbsolutePath(), feat);
+ }
+
+ Aggregate ag = new Aggregate();
+ ag.setFilesInclude("*.json");
+ ag.classifier = "myagg";
+ ag.artifactsOverrides = Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:1.1.0", "org.apache.sling:somebundle:2.0.0");
+
+ Build mockBuild = Mockito.mock(Build.class);
+ Mockito.when(mockBuild.getDirectory()).thenReturn(tempDir.toString());
+
+ Artifact parentArt = createMockArtifact();
+ MavenProject mockProj = Mockito.mock(MavenProject.class);
+ Mockito.when(mockProj.getBuild()).thenReturn(mockBuild);
+ Mockito.when(mockProj.getId()).thenReturn("test.aggregate.project1");
+ Mockito.when(mockProj.getGroupId()).thenReturn("org.apache.sling");
+ Mockito.when(mockProj.getArtifactId()).thenReturn("org.apache.sling.test");
+ Mockito.when(mockProj.getVersion()).thenReturn("1.0.1");
+ Mockito.when(mockProj.getArtifact()).thenReturn(parentArt);
+ Mockito.when(mockProj.getContextValue(Feature.class.getName() + "/rawmain.json-cache"))
+ .thenReturn(featureMap);
+ Mockito.when(mockProj.getContextValue(Feature.class.getName() + "/assembledmain.json-cache"))
+ .thenReturn(featureMap);
+ Mockito.when(mockProj.getContextValue(Preprocessor.class.getName())).thenReturn(Boolean.TRUE);
+ Map<Aggregate, Feature> handledAggregates = new HashMap<>();
+ Mockito.when(mockProj.getContextValue(AggregateFeaturesMojo.class.getName() + "/generated")).thenReturn(handledAggregates);
+
+ AggregateFeaturesMojo af = new AggregateFeaturesMojo();
+ af.aggregates = Collections.singletonList(ag);
+ af.project = mockProj;
+ af.projectHelper = new DefaultMavenProjectHelper();
+ af.features = featuresDir;
+ af.handlerConfiguration = new HashMap<>();
+
+ // execute the first time
+ af.execute();
+
+ // and executing again with different (but equal) Aggregate objects should not fail either
+ Aggregate ag2 = new Aggregate();
+ ag2.setFilesInclude("*.json");
+ ag2.classifier = "myagg";
+ ag2.artifactsOverrides = Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:1.1.0", "org.apache.sling:somebundle:2.0.0");
+ af.aggregates = Collections.singletonList(ag2);
+
+ af.execute();
+ }
+
+ /**
+ * Sling-9656 - verify configuration with duplicate Aggregate classifiers fails
+ */
+ @Test
+ public void testAggregateFeaturesDuplicateClassifier() throws Exception {
+ File featuresDir = new File(
+ getClass().getResource("/aggregate-features/dir5").getFile());
+
+ Map<String, Feature> featureMap = new HashMap<>();
+ for (File f : featuresDir.listFiles((d,f) -> f.endsWith(".json"))) {
+ Feature feat = FeatureJSONReader.read(new FileReader(f), null);
+ featureMap.put(f.getAbsolutePath(), feat);
+ }
+
+ Aggregate ag = new Aggregate();
+ ag.setFilesInclude("*.json");
+ ag.classifier = "myagg";
+ ag.artifactsOverrides = Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:1.1.0", "org.apache.sling:somebundle:2.0.0");
+
+ // a second different aggregate with the same classifier
+ Aggregate ag2 = new Aggregate();
+ ag2.setFilesInclude("*.json");
+ ag2.classifier = "myagg";
+ ag2.artifactsOverrides = Arrays.asList("org.apache.sling:mybundle:HIGHEST",
+ "org.apache.sling:somebundle:1.1.0", "org.apache.sling:somebundle:2.0.2");
+
+ Build mockBuild = Mockito.mock(Build.class);
+ Mockito.when(mockBuild.getDirectory()).thenReturn(tempDir.toString());
+
+ Artifact parentArt = createMockArtifact();
+ MavenProject mockProj = Mockito.mock(MavenProject.class);
+ Mockito.when(mockProj.getBuild()).thenReturn(mockBuild);
+ Mockito.when(mockProj.getId()).thenReturn("test.aggregate.project1");
+ Mockito.when(mockProj.getGroupId()).thenReturn("org.apache.sling");
+ Mockito.when(mockProj.getArtifactId()).thenReturn("org.apache.sling.test");
+ Mockito.when(mockProj.getVersion()).thenReturn("1.0.1");
+ Mockito.when(mockProj.getArtifact()).thenReturn(parentArt);
+ Mockito.when(mockProj.getContextValue(Feature.class.getName() + "/rawmain.json-cache"))
+ .thenReturn(featureMap);
+ Mockito.when(mockProj.getContextValue(Feature.class.getName() + "/assembledmain.json-cache"))
+ .thenReturn(featureMap);
+ Mockito.when(mockProj.getContextValue(Preprocessor.class.getName())).thenReturn(Boolean.TRUE);
+ Map<Aggregate, Feature> handledAggregates = new HashMap<>();
+ Mockito.when(mockProj.getContextValue(AggregateFeaturesMojo.class.getName() + "/generated")).thenReturn(handledAggregates);
+
+ AggregateFeaturesMojo af = new AggregateFeaturesMojo();
+ af.aggregates = Arrays.asList(ag, ag2);
+ af.project = mockProj;
+ af.projectHelper = new DefaultMavenProjectHelper();
+ af.features = featuresDir;
+ af.handlerConfiguration = new HashMap<>();
+
+ try {
+ af.execute();
+
+ fail("Expected RuntimeException about duplicate aggregate classifier");
+ } catch (RuntimeException e) {
+ assertEquals("More than one feature file for classifier myagg in project test.aggregate.project1 : [aggregate myagg, aggregate myagg]", e.getMessage());
+ }
+ }
+
private Artifact createMockArtifact() {
Artifact parentArtifact = Mockito.mock(Artifact.class);
Mockito.when(parentArtifact.getGroupId()).thenReturn("gid");