Skip to content

Commit 7026815

Browse files
committed
Further refactoring
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
1 parent e79b84f commit 7026815

File tree

2 files changed

+33
-56
lines changed

2 files changed

+33
-56
lines changed

src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGenerator.java

+33-52
Original file line numberDiff line numberDiff line change
@@ -345,79 +345,60 @@ String buildFieldDataString(
345345
}
346346

347347
String getFieldType(String fieldName, Map<String, Object> propertiesAsMap, Index index) {
348+
// Attempt to get field type from cache
348349
String fieldType = getFieldTypeFromCache(fieldName, index);
349-
if (fieldType != null) {
350-
if (fieldType.equals(NO_FIELD_TYPE_VALUE)) {
351-
return null;
352-
} else {
353-
return fieldType;
354-
}
355-
}
356350

357-
fieldType = getFieldTypeFromMapping(index, fieldName, propertiesAsMap);
358351
if (fieldType != null) {
359-
String finalFieldType = fieldType;
360-
fieldTypeMap.computeIfAbsent(index, k -> new ConcurrentHashMap<>()).computeIfAbsent(fieldName, k -> finalFieldType);
361352
return fieldType;
362353
}
363-
return null;
364-
}
365354

366-
String getFieldTypeFromMapping(Index index, String fieldName, Map<String, Object> propertiesAsMap) {
367-
// Recursively find the field type from properties
368-
if (propertiesAsMap != null) {
369-
String fieldType = findFieldTypeInProperties(propertiesAsMap, fieldName.split("\\."), 0);
355+
// Retrieve field type from mapping and cache it if found
356+
fieldType = getFieldTypeFromProperties(fieldName, propertiesAsMap);
370357

371-
// Cache the NO_FIELD_TYPE_VALUE result if no field type is found in this index
372-
if (fieldType == null) {
373-
fieldTypeMap.computeIfAbsent(index, k -> new ConcurrentHashMap<>()).computeIfAbsent(fieldName, k -> fieldType);
374-
}
375-
return fieldType;
376-
}
377-
return null;
358+
// Cache field type or NO_FIELD_TYPE_VALUE if not found
359+
fieldTypeMap.computeIfAbsent(index, k -> new ConcurrentHashMap<>())
360+
.putIfAbsent(fieldName, fieldType != null ? fieldType : NO_FIELD_TYPE_VALUE);
361+
362+
return fieldType;
378363
}
379364

380-
private String findFieldTypeInProperties(Map<String, ?> properties, String[] fieldParts, int index) {
381-
if (index >= fieldParts.length) {
365+
String getFieldTypeFromProperties(String fieldName, Map<String, Object> propertiesAsMap) {
366+
if (propertiesAsMap == null) {
382367
return null;
383368
}
384369

385-
String currentField = fieldParts[index];
386-
Object currentMapping = properties.get(currentField);
370+
String[] fieldParts = fieldName.split("\\.");
371+
Map<String, ?> currentProperties = propertiesAsMap;
387372

388-
// Check if current mapping is a map and contains further nested properties or a type
389-
if (currentMapping instanceof Map) {
390-
Map<String, ?> currentMappingMap = (Map<String, ?>) currentMapping;
373+
for (int depth = 0; depth < fieldParts.length; depth++) {
374+
Object currentMapping = currentProperties.get(fieldParts[depth]);
391375

392-
// If it has a 'properties' key, go into the nested object
393-
if (currentMappingMap.containsKey("properties")) {
394-
Map<String, ?> nestedProperties = (Map<String, ?>) currentMappingMap.get("properties");
395-
return findFieldTypeInProperties(nestedProperties, fieldParts, index + 1);
396-
}
397-
398-
// If it has a 'fields' key, handle multifields (e.g., title.raw) and is not the parent field (parent field case handled below)
399-
if (currentMappingMap.containsKey("fields") && index + 1 < fieldParts.length) {
400-
Map<String, ?> fieldsMap = (Map<String, ?>) currentMappingMap.get("fields");
401-
return findFieldTypeInProperties(fieldsMap, fieldParts, index + 1); // Recursively check subfield
402-
}
376+
if (currentMapping instanceof Map) {
377+
Map<String, ?> currentMap = (Map<String, ?>) currentMapping;
403378

404-
// If it has a 'type' key, return the type. Also handles parent field case in multifields
405-
if (currentMappingMap.containsKey("type")) {
406-
return currentMappingMap.get("type").toString();
379+
// Navigate into nested properties if available
380+
if (currentMap.containsKey("properties")) {
381+
currentProperties = (Map<String, ?>) currentMap.get("properties");
382+
}
383+
// Handle multifields (e.g., title.raw)
384+
else if (currentMap.containsKey("fields") && depth + 1 < fieldParts.length) {
385+
currentProperties = (Map<String, ?>) currentMap.get("fields");
386+
}
387+
// Return type if found
388+
else if (currentMap.containsKey("type")) {
389+
return (String) currentMap.get("type");
390+
} else {
391+
return null;
392+
}
393+
} else {
394+
return null;
407395
}
408396
}
409397

410398
return null;
411399
}
412400

413401
String getFieldTypeFromCache(String fieldName, Index index) {
414-
Map<String, String> indexMap = fieldTypeMap.get(index);
415-
if (indexMap != null) {
416-
String fieldType = indexMap.get(fieldName);
417-
if (fieldType != null) {
418-
return fieldType;
419-
}
420-
}
421-
return null;
402+
return fieldTypeMap.getOrDefault(index, new ConcurrentHashMap<>()).get(fieldName);
422403
}
423404
}

src/test/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGeneratorTests.java

-4
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import static org.mockito.Mockito.atLeastOnce;
1414
import static org.mockito.Mockito.mock;
1515
import static org.mockito.Mockito.spy;
16-
import static org.mockito.Mockito.times;
1716
import static org.mockito.Mockito.verify;
1817
import static org.mockito.Mockito.when;
1918
import static org.opensearch.index.query.QueryBuilders.boolQuery;
@@ -597,11 +596,8 @@ public void testFieldTypeCachingForNonExistentField() throws IOException {
597596

598597
verify(queryShapeGeneratorSpy, atLeastOnce()).getFieldTypeFromCache(eq(nonExistentField), any(Index.class));
599598

600-
verify(queryShapeGeneratorSpy, atLeastOnce()).getFieldTypeFromMapping(any(Index.class), eq(nonExistentField), any());
601-
602599
queryShapeGeneratorSpy.buildShape(sourceBuilder, true, true, successfulSearchShardIndices);
603600

604-
verify(queryShapeGeneratorSpy, times(2)).getFieldTypeFromMapping(any(Index.class), eq(nonExistentField), any());
605601
verify(queryShapeGeneratorSpy, atLeastOnce()).getFieldTypeFromCache(eq(nonExistentField), any(Index.class));
606602
}
607603

0 commit comments

Comments
 (0)