Skip to content

Commit

Permalink
split processing of services & default to 'auto'
Browse files Browse the repository at this point in the history 
we decided to make '-metainf-services: annotation the default. so '-metainf-services: auto' has to explicity be enabled in bnd file.
This decision was due to the fact that it is a brand new feature, and the impact on projects in the wild are hard to predict, since it changes metadata in MANIFEST.MF. So it is unclear at the moment if this would be considered a breaking change. So to be safe, we do not do 'auto' by default but 'annotation'. that means that textual annotations in META-INF/services files are automatically processed, because they were put there on purpose.

this commit also adjusts and adds some testcases based on that behavior.

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
  • Loading branch information
@chrisrueger
chrisrueger committed Oct 9, 2024
1 parent eb69453 commit 1e130b2
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 23 deletions.
144 changes: 133 additions & 11 deletions biz.aQute.bndlib.tests/test/test/annotationheaders/SPIAnnotationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,47 @@ public void testServiceProvider_existingdescriptor() throws Exception {
"META-INF/services/test.annotationheaders.spi.SPIService;literal='test.annotationheaders.spi.providerE.Provider'");
b.setProperty("Provide-Capability",
"osgi.serviceloader;osgi.serviceloader='test.annotationheaders.spi.SPIService';register:='test.annotationheaders.spi.providerE.Provider'");
b.setProperty(Constants.METAINF_SERVICES, Constants.METAINF_SERVICES_STRATEGY_ANNOTATION);
b.build();
b.getJar()
.getManifest()
.write(System.out);
assertTrue(b.check());

Attributes mainAttributes = b.getJar()
.getManifest()
.getMainAttributes();

Header req = Header.parseHeader(mainAttributes.getValue(Constants.REQUIRE_CAPABILITY));
assertEquals(1, req.size());

assertEE(req);

Header cap = Header.parseHeader(mainAttributes.getValue(Constants.PROVIDE_CAPABILITY));
assertEquals(1, cap.size());

Props p = cap.get("osgi.serviceloader");
assertNotNull(p);
assertNotNull(p.get("osgi.serviceloader"));
assertEquals("test.annotationheaders.spi.SPIService", p.get("osgi.serviceloader"));
assertNotNull(p.get("register:"));
assertEquals("test.annotationheaders.spi.providerE.Provider", p.get("register:"));

assertServiceMappingFile(b.getJar(), "test.annotationheaders.spi.SPIService",
"test.annotationheaders.spi.providerE.Provider");
}
}

@Test
public void testServiceProvider_existingdescriptorAuto() throws Exception {
try (Builder b = new Builder();) {
b.addClasspath(IO.getFile("bin_test"));
b.setPrivatePackage("test.annotationheaders.spi.providerE");
b.setProperty("-includeresource",
"META-INF/services/test.annotationheaders.spi.SPIService;literal='test.annotationheaders.spi.providerE.Provider'");
b.setProperty("Provide-Capability",
"osgi.serviceloader;osgi.serviceloader='test.annotationheaders.spi.SPIService';register:='test.annotationheaders.spi.providerE.Provider'");
b.setProperty(Constants.METAINF_SERVICES, Constants.METAINF_SERVICES_STRATEGY_AUTO);
b.build();
b.getJar()
.getManifest()
Expand Down Expand Up @@ -144,18 +185,15 @@ public void testServiceProvider_nowarning_onexisting() throws Exception {
b.addClasspath(IO.getFile("bin_test"));
b.setPrivatePackage("test.annotationheaders.spi.providerE");
b.setProperty("-includeresource",
"META-INF/services/test.annotationheaders.spi.SPIService;literal='some.other.Provider'");
"META-INF/services/test.annotationheaders.spi.SPIService;literal='java.lang.String'");
b.setProperty("Provide-Capability",
"osgi.serviceloader;osgi.serviceloader=\"test.annotationheaders.spi.SPIService\"");
b.setProperty(Constants.METAINF_SERVICES, Constants.METAINF_SERVICES_STRATEGY_ANNOTATION);
b.build();
b.getJar()
.getManifest()
.write(System.out);
assertFalse(b.check());
assertThat(b.getErrors()).hasSize(1);
assertThat(b.getErrors()
.get(0)).contains(
"analyzer processing annotation some.other.Provider but the associated class is not found in the JAR");
assertTrue(b.check());

Attributes mainAttributes = b.getJar()
.getManifest()
Expand All @@ -175,7 +213,47 @@ public void testServiceProvider_nowarning_onexisting() throws Exception {
assertEquals("test.annotationheaders.spi.SPIService", p.get("osgi.serviceloader"));
assertNull(p.get("register:"));

assertServiceMappingFile(b.getJar(), "test.annotationheaders.spi.SPIService", "some.other.Provider");
assertServiceMappingFile(b.getJar(), "test.annotationheaders.spi.SPIService", "java.lang.String");
assertServiceMappingFileNotContains(b.getJar(), "test.annotationheaders.spi.SPIService",
"another.provider.ProviderImpl");
}
}

@Test
public void testServiceProvider_nowarning_onexistingAuto() throws Exception {
try (Builder b = new Builder();) {
b.addClasspath(IO.getFile("bin_test"));
b.setPrivatePackage("test.annotationheaders.spi.providerE");
b.setProperty("-includeresource",
"META-INF/services/test.annotationheaders.spi.SPIService;literal='java.lang.String'");
b.setProperty("Provide-Capability",
"osgi.serviceloader;osgi.serviceloader=\"test.annotationheaders.spi.SPIService\"");
b.setProperty(Constants.METAINF_SERVICES, Constants.METAINF_SERVICES_STRATEGY_AUTO);
b.build();
b.getJar()
.getManifest()
.write(System.out);
assertTrue(b.check());

Attributes mainAttributes = b.getJar()
.getManifest()
.getMainAttributes();

Header req = Header.parseHeader(mainAttributes.getValue(Constants.REQUIRE_CAPABILITY));
assertEquals(2, req.size());

assertEE(req);

Header cap = Header.parseHeader(mainAttributes.getValue(Constants.PROVIDE_CAPABILITY));
assertEquals(3, cap.size());

Props p = cap.get("osgi.serviceloader");
assertNotNull(p);
assertNotNull(p.get("osgi.serviceloader"));
assertEquals("test.annotationheaders.spi.SPIService", p.get("osgi.serviceloader"));
assertNull(p.get("register:"));

assertServiceMappingFile(b.getJar(), "test.annotationheaders.spi.SPIService", "java.lang.String");
assertServiceMappingFileNotContains(b.getJar(), "test.annotationheaders.spi.SPIService",
"another.provider.ProviderImpl");
}
Expand All @@ -188,12 +266,60 @@ public void testServiceProvider_mergeDescriptor() throws Exception {
b.setPrivatePackage("test.annotationheaders.spi.providerD");
b.setProperty("-includeresource",
"META-INF/services/test.annotationheaders.spi.SPIService=testresources/services");
b.setProperty(Constants.METAINF_SERVICES, Constants.METAINF_SERVICES_STRATEGY_ANNOTATION);
b.build();
b.getJar()
.getManifest()
.write(System.out);
assertTrue(b.check());

Attributes mainAttributes = b.getJar()
.getManifest()
.getMainAttributes();

Header req = Header.parseHeader(mainAttributes.getValue(Constants.REQUIRE_CAPABILITY));
assertEquals(2, req.size());

assertExtender(req, "osgi.serviceloader.registrar");
assertEE(req);

Header cap = Header.parseHeader(mainAttributes.getValue(Constants.PROVIDE_CAPABILITY));
assertEquals(2, cap.size());

Props p = cap.get("osgi.serviceloader");
assertNotNull(p);
assertNotNull(p.get("osgi.serviceloader"));
assertEquals("test.annotationheaders.spi.SPIService", p.get("osgi.serviceloader"));
assertNotNull(p.get("register:"));
assertThat(p.get("register:")).isIn("test.annotationheaders.spi.providerD.Provider");
assertNotNull(p.get("foo"));
assertEquals("bar", p.get("foo"));

assertServiceMappingFile(b.getJar(), "test.annotationheaders.spi.SPIService",
"test.annotationheaders.spi.providerD.Provider");
assertServiceMappingFile(b.getJar(), "test.annotationheaders.spi.SPIService",
"another.provider.ProviderImpl");
}
}

@Test
public void testServiceProvider_mergeDescriptorAuto() throws Exception {
try (Builder b = new Builder();) {
b.addClasspath(IO.getFile("bin_test"));
b.setPrivatePackage("test.annotationheaders.spi.providerD");
b.setProperty("-includeresource",
"META-INF/services/test.annotationheaders.spi.SPIService=testresources/services");
b.setProperty(Constants.METAINF_SERVICES, Constants.METAINF_SERVICES_STRATEGY_AUTO);
b.build();
b.getJar()
.getManifest()
.write(System.out);
assertFalse(b.check());

Attributes mainAttributes = b.getJar()
.getManifest()
.getMainAttributes();

assertThat(b.getErrors()).hasSize(2);
assertThat(b.getErrors()
.get(0)).contains(
Expand All @@ -202,10 +328,6 @@ public void testServiceProvider_mergeDescriptor() throws Exception {
.get(1)).contains(
"analyzer processing annotation another.provider.ProviderImpl but the associated class is not found in the JAR");

Attributes mainAttributes = b.getJar()
.getManifest()
.getMainAttributes();

Header req = Header.parseHeader(mainAttributes.getValue(Constants.REQUIRE_CAPABILITY));
assertEquals(2, req.size());

Expand Down
30 changes: 29 additions & 1 deletion biz.aQute.bndlib.tests/test/test/annotationheaders/ServiceProviderFileTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
package test.annotationheaders;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.List;

import org.junit.jupiter.api.Test;
import org.osgi.resource.Requirement;

import aQute.bnd.header.Parameters;
import aQute.bnd.osgi.Builder;
import aQute.bnd.osgi.Domain;
import aQute.bnd.osgi.resource.CapReqBuilder;
import aQute.lib.io.IO;

public class ServiceProviderFileTest {
Expand Down Expand Up @@ -96,6 +103,16 @@ public void testBothMetaInfoAndAnnotationsNoParentheses() throws Exception {
Parameters requireCapability = manifest.getRequireCapability();
assertThat(provideCapability.size()).isEqualTo(4);
assertThat(requireCapability.size()).isEqualTo(2);

Requirement req = CapReqBuilder.getRequirementsFrom(requireCapability)
.get(0);
assertEquals("osgi.extender", req.getNamespace());
assertNull(req.getDirectives()
.get("resolution"));
assertEquals("(&(osgi.extender=osgi.serviceloader.registrar)(version>=1.0.0)(!(version>=2.0.0)))",
req.getDirectives()
.get("filter"));

}
}

Expand Down Expand Up @@ -127,6 +144,14 @@ public void testAutoGenerateServiceProviderAnnotation() throws Exception {
.containsEntry("register:", "java.lang.String");

Parameters requireCapability = manifest.getRequireCapability();
Requirement req = CapReqBuilder.getRequirementsFrom(requireCapability)
.get(0);
assertEquals("osgi.extender", req.getNamespace());
assertEquals("optional", req.getDirectives()
.get("resolution"));
assertEquals("(&(osgi.extender=osgi.serviceloader.registrar)(version>=1.0.0)(!(version>=2.0.0)))",
req.getDirectives()
.get("filter"));

System.out.println(provideCapability.toString()
.replace(',', '\n'));
Expand All @@ -147,7 +172,10 @@ public void testInvalidServiceImplementationNamesShouldBeIgnored() throws Except
""");
b.setProperty("-metainf-services", "auto");
b.build();
assertTrue(b.check());
assertFalse(b.check());
List<String> errors = b.getErrors();
assertEquals("analyzer processing annotation key=value but the associated class is not found in the JAR",
errors.get(0));
Domain manifest = Domain.domain(b.getJar()
.getManifest());
Parameters provideCapability = manifest.getProvideCapability();
Expand Down
61 changes: 50 additions & 11 deletions biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import static aQute.bnd.osgi.Constants.METAINF_SERVICES_STRATEGY_NONE;

import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Map;

import aQute.bnd.annotation.Resolution;
Expand All @@ -23,7 +25,8 @@

/**
* process the META-INF/services/* files. These files can contain bnd
* annotations.
* annotations. Use instruction {@link Constants#METAINF_SERVICES} to control
* this.
*/

public class MetaInfServiceParser implements AnalyzerPlugin {
Expand All @@ -49,26 +52,62 @@ public boolean analyzeJar(Analyzer analyzer) throws Exception {
}
}

MetaInfService.getServiceFiles(analyzer.getJar())
.values()
Collection<MetaInfService> allServices = MetaInfService.getServiceFiles(analyzer.getJar())
.values();

// "auto" applies only to services without any annotation at all. so
// divide them
Collection<MetaInfService> withAnnotations = new ArrayList<MetaInfService>();
Collection<MetaInfService> withoutAnnotations = new ArrayList<MetaInfService>();

allServices.forEach(mis -> {
mis.getImplementations()
.values()
.forEach(impl -> {
Parameters annotations = impl.getAnnotations();

if (!annotations.isEmpty()) {
withAnnotations.add(mis);
} else {
withoutAnnotations.add(mis);
}

});
});

if (METAINF_SERVICES_STRATEGY_AUTO.equals(strategy)) {
withoutAnnotations
.stream()
.flatMap(mis -> mis.getImplementations()
.values()
.stream())
.forEach(impl -> {

Parameters annotations = new Parameters();
Attrs attrs1 = new Attrs();
attrs1.put(Constants.RESOLUTION_DIRECTIVE, Resolution.OPTIONAL);
attrs1.addDirectiveAliases();
annotations.add(ServiceProvider.class.getName(), attrs1);

annotations.forEach((annotationName, attrs) -> {
doAnnotationsforMetaInf(analyzer, impl, Processor.removeDuplicateMarker(annotationName), attrs);
});
});
}

withAnnotations
.stream()
.flatMap(mis -> mis.getImplementations()
.values()
.stream())
.forEach(impl -> {
Parameters annotations = impl.getAnnotations();

if (annotations.isEmpty() && METAINF_SERVICES_STRATEGY_AUTO.equals(strategy)) {
Attrs attrs = new Attrs();
attrs.put(Constants.RESOLUTION_DIRECTIVE, Resolution.OPTIONAL);
attrs.addDirectiveAliases();
annotations.add(ServiceProvider.class.getName(), attrs);
}

annotations.forEach((annotationName, attrs) -> {
doAnnotationsforMetaInf(analyzer, impl, Processor.removeDuplicateMarker(annotationName), attrs);
});
});

return false;
}

Expand All @@ -90,6 +129,6 @@ private void doAnnotationsforMetaInf(Analyzer analyzer, Implementation impl, Str
}

private String strategy(Analyzer analyzer) {
return analyzer.getProperty(METAINF_SERVICES, METAINF_SERVICES_STRATEGY_AUTO);
return analyzer.getProperty(METAINF_SERVICES, METAINF_SERVICES_STRATEGY_ANNOTATION);
}
}

0 comments on commit 1e130b2

Please sign in to comment.