feat: optimize eval and error logs (#360)

casbin · Sep 17, 2023 · a2067cc · a2067cc
1 parent 3d22ec5
commit a2067cc
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 2 deletions.
diff --git a/pom.xml b/pom.xml
@@ -185,6 +185,12 @@
             <version>4.13.2</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-inline</artifactId>
+            <version>4.11.0</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>com.googlecode.aviator</groupId>
             <artifactId>aviator</artifactId>

diff --git a/src/main/java/org/casbin/jcasbin/util/BuiltInFunctions.java b/src/main/java/org/casbin/jcasbin/util/BuiltInFunctions.java
@@ -410,6 +410,7 @@ public static boolean eval(String eval, Map<String, Object> env, AviatorEvaluato
             try {
                 res = (boolean) aviatorEval.execute(eval, env);
             } catch (Exception e) {
+                Util.logPrintfWarn("Execute 'eval' function error, nested exception is: {}", e.getMessage());
                 res = false;
             }
         } else {

diff --git a/src/main/java/org/casbin/jcasbin/util/Util.java b/src/main/java/org/casbin/jcasbin/util/Util.java
@@ -87,6 +87,18 @@ public static void logPrintfError(String format, Object... v) {
         }
     }
 
+    /**
+     * logPrintf prints the log with the format as an error.
+     *
+     * @param message the message accompanying the exception
+     * @param t       the exception (throwable) to log
+     */
+    public static void logPrintfError(String message, Throwable t) {
+        if (enableLog) {
+            LOGGER.error(message, t);
+        }
+    }
+
     /**
      * logEnforce prints the log of Enforce.
      *
@@ -264,8 +276,7 @@ public static String[] splitCommaDelimited(String s) {
                     records[i] = csvRecords.get(0).get(i).trim();
                 }
             } catch (IOException e) {
-                e.printStackTrace();
-                Util.logPrintfError("CSV parser failed to parse this line:", s);
+                Util.logPrintfError("CSV parser failed to parse this line: " + s, e);
             }
         }
         return records;

diff --git a/src/test/java/org/casbin/jcasbin/main/BuiltInFunctionsUnitTest.java b/src/test/java/org/casbin/jcasbin/main/BuiltInFunctionsUnitTest.java
@@ -16,12 +16,17 @@
 
 import com.googlecode.aviator.AviatorEvaluator;
 import com.googlecode.aviator.AviatorEvaluatorInstance;
+import org.casbin.jcasbin.util.BuiltInFunctions;
+import org.casbin.jcasbin.util.Util;
 import org.junit.Test;
+import org.mockito.BDDMockito;
+import org.mockito.MockedStatic;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import static org.casbin.jcasbin.main.TestUtil.*;
+import static org.mockito.ArgumentMatchers.*;
 
 public class BuiltInFunctionsUnitTest {
 
@@ -260,4 +265,20 @@ public void testGlobMatchFunc() {
         testGlobMatch("/prefix/subprefix/foobar", "*/foo*", false);
         testGlobMatch("/prefix/subprefix/foobar", "*/foo/*", false);
     }
+
+    @Test
+    public void should_logged_when_eval_given_errorExpression() {
+        // given
+        AviatorEvaluatorInstance instance = AviatorEvaluator.getInstance();
+        Map<String, Object> env = new HashMap<>();
+
+        try (MockedStatic<Util> utilMocked = BDDMockito.mockStatic(Util.class)) {
+            utilMocked.when(() -> Util.logPrintfWarn(anyString(), anyString())).thenCallRealMethod();
+            // when
+            BuiltInFunctions.eval("error", env, instance);
+            // then
+            utilMocked.verify(() -> Util.logPrintfWarn(eq("Execute 'eval' function error, nested exception is: {}"), any()));
+        }
+    }
+
 }
diff --git a/src/test/java/org/casbin/jcasbin/main/UtilTest.java b/src/test/java/org/casbin/jcasbin/main/UtilTest.java
@@ -14,11 +14,21 @@
 
 package org.casbin.jcasbin.main;
 
+import org.apache.commons.csv.CSVFormat;
+import org.apache.commons.csv.CSVParser;
 import org.casbin.jcasbin.util.SyncedLRUCache;
 import org.casbin.jcasbin.util.Util;
 import org.junit.Test;
+import org.mockito.ArgumentMatchers;
+import org.mockito.BDDMockito;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+
+import java.io.IOException;
+import java.io.StringReader;
 
 import static org.junit.Assert.*;
+import static org.mockito.ArgumentMatchers.*;
 
 public class UtilTest {
 
@@ -92,4 +102,24 @@ public void testLruCache() {
       TestUtil.testCacheGet(cache, "three", 3, true);
       TestUtil.testCacheGet(cache, "four", 4, true);
   }
+
+    @Test
+    public void should_logged_when_splitCommaDelimited_given_ioException() {
+        IOException ioEx = new IOException("Mock IOException");
+        try (
+            MockedStatic<Util> utilMocked = BDDMockito.mockStatic(Util.class);
+            MockedConstruction<CSVFormat> stringReaderMocked = BDDMockito.mockConstruction(CSVFormat.class, (mock, context) -> {
+                BDDMockito.given(mock.parse(any(StringReader.class))).willThrow(ioEx);
+            })) {
+            // given
+            utilMocked.when(() -> Util.splitCommaDelimited(anyString())).thenCallRealMethod();
+            String csv = "\n";
+
+            // when
+            Util.splitCommaDelimited(csv);
+
+            // then
+            utilMocked.verify(() -> Util.logPrintfError(eq("CSV parser failed to parse this line: " + csv), eq(ioEx)));
+        }
+    }
 }