Bug 406518 - migrate OT/Equinox to the standard OSGi WeavingHook
- record which teams have been activated, avoid double activation
- protect against missing 'class' attribute in aspectBinding extension
- cleanup: WaitingTeamRecord only operates on Class never on instance
diff --git a/plugins/org.eclipse.objectteams.otequinox/.settings/org.eclipse.jdt.core.prefs b/plugins/org.eclipse.objectteams.otequinox/.settings/org.eclipse.jdt.core.prefs
index 2e0f716..718115f 100644
--- a/plugins/org.eclipse.objectteams.otequinox/.settings/org.eclipse.jdt.core.prefs
+++ b/plugins/org.eclipse.objectteams.otequinox/.settings/org.eclipse.jdt.core.prefs
@@ -31,7 +31,7 @@
 org.eclipse.jdt.core.compiler.problem.finallyBlockNotCompletingNormally=warning
 org.eclipse.jdt.core.compiler.problem.forbiddenReference=error
 org.eclipse.jdt.core.compiler.problem.hiddenCatchBlock=warning
-org.eclipse.jdt.core.compiler.problem.includeNullInfoFromAsserts=disabled
+org.eclipse.jdt.core.compiler.problem.includeNullInfoFromAsserts=enabled
 org.eclipse.jdt.core.compiler.problem.incompatibleNonInheritedInterfaceMethod=warning
 org.eclipse.jdt.core.compiler.problem.incompleteEnumSwitch=warning
 org.eclipse.jdt.core.compiler.problem.indirectStaticAccess=ignore
@@ -67,7 +67,7 @@
 org.eclipse.jdt.core.compiler.problem.reportMethodCanBeStatic=ignore
 org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
 org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=warning
-org.eclipse.jdt.core.compiler.problem.suppressOptionalErrors=disabled
+org.eclipse.jdt.core.compiler.problem.suppressOptionalErrors=enabled
 org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
 org.eclipse.jdt.core.compiler.problem.syntacticNullAnalysisForFields=disabled
 org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=ignore
diff --git a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBinding.java b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBinding.java
index b3988d6..f0fa490 100644
--- a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBinding.java
+++ b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBinding.java
@@ -50,6 +50,7 @@
 	public String[]         teamClasses;
 	public ActivationKind[] activations; 
 	public List<String>[]   subTeamClasses;
+	public boolean[]		 isActivated;
 
 	public State 			 state = State.Initial;
 	
@@ -69,6 +70,7 @@
 		this.teamClasses    = new String[count];
 		this.subTeamClasses = new List[count]; // new List<String>[count] is illegal!
 		this.activations    = new ActivationKind[count];
+		this.isActivated    = new boolean[count]; 
 	}
 	
 	public void setActivation(int i, @Nullable String specifier) {
@@ -132,7 +134,8 @@
 			try {
 				teamName = scanner.readOTAttributes(bundle, teamName);
 				Collection<String> baseClassNames = scanner.getCollectedBaseClassNames();
-				addBaseClassNames(teamName, baseClassNames);
+				if (!basesPerTeam.containsKey(teamName))
+					addBaseClassNames(teamName, baseClassNames);
 				log(IStatus.INFO, "Scanned team class "+teamName+", found "+baseClassNames.size()+" base classes");
 			} catch (Exception e) {
 				log(e, "Failed to scan team class "+teamName);
@@ -140,4 +143,20 @@
 		}
 		this.state = State.TeamsScanned;
 	}
+
+	public void markAsActivated(String teamName) {
+		for (int i = 0; i < this.teamClasses.length; i++) {
+			if (this.teamClasses[i].equals(teamName)) {
+				this.isActivated[i] = true;
+				return;
+			}
+		}
+	}
+	
+	public boolean isActivated(String teamName) {
+		for (int i = 0; i < this.teamClasses.length; i++)
+			if (this.teamClasses[i].equals(teamName))
+				return this.isActivated[i];
+		return false;
+	}
 }
\ No newline at end of file
diff --git a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBindingRegistry.java b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBindingRegistry.java
index 5070380..b276b73 100644
--- a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBindingRegistry.java
+++ b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/AspectBindingRegistry.java
@@ -114,19 +114,22 @@
 				continue;
 			
 			IConfigurationElement[] teams = currentBindingConfig.getChildren(TEAM);
+			int teamCount = teams.length;
+			for (int j = 0; j < teams.length; j++) if (teams[j].getAttribute(CLASS) == null) teamCount --;
 			AspectBinding binding = new AspectBinding(aspectBundleId,
 														baseBundleId,
 														basePlugins[0].getChildren(Constants.FORCED_EXPORTS_ELEMENT), 
-														teams.length);
+														teamCount);
 			// TODO(SH): maybe enforce that every bundle id is given only once?
 
 			//teams:
 			try {
-				for (int j = 0; j < teams.length; j++) {
+				for (int j = 0, count = 0; count < teamCount; j++) {
 					String teamClass = teams[j].getAttribute(CLASS);
-					binding.teamClasses[j] = teamClass;
+					if (teamClass == null) continue;
+					binding.teamClasses[count] = teamClass;
 					String activation = teams[j].getAttribute(ACTIVATION);
-					binding.setActivation(j, activation);
+					binding.setActivation(count++, activation);
 				}
 				
 				@NonNull String realBaseBundleId = baseBundleId.toUpperCase().equals(SELF) ? aspectBundleId : baseBundleId;
@@ -136,10 +139,14 @@
 
 				
 				// now that binding.teamClasses is filled connect to super team, if requested:
-				for (int j = 0; j < teams.length; j++) {
+				for (int j = 0; j < teamCount; j++) {
+					if (teams[j].getAttribute(CLASS) == null) continue;
 					String superTeamName = teams[j].getAttribute(SUPERCLASS);
-					if (superTeamName != null)
-						addSubTeam(aspectBundleId, binding.teamClasses[j], superTeamName);
+					if (superTeamName != null) {
+						String teamName = binding.teamClasses[j];
+						assert teamName != null : "array should not contain nulls";
+						addSubTeam(aspectBundleId, teamName, superTeamName);
+					}
 				}
 				log(ILogger.INFO, "registered:\n"+binding);
 			} catch (Throwable t) {
diff --git a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/OTWeavingHook.java b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/OTWeavingHook.java
index 88544be..e61a52f 100644
--- a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/OTWeavingHook.java
+++ b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/OTWeavingHook.java
@@ -188,6 +188,8 @@
 		}
 		if (scheduledTeams == null) return;
 		for(WaitingTeamRecord record : scheduledTeams) {
+			if (record.aspectBinding.isActivated(record.getTeamName()))
+				continue;
 			log(IStatus.INFO, "Consider for instantiation/activation: team "+record.getTeamName());
 			try {
 				new TeamLoader(deferredTeams, beingDefined).instantiateWaitingTeam(record); // may re-insert to deferredTeams
diff --git a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/TeamLoader.java b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/TeamLoader.java
index c518984..32a86f3 100644
--- a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/TeamLoader.java
+++ b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/TeamLoader.java
@@ -61,8 +61,8 @@
 
 	/**
 	 * Team loading, 1st attempt before the base class is even loaded
-	 * Trying to do these phases load/instantiate/activate,
-	 * and also adds a reverse import to the base.
+	 * Trying to do these phases: load/instantiate/activate (if ready),
+	 * and also adds a reverse import to the base (always).
 	 */
 	public boolean loadTeamsForBase(Bundle aspectBundle, AspectBinding aspectBinding, WovenClass baseClass) {
 		@SuppressWarnings("null")@NonNull String className = baseClass.getClassName();
@@ -89,7 +89,7 @@
 			ActivationKind activationKind = aspectBinding.getActivation(teamForBase);
 			if (activationKind == ActivationKind.NONE)
 				continue;
-			Team teamInstance = instantiateAndActivate(aspectBinding, teamClass, teamForBase, activationKind);
+			Team teamInstance = instantiateAndActivate(aspectBinding, teamClass, activationKind);
 			if (teamInstance == null)
 				continue;
 		}
@@ -100,14 +100,10 @@
 	public void instantiateWaitingTeam(WaitingTeamRecord record)
 			throws InstantiationException, IllegalAccessException 
 	{
-		Team teamInstance = record.teamInstance;
-		String teamName = record.getTeamName();
-		if (teamInstance == null) {
-			// Instantiate (we only get here if activationKind != NONE)
-			Class<? extends Team> teamClass = record.teamClass;
-			assert teamClass != null : "cannot be null if teamInstance is null";
-			teamInstance = instantiateAndActivate(record.aspectBinding, teamClass, teamName, record.activationKind);
-		}
+		// Instantiate (we only get here if activationKind != NONE)
+		Class<? extends Team> teamClass = record.teamClass;
+		assert teamClass != null : "cannot be null if teamInstance is null";
+		instantiateAndActivate(record.aspectBinding, teamClass, record.activationKind);
 	}
 
 	public static @Nullable Pair<URL,String> findTeamClassResource(String className, Bundle bundle) {
@@ -168,14 +164,18 @@
 		return result;
 	}
 
-	@Nullable Team instantiateAndActivate(AspectBinding aspectBinding, Class<? extends Team> teamClass, String teamName, ActivationKind activationKind) 
+	@Nullable Team instantiateAndActivate(AspectBinding aspectBinding, Class<? extends Team> teamClass, ActivationKind activationKind) 
 	{
+		@SuppressWarnings("null")@NonNull String teamName = teamClass.getName();
 		// don't try to instantiate before all base classes successfully loaded.
-		if (!isReadyToLoad(aspectBinding, teamClass, null, teamName, activationKind))
-			return null;
+		synchronized(aspectBinding) {
+			if (!isReadyToLoad(aspectBinding, teamClass, teamName, activationKind))
+				return null;
+			aspectBinding.markAsActivated(teamName);
+		}
 
 		try {
-			Team instance = teamClass.newInstance();
+			@SuppressWarnings("null")@NonNull Team instance = teamClass.newInstance();
 			TransformerPlugin.registerTeamInstance(instance);
 			log(ILogger.INFO, "Instantiated team "+teamName);
 			
@@ -207,16 +207,14 @@
 	}
 
 	boolean isReadyToLoad(AspectBinding aspectBinding,
-			Class<? extends Team> teamClass, @Nullable Team teamInstance,
-			String teamName, ActivationKind activationKind)
+			Class<? extends Team> teamClass, String teamName,
+			ActivationKind activationKind)
 	{
 		for (@SuppressWarnings("null")@NonNull String baseclass : aspectBinding.basesPerTeam.get(teamName)) {
 			if (this.beingDefined.contains(baseclass)) {
 				synchronized (deferredTeams) {
-					WaitingTeamRecord record = teamInstance != null
-							? new WaitingTeamRecord(teamInstance, aspectBinding, activationKind, baseclass)
-							: new WaitingTeamRecord(teamClass, aspectBinding, activationKind, baseclass);
-					deferredTeams.add(record); // TODO(SH): synchronization, deadlock?
+					WaitingTeamRecord record = new WaitingTeamRecord(teamClass, aspectBinding, activationKind, baseclass);
+					deferredTeams.add(record); // TODO(SH): synchronization, deadlock? performed while holding lock an aspectBinding
 				}
 				log(IStatus.INFO, "Defer instantation/activation of team "+teamName);
 				return false;
diff --git a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/WaitingTeamRecord.java b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/WaitingTeamRecord.java
index cfc7756..5438241 100644
--- a/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/WaitingTeamRecord.java
+++ b/plugins/org.eclipse.objectteams.otequinox/src/org/eclipse/objectteams/internal/osgi/weaving/WaitingTeamRecord.java
@@ -16,15 +16,13 @@
 package org.eclipse.objectteams.internal.osgi.weaving;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.objectteams.otequinox.ActivationKind;
 import org.objectteams.Team;
 
-/** Record for one team waiting for instantiation/activation. */
+/** Record for one team waiting for instantiation & activation. */
 @NonNullByDefault
 class WaitingTeamRecord {
-	@Nullable Class<? extends Team> teamClass; // ... either this is set
-	@Nullable Team teamInstance;				// ... or this
+	Class<? extends Team> teamClass;
 	AspectBinding aspectBinding;
 	ActivationKind activationKind;
 	String notFoundClass;
@@ -35,29 +33,9 @@
 		this.notFoundClass = notFoundClass;
 		this.activationKind = activationKind;
 	}
-	public WaitingTeamRecord(Team teamInstance, AspectBinding aspectBinding, ActivationKind activationKind, String notFoundClass) {
-		this.teamInstance = teamInstance;
-		this.aspectBinding = aspectBinding;
-		this.notFoundClass = notFoundClass;
-		this.activationKind = activationKind;
-	}
-	public WaitingTeamRecord(WaitingTeamRecord record, String notFoundClass) {
-		this.teamClass = record.teamClass;
-		this.teamInstance = record.teamInstance;
-		this.aspectBinding = record.aspectBinding;
-		this.activationKind = record.activationKind;
-		this.notFoundClass = notFoundClass;
-	}
-	@SuppressWarnings("null") // calling well-known library functions
+
+	@SuppressWarnings("null") // calling well-known library function
 	public String getTeamName() {
-		final Class<? extends Team> clazz = teamClass;
-		if (clazz != null) {
-			return clazz.getName();
-		} else {
-			final Team instance = teamInstance;
-			if (instance != null)
-				return instance.getClass().getName();
-		}
-		return "<unknown team>";
+		return teamClass.getName();
 	}		
 }
\ No newline at end of file