test and fix for 131505: keep aspectnames in the state object

8 files changed, 153 insertions, 6 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
index 9d6d726ec..b08298b0a 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
@@ -25,7 +25,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -132,7 +131,6 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
 	// FIXME asc should this really be in here?
 	private IHierarchy structureModel;
 	public AjBuildConfig buildConfig;
-	private List aspectNames = new LinkedList();
 	
 	AjState state = new AjState(this);
     
@@ -535,9 +533,11 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
 		PrintStream ps = new PrintStream(baos);
 		ps.println("<aspectj>");
 		ps.println("<aspects>");
-		for (Iterator i = aspectNames.iterator(); i.hasNext();) {
-			String name = (String)i.next();
-			ps.println("<aspect name=\"" + name + "\"/>");
+		if (state.getAspectNames() != null) {
+			for (Iterator i = state.getAspectNames().iterator(); i.hasNext();) {
+				String name = (String)i.next();
+				ps.println("<aspect name=\"" + name + "\"/>");
+			}			
 		}
 		ps.println("</aspects>");
 		ps.println("</aspectj>");
@@ -951,7 +951,12 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
 				ResolvedType type = world.resolve(name);
 //				System.err.println("? writeAspectName() type=" + type);
 				if (type.isAspect()) {
-					aspectNames.add(name);
+					if (state.getAspectNames() == null) {
+						state.initializeAspectNamesList();
+					}
+					if (!state.getAspectNames().contains(name)) {
+						state.getAspectNames().add(name);
+					}
 				}
 			}
 		};
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java
index e437b5e03..2a9a721ec 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java
@@ -161,6 +161,7 @@ public class AjState {
 	
 	private List/*File*/ compiledSourceFiles = new ArrayList();
 	private List/*String*/ resources = new ArrayList();
+	private List/*String*/ aspectNames;
 	
 	private ArrayList/*<String>*/ qualifiedStrings;
 	private ArrayList/*<String>*/ simpleStrings;
@@ -1200,4 +1201,12 @@ public class AjState {
 		buildManager.state = null;
 		buildManager.setStructureModel(null);
 	}
+	
+	public List getAspectNames() {
+		return aspectNames;
+	}
+	
+	public void initializeAspectNamesList() {
+		this.aspectNames = new LinkedList();
+	}
 }
diff --git a/tests/multiIncremental/PR131505/base/C.java b/tests/multiIncremental/PR131505/base/C.java
new file mode 100644
index 000000000..935a67ad9
--- /dev/null
+++ b/tests/multiIncremental/PR131505/base/C.java
@@ -0,0 +1,8 @@
+public class C {
+
+	public void method1() {	
+	}
+	
+	public void method2() {
+	}
+}
diff --git a/tests/multiIncremental/PR131505/inc1/A.aj b/tests/multiIncremental/PR131505/inc1/A.aj
new file mode 100644
index 000000000..b3ad1799c
--- /dev/null
+++ b/tests/multiIncremental/PR131505/inc1/A.aj
@@ -0,0 +1,8 @@
+public aspect A {
+
+	pointcut p() : execution(public void C.method1(..));
+	
+	before() : p() {
+	}
+	
+}
diff --git a/tests/multiIncremental/PR131505/inc2/C.java b/tests/multiIncremental/PR131505/inc2/C.java
new file mode 100644
index 000000000..5029a04f0
--- /dev/null
+++ b/tests/multiIncremental/PR131505/inc2/C.java
@@ -0,0 +1,11 @@
+public class C {
+
+	public void method1() {	
+	}
+	
+	public void method2() {
+	}
+	
+	public void method3() {
+	}
+}
diff --git a/tests/multiIncremental/PR131505/inc3/A1.aj b/tests/multiIncremental/PR131505/inc3/A1.aj
new file mode 100644
index 000000000..19b7c1284
--- /dev/null
+++ b/tests/multiIncremental/PR131505/inc3/A1.aj
@@ -0,0 +1,7 @@
+public aspect A1 {
+	
+	pointcut p() : execution(public void C.method2(..));
+	
+	before() : p() {
+	}
+}
diff --git a/tests/multiIncremental/PR131505/inc4/pkg/A.aj b/tests/multiIncremental/PR131505/inc4/pkg/A.aj
new file mode 100644
index 000000000..2b3c0aaad
--- /dev/null
+++ b/tests/multiIncremental/PR131505/inc4/pkg/A.aj
@@ -0,0 +1,10 @@
+package pkg;
+
+public aspect A {
+
+	pointcut p() : execution(public void C.method2(..));
+	
+	before() : p() {
+	}
+	
+}
diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
index f307df72b..7e936c80c 100644
--- a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
+++ b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
@@ -11,9 +11,11 @@
 * ******************************************************************/
 package org.aspectj.systemtest.incremental.tools;
 
+import java.io.BufferedReader;
 import java.io.DataOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
+import java.io.FileReader;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -687,6 +689,54 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 		configureBuildStructureModel(false);
 	}
 	
+	public void testPr131505() {
+		configureNonStandardCompileOptions("-outxml");
+		initialiseProject("PR131505");
+		build("PR131505");
+		checkWasFullBuild();
+		// aop.xml file shouldn't contain any aspects
+		checkXMLAspectCount("PR131505","",0);
+		// add a new aspect A which should be included in the aop.xml file
+		alter("PR131505","inc1");
+		build("PR131505");
+		checkWasFullBuild();
+		checkXMLAspectCount("PR131505","",1);
+		checkXMLAspectCount("PR131505","A",1);
+		// make changes to the class file which shouldn't affect the contents
+		// of the aop.xml file
+		alter("PR131505","inc2");
+		build("PR131505");
+		checkWasntFullBuild();
+		checkXMLAspectCount("PR131505","",1);
+		checkXMLAspectCount("PR131505","A",1);		
+		// add another new aspect A1 which should also be included in the aop.xml file
+		// ...there should be no duplicate entries in the file
+		alter("PR131505","inc3");
+		build("PR131505");
+		checkWasFullBuild();
+		checkXMLAspectCount("PR131505","",2);
+		checkXMLAspectCount("PR131505","A1",1);
+		checkXMLAspectCount("PR131505","A",1);
+		// delete aspect A1 which meanss that aop.xml file should only contain A
+		File a1 = new File(getWorkingDir().getAbsolutePath() 
+				+ File.separatorChar + "PR131505" + File.separatorChar + "A1.aj");
+		a1.delete();
+		build("PR131505");
+		checkWasFullBuild();
+		checkXMLAspectCount("PR131505","",1);
+		checkXMLAspectCount("PR131505","A1",0);
+		checkXMLAspectCount("PR131505","A",1);	
+		// add another aspect called A which is in a different package, both A
+		// and pkg.A should be included in the aop.xml file
+		alter("PR131505","inc4");
+		build("PR131505");
+		checkWasFullBuild();
+		checkXMLAspectCount("PR131505","",2);
+		checkXMLAspectCount("PR131505","A",1);
+		checkXMLAspectCount("PR131505","pkg.A",1);
+	}
+	
+	
 	// other possible tests:
 	// - memory usage (freemem calls?)
 	// - relationship map
@@ -848,4 +898,43 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 	private static void log(String msg) {
 		if (VERBOSE) System.out.println(msg);
 	}
+	
+	/**
+	 * Count the number of times a specified aspectName appears in the default
+	 * aop.xml file and compare with the expected number of occurrences. If just 
+	 * want to count the number of aspects mentioned within the file then 
+	 * pass "" for the aspectName, otherwise, specify the name of the 
+	 * aspect interested in.
+	 */
+	private void checkXMLAspectCount(String projectName, String aspectName, int expectedOccurrences) {
+		int aspectCount = 0;
+		File aopXML = new File(getWorkingDir().getAbsolutePath() 
+				+ File.separatorChar + projectName + File.separatorChar 
+				+ "bin" + File.separatorChar + "META-INF" + File.separatorChar + "aop.xml");
+
+		if (!aopXML.exists()) {
+			fail("Expected file " + aopXML.getAbsolutePath() + " to exist but it doesn't");
+		}
+		try {
+			BufferedReader reader = new BufferedReader(new FileReader(aopXML));
+			String line = reader.readLine();
+			while (line != null) {
+				if (aspectName.equals("") && line.indexOf("aspect name=\"") != -1) {
+					aspectCount++;
+				} else if (line.indexOf("aspect name=\""+aspectName+"\"") != -1) {
+					aspectCount++;
+				}
+				line = reader.readLine();
+			}
+			reader.close();
+		} catch (IOException ie) {
+			ie.printStackTrace();
+		}
+		if (aspectCount != expectedOccurrences) {
+			fail("Expected aspect " + aspectName + " to appear " + expectedOccurrences + " times" +
+					" in the aop.xml file but found " + aspectCount + " occurrences");
+		}
+	}
+	
+	
 }