karlpauls commented on a change in pull request #74:
URL:
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/74#discussion_r622828946
##########
File path:
src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java
##########
@@ -109,111 +154,324 @@ public void handle(@NotNull String path, @NotNull
Archive archive, @NotNull Entr
logger.debug("Start level {} was extracted from path {}",
startLevel, path);
}
- try (JarInputStream jarInput = new
JarInputStream(Objects.requireNonNull(archive.openInputStream(entry)))) {
- Properties properties = readGav(entry.getName(), jarInput);
- manifest = jarInput.getManifest();
-
- if (!properties.isEmpty()) {
- groupId = getCheckedProperty(properties, NAME_GROUP_ID);
- artifactId = getCheckedProperty(properties, NAME_ARTIFACT_ID);
- version = getCheckedProperty(properties, NAME_VERSION);
- classifier = properties.getProperty(NAME_CLASSIFIER);
- } else { // maybe the included jar is just an OSGi bundle but not
a valid Maven artifact
- groupId = getCheckedProperty(manifest, BUNDLE_SYMBOLIC_NAME);
- // Make sure there are not spaces in the name to adhere to the
Maven Group Id specification
- groupId = groupId.replace(' ', '_').replace(':',
'_').replace('/', '_').replace('\\', '_');
- if (groupId.indexOf('.') != -1) {
- artifactId = groupId.substring(groupId.lastIndexOf('.') +
1);
- groupId = groupId.substring(0, groupId.lastIndexOf('.'));
+ String bundleName = entry.getName();
+ // Remove the leading path
+ int idx = bundleName.lastIndexOf('/');
+ if (idx >= 0) {
+ bundleName = bundleName.substring(idx + 1);
+ }
+ // Remove the extension
+ int edx = bundleName.lastIndexOf('.');
+ if (edx > 0) {
+ bundleName = bundleName.substring(0, edx);
+ }
+
+ // create a temporary JAR file (extracted from archive)
+ Path tmpBundleJar =
Files.createTempFile(converter.getTempDirectory().toPath(), "extracted",
bundleName + ".jar");
+ try (OutputStream output = Files.newOutputStream(tmpBundleJar);
+ InputStream input =
Objects.requireNonNull(archive.openInputStream(entry))) {
+ IOUtils.copy(input, output);
+ }
+ try {
+ processBundleInputStream(tmpBundleJar, bundleName, runMode,
startLevel, converter);
+ } finally {
+ Files.delete(tmpBundleJar);
+ }
+ }
+
+ void processBundleInputStream(@NotNull Path originalBundleFile, @NotNull
String bundleName, @Nullable String runMode, @Nullable Integer startLevel,
@NotNull ContentPackage2FeatureModelConverter converter) throws Exception {
+ try (JarFile jarFile = new JarFile(originalBundleFile.toFile())) {
+ // first extract bundle metadata from JAR input stream
+ ArtifactId id = extractArtifactId(bundleName, jarFile);
+
+ try (InputStream strippedBundleInput =
extractSlingInitialContent(id, jarFile, converter, runMode)) {
+ if (strippedBundleInput != null && slingInitialContentPolicy
== SlingInitialContentPolicy.EXTRACT_AND_REMOVE) {
+ id = id.changeVersion(id.getVersion() + "-" +
ContentPackage2FeatureModelConverter.PACKAGE_CLASSIFIER);
+
Objects.requireNonNull(converter.getArtifactsDeployer()).deploy(new
InputStreamArtifactWriter(strippedBundleInput), id);
+ } else {
+ try (InputStream originalBundleInput =
Files.newInputStream(originalBundleFile)) {
+
Objects.requireNonNull(converter.getArtifactsDeployer()).deploy(new
InputStreamArtifactWriter(originalBundleInput), id);
+ }
}
- if (artifactId == null || artifactId.isEmpty()) {
- artifactId = groupId;
+ }
+
Objects.requireNonNull(converter.getFeaturesManager()).addArtifact(runMode, id,
startLevel);
+ String exportHeader =
Objects.requireNonNull(jarFile.getManifest()).getMainAttributes().getValue(Constants.EXPORT_PACKAGE);
+ if (exportHeader != null) {
+ for (Clause clause : Parser.parseHeader(exportHeader)) {
+ converter.getFeaturesManager().addAPIRegionExport(runMode,
clause.getName());
}
- Version osgiVersion =
Version.parseVersion(getCheckedProperty(manifest, BUNDLE_VERSION));
- version = osgiVersion.getMajor() + "." +
osgiVersion.getMinor() + "." + osgiVersion.getMicro() +
(osgiVersion.getQualifier().isEmpty() ? "" : "-" + osgiVersion.getQualifier());
}
}
+ }
- try (InputStream input = archive.openInputStream(entry)) {
- if (input != null) {
- ArtifactId id = new ArtifactId(groupId, artifactId, version,
classifier, JAR_TYPE);
+ static Version getModifiedOsgiVersion(Version originalVersion) {
+ return new Version(originalVersion.getMajor(),
originalVersion.getMinor(), originalVersion.getMicro(),
originalVersion.getQualifier() + "_" +
ContentPackage2FeatureModelConverter.PACKAGE_CLASSIFIER);
+ }
-
Objects.requireNonNull(converter.getArtifactsDeployer()).deploy(new
InputStreamArtifactWriter(input), id);
+ @Nullable InputStream extractSlingInitialContent(@NotNull ArtifactId
bundleArtifactId, @NotNull JarFile jarFile, @NotNull
ContentPackage2FeatureModelConverter converter, @Nullable String runMode)
throws Exception {
+ if (slingInitialContentPolicy == SlingInitialContentPolicy.KEEP) {
+ return null;
+ }
+ // parse "Sling-Initial-Content" header
+ Manifest manifest = Objects.requireNonNull(jarFile.getManifest());
+ Iterator<PathEntry> pathEntries = PathEntry.getContentPaths(manifest,
-1);
+ if (pathEntries == null) {
+ return null;
+ }
+ logger.info("Extracting Sling-Initial-Content from '{}'",
bundleArtifactId);
+ Collection<PathEntry> pathEntryList = new ArrayList<>();
+ pathEntries.forEachRemaining(pathEntryList::add);
+
+ // remove header
+ manifest.getMainAttributes().remove(new
Attributes.Name(PathEntry.CONTENT_HEADER));
+ // change version to have suffix
+ Version originalVersion = new
Version(Objects.requireNonNull(manifest.getMainAttributes().getValue(Constants.BUNDLE_VERSION)));
+ manifest.getMainAttributes().putValue(Constants.BUNDLE_VERSION,
getModifiedOsgiVersion(originalVersion).toString());
+ Path newBundleFile =
Files.createTempFile(converter.getTempDirectory().toPath(), "newBundle",
".jar");
+
+ // create JAR file to prevent extracting it twice and for random access
+ JcrNamespaceRegistry namespaceRegistry =
createNamespaceRegistry(manifest, jarFile,
converter.getFeaturesManager().getNamespaceUriByPrefix());
+
+ Map<PackageType, VaultPackageAssembler> packageAssemblers = new
EnumMap<>(PackageType.class);
+ try (OutputStream fileOutput = Files.newOutputStream(newBundleFile,
StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING);
+ JarOutputStream bundleOutput = new JarOutputStream(fileOutput,
manifest)) {
+
+ for (Enumeration<JarEntry> e = jarFile.entries();
e.hasMoreElements();) {
+ JarEntry jarEntry = e.nextElement();
+ if (!jarEntry.isDirectory()) {
+ try (InputStream input = jarFile.getInputStream(jarEntry))
{
+ if (!extractSlingInitialContent(jarEntry, input,
bundleArtifactId, pathEntryList, packageAssemblers, namespaceRegistry,
converter)) {
+ // skip manifest, as already written in the
constructor (as first entry)
+ if
(jarEntry.getName().equals(JarFile.MANIFEST_NAME)) {
+ continue;
+ }
+ // copy entry as is to the stripped bundle
+ ZipEntry ze = new ZipEntry(jarEntry.getName());
Review comment:
I'm not sure about this one - a JarEntry can have attributes (like e.g.
last modified etc) which this code will remove. I think it would be better to
use the original entry instead of a copy (and keep it a real JarEntry).
##########
File path:
src/test/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandlerGAVTest.java
##########
@@ -16,73 +16,168 @@
*/
package org.apache.sling.feature.cpconverter.handlers;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Dictionary;
+import java.util.Map;
+
+import org.apache.commons.io.FileUtils;
import org.apache.jackrabbit.vault.fs.io.Archive;
+import org.apache.jackrabbit.vault.fs.io.Archive.Entry;
+import org.apache.jackrabbit.vault.packaging.PackageId;
+import org.apache.jackrabbit.vault.packaging.VaultPackage;
+import org.apache.jackrabbit.vault.packaging.impl.PackageManagerImpl;
import org.apache.sling.feature.ArtifactId;
import
org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter;
+import
org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter.SlingInitialContentPolicy;
import org.apache.sling.feature.cpconverter.artifacts.ArtifactsDeployer;
import org.apache.sling.feature.cpconverter.features.FeaturesManager;
+import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Captor;
+import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
-import static org.junit.Assert.assertThrows;
-import static org.mockito.Mockito.when;
-
+@RunWith(MockitoJUnitRunner.class)
public class BundleEntryHandlerGAVTest {
- private final BundleEntryHandler handler = new BundleEntryHandler();
- @Test
- public void testGAVwithProperties() throws Exception {
- String path = "/jcr_root/apps/gav/install/core-1.0.0-SNAPSHOT.jar";
- Archive.Entry entry = Mockito.mock(Archive.Entry.class);
- when(entry.getName()).thenReturn(path);
- Archive archive = Mockito.mock(Archive.class);
-
when(archive.openInputStream(entry)).thenReturn(BundleEntryHandlerGAVTest.class.getResourceAsStream("core-1.0.0-SNAPSHOT.jar"));
- ContentPackage2FeatureModelConverter converter =
Mockito.mock(ContentPackage2FeatureModelConverter.class);
+ @Mock
+ private Archive.Entry entry;
+ @Mock
+ private Archive archive;
+ @Spy
+ private ContentPackage2FeatureModelConverter converter;
+ @Spy
+ private FeaturesManager featuresManager;
+ @Rule
+ public TemporaryFolder tmpFolder = new TemporaryFolder();
+ @Captor
+ ArgumentCaptor<Dictionary<String, Object>> dictionaryCaptor;
+
+ private BundleEntryHandler handler;
+
+ @Before
+ public void setUp() throws IOException {
+ handler = new BundleEntryHandler();
ArtifactsDeployer deployer = Mockito.spy(ArtifactsDeployer.class);
when(converter.getArtifactsDeployer()).thenReturn(deployer);
- FeaturesManager manager = Mockito.spy(FeaturesManager.class);
- when(converter.getFeaturesManager()).thenReturn(manager);
- handler.handle(path, archive, entry, converter);
- Mockito.verify(manager).addArtifact(null,
ArtifactId.fromMvnId("com.madplanet.sling.cp2sf:core:1.0.0-SNAPSHOT"),null);
+ when(converter.getTempDirectory()).thenReturn(tmpFolder.getRoot());
+ featuresManager = Mockito.spy(FeaturesManager.class);
+ when(converter.getFeaturesManager()).thenReturn(featuresManager);
+ }
+
+ private void setUpArchive(String entryPath, String resourcePath) throws
IOException {
+ when(entry.getName()).thenReturn(entryPath);
+
when(archive.openInputStream(entry)).thenReturn(BundleEntryHandlerGAVTest.class.getResourceAsStream(resourcePath));
+ }
+
+ @Test
+ public void testGAV() throws Exception {
+ setUpArchive("/jcr_root/apps/gav/install/core-1.0.0-SNAPSHOT.jar",
"core-1.0.0-SNAPSHOT.jar");
+ handler.handle("/jcr_root/apps/gav/install/core-1.0.0-SNAPSHOT.jar",
archive, entry, converter);
+ Mockito.verify(featuresManager).addArtifact(null,
ArtifactId.fromMvnId("com.madplanet.sling.cp2sf:core:1.0.0-SNAPSHOT"), null);
}
@Test
public void testGAVwithPom() throws Exception{
- String path =
"/jcr_root/apps/gav/install/org.osgi.service.jdbc-1.0.0.jar";
- Archive.Entry entry = Mockito.mock(Archive.Entry.class);
- when(entry.getName()).thenReturn(path);
- Archive archive = Mockito.mock(Archive.class);
-
when(archive.openInputStream(entry)).thenReturn(BundleEntryHandlerGAVTest.class.getResourceAsStream("org.osgi.service.jdbc-1.0.0.jar"));
- ContentPackage2FeatureModelConverter converter =
Mockito.mock(ContentPackage2FeatureModelConverter.class);
- ArtifactsDeployer deployer = Mockito.spy(ArtifactsDeployer.class);
- when(converter.getArtifactsDeployer()).thenReturn(deployer);
- FeaturesManager manager = Mockito.spy(FeaturesManager.class);
- when(converter.getFeaturesManager()).thenReturn(manager);
- handler.handle(path, archive, entry, converter);
- Mockito.verify(manager).addArtifact(null,
ArtifactId.fromMvnId("org.osgi:org.osgi.service.jdbc:1.0.0"),null);
+
setUpArchive("/jcr_root/apps/gav/install/org.osgi.service.jdbc-1.0.0.jar",
"org.osgi.service.jdbc-1.0.0.jar");
+
handler.handle("/jcr_root/apps/gav/install/org.osgi.service.jdbc-1.0.0.jar",
archive, entry, converter);
+ Mockito.verify(featuresManager).addArtifact(null,
ArtifactId.fromMvnId("org.osgi:org.osgi.service.jdbc:1.0.0"), null);
}
@Test
public void testNoGAV() throws Exception {
- String path =
"/jcr_root/apps/gav/install/org.osgi.service.jdbc-1.0.0-nogav.jar";
- Archive.Entry entry = Mockito.mock(Archive.Entry.class);
- when(entry.getName()).thenReturn(path);
- Archive archive = Mockito.mock(Archive.class);
-
when(archive.openInputStream(entry)).thenReturn(BundleEntryHandlerGAVTest.class.getResourceAsStream("org.osgi.service.jdbc-1.0.0-nogav.jar"));
- ContentPackage2FeatureModelConverter converter =
Mockito.mock(ContentPackage2FeatureModelConverter.class);
- ArtifactsDeployer deployer = Mockito.spy(ArtifactsDeployer.class);
- when(converter.getArtifactsDeployer()).thenReturn(deployer);
- FeaturesManager manager = Mockito.spy(FeaturesManager.class);
- when(converter.getFeaturesManager()).thenReturn(manager);
- handler.handle(path, archive, entry, converter);
- Mockito.verify(manager).addArtifact(null,
ArtifactId.fromMvnId("org.osgi.service:jdbc:1.0.0-201505202023"),null);
+
setUpArchive("/jcr_root/apps/gav/install/org.osgi.service.jdbc-1.0.0-nogav.jar",
"org.osgi.service.jdbc-1.0.0-nogav.jar");
+
handler.handle("/jcr_root/apps/gav/install/org.osgi.service.jdbc-1.0.0-nogav.jar",
archive, entry, converter);
+ Mockito.verify(featuresManager).addArtifact(null,
ArtifactId.fromMvnId("org.osgi.service:jdbc:1.0.0-201505202023"), null);
}
@Test
public void testBundleBelowConfigFolderWithEnforcement() throws Exception {
handler.setEnforceBundlesBelowInstallFolder(true);
- Archive.Entry entry = Mockito.mock(Archive.Entry.class);
when(entry.getName()).thenReturn("mybundle.jar");
assertThrows(IllegalStateException.class, () -> {
handler.handle("/jcr_root/apps/myapp/config/mybundle.jar", null, entry, null);
});
}
+
+ @Test
+ public void testSlingInitialContent() throws Exception {
+
setUpArchive("/jcr_root/apps/gav/install/io.wcm.handler.media-1.11.6.jar",
"io.wcm.handler.media-1.11.6.jar");
+ DefaultEntryHandlersManager handlersManager = new
DefaultEntryHandlersManager();
+ converter.setEntryHandlersManager(handlersManager);
+ Map<String, String> namespaceRegistry =
Collections.singletonMap("granite", "http://www.adobe.com/jcr/granite/1.0";);
+
when(featuresManager.getNamespaceUriByPrefix()).thenReturn(namespaceRegistry);
+
+ File newPackageFile = tmpFolder.newFile();
+ Mockito.doAnswer(invocation -> {
+ // capture package
+ File sourcePackage = (File)invocation.getArgument(0);
+ FileUtils.copyFile(sourcePackage, newPackageFile);
+ return null;
+ }).when(converter).processContentPackageArchive(Mockito.any(),
Mockito.isNull());
+
handler.setSlingInitialContentPolicy(SlingInitialContentPolicy.EXTRACT_AND_REMOVE);
+
handler.handle("/jcr_root/apps/gav/install/io.wcm.handler.media-1.11.6.jar",
archive, entry, converter);
+ // verify package contents
+ try (VaultPackage vaultPackage = new
PackageManagerImpl().open(newPackageFile);
+ Archive archive = vaultPackage.getArchive()) {
+ archive.open(true);
+ PackageId targetId =
PackageId.fromString("io.wcm:io.wcm.handler.media-apps:1.11.6-cp2fm-converted");
+ assertEquals(targetId, vaultPackage.getId());
+ Entry entry =
archive.getEntry("jcr_root/apps/wcm-io/handler/media/components/global/include/responsiveImageSettings.xml");
+ assertNotNull("Archive does not contain expected item", entry);
+ }
+ // changed id
+ Mockito.verify(featuresManager).addArtifact(null,
ArtifactId.fromMvnId("io.wcm:io.wcm.handler.media:1.11.6-cp2fm-converted"),
null);
+ }
+
+ @Test
+ public void
testSlingInitialContentContainingConfigurationExtractAndRemove() throws
Exception {
+
setUpArchive("/jcr_root/apps/gav/install/composum-nodes-config-2.5.3.jar",
"composum-nodes-config-2.5.3.jar");
+ DefaultEntryHandlersManager handlersManager = new
DefaultEntryHandlersManager();
+ converter.setEntryHandlersManager(handlersManager);
+
handler.setSlingInitialContentPolicy(SlingInitialContentPolicy.EXTRACT_AND_REMOVE);
+
handler.handle("/jcr_root/apps/gav/install/composum-nodes-config-2.5.3.jar",
archive, entry, converter);
+ // verify no additional content package created (as it only contains
the configuration which should end up in the feature model only)
+ Mockito.verify(converter,
Mockito.never()).processContentPackageArchive(Mockito.any(), Mockito.isNull());
+ // modified bundle
+ Mockito.verify(featuresManager).addArtifact(null,
ArtifactId.fromMvnId("com.composum.nodes:composum-nodes-config:2.5.3-cp2fm-converted"),
null);
+ // need to use ArgumentCaptur to properly compare string arrays
+
Mockito.verify(featuresManager).addConfiguration(ArgumentMatchers.isNull(),
ArgumentMatchers.eq("org.apache.sling.jcr.base.internal.LoginAdminWhitelist.fragment~composum_core_v2"),
ArgumentMatchers.eq("/jcr_root/libs/composum/nodes/install/org.apache.sling.jcr.base.internal.LoginAdminWhitelist.fragment-composum_core_v2.config"),
dictionaryCaptor.capture());
+ assertEquals("composum_core",
dictionaryCaptor.getValue().get("whitelist.name"));
+ assertArrayEquals(new String[] {
+ "com.composum.nodes.commons",
+ "com.composum.nodes.pckgmgr",
+ "com.composum.nodes.pckginstall" },
(String[])dictionaryCaptor.getValue().get("whitelist.bundles"));
+ }
+
+ @Test
+ public void testSlingInitialContentContainingConfigurationExtractAndKeep()
throws Exception {
+
setUpArchive("/jcr_root/apps/gav/install/composum-nodes-config-2.5.3.jar",
"composum-nodes-config-2.5.3.jar");
+ DefaultEntryHandlersManager handlersManager = new
DefaultEntryHandlersManager();
+ converter.setEntryHandlersManager(handlersManager);
+
handler.setSlingInitialContentPolicy(SlingInitialContentPolicy.EXTRACT_AND_KEEP);
+
handler.handle("/jcr_root/apps/gav/install/composum-nodes-config-2.5.3.jar",
archive, entry, converter);
+ // original bundle
+ Mockito.verify(featuresManager).addArtifact(null,
ArtifactId.fromMvnId("com.composum.nodes:composum-nodes-config:2.5.3"), null);
+ // need to use ArgumentCaptur to properly compare string arrays
+
Mockito.verify(featuresManager).addConfiguration(ArgumentMatchers.isNull(),
ArgumentMatchers.eq("org.apache.sling.jcr.base.internal.LoginAdminWhitelist.fragment~composum_core_v2"),
ArgumentMatchers.eq("/jcr_root/libs/composum/nodes/install/org.apache.sling.jcr.base.internal.LoginAdminWhitelist.fragment-composum_core_v2.config"),
dictionaryCaptor.capture());
+ assertEquals("composum_core",
dictionaryCaptor.getValue().get("whitelist.name"));
+ assertArrayEquals(new String[] {
+ "com.composum.nodes.commons",
+ "com.composum.nodes.pckgmgr",
+ "com.composum.nodes.pckginstall" },
(String[])dictionaryCaptor.getValue().get("whitelist.bundles"));
+ }
Review comment:
Should these tests be in the GAV test class? It seems they would make
more sense in either a new dedicated test class or in the main
BundleEntryHandlerTest class - I guess this is more nitpicking but it wouldn't
be the first place I would look for them...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
