improved spec parsing error handing

ben-manes · Mar 31, 2024 · 5d453e9 · 5d453e9
1 parent bd4f491
commit 5d453e9
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 6 deletions.
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CaffeineSpec.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CaffeineSpec.java
@@ -155,18 +155,19 @@ void parseOption(String option) {
     }
 
     @SuppressWarnings("StringSplitter")
-    String[] keyAndValue = option.split(SPLIT_KEY_VALUE);
-    requireArgument((keyAndValue.length >= 1) && (keyAndValue.length <= 2),
+    String[] keyAndValue = option.split(SPLIT_KEY_VALUE, 3);
+    requireArgument(keyAndValue.length >= 1, "blank key-value pair");
+    requireArgument(keyAndValue.length <= 2,
         "key-value pair %s with more than one equals sign", option);
 
     String key = keyAndValue[0].trim();
     String value = (keyAndValue.length == 1) ? null : keyAndValue[1].trim();
 
-    configure(key, value);
+    configure(option, key, value);
   }
 
   /** Configures the setting. */
-  void configure(String key, @Nullable String value) {
+  void configure(String option, String key, @Nullable String value) {
     switch (key) {
       case "initialCapacity":
         initialCapacity(key, value);
@@ -199,7 +200,7 @@ void configure(String key, @Nullable String value) {
         recordStats(value);
         return;
       default:
-        throw new IllegalArgumentException("Unknown key " + key);
+        throw new IllegalArgumentException("Invalid option " + option);
     }
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineSpecTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineSpecTest.java
@@ -69,6 +69,15 @@ public void parseTimeUnit_exception() {
     assertThrows(IllegalArgumentException.class, () -> CaffeineSpec.parseTimeUnit("key", "value"));
   }
 
+  @Test
+  public void parse_exception() {
+    assertThrows(IllegalArgumentException.class, () -> CaffeineSpec.parse("="));
+    assertThrows(IllegalArgumentException.class, () -> CaffeineSpec.parse("=="));
+    assertThrows(IllegalArgumentException.class, () -> CaffeineSpec.parse("key="));
+    assertThrows(IllegalArgumentException.class, () -> CaffeineSpec.parse("=value"));
+    assertThrows(IllegalArgumentException.class, () -> CaffeineSpec.parse("key=value="));
+  }
+
   @Test(dataProvider = "caches")
   @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY,
       initialCapacity = {InitialCapacity.DEFAULT, InitialCapacity.FULL},
@@ -122,6 +131,7 @@ private void runTest(CacheContext context, Epoch epoch, Function<Duration, Strin
     checkRefreshAfterWrite(spec, context, builder, epoch);
 
     assertThat(spec).isEqualTo(CaffeineSpec.parse(spec.toParsableString()));
+    assertThat(spec).isEqualTo(CaffeineSpec.parse(spec.toParsableString().replaceAll(",", ",,")));
   }
 
   static CaffeineSpec toSpec(CacheContext context,

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -20,7 +20,7 @@ commons-text = "1.11.0"
 concurrentlinkedhashmap = "1.4.2"
 config = "1.4.3"
 coveralls = "2.12.2"
-dependency-check = "9.0.10"
+dependency-check = "9.1.0"
 eclipse-collections = "12.0.0.M3"
 ehcache3 = "3.10.8"
 errorprone-core = "2.26.1"