Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

allow method injection #204#1457

Merged
WonderCsabo merged 40 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:204_method_injectionCopy head branch name to clipboard
Dec 20, 2015
Merged

allow method injection #204#1457
WonderCsabo merged 40 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:204_method_injectionCopy head branch name to clipboard

Conversation

@dodgex

@dodgex dodgex commented Jun 7, 2015

Copy link
Copy Markdown
Member

this is the WIP branch for method injection. in its initial implementation only for @Bean but planned to be expanded to most (if not all) injection related annotations.

@dodgex

dodgex commented Jun 7, 2015

Copy link
Copy Markdown
Member Author

related to #204

@WonderCsabo

WonderCsabo commented Sep 12, 2015

Copy link
Copy Markdown
Member

Nice start! Can you please rebase this onto develop?

@dodgex

dodgex commented Sep 12, 2015

Copy link
Copy Markdown
Member Author

rebased

yDelouis
yDelouis reviewed Sep 19, 2015
View reviewed changes
Comment thread ...droidannotations/src/main/java/org/androidannotations/internal/core/handler/BeanHandler.java Outdated

@yDelouis yDelouis Sep 19, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the method name.

@dodgex dodgex Sep 20, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo fixed

@yDelouis

yDelouis commented Sep 19, 2015

Copy link
Copy Markdown
Contributor

Seems good. However, if you want to extend this to all injection annotation, I think we will need to create a helper in order not to have to duplicate this code structure in all Handlers.

@dodgex

dodgex commented Sep 20, 2015

Copy link
Copy Markdown
Member Author

i started this as PoC to see what is possible. but yeah, how that we found that it is possible the plan is to extend it to (if possible) all injections

@dodgex

dodgex commented Sep 20, 2015

Copy link
Copy Markdown
Member Author

@WonderCsabo @yDelouis i updated the PR. i refactored the code into a helper class and added method injection for @SystemService.

feedback is highly appreciated!

WonderCsabo
WonderCsabo reviewed Sep 21, 2015
View reviewed changes
Comment thread .../androidannotations-test/src/main/java/org/androidannotations/test/ActivityWithServices.java

@WonderCsabo WonderCsabo Sep 21, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields should be private.

@dodgex dodgex Sep 21, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should they? in other tests similar fields are default too. thats why i left the visibiliy.

@WonderCsabo WonderCsabo Sep 21, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. The fields are packapege private because we have to access them from the test class.

I just said it because we want method injections to be able to inject to private fields (at least this is on of the reasons). But we still have to access the fields in the test class, so just forget my comment.

@dodgex dodgex Sep 21, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just realized that there is no consistency at all in the test classes. the BeanInjctedActivity has all fields public

@WonderCsabo WonderCsabo Sep 21, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields should be pp, but it is not a big deal.

@WonderCsabo

WonderCsabo commented Sep 21, 2015

Copy link
Copy Markdown
Member

I am wondering, could it be easier to create a common super AnnotationHandler class instead of the InjectHelper for method injecting annotations?

@WonderCsabo

WonderCsabo commented Sep 21, 2015

Copy link
Copy Markdown
Member

This should be rebased.

@dodgex

dodgex commented Sep 21, 2015

Copy link
Copy Markdown
Member Author

rebased an adressed some of the comments

@dodgex

dodgex commented Sep 21, 2015

Copy link
Copy Markdown
Member Author

about the common super class.

first the question is what injections should be supported? e.g. @Pref has CoreBaseAnnotationHandler as parent class but the @Bean and @SystemService have BaseAnnotationHandler

not sure what the purpose of CoreBaseAnnotationHandler beside the fact that has a CoreValidationHelper. if it is ok we could use this as a common base or better extend it for that base.

@dodgex

dodgex commented Sep 22, 2015

Copy link
Copy Markdown
Member Author

here is a list of annotations that ca be applied to fields. the checked are already supported.

  • @App
  • @Bean
  • @Extra
  • @FragmentArg
  • @FragmentById
  • @FragmentByTag
  • @HttpsClient
  • @InjectMenu
  • @OptionsMenuItem
  • @PreferenceByKey
  • @AnimationRes
  • @BooleanRes
  • @ColorRes
  • @ColorStateListRes
  • @DimensionPixelOffsetRes
  • @DimensionPixelSizeRes
  • @DimensionRes
  • @DrawableRes
  • @HtmlRes
  • @IntArrayRes
  • @IntegerRes
  • @LayoutRes
  • @MovieRes
  • @StringArrayRes
  • @StringRes
  • @TextArrayRes
  • @TextRes
  • @RootContext
  • @Pref
  • @SystemService
  • @ViewById
  • @ViewsById

🚫 @InstanceState
🚫 @FromHtml
🚫 @NonConfigurationInstance

The last annotations are not related to injection. :)

@WonderCsabo

WonderCsabo commented Sep 22, 2015

Copy link
Copy Markdown
Member

I think all listed annotations are valid. And do not forget annotations from plugins (@RestClient).
@yDelouis do you agree?

@dodgex

dodgex commented Sep 23, 2015

Copy link
Copy Markdown
Member Author

so i need to find/create a common level of BaseAnnotationHandler where i can implement the method injection and it also needs to be available for plugins? :)

@WonderCsabo

WonderCsabo commented Sep 23, 2015 via email

Copy link
Copy Markdown
Member

@dodgex

dodgex commented Sep 26, 2015

Copy link
Copy Markdown
Member Author

currently broken, but that should be fixable with #1566

@yDelouis

yDelouis commented Sep 28, 2015

Copy link
Copy Markdown
Contributor

I'm okay with the listed annotations. If we missed some, we could add them later.
The helper and the parent class are not incompatible. The parent class could use the helper so if we want a handler to extend another class, we can also add this feature to it.

@WonderCsabo

WonderCsabo commented Oct 10, 2015

Copy link
Copy Markdown
Member

@dodgex this is no longer blocked.

@dodgex

dodgex commented Oct 11, 2015

Copy link
Copy Markdown
Member Author

@WonderCsabo i know, i already rebased after the blocker PR was merged. but i had no time yet to continue work on this.

@dodgex

dodgex commented Oct 11, 2015

Copy link
Copy Markdown
Member Author

@ViewById and @ViewsById depend of the release jcodemodel 2.7.12 see phax/jcodemodel#32. i have added 4 TODO notes for the update to 2.7.12 to remove some workarounds i added to have to code compiling right now.

@dodgex

dodgex commented Oct 11, 2015

Copy link
Copy Markdown
Member Author

same for @PreferenceByKey

@WonderCsabo

WonderCsabo commented Oct 11, 2015

Copy link
Copy Markdown
Member

Some annotations are missing from plugins (OrmLiteDao, RestService,? )

@dodgex

dodgex commented Oct 11, 2015

Copy link
Copy Markdown
Member Author

arg... yeah. i think i have not checked the plugins when generating the above list of annotations todo.

@WonderCsabo

WonderCsabo commented Oct 11, 2015

Copy link
Copy Markdown
Member

Are you planning to add those to this PR as well?

@dodgex

dodgex commented Oct 11, 2015

Copy link
Copy Markdown
Member Author

yeah. if it is ok.

dodgex and others added 15 commits December 20, 2015 11:30
@dodgex
add test for method injected fragments
9d796cd
@dodgex
allow to inject views into @ViewById annotated methods
cade871
@dodgex
add test for method injected views
bbda8a1
@dodgex
allow to inject lists of views into @ViewsById annotated methods
161f128
@dodgex
add test for method injected lists of views
4e1ba9d
@dodgex
allow to inject preferences into @PreferenceByKey annotated methods
27ea853
@dodgex
add test for method injected preferences
9121bfc
@dodgex
allow to inject ormlite daos into @OrmLiteDao annotated methods
7d0c59e
@dodgex
add test for method injected ormlite daos
a3256f4
@dodgex
allow to inject rest service into @restservice annotated methods
40b6f7d
@dodgex
add test for method injected rest services
4396b44
@dodgex
update javadocs for method injection
c7d182d
@dodgex
generate only one method in Fragment-/IntentBuilder for methods with …
a2f3bfd 
…mutltiple injections
@dodgex
use the view field for FoundViewHolder
1c2575c
@dodgex
use the preference field for FoundPreferenceHolder
03bf6bb
WonderCsabo added a commit that referenced this pull request Dec 20, 2015
@WonderCsabo
Merge pull request #1457 from dodgex/204_method_injection
ab53a8b 
Allow method injection
@WonderCsabo WonderCsabo merged commit ab53a8b into androidannotations:develop Dec 20, 2015
@WonderCsabo WonderCsabo added this to the 4.0 milestone Dec 20, 2015
@dodgex dodgex deleted the 204_method_injection branch December 20, 2015 12:30
@WonderCsabo

WonderCsabo commented Dec 30, 2015

Copy link
Copy Markdown
Member

@dodgex can you update the wiki sometime?

@dodgex

dodgex commented Jan 4, 2016

Copy link
Copy Markdown
Member Author

I'll try to do it the next few days.

@dodgex

dodgex commented Jan 9, 2016

Copy link
Copy Markdown
Member Author

Checklist for Wiki Updates:

  • @App
  • @Bean
  • @Extra
  • @FragmentArg
  • @FragmentById
  • @FragmentByTag
  • @HttpsClient skipped
  • @InjectMenu
  • @OptionsMenuItem
  • @PreferenceByKey
  • @AnimationRes
  • @BooleanRes
  • @ColorRes
  • @ColorStateListRes
  • @DimensionPixelOffsetRes
  • @DimensionPixelSizeRes
  • @DimensionRes
  • @DrawableRes
  • @HtmlRes
  • @IntArrayRes
  • @IntegerRes
  • @LayoutRes
  • @MovieRes
  • @StringArrayRes
  • @StringRes
  • @TextArrayRes
  • @TextRes
  • @RootContext
  • @Pref
  • @SystemService
  • @ViewById
  • @ViewsById

@dodgex

dodgex commented Jan 9, 2016

Copy link
Copy Markdown
Member Author

wiki on my fork is updated. also i have added the commit 6ba5e68 that updates the year in FAQ to 2016. :)

@dodgex

dodgex commented Jan 9, 2016

Copy link
Copy Markdown
Member Author

added @RestService and @OrmLiteDao

@dodgex

dodgex commented Jan 15, 2016

Copy link
Copy Markdown
Member Author

@WonderCsabo I assume you missed that i updated the wiki?

@WonderCsabo

WonderCsabo commented Jan 15, 2016 via email

Copy link
Copy Markdown
Member

@dodgex

dodgex commented Jan 15, 2016

Copy link
Copy Markdown
Member Author

no problem

@WonderCsabo

WonderCsabo commented Jan 15, 2016

Copy link
Copy Markdown
Member

I merged your wiki changes. Thanks again for your great work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@dodgex @WonderCsabo @yDelouis @ened
Morty Proxy This is a proxified and sanitized view of the page, visit original site.