Make Response object return optional if such config selected #331
Conversation
sav007
requested review from
brianPlummer,
BenSchwab,
digitalbuddha and
marwanad
| ResponseFieldMapper<? extends Operation.Data> responseFieldMapper(); | ||
| ResponseFieldMapper<D> responseFieldMapper(); | ||
|
|
||
| T wrapData(D data); |
There was a problem hiding this comment.
will be called by ResponseConverter
| @@ -44,12 +44,17 @@ public String queryDocument() { | |||
| } | |||
|
|
|||
| @Override | |||
| public Optional<TestQuery.Data> wrapData(TestQuery.Data data) { | |||
| return Optional.fromNullable(data); | |||
There was a problem hiding this comment.
Example how data will be wrapped with optional
| @@ -55,7 +55,7 @@ | |||
| } | |||
| } | |||
| jsonReader.endObject(); | |||
| return new Response<>(operation, data, errors); | |||
| return new Response<>(operation, operation.wrapData(data), errors); | |||
There was a problem hiding this comment.
this call will wrap data
|
Do we have any tests that test what happens when Edit: looks like we do: https://github.com/apollographql/apollo-android/pull/331/files#diff-37977119f5d5bf9f9b3285afc785edd3R53 |
There was a problem hiding this comment.
This looks good to me --
just to double check, the reason we need to add a new method to every model instead of just adding a boolean wrap field, and a generic runtime wrap utility method is that depending on the presence of libraries (like guava) the wrap code might access an optional implementation that not all user's have a library for?
|
Yes, that correct. We can have 2 Optional implementations: guava or our own shadowed version. So boolean field is not enough. Plus our Runtime lib knows nothing about guava. So only if user uses Guava we will wrap data (via method |
|
@digitalbuddha @marwanad any comments? |
|
Not from first glance need to look tonight
…On Mar 17, 2017 1:38 PM, "Ivan Savytskyi" ***@***.***> wrote:
@digitalbuddha <https://github.com/digitalbuddha> @marwanad
<https://github.com/marwanad> any comments?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#331 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEUX3LeLWtkyxaSgbPDiXeZM5Z7bxMxIks5rmsUsgaJpZM4Mf5Tp>
.
|
Closes #290
Address issue with
Responsedata field doesn't return data wrapped with optional.Take a look at https://github.com/apollographql/apollo-android/pull/331/files#diff-bd1f7dd25c2f308ad46f434f16893f11 and https://github.com/apollographql/apollo-android/pull/331/files#diff-1d4349ba6464dd54d277f125b3253316 as example how it will be used.