From a2067ccbc1f732e90ac7e8f8ad4fec5ff4a55736 Mon Sep 17 00:00:00 2001 From: Hason <258831020@qq.com> Date: Sun, 17 Sep 2023 21:44:16 +0800 Subject: [PATCH] feat: optimize eval and error logs (#360) --- pom.xml | 6 ++++ .../casbin/jcasbin/util/BuiltInFunctions.java | 1 + .../java/org/casbin/jcasbin/util/Util.java | 15 ++++++++-- .../main/BuiltInFunctionsUnitTest.java | 21 +++++++++++++ .../org/casbin/jcasbin/main/UtilTest.java | 30 +++++++++++++++++++ 5 files changed, 71 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 651f719b..df5a0b9d 100644 --- a/pom.xml +++ b/pom.xml @@ -185,6 +185,12 @@ 4.13.2 test + + org.mockito + mockito-inline + 4.11.0 + test + com.googlecode.aviator aviator diff --git a/src/main/java/org/casbin/jcasbin/util/BuiltInFunctions.java b/src/main/java/org/casbin/jcasbin/util/BuiltInFunctions.java index 1dd85a63..de4d7fda 100644 --- 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 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 index e8a5df70..5bd98992 100644 --- 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 index 95c9b7e6..0419e1c9 100644 --- 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 env = new HashMap<>(); + + try (MockedStatic 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 index e5cec81b..545bc373 100644 --- 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 utilMocked = BDDMockito.mockStatic(Util.class); + MockedConstruction 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))); + } + } }