[460947] Support Java Services reloading in AQL

Reworks JavaExtensions to guarantee to always call unload() methods
first on the ClassLoadingcallBacks and then triggers all the loads(). 
Use the dedicated API in Acceleo Query to unregister the service
instances.

Bug: 460947
Change-Id: I165e16f2fa4d6956d74982d198f2dda5bb1387d2
Signed-off-by: Cedric Brun <cedric.brun@obeo.fr>

2 files changed, 48 insertions, 34 deletions

diff --git a/plugins/org.eclipse.sirius.common.acceleo.aql/src/org/eclipse/sirius/common/acceleo/aql/business/internal/AQLSiriusInterpreter.java b/plugins/org.eclipse.sirius.common.acceleo.aql/src/org/eclipse/sirius/common/acceleo/aql/business/internal/AQLSiriusInterpreter.java
index b9e9171884..46dc9a4eaf 100644
--- a/plugins/org.eclipse.sirius.common.acceleo.aql/src/org/eclipse/sirius/common/acceleo/aql/business/internal/AQLSiriusInterpreter.java
+++ b/plugins/org.eclipse.sirius.common.acceleo.aql/src/org/eclipse/sirius/common/acceleo/aql/business/internal/AQLSiriusInterpreter.java
@@ -112,8 +112,7 @@ public class AQLSiriusInterpreter extends AcceleoAbstractInterpreter {
 
         @Override
         public void unloaded(String qualifiedName, Class<?> clazz) {
-            // TODO implement the un-register once it is available in AQL.
-            // see Bug 461072
+            queryEnvironment.removeServicePackage(clazz);
 
         }
     };
diff --git a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/interpreter/JavaExtensionsManager.java b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/interpreter/JavaExtensionsManager.java
index ab81c30dd3..5dfdc0cb5a 100644
--- a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/interpreter/JavaExtensionsManager.java
+++ b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/interpreter/JavaExtensionsManager.java
@@ -15,7 +15,6 @@ import java.lang.reflect.Method;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -90,9 +89,8 @@ public final class JavaExtensionsManager {
              * we get a notification if something in the classpath we used so
              * far has changed.
              */
-            if (viewpointPlugins.size() > 0 && viewpointProjects.size() > 0) {
-                reloadEPackages();
-                reloadJavaExtensions();
+            if (viewpointPlugins.size() > 0 || viewpointProjects.size() > 0) {
+                reload();
             }
         }
     };
@@ -180,7 +178,6 @@ public final class JavaExtensionsManager {
         clearImports();
         this.viewpointPlugins.clear();
         this.viewpointProjects.clear();
-        this.couldNotBeLoaded.clear();
     }
 
     /**
@@ -204,14 +201,24 @@ public final class JavaExtensionsManager {
          * extensions.
          */
         if (couldNotBeLoaded.size() > 0 || removedAddedAtLeastOnePlugin || removedAddedAtLeastOnProject) {
-            if (this.viewpointPlugins.size() > 0 && this.viewpointProjects.size() > 0) {
-                reloadEPackages();
-                reloadJavaExtensions();
+            if (this.viewpointPlugins.size() > 0 || this.viewpointProjects.size() > 0) {
+                reload();
             }
         }
 
     }
 
+    /*
+     * This method is synchronized as we expect the workspace listener to call
+     * it when an event requires a reload. There is no guarantee this call is
+     * made from the same thread as the others and we don't want several reloads
+     * being done concurrently.
+     */
+    private synchronized void reload() {
+        reloadEPackages();
+        reloadJavaExtensions();
+    }
+
     private void reloadEPackages() {
         Multimap<String, EPackage> newDeclarations = HashMultimap.create();
         Set<String> newDeclarersAsBundles = Sets.newLinkedHashSet();
@@ -417,13 +424,10 @@ public final class JavaExtensionsManager {
      *            Extension.
      */
     public void addImport(String classQualifiedName) {
-        if (couldNotBeLoaded.contains(classQualifiedName)) {
+        if (classQualifiedName != null && classQualifiedName.contains(".")) {
+            this.imports.add(classQualifiedName);
             loadJavaExtensions(Sets.newHashSet(classQualifiedName));
         }
-        if (classQualifiedName != null && classQualifiedName.contains(".") && !imports.contains(classQualifiedName)) {
-            imports.add(classQualifiedName);
-            loadJavaExtensions(Collections.singleton(classQualifiedName));
-        }
     }
 
     /**
@@ -435,9 +439,7 @@ public final class JavaExtensionsManager {
      */
     public void removeImport(String classQualifiedName) {
         if (this.imports.contains(classQualifiedName)) {
-            if (couldNotBeLoaded.contains(classQualifiedName)) {
-                couldNotBeLoaded.remove(classQualifiedName);
-            }
+            couldNotBeLoaded.remove(classQualifiedName);
             Set<String> removedImport = Sets.newLinkedHashSet();
             removedImport.add(classQualifiedName);
             this.imports.remove(classQualifiedName);
@@ -464,29 +466,42 @@ public final class JavaExtensionsManager {
     }
 
     private void reloadJavaExtensions() {
-        unloadJavaExtensions(this.imports);
         loadJavaExtensions(this.imports);
     }
 
     private void loadJavaExtensions(Set<String> addedImports) {
+        Map<String, Class> toUnload = Maps.newLinkedHashMap();
+        Map<String, Class> toLoad = Maps.newLinkedHashMap();
+        Set<String> notFound = Sets.newLinkedHashSet();
         for (String qualifiedName : addedImports) {
             Class<?> found = classLoading.findClass(viewpointProjects, viewpointPlugins, qualifiedName);
-            processResult(qualifiedName, found);
-        }
-    }
-
-    private void processResult(String qualifiedName, Class<?> found) {
-        if (found != null) {
-            this.couldNotBeLoaded.remove(qualifiedName);
-            Class<?> alreadyHere = loadedClasses.get(qualifiedName);
-            if (alreadyHere != null) {
-                unloaded(qualifiedName, alreadyHere);
+            if (found != null) {
+                this.couldNotBeLoaded.remove(qualifiedName);
+                Class<?> alreadyHere = loadedClasses.get(qualifiedName);
+                if (alreadyHere != null) {
+                    toUnload.put(qualifiedName, alreadyHere);
+                }
+                loadedClasses.put(qualifiedName, found);
+                toLoad.put(qualifiedName, found);
+            } else {
+                notFound.add(qualifiedName);
+                this.couldNotBeLoaded.add(qualifiedName);
             }
-            loadedClasses.put(qualifiedName, found);
-
-            loaded(qualifiedName, found);
-        } else {
-            this.couldNotBeLoaded.add(qualifiedName);
+        }
+        /*
+         * We make sure we notify the callbacks with the following orders :
+         * always notify an unload first, then the loaded classes, then the
+         * qualified names which were not found. This makes it easier for the
+         * callbacks to maintain their own data structures in sync regarding the
+         * latest versions of the classes.
+         */
+        for (Map.Entry<String, Class> classToUnload : toUnload.entrySet()) {
+            unloaded(classToUnload.getKey(), classToUnload.getValue());
+        }
+        for (Map.Entry<String, Class> classToLoad : toLoad.entrySet()) {
+            loaded(classToLoad.getKey(), classToLoad.getValue());
+        }
+        for (String qualifiedName : notFound) {
             notFound(qualifiedName);
         }
     }