You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by si...@apache.org on 2019/06/24 12:40:25 UTC
[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-8529 -
Avoid flattening of subpackages during convertion in featuremodel
This is an automated email from the ASF dual-hosted git repository.
simonetripodi pushed a commit to branch avoid_subpackages_flattering
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit abb57436476eb81b16a9292e6f61e2f2ef86f24e
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Mon Jun 24 14:40:06 2019 +0200
SLING-8529 - Avoid flattening of subpackages during convertion in
featuremodel
initial checkin
---
.../ContentPackage2FeatureModelConverter.java | 82 ++++++++++++----------
.../cpconverter/artifacts/ArtifactsDeployer.java | 9 +--
.../artifacts/DefaultArtifactsDeployer.java | 33 ++++-----
.../artifacts/MavenPomSupplierWriter.java | 24 +++----
.../features/DefaultFeaturesManager.java | 18 ++---
.../cpconverter/features/FeaturesManager.java | 8 +--
.../cpconverter/handlers/BundleEntryHandler.java | 19 ++---
.../artifacts/DefaultBundlesDeployerTest.java | 19 +----
.../cpconverter/features/FeaturesManagerTest.java | 24 +------
.../handlers/BundleEntryHandlerTest.java | 3 +-
10 files changed, 88 insertions(+), 151 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java b/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java
index 522598d..3bd9fe8 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java
@@ -34,6 +34,7 @@ import org.apache.jackrabbit.vault.packaging.Dependency;
import org.apache.jackrabbit.vault.packaging.PackageId;
import org.apache.jackrabbit.vault.packaging.PackageProperties;
import org.apache.jackrabbit.vault.packaging.VaultPackage;
+import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.cpconverter.acl.AclManager;
import org.apache.sling.feature.cpconverter.artifacts.ArtifactsDeployer;
import org.apache.sling.feature.cpconverter.artifacts.FileArtifactWriter;
@@ -162,27 +163,10 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
for (VaultPackage vaultPackage : orderedContentPackages) {
try {
mainPackageAssembler = VaultPackageAssembler.create(vaultPackage);
- PackageId packageProperties = vaultPackage.getId();
-
- String group = requireNonNull(packageProperties.getGroup(),
- PackageProperties.NAME_GROUP
- + " property not found in content-package "
- + vaultPackage
- + ", please check META-INF/vault/properties.xml")
- .replace('/', '.');
-
- String name = requireNonNull(packageProperties.getName(),
- PackageProperties.NAME_NAME
- + " property not found in content-package "
- + vaultPackage
- + ", please check META-INF/vault/properties.xml");
-
- String version = packageProperties.getVersionString();
- if (version == null || version.isEmpty()) {
- version = DEFEAULT_VERSION;
- }
- featuresManager.init(group, name, version);
+ ArtifactId packageId = toArtifactId(vaultPackage);
+
+ featuresManager.init(packageId.getGroupId(), packageId.getArtifactId(), packageId.getVersion());
logger.info("Converting content-package '{}'...", vaultPackage.getId());
@@ -194,19 +178,9 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
// deploy the new zip content-package to the local mvn bundles dir
- artifactsDeployer.deploy(new FileArtifactWriter(contentPackageArchive),
- featuresManager.getTargetFeature().getId().getGroupId(),
- featuresManager.getTargetFeature().getId().getArtifactId(),
- featuresManager.getTargetFeature().getId().getVersion(),
- PACKAGE_CLASSIFIER,
- ZIP_TYPE);
+ artifactsDeployer.deploy(new FileArtifactWriter(contentPackageArchive), packageId);
- featuresManager.addArtifact(null,
- featuresManager.getTargetFeature().getId().getGroupId(),
- featuresManager.getTargetFeature().getId().getArtifactId(),
- featuresManager.getTargetFeature().getId().getVersion(),
- PACKAGE_CLASSIFIER,
- ZIP_TYPE);
+ featuresManager.addArtifact(null, packageId);
// finally serialize the Feature Model(s) file(s)
@@ -257,15 +231,27 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
return;
}
+ ArtifactId packageId = toArtifactId(vaultPackage);
+ VaultPackageAssembler clonedPackage = VaultPackageAssembler.create(vaultPackage);
+
+ // Please note: THIS IS A HACK to meet the new requirement without drastically change the original design
+ // temporary swap the main handler to collect stuff
+ VaultPackageAssembler handler = mainPackageAssembler;
+ mainPackageAssembler = clonedPackage;
+
// scan the detected package, first
traverse(vaultPackage);
- // merge filters to the main new package
- mainPackageAssembler.mergeFilters(vaultPackage.getMetaInf().getFilter());
+ File contentPackageArchive = clonedPackage.createPackage();
+
+ // deploy the new content-package to the local mvn bundles dir and attach it to the feature
+
+ artifactsDeployer.deploy(new FileArtifactWriter(contentPackageArchive), packageId);
- // add the metadata-only package one to the main package with overriden filter
- File clonedPackage = VaultPackageAssembler.createSynthetic(vaultPackage);
- mainPackageAssembler.addEntry(path, clonedPackage);
+ featuresManager.addArtifact(null, packageId);
+
+ // restore the previous assembler
+ mainPackageAssembler = handler;
}
protected boolean isSubContentPackageIncluded(String path) {
@@ -290,4 +276,26 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
entryHandler.handle(entryPath, archive, entry, this);
}
+ private static ArtifactId toArtifactId(VaultPackage vaultPackage) {
+ PackageId packageId = vaultPackage.getId();
+ String groupId = requireNonNull(packageId.getGroup(),
+ PackageProperties.NAME_GROUP
+ + " property not found in content-package "
+ + vaultPackage
+ + ", please check META-INF/vault/properties.xml").replace('/', '.');
+
+ String artifactid = requireNonNull(packageId.getName(),
+ PackageProperties.NAME_NAME
+ + " property not found in content-package "
+ + vaultPackage
+ + ", please check META-INF/vault/properties.xml");
+
+ String version = packageId.getVersionString();
+ if (version == null || version.isEmpty()) {
+ version = DEFEAULT_VERSION;
+ }
+
+ return new ArtifactId(groupId, artifactid, version, PACKAGE_CLASSIFIER, ZIP_TYPE);
+ }
+
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/artifacts/ArtifactsDeployer.java b/src/main/java/org/apache/sling/feature/cpconverter/artifacts/ArtifactsDeployer.java
index 27548cf..3e189fd 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/artifacts/ArtifactsDeployer.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/artifacts/ArtifactsDeployer.java
@@ -19,15 +19,12 @@ package org.apache.sling.feature.cpconverter.artifacts;
import java.io.File;
import java.io.IOException;
+import org.apache.sling.feature.ArtifactId;
+
public interface ArtifactsDeployer {
File getBundlesDirectory();
- void deploy(ArtifactWriter artifactWriter,
- String groupId,
- String artifactId,
- String version,
- String classifier,
- String type) throws IOException;
+ void deploy(ArtifactWriter artifactWriter, ArtifactId id) throws IOException;
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/artifacts/DefaultArtifactsDeployer.java b/src/main/java/org/apache/sling/feature/cpconverter/artifacts/DefaultArtifactsDeployer.java
index c728642..31b519b 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/artifacts/DefaultArtifactsDeployer.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/artifacts/DefaultArtifactsDeployer.java
@@ -23,6 +23,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.util.StringTokenizer;
+import org.apache.sling.feature.ArtifactId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -45,42 +46,34 @@ public final class DefaultArtifactsDeployer implements ArtifactsDeployer {
}
@Override
- public void deploy(ArtifactWriter artifactWriter,
- String groupId,
- String artifactId,
- String version,
- String classifier,
- String type) throws IOException {
+ public void deploy(ArtifactWriter artifactWriter, ArtifactId id) throws IOException {
requireNonNull(artifactWriter, "Null ArtifactWriter can not install an artifact to a Maven repository.");
- requireNonNull(groupId, "Bundle can not be installed to a Maven repository without specifying a valid 'groupId'.");
- requireNonNull(artifactId, "Bundle can not be installed to a Maven repository without specifying a valid 'artifactId'.");
- requireNonNull(version, "Bundle can not be installed to a Maven repository without specifying a valid 'version'.");
- requireNonNull(type, "Bundle can not be installed to a Maven repository without specifying a valid 'type'.");
+ requireNonNull(id, "Bundle can not be installed to a Maven repository without specifying a valid id.");
File targetDir = artifactsDirectory;
- StringTokenizer tokenizer = new StringTokenizer(groupId, ".");
+ StringTokenizer tokenizer = new StringTokenizer(id.getGroupId(), ".");
while (tokenizer.hasMoreTokens()) {
String current = tokenizer.nextToken();
targetDir = new File(targetDir, current);
}
- targetDir = new File(targetDir, artifactId);
- targetDir = new File(targetDir, version);
+ targetDir = new File(targetDir, id.getArtifactId());
+ targetDir = new File(targetDir, id.getVersion());
targetDir.mkdirs();
// deploy the main artifact
StringBuilder nameBuilder = new StringBuilder()
- .append(artifactId)
+ .append(id.getArtifactId())
.append('-')
- .append(version);
+ .append(id.getVersion());
- if (classifier != null) {
- nameBuilder.append('-').append(classifier);
+ if (id.getClassifier() != null) {
+ nameBuilder.append('-').append(id.getClassifier());
}
- nameBuilder.append('.').append(type);
+ nameBuilder.append('.').append(id.getType());
File targetFile = new File(targetDir, nameBuilder.toString());
@@ -94,10 +87,10 @@ public final class DefaultArtifactsDeployer implements ArtifactsDeployer {
// automatically deploy the supplied POM file
- targetFile = new File(targetDir, String.format("%s-%s.pom", artifactId, version));
+ targetFile = new File(targetDir, String.format("%s-%s.pom", id.getArtifactId(), id.getVersion()));
try (FileOutputStream targetStream = new FileOutputStream(targetFile)) {
- new MavenPomSupplierWriter(groupId, artifactId, version, type).write(targetStream);
+ new MavenPomSupplierWriter(id).write(targetStream);
}
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/artifacts/MavenPomSupplierWriter.java b/src/main/java/org/apache/sling/feature/cpconverter/artifacts/MavenPomSupplierWriter.java
index cff6b4c..1d09166 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/artifacts/MavenPomSupplierWriter.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/artifacts/MavenPomSupplierWriter.java
@@ -21,31 +21,23 @@ import java.io.OutputStream;
import org.apache.maven.model.Model;
import org.apache.maven.model.io.xpp3.MavenXpp3Writer;
+import org.apache.sling.feature.ArtifactId;
public final class MavenPomSupplierWriter implements ArtifactWriter {
- private final String groupId;
+ private final ArtifactId id;
- private final String artifactId;
-
- private final String version;
-
- private final String type;
-
- public MavenPomSupplierWriter(String groupId, String artifactId, String version, String type) {
- this.groupId = groupId;
- this.artifactId = artifactId;
- this.version = version;
- this.type = type;
+ public MavenPomSupplierWriter(ArtifactId id) {
+ this.id = id;
}
@Override
public void write(OutputStream outputStream) throws IOException {
Model model = new Model();
- model.setGroupId(groupId);
- model.setArtifactId(artifactId);
- model.setVersion(version);
- model.setPackaging(type);
+ model.setGroupId(id.getGroupId());
+ model.setArtifactId(id.getArtifactId());
+ model.setVersion(id.getVersion());
+ model.setPackaging(id.getType());
new MavenXpp3Writer().write(outputStream, model);
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java b/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java
index 526cd9e..f805eef 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java
@@ -109,23 +109,15 @@ public class DefaultFeaturesManager implements FeaturesManager {
return runModes.computeIfAbsent(runMode, k -> new Feature(newId));
}
- public void addArtifact(String runMode,
- String groupId,
- String artifactId,
- String version,
- String classifier,
- String type) {
- requireNonNull(groupId, "Artifact can not be attached to a feature without specifying a valid 'groupId'.");
- requireNonNull(artifactId, "Artifact can not be attached to a feature without specifying a valid 'artifactId'.");
- requireNonNull(version, "Artifact can not be attached to a feature without specifying a valid 'version'.");
- requireNonNull(type, "Artifact can not be attached to a feature without specifying a valid 'type'.");
-
- Artifact artifact = new Artifact(new ArtifactId(groupId, artifactId, version, classifier, type));
+ public void addArtifact(String runMode, ArtifactId id) {
+ requireNonNull(id, "Artifact can not be attached to a feature without specifying a valid ArtifactId.");
+
+ Artifact artifact = new Artifact(id);
Feature targetFeature = getRunMode(runMode);
Artifacts artifacts;
- if (ZIP_TYPE.equals(type) ) {
+ if (ZIP_TYPE.equals(id.getType()) ) {
Extensions extensions = targetFeature.getExtensions();
Extension extension = extensions.getByName(CONTENT_PACKAGES);
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/features/FeaturesManager.java b/src/main/java/org/apache/sling/feature/cpconverter/features/FeaturesManager.java
index 5c0b672..30851be 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/features/FeaturesManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/features/FeaturesManager.java
@@ -18,6 +18,7 @@ package org.apache.sling.feature.cpconverter.features;
import java.util.Dictionary;
+import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Feature;
public interface FeaturesManager {
@@ -28,12 +29,7 @@ public interface FeaturesManager {
Feature getRunMode(String runMode);
- void addArtifact(String runMode,
- String groupId,
- String artifactId,
- String version,
- String classifier,
- String type);
+ void addArtifact(String runMode, ArtifactId id);
void addConfiguration(String runMode, String pid, Dictionary<String, Object> configurationProperties);
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java
index 63b9533..1f78b72 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java
@@ -30,6 +30,7 @@ import java.util.regex.Pattern;
import org.apache.jackrabbit.vault.fs.io.Archive;
import org.apache.jackrabbit.vault.fs.io.Archive.Entry;
+import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter;
import org.apache.sling.feature.cpconverter.artifacts.InputStreamArtifactWriter;
import org.slf4j.Logger;
@@ -99,19 +100,11 @@ public final class BundleEntryHandler extends AbstractRegexEntryHandler {
}
try (InputStream input = archive.openInputStream(entry)) {
- converter.getArtifactsDeployer().deploy(new InputStreamArtifactWriter(input),
- groupId,
- artifactId,
- version,
- classifier,
- JAR_TYPE);
-
- converter.getFeaturesManager().addArtifact(runMode,
- groupId,
- artifactId,
- version,
- classifier,
- JAR_TYPE);
+ ArtifactId id = new ArtifactId(groupId, artifactId, version, classifier, JAR_TYPE);
+
+ converter.getArtifactsDeployer().deploy(new InputStreamArtifactWriter(input), id);
+
+ converter.getFeaturesManager().addArtifact(runMode, id);
}
}
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/artifacts/DefaultBundlesDeployerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/artifacts/DefaultBundlesDeployerTest.java
index d5f9f73..0b9b914 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/artifacts/DefaultBundlesDeployerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/artifacts/DefaultBundlesDeployerTest.java
@@ -54,27 +54,12 @@ public class DefaultBundlesDeployerTest {
@Test(expected = NullPointerException.class)
public void deployLocallyAndAttachRequiresNonNullInput() throws Exception {
- artifactDeployer.deploy(null, null, null, null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndAttachRequiresNonNullGroupId() throws Exception {
- artifactDeployer.deploy(mock(ArtifactWriter.class), null, null, null, null, null);
+ artifactDeployer.deploy(null, null);
}
@Test(expected = NullPointerException.class)
public void deployLocallyAndAttachRequiresNonNullArtifactId() throws Exception {
- artifactDeployer.deploy(mock(ArtifactWriter.class), "org.apache.sling", null, null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndAttachRequiresNonNullVersion() throws Exception {
- artifactDeployer.deploy(mock(ArtifactWriter.class), "org.apache.sling", "org.apache.sling.cm2fm", null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndAttachRequiresNonNullType() throws Exception {
- artifactDeployer.deploy(mock(ArtifactWriter.class), "org.apache.sling", "org.apache.sling.cm2fm", "0.0.1", null, null);
+ artifactDeployer.deploy(mock(ArtifactWriter.class), null);
}
}
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/features/FeaturesManagerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/features/FeaturesManagerTest.java
index 02c3882..7cb6c20 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/features/FeaturesManagerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/features/FeaturesManagerTest.java
@@ -40,28 +40,8 @@ public class FeaturesManagerTest {
}
@Test(expected = NullPointerException.class)
- public void deployLocallyAndaddArtifactRequiresNonNullInput() throws Exception {
- featuresManager.addArtifact(null, null, null, null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndaddArtifactRequiresNonNullGroupId() throws Exception {
- featuresManager.addArtifact(null, null, null, null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndaddArtifactRequiresNonNullArtifactId() throws Exception {
- featuresManager.addArtifact(null, "org.apache.sling", null, null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndaddArtifactRequiresNonNullVersion() throws Exception {
- featuresManager.addArtifact(null, "org.apache.sling", "org.apache.sling.cm2fm", null, null, null);
- }
-
- @Test(expected = NullPointerException.class)
- public void deployLocallyAndaddArtifactRequiresNonNullType() throws Exception {
- featuresManager.addArtifact(null, "org.apache.sling", "org.apache.sling.cm2fm", "0.0.1", null, null);
+ public void deployLocallyAndaddArtifactRequiresNonNullId() throws Exception {
+ featuresManager.addArtifact(null, null);
}
}
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandlerTest.java
index e336785..9b04551 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandlerTest.java
@@ -19,6 +19,7 @@ package org.apache.sling.feature.cpconverter.handlers;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.mock;
@@ -91,7 +92,7 @@ public final class BundleEntryHandlerTest {
FeaturesManager featuresManager = spy(DefaultFeaturesManager.class);
when(featuresManager.getTargetFeature()).thenReturn(feature);
when(featuresManager.getRunMode(anyString())).thenReturn(feature);
- doCallRealMethod().when(featuresManager).addArtifact(anyString(), anyString(), anyString(), anyString(), anyString(), anyString());
+ doCallRealMethod().when(featuresManager).addArtifact(anyString(), any(ArtifactId.class));
ContentPackage2FeatureModelConverter converter = spy(ContentPackage2FeatureModelConverter.class);