-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
Feature request: registering subtypes without annotating the parent class #2104
Comments
Hmm, I realized that getting the subclasses of a given class requires scanning the entire classpath, so it might not be something you want to do by default in Jackson, for performance reasons. |
There's a pattern used by the Dropwizard framework that works pretty well. Put the subclasses in the Example:
In your jackson module: class MyJacksonModule extends SimpleModule {
public MyJacksonModule() {
final List<Class<? extends T>> serviceClasses = new ArrayList<>();
try {
// use classloader that loaded this class to find the service descriptors on the classpath
// better than ClassLoader.getSystemResources() which may not be the same classloader if ths app
// is running in a container (e.g. via maven exec:java)
final Enumeration<URL> resources = getClassLoader().getResources("META-INF/services/" + serviceType.getName());
while (resources.hasMoreElements()) {
final URL url = resources.nextElement();
CharStreams.readLines(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8)).forEach(line -> {
@SuppressWarnings("unchecked")
final Class<? extends T> loadedClass = (Class<? extends T>) loadClass(line);
if (loadedClass != null) {
serviceClasses.add(loadedClass);
}
});
}
} catch (IOException e) {
log.warn("Unable to load META-INF/services/{}", serviceType.getName(), e);
}
registerSubtypes(serviceClasses);
}
} |
@toadzky So you never want to put anything but an SPI into that services folder hey, likeever. It is a reserved java folder and when you start with your servicing layer and SPI, you are gonna kick yourself (Like I did) Only ServiceLoader.load() must ever use the META-INF/services folder @wetneb ClassGraph is the one to go for, https://github.com/classgraph/classgraph - Only JPMS one, and very fast moving Why not just register them with SPI? Whats the reason for not doing this, skip the entire classpath scan altogether |
There nothing wrong with using the services folder if you follow the conventions. As for why not use SPI, because SPI requires a no-args constructor and instantiates an instance of the classes in the list. Since what we want is a class object, not an instance of the class, using SPI wouldn't help. And using things like class graph is not my preference because I prefer to be explicit about what's going on in my apps. I don't like classpath scanning. As for JPMS, it's not really relevant to the discussion because the original request doesn't specify a version and a lot of places aren't using 9+. Most people I know who are running in 9+ just turn off JPMS because they don't want to deal with it. |
Using services folder : agree to disagree, also since jdk 8 its been in stone, you just don't do it. you can do it,sure, but you really really really shouldn't. https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#The_META-INF_directory Well.... pick one David, SPI or Classgraph, you can't have software engineer syndrome (i didn't make it i don't trust it) and get the best of both worlds you know xD You are going to have to choose at some point. JPMS is where it is at, most people are running because they don't know, and the people that do know, we don't even entertain class path mode anymore. Not thinking in terms of the future of the language, and how for the first time Java is no longer "Backwards Compatible", not following will leave you by the river. We now make modules "Forward Compatible", we ignore backwards compatibility. Loading the SPI object from the module path in JPMS btw, is better performing than loading a class from the class loader. |
That Oracle link doesn't say a single word about not using it. What I suggested doing with it is 100% in line with everything in that document. As for the rest, you have completely derailed the thread ranting about philosophy and arguing strawmen. None of this is helpful for the original poster. The fact that you think JPMS is a great thing doesn't change the fact that there are a lot of companies that are still on JDK8 and have no plans to upgrade any time soon. There is a reason that RedHat, Amazon, among others are promising to continue producing supported JDK8 builds and backporting security patches. I'm unsubscribing from this issue because this discussion is 100% unproductive and pointless. |
agh please, Java is based on ones, one purpose. My suggestion is to use SPI on this, place the files in the right place, but not hack a solution to read custom files from a designatedx location which are not Services. |
A few workable ideas: My experiences with SPI for Java 8 and earlier make me pretty skeptic of using it for anything (and possible even more so for registering services actually.... performance is horrendous, no way to deal with duplicates, mere existence of something in classpath changing things in weird and non-reproducible ways). But then again, it is a somewhat documented mechanism and Jackson does expose metadata already and even allows auto-registering of Modules (which I personally never use and do not recommend but that's matter of taste :) ). So: if anyone wants to create prototype module, great; that would be reasonable way to add such functionality for users I think. |
I have made a simple module implementation (with SPI) and look forward to your suggestions :) |
Hi all, any news on this? There is another approach that would make the listing of This is what Spring does in similar cases. As in Spring, the default might be the same package where the annotation is used. Details for each subtype could be provided by |
No news on this. I am firmly against adding any form of class/module-path scanning within core Jackson codebase, but positive about additional module. Unfortunately I haven't had time to actually look into module contributed -- I hope someone can help me here. |
@black-06 I think this would make sense: do you think you have time to try to merge new module into https://github.com/FasterXML/jackson-modules-base ? An example PR for something I just merged, Android Record module: FasterXML/jackson-modules-base#227 could help. And if we could get this done quickly, it just might make it into 2.16.0 -- for that branch to do PR against would be EDIT: due to timing issues, 2.16 did not happen. Now targeting 2.17. |
When using
@JsonTypeInfo
withuse = Id.NAME
, it is currently required to provide the name to class map with the@JsonSubTypes
annotation on the parent class. This is problematic if a third-party wants to extend my code: they will have to modify my code to register their new subclass.It would be fantastic if the mapping could be provided via annotations on the subclasses themselves. Something like this:
This would require retrieving at runtime the subclasses and building the map based on their annotations. This can already be achieved by implementing a custom
TypeIdResolver
but the use case seems to be fairly common so I think it would make sense to add this to Jackson directly.Related SO question: https://stackoverflow.com/questions/34079050/add-subtype-information-at-runtime-using-jackson-for-polymorphism
The text was updated successfully, but these errors were encountered: