Skip to content

Commit 5a45f97

Browse files
stu-elasticstu-elastic
committed
Don't clone properties, add MetadataTests
1 parent 9c7b389 commit 5a45f97

File tree

2 files changed

+210
-2
lines changed

2 files changed

+210
-2
lines changed

‎server/src/main/java/org/elasticsearch/script/Metadata.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public class Metadata {
5252

5353
public Metadata(Map<String, Object> map, Map<String, FieldProperty<?>> properties) {
5454
this.map = map;
55-
this.properties = properties;
55+
this.properties = Collections.unmodifiableMap(properties);
5656
validateMetadata();
5757
}
5858

@@ -223,7 +223,8 @@ public int size() {
223223

224224
@Override
225225
public Metadata clone() {
226-
return new Metadata(new HashMap<>(map), new HashMap<>(properties));
226+
// properties is an UnmodifiableMap, no need to create a copy
227+
return new Metadata(new HashMap<>(map), properties);
227228
}
228229

229230
/**

‎server/src/test/java/org/elasticsearch/script/MetadataTests.java

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@
1010

1111
import org.elasticsearch.test.ESTestCase;
1212

13+
import java.util.Collections;
1314
import java.util.HashMap;
1415
import java.util.Map;
16+
import java.util.Set;
1517

1618
public class MetadataTests extends ESTestCase {
1719
Metadata md;
20+
private static final Metadata.FieldProperty<String> STRING_PROP = new Metadata.FieldProperty<>(String.class, true, true, null);
1821

1922
public void testGetString() {
2023
Map<String, Object> metadata = new HashMap<>();
@@ -57,4 +60,208 @@ private static Map<String, Metadata.FieldProperty<?>> allowAllValidators(String.
5760
}
5861
return validators;
5962
}
63+
64+
public void testValidateMetadata() {
65+
IllegalArgumentException err = expectThrows(
66+
IllegalArgumentException.class,
67+
() -> new Metadata(Map.of("foo", 1), Map.of("foo", STRING_PROP))
68+
);
69+
assertEquals("foo [1] is wrong type, expected assignable to [java.lang.String], not [java.lang.Integer]", err.getMessage());
70+
71+
err = expectThrows(
72+
IllegalArgumentException.class,
73+
() -> new Metadata(Map.of("foo", "abc", "bar", "def"), Map.of("foo", STRING_PROP))
74+
);
75+
assertEquals("Unexpected metadata keys [bar:def]", err.getMessage());
76+
}
77+
78+
public void testIsAvailable() {
79+
md = new Metadata(Map.of("bar", "baz"), Map.of("foo", STRING_PROP, "bar", STRING_PROP));
80+
assertTrue(md.isAvailable("bar"));
81+
assertTrue(md.isAvailable("foo"));
82+
}
83+
84+
public void testPut() {
85+
md = new Metadata(new HashMap<>(Map.of("foo", "bar")), Map.of("foo", STRING_PROP));
86+
assertEquals("bar", md.get("foo"));
87+
md.put("foo", "baz");
88+
assertEquals("baz", md.get("foo"));
89+
md.put("foo", null);
90+
assertNull(md.get("foo"));
91+
IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> md.put("foo", 1));
92+
assertEquals("foo [1] is wrong type, expected assignable to [java.lang.String], not [java.lang.Integer]", err.getMessage());
93+
}
94+
95+
public void testContainsKey() {
96+
md = new Metadata(new HashMap<>(Map.of("foo", "bar")), Map.of("foo", STRING_PROP, "baz", STRING_PROP));
97+
assertTrue(md.containsKey("foo"));
98+
assertFalse(md.containsKey("baz"));
99+
assertTrue(md.isAvailable("baz"));
100+
}
101+
102+
public void testContainsValue() {
103+
md = new Metadata(new HashMap<>(Map.of("foo", "bar")), Map.of("foo", STRING_PROP, "baz", STRING_PROP));
104+
assertTrue(md.containsValue("bar"));
105+
assertFalse(md.containsValue("foo"));
106+
}
107+
108+
public void testRemove() {
109+
md = new Metadata(
110+
new HashMap<>(Map.of("foo", "bar", "baz", "qux")),
111+
Map.of("foo", STRING_PROP, "baz", new Metadata.FieldProperty<>(String.class, false, true, null))
112+
);
113+
assertTrue(md.containsKey("foo"));
114+
md.remove("foo");
115+
assertFalse(md.containsKey("foo"));
116+
117+
assertTrue(md.containsKey("baz"));
118+
IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> md.remove("baz"));
119+
assertEquals("baz cannot be removed", err.getMessage());
120+
}
121+
122+
public void testKeySetAndSize() {
123+
md = new Metadata(new HashMap<>(Map.of("foo", "bar", "baz", "qux")), Map.of("foo", STRING_PROP, "baz", STRING_PROP));
124+
Set<String> expected = Set.of("foo", "baz");
125+
assertEquals(expected, md.keySet());
126+
assertEquals(2, md.size());
127+
md.remove("foo");
128+
assertEquals(Set.of("baz"), md.keySet());
129+
assertEquals(1, md.size());
130+
md.put("foo", "abc");
131+
assertEquals(expected, md.keySet());
132+
assertEquals(2, md.size());
133+
md.remove("foo");
134+
md.remove("baz");
135+
assertEquals(Collections.emptySet(), md.keySet());
136+
assertEquals(0, md.size());
137+
}
138+
139+
public void testTestClone() {
140+
md = new Metadata(new HashMap<>(Map.of("foo", "bar", "baz", "qux")), Map.of("foo", STRING_PROP, "baz", STRING_PROP));
141+
Metadata md2 = md.clone();
142+
md2.remove("foo");
143+
md.remove("baz");
144+
assertEquals("bar", md.get("foo"));
145+
assertNull(md2.get("foo"));
146+
assertNull(md.get("baz"));
147+
assertEquals("qux", md2.get("baz"));
148+
}
149+
150+
public void testFieldPropertyType() {
151+
Metadata.FieldProperty<A> aProp = new Metadata.FieldProperty<>(A.class, true, true, null);
152+
aProp.check(Metadata.MapOperation.UPDATE, "a", new A());
153+
aProp.check(Metadata.MapOperation.INIT, "a", new A());
154+
aProp.check(Metadata.MapOperation.UPDATE, "a", new B());
155+
aProp.check(Metadata.MapOperation.INIT, "a", new B());
156+
157+
IllegalArgumentException err = expectThrows(
158+
IllegalArgumentException.class,
159+
() -> aProp.check(Metadata.MapOperation.UPDATE, "a", new C())
160+
);
161+
String expected = "a [I'm C] is wrong type, expected assignable to [org.elasticsearch.script.MetadataTests$A], not"
162+
+ " [org.elasticsearch.script.MetadataTests$C]";
163+
assertEquals(expected, err.getMessage());
164+
err = expectThrows(IllegalArgumentException.class, () -> aProp.check(Metadata.MapOperation.INIT, "a", new C()));
165+
assertEquals(expected, err.getMessage());
166+
}
167+
168+
static class A {}
169+
170+
static class B extends A {}
171+
172+
static class C {
173+
@Override
174+
public String toString() {
175+
return "I'm C";
176+
}
177+
}
178+
179+
public void testFieldPropertyNullable() {
180+
Metadata.FieldProperty<String> cantNull = new Metadata.FieldProperty<>(String.class, false, true, null);
181+
Metadata.FieldProperty<String> canNull = new Metadata.FieldProperty<>(String.class, true, true, null);
182+
183+
IllegalArgumentException err;
184+
{
185+
Metadata.MapOperation op = Metadata.MapOperation.INIT;
186+
err = expectThrows(IllegalArgumentException.class, () -> cantNull.check(op, "a", null));
187+
assertEquals("a cannot be null", err.getMessage());
188+
canNull.check(op, "a", null);
189+
}
190+
191+
{
192+
Metadata.MapOperation op = Metadata.MapOperation.UPDATE;
193+
err = expectThrows(IllegalArgumentException.class, () -> cantNull.check(op, "a", null));
194+
assertEquals("a cannot be null", err.getMessage());
195+
canNull.check(Metadata.MapOperation.UPDATE, "a", null);
196+
}
197+
198+
{
199+
Metadata.MapOperation op = Metadata.MapOperation.REMOVE;
200+
err = expectThrows(IllegalArgumentException.class, () -> cantNull.check(op, "a", null));
201+
assertEquals("a cannot be removed", err.getMessage());
202+
canNull.check(Metadata.MapOperation.REMOVE, "a", null);
203+
}
204+
}
205+
206+
public void testFieldPropertyWritable() {
207+
Metadata.FieldProperty<String> writable = new Metadata.FieldProperty<>(String.class, true, true, null);
208+
Metadata.FieldProperty<String> readonly = new Metadata.FieldProperty<>(String.class, true, false, null);
209+
210+
String key = "a";
211+
String value = "abc";
212+
213+
IllegalArgumentException err;
214+
{
215+
Metadata.MapOperation op = Metadata.MapOperation.INIT;
216+
writable.check(op, key, value);
217+
readonly.check(op, key, value);
218+
}
219+
220+
{
221+
Metadata.MapOperation op = Metadata.MapOperation.UPDATE;
222+
err = expectThrows(IllegalArgumentException.class, () -> readonly.check(op, key, value));
223+
assertEquals("a cannot be updated", err.getMessage());
224+
writable.check(op, key, value);
225+
}
226+
227+
{
228+
Metadata.MapOperation op = Metadata.MapOperation.REMOVE;
229+
err = expectThrows(IllegalArgumentException.class, () -> readonly.check(op, key, value));
230+
assertEquals("a cannot be removed", err.getMessage());
231+
writable.check(op, key, value);
232+
}
233+
}
234+
235+
public void testFieldPropertyExtendedValidation() {
236+
Metadata.FieldProperty<Number> any = new Metadata.FieldProperty<>(Number.class, true, true, null);
237+
Metadata.FieldProperty<Number> odd = new Metadata.FieldProperty<>(Number.class, true, true, (k, v) -> {
238+
if (v.intValue() % 2 == 0) {
239+
throw new IllegalArgumentException("not odd");
240+
}
241+
});
242+
243+
String key = "a";
244+
int value = 2;
245+
246+
IllegalArgumentException err;
247+
{
248+
Metadata.MapOperation op = Metadata.MapOperation.INIT;
249+
any.check(op, key, value);
250+
err = expectThrows(IllegalArgumentException.class, () -> odd.check(op, key, value));
251+
assertEquals("not odd", err.getMessage());
252+
}
253+
254+
{
255+
Metadata.MapOperation op = Metadata.MapOperation.UPDATE;
256+
any.check(op, key, value);
257+
err = expectThrows(IllegalArgumentException.class, () -> odd.check(op, key, value));
258+
assertEquals("not odd", err.getMessage());
259+
}
260+
261+
{
262+
Metadata.MapOperation op = Metadata.MapOperation.REMOVE;
263+
any.check(op, key, value);
264+
odd.check(op, key, value);
265+
}
266+
}
60267
}

0 commit comments

Comments
 (0)