-
Notifications
You must be signed in to change notification settings - Fork 662
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
Add support of handling custom scalar types to OperationRequest #165
Add support of handling custom scalar types to OperationRequest #165
Conversation
… defined custom scalar type adapters
@@ -21,11 +27,26 @@ | |||
public final class ApolloConverterFactory extends Converter.Factory { | |||
private final Map<Type, ResponseFieldMapper> responseFieldMappers; | |||
private final Map<ScalarType, CustomTypeAdapter> customTypeAdapters; | |||
private final Moshi moshi; |
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.
We use moshi to serialize query request that can have variables (with scalar or input object types).
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.
I'm not crazy about having a dependency on moshi. I still feel like gson is the most popular converter on android. Having both including in classpath is extraneous methods. Maybe we can make a convert factory type setup similar to what retrofit does?
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.
Moshi lib is tiny: 457 methods only, we can eventually remove it and generate own serialization (the same as we did with ResponseReader), but for now to save time lets leave.
As for Gson, we could allow user to define own lib for serialization request but then user must handle custom scalar type serialization in request by himself. So in this case he is going to have 2 places where define scalar type conversion: first for response (in com.apollographql.android.converter.pojo.CustomTypeAdapter
), second for Gson.
In current implementation he defines these conversion in one place for both request / response:
CustomTypeAdapter<Date> dateCustomTypeAdapter = new CustomTypeAdapter<Date>() {
@Override public Date decode(String value) {
try {
return DATE_FORMAT.parse(value);
} catch (ParseException e) {
throw new RuntimeException(e);
}
}
@Override public String encode(Date value) {
return DATE_FORMAT.format(value);
}
};
....
new ApolloConverterFactory.Builder()
.withCustomTypeAdapter(CustomType.DATETIME, dateCustomTypeAdapter)
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.
I'm not saying to generate parsing. I'd like to be open to having a ConverterFacotory which can be a gson or moshi factory. Apollo client should be agnostic to how the parsing is done but instead just delegate to a user defined parser factory.
This PR is part of #161
Adds support of user defined scalar type adapters to OperationRequest serialization.
Internally converter uses moshi to serialize OperationRequest object (query text and variables).
Next final step of #161 is renaming :apollo-converters:pojo to -> :apollo-converter
Closes #144