Merge "[562369] Ensures delayed commands are executed on listener registration"

3 files changed, 153 insertions, 20 deletions

diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/ReteEngine.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/ReteEngine.java
index 2c26fd946..07058e888 100644
--- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/ReteEngine.java
+++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/ReteEngine.java
@@ -258,14 +258,22 @@ public class ReteEngine implements IQueryBackend {
         });
     }
 
-    private void constructionWrapper(final Callable<Void> payload) {
+    /**
+     * @since 2.4
+     */
+    public <T> T constructionWrapper(final Callable<T> payload) {
+        T result = null;
 //		context.modelReadLock();
 //		    try {
                 if (parallelExecutionEnabled)
                     reteNet.getStructuralChangeLock().lock();
                 try {
                     try {
-                        runtimeContext.coalesceTraversals(() -> { payload.call(); this.executeDelayedCommands(); return null;});
+                        result = runtimeContext.coalesceTraversals(() -> { 
+                            T innerResult = payload.call(); 
+                            this.executeDelayedCommands(); 
+                            return innerResult;
+                        });
                     } catch (InvocationTargetException ex) {
                         final Throwable cause = ex.getCause();
                         if (cause instanceof RetePatternBuildException)
@@ -282,6 +290,7 @@ public class ReteEngine implements IQueryBackend {
 //		    } finally {
 //		        context.modelReadUnLock();
 //		    }
+            return result;
     }
 
     // /**
@@ -351,22 +360,10 @@ public class ReteEngine implements IQueryBackend {
         NodeProvisioner nodeProvisioner = reteNet.getHeadContainer().getProvisioner();
         Indexer result = nodeProvisioner.peekProjectionIndexer(production, mask);
         if (result == null) {
-//            context.modelReadLock();
-//            try {
-                if (parallelExecutionEnabled)
-                    reteNet.getStructuralChangeLock().lock();
-                try {
-                    result = nodeProvisioner.accessProjectionIndexerOnetime(production, mask);
-                } finally {
-                    if (parallelExecutionEnabled)
-                        reteNet.getStructuralChangeLock().unlock();
-                }
-//            } finally {
-//                context.modelReadUnLock();
-//            }
+            result = constructionWrapper(() -> 
+                nodeProvisioner.accessProjectionIndexerOnetime(production, mask)
+            );
         }
-        
-        executeDelayedCommands();
 
         return result;
     }
diff --git a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/RetePatternMatcher.java b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/RetePatternMatcher.java
index 9d4359deb..71cf3bcfc 100644
--- a/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/RetePatternMatcher.java
+++ b/query/plugins/org.eclipse.viatra.query.runtime.rete/src/org/eclipse/viatra/query/runtime/rete/matcher/RetePatternMatcher.java
@@ -243,6 +243,7 @@ public class RetePatternMatcher extends TransformerNode implements IQueryResultP
             this.signature = signature;
         }
 
+        @Override
         public void run() {
             fetch(indexer.get(signature));
         }
@@ -365,6 +366,7 @@ public class RetePatternMatcher extends TransformerNode implements IQueryResultP
             this.indexer = indexer;
         }
 
+        @Override
         public void run() {
             size = indexer.getBucketCount();
         }
@@ -437,13 +439,21 @@ public class RetePatternMatcher extends TransformerNode implements IQueryResultP
 
     @Override
     public void addUpdateListener(final IUpdateable listener, final Object listenerTag, boolean fireNow) {
-        final CallbackNode callbackNode = new CallbackNode(this.reteContainer, listener);
-        connect(callbackNode, listenerTag, fireNow);
+        // As a listener is added as a delayed command, they should be executed to make sure everything is consistent on
+        // return, see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=562369
+        engine.constructionWrapper(() -> {
+            final CallbackNode callbackNode = new CallbackNode(this.reteContainer, listener);
+            connect(callbackNode, listenerTag, fireNow);
+            return null;
+        });
     }
 
     @Override
     public void removeUpdateListener(Object listenerTag) {
-        disconnectByTag(listenerTag);
+        engine.constructionWrapper(() -> {
+            disconnectByTag(listenerTag);
+            return null;
+        });
     }
 
 }
diff --git a/query/tests/org.eclipse.viatra.query.runtime.cps.tests/src/org/eclipse/viatra/query/runtime/cps/tests/api/QueryEngineAPITest.java b/query/tests/org.eclipse.viatra.query.runtime.cps.tests/src/org/eclipse/viatra/query/runtime/cps/tests/api/QueryEngineAPITest.java
new file mode 100644
index 000000000..e175ebd2e
--- /dev/null
+++ b/query/tests/org.eclipse.viatra.query.runtime.cps.tests/src/org/eclipse/viatra/query/runtime/cps/tests/api/QueryEngineAPITest.java
@@ -0,0 +1,126 @@
+/*******************************************************************************
+ * Copyright (c) 2010-2018, Zoltan Ujhelyi, IncQuery Labs Ltd.
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-v20.html.
+ * 
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.viatra.query.runtime.cps.tests.api;
+
+import static org.junit.Assert.assertEquals;
+
+import org.eclipse.emf.common.util.URI;
+import org.eclipse.emf.ecore.resource.ResourceSet;
+import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;
+import org.eclipse.viatra.query.runtime.api.AdvancedViatraQueryEngine;
+import org.eclipse.viatra.query.runtime.api.IMatchUpdateListener;
+import org.eclipse.viatra.query.runtime.api.ViatraQueryEngine;
+import org.eclipse.viatra.query.runtime.cps.tests.queries.ApplicationTypesMatch;
+import org.eclipse.viatra.query.runtime.cps.tests.queries.ApplicationTypesMatcher;
+import org.eclipse.viatra.query.runtime.cps.tests.queries.HostTypesMatcher;
+import org.eclipse.viatra.query.runtime.emf.EMFScope;
+import org.junit.Before;
+import org.junit.Test;
+
+public class QueryEngineAPITest {
+    
+    private final class CounterListener implements IMatchUpdateListener<ApplicationTypesMatch> {
+        int appearCount = 0;
+        int disappearCount = 0;
+
+        @Override
+        public void notifyAppearance(ApplicationTypesMatch match) {
+            appearCount++;
+        }
+
+        @Override
+        public void notifyDisappearance(ApplicationTypesMatch match) {
+            disappearCount++;                
+        }
+    }
+
+
+    AdvancedViatraQueryEngine engine;
+    
+    @Before
+    public void initializeEngine() {
+        ResourceSet rs = new ResourceSetImpl();
+        rs.getResource(URI.createPlatformPluginURI("org.eclipse.viatra.query.runtime.cps.tests/models/instances/demo.cyberphysicalsystem", false), true);
+        engine = AdvancedViatraQueryEngine.from(ViatraQueryEngine.on(new EMFScope(rs)));
+    }
+    
+    
+    /**
+     * Test case for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=562369
+     */
+    @Test
+    public void testListenerAddition1() {
+        final CounterListener listener = new CounterListener();
+        final ApplicationTypesMatcher matcher = ApplicationTypesMatcher.on(engine);
+        try {
+            engine.addMatchUpdateListener(matcher, listener, true);
+            
+            int appearCount = listener.appearCount;
+            int disappearCount = listener.disappearCount;
+            ApplicationTypesMatcher.on(engine);
+            
+            assertEquals("Late appearance count incorrect", 2, listener.appearCount);
+            assertEquals("Late disappearance count incorrect", 0, listener.disappearCount);
+            assertEquals("Early appearance count incorrect", 2, appearCount);
+            assertEquals("Early disappearance count incorrect", 0, disappearCount);
+        } finally {
+            engine.removeMatchUpdateListener(matcher, listener);
+        }
+
+    }
+    
+    /**
+     * Test case for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=562369
+     */
+    @Test
+    public void testListenerAddition2() {
+        final CounterListener listener = new CounterListener();
+        final ApplicationTypesMatcher matcher = ApplicationTypesMatcher.on(engine);
+        try {
+            engine.addMatchUpdateListener(matcher, listener, true);
+            
+            int appearCount = listener.appearCount;
+            int disappearCount = listener.disappearCount;
+            
+            matcher.countMatches();
+            
+            assertEquals("Late appearance count incorrect", 2, listener.appearCount);
+            assertEquals("Late disappearance count incorrect", 0, listener.disappearCount);
+            assertEquals("Early appearance count incorrect", 2, appearCount);
+            assertEquals("Early disappearance count incorrect", 0, disappearCount);
+        } finally {
+            engine.removeMatchUpdateListener(matcher, listener);
+        }
+    }
+    
+    /**
+     * Test case for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=562369
+     */
+    @Test
+    public void testListenerAddition3() {
+        final CounterListener listener = new CounterListener();
+        final ApplicationTypesMatcher matcher = ApplicationTypesMatcher.on(engine);
+        try {
+            engine.addMatchUpdateListener(matcher, listener, true);
+            
+            int appearCount = listener.appearCount;
+            int disappearCount = listener.disappearCount;
+            
+            HostTypesMatcher.on(engine);
+            
+            assertEquals("Late appearance count incorrect", 2, listener.appearCount);
+            assertEquals("Late disappearance count incorrect", 0, listener.disappearCount);
+            assertEquals("Early appearance count incorrect", 2, appearCount);
+            assertEquals("Early disappearance count incorrect", 0, disappearCount);
+        } finally {
+            engine.removeMatchUpdateListener(matcher, listener);
+        }
+    }
+    
+}