-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
TypeAdapters don't apply to map keys #1722
Comments
I think they don't have to. If you'd change the type adapter so that it writes/reads other than just a single string (including nulls) how would this type adapter recognize the context and distinguish between map keys that must be strings and map values of arbitrary JSON structure? |
Yup, I definitely agree that it’s problematic. But I think the spirit of this request is that some mechanism should exist for encoding keys. For example, a new function could be added to TypeAdapter that is just responsible for encoding a key name. |
Actually, it looks like there is already a built-in mechanism for handling complex JsonElements as keys in that file: gson/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java Lines 245 to 262 in ceae88b
That appears to be for when you're encoding JsonElements, not encoding into a stream, though. However, I think it definitely defines a precedent. |
Looks like this is my miss. Picking through the code more, this functionality is hidden behind a feature flag, final Gson gson = new GsonBuilder()
.enableComplexMapKeySerialization()
.registerTypeAdapter(Thinger.class, new ThingerAdapter())
.create(); Based on the implementation, I understand why this is hidden behind a flag -- it has to instantiate JsonElement instances for every key, which is slow. Would you be amenable to adding functionality to public abstract class TypeAdapter<T> {
// ... SNIP ...
public String toKey(T value) throws IOException {
return String.valueOf(value);
}
// ... SNIP ...
} This would allow the default behaviour to continue, shouldn't break any existing consumers, would allow custom key serialization, maintains type safety, and should have reasonable performance. Thoughts? Again, I'm new to your code base so I assume I'm missing some nuances here. |
You could just implement your custom map type adapter factory that might do the job for you or provide a transformed
It does not produce/consume a JSON object: it represents a map using a two-dimensional JSON array (with n x 2 elements, literally I do believe that if you find that the final class TransformedKeyMapTypeAdapterFactory
implements TypeAdapterFactory {
private static final Type[] rawKeyValueTypes = { Object.class, Object.class };
private final IKeyConverter keyConverter;
private final Function<? super Type, ? extends Map<?, ?>> createMap;
private TransformedKeyMapTypeAdapterFactory(final IKeyConverter keyConverter, final Function<? super Type, ? extends Map<?, ?>> createMap) {
this.keyConverter = keyConverter;
this.createMap = createMap;
}
static TypeAdapterFactory create(final IKeyConverter keyConverter) {
return new TransformedKeyMapTypeAdapterFactory(keyConverter, MapFactory::create);
}
static TypeAdapterFactory create(final IKeyConverter keyConverter, final Function<? super Type, ? extends Map<?, ?>> createMap) {
return new TransformedKeyMapTypeAdapterFactory(keyConverter, createMap);
}
@Override
@Nullable
public <T> TypeAdapter<T> create(final Gson gson, final TypeToken<T> typeToken) {
if ( !Map.class.isAssignableFrom(typeToken.getRawType()) ) {
return null;
}
final Type mapType = typeToken.getType();
// insufficient type check for simplicity
final Type[] actualTypeArguments = mapType instanceof ParameterizedType
? ((ParameterizedType) mapType).getActualTypeArguments()
: rawKeyValueTypes;
final Type keyType = actualTypeArguments[0];
final Type valueType = actualTypeArguments[1];
@SuppressWarnings("unchecked")
final TypeAdapter<Object> valueTypeAdapter = gson.getAdapter((TypeToken<Object>) TypeToken.get(valueType));
final TypeAdapter<? extends Map<?, ?>> mapTypeAdapter = TransformedKeyMapTypeAdapter.create(
key -> keyConverter.toName(keyType, key),
name -> keyConverter.fromName(keyType, name),
() -> {
@SuppressWarnings("unchecked")
final Map<Object, Object> castMap = (Map<Object, Object>) createMap.apply(mapType);
return castMap;
},
valueTypeAdapter
);
@SuppressWarnings("unchecked")
final TypeAdapter<T> castMapTypeAdapter = (TypeAdapter<T>) mapTypeAdapter;
return castMapTypeAdapter;
}
private static final class TransformedKeyMapTypeAdapter<K, V>
extends TypeAdapter<Map<K, V>> {
private final Function<? super K, String> toName;
private final Function<? super String, ? extends K> fromName;
private final Supplier<? extends Map<K, V>> createMap;
private final TypeAdapter<V> valueTypeAdapter;
private TransformedKeyMapTypeAdapter(final Function<? super K, String> toName, final Function<? super String, ? extends K> fromName,
final Supplier<? extends Map<K, V>> createMap, final TypeAdapter<V> valueTypeAdapter) {
this.toName = toName;
this.fromName = fromName;
this.createMap = createMap;
this.valueTypeAdapter = valueTypeAdapter;
}
private static <K, V> TypeAdapter<Map<K, V>> create(final Function<? super K, String> toName, final Function<? super String, ? extends K> fromName,
final Supplier<? extends Map<K, V>> createMap, final TypeAdapter<V> valueTypeAdapter) {
return new TransformedKeyMapTypeAdapter<>(toName, fromName, createMap, valueTypeAdapter)
.nullSafe();
}
@Override
@SuppressWarnings("resource")
public void write(final JsonWriter out, final Map<K, V> map)
throws IOException {
out.beginObject();
for ( final Map.Entry<K, V> e : map.entrySet() ) {
out.name(toName.apply(e.getKey()));
valueTypeAdapter.write(out, e.getValue());
}
out.endObject();
}
@Override
public Map<K, V> read(final JsonReader in)
throws IOException {
in.beginObject();
final Map<K, V> map = createMap.get();
while ( in.hasNext() ) {
final String name = in.nextName();
final K key = fromName.apply(name);
final V value = valueTypeAdapter.read(in);
final V replaced = map.put(key, value);
if ( replaced != null ) {
throw new JsonSyntaxException("duplicate key: " + key);
}
}
in.endObject();
return map;
}
}
} interface IKeyConverter {
@Nonnull
<K> String toName(@Nonnull Type type, @Nullable K key);
@Nullable
<K> K fromName(@Nonnull Type type, @Nonnull String name);
} final class KeyConverter
implements IKeyConverter {
private final BiFunction<? super Type, Object, String> toName;
private final BiFunction<? super Type, ? super String, Object> fromName;
private KeyConverter(final BiFunction<? super Type, Object, String> toName, final BiFunction<? super Type, ? super String, Object> fromName) {
this.toName = toName;
this.fromName = fromName;
}
public static IKeyConverter of(final BiFunction<? super Type, Object, String> toName,
final BiFunction<? super Type, ? super String, Object> fromName) {
return new KeyConverter(toName, fromName);
}
@Nonnull
@Override
public <K> String toName(@Nonnull final Type type, @Nullable final K key) {
return toName.apply(type, key);
}
@Nullable
@Override
public <K> K fromName(@Nonnull final Type type, @Nonnull final String name) {
@SuppressWarnings("unchecked")
final K key = (K) fromName.apply(type, name);
return key;
}
} final class KeyConverterBuilder {
@AllArgsConstructor(access = AccessLevel.PRIVATE)
private static final class Mapper<K> {
private final Function<? super K, String> toName;
private final Function<? super String, ? extends K> fromName;
}
private final Map<Class<?>, Mapper<?>> classMappers = new HashMap<>();
private final Map<Class<?>, Mapper<?>> subClassMappers = new HashMap<>();
private KeyConverterBuilder() {
}
static KeyConverterBuilder create() {
return new KeyConverterBuilder();
}
<K> KeyConverterBuilder forClass(final Class<K> clazz, final Function<? super K, String> toName, final Function<? super String, ? extends K> fromName) {
classMappers.put(clazz, new Mapper<>(toName, fromName));
return this;
}
<K> KeyConverterBuilder forSubClass(final Class<K> clazz, final Function<? super K, String> toName, final Function<? super String, ? extends K> fromName) {
subClassMappers.put(clazz, new Mapper<>(toName, fromName));
return this;
}
IKeyConverter build() {
return KeyConverter.of(
(type, key) -> toName(type, key, classMappers, subClassMappers),
(type, name) -> fromName(type, name, classMappers, subClassMappers)
);
}
private static <K> String toName(final Type type, final K key, final Map<Class<?>, Mapper<?>> classMappers,
final Map<Class<?>, Mapper<?>> subClassMappers) {
@Nullable
final Mapper<K> classMapper = resolveMapper(type, classMappers, subClassMappers);
if ( classMapper == null ) {
return String.valueOf(key);
}
return classMapper.toName.apply(key);
}
private static <K> K fromName(@Nonnull final Type type, @Nonnull final String name, final Map<Class<?>, Mapper<?>> classMappers,
final Map<Class<?>, Mapper<?>> subClassMappers) {
@Nullable
final Mapper<K> classMapper = resolveMapper(type, classMappers, subClassMappers);
if ( classMapper == null ) {
throw new IllegalArgumentException("Cannot convert " + name + " to " + type);
}
return classMapper.fromName.apply(name);
}
@Nullable
private static <K> Mapper<K> resolveMapper(final Type type, final Map<Class<?>, Mapper<?>> classMappers,
final Map<Class<?>, Mapper<?>> subClassMappers) {
// TODO cache
if ( type instanceof Class ) {
final Class<?> clazz = (Class<?>) type;
@Nullable
@SuppressWarnings("unchecked")
final Mapper<K> classMapper = (Mapper<K>) classMappers.get(type);
if ( classMapper != null ) {
return classMapper;
}
for ( final Map.Entry<Class<?>, Mapper<?>> e : subClassMappers.entrySet() ) {
if ( e.getKey().isAssignableFrom(clazz) ) {
@SuppressWarnings("unchecked")
final Mapper<K> subClassMapper = (Mapper<K>) e.getValue();
return subClassMapper;
}
}
}
return null;
}
} final class MapFactory {
private MapFactory() {
}
static <K, V> Map<K, V> create(final Type mapType) {
if ( mapType instanceof Class ) {
return create((Class<?>) mapType);
}
if ( mapType instanceof ParameterizedType ) {
final ParameterizedType mapParameterizedType = (ParameterizedType) mapType;
return create(mapParameterizedType.getRawType());
}
throw new IllegalArgumentException("Cannot resolve a default map instance for " + mapType);
}
private static <K, V> Map<K, V> create(final Class<?> mapClass) {
if ( LinkedHashMap.class.isAssignableFrom(mapClass) ) {
return new LinkedHashMap<>();
}
if ( HashMap.class.isAssignableFrom(mapClass) ) {
return new HashMap<>();
}
if ( TreeMap.class.isAssignableFrom(mapClass) ) {
return new TreeMap<>();
}
if ( Map.class.isAssignableFrom(mapClass) ) {
return new LinkedHashMap<>();
}
throw new IllegalArgumentException("Cannot create a default map instance for " + mapClass);
}
} @Data
private static final class Name {
private final String value;
}
private static final Type nameToNameMapType = new TypeToken<Map<Name, Name>>() {}.getType();
private static final Gson gson = new GsonBuilder()
.disableHtmlEscaping()
.registerTypeAdapterFactory(TransformedKeyMapTypeAdapterFactory.create(
KeyConverterBuilder.create()
.forClass(Name.class, name -> name.value, Name::new)
.build()
))
.create();
public static void main(final String... args) {
final Map<Name, Name> before = ImmutableMap.of(new Name("foo"), new Name("bar"));
System.out.println(before);
final String json = gson.toJson(before, nameToNameMapType);
System.out.println(json);
final Map<Name, Name> after = gson.fromJson(json, nameToNameMapType);
System.out.println(after);
System.out.println(before.equals(after));
}
|
Thanks for your help with this! I'm going to go ahead and resolve this issue. Seems like, at a philosophical level, it's believed this should be the responsibility of the library consumers and not the library itself. Cheers! |
When converting a Map to Json,
TypeAdapters
are not being used to generate the keys. Here is a minimal repro:I expect this to print:
But it actually calls the
toString
method and prints:I'm not familiar with your code base, but it looks like the problem is related to this line of code
The text was updated successfully, but these errors were encountered: