This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to branch issue/SLING-10288 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-analyser.git
commit 0fe99889ebfc38c83dde5c1be9456891219b1774 Author: Robert Munteanu <[email protected]> AuthorDate: Wed Jun 12 15:39:01 2024 +0200 SLING-10288 - FelixFrameworkScanner sets framework properties always for current Java version Rough implementation --- pom.xml | 11 +++ readme.md | 25 +++++- .../extensions/AnalyserMetaDataExtension.java | 47 ++++++++++- .../extensions/AnalyserMetaDataHandler.java | 62 ++++++++++++--- .../task/impl/CheckRequirementsCapabilities.java | 4 +- .../org/apache/sling/feature/scanner/Scanner.java | 74 +++++++++++++++++- .../extensions/AnalyserMetaDataHandlerTest.java | 91 +++++++++++++++++++++- .../feature-input-system-bundle.json | 12 +++ 8 files changed, 305 insertions(+), 21 deletions(-) diff --git a/pom.xml b/pom.xml index 146539e..b4199c2 100644 --- a/pom.xml +++ b/pom.xml @@ -212,6 +212,17 @@ <version>1.2.14</version> <scope>test</scope> </dependency> + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-simple</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.assertj</groupId> + <artifactId>assertj-core</artifactId> + <version>3.26.0</version> + <scope>test</scope> + </dependency> <dependency> <!-- Used by some tests --> <groupId>org.apache.felix</groupId> diff --git a/readme.md b/readme.md index 7530b1f..93cbd10 100644 --- a/readme.md +++ b/readme.md @@ -116,16 +116,25 @@ Checks bundle requirements/capabilities for consistency and completeness. Generates additional metadata that will be recorded in the feature model definition. It is configured by defining an `analyser-metadata` section in the feature model definition. The section will be processed by the extension when the feature models are aggregated and will be replaced with the required entries for bundles matching the configuration. +### Bundle metadata + The section can have entries that match individual bundle names and entries that match based on regular expressions (if the key contains the "*" character). Each individual entry can contain the following keys: - Configuration key | Allowed values | Description ----- | ----- | ----- `manifest` | `null` or Object | If null, the manifest is not generated. If an object, the values are copied over. If absent, the values are extracted from the OSGi bundle `report` | Object with keys `warning` and `error` | If any of the values are set to `false`, reporting is suppressed for those kind of occurences. +### Framework metadata + +A special case is when an entry with the name `system.bundle` is found. This will record information about the system bundle exported packages and capabilites as present at the time of the feature aggregation. When these are present in an aggregated feature the analysers will use that information instead of the one discovered during analysis time. + +The system bundle information will be extracted from the `execution-environment` extension. In this extension, the `framework` entry is required and is used to gather information about the system bundle. The `javaVersion` property is optional but recommended and is used to validate that the Java version used to generate the metadata matches the one in the execution environment. + +### Example + A typical configuration for platform applications is: ```javascript @@ -138,9 +147,19 @@ A typical configuration for platform applications is: "error": false, "warning": false } - } + }, + "system.bundle": {} + }, + "framework":{ + "id":"org.apache.felix:org.apache.felix.framework:7.0.5" + }, + "javaVersion": "21" } } ``` -This ensures that warnings related to the platform are not reported when the feature is aggregated with downstream (consumer) applications. The manifests should not be inlined under normal circumstances, since it greatly increases the size of the resulting features. +This ensures that + +- warnings related to the platform are not reported when the feature is aggregated with downstream (consumer) applications. The manifests should not be inlined under normal circumstances, since it greatly increases the size of the resulting features. +- metadata related to the system bundle is recorded at the Java 21 compatibility level + diff --git a/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataExtension.java b/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataExtension.java index 40b7172..bc3bdb2 100644 --- a/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataExtension.java +++ b/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataExtension.java @@ -39,6 +39,7 @@ public class AnalyserMetaDataExtension { private final Map<ArtifactId, Map<String, String>> manifests = new HashMap<>(); private final Map<ArtifactId, Boolean> reportWarnings = new HashMap<>(); private final Map<ArtifactId, Boolean> reportErrors = new HashMap<>(); + private SystemBundle systemBundle; public static AnalyserMetaDataExtension getAnalyserMetaDataExtension(Feature feature) { Extension ext = feature == null ? null : feature.getExtensions().getByName(EXTENSION_NAME); @@ -56,10 +57,25 @@ public class AnalyserMetaDataExtension { } private AnalyserMetaDataExtension(JsonObject json) { + for (Map.Entry<String, JsonValue> entry : json.entrySet()) { + + // handle system bundle separately if ( entry.getKey().equals(Constants.SYSTEM_BUNDLE_SYMBOLICNAME) ) { - continue; // not used for now + JsonObject systemBundleConfig = entry.getValue().asJsonObject(); + JsonObject manifestObj = systemBundleConfig.getJsonObject("manifest"); + String artifactId = systemBundleConfig.getJsonString("artifactId").getString(); + String sortedFrameworkProperties = systemBundleConfig.getJsonString("sortedFrameworkProperties").getString(); + + Map<String, String> manifest = new HashMap<>(); + for (String key : manifestObj.keySet()) { + manifest.put(key, manifestObj.getString(key)); + } + systemBundle = new SystemBundle(manifest, ArtifactId.fromMvnId(artifactId), sortedFrameworkProperties); + + continue; } + ArtifactId id = ArtifactId.fromMvnId(entry.getKey()); JsonObject headers = entry.getValue().asJsonObject(); if (headers.containsKey("manifest")) { @@ -89,6 +105,10 @@ public class AnalyserMetaDataExtension { public Map<String, String> getManifest(final ArtifactId artifactId) { return this.manifests.get(artifactId); } + + public SystemBundle getSystemBundle() { + return systemBundle; + } public boolean reportWarning(ArtifactId artifactId) { return !this.reportWarnings.containsKey(artifactId) || this.reportWarnings.get(artifactId); @@ -144,4 +164,29 @@ public class AnalyserMetaDataExtension { public void setReportErrors(ArtifactId id, boolean enabled) { reportErrors.put(id, enabled); } + + public static class SystemBundle { + + private Map<String, String> manifest = new HashMap<>(); + private ArtifactId artifactId; + private String sortedFrameworkProperties; + + public SystemBundle(Map<String, String> manifest, ArtifactId artifactId, String sortedFrameworkProperties) { + this.manifest = manifest; + this.artifactId = artifactId; + this.sortedFrameworkProperties = sortedFrameworkProperties; + } + + public ArtifactId getArtifactId() { + return artifactId; + } + + public Map<String, String> getManifest() { + return manifest; + } + + public String getSortedFrameworkProperties() { + return sortedFrameworkProperties; + } + } } diff --git a/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandler.java b/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandler.java index 06c02d9..9e99598 100644 --- a/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandler.java +++ b/src/main/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandler.java @@ -24,6 +24,9 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; import java.util.ServiceLoader; +import java.util.Set; +import java.util.StringJoiner; +import java.util.TreeMap; import java.util.jar.JarFile; import org.apache.commons.lang3.SystemUtils; @@ -35,11 +38,14 @@ import org.apache.sling.feature.ExtensionType; import org.apache.sling.feature.Feature; import org.apache.sling.feature.builder.HandlerContext; import org.apache.sling.feature.builder.PostProcessHandler; +import org.apache.sling.feature.impl.felix.utils.resource.ResourceUtils; import org.apache.sling.feature.io.IOUtils; import org.apache.sling.feature.scanner.BundleDescriptor; +import org.apache.sling.feature.scanner.PackageInfo; import org.apache.sling.feature.scanner.spi.FrameworkScanner; import org.osgi.framework.Constants; import org.osgi.framework.Version; +import org.osgi.resource.Capability; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,13 +68,13 @@ public class AnalyserMetaDataHandler implements PostProcessHandler { JsonObjectBuilder result = Json.createObjectBuilder(); Map<String, JsonValue> directEntries = new HashMap<>(); Map<String, JsonValue> wildcardEntries = new LinkedHashMap<>(); - JsonObject[] frameworkDefinitionHolder = new JsonObject[1]; + JsonObject[] systemBundleHolder = new JsonObject[1]; extensionJSONStructure.entrySet().forEach( entry -> { if (entry.getKey().contains("*")) { wildcardEntries.put(entry.getKey(), entry.getValue()); } else if (entry.getKey().equals(Constants.SYSTEM_BUNDLE_SYMBOLICNAME)) { - frameworkDefinitionHolder[0] = entry.getValue().asJsonObject(); + systemBundleHolder[0] = entry.getValue().asJsonObject(); } else { directEntries.put(entry.getKey(), entry.getValue()); } @@ -95,12 +101,9 @@ public class AnalyserMetaDataHandler implements PostProcessHandler { ) ); - if (frameworkDefinitionHolder[0] != null) { - JsonObject v = frameworkDefinitionHolder[0]; - // TODO - add test - // TODO - finalise contract - // TODO - make use of the information in the Analyser, if present - FrameworkScanner scanner = ServiceLoader.load(FrameworkScanner.class).iterator().next(); + if (systemBundleHolder[0] != null) { + JsonObject systemBundle = systemBundleHolder[0]; + // TODO - skip if the entries already exist? try { ExecutionEnvironmentExtension executionEnv = ExecutionEnvironmentExtension.getExecutionEnvironmentExtension(feature); if ( executionEnv != null ) { @@ -115,10 +118,16 @@ public class AnalyserMetaDataHandler implements PostProcessHandler { throw new IllegalStateException("Execution environment requires Java " + requiredJavaVersion.getMajor() + ", but running on " + currentJavaVersion.getMajor() + ". Aborting."); } + FrameworkScanner scanner = ServiceLoader.load(FrameworkScanner.class).iterator().next(); + BundleDescriptor fw = scanner.scan(frameworkId, feature.getFrameworkProperties(), handlerContext.getArtifactProvider()); - JsonObjectBuilder wrapper = Json.createObjectBuilder(v); - wrapper.add(Constants.PROVIDE_CAPABILITY, fw.getCapabilities().toString()); - wrapper.add(Constants.EXPORT_PACKAGE, fw.getExportedPackages().toString()); + JsonObjectBuilder wrapper = Json.createObjectBuilder(systemBundle); + JsonObjectBuilder manifest = Json.createObjectBuilder(); + manifest.add(Constants.PROVIDE_CAPABILITY, capabilitiesToString(fw.getCapabilities())); + manifest.add(Constants.EXPORT_PACKAGE, exportedPackagesToString(fw.getExportedPackages())); + wrapper.add(MANIFEST_KEY, manifest); + wrapper.add("artifactId", frameworkId.toMvnId()); + wrapper.add("sortedFrameworkProperties", frameworkPropertiesSortedToString(feature.getFrameworkProperties())); result.add(Constants.SYSTEM_BUNDLE_SYMBOLICNAME, wrapper); } else { LOG.warn("No execution environment found, not creating framework capabilities"); @@ -138,6 +147,37 @@ public class AnalyserMetaDataHandler implements PostProcessHandler { } } + private String frameworkPropertiesSortedToString(Map<String, String> frameworkProperties) { + if (frameworkProperties == null || frameworkProperties.isEmpty()) { + return ""; + } + StringBuilder sb = new StringBuilder(); + // TODO - duplication with Scanner.scan + Map<String, String> sortedMap = new TreeMap<>(frameworkProperties); + for (final Map.Entry<String, String> entry : sortedMap.entrySet()) { + sb.append(":").append(entry.getKey()).append("=").append(entry.getValue()); + } + return sb.toString(); + } + + private static String exportedPackagesToString(Set<PackageInfo> exportedPackages) { + StringJoiner joiner = new StringJoiner(","); + for (PackageInfo packageInfo : exportedPackages) { + joiner.add(packageInfo.getName() + ";version=" + packageInfo.getVersion()); + } + return joiner.toString(); + } + + private static String capabilitiesToString(Set<Capability> capabilities) { + + StringJoiner joiner = new StringJoiner(","); + for (Capability c : capabilities) { + joiner.add(ResourceUtils.toString(null, c.getNamespace(), c.getAttributes(), c.getDirectives())); + } + + return joiner.toString(); + } + private boolean noManifest(JsonObject object) { return manifest(object, null) && !object.getBoolean("no-manifest", false); } diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckRequirementsCapabilities.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckRequirementsCapabilities.java index 3492ae3..0979f69 100644 --- a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckRequirementsCapabilities.java +++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckRequirementsCapabilities.java @@ -155,7 +155,9 @@ public class CheckRequirementsCapabilities implements AnalyserTask { private List<Descriptor> getCandidates(List<Descriptor> artifactDescriptors, Requirement requirement) { return artifactDescriptors.stream() .filter(artifactDescriptor -> artifactDescriptor.getCapabilities() != null) - .filter(artifactDescriptor -> artifactDescriptor.getCapabilities().stream().anyMatch(capability -> CapabilitySet.matches(capability, requirement))) + .filter(artifactDescriptor -> artifactDescriptor.getCapabilities().stream().anyMatch( + capability -> CapabilitySet.matches(capability, requirement)) + ) .collect(Collectors.toList()); } diff --git a/src/main/java/org/apache/sling/feature/scanner/Scanner.java b/src/main/java/org/apache/sling/feature/scanner/Scanner.java index a55f8bb..954af5c 100644 --- a/src/main/java/org/apache/sling/feature/scanner/Scanner.java +++ b/src/main/java/org/apache/sling/feature/scanner/Scanner.java @@ -28,17 +28,24 @@ import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.felix.utils.manifest.Clause; +import org.apache.felix.utils.manifest.Parser; import org.apache.sling.feature.Artifact; import org.apache.sling.feature.ArtifactId; import org.apache.sling.feature.Bundles; import org.apache.sling.feature.Extension; import org.apache.sling.feature.Feature; import org.apache.sling.feature.analyser.extensions.AnalyserMetaDataExtension; +import org.apache.sling.feature.analyser.extensions.AnalyserMetaDataExtension.SystemBundle; import org.apache.sling.feature.builder.ArtifactProvider; +import org.apache.sling.feature.impl.felix.utils.resource.ResourceBuilder; import org.apache.sling.feature.scanner.impl.BundleDescriptorImpl; import org.apache.sling.feature.scanner.impl.FeatureDescriptorImpl; import org.apache.sling.feature.scanner.spi.ExtensionScanner; import org.apache.sling.feature.scanner.spi.FrameworkScanner; +import org.osgi.framework.BundleException; +import org.osgi.framework.Constants; +import org.osgi.resource.Capability; /** * The scanner is a service that scans items and provides descriptions for @@ -248,6 +255,71 @@ public class Scanner { } } } + + SystemBundle systemBundle = extension.getSystemBundle(); + if ( systemBundle != null ) { + String key = systemBundle.getArtifactId().toMvnId(); + if ( systemBundle.getSortedFrameworkProperties() != null ) + key += systemBundle.getSortedFrameworkProperties(); + + URL artifactUrl = artifactProvider.provide(systemBundle.getArtifactId()); + if (artifactUrl == null) { + throw new IOException("Unable to find file for " + systemBundle.getArtifactId()); + } + + // TODO - duplicated with FelixFrameworkScanner + BundleDescriptor desc = new BundleDescriptor(systemBundle.getArtifactId().toMvnId()) { + + @Override + public URL getArtifactFile() { + return artifactUrl; + } + + @Override + public Artifact getArtifact() { + return new Artifact(systemBundle.getArtifactId()); + } + + @Override + public Manifest getManifest() { + return new Manifest(); + } + + @Override + public String getBundleVersion() { + return systemBundle.getArtifactId().getOSGiVersion().toString(); + } + + @Override + public String getBundleSymbolicName() { + return Constants.SYSTEM_BUNDLE_SYMBOLICNAME; + } + }; + + String capabilities = systemBundle.getManifest().get(Constants.PROVIDE_CAPABILITY); + if ( capabilities != null ) { + try { + List<Capability> parsedCapabilities = ResourceBuilder.parseCapability(null, capabilities); + desc.getCapabilities().addAll(parsedCapabilities); + } catch (BundleException e) { + throw new IOException("Failed to parse capabilites for the system bundle", e); + } + } + + String exports = systemBundle.getManifest().get(Constants.EXPORT_PACKAGE); + if ( exports != null ) { + Clause[] pcks = Parser.parseHeader(exports); + for (final Clause pck : pcks) { + String version = pck.getAttribute("version"); + PackageInfo info = new PackageInfo(pck.getName(), version, false); + desc.getExportedPackages().add(info); + } + } + desc.lock(); + + this.cache.put(key, desc); + + } } } @@ -263,7 +335,7 @@ public class Scanner { final StringBuilder sb = new StringBuilder(); sb.append(framework.toMvnId()); if (props != null) { - final Map<String, String> sortedMap = new TreeMap<String, String>(props); + final Map<String, String> sortedMap = new TreeMap<>(props); for (final Map.Entry<String, String> entry : sortedMap.entrySet()) { sb.append(":").append(entry.getKey()).append("=").append(entry.getValue()); } diff --git a/src/test/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandlerTest.java b/src/test/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandlerTest.java index 9e4a668..59bb45f 100644 --- a/src/test/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/analyser/extensions/AnalyserMetaDataHandlerTest.java @@ -16,16 +16,22 @@ */ package org.apache.sling.feature.analyser.extensions; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.assertj.core.api.Assertions.assertThat; +import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; import java.io.StringWriter; import java.io.UncheckedIOException; import java.net.MalformedURLException; import java.net.URL; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import org.apache.felix.utils.manifest.Clause; +import org.apache.felix.utils.manifest.Parser; +import org.apache.sling.feature.ArtifactId; import org.apache.sling.feature.Feature; import org.apache.sling.feature.builder.ArtifactProvider; import org.apache.sling.feature.builder.HandlerContext; @@ -34,6 +40,10 @@ import org.apache.sling.feature.io.json.FeatureJSONWriter; import org.junit.Test; import org.mockito.Mockito; +import jakarta.json.JsonObject; +import jakarta.json.JsonString; +import jakarta.json.JsonValue; + public class AnalyserMetaDataHandlerTest { @Test @@ -45,7 +55,7 @@ public class AnalyserMetaDataHandlerTest { feature = FeatureJSONReader.read(r, url.toString()); } - assertNotNull(feature); + assertThat(feature).isNotNull(); HandlerContext ctx = Mockito.mock(HandlerContext.class); ArtifactProvider provider = artifactId -> { @@ -74,7 +84,80 @@ public class AnalyserMetaDataHandlerTest { FeatureJSONWriter.write(featureWriter, feature); FeatureJSONWriter.write(expectedWriter, expected); - assertEquals(expectedWriter.toString(), featureWriter.toString()); + assertThat(expectedWriter.toString()).isEqualTo(featureWriter.toString()); + } + + @Test + public void testMetadataHandlerSystemBundle() throws IOException { + URL url = getClass().getResource("/analyse-metadata/feature-input-system-bundle.json"); + + Feature feature; + try (Reader r = new InputStreamReader(url.openStream())) { + feature = FeatureJSONReader.read(r, url.toString()); + } + + assertThat(feature).isNotNull(); + + HandlerContext ctx = Mockito.mock(HandlerContext.class); + ArtifactProvider provider = artifactId -> { + try { + return new URL("https://repo1.maven.org/maven2/"; + artifactId.toMvnPath()); + } catch (MalformedURLException e) { + throw new UncheckedIOException(e); + } + }; + + Mockito.when(ctx.getArtifactProvider()).thenReturn(provider); + + new AnalyserMetaDataHandler().postProcess(ctx, feature, feature.getExtensions().getByName("analyser-metadata")); + + JsonObject metadata = feature.getExtensions().getByName("analyser-metadata").getJSONStructure().asJsonObject(); + assertThat(metadata).as("analyser-metadata extension").isNotNull(); + + JsonValue systemBundle = metadata.get("system.bundle"); + assertThat(systemBundle).as("system.bundle property of the metadata extension").isNotNull(); + + // ensure the artifactId is recorded and correctly formed + JsonValue artifactId = systemBundle.asJsonObject().get("artifactId"); + assertThat(artifactId).as("artifactId of the system bundle").isNotNull(); + ArtifactId.fromMvnId(artifactId.toString()); + + JsonValue manifest = systemBundle.asJsonObject().get("manifest"); + assertThat(manifest).as("manifest of the system bundle").isNotNull(); + + JsonString systemBundleCapabilities = manifest.asJsonObject().getJsonString("Provide-Capability"); + assertThat(systemBundleCapabilities).as("capabilities for the system bundle").isNotNull(); + String capabilities = systemBundleCapabilities.getString(); + + // validate that the capabilities header is correctly formed + Clause[] clauses = Parser.parseHeader(capabilities); + + // validate that we have exactly one osgi.ee clause with the value "JavaSE" + List<Clause> javaSeEeClauses = Arrays.stream(clauses) + .filter( c -> c.getName().equals("osgi.ee") ) + .filter( c -> c.getAttribute("osgi.ee").equals("JavaSE") ) + .collect(Collectors.toList()); + + assertThat(javaSeEeClauses).as("osgi.ee=JavaSE capabilities") + .hasSize(1) + .element(0).extracting( c -> c.getAttribute("version:List<Version>") ) + .asString().contains("1.8"); + + JsonString systemBundleExports = manifest.asJsonObject().getJsonString("Export-Package"); + assertThat(systemBundleExports).as("exports for the system bundle").isNotNull(); + String exports = systemBundleExports.getString(); + + // validate that the exports header is correctly formed + Clause[] exportClauses = Parser.parseHeader(exports); + List<Clause> javaUtilExports = Arrays.stream(exportClauses) + .filter( c -> c.getName().equals("java.util") ) + .collect(Collectors.toList()); + + // validate that the java.util.function export is present + assertThat(javaUtilExports).as("java.util package exports") + .hasSize(1) + .element(0).extracting( c -> c.getAttribute("version") ) + .isNotNull(); } } diff --git a/src/test/resources/analyse-metadata/feature-input-system-bundle.json b/src/test/resources/analyse-metadata/feature-input-system-bundle.json new file mode 100644 index 0000000..02053eb --- /dev/null +++ b/src/test/resources/analyse-metadata/feature-input-system-bundle.json @@ -0,0 +1,12 @@ +{ + "id": "org.acme:acmefeature:slingosgifeature:metadata-feature:0.0.1", + "execution-environment:JSON|false":{ + "framework":{ + "id":"org.apache.felix:org.apache.felix.framework:7.0.5" + } + }, + "analyser-metadata:JSON|true": { + "system.bundle" : { + } + } +} \ No newline at end of file
