[8.18] setting: enforce non-nullable string (restore 8.15.x behavior) (backport #17522)

logstash-core/spec/logstash/settings/string_spec.rb

@@ -34,5 +34,10 @@
         expect(subject.value).to eq("a")
       end
     end
+    context "when a null value is given" do
+      it "raises an ArgumentError" do
+        expect { subject.set(nil) }.to raise_error(java.lang.IllegalArgumentException)
+      end
+    end
   end
 end

logstash-core/src/main/java/org/logstash/settings/SettingString.java

@@ -45,6 +45,9 @@ public SettingString(String name, String defaultValue, boolean strict) {
 
     @Override
     public void validate(String input) throws IllegalArgumentException {
+        if (input == null) {
+            throw new IllegalArgumentException(String.format("Setting \"%s\" must be a String. Received:  (NilClass)", this.getName()));
+        }
         staticValidate(input, possibleStrings, this.getName());
     }
 

logstash-core/src/test/java/org/logstash/settings/SettingNullableStringTest.java

@@ -0,0 +1,71 @@
+package org.logstash.settings;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
+
+import java.util.List;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.Assert.*;
+
+@RunWith(Enclosed.class)
+public class SettingNullableStringTest {
+
+    public static class WithValueConstraintCase{
+        private static final List<String> POSSIBLE_VALUES = List.of("a", "b", "c");
+        private SettingString sut;
+
+        @Before
+        public void setUp() throws Exception {
+            sut = new SettingNullableString("mytext", POSSIBLE_VALUES.iterator().next(), true, POSSIBLE_VALUES);
+        }
+
+        @Test
+        public void whenSetConstrainedToValueNotPresentInPossibleValuesThenThrowAHelpfulError() {
+            IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
+                sut.set("d");
+            });
+            assertThat(ex.getMessage(), containsString("Invalid value \"mytext: d\""));
+        }
+
+        @Test
+        public void whenSetConstrainedToValuePresentInPossibleValuesThenSetValue() {
+            sut.set("a");
+
+            assertEquals("a", sut.value());
+        }
+
+        @Test
+        public void whenSetConstrainedToNullThenSetValue() {
+            sut.set(null);
+
+            assertNull(sut.value());
+        }
+    }
+
+    public static class WithoutValueConstraintCase {
+        private SettingString sut;
+
+        @Before
+        public void setUp() throws Exception {
+            sut = new SettingNullableString("mytext", "foo", true);
+        }
+
+        @Test
+        public void whenSetUnconstrainedToNonNullValueThenSetValue() {
+            sut.set("a");
+
+            assertEquals("a", sut.value());
+        }
+
+        @Test
+        public void whenSetUnconstrainedToNullThenSetValue() {
+            sut.set(null);
+
+            assertNull(sut.value());
+        }
+    }
+}

logstash-core/src/test/java/org/logstash/settings/SettingStringTest.java

@@ -20,31 +20,73 @@
 
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
 
 import java.util.List;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 
-// Mirrored from logstash-core/spec/logstash/settings/string_spec.rb
+@RunWith(Enclosed.class)
 public class SettingStringTest {
 
-    private static final List<String> POSSIBLE_VALUES = List.of("a", "b", "c");
-    private SettingString sut;
+    public static class WithValueConstraintCase {
+        private static final List<String> POSSIBLE_VALUES = List.of("a", "b", "c");
+        private SettingString sut;
 
-    @Before
-    public void setUp() throws Exception {
-        sut = new SettingString("mytext", POSSIBLE_VALUES.iterator().next(), true, POSSIBLE_VALUES);
-    }
+        @Before
+        public void setUp() throws Exception {
+            sut = new SettingString("mytext", POSSIBLE_VALUES.iterator().next(), true, POSSIBLE_VALUES);
+        }
+
+        @Test
+        public void whenSetValueNotPresentInPossibleValuesThenThrowAHelpfulError() {
+            IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
+                sut.set("d");
+            });
+            assertThat(ex.getMessage(), containsString("Invalid value \"mytext: d\""));
+        }
+
+        @Test
+        public void whenSetConstrainedToValuePresentInPossibleValuesThenSetValue() {
+            sut.set("a");
+
+            assertEquals("a", sut.value());
+        }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void whenSetValueNotPresentInPossibleValuesThenThrowAnError() {
-        sut.set("d");
+        @Test
+        public void whenSetConstrainedToNullThenThrowAHelpfulError() {
+            IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
+                sut.set(null);
+            });
+            assertThat(ex.getMessage(), containsString("Setting \"mytext\" must be a String"));
+        }
     }
 
-    @Test
-    public void whenSetValuePresentInPossibleValuesThenSetValue() {
-        sut.set("a");
+    public static class WithoutValueConstraintCase {
+        private SettingString sut;
+
+        @Before
+        public void setUp() throws Exception {
+            sut = new SettingString("mytext", "foo", true);
+        }
+
+        @Test
+        public void whenSetUnconstrainedToNonNullValueThenSetValue() {
+            sut.set("a");
+
+            assertEquals("a", sut.value());
+        }
 
-        assertEquals("a", sut.value());
+        @Test
+        public void whenSetUnconstrainedToNullThenThrowAHelpfulError() {
+            IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
+                sut.set(null);
+            });
+            assertThat(ex.getMessage(), containsString("Setting \"mytext\" must be a String"));
+        }
     }
 }