Summary: | Improve the extensibility of AspectJ weaver to support PointcutDoctor | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Tools] AspectJ | Reporter: | Linton Ye <lintonye> | ||||||||
Component: | Compiler | Assignee: | aspectj inbox <aspectj-inbox> | ||||||||
Status: | RESOLVED FIXED | QA Contact: | |||||||||
Severity: | enhancement | ||||||||||
Priority: | P2 | CC: | aclement, lintonye | ||||||||
Version: | unspecified | ||||||||||
Target Milestone: | 1.5.4 | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Whiteboard: | |||||||||||
Attachments: |
|
Description
Linton Ye
2007-06-18 02:29:53 EDT
Sounds like an aspect to me :-) have you started on this Linton? Created attachment 81273 [details]
Preliminary Implementation
Yes, I have started on this earlier. Please see the attached patch for a preliminary implementation. What I have done include: - Converted weaver project into an AspectJ project, in order to better modularize my change. - Added an aspect "CustomMungerExtension", which weaves into CrosscuttingMemberSet to enable the extension. It also has a flag to turn on/off the extension. - Added an interface "CustomMungerFactory", which can be implemented by extenders to create actual mungers. Now I am refactoring the code of PointcutDoctor to test the validity of this approach. I have planned to do the following hopefully this weekend and the following week: - Add custom shadow functionality into "CustomMungerExtension". This is to support the virtual join point shadow in PointcutDoctor. - Write tests for extensions added in weaver in weaver/testsrc - Adjust and write more tests for PointcutDoctor I would greatly appreciate your comments, Andy. Especially for my change of converting weaver to AspectJ project, I am not sure if it's an acceptable change but it does enable a nice encapsulation of the extension code. Please see the updated implementation in the attachment. There are some major changes in this patch: - Leave "weaver" as a Java project, rather than an AspectJ project in the preliminary implementation - According to the new interface of AspectJ compiler, associated a CustomMungerFactory with a AJCompiler. If a user wants to turn on this extension, he can just write an implementation of CustomMungerFactory interface and call AjCompiler.setCustomMungerFactory(CustomMungerFactory) when initializing their compiler. If a user needs to turn off this extension, call AjCompiler.setCustomMungerFactory(null). By default, the extension is off. I would greatly appreciate your comments and suggestions on this implementation. Hope this can be included into the next release of AspectJ. Thank you very much! Created attachment 83799 [details]
2nd Impl
thanks Linton - i will get to this, I promise. I'm just drowning in some complexities around moving to Java6 at the mo. A few comments: - i've removed the generics from your patch, we do not use Java5 internally at the moment. - i've removed use of the new for loop, same reason - could do with some javadoc on the interface you are adding - could do with a couple of test cases (which would also demonstrate how to use the new CustomMunger support). This would be in your interest to add since I may accidentally break it when refactoring the codebase if there are no tests for it. So I've committed your - but it would be nice if you at least did another patch containing javadoc. thanks! keyword: iplog (reminds me I have a non-committer patch in the codebase) Thanks Andy! I will write some unit tests and javadoc and submit a new patch soon. I noticed that you have checked in the changes for project "weaver". What about the other two projects "ajde.core" and "org.aspectj.ajdt.core"? Those changes allow PointcutDoctor set the CustomMungerFactory through a AjCompiler, since it seems the weaver is not directly visible to compiler users. Does this make sense? all changes in, my synchronize didnt behave itself so i missed committing the changes in the other projects. I had to remove your changes from ajde.core, the dependency that introduces from ajde.core to weaver causes chaos on the build machine. I need a clean build so will get back to this when i can and try and work out a better way to do it. I see. Would it be possible to expose the AjBuildManager in AjCompiler? That way I could call the setCustomMungerFactory method in AjBuildManager without requiring that dependency. something like: compiler.getAjBuildManager().setCustomMungerFactory(factory) One concern is that AjBuildManager is in an internal package though. Andy, I just noticed your recent change in cvs. But I need to access the instance of the factory assigned to the weaver. Another alternative is to change the signature of the method to: void AjCompiler.setCustomMungerFactory(Object) Object AjCompiler.getCustomMungerFactory() and void AjdeCoreBuildManager.setCustomMungerFactory(Object) Object AjdeCoreBuildManager.getCustomMungerFactory() Compared to previous approaches, which one works better? I thought about Object signatures but I dont like that either - but if you need the instance it may be what I do for now. I wish I had more time to think it through properly. hi Andy, I am attaching a new patch, which consists of a test case and some javadoc comments. I am not sure where to put the test class (and test data), so it is in tests/org.aspectj.systemtest.ajc154 for now. Please let me know if there is any problem. I appreciate your help! Created attachment 84711 [details]
Test case and javadoc
thanks Linton, all committed. I still haven't integrated the single test into the entire suite because when I tried that it failed. (It runs fine standalone). I dont think it plays nicely when run from a different project - possibly it is abusing the sandbox dir. iplog. (In reply to comment #17) > thanks Linton, all committed. I still haven't integrated the single test into > the entire suite because when I tried that it failed. (It runs fine > standalone). I dont think it plays nicely when run from a different project - > possibly it is abusing the sandbox dir. > > > > iplog. > I just got time to look at this. I didn't know that the test will be run from a different project. Would the following "less abusing" update work? It works on my machine at least. Could you give me some suggestions if it doesn't work well? <code> class CustomMungerExtensionTest { ... File oldSandBoxDir; protected void setUp() throws Exception { super.setUp(); oldSandBoxDir = sandboxDir; sandboxDir = new File("../tests"); } @Override protected void tearDown() throws Exception { super.tearDown(); sandboxDir = oldSandBoxDir; } } </code> |