Skip to content

Commit 41e0f1c

Browse files
joegallojoegallo
committed
Refactor IngestDocument's resolve method (elastic#125051)
1 parent 7cffc50 commit 41e0f1c

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
@@ -192,17 +192,14 @@ public <T> T getFieldValue(String path, Class<T> clazz) {
192192
public <T> T getFieldValue(String path, Class<T> clazz, boolean ignoreMissing) {
193193
final FieldPath fieldPath = FieldPath.of(path);
194194
Object context = fieldPath.initialContext(this);
195-
for (String pathElement : fieldPath.pathElements) {
196-
ResolveResult result = resolve(pathElement, path, context);
197-
if (result.wasSuccessful) {
198-
context = result.resolvedObject;
199-
} else if (ignoreMissing && hasField(path) == false) {
200-
return null;
201-
} else {
202-
throw new IllegalArgumentException(result.errorMessage);
203-
}
195+
ResolveResult result = resolve(fieldPath.pathElements, fieldPath.pathElements.length, path, context);
196+
if (result.wasSuccessful) {
197+
return cast(path, result.resolvedObject, clazz);
198+
} else if (ignoreMissing) {
199+
return null;
200+
} else {
201+
throw new IllegalArgumentException(result.errorMessage);
204202
}
205-
return cast(path, context, clazz);
206203
}
207204

208205
/**
@@ -265,6 +262,8 @@ public boolean hasField(String path, boolean failOutOfRange) {
265262
String pathElement = fieldPath.pathElements[i];
266263
if (context == null) {
267264
return false;
265+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
266+
context = map.get(pathElement);
268267
} else if (context instanceof Map<?, ?> map) {
269268
context = map.get(pathElement);
270269
} else if (context instanceof List<?> list) {
@@ -291,6 +290,8 @@ public boolean hasField(String path, boolean failOutOfRange) {
291290
String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1];
292291
if (context == null) {
293292
return false;
293+
} else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map
294+
return map.containsKey(leafKey);
294295
} else if (context instanceof Map<?, ?> map) {
295296
return map.containsKey(leafKey);
296297
} else if (context instanceof List<?> list) {
@@ -321,18 +322,22 @@ public boolean hasField(String path, boolean failOutOfRange) {
321322
public void removeField(String path) {
322323
final FieldPath fieldPath = FieldPath.of(path);
323324
Object context = fieldPath.initialContext(this);
324-
for (int i = 0; i < fieldPath.pathElements.length - 1; i++) {
325-
ResolveResult result = resolve(fieldPath.pathElements[i], path, context);
326-
if (result.wasSuccessful) {
327-
context = result.resolvedObject;
328-
} else {
329-
throw new IllegalArgumentException(result.errorMessage);
330-
}
325+
ResolveResult result = resolve(fieldPath.pathElements, fieldPath.pathElements.length - 1, path, context);
326+
if (result.wasSuccessful) {
327+
context = result.resolvedObject;
328+
} else {
329+
throw new IllegalArgumentException(result.errorMessage);
331330
}
332331

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

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

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

0 commit comments

Comments
 (0)