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

Another approach to fix 2563 #2573

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

msevcenko
Copy link

@msevcenko msevcenko commented Dec 11, 2023

Fixes #2563

This is alternative, more conservative approach to #2571 . It doesn't attempt to fix the issue out of the box, but allows user code to intercept type variables.

The trick is the following. ObjectTypeAdapter instance is split into two. The first instance, which cannot be overriden, does not trigger on the mere condition type.getRawType() == Object.class. Specifically, it passes through TypeTokens if TypeToken.getType is a type variable, with a non-Object bound.

There is another instance that maps all remaining TypeTokens with rawType being Object.class, to ObjectTypeAdapter. But this second instance comes only after user defined adapters, so user has a chance to intercept type variables.

You may then fix the issue with the following custom adapter factory:

/**
 * This adapter factory works around the https://github.com/google/gson/issues/2563 gson issue. Unfortunately it must be
 * applied to each affected field using the @JsonAdapter(ResolveGenericBoundFactory.class) declaration, as it is
 * otherwise superseded by the ObjectTypeAdapter.
 * <p>
 * It is also registered with GsonFactory, but is ineffective without annotation, unless PR 2573 is implemented.
 */
public class ResolveGenericBoundFactory implements TypeAdapterFactory  {

	@Override
	public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
		if (type.getType() instanceof TypeVariable<?> tv) {
			Type[] bounds = tv.getBounds();
			if (bounds.length == 1 && bounds[0] != Object.class) {
				Type bound = bounds[0];
				return (TypeAdapter<T>) gson.getAdapter(TypeToken.get(bound));
			}
		}
		return null;
	}
	
}

There is still a change in behavior. Namely, custom type adapters get type variable tokens to process. This may be unexpected for them, they may fail on such an input. Before the change in behavior, custom type adapters never get the Type of TypeVariable, as this was unconditionally handled by the non-overridable ObjectTypeAdapter.

Even more conservative approach may be to introduce new option to GsonBuilder to trigger this new behavior.

Purpose

The purpose is to fix #2563 as described therein.

Description

See discussion in #2563. See InferenceFromTypeVariableTest for demo.

@msevcenko msevcenko mentioned this pull request Dec 11, 2023
Comment on lines 85 to 87
if (toNumberStrategy == ToNumberPolicy.DOUBLE) {
return DOUBLE_FACTORY;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? DOUBLE_FACTORY is created with skipTypeVariable = true, but here the skipTypeVariable argument of this method is not checked, so it might actually be false.

return (TypeAdapter<T>) new ObjectTypeAdapter(gson, toNumberStrategy);
}
return null;
}

private boolean isTypeVariableWithBound(Type type) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rewrite this method as

private boolean isTypeVariableWithBound(Type type) {
  return type instanceof TypeVariable<?> && 
         type != Object.class && 
         Arrays.stream(((TypeVariable<?>) type).getBounds()).anyMatch(Class.class::isInstance);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic deserializer should extract type information from generic boundary
3 participants