Skip to content

Commit a4d0288

Browse files
joegallojoegallo
committed
Refactor IngestDocument's resolve method (elastic#125051)
1 parent 0b83425 commit a4d0288

File tree

3 files changed

+328
-245
lines changed

3 files changed

+328
-245
lines changed

‎server/src/main/java/org/elasticsearch/ingest/IngestCtxMap.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,17 @@
1717

1818
/**
1919
* Map containing ingest source and metadata.
20-
*
21-
* The Metadata values in {@link IngestDocument.Metadata} are validated when put in the map.
22-
* _index, _id and _routing must be a String or null
23-
* _version_type must be a lower case VersionType or null
24-
* _version must be representable as a long without loss of precision or null
25-
* _dynamic_templates must be a map
26-
* _if_seq_no must be a long or null
27-
* _if_primary_term must be a long or null
28-
*
20+
* <p>
21+
* The Metadata values in {@link IngestDocument.Metadata} are validated when put in the map:
22+
* <ul>
23+
* <li>{@code _index}, {@code _id} and {@code _routing} must be a String or null</li>
24+
* <li>{@code _version_type} must be a lower case VersionType or null</li>
25+
* <li>{@code _version} must be representable as a long without loss of precision or null</li>
26+
* <li>{@code _dynamic_templates} must be a map</li>
27+
* <li>{@code _if_seq_no} must be a long or null</li>
28+
* <li>{@code _if_primary_term} must be a long or null</li>
29+
* </ul>
30+
* <p>
2931
* The map is expected to be used by processors, server code should the typed getter and setters where possible.
3032
*/
3133
final class IngestCtxMap extends CtxMap<IngestDocMetadata> {

‎server/src/main/java/org/elasticsearch/ingest/IngestDocument.java‎

Lines changed: 86 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -193,17 +193,14 @@ public <T> T getFieldValue(String path, Class<T> clazz) {
193193
public <T> T getFieldValue(String path, Class<T> clazz, boolean ignoreMissing) {
194194
final FieldPath fieldPath = FieldPath.of(path);
195195
Object context = fieldPath.initialContext(this);
196-
for (String pathElement : fieldPath.pathElements) {
197-
ResolveResult result = resolve(pathElement, path, context);
198-
if (result.wasSuccessful) {
199-
context = result.resolvedObject;
200-
} else if (ignoreMissing && hasField(path) == false) {
201-
return null;
202-
} else {
203-
throw new IllegalArgumentException(result.errorMessage);
204-
}
196+
ResolveResult result = resolve(fieldPath.pathElements, fieldPath.pathElements.length, path, context);
197+
if (result.wasSuccessful) {
198+
return cast(path, result.resolvedObject, clazz);
199+
} else if (ignoreMissing) {
200+
return null;
201+
} else {
202+
throw new IllegalArgumentException(result.errorMessage);
205203
}
206-
return cast(path, context, clazz);
207204
}
208205

209206
/**
@@ -266,6 +263,8 @@ public boolean hasField(String path, boolean failOutOfRange) {
266263
String pathElement = fieldPath.pathElements[i];
267264
if (context == null) {
268265
return false;
266+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
267+
context = map.get(pathElement);
269268
} else if (context instanceof Map<?, ?> map) {
270269
context = map.get(pathElement);
271270
} else if (context instanceof List<?> list) {
@@ -292,6 +291,8 @@ public boolean hasField(String path, boolean failOutOfRange) {
292291
String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1];
293292
if (context == null) {
294293
return false;
294+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
295+
return map.containsKey(leafKey);
295296
} else if (context instanceof Map<?, ?> map) {
296297
return map.containsKey(leafKey);
297298
} else if (context instanceof List<?> list) {
@@ -322,18 +323,22 @@ public boolean hasField(String path, boolean failOutOfRange) {
322323
public void removeField(String path) {
323324
final FieldPath fieldPath = FieldPath.of(path);
324325
Object context = fieldPath.initialContext(this);
325-
for (int i = 0; i < fieldPath.pathElements.length - 1; i++) {
326-
ResolveResult result = resolve(fieldPath.pathElements[i], path, context);
327-
if (result.wasSuccessful) {
328-
context = result.resolvedObject;
329-
} else {
330-
throw new IllegalArgumentException(result.errorMessage);
331-
}
326+
ResolveResult result = resolve(fieldPath.pathElements, fieldPath.pathElements.length - 1, path, context);
327+
if (result.wasSuccessful) {
328+
context = result.resolvedObject;
329+
} else {
330+
throw new IllegalArgumentException(result.errorMessage);
332331
}
333332

334333
String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1];
335334
if (context == null) {
336335
throw new IllegalArgumentException(Errors.cannotRemove(path, leafKey, null));
336+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
337+
if (map.containsKey(leafKey)) {
338+
map.remove(leafKey);
339+
} else {
340+
throw new IllegalArgumentException(Errors.notPresent(path, leafKey));
341+
}
337342
} else if (context instanceof Map<?, ?> map) {
338343
if (map.containsKey(leafKey)) {
339344
map.remove(leafKey);
@@ -357,33 +362,48 @@ public void removeField(String path) {
357362
}
358363
}
359364

360-
private static ResolveResult resolve(String pathElement, String fullPath, Object context) {
361-
if (context == null) {
362-
return ResolveResult.error(Errors.cannotResolve(fullPath, pathElement, null));
363-
} else if (context instanceof Map<?, ?>) {
364-
@SuppressWarnings("unchecked")
365-
Map<String, Object> map = (Map<String, Object>) context;
366-
Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get
367-
if (object == NOT_FOUND) {
368-
return ResolveResult.error(Errors.notPresent(fullPath, pathElement));
369-
} else {
370-
return ResolveResult.success(object);
371-
}
372-
} else if (context instanceof List<?> list) {
373-
int index;
374-
try {
375-
index = Integer.parseInt(pathElement);
376-
} catch (NumberFormatException e) {
377-
return ResolveResult.error(Errors.notInteger(fullPath, pathElement));
378-
}
379-
if (index < 0 || index >= list.size()) {
380-
return ResolveResult.error(Errors.outOfBounds(fullPath, index, list.size()));
365+
/**
366+
* Resolves the path elements (up to the limit) within the context. The result of such resolution can either be successful,
367+
* or can indicate a failure.
368+
*/
369+
private static ResolveResult resolve(final String[] pathElements, final int limit, final String fullPath, Object context) {
370+
for (int i = 0; i < limit; i++) {
371+
String pathElement = pathElements[i];
372+
if (context == null) {
373+
return ResolveResult.error(Errors.cannotResolve(fullPath, pathElement, null));
374+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
375+
Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get
376+
if (object == NOT_FOUND) {
377+
return ResolveResult.error(Errors.notPresent(fullPath, pathElement));
378+
} else {
379+
context = object;
380+
}
381+
} else if (context instanceof Map<?, ?>) {
382+
@SuppressWarnings("unchecked")
383+
Map<String, Object> map = (Map<String, Object>) context;
384+
Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get
385+
if (object == NOT_FOUND) {
386+
return ResolveResult.error(Errors.notPresent(fullPath, pathElement));
387+
} else {
388+
context = object;
389+
}
390+
} else if (context instanceof List<?> list) {
391+
int index;
392+
try {
393+
index = Integer.parseInt(pathElement);
394+
} catch (NumberFormatException e) {
395+
return ResolveResult.error(Errors.notInteger(fullPath, pathElement));
396+
}
397+
if (index < 0 || index >= list.size()) {
398+
return ResolveResult.error(Errors.outOfBounds(fullPath, index, list.size()));
399+
} else {
400+
context = list.get(index);
401+
}
381402
} else {
382-
return ResolveResult.success(list.get(index));
403+
return ResolveResult.error(Errors.cannotResolve(fullPath, pathElement, context));
383404
}
384-
} else {
385-
return ResolveResult.error(Errors.cannotResolve(fullPath, pathElement, context));
386405
}
406+
return ResolveResult.success(context);
387407
}
388408

389409
/**
@@ -518,6 +538,15 @@ private void setFieldValue(String path, Object value, boolean append, boolean al
518538
String pathElement = fieldPath.pathElements[i];
519539
if (context == null) {
520540
throw new IllegalArgumentException(Errors.cannotResolve(path, pathElement, null));
541+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
542+
Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get
543+
if (object == NOT_FOUND) {
544+
Map<Object, Object> newMap = new HashMap<>();
545+
map.put(pathElement, newMap);
546+
context = newMap;
547+
} else {
548+
context = object;
549+
}
521550
} else if (context instanceof Map<?, ?>) {
522551
@SuppressWarnings("unchecked")
523552
Map<String, Object> map = (Map<String, Object>) context;
@@ -549,6 +578,22 @@ private void setFieldValue(String path, Object value, boolean append, boolean al
549578
String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1];
550579
if (context == null) {
551580
throw new IllegalArgumentException(Errors.cannotSet(path, leafKey, null));
581+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
582+
if (append) {
583+
Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get
584+
if (object == NOT_FOUND) {
585+
List<Object> list = new ArrayList<>();
586+
appendValues(list, value);
587+
map.put(leafKey, list);
588+
} else {
589+
Object list = appendValues(object, value, allowDuplicates);
590+
if (list != object) {
591+
map.put(leafKey, list);
592+
}
593+
}
594+
return;
595+
}
596+
map.put(leafKey, value);
552597
} else if (context instanceof Map<?, ?>) {
553598
@SuppressWarnings("unchecked")
554599
Map<String, Object> map = (Map<String, Object>) context;

0 commit comments

Comments
 (0)