FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.52k stars 1.38k forks source link

Feature request: registering subtypes without annotating the parent class #2104

Open wetneb opened 6 years ago

wetneb commented 6 years ago

When using @JsonTypeInfo with use = 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:

@JsonTypeInfo(use = Id.NAME,
              include = JsonTypeInfo.As.PROPERTY,
              property = "type")
public interface Parent {
     // ...
}

@JsonTypeName(name = "first-child")
public class FirstChild extends Parent {
    // ... 
}

@JsonTypeName(name = "second-child")
public class SecondChild extends Parent {
   // ...
}

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

wetneb commented 6 years ago

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.

toadzky commented 5 years ago

There's a pattern used by the Dropwizard framework that works pretty well. Put the subclasses in the META-INF/services directory under the interface and just load the file, iterating through the lines of the file in a Jackson module calling registerSubtypes. Then you can just annotate each class with @JsonTypeName and it just works.

Example: META-INF/services/package.Parent

package.FirstChild
package.SecondChild

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);
    }
}
GedMarc commented 5 years ago

@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 In JPMS this becomes "provides JSonTypeInfo with Animal" - which should be the end goal as it circumvents the strict module declarations, and you don't have to "export animal", because it is an SPI?

toadzky commented 5 years ago

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.

GedMarc commented 5 years ago

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. But regardless of what our opinions are on the topic, one day one way or another class path mode is going to go away. And abusing backwards compatibility will be that Rottweiler that you pocked fun at, walk past, and on that day the gate is open. And you're stuck.

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. When it releases, you have 6 months to implement, then 6 months to run, then you've got to up your version again. Welcome to the age of predictability within Java. Topping it all off, wasn't even Java's idea to roll out every 6 months. Was IBM, everything now releases in 6 month cycles, from Ubuntu, Windows, Office, Wildfly, MicroProfile, all of it.

Loading the SPI object from the module path in JPMS btw, is better performing than loading a class from the class loader.

toadzky commented 5 years ago

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.

GedMarc commented 5 years ago

agh please, Java is based on ones, one purpose. I do not consider your solution viable, I consider it as breaking a structure that is already in place, or misusing it over it's original purpose. The services directory is reserved.

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.

image

image

cowtowncoder commented 5 years ago

A few workable ideas: registerSubtypes() (esp. via Module) is the way to go. I am not sure I would want to have functionality for this directly in databind, but if someone wants to work on general-purpose extension module, that could be added under FasterXML if it proves useful.

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 :) ). And Java 9+ module path probably makes access less inefficient as well, so as mechanism maybe it is becoming more usable.

So: if anyone wants to create prototype module, great; that would be reasonable way to add such functionality for users I think.

black-06 commented 1 year ago

I have made a simple module implementation (with SPI) and look forward to your suggestions :) jackson-modules-dynamic-subtype

marco-brandizi commented 1 year ago

Hi all, any news on this?

There is another approach that would make the listing of @JsonSubTypes simpler: use an annotation, eg, @JsonSubTypes ( scan = "com.foo" ) to tell Jackson where to find subtypes.

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 @JsonSubTypes.Type placed on the top of each subtype, which still requires some explicit definitions, but at least they would be placed in the subclasses, rather than in their parents. The major problem with the latter is that it violates the Open-Closed principles from SOLID and makes a class annotated with @JsonSubTypes + @Type(s) hard to extend.

cowtowncoder commented 1 year ago

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. If it is well implemented, we could consider addition within jackson-modules-base.

cowtowncoder commented 11 months ago

@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:

https://github.com/FasterXML/jackson-modules-base/pull/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 2.16.

EDIT: due to timing issues, 2.16 did not happen. Now targeting 2.17.