Skip to content
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

Deprecated ObjectReader.withType(Type) has no direct replacement; need forType(Type). #4153

Closed
garretwilson opened this issue Oct 10, 2023 · 6 comments
Labels
2.16 Issues planned for 2.16 to-evaluate Issue that has been received but not yet evaluated
Milestone

Comments

@garretwilson
Copy link

garretwilson commented Oct 10, 2023

Describe your Issue

The latest Jackson databind 2.15.2 deprecates ObjectReader.withType(java.lang.reflect.Type valueType). The API documentation says:

Use {@link #forType(Class)} instead

However ObjectReader.forType(Class<?> valueType) is not a direct replacement, as not all Type instances are Class instances.

For example, let's say I want to unmashal some JSON to match the type of a method parameter. (This follows some discussion on FasterXML/java-classmate#69.) With the deprecated method I could do this:

Type parameter1Type = method.getGenericParameterTypes()[0];
ObjectReader typeAwareObjectReader = objectReader.withType(parameter1Type);

But now that the method ObjectReader.withType(java.lang.reflect.Type valueType) is deprecated and going away, I'll have to do this:

Type parameter1Type = method.getGenericParameterTypes()[0];
JavaType parameter1JavaType = objectReader.getConfig().getTypeFactory().constructType(parameter1Type);
ObjectReader typeAwareObjectReader = objectReader.forType(parameter1JavaType);

An extra step with no benefit of doing it manually. (It is included as part of the deprecated ObjectReader.withType(java.lang.reflect.Type valueType).)

Is there any reason not to have an ObjectReader.forType(java.lang.reflect.Type valueType) like the one that is going away, just with this better name?

@garretwilson garretwilson added the to-evaluate Issue that has been received but not yet evaluated label Oct 10, 2023
@cowtowncoder
Copy link
Member

I think this may be an oversight: the idea wrt deprecating withType() was to instead use name forType().

I'll see if there's anything else going on: if not, can (and should) add to 2.16 -- the main concern being that Class<?> taking one cannot really be removed for backwards compatibility.

@cowtowncoder
Copy link
Member

Ok. I suppose it may have seemed unnecessary to offer 4th overload as 3 main types are covered; and possibly to remove temptation to just pass java.lang.reflect.Type. In fact your example:

Type parameter1Type = method.getGenericParameterTypes()[0];
ObjectReader typeAwareObjectReader = objectReader.withType(parameter1Type);

is probably something I'd suggest as sort of dangerous (given that it does not actually pass enough type context to resolve type parameters declared in either enclosing Class or method-local upper bounds) and sort of "please dont do this unless you REALLY know what's going on" case.

But I guess I can add this, with appropriate warnings: it will go in 2.16.0 (being API addition).

@garretwilson
Copy link
Author

garretwilson commented Oct 10, 2023

… it does not actually pass enough type context to resolve type parameters declared in either enclosing Class or method-local upper bounds …

I'll admit I need to re-read and re-re-read your blog post about this so I can get all the subtleties straight …

… is probably something I'd suggest as sort of dangerous …

… but for the moment, I just want to parse some JSON and get it so that it matches a method parameter type. Currently the Method gives me a Type for the parameter generic parameter type, so that's all I have to go on.

Maybe I should use ClassMate to resolve the method type (side note: I couldn't immediately find a way to do that without resolving the entire class and all its methods and fields, etc.), and that would give me a ResolvedType, which I'm sure gives me the information you note that I'm missing with just a Type, but then … what would I do with it? (see FasterXML/java-classmate#69)

As it turns out, in my code I didn't wind up needing an ObjectReader for this, as (for the moment) I'm parsing the JSON into a map and then converting it (actually just part of the Map<> object graph), so right now I'm using ObjectMapper—but the same ideas apply:

/**
  * Converts a value (which may be a node in a map of already parsed JSON) to the indicated type.
  * @param <T> The expected type.
  * @param fromValue The existing value.
  * @param toValueType The destination type, which may be a {@link Class} or even some other type that provides further generics information, such as
  *          {@link ParameterizedType}.
  * @return The converted value.
  * @throws IllegalArgumentException if the argument cannot be converted.
  */
public static <T> T convertValue(final Object fromValue, final Type toValueType) throws IllegalArgumentException {
  return OBJECT_MAPPER.convertValue(fromValue, OBJECT_MAPPER.getTypeFactory().constructType(toValueType));
}

@cowtowncoder
Copy link
Member

No problem; I am adding the overload just for API compatibility (granted method has been missing for a while but... oh well).

@garretwilson
Copy link
Author

garretwilson commented Oct 10, 2023

… "please dont do this unless you REALLY know what's going on" …

I sort-of know what's going on, but if you know a better way to get the full type information from a method parameter, and then tell Jackson to use that full type information instead of the Type information I'm using now, I would sincerely love to know! What's a better way to tell Jackson to parse something and produce exactly the type indicated by a method parameter, determined at runtime? (I suppose "exactly" isn't the right word here. Perhaps "completely assignable".)

@cowtowncoder cowtowncoder added this to the 2.16.0 milestone Oct 10, 2023
@cowtowncoder cowtowncoder added the 2.16 Issues planned for 2.16 label Oct 10, 2023
@cowtowncoder
Copy link
Member

@garretwilson It all goes back to having context, so -- as far as I know -- the only reliable way is to resolve full Class<?> declarations; even when only needing Method, Field etc types. This is why ClassMate does this.
And Jackson tries to encourage that, to a degree.

And yes, this sort of suggests that ability to use types ClassMate resolves -- it does it still better than Jackson, since I wrote it after Jackson -- would be good addition to Jackson, in some way or form.

But as to how to make Jackson use full type information... TypeFactory does allow constructing things programmatically to cover cases I can think of. Whether that can be used for general handling I don't know.

And of course there is always Java Type Erasure that ultimately limits runtime information determination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.16 Issues planned for 2.16 to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

2 participants