COM3205 Homework 4 Paul Freeman October 22, 2002 Part 1: Testability - It is very easy to do unit testing as our application is written in AspectJ - taking full advantage of the ability to seperate concerns. Each part of the Class form of the LoD has been separated nicely into it's own aspect. Simply recompiling with different aspectj ".lst" files that include only the part of the app (or portion of the Class form of LoD) that you wish to test, allows you to run each part of the app over the same test. One small improvement was required to improve the testability. I was unable to run the tests with only the checker, i.e. no gathering portions of the application. This is due to the fact that the LoDPointcutsContext's stack of hashsets is referenced in the Checker aspect, instead of a static HashSet, as was done with LoDPointCutsGlobal. To fix this, I modified LoDPointcutsContext to include a static HashSet that could be empty, allowing us to be able to test the Checker aspect by itself. During testing, I noticed a few problems with the application: 1. In the Any class, the Execution pointcut did not include constructors so the following change was made: // CORRECTED CODE - the Execution pointcut did not include constructor executions pointcut Execution(): scope() && (execution(* *(..)) || ConstructorExecution()); // any execution // scope() && execution(* *(..)); // any execution pointcut ConstructorExecution(): scope() && execution(*.new(..)); // END CORRECTED CODE 2. In the Arguments Aspect, code was added to include the "this/self" object. Also since the Any.Execution pointcut was modified to include constructor calls, the cast on the signature had to be changed from MethodSignature to CodeSignature. // CORRECTED CODE - arguments did not include the this/self class // also change in Any.Execution to include constructors // required change in cast from MethodSignature to CodeSignature Class[] methArgs = ((CodeSignature)jsp.getSignature()).getParameterTypes(); Vector allArgs = new Vector(); allArgs.addAll(Arrays.asList(methArgs)); allArgs.add( jsp.getSignature().getDeclaringType() ); // add this/self return (Class[])allArgs.toArray(methArgs); // return ((MethodSignature)jsp.getSignature()).getParameterTypes(); // END CORRECTED CODE 3. In the Checker aspect, the target type being searched for was the "declaring type" of the method. The declaring type of the method could be a super class if the method was inherited, not the actual "object" the method is being called upon. Even though we are testing the Class form of the LoD, I would argue that we need to test the object target of the method call as we are testing that the call to the method of the object is legal, not the call to the object that declared the method. Therefore the following change was made - note that targets that are null are static targets, therefore the call is a global method and assumed to be OK: // CORRECTED CODE - was examining declaring type of the method and not target // object. Declaring type of superclass methods is the super // class itself, so was looking for wrong Class in some instances. // Corrected code also allows calls to static methods. Object targetObj = thisJoinPoint.getTarget(); if(targetObj == null) return; // static method call // else Class targetType = thisJoinPoint.getTarget().getClass(); // Class targetType = thisJoinPointStaticPart.getSignature().getDeclaringType(); // END CORRRECTED CODE Uncorrectable Imperfections: The only imperfection that I found, is a case involving ReturnValues: // fail Object - fail Class D d = (new A()).newD(); d.foo(); // pass d = newD(); d.foo(); // fail Object - Pass Class!? d = (new A()).newD(); d.foo(); You will notice in the above code that the first call to d.foo is invalid. This could be considered either incorrect or correct depending on your point of view, however the point of view you choose determines the validity of the 3rd call of d.foo(). Either the 1st call should be valid because the class contains a method that returns a class type of D, making the 3rd call also valid. Or the first call is invalid as the object being tested is not the direct result of a call to a LOCAL method returning a class type of D, making the 3rd call invalid as well. I lean towards the former, interpreting the class form of the LoD as being one that allows calls to objects of a type equal to any local method return type. My argument is that if the latter was chosen, the return type portion of the class form of the LoD would be no different that the object form. Unfortunately there is no way to statically gather all of the return types of local methods in a class prior to method calls using AspectJ (see part1_code_solution.zip for working code and tests. To test: checker only - use None.lst Arguments only - use ArgumentsOnly.lst Direct Parts only - use DirectPartsOnly.lst Locally Constructed classes only - use LocallyConstructed.lst ReturnValues only - use ReturnValues.lst All Parts - user part1.lst) Part 2: 2.1 Implement the remaining two rules: locally constructed objects and objects returned from local calls are both preferred supplier objects. To accomplish this, I noticed that the target hashmap stacking behavior of the ArgumentBin aspect would be used in the soon to be created LocallyConstructedBin and ReturnValueBin aspects. So I abstracted this behavior to a super class called TargetBinStack. I noticed in doing so that super class advice always executes after subclass advice. So it was not enough to only abstract stacking behavior. I also had to abstract gathering behavior. The gathering in the subclasses was then performed in the implementation of two abstract methods rather than advice. This allowed the gathering advice to be placed within the superclass where it could be ordered appropriately. Finally, I abstracted the bin printing operations to the super class as well. 2.2 Also implement what we have left out so far: a data member access is like a local method call. This was easily accomplished by adding another pointcut to the Any class describing all "get" join points. The Checker aspect now was simply required to check all MethodCalls as well as all Gets. The current application was already able to verify if the target of a get is valid so no further modifications were needed. 2.3 Collect the following statistics: What is the percentage of the calls where there are two or more reasons why an object is a preferred supplier object. This was accomplished by extending the Checker Aspect. Checking was made as robust as possible, and in doing so the testability of the application was improved. All Bin objects are stored in a vector called Suppliers: // get the global "Bin" objects suppliers.add( ArgumentBin.aspectOf() ); suppliers.add( LocallyConstructedBin.aspectOf() ); suppliers.add( ReturnValueBin.aspectOf() ); To verify if a target is valid, each of the suppliers in the vector has it's "contains" method called. Testing can now easily be performed on the individual LoD aspects (ArgumentBin, etc.) by simply commenting out any of the lines above. Logging was added using the Log4J library available from: http://jakarta.apache.org/ To implement the logging, no other classes were modified. A logging aspect was simply added which catches print statements that occur within the lawOfDemeter package. The argument to the print statement captured is redirected to the logging statement. This makes it quite simple to choose between encorporating logging, or not encorporating logging. Simply choose between compiling with the logging aspect or without. (see part2_code_solution.zip for working code) To run the application without logging compile using: noLogging.lst To run the application with the logging attatched, compile with: part2.lst add the 'log4j-1.2.7.jar' file located in the 'lib' directory in your classpath and finally call the java command from the directory where the file 'LoDLogger.lcf' is located. Part 3. Develop a method to allow exceptions for the following: 1. classes whose static members return preferred supplier objects (global members). 2. classes whose public methods may always be called. Both exceptions were handled with a series of pointcuts added to the Any class. // denotes all the join points to check in the code being examined pointcut ToCheck(): (Any.MethodCall() || Any.Get()) && !Any.ToAvoid(); // denotes classes to avoid checking pointcut ToAvoid(): AllStaticAccesses() && Stable(); // NOTE: With the following pointcuts, to nullify the pointcut, i.e. // have it pick out no join points, use: // Nothing() // as the sole designation // denotes all globally preferred static members // NOTE: scope() must always remain and be &&'ed with any further // pointcuts added. Multiple pointcuts added should be placed in parenteses // and separated with ||, i.e. scope() && (pointcut1() || pointcut2()) pointcut GlobalPreferred(): scope() && AllStaticAccesses(); // denotes stable classes calls to which should not be checked; // multiple pointcuts should be separated with || pointcut Stable(): AllJavaPackageAccesses() || call(* testcase..*(..)) || get(testcase..* *); // // what follows is a series of usefull predefined pointcuts // // denotes the opposite of all classes // !within(*) can be read as not within any package. // It can be used to represent the empty set private pointcut Nothing(): !within(*); //denotes all statically accessed immediate parts and methods private pointcut AllStaticAccesses(): scope() && (call(static * *.*(..)) || get(static * *)); private pointcut AllJavaPackageAccesses(): scope() && (call(* java..*(..)) || get(java..* *)); The ToAvoid() pointcut is used by ToCheck pointcut which is in turn used by the Checker aspect: before(): Any.ToCheck(); ToAvoid uses the pointcuts AllStaticAccesses() and Stable(). An interesting side effect of this design is that it is now possible to allow/disallow calls to global methods/instance variables by simply including or excluding the pointcut AllStaticAccesses() from ToAvoid(). There is no longer a need to provide code in the Checker aspect which denotes if(target == null) method is static. GlobalPreferred() is used in it's own bin called GlobalPreferredBin to gather global preferred objects. pointcut EventToGather(): Any.GlobalPreferred(); It can denote all static method calls using the pointcut named AllStaticAccesses(). Or it can include the empty set using the pointcut named Nothing(). It can further be modified to include any packages/classes you wish to denote as GlobalPreffered if the user defines and adds their own pointcuts. It must be noted that the GlobalPreferred pointcut itself is required to include the scope pointcut so that calls within the lawOfDemeter package are not captured. Stable() can denote the empty set using the predifined pointcut Nothing(), or it can be used to denote method calls and/or instance variable gets for a particular package, class, and or method name. A predifined pointcut named AllJavaPackageAccesses() shows how this can be done: private pointcut AllJavaPackageAccesses(): scope() && (call(* java..*(..)) || get(java..* *)); The log file included with the solution (LoDChecker_solution.log) shows the testcases in the 'testcase' package run with the following pointcut values: - Session 1 - pointcut ToAvoid(): !Stable(); pointcut GlobalPreferred(): scope() && Nothing(); pointcut Stable(): Nothing(); - Session 2 - pointcut ToAvoid(): !Stable(); pointcut GlobalPreferred(): scope() && AllStaticAccesses(); pointcut Stable(): Nothing(); - Session 3 - pointcut ToAvoid(): !Stable() && !AllStaticAccesses(); pointcut GlobalPreferred(): scope() && Nothing(); pointcut Stable(): Nothing(); - Session 4 - pointcut ToAvoid(): !Stable() && !AllStaticAccesses(); pointcut GlobalPreferred(): scope() && AllStaticAccesses(); pointcut Stable(): Nothing(); - Session 5 - pointcut ToAvoid(): !Stable() && !AllStaticAccesses(); pointcut GlobalPreferred(): scope() && Nothing(); pointcut Stable(): AllJavaPackageAccesses(); - Session 6 - avoids all testable calls pointcut ToAvoid(): !Stable() && !AllStaticAccesses(); pointcut GlobalPreferred(): scope() && AllStaticAccesses(); pointcut Stable(): AllJavaPackageAccesses() || call(* testcase..*(..)) || get(testcase..* *); - Session 7 - pointcut ToAvoid(): !Stable() && !(call(static * *.Foo.*(..)) || get(static *.Foo.* *)); pointcut GlobalPreferred(): scope() && Nothing(); pointcut Stable(): AllJavaPackageAccesses() ; - Session 8 - pointcut ToAvoid(): !Stable() && !(call(static * *.Main.*(..)) || get(static *.Main.* *)); pointcut GlobalPreferred(): scope() && Nothing(); pointcut Stable(): AllJavaPackageAccesses() ; - Session 9 - pointcut ToAvoid(): !Stable() && !(call(static * testcase..*(..)) || get(static testcase..* *)); pointcut GlobalPreferred(): scope() && Nothing(); pointcut Stable(): AllJavaPackageAccesses() ; (see part3_code_solution.zip for working code) To run the application without logging compile using: noLogging.lst To run the application with the logging attatched, compile with: part3.lst add the 'log4j-1.2.7.jar' file located in the 'lib' directory in your classpath and finally call the java command from the directory where the file 'LoDLogger.lcf' is located. 3.2 Run with larger application... The user does have quite a bit of control over what to include and exclude. That said, it can be tricky to name the pointcuts correctly as AspectJ's type patterns and pointcut designations can be quite confusing at times. However they are very powerfull and will give the user quite a bit of control over what can be excluded from examination. Perhaps the best feature is the ability to exclude broad designations such as is described by the pointcuts AllStaticAccesses() and AllJavaPackageAccesses() above. ************************************************************************************ Addendum: ************************************************************************************ The design for Part 3 was modified to improve the separation of concerns. The improved design allows for Unit testing of the various portions of the LoDChecker application. Changes can be noticed by comparing the code for Part 2 with the code for Part 3. What was done: A small modification was made to some of the pointcuts in the Any class. A ToCheck pointcut was added so that it could be used in some of the other changes mentioned below: // denotes all the join points to check in the code being examined pointcut ToCheck(): AnyDynamic.ToCheck; Also, the dynamic pointcuts, i.e. pointcuts to be changed by the user have now been moved to their own class called AnyDynamic. This facilitates quick identification of the pointcuts that can be modified by the user, and it makes it quite simple to rewrite this simple class prior to a build based on options selected by a user through a user interface. Major Changes: There was an undesirable dependency between Checker and all of the implimentations of Supplier, namely that the code in Checker was hardwired with the various Supplier implimentations that were being used. It would be ideal if the checker aspect was only aware of the supplier class itself, and did not care what implimentations of supplier exist. It would also be ideal if future implimentations of Supplier could also be added without modifying any existing code in the program. The solution provided accomplishes just this. First, an abstract method was added to Supplier: abstract public Supplier getSupplier(Object thisObj); Each aspect that impliments Supplier must provide an implimentation of the get supplier method which returns a Supplier based on the thisObj supplied if necessary. Then a static Vector was added to Supplier to hold all of the implimentations of Supplier being used in the program. This vector is populated through a small bit of advice: /** * Advice that gathers supplier objects at a join point before that * join point is checked in the checker aspect. */ before(): Any.ToCheck(){ Supplier s = getSupplier(thisJoinPoint.getThis()); if(s != null){ suppliers.add(s); } } This advice will execute once for every aspect that impliments supplier, adding that instance of Supplier to the vector of suppliers before the advice in the Checker aspect is executed. The Supplier aspect also needed to be modified so that it declared that it "dominates Checker" to ensure the advice above executed before the advice in Checker. Notice that there is no code here to erase the vector of suppliers prior to adding an instance of Supplier in the advice, i.e. before(): Any.ToCheck(){ suppliers = new Vector(); //***** reset suppliers Supplier s = getSupplier(thisJoinPoint.getThis()); if(s != null){ suppliers.add(s); } } Unfortunately a reset of the supplier vector cannot be placed here because each implimenting aspect would reinitialize the vector before it added itself to the vector, resulting in suppliers containing at most one Supplier instance and possible no Supplier instances. To accomplish this correctly, what we really need is a piece of Static advice will execute only once, reinitializing the supplier vector prior to the execution of the advice above. Since AspectJ does not allow advice to be declared as static, we need to create another small concrete aspect that provides this functionality: privileged aspect ConcreteSupplier dominates Supplier { // initialize the suppliers vector before(): Any.ToCheck() { Supplier.suppliers = new Vector(); } } Finally, as is good coding practice, a get method was added to Supplier to return the current Vector of available suppliers. protected static Vector getSuppliers(){ return suppliers; } Some other small modifications were made to Supplier and TargetBinStack. The public String contains() method was moved up from TargetBinStack into Supplier and a printTargets() method was also added to supplier - this method makes use of the existing printHashMap method to printed a more nicely formatted display. TargetBinStack, now overrides the printTargets method and also provides another method called printAllTargets. All the printing methods are only used for debugging purposes. The other major modification that was made was the abstraction of the statistic gathering portion of the application out of Checker and into a separate aspect called StatMonitor. Checker now only does what you would expect, it checks. However it was modified slightly to provide an event hook to any Aspects that wish to monitor the checking. Two methods were added along with two pointcuts: /** * Prints a violation noitice using the thisJoinPoint argument. * Also provides a method that can be captured using the included pointcut * Checker.Violation(). * * @param thisJoinPoint */ pointcut Violation(JoinPoint tjp): execution(private void Checker.raiseViolation(JoinPoint)) && args(tjp); private void raiseViolation(JoinPoint thisJoinPoint){ System.out.println("!! LoD violation: "+thisJoinPoint+JPUtil.at(thisJoinPoint)); } /** * Prints a success noitice using the thisJoinPoint argument. * Also provides a method that can be captured using the included pointcut * Checker.Success(). * * @param thisJoinPoint * @param binsIn */ pointcut Success(JoinPoint tjp, Vector binsIn): execution(private void Checker.raiseSuccess(JoinPoint, Vector)) && args(tjp, binsIn); private void raiseSuccess(JoinPoint thisJoinPoint, Vector binsIn){ // used for debugging // if(binsIn.size() > 1){ // System.out.println("Call OK ("+thisJoinPoint+")"+JPUtil.at(thisJoinPoint)+ // " target found in " + binsIn); // } } The StatMonitor aspect now simply uses the pointcuts provided by checker to analyze the statistics of the program. The new pieces of advice are included below, all the other statistic code that was in checker has been moved unchanged to the StatMonitor aspect and so will not be shown below: public aspect StatMonitor { // statistcs variables initialized before every main method execution private long totalChecks; private long numViolations; private Vector validChecks; // initialize statistical variables before(): Any.MainMethodExecution(){ totalChecks = 0; numViolations = 0; validChecks = new Vector(); validChecks.add(new Long(0)); // set the total valid counts = 0 } // increase the total count of checks before(): Any.ToCheck(){ totalChecks++; } // increase the violation count before(JoinPoint tjp): Checker.Violation(tjp){ numViolations++; } // increase the correct count before(JoinPoint tjp, Vector binsIn): Checker.Success(tjp, binsIn) { addToStats(binsIn.size()); } ... } A few get functions were also added to provide access to the current counts during program execution. Bug Fixes: A few bug fixes were also implimented. One major bug noticed by Pengcheng Wu was in the use of perthis() in the AspectDeclaration of the ImmediatePartBin. It turns out that not only does AspectJ associate an aspect with the "this" object of the join point described in the perthis statement, all of the advice in the corresponding aspect instance only executes if and only if the advice uses a join point where the aspect is associated with the "this" object. This is similar for the pertarget aspect declaration keyword. The difference can be seen through the following example. Suppose we have: aspect A perthis(Any.ConstructorExecution()){ before(): Any.MethodCall(){ System.out.println("Method Call"); } } class Foo { public void foo(){ System.out.println("foo");} public static void main(String[] args){ Foo f = new Foo(); f.foo(); } } You might expect the output to be: Method Call // call to main Method Call // call of f.foo foo Method Call // call to println However, there is no aspect A associated with the null "this" object at the point of the call to main, so method call will not be printed. There is also no aspect A associated with the null "this" object at the point f.foo() in the main method. This is because the currently executing objects in both cases are null, i.e. this == null. So your output will be only: foo Method Call // call to println If you change the perthis to pertarget in A however, you will still not get the first method call to print as there is no aspect A associated with the null target object at the call to main. However, the "Method Call" statement will print at the line f.foo() since there is an instance of Aspect A associated with the target object f of type Foo at that join point. So the complete output will be: Method Call // call of f.foo foo Method Call // call to println Another bug in the ImmediatePartBin was in the adding of the arguments to the set join point. Set join points may or may not occur in the object in which the member variable exists. For example, assuming: aspect A pertarget(Any.ConstructorExecution()){ before(): Any.MethodCall(){ System.out.println("Method Call"); } } class Foo { Bar b; public void foo(){ System.out.println("foo");} public static void main(String[] args){ Foo f = new Foo(); f.foo(); f.b = new Bar(); } } class Bar{ } The "this" object at the line f.b = new Bar() above is actually null since it occurs in a static method. However, the target object at that point is the correct object, i.e. the Foo object whose immediate part b we would like to capture the set of. So the actuall addition of the new Bar() object should be made to the target of the set, not necessarily the object associated with the Aspect. Although the previous reasoning about pertarget would make this seem unnecessary, it is better to be safe than sorry so the advice was changed to: before(): // direct parts Any.Set() { // There is only one argument to a set join point. // Becomes a preferred supplier and is therefore added to targets. ImmediatePartBin.aspectOf(thisJoinPoint.getTarget()).addAll(thisJoinPoint.getArgs(), direct+at(thisJoinPoint)); } A modification was also made to the add method in supplier, disallowing the addition of any null objects to the targets hashmap. And finally, a bug was fixed in the original statistic gathering code when it was moved to the StatMonitor aspect. Bug Fix 11/18/2002 - Modified the advice in the TargetBinStack. The ArgumentBin, which extended the TargetBinStack, used identitcal join points in the EventToGather pointcut and the EventToStack pointcut. The TargetBinStack used around advice with both pointcuts, and it seems that the around advice would not function correctly when both pieces of advice used the same pointcut. Splitting the EventToStack around advice into before and after advice fixed this problem. I'm not sure exactly what was happening. It seemed as though the Pop portion of the EventToStack around advice would not always execute. KNOWN ISSUES: Objects are stored in the hashmap based on their toString() value, an invalid target with the same value as a valid target will not raise a violation -- FIXED 11/18/2002 Fixed by replacing HashMap with IdentityHashMap made available in jsdk 1.4. Old immediate part object values are never removed from the hashmap after they have been replaced with a new value. -- FIXED 11/18/2002 Fixed by adding a second IdentityHashMap to the ImmediatePartBin that objects referenced by their variable name. There is no strong form implimentation in this Object form checker.