#1597: CacheFilter,when value is null,it will throw NPE(if use ehcac…

…he for jcache),why not check null here (#1828)
apache · May 22, 2018 · a6d8f1c · a6d8f1c
1 parent d03ff27
commit a6d8f1c
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 1 deletion.
diff --git a/...o-filter/dubbo-filter-cache/src/main/java/com/alibaba/dubbo/cache/filter/CacheFilter.java b/...o-filter/dubbo-filter-cache/src/main/java/com/alibaba/dubbo/cache/filter/CacheFilter.java
@@ -52,7 +52,7 @@ public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcExcept
                     return new RpcResult(value);
                 }
                 Result result = invoker.invoke(invocation);
-                if (!result.hasException()) {
+                if (!result.hasException() && result.getValue() != null) {
                     cache.put(key, result.getValue());
                 }
                 return result;

diff --git a/...lter/dubbo-filter-cache/src/test/java/com/alibaba/dubbo/cache/filter/CacheFilterTest.java b/...lter/dubbo-filter-cache/src/test/java/com/alibaba/dubbo/cache/filter/CacheFilterTest.java
@@ -44,6 +44,8 @@ public class CacheFilterTest {
     private Invoker<?> invoker = mock(Invoker.class);
     private Invoker<?> invoker1 = mock(Invoker.class);
     private Invoker<?> invoker2 = mock(Invoker.class);
+    private Invoker<?> invoker3 = mock(Invoker.class);
+    private Invoker<?> invoker4 = mock(Invoker.class);
     private String cacheType;
     private CacheFactory cacheFactory;
 
@@ -77,6 +79,11 @@ public void setUp() throws Exception {
         given(invoker2.invoke(invocation)).willReturn(new RpcResult("value2"));
         given(invoker2.getUrl()).willReturn(url);
 
+        given(invoker3.invoke(invocation)).willReturn(new RpcResult(new RuntimeException()));
+        given(invoker3.getUrl()).willReturn(url);
+
+        given(invoker4.invoke(invocation)).willReturn(new RpcResult());
+        given(invoker4.getUrl()).willReturn(url);
     }
 
     @Test
@@ -89,6 +96,7 @@ public void testNonArgsMethod() {
         RpcResult rpcResult1 = (RpcResult) cacheFilter.invoke(invoker1, invocation);
         RpcResult rpcResult2 = (RpcResult) cacheFilter.invoke(invoker2, invocation);
         Assert.assertEquals(rpcResult1.getValue(), rpcResult2.getValue());
+        Assert.assertEquals(rpcResult1.getValue(), "value");
     }
 
     @Test
@@ -101,5 +109,30 @@ public void testMethodWithArgs() {
         RpcResult rpcResult1 = (RpcResult) cacheFilter.invoke(invoker1, invocation);
         RpcResult rpcResult2 = (RpcResult) cacheFilter.invoke(invoker2, invocation);
         Assert.assertEquals(rpcResult1.getValue(), rpcResult2.getValue());
+        Assert.assertEquals(rpcResult1.getValue(), "value");
+    }
+
+    @Test
+    public void testException() {
+        invocation.setMethodName("echo1");
+        invocation.setParameterTypes(new Class<?>[]{String.class});
+        invocation.setArguments(new Object[]{"arg2"});
+
+        cacheFilter.invoke(invoker3, invocation);
+        RpcResult rpcResult = (RpcResult) cacheFilter.invoke(invoker2, invocation);
+        Assert.assertEquals(rpcResult.getValue(), "value2");
+    }
+
+    @Test
+    public void testNull() {
+        invocation.setMethodName("echo1");
+        invocation.setParameterTypes(new Class<?>[]{String.class});
+        invocation.setArguments(new Object[]{"arg3"});
+
+        cacheFilter.invoke(invoker4, invocation);
+        RpcResult rpcResult1 = (RpcResult) cacheFilter.invoke(invoker1, invocation);
+        RpcResult rpcResult2 = (RpcResult) cacheFilter.invoke(invoker2, invocation);
+        Assert.assertEquals(rpcResult1.getValue(), "value1");
+        Assert.assertEquals(rpcResult2.getValue(), "value1");
     }
 }