-
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
Support serializing anonymous and local class with custom adapter #2498
Support serializing anonymous and local class with custom adapter #2498
Conversation
} | ||
|
||
if (isAnonymousOrNonStaticLocal(field.getType())) { | ||
if (excludeClass(field.getType(), serialize)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this to avoid code duplication. This should be identical to the previous behavior, but previously excludeClass
was called separately:
gson/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java
Lines 80 to 82 in ff401f1
private boolean includeField(Field f, boolean serialize) { | |
return !excluder.excludeClass(f.getType(), serialize) && !excluder.excludeField(f, serialize); | |
} |
(and it looks like there are no other callers of excludeField
)
assertThat(EnumSet.allOf(Roshambo.class)).isEqualTo( | ||
gson.fromJson("[\"123ROCK\",\"123PAPER\",\"123SCISSORS\"]", new TypeToken<Set<Roshambo>>() {}.getType()) | ||
); | ||
Set<Roshambo> deserialized = gson.fromJson("[\"123ROCK\",\"123PAPER\",\"123SCISSORS\"]", new TypeToken<Set<Roshambo>>() {}.getType()); | ||
assertThat(deserialized).isEqualTo(EnumSet.allOf(Roshambo.class)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this because previously "actual" and "expected" were switched.
@eamonnmcmanus, I hope that is what you had in mind. But, in case you are not planning to do so anyway, could you please run this against the Google projects to see if there are any unintended side-effects? |
…tion # Conflicts: # gson/src/main/java/com/google/gson/internal/Excluder.java # gson/src/test/java/com/google/gson/functional/EnumTest.java # gson/src/test/java/com/google/gson/functional/ObjectTest.java
915ce56
to
fdc1a76
Compare
Sorry about the delay. I've run this against Google's tests and everything passed, except a couple of tests that were explicitly testing for the current behaviour. I doubt that that's critical, but will investigate further. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for doing this!
…ogle#2498) * Support serializing anonymous and local class with custom adapter * Fix formatting and fix switched 'expected' and 'actual' in EnumTest * Minor code improvements --------- Co-authored-by: Éamonn McManus <emcmanus@google.com>
Purpose
Resolves #1510
(but only for the cases where a custom adapter exists for the class; not if the reflection-based adapter would be used)
Description
When serializing an anonymous or local class for which a custom adapter exists (i.e. not the reflection-based adapter), it will now use that adapter instead of serializing
null
.Deserialization is still not supported, even if a custom adapter exists, because some custom adapters like the built-in
Collection
adapter might fall back to using JDK Unsafe to create an instance, and that might lead to runtime exceptions then due to uninitialized fields. Also, for deserialization this issue most likely can only occur for local classes, and can be worked around by making the class non-local.Future topics:
(And are there possibly cases where a field is compiler-generated but not marked as synthetic?)
(The main issue here is making sure JDK Unsafe is not used to create an instance of such a class, to avoid ending up with uninitialized fields.)
I am not planning to submit any pull requests for these future topics yet. It might be better if we first see which of these features the users actually need, and if these use cases are common.
Checklist
null
@since $next-version$
(
$next-version$
is a special placeholder which is automatically replaced during release)TestCase
)mvn clean verify javadoc:jar
passes without errors