Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GERONIMO-6560 make use of geronimo-osgi-locator for loading service loader implementations in OSGi environment #5

Closed
wants to merge 1 commit into from
Closed

GERONIMO-6560 make use of geronimo-osgi-locator for loading service loader implementations in OSGi environment #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions geronimo-json_1.0_spec/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@
</repositories>

<dependencies>
<dependency>
<groupId>org.apache.geronimo.specs</groupId>
<artifactId>geronimo-osgi-locator</artifactId>
<version>1.1</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand Down Expand Up @@ -97,6 +103,9 @@
<Export-Package>
javax.json*;version=1.0
</Export-Package>
<Import-Package>org.apache.geronimo.osgi.registry.api;resolution:=optional,*</Import-Package>
<Private-Package>org.apache.geronimo.osgi.locator</Private-Package>
<Bundle-Activator>org.apache.geronimo.osgi.locator.Activator</Bundle-Activator>
</instructions>
</configuration>
</plugin>
Expand Down
8 changes: 5 additions & 3 deletions geronimo-json_1.0_spec/src/main/java/javax/json/spi/JsonProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
import javax.json.stream.JsonGeneratorFactory;
import javax.json.stream.JsonParser;
import javax.json.stream.JsonParserFactory;

import org.apache.geronimo.osgi.locator.ProviderLocator;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you care updating it to do it by reflection? I'd like to keep that dependency running without osgi-locator if possible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the required classes are embedded in the jar file (via maven-bundle-plugin) - so the resulting jar file does no longer have a dependency to geronimo-osgi-locator - neither in OSGi nor in maven.
it's the same pattern as used by geronimo-jaxrs_1.1_spec for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then please ensure it is shaded in another package. issue is several spec jars have all the same classes and it can lead to issues in some classloading environments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although i've solved this successful in other osgi bundles using maven-shade-plugin it does not work very well in this case - because one of the located classes is used as Bundle-Activator, and relocating the class breaks the magic of the bundle plugin generating the correct imports. after some puzzling i got it working: https://github.com/stefanseifert/geronimo-specs/blob/feature/GERONIMO-6560-json10-osgi-shade/geronimo-json_1.0_spec/pom.xml
but the solution is quite hacky and fragile - i advice against using it.

i would propose that we either use the original PR without relocating the classes (as in jaxrs) - or completely drop the support for geronimo osgi-support (which is older than the release of the osgi r5 spec, which is already old), ans switch for this spec jar and johnzon to an osgi-compatible way with capabilities. i've never tried it out, but in my understanding no special embedded classes are required in this case. aries spyfly has to be used then.


import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -156,9 +159,8 @@ private static JsonProvider doLoadProvider() throws JsonException {
tccl = ClassLoader.getSystemClassLoader();
}
try {
final Class<?> clazz = Class.forName("org.apache.geronimo.osgi.locator.ProviderLocator");
final Method getServices = clazz.getDeclaredMethod("getServices", String.class, Class.class, ClassLoader.class);
final List<JsonProvider> osgiProviders = (List<JsonProvider>) getServices.invoke(null, JsonProvider.class.getName(), JsonProvider.class, tccl);
@SuppressWarnings("unchecked")
final List<JsonProvider> osgiProviders = (List)ProviderLocator.getServices(JsonProvider.class.getName(), JsonProvider.class, tccl);
if (osgiProviders != null && !osgiProviders.isEmpty()) {
return osgiProviders.iterator().next();
}
Expand Down
12 changes: 12 additions & 0 deletions geronimo-json_1.1_spec/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@
</releases>
</repository>
</repositories>

<dependencies>
<dependency>
<groupId>org.apache.geronimo.specs</groupId>
<artifactId>geronimo-osgi-locator</artifactId>
<version>1.1</version>
<scope>provided</scope>
</dependency>
</dependencies>

<build>
<plugins>
Expand All @@ -83,6 +92,9 @@
<Export-Package>
javax.json*;version=1.0
</Export-Package>
<Import-Package>org.apache.geronimo.osgi.registry.api;resolution:=optional,*</Import-Package>
<Private-Package>org.apache.geronimo.osgi.locator</Private-Package>
<Bundle-Activator>org.apache.geronimo.osgi.locator.Activator</Bundle-Activator>
</instructions>
</configuration>
</plugin>
Expand Down
8 changes: 4 additions & 4 deletions geronimo-json_1.1_spec/src/main/java/javax/json/spi/JsonProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,15 @@
import javax.json.stream.JsonParser;
import javax.json.stream.JsonParserFactory;

import org.apache.geronimo.osgi.locator.ProviderLocator;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.Reader;
import java.io.Writer;
import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.net.URL;
Expand Down Expand Up @@ -90,9 +91,8 @@ public JsonProvider run() {
private static JsonProvider doLoadProvider() throws JsonException {
final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
try {
final Class<?> clazz = Class.forName("org.apache.geronimo.osgi.locator.ProviderLocator");
final Method getServices = clazz.getDeclaredMethod("getServices", String.class, Class.class, ClassLoader.class);
final List<JsonProvider> osgiProviders = (List<JsonProvider>) getServices.invoke(null, JsonProvider.class.getName(), JsonProvider.class, tccl);
@SuppressWarnings("unchecked")
final List<JsonProvider> osgiProviders = (List)ProviderLocator.getServices(JsonProvider.class.getName(), JsonProvider.class, tccl);
if (osgiProviders != null && !osgiProviders.isEmpty()) {
return osgiProviders.iterator().next();
}
Expand Down