I *also* wonder if it's just the Oracle JDK7 compiler implementation which is introducing this restriction. I'm not opposed to mandating we build the project with another vendor's compiler.
IBM JDK: Java version: 1.7.0, vendor: IBM Corporation
Oracle JDK: Java version: 1.7.0_03, vendor: Oracle Corporation
OpenJDK: Java version: 1.7.0_147-icedtea, vendor: Oracle Corporation
This is legal compilation under JDK6:
// access flags 0x401
// signature (Ljava/lang/Class<Ljava/io/File;>;)[Ljava/io/File;
// declaration: java.io.File as(java.lang.Class<java.io.File>)
public abstract as(Ljava/lang/Class;)[Ljava/io/File; throws java/lang/IllegalArgumentException
// access flags 0x401
// signature (Ljava/lang/Class<Ljava/io/InputStream;>;)[Ljava/io/InputStream;
// declaration: java.io.InputStream as(java.lang.Class<java.io.InputStream>)
public abstract as(Ljava/lang/Class;)[Ljava/io/InputStream; throws java/lang/IllegalArgumentException
I was thinking of making as() an SPI point. So methods would be T as(Class<T> clazz) and T asSingle(Class<T> clazz). T would be any type,
there would be a FormatterFactory, which will delegate to an impl which is able to handle T or fail with an exception.
That way we:
+ 1/ Keep the API simpler and nicer
+ 2/ Allow users to wrap results as whatever type they want
+ 3/ It would force us to wrap ArtifactResult object into proper SPI
- 1/ We loose type safe API in sense that user would be able to retype to JavaArchive.class even if that formatter is not on classpath, failing in runtime instead of compile time.
I have to do a prototype impl to see if that works first though.
I believe 2) and 3) are still covered by my patch, which continues to provide:
<RETURNTYPE> RETURNTYPE as(Class<RETURNTYPE> type, FormatProcessor<RETURNTYPE> processor) throws IllegalArgumentException; <RETURNTYPE> RETURNTYPE asSingle(Class<RETURNTYPE> type, FormatProcessor<RETURNTYPE> processor) throws IllegalArgumentException, NonUniqueResultException, NoResolvedResultException;
In this way we still accept an extensible FormatProcessor without losing type safety (and IDE autocomplete makes it clear to the user what operations are permitted:
...or am I missing some other benefit to your FormatterFactory approach? If you put up a commit/patch for review we can debate the path to move us forward.
Seen that, but currently occupied with other stuff. I'll definitely provide an implementation to discuss "soonish". The only benefit of my approach is that it avoids method explosion.
Cool. My preference will probably go for whatever's most intuitive, looking forward to seeing it.
I finally managed to do the prototype impl. It is stored here:
1/ An SPI service to replace existing one allowing to register multiple services per interface
2/ Changed FormatProcessor API
3/ Changed MavenFormatStageImpl to dynamically load FormatProcessors
4/ Changed FormatStage to allow any RETURTYPE type, this means we do not longer fail compile-time
5/ Aligned ResolvedArtifactInfo
Side effect fixes:
1/ [not yet reported] Former SPI does not close input stream
2/ [SHRINKRES-79] MavenDependencyImpl exclusion ordering
Changes to be done if this implementation is preferred:
1/ Create a proper SPI module
2/ Propagate registered services, e.g. search classpath only once
3/ Remove all other FormatStages and reduce number of entry points, simply generics in the project
Karel and I reviewed this in IRC, and iniitally I really liked this proposal. On further inspection, though, I've found a problem:
By registering the supported FormatProcessors at runtime, we lose the ability to give the users a hint as to what's supported at compile-time. They know they're supposed to put in Class<SOMETHING>, but how are they to know that File.class, InputStream.class, etc are supported?
For this reason and this reason alone, I'd prefer to go with the method-overload approach in my patch. It's not as pretty, but IMO it does make for a more intuitive experience.
Another hitch: it's impossible to resolve a ResolvedArtifactInfo from the FormatProcessor SPI - the contract doesn't provide enough info.
As for FormatProcessor, its API is poorly designed...we need a specific object to be passed between Strategy and Formatter. Then we can have a contract which will make ResolvedArtifactInfo processor possible.
I'm thinking of a compromise. Having asFile() and as(Class<?> returnType). People need mostly files, right? More advanced types we can document.
I've updated SHRINKRES-78 branch at https://github.com/kpiwko/resolver/tree/SHRINKRES-78 with following changes:
1/ Refactored ResolvedArtifactInfo into ResolvedArtifact, which is now primary object passed from StrategyStage to FormatStage
2/ Modified FormatProcessors to handle ResolvedArtifact (or its subtype) instead of File
3/ Modified FormatStage to have following methods (e.g. used almost your approach while removing direct injection of FormatProcessors and adding asResolvedArtifact()):
- as(Class<RETURNTYPE), asFile(), asInputStream(), asResolvedArtifact()
- asSingle variants of above
4/ ServiceRegistry which acts as an app-scoped container for registered services, solving ClassLoader propagation
5/ Updated MavenResolvedArtifact to have the same API as FormatStage
The only call where as(X) is needed is with as(Class<Archive<?>>). With SPI support, we still have possibility to trim number of interfaces and remove ShrinkWrapMaven altogether, as discussed on irc.
Great hybrid approach, I think. Well done.
Currently in upstream/master:
- I pulled from Karel's branch and fixed a failing test
- I separated out the FormatProcessor stuff from the API into a proper SPI - users shouldn't be seeing it. Some reflection was required for this bit. Documented as https://issues.jboss.org/browse/SHRINKRES-82.
- Removed ShrinkWrapMaven and the rest of the api-maven-archive module. Documented as https://issues.jboss.org/browse/SHRINKRES-83.
So there's no longer IDE autocomplete for as(JavaArchive.class), but I think that's livable, especially since we got to remove that pesky second ShrinkWrapMaven entry point (which served only to close the generic context in a way that extended the API).
All of this in 2.0.0-alpha-5. Not a bad release, I must say. Thanks to all.