On Tue, Apr 21, 2015 at 11:06 PM, Romain Manni-Bucau
<[email protected]> wrote:
> Le 21 avr. 2015 22:51, "Hendrik Dev" <[email protected]> a écrit :
>>
>> A few thoughts and questions on
>>
>> JsonProvider.doLoadProvider():
>> - "tccl" can be null (in case of system classloader) but thats never
>> really checked
>
> If so johnzon cant be loaded isnt it? So not a big deal imo
someone could set the context classloader explicitly to null, see attached diff
>
>> - special handling org.apache.geronimo.osgi.locator.ProviderLocator
>> really needed here?
>>
>
> In G spec jars yes.
OK
>
>> JsonProvider.provider():
>> - doPrivileged/SecurityManager check really needed here?
>
> For containers yes and doesnt hurt at runtime normally.
OK
>
>> - method seems thread safe but we do not cache the returned provider
>> instance. Maybe we can to this in a thread local variable?
>>
>
> Not cached for container case + i dont expect it to be called often.
>
> Thread local would break ears or wars if johnzon is in one war, jackson in
> another and api in the container for instance + it would leak on undeploy.
i see, so maybe caching the hole provider, see attached diff
>
> Did you hit any issue?
nothing special, originally working on that to make sure that Johnzon
is working under OSGi (Karaf), so i looked at the RI and johnzon
specs.
>
>> Thanks
>> Hendrik
>>
>> --
>> Hendrik Saly (salyh, hendrikdev22)
>> @hendrikdev22
>> PGP: 0x22D7F6EC
--
Hendrik Saly (salyh, hendrikdev22)
@hendrikdev22
PGP: 0x22D7F6EC
Index: src/main/java/javax/json/Json.java
===================================================================
--- src/main/java/javax/json/Json.java (revision 1675192)
+++ src/main/java/javax/json/Json.java (working copy)
@@ -28,67 +28,70 @@
import java.util.Map;
public class Json {
+
+ private static final JsonProvider PROVIDER = JsonProvider.provider();
+
private Json() {
// no-op
}
public static JsonParser createParser(Reader reader) {
- return JsonProvider.provider().createParser(reader);
+ return PROVIDER.createParser(reader);
}
public static JsonParser createParser(InputStream in) {
- return JsonProvider.provider().createParser(in);
+ return PROVIDER.createParser(in);
}
public static JsonGenerator createGenerator(Writer writer) {
- return JsonProvider.provider().createGenerator(writer);
+ return PROVIDER.createGenerator(writer);
}
public static JsonGenerator createGenerator(OutputStream out) {
- return JsonProvider.provider().createGenerator(out);
+ return PROVIDER.createGenerator(out);
}
public static JsonParserFactory createParserFactory(Map<String, ?> config)
{
- return JsonProvider.provider().createParserFactory(config);
+ return PROVIDER.createParserFactory(config);
}
public static JsonGeneratorFactory createGeneratorFactory(Map<String, ?>
config) {
- return JsonProvider.provider().createGeneratorFactory(config);
+ return PROVIDER.createGeneratorFactory(config);
}
public static JsonWriter createWriter(Writer writer) {
- return JsonProvider.provider().createWriter(writer);
+ return PROVIDER.createWriter(writer);
}
public static JsonWriter createWriter(OutputStream out) {
- return JsonProvider.provider().createWriter(out);
+ return PROVIDER.createWriter(out);
}
public static JsonReader createReader(Reader reader) {
- return JsonProvider.provider().createReader(reader);
+ return PROVIDER.createReader(reader);
}
public static JsonReader createReader(InputStream in) {
- return JsonProvider.provider().createReader(in);
+ return PROVIDER.createReader(in);
}
public static JsonReaderFactory createReaderFactory(Map<String, ?> config)
{
- return JsonProvider.provider().createReaderFactory(config);
+ return PROVIDER.createReaderFactory(config);
}
public static JsonWriterFactory createWriterFactory(Map<String, ?> config)
{
- return JsonProvider.provider().createWriterFactory(config);
+ return PROVIDER.createWriterFactory(config);
}
public static JsonArrayBuilder createArrayBuilder() {
- return JsonProvider.provider().createArrayBuilder();
+ return PROVIDER.createArrayBuilder();
}
public static JsonObjectBuilder createObjectBuilder() {
- return JsonProvider.provider().createObjectBuilder();
+ return PROVIDER.createObjectBuilder();
}
public static JsonBuilderFactory createBuilderFactory(Map<String, ?>
config) {
- return JsonProvider.provider().createBuilderFactory(config);
+ return PROVIDER.createBuilderFactory(config);
}
}
Index: src/main/java/javax/json/spi/JsonProvider.java
===================================================================
--- src/main/java/javax/json/spi/JsonProvider.java (revision 1675192)
+++ src/main/java/javax/json/spi/JsonProvider.java (working copy)
@@ -28,6 +28,7 @@
import javax.json.stream.JsonGeneratorFactory;
import javax.json.stream.JsonParser;
import javax.json.stream.JsonParserFactory;
+
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
@@ -39,15 +40,25 @@
import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;
-import java.util.ArrayList;
import java.util.Enumeration;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.ServiceLoader;
public abstract class JsonProvider {
+ static {
+ try {
+ Class<?> clazz =
Class.forName("org.apache.geronimo.osgi.locator.ProviderLocator");
+ getServices = clazz.getDeclaredMethod("getServices", String.class,
Class.class, ClassLoader.class);
+ } catch (Exception e) {
+ //no-op
+ }
+ }
+
+ private static final String UTF_8 = "utf-8";
private static final String DEFAULT_PROVIDER =
"org.apache.johnzon.core.JsonProviderImpl";
+ private static final String JSON_PROVIDER_CLASS_NAME =
JsonProvider.class.getName();
+ private static final String name = "META-INF/services/" +
JSON_PROVIDER_CLASS_NAME;
+ private static Method getServices;
protected JsonProvider() {
// no-op
@@ -65,11 +76,15 @@
}
private static JsonProvider doLoadProvider() throws JsonException {
- final ClassLoader tccl =
Thread.currentThread().getContextClassLoader();
+ ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+
+ if(tccl == null) {
+ //No thread context class loader set so we use the classloader of
this class
+ tccl = JsonProvider.class.getClassLoader();
+ }
+
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);
+ final List<JsonProvider> osgiProviders = (List<JsonProvider>)
getServices.invoke(null, JSON_PROVIDER_CLASS_NAME, JsonProvider.class, tccl);
if (osgiProviders != null && !osgiProviders.isEmpty()) {
return osgiProviders.iterator().next();
}
@@ -79,24 +94,18 @@
// don't use Class.forName() to avoid to bind class to tccl if thats a
classloader facade
// so implementing a simple SPI when ProviderLocator is not here
- final String name = "META-INF/services/" +
JsonProvider.class.getName();
try {
- Enumeration<URL> configs;
- if (tccl == null) {
- configs = ClassLoader.getSystemResources(name);
- } else {
- configs = tccl.getResources(name);
- }
+ final Enumeration<URL> configs = tccl.getResources(name);
if (configs.hasMoreElements()) {
InputStream in = null;
BufferedReader r = null;
- final List<String> names = new ArrayList<String>();
try {
in = configs.nextElement().openStream();
- r = new BufferedReader(new InputStreamReader(in, "utf-8"));
+ r = new BufferedReader(new InputStreamReader(in,
JsonProvider.UTF_8));
String l;
while ((l = r.readLine()) != null) {
+ l=l.trim();
if (l.startsWith("#")) {
continue;
}
