Investigate switching from APT to compile time weaving (AOP)

[GoogleCodeExporter]

Annotation Processing is powerful, but it has a few limits :

- We can only generate new classes, we cannot modify existing ones (which is 
way devs must use Activity_ in their intents and in the Manifest).
- The programming model is hard to get, which leads to few core contributions
- Poor tooling. We are currently not able to debug the annotation processor. 
Users have to activate annotation processing in Eclipse manually, even when 
using Maven.

We should investigate switching to AspectJ, with a compile time weaver. This 
tool has been there for long, so there should be quite a good tooling. No more 
"_" in activity names. We could access private fields and methods.

Potential disadvantages :

- Compile time weaving modifies the bytecode. This means that understanding 
what AndroidAnnotations does, and reproducing / fixing bugs will be harder for 
the developers.
- Annotation Processing allows for compile time validation, adding compile time 
errors to the annotations when the user is not using them in a proper way. When 
a weaving doesn't match, it just doesn't match, and nothing else happen. Maybe 
we could keep APT for the annotation validation.
- We would have to rewrite the whole codebase. Not really a problem, let's be 
courageous ;-)  .

The ignition framework (previously called droid-fu) already does some aspectJ 
injection, in it's ignition-location module 
(https://github.com/kaeppler/ignition), we should probably have a look at how 
they do it.

Roadmap :

1) We need to make a proof of concept, a small Android Project that uses 
compile time weaving with some basic features such as @ViewById / @Click

2) We should check how easy it is to make this working as a library, with Maven 
AND without Maven. Everybody should be able to use it. We also need to 
investigate IDE support.

3) If everything's OK, we'll start an AndroidAnnotations 3 branch and work on 
reproducing the current features. No new features will be introduced in the 2.X 
branch. We should also adapt the functional tests, and even add some more.

@jzapata, @athomas, @bluepyth, let me know if you are interested in working on 
the POC ;-)

Original issue reported on code.google.com by py.ricau on 17 Oct 2011 at 7:34

[GoogleCodeExporter]

I tried AspectJ last week, just a few tests but something came out pretty fast 
: we won't get rid of APT at all. Because the aspects won't use reflexion at 
runtime, we need to generate the aspects' code at compile time an weave them 
with the user's code.

That's not unfortunate : we can probably reuse a lot of existing code !

For IDE integration we would probably have to develop our own Eclipse plugin.
- AJDT offers a way to do all the weaving for us;
- ADT offers the "Android packaging" for us;
- Androidannotations Dev Tool would have to link them all !

Hopefully it should be pretty easy to quickly have a working maven build, so 
these two issues (1/ and 2/ of the roadmap) can be addressed at the same time.

It's a lot of interesting work ! 
I'll try to have a working POC before next Thursday, so we can talk about it ;)

Original comment by Shuya....@gmail.com on 17 Oct 2011 at 8:08

[GoogleCodeExporter]

"we won't get rid of APT at all" : I don't understand that statement. APT ? 
Annotation Processing Tool ?  If we use AspectJ, we don't need to generate code 
with APT, do we ?

"we would probably have to develop our own Eclipse plugin" : Why is that, if 
all the plugins already exists and work together in a good fashion ? Creating 
Eclipse plugin is extra work ;-) .

Original comment by py.ricau on 17 Oct 2011 at 4:26

[GoogleCodeExporter]

If we don't generate code then the aspects will have to use reflection at 
runtime right ? And we don't want that, do we ? : \

I'm not sure AJDT and ADT actually work together, but I'm not well aware of how 
these things work so I guess it's something that needs to be checked out.

Original comment by jzap...@excilys.com on 17 Oct 2011 at 4:42

[GoogleCodeExporter]

The conclusion of all that is : let's try and implement ;-) . 

Original comment by py.ricau on 17 Oct 2011 at 10:00

[GoogleCodeExporter]

Hi everyone,
I am the guy who's working at the ignition-location module. I haven't used APT 
that much much I've been using AspectJ for some months now and I maybe can help 
on a couple of doubts you have. In response to comment 3: you can avoid using 
reflection in AspectJ by using context binding (more info here: 
http://stackoverflow.com/questions/7646097/what-is-aspectj-context-binding/). 
ADT and AJDT work well together, we are using them successfully in our prod 
environment.

Original comment by stefano....@gmail.com on 19 Oct 2011 at 9:15

[GoogleCodeExporter]

Thanks Stefano !!

We'll start investigating AspectJ this afternoon,  and write questions here if 
we get stuck.

Original comment by py.ricau on 19 Oct 2011 at 10:00

[GoogleCodeExporter]

Original comment by py.ricau on 20 Oct 2011 at 7:34

• Added labels: Milestone-

[GoogleCodeExporter]

I got it working with 2 projects :
- A JAR project containing compiled aspects
- An APK project containing a basic android app, whose code's weaved with the 
other project's aspects.

I'll start digging into AspectJ and context binding, thanks Stefano, and thanks 
to Daniel Williams who helped a lot for this first environment step !

Original comment by jzap...@excilys.com on 21 Oct 2011 at 3:09

[GoogleCodeExporter]

The current progress on the subject (in french) :
http://labs.excilys.com/2011/10/28/android-annotations-des-nouvelles-du-front/

Original comment by Shuya....@gmail.com on 27 Oct 2011 at 11:29

[GoogleCodeExporter]

Can you explain the issue with the OnClickListener? Just want to make sure I 
understood it properly cause my French used to be good but not any more :).

Original comment by stefano....@gmail.com on 28 Oct 2011 at 7:28

[GoogleCodeExporter]

Yes sure. :)
You see this AndroProjActivity with no onCreate() method overridden ?
To make the @Click work, we need to find the TextView after the onCreate, and 
add an OnClickListener on it.

But here's the problem : we can't define a JoinPoint anywhere in this method to 
add this Listener. In the constructor it's too early. And we can't try to 
define a JoinPoint on the onCreate() or setContentView() because it's not in 
the user code and our aspects won't be weaved with.

So the only solution is to add the onCreate() method in the user's code when 
the method is not already there, and define our JoinPoints on it. For this 
purpose we can use Inter-type declaration, but we need to know the class name 
at the time we write the aspect, and we cannot specifie a condition like "if 
the method doesn't exist already". 

That's why the conclusion is : we can't use hand-written aspects, we need to 
generate the aspects regarding to the user's code. And it solves a lot of 
reflection problems !

Original comment by jzap...@excilys.com on 28 Oct 2011 at 8:27

[GoogleCodeExporter]

Not sure if I understand correctly here: "And we can't try to define a 
JoinPoint on the onCreate() or setContentView() because it's not in the user 
code and our aspects won't be weaved with." and if that's what you want/need. 
Anyway it's true that you cannot define a JointPoint on onCreate()'s *call* 
because this callback is not invoked in your code but you can define it on its 
*execution*, i.e.: after() : execution(* onCreate(..)). 

Another solution would be to add the Android jar in the InPath of the ajc, but 
I have to play with this solution a bit more because for me it didn't work that 
well.

Original comment by stefano....@gmail.com on 28 Oct 2011 at 9:14

[GoogleCodeExporter]

execution(* onCreate(..)) doesn't work if the user doesn't override the 
onCreate() method, because in this case it's executed in the Activity.class, 
which is not woven with the aspect.

I'm not very well aware of the specifics of ajc, but in my point of view, at 
the end we're providing an APK which is then deployed on a device, and executed 
on a VM that already contains the Android framework. 

If we want to weave the code of Android, we would have to provide the woven 
Android code in the APK, and that seems insane. I know there is load-time 
weaving, but we can't control the Android classes loading. What am I missing ?

Original comment by Shuya....@gmail.com on 28 Oct 2011 at 9:36

[GoogleCodeExporter]

True, but how likely is it that onCreate() is not overridden? Or you could just 
require onCreate() to be overridden in order for the lib to work(ignition 
location requires that for onCreate(), onResume() and onPause()). I think it's 
not that much hassle considering all the 'magic' that the lib does, don't you 
think?

Regarding the second approach: the lib jar should must be in the Aspect Path 
and the Android jar in the InPath of the app. But again, not 100% if it will 
work correctly. Definitely you don't have to provide the woven Android code :).

Original comment by stefano....@gmail.com on 28 Oct 2011 at 9:56

[GoogleCodeExporter]

I've been using AndroidAnnotations a lot in one of my projects and it's really 
common not to override the onCreate() method. It makes the code more 
light-weight because if you override onCreate() you'll probably call 
super.onCreate(); too.

But, even if we can bind an aspect to this method, we'll need a list of 
annotated attributes to inject, and that's not acceptable since it would be 
based on reflection. So we need to generate the aspects anyway.

If you happen to successfully bind an aspect in Android code, keep me posted, 
I'm very curious about that ! And sceptic, I admit ! :P

(I just noticed I'm logged in with the bad address once out of two. 
Shuya.inc@gmail.com == jzapata@excilys.com)

Original comment by jzap...@excilys.com on 28 Oct 2011 at 2:37

[GoogleCodeExporter]

Well...my previous comment clearly shows that I haven't use AndroidAnnotations 
that much! :)

Original comment by stefano....@gmail.com on 2 Nov 2011 at 4:42

[GoogleCodeExporter]

"aspectj" branch created with everything ready-to-go for development. 

In this branch, androidannotations has been forked to 
androidannotations-aspect, but not deleted, so that we'll be able to merge it 
with master if we want to, in the future.

A project called androidannotation-aspectj-showcase has been created, already 
configured for AndroidAnnotations and AspectJ. (AndroidAnnotations processing 
is temporarily turned off to avoid conflict with aspects, but it's working.)

We won't use the aspectj specific langage, we'll use the annotation based style 
(http://bit.ly/voQrvq), for two reasons:
1- with annotation oriented aspect, user will not need to install AJDT
2- we don't have anything to generate the aspectj specific code, though we 
already have experience with codemodel to generate Java code. Pointcut 
definitions will have to be generated as raw strings, unfortunately.

Here is what it looks like: http://goo.gl/0DFSW
And the logcat entries at runtime :

Activity: Will now call init()
The Aspect: Called method on package name 
com.googlecode.androidannotations.aspectj.showcase
The Aspect: Called method init(..) on class ShowcaseActivity on package name 
com.googlecode.androidannotations.aspectj.showcase
Activity: in init()

Next steps: define the bounds of what we want to reproduce with aspects for 
this first POC, create a specific Aspect that makes them work for a specific 
Activity, and then turn this aspect into a generated Aspect and test, test, 
test.

Original comment by jzap...@excilys.com on 31 Dec 2011 at 2:28

[GoogleCodeExporter]

(You should ignore the @EActivity and @AfterViews annotations in the test 
Activity.)

Original comment by jzap...@excilys.com on 31 Dec 2011 at 2:31

[GoogleCodeExporter]

About Comment 11 and Comment 13 :

AspectJ inter-type declaration does *not* allow us to force the target Activity 
to override the onCreate() method as I though it would. It's not the purpose of 
inter-type declarations, and even if I'm not sure how it works, I'm pretty sure 
now, after a lot of testing, that we can't use it this way.

It means the user would always have to put something like this in his 
Activities :

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
}

Without that, we don't have a Join Point and we can't create one.

Another solution is to ask the user to extends a subclass of Activity of our 
own :

@EActivity(R.layout.main)
public class ShowcaseActivity extends EnhancedActivity { ... }

These two solutions seem too intrusive to me. The former is ugly, the latter 
could be restrictive for other frameworks. (RoboGuice for example, even if we 
*do* support this one without using RoboActivity).

Piwai, I need your opinion on this point. Can we afford forcing the user to 
extend one of our class in their Activity ? Or, can we afford forcing them 
override onCreate() in each activity ? If both these questions receive a "no", 
we can close the issue. :p

Original comment by jzap...@excilys.com on 1 Jan 2012 at 8:48

[GoogleCodeExporter]

I don't get your point.

If you go the "generate aspect" way, then you can do whatever you want. Which 
means, do something like that :

- When generating an aspect for an activity, check if the classes has the 
overriden onCreate Method.

=> if yes, create an aspect that weaves the bytecode of the onCreate method
=> if no, create an aspect that adds an onCreate method (is that feasible?)

Original comment by py.ricau on 2 Jan 2012 at 7:29

[GoogleCodeExporter]

My point is precisely that I think it's not feasible, like I said : "AspectJ 
inter-type declaration does *not* allow us to force the target Activity to 
override the onCreate() method as I though it would".

So the question is "What then ?".

Original comment by jzap...@excilys.com on 2 Jan 2012 at 8:33

[GoogleCodeExporter]

This issue is closed, aspects were supposed to be even less intrusive, but for 
the reasons above, it's not worth it.

Original comment by jzap...@excilys.com on 3 Jan 2012 at 7:25

• Changed state: Done