SLING-12181 Evaluate provider type from annotations
Only in case META-INF/api-info.json is not available in the class path
(no apis-jar) and if enabled via attribute
diff --git a/pom.xml b/pom.xml
index 0ae2e95..0a717a6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -44,7 +44,33 @@
<url>https://github.com/apache/sling-org-apache-sling-providertype-bnd-plugin/tree/${project.scm.tag}</url>
<tag>HEAD</tag>
</scm>
-
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-dependency-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>copy-dependencies</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>copy-dependencies</goal>
+ </goals>
+ <configuration>
+ <outputDirectory>${project.build.directory}/test-classpath</outputDirectory>
+ <overWriteReleases>false</overWriteReleases>
+ <overWriteSnapshots>false</overWriteSnapshots>
+ <overWriteIfNewer>true</overWriteIfNewer>
+ <includeArtifactIds>oak-jackrabbit-api</includeArtifactIds>
+ <stripVersion>true</stripVersion>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+
<dependencyManagement>
<dependencies>
<dependency>
@@ -87,5 +113,11 @@
<artifactId>slf4j-simple</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-jackrabbit-api</artifactId>
+ <version>1.58.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
\ No newline at end of file
diff --git a/readme.md b/readme.md
index ef277d8..b1af77e 100644
--- a/readme.md
+++ b/readme.md
@@ -14,7 +14,7 @@
That ensures that the `import-package` version ranges are not narrow but [broad][semantic-versioning] and the risk that the bundle is incompatible with newer versions of its dependent bundles is less likely.
-# Usage
+## Usage
For usage with Maven the Bnd plugin has to be added to the plugin dependencies of `bnd-maven-plugin` (or `maven-bundle-plugin`) like this:
@@ -39,7 +39,7 @@
-plugin.providertype:org.apache.sling.providertype.bndplugin.ProviderTypeScanner
```
-## Configuration
+### Configuration
To explicitly ignore certain provider types (i.e. don't fail when these are extended/implemented) one can use the attribute `ignored` with one or multiple comma-separated fully qualified provider type names. For example
@@ -47,18 +47,28 @@
-plugin.providertype:org.apache.sling.providertype.bndplugin.ProviderTypeScanner;ignored=org.apache.jackrabbit.api.security.user.User
```
-## Prerequisites
+In case the provider types are not provided in a classpath resource named [`META-INF/api-info.json`][api-info.json] the plugin can optionally evaluate the classpath for the relevant annotations directly. Since this is time consuming it needs to be explicitly enabled via attribute `evaluateAnnotations` with value `true`.
+
+```
+-plugin.providertype:org.apache.sling.providertype.bndplugin.ProviderTypeScanner;evaluateAnnotations=true
+```
+
+Multiple attributes must be separated with `;` according to [OSGi Common Header Syntax][common-header].
+
+### Prerequisites
* Bnd 6.0 or newer (integrated in `bnd-maven-plugin` version 6.0.0+ or `maven-bundle-plugin` version 5.1.5+)
* Java 11 or newer
-# Provider Type Information
+## Provider Type Information
The information whether a type (i.e. a class or interface) is designed to be extended/implemented only by providers or also by consumers is determined originally from the the annotations [`@org.osgi.annotation.versioning.ProviderType`][provider-type] or [`@org.osgi.annotation.versioning.ConsumerType`][consumer-type].
-In order to speed up the check [the annotation is evaluated and extracted into a dedicated JSON file named `META-INF/api-info.json` when generating the apis jar](https://issues.apache.org/jira/browse/SLING-12135) and being looked up from there within this plugin.
+In order to speed up the check [the annotation is evaluated and extracted into a dedicated JSON file named `META-INF/api-info.json` when generating the apis jar][api-info.json] and being looked up from there within this plugin. Only as fallback and on demand this plugin evaluates the annotations from the classpath directly.
[bnd-plugins]: https://bnd.bndtools.org/chapters/870-plugins.html
[provider-type]: https://docs.osgi.org/javadoc/osgi.annotation/8.0.0/org/osgi/annotation/versioning/ProviderType.html
[consumer-type]: https://docs.osgi.org/javadoc/osgi.annotation/8.0.0/org/osgi/annotation/versioning/ConsumerType.html
[semantic-versioning]: https://docs.osgi.org/whitepaper/semantic-versioning/060-importer-policy.html
+[api-info.json]: https://issues.apache.org/jira/browse/SLING-12135
+[common-header]: https://docs.osgi.org/specification/osgi.core/8.0.0/framework.module.html#framework.common.header.syntax
diff --git a/src/main/java/org/apache/sling/providertype/bndplugin/ProviderTypeScanner.java b/src/main/java/org/apache/sling/providertype/bndplugin/ProviderTypeScanner.java
index d4c3b0a..84b814a 100644
--- a/src/main/java/org/apache/sling/providertype/bndplugin/ProviderTypeScanner.java
+++ b/src/main/java/org/apache/sling/providertype/bndplugin/ProviderTypeScanner.java
@@ -29,9 +29,11 @@
import java.util.Set;
import java.util.stream.Collectors;
+import aQute.bnd.build.Classpath.ClassVisitor;
import aQute.bnd.osgi.Analyzer;
import aQute.bnd.osgi.Clazz;
import aQute.bnd.osgi.Descriptors.TypeRef;
+import aQute.bnd.osgi.Jar;
import aQute.bnd.osgi.Resource;
import aQute.bnd.service.AnalyzerPlugin;
import aQute.bnd.service.Plugin;
@@ -39,19 +41,19 @@
import aQute.lib.json.JSONCodec;
import aQute.service.reporter.Reporter;
-/**
- * Enforces that no classes implement or extend a type marked as provider.
- * Provider types are retrieved from the resource "META-INF/api-info.json" which is expected to be provided
- * in the class path.
- */
+/** Enforces that no classes implement or extend a type marked as provider. Provider types are retrieved from the resource
+ * "META-INF/api-info.json" which is expected to be provided in the class path. */
public class ProviderTypeScanner implements AnalyzerPlugin, Plugin {
private static final String API_INFO_JSON_RESOURCE_PATH = "META-INF/api-info.json";
private static final String FIELD_PROVIDER_TYPES = "providerTypes";
private static final String MESSAGE = "Type \"%s\" %s provider type \"%s\". This is not allowed!";
private static final String ATTRIBUTE_IGNORED_PROVIDER_TYPES = "ignored";
+ private static final String ATTRIBUTE_EVALUATE_ANNOTATIONS_IN_CLASSPATH = "evaluateAnnotations";
+
+ private static final List<String> PROVIDER_TYPE_ANNOTATION_FQNS = List.of("org.osgi.annotation.versioning.ProviderType", "aQute.bnd.annotation.ProviderType");
- private Map<String,String> parameters = new HashMap<>();
+ private Map<String, String> parameters = new HashMap<>();
@Override
public void setProperties(Map<String, String> map) throws Exception {
@@ -67,28 +69,42 @@
@Override
public boolean analyzeJar(Analyzer analyzer) throws Exception {
- List<Resource> apiInfoJsonResources = analyzer.findResources(s -> s.equals(API_INFO_JSON_RESOURCE_PATH)).collect(Collectors.toList());
- if(apiInfoJsonResources.isEmpty()) {
- analyzer.warning("Could not find resource \"%s\" in the classpath", API_INFO_JSON_RESOURCE_PATH);
+ List<Resource> apiInfoJsonResources = analyzer.findResources(s -> s.equals(API_INFO_JSON_RESOURCE_PATH))
+ .collect(Collectors.toList());
+ final Set<String> providerTypes = new HashSet<>(); // This may contain fully qualified class names or package names
+ if (apiInfoJsonResources.isEmpty()) {
+ if (Boolean.parseBoolean(parameters.get(ATTRIBUTE_EVALUATE_ANNOTATIONS_IN_CLASSPATH))) {
+ analyzer.warning("Retrieving provider type info from annotations found in classpath...");
+ providerTypes.addAll(collectProviderTypesFromClasspath(analyzer));
+ } else {
+ analyzer.warning("Could not find resource \"%s\" in the classpath", API_INFO_JSON_RESOURCE_PATH);
+ }
} else {
- Set<String> providerTypes = new HashSet<>();
for (Resource apiInfoJsonResource : apiInfoJsonResources) {
try {
- Set<String> resourceProviderTypes = collectProviderTypes(analyzer, apiInfoJsonResource);
- analyzer.trace("Added provider types from resource \"%s\": %s", apiInfoJsonResource, String.join(",", resourceProviderTypes));
+ Set<String> resourceProviderTypes = collectProviderTypesFromApiInfo(analyzer, apiInfoJsonResource);
+ analyzer.trace("Added provider types from resource \"%s\": %s", apiInfoJsonResource,
+ String.join(",", resourceProviderTypes));
providerTypes.addAll(resourceProviderTypes);
} catch (Exception e) {
throw new IllegalStateException("Could not parse JSON from resource " + apiInfoJsonResource, e);
}
}
+ }
+ if (providerTypes.isEmpty()) {
+ analyzer.warning("No provider types found, skip checking bundle's classes");
+ } else {
// remove ignored provider types
- Arrays.stream(parameters.getOrDefault(ATTRIBUTE_IGNORED_PROVIDER_TYPES, "").split(",")).filter(s -> !s.isBlank()).forEach(ignored -> {
- if (providerTypes.remove(ignored)) {
- analyzer.trace("Ignore extensions of provider type \"%s\" due to plugin configuration", ignored);
- } else {
- analyzer.warning("Ignored class \"%s\" is not defined as provider type at all, you can safely remove the according plugin parameter", ignored);
- }
- });
+ Arrays.stream(parameters.getOrDefault(ATTRIBUTE_IGNORED_PROVIDER_TYPES, "").split(",")).filter(s -> !s.isBlank())
+ .forEach(ignored -> {
+ if (providerTypes.remove(ignored)) {
+ analyzer.trace("Ignore extensions of provider type \"%s\" due to plugin configuration", ignored);
+ } else {
+ analyzer.warning(
+ "Ignored class \"%s\" is not defined as provider type at all, you can safely remove the according plugin parameter",
+ ignored);
+ }
+ });
checkIfExtendingType(analyzer, analyzer.getClassspace().values(), providerTypes);
}
return false;
@@ -96,35 +112,78 @@
private void checkIfExtendingType(Reporter reporter, Collection<Clazz> clazzes, Set<String> providerTypes) {
for (Clazz clazz : clazzes) {
- if (clazz.getSuper() != null && (providerTypes.contains(clazz.getSuper().getFQN()))) {
+ if (clazz.getSuper() != null && (isProvider(clazz.getSuper(), providerTypes))) {
reporter.error(MESSAGE, clazz.getFQN(), "extends", clazz.getSuper().getFQN()).file(clazz.getSourceFile());
}
for (TypeRef interfaceType : clazz.interfaces()) {
- if (providerTypes.contains(interfaceType.getFQN())) {
+ if (isProvider(interfaceType, providerTypes)) {
reporter.error(MESSAGE, clazz.getFQN(), "implements", interfaceType.getFQN()).file(clazz.getSourceFile());
}
}
}
}
+ static boolean isProvider(TypeRef type, Set<String> providerTypes) {
+ return providerTypes.stream().anyMatch(providerType -> providerType.equals(type.getFQN()) || providerType.equals(type.getPackageRef().getFQN()));
+ }
+
@SuppressWarnings("unchecked")
- private Set<String> collectProviderTypes(Reporter reporter, Resource apiInfoResource) throws Exception {
+ private Set<String> collectProviderTypesFromApiInfo(Reporter reporter, Resource apiInfoResource) throws Exception {
JSONCodec codec = new JSONCodec();
// read JSON file
try (InputStream input = apiInfoResource.openInputStream();
- Decoder decoder = codec.dec().from(input)) {
+ Decoder decoder = codec.dec().from(input)) {
Map<?, ?> jsonMap = decoder.get(Map.class);
Object providerTypes = jsonMap.get(FIELD_PROVIDER_TYPES);
if (providerTypes == null) {
reporter.error("Resource \"%s\" does not contain a field named \"%s\"", API_INFO_JSON_RESOURCE_PATH, FIELD_PROVIDER_TYPES);
} else if (!(providerTypes instanceof Collection)) {
- reporter.error("Field \"%s\" in JSON resource \"%s\" is not containing a string array but a type converted to %s", FIELD_PROVIDER_TYPES, API_INFO_JSON_RESOURCE_PATH, providerTypes.getClass().getName());
+ reporter.error("Field \"%s\" in JSON resource \"%s\" is not containing a string array but a type converted to %s",
+ FIELD_PROVIDER_TYPES, API_INFO_JSON_RESOURCE_PATH, providerTypes.getClass().getName());
} else {
- return new HashSet<>((Collection<String>)providerTypes);
+ return new HashSet<>((Collection<String>) providerTypes);
}
}
return Collections.emptySet();
}
+ private Set<String> collectProviderTypesFromClasspath(Analyzer analyzer) throws Exception {
+ // unfortunately bnd does not cache this information in an accessible way
+ ProviderTypeCollectorClassVisitor visitor = new ProviderTypeCollectorClassVisitor();
+ visit(visitor, analyzer);
+ return visitor.getProviderTypes();
+ }
+ /** Visit each class on the class path (includes package-info.class).
+ *
+ * @param visitor the visitor */
+ private void visit(ClassVisitor visitor, Analyzer analyzer) throws Exception {
+ for (Jar classpathJar : analyzer.getClasspath()) {
+ for (String path : classpathJar.getResources().keySet()) {
+ if (path.endsWith(".class")) {
+ Resource r = classpathJar.getResource(path);
+ Clazz c = new Clazz(analyzer, path, r);
+ c.parseClassFile();
+ visitor.visit(c);
+ }
+ }
+ }
+ }
+
+ private static final class ProviderTypeCollectorClassVisitor implements ClassVisitor {
+
+ final Set<String> providerTypes = new HashSet<>();
+
+ @Override
+ public boolean visit(Clazz clazz) throws Exception {
+ if (clazz.annotations().stream().anyMatch(at -> PROVIDER_TYPE_ANNOTATION_FQNS.contains(at.getFQN()))) {
+ providerTypes.add(clazz.getFQN());
+ }
+ return true;
+ }
+
+ public Set<String> getProviderTypes() {
+ return providerTypes;
+ }
+ }
}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/providertype/bndplugin/BndBuilderExtension.java b/src/test/java/org/apache/sling/providertype/bndplugin/BndBuilderExtension.java
index d14add8..3e662f2 100644
--- a/src/test/java/org/apache/sling/providertype/bndplugin/BndBuilderExtension.java
+++ b/src/test/java/org/apache/sling/providertype/bndplugin/BndBuilderExtension.java
@@ -20,9 +20,8 @@
import java.io.File;
import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.HashMap;
+import java.util.Collections;
+import java.util.Map;
import java.util.jar.Manifest;
import org.junit.jupiter.api.extension.AfterEachCallback;
@@ -35,11 +34,11 @@
public class BndBuilderExtension implements BeforeEachCallback, AfterEachCallback {
- protected Builder builder;
- private final Collection<Object> plugins;
+ private Builder builder;
+ private final Object plugin;
- public BndBuilderExtension(Object... plugins) {
- this.plugins = Arrays.asList(plugins);
+ public BndBuilderExtension(Object plugin) {
+ this.plugin = plugin;
}
@Override
@@ -49,13 +48,24 @@
classesDirJar.setManifest(new Manifest());
builder.setJar(classesDirJar); // jar closed with builder
builder.setSourcepath(new File[] { new File("src/test/java") } );
- for (Object plugin : plugins) {
- if (plugin instanceof Plugin) {
- Plugin pluginPlugin = (Plugin)plugin;
- pluginPlugin.setReporter(builder);
- pluginPlugin.setProperties(new HashMap<>()); // not used
- }
- builder.addBasicPlugin(plugin);
+ if (plugin instanceof Plugin) {
+ Plugin pluginPlugin = (Plugin)plugin;
+ pluginPlugin.setReporter(builder);
+ pluginPlugin.setProperties(Collections.emptyMap());
+ }
+ builder.addBasicPlugin(plugin);
+ }
+
+ protected Builder getBuilder() {
+ return builder;
+ }
+
+ protected void setPluginProperties(Map<String, String> pluginProperties) throws Exception{
+ if (plugin instanceof Plugin) {
+ Plugin pluginPlugin = (Plugin)plugin;
+ pluginPlugin.setProperties(pluginProperties);
+ } else {
+ throw new IllegalStateException("Given plugin does not implement class Plugin but is " + plugin);
}
}
diff --git a/src/test/java/org/apache/sling/providertype/bndplugin/MyBinaryDownload.java b/src/test/java/org/apache/sling/providertype/bndplugin/MyBinaryDownload.java
new file mode 100644
index 0000000..13ebaca
--- /dev/null
+++ b/src/test/java/org/apache/sling/providertype/bndplugin/MyBinaryDownload.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.providertype.bndplugin;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+
+import javax.jcr.RepositoryException;
+
+import org.apache.jackrabbit.api.binary.BinaryDownload;
+import org.apache.jackrabbit.api.binary.BinaryDownloadOptions;
+
+public class MyBinaryDownload implements BinaryDownload {
+
+ @Override
+ public InputStream getStream() throws RepositoryException {
+ // TODO Auto-generated method stub
+ return null;
+ }
+
+ @Override
+ public int read(byte[] b, long position) throws IOException, RepositoryException {
+ // TODO Auto-generated method stub
+ return 0;
+ }
+
+ @Override
+ public long getSize() throws RepositoryException {
+ // TODO Auto-generated method stub
+ return 0;
+ }
+
+ @Override
+ public void dispose() {
+ // TODO Auto-generated method stub
+
+ }
+
+ @Override
+ public URI getURI(BinaryDownloadOptions downloadOptions) throws RepositoryException {
+ // TODO Auto-generated method stub
+ return null;
+ }
+
+}
diff --git a/src/test/java/org/apache/sling/providertype/bndplugin/ProviderTypeScannerTest.java b/src/test/java/org/apache/sling/providertype/bndplugin/ProviderTypeScannerTest.java
index bf26f0f..fa41758 100644
--- a/src/test/java/org/apache/sling/providertype/bndplugin/ProviderTypeScannerTest.java
+++ b/src/test/java/org/apache/sling/providertype/bndplugin/ProviderTypeScannerTest.java
@@ -25,6 +25,7 @@
import java.io.File;
import java.util.Arrays;
import java.util.List;
+import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -38,8 +39,8 @@
static BndBuilderExtension bndBuilderExtension = new BndBuilderExtension(new ProviderTypeScanner());
@Test
- void testBuildWithViolations() throws Exception {
- Builder builder = bndBuilderExtension.builder;
+ void testBuildWithViolationsAndApiInfoJson() throws Exception {
+ Builder builder = bndBuilderExtension.getBuilder();
// add classpath entry with api-info.json
builder.setClasspath(new File[] { new File("src/test/resources") });
try (Jar jar = builder.build()) {
@@ -55,35 +56,38 @@
@Test
void testBuildWithoutProviderTypeMetadata() throws Exception {
- Builder builder = bndBuilderExtension.builder;
+ Builder builder = bndBuilderExtension.getBuilder();
+ // add classpath entry without api-info.json
+ builder.setClasspath(new File[] { new File("target/test-classpath/oak-jackrabbit-api.jar") });
+ bndBuilderExtension.setPluginProperties(Map.of("evaluateAnnotations", "true"));
try (Jar jar = builder.build()) {
- if (!builder.getErrors().isEmpty()) {
- fail(String.join("\n", builder.getErrors()));
- }
+ List<String> expectedErrors = Arrays.asList(
+ "Type \"org.apache.sling.providertype.bndplugin.MyBinaryDownload\" implements provider type \"org.apache.jackrabbit.api.binary.BinaryDownload\". This is not allowed!");
+ assertEquals(expectedErrors, builder.getErrors());
List<String> expectedWarnings = Arrays.asList(
- "Could not find resource \"META-INF/api-info.json\" in the classpath");
+ "Retrieving provider type info from annotations found in classpath...");
assertEquals(expectedWarnings, builder.getWarnings());
}
}
@Test
void testBuildWithInvalidProviderTypeMetadata() throws Exception {
- Builder builder = bndBuilderExtension.builder;
+ Builder builder = bndBuilderExtension.getBuilder();
// add classpath entry with api-info.json
builder.setClasspath(new File[] { new File("src/test/resources2") });
try (Jar jar = builder.build()) {
List<String> expectedErrors = Arrays.asList(
"Resource \"META-INF/api-info.json\" does not contain a field named \"providerTypes\"");
assertEquals(expectedErrors, builder.getErrors());
- if (!builder.getWarnings().isEmpty()) {
- fail(String.join("\n", builder.getWarnings()));
- }
+ List<String> expectedWarnings = Arrays.asList(
+ "No provider types found, skip checking bundle's classes");
+ assertEquals(expectedWarnings, builder.getWarnings());
}
}
@Test
void testBuildWithInvalidProviderTypeMetadata2() throws Exception {
- Builder builder = bndBuilderExtension.builder;
+ Builder builder = bndBuilderExtension.getBuilder();
// add classpath entry with api-info.json
builder.setClasspath(new File[] { new File("src/test/resources3") });
try (Jar jar = builder.build()) {
