FactoryRegistry Refactoring for Java 9 Compatibility
Details:
- contact: Nicolai
- issue: GEOT-5289
- TL;DR:
org.geotools.factory.FactoryRegistry
extendsjavax.imageio.spi.ServiceRegistry
, but on Java 9 the latter throws an exception if extended by non-JDK classes; this breaks GeoTools on Java 9 - pull request: #1670
- target: GeoTools 19
In JDK 9 javax.imageio.spi.ServiceRegistry
verifies that it is extended by JDK-classes only. For GeoTools' org.geotools.factory.FactoryRegistry
that means that an exception is thrown, thus crashing GeoTools. To fix this, it is necessary to reimplement the ServiceRegistry
functionality that is needed by GeoTools in FactoryRegistry
.
Any such reimplementation causes (at least) a runtime incompatibility, though: Factories that implement javax.imageio.spi.RegisterableService
are supposed to be called when registered but the interface's methods need to be called with a ServiceRegistry
instance - which FactoryRegistry
can no longer be. There is hence no way to call back to factories that implement RegisterableService
.
That incompatibility requires FactoryRegistry
clients to invest some time into making their code work with the new version. This can be used to further improve the class' API:
- name new methods according to GeoTools' terminology (e.g.
registerFactory
) as opposed to the service registry's (registerServiceProvider
) - have registry methods return
Stream
instead ofIterator
to allow clients to use more modern APIs and language features (note that Stream::iterator makes it easy to switch from streaming to iterating) - replace use of
javax.imageio.spi.RegisterableService
with the neworg.geotools.factory.RegistrableFactory
- replace use of
javax.imageio.spi.ServiceRegistry.Filter
withjava.util.function.Predicate
Together, this would eliminate all dependencies on the javax.imageio.spi
package.
To give users a transition period, it is possible to provide deprecated overloads of all new methods that have the old signature (name, parameters, return types). Note that due to the required switch from RegisterableService
to RegistrableFactory
this can still not provide 100% compatibility. To inform users about this, we should detect instances of RegisterableService
that are not also of type RegistrableFactory
and log a warning.
References:
- Under Discussion
- In Progress
- Completed
- Rejected
- Deferred
Voting:
- Andrea Aime +1
- Ben Caradoc-Davies +1
- Christian Mueller
- Ian Turton +1
- Justin Deoliveira +1
- Jody Garnett +1
- Simone Giannecchini +0
- reimplement
ServiceRegistry
capabilities inFactoryRegistry
, so that it works on Java 9 - use GeoTools' terminology for method names
- use
Predicate
instead ofServiceRegistry.Filter
- return
Stream
instead ofIterator
- provide backwards-compatible overloads
- update GeoTools-internal calls to
FactoryRegistry
- document new API with JavaDoc
- update GeoTools documentation to inform users of changes
public class FactoryRegistry extends ServiceRegistry {
...
public synchronized <T> Iterator<T> getServiceProviders(final Class<T> category, final Filter filter, final Hints hints) {
..
}
...
public class FactoryRegistry {
...
public synchronized <T> Iterator<T> getFactories(final Class<T> category, final Filter filter, final Hints hints) {
..
}
...
Iterator<?> factories = // ...
registry.registerServiceProviders(factories)
Object factory = // ...
registry.deregisterServiceProvider(factory)
Iterator<?> factories = // ...
registry.registerFactories(factories)
Object factory = // ...
registry.deregisterFactory(factory)
Class<?> category = // ...
Filter filter = // ...
boolean useOrdering = // ...
for (final Iterator it = registry.getServiceProviders(category, filter, useOrdering); it.hasNext(); ) {
final Object factory = it.next();
// ....
}
Class<?> category = // ...
Filter filter = // ...
boolean useOrdering = // ...
registry.getFactories(category, filter, useOrdering)
.forEach( /* ... */ );
If streams don't work for some use case:
for (final Iterator it = registry.getFactories(category, filter, useOrdering).iterator(); it.hasNext(); ) {
final Object factory = it.next();
// ....
}
Alternatively:
final Iterable<?> factories = registry.getFactories(category, filter, useOrdering)::iterator;
for (Object factory : factories) {
// ....
}
Reviewing the GeoServer codebase shows two sections that will need to be updated:
spiExtensions = new ArrayList<Object>();
Iterator i = FactoryRegistry.lookupFactories(extensionPoint);
while( i.hasNext() ) {
spiExtensions.add( i.next() );
}
Iterator<DXFWriter> it = writerRegistry.getFactories(DXFWriter.class, null, null)::iterator;
FactoryFinder implementations already use GeoTools FactoryRegistry, the implementation is now standalone and no longer extends the Java ServiceRegistry base class.
BEFORE:
public final class CommonFactoryFinder extends FactoryFinder {
/**
* The service registry for this manager.
* Will be initialized only when first needed.
*/
private static FactoryRegistry registry;
...
}
AFTER:
public final class CommonFactoryFinder extends FactoryFinder {
/**
* The factory registry for this manager.
* Will be initialized only when first needed.
*/
private static FactoryRegistry registry;
...
}
Code using a FactoryFinder remains unchanged:
final FilterFactory ff = CommonFactoryFinder.getFilterFactory();
Filter filter = ff.propertyLessThan( ff.property( "AGE"), ff.literal( 12 ) );
File file = new File("example.shp");
Map map = new HashMap();
map.put( "url", file.toURL() );
DataStore dataStore = DataStoreFinder.getDataStore(map );