Added parceler `@Extra` and `@InstanceState` support by johncarl81 · Pull Request #1559 · androidannotations/androidannotations

johncarl81 · 2015-09-19T06:32:39Z

This PR offers Parceler integration which wraps and unwraps beans annotated with @Parcel transparently when they are used in @Extra or @InstanceState aspects. This is a rework of this previous PR: #977

dodgex · 2015-09-19T06:37:23Z

...androidannotations-test/src/main/java/org/androidannotations/test/parceler/ParcelerBean.java

no newline at end of file

Oops.. done.

dodgex · 2015-09-19T06:42:17Z

...roidannotations/src/main/java/org/androidannotations/internal/core/helper/IntentBuilder.java

you already added this method in AnnotationHelper any chance to use that one?

I certainly could, but I wasn't sure how to get ahold of AnnotationHelper other than making isAnnotatedWith static. What do you suggest?

I'd pass the AnnotationHelper to the intent builder.

Add it to the constructor, then update the ActivityIntentBuilder and ServiceIntentBuilder to constructors and thier instantiations. where they are instantiated there is annotationHelper field from parent class available.

but i think you should do that in a separate commit before doing the work that adds the parceler support

Got it... done.

I think you still forgot to remove the method from here.

It got missed in the rebase.. should be removed now.

WonderCsabo · 2015-09-19T10:58:02Z

...ations-core/androidannotations/src/main/java/org/androidannotations/helper/BundleHelper.java

Parcels.wrap/unwrap method can handle List<AnnotatedWithParcel> AFAIK, or even more. We should also allow those.

Parceler can also handle boxed primitives in collections:
List<Integer>, Map<Integer, String>
A mix:
Map<String, AnnotatedWithParcel>
And nested collections:
Map<List<String>, List<AnnotatedWithParcel>>
Would you like these cases handled as well?

If you can add it, it would be great. However we should also check that we have Parceler on the classpath (when the user adds a List of primitives for examples, we cannot be sure because we are not checking any annotation).

I've been going with the assumption that if you annotate your bean with @Parcel that would mean Parceler is on the classpath. Perhaps we should just support collections with supported types and at least one bean annotated with @Parcel... would that be acceptable instead of checking explicitly that Parceler is on the classpath?

@johncarl81 it is very easy to check it is on the classpath: elementUtils.getTypeElement("org.parceler.Parcel") != null. We use this type of check for lots of things throughout AA.

Ok, that will work nicely, I'll move forward with that then.

I'm finding that matching types following the critieria we discussed may be a bit overzelous. For instance, this is matching the ArrayList<String> listExtra in ExtraInjectedActivity. Should we be serializing this through Parceler?

No, i think we should only serialize those objects through Parceler which are not in plain Android.

Agreed... I think I have a solution, pushing shortly.

WonderCsabo · 2015-09-19T11:02:50Z

Sorry, but i am not sure the refactor with passing the AnnotationHelper instance around is needed. You could just create a new AnnotationHelper in the intent builder constructor.

annotationHelper = new AnnotationHelper(environment);

yDelouis · 2015-09-19T17:01:57Z

Could you please update the javadoc of the annotations @Extra, @FragmentArg and @SavedInstanceState to specify that these annotations are compatible with classes annotated with @Parceler ?

yDelouis · 2015-09-19T17:04:59Z

And I think you forgot to handle inner Extra annotations like @OnActivityResult.Extra. You have to modify the class ExtraParameterHandler.

WonderCsabo · 2015-09-19T21:47:51Z

@yDelouis Those extra annotations are already handled by the BundleHelper.

yDelouis · 2015-09-20T11:55:25Z

@WonderCsabo, you're right, sorry.

WonderCsabo · 2015-09-20T21:13:35Z

...ns-core/androidannotations/src/main/java/org/androidannotations/helper/AnnotationHelper.java

Can you move all Parceler-related things to another helper class (which can extend AnnotationHelper)?

I would move PARCEL_ANNOTATION to CanonicalNameConstants and use that constant everywhere.

WonderCsabo · 2015-09-20T22:06:41Z

This is OK, however i realized METHOD_SUFFIX_BY_TYPE_NAME is outdated and we also do not support SparceArray<T extends Parcelable>. We have to fix that later.

johncarl81 · 2015-09-29T01:05:33Z

Ok, let me know if you like that language.

By the way, if you guys would like great looking javadocs with less ceremony I'd recommend Asciidoclet

WonderCsabo · 2015-09-29T07:12:03Z

We also have to add info for:

OnActivityResult.Extra
Receiver.Extra
ReceiverAction.Extra
ServiceAction.Extra (actually we already miss parameter type info on that :( )

johncarl81 · 2015-09-29T14:12:01Z

I don't see ServiceAction.Extra...

WonderCsabo · 2015-09-29T14:16:29Z

Sorry. Just ServiceAction.

johncarl81 · 2015-09-29T23:53:04Z

Alright, I think I got them all and rebased against develop... let me know if there's anything else.

johncarl81 · 2015-10-06T14:46:13Z

Ping

WonderCsabo · 2015-10-06T14:52:57Z

Sorry for not adding feedback, i am very busy at work :(

I think this can be merged. @yDelouis ?

yDelouis · 2015-10-06T15:08:01Z

I think so ! Thanks !

Added parceler `@Extra` and `@InstanceState` support

johncarl81 · 2015-10-06T16:35:26Z

Fantastic!

Thanks @WonderCsabo for your help on this effort, both on this PR and your attention to Eclipse support in Parceler. Without your diligence this feature would have never reached this point.

WonderCsabo · 2015-10-07T09:25:54Z

@johncarl81 i hope i could help, and thank you for implementing this in AA, it was not a quick task.

@yDelouis we have to update the wiki. I think we can add this information in the third party integration section. Is that OK?

WonderCsabo · 2015-10-09T18:56:54Z

@johncarl81 i created a wiki page for Parceler integration, i hope everything is correct.

johncarl81 · 2015-10-09T19:32:43Z

Looks spot on @WonderCsabo.

WonderCsabo · 2015-10-09T19:41:57Z

Great, thanks for checking.

WonderCsabo · 2015-10-14T20:56:42Z

@johncarl81 FYI i tried this in Gradle, and the JCodeModel#ref method throwed an NoClassDefFoundError about android.os.Parcelable when you are referencing the Parcels utility class. So i changed to use JCodeModel#directClass instead 1234280.

johncarl81 · 2015-10-14T20:58:39Z

Ah, good catch.
On Oct 14, 2015 2:56 PM, "Csaba Kozák" notifications@github.com wrote:

@johncarl81 https://github.com/johncarl81 FYI i tried this in Gradle,
and the JCodeModel#ref method throwed an NoClassDefFoundError about
android.os.Parcelable when you are referencing the Parcels utility class.
So i changed to use JCodeModel#directClass instead 1234280
1234280
.

—
Reply to this email directly or view it on GitHub
#1559 (comment)
.

dodgex reviewed Sep 19, 2015
View reviewed changes

johncarl81 force-pushed the parceler branch 2 times, most recently from 8c4d13e to 4849b9c Compare September 19, 2015 06:40

dodgex reviewed Sep 19, 2015
View reviewed changes

johncarl81 force-pushed the parceler branch from 4849b9c to 384f4d9 Compare September 19, 2015 07:07

WonderCsabo reviewed Sep 19, 2015
View reviewed changes

johncarl81 force-pushed the parceler branch 2 times, most recently from 1a3c81a to fdce23e Compare September 19, 2015 15:54

johncarl81 force-pushed the parceler branch 3 times, most recently from 190d955 to 3dba0f3 Compare September 20, 2015 17:13

WonderCsabo reviewed Sep 20, 2015
View reviewed changes

johncarl81 force-pushed the parceler branch 5 times, most recently from f208bb5 to b12377d Compare September 20, 2015 22:35

johncarl81 force-pushed the parceler branch from 51d3ead to 09117a1 Compare September 29, 2015 14:12

johncarl81 force-pushed the parceler branch from 58520e6 to 40d19d8 Compare September 29, 2015 23:51

johncarl81 added 4 commits September 29, 2015 17:52

Added AnnotationHelper instance to IntentBuilder

1a6d0d3

Added parceler @Extra and @InstanceState support

8cdcd70

Refactored Parceler specific helper and some other minor updates.

34204c2

Added comments relating to Parceler usage

5df3858

johncarl81 force-pushed the parceler branch from 40d19d8 to 5df3858 Compare September 29, 2015 23:52

yDelouis added a commit that referenced this pull request Oct 6, 2015

Merge pull request #1559 from johncarl81/parceler

7cd7718

Added parceler `@Extra` and `@InstanceState` support

yDelouis merged commit 7cd7718 into androidannotations:develop Oct 6, 2015

yDelouis added this to the 4.0 milestone Oct 6, 2015

yDelouis mentioned this pull request Oct 6, 2015

@Parcelable #301

Closed

johncarl81 deleted the parceler branch October 6, 2015 16:21

WonderCsabo added the WikiUpdateNeeded label Oct 7, 2015

WonderCsabo mentioned this pull request Oct 7, 2015

Support new types in bundles #1577

Closed

WonderCsabo removed the WikiUpdateNeeded label Oct 9, 2015

Conversation

johncarl81 commented Sep 19, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WonderCsabo commented Sep 19, 2015

Uh oh!

yDelouis commented Sep 19, 2015

Uh oh!

yDelouis commented Sep 19, 2015

Uh oh!

WonderCsabo commented Sep 19, 2015

Uh oh!

yDelouis commented Sep 20, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WonderCsabo commented Sep 20, 2015

Uh oh!

johncarl81 commented Sep 29, 2015

Uh oh!

WonderCsabo commented Sep 29, 2015

Uh oh!

johncarl81 commented Sep 29, 2015

Uh oh!

WonderCsabo commented Sep 29, 2015 via email

Uh oh!

johncarl81 commented Sep 29, 2015

Uh oh!

johncarl81 commented Oct 6, 2015

Uh oh!

WonderCsabo commented Oct 6, 2015

Uh oh!

yDelouis commented Oct 6, 2015

Uh oh!

johncarl81 commented Oct 6, 2015

Uh oh!

WonderCsabo commented Oct 7, 2015

Uh oh!

WonderCsabo commented Oct 9, 2015

Uh oh!

johncarl81 commented Oct 9, 2015