From 984ed215a59c2c9932131a6fd8ba57db7d331760 Mon Sep 17 00:00:00 2001
From: tbml <tibor.mlynarik@adleritech.com>
Date: Thu, 1 Nov 2018 18:51:52 +0100
Subject: [PATCH] ReturnCachedValueConcern is not updating cache

---
 .../ReturnCachedValueConcern.java             | 16 ++--
 .../ReturnCachedValueTest.java                | 80 +++++++++++++++++++
 2 files changed, 90 insertions(+), 6 deletions(-)
 create mode 100644 libraries/invocation-cache/src/test/java/org/apache/polygene/library/invocationcache/ReturnCachedValueTest.java

diff --git a/libraries/invocation-cache/src/main/java/org/apache/polygene/library/invocationcache/ReturnCachedValueConcern.java b/libraries/invocation-cache/src/main/java/org/apache/polygene/library/invocationcache/ReturnCachedValueConcern.java
index 9e1abb35b..d1a7fcea1 100644
--- a/libraries/invocation-cache/src/main/java/org/apache/polygene/library/invocationcache/ReturnCachedValueConcern.java
+++ b/libraries/invocation-cache/src/main/java/org/apache/polygene/library/invocationcache/ReturnCachedValueConcern.java
@@ -32,15 +32,15 @@
  */
 @AppliesTo( Cached.class )
 public class ReturnCachedValueConcern
-    extends ConcernOf<InvocationHandler>
-    implements InvocationHandler
+        extends ConcernOf<InvocationHandler>
+        implements InvocationHandler
 {
     @This @Optional
     private InvocationCache cache;
 
     @Override
     public Object invoke( Object proxy, Method method, Object[] args )
-        throws Throwable
+            throws Throwable
     {
         boolean voidReturnType = method.getReturnType().equals( Void.TYPE );
         if( cache != null || voidReturnType )
@@ -52,12 +52,16 @@ public Object invoke( Object proxy, Method method, Object[] args )
                 cacheName += Arrays.asList( args );
             }
             Object result = cache.cachedValue( cacheName );
-            if( result != null )
+            if (result == null)
             {
-                return result;
+                // No cached value found
+                result = next.invoke(proxy, method, args);
+                // Update cache
+                cache.setCachedValue(cacheName, result);
             }
+            return result;
         }
-        // No cached value found or no InvocationCache defined - call method
+        // No InvocationCache defined - call method
         return next.invoke( proxy, method, args );
     }
 }
diff --git a/libraries/invocation-cache/src/test/java/org/apache/polygene/library/invocationcache/ReturnCachedValueTest.java b/libraries/invocation-cache/src/test/java/org/apache/polygene/library/invocationcache/ReturnCachedValueTest.java
new file mode 100644
index 000000000..e76ba5d08
--- /dev/null
+++ b/libraries/invocation-cache/src/test/java/org/apache/polygene/library/invocationcache/ReturnCachedValueTest.java
@@ -0,0 +1,80 @@
+package org.apache.polygene.library.invocationcache;
+
+import java.util.Arrays;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.polygene.api.injection.scope.Service;
+import org.apache.polygene.api.mixin.Mixins;
+import org.apache.polygene.bootstrap.AssemblyException;
+import org.apache.polygene.bootstrap.ModuleAssembly;
+import org.apache.polygene.test.AbstractPolygeneTest;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsEqual.equalTo;
+
+public class ReturnCachedValueTest extends AbstractPolygeneTest {
+
+    @Service
+    ExpensiveOperation srv;
+
+    @BeforeEach
+    void resetInvoicationCounter() {
+        ExpensiveOperation.invoicationCount.set(0);
+    }
+
+    @Test
+    public void methodResultIsCachedForEqualParameters() {
+
+        String parameter = "cachedHash";
+        assertThat(srv.compute(parameter), equalTo(parameter.hashCode()));
+        assertThat(srv.compute(parameter), equalTo(parameter.hashCode()));
+
+        assertThat(ExpensiveOperation.invoicationCount.intValue(), equalTo(1));
+    }
+
+    //@Test
+    //Ignored: cache key builder has to be fixed to support this case
+    public void cacheKeyNotWorkWithVarargsParameter() {
+        ExpensiveOperation.invoicationCount.set(0);
+        srv.compute(7, 13);
+        srv.compute(7, 13);
+
+        assertThat(ExpensiveOperation.invoicationCount.intValue(), equalTo(1));
+    }
+
+    @Override
+    public void assemble(ModuleAssembly module) throws AssemblyException {
+        module.services(ExpensiveOperation.class)
+                .withMixins(SimpleInvocationCacheMixin.class)
+                .withConcerns(ReturnCachedValueConcern.class);
+    }
+
+
+    @Mixins(Impl.class)
+    public interface ExpensiveOperation {
+
+        AtomicInteger invoicationCount = new AtomicInteger(0);
+
+        @Cached
+        int compute(String param);
+
+        int compute(int... arguments);
+    }
+
+    public abstract static class Impl implements ExpensiveOperation {
+
+        @Override
+        public int compute(String param) {
+            invoicationCount.incrementAndGet();
+            return param.hashCode();
+        }
+
+        @Override
+        public int compute(int... arguments) {
+            invoicationCount.incrementAndGet();
+            return Arrays.stream(arguments).sum();
+        }
+    }
+}
+