From cfe914b802de5e60ee02ca2b018d7f2a5d420bd6 Mon Sep 17 00:00:00 2001 From: John Plaisted Date: Fri, 4 Dec 2020 09:09:54 -0800 Subject: [PATCH] fix new modules since last rebase (#62) --- .../linkedin/metadata/dao/BaseQueryDAO.java | 2 +- .../metadata/dao/utils/ModelUtils.java | 1 - .../metadata/dao/browse/ESBrowseDAO.java | 16 ++++++----- ...CompleteQueryForHighCardinalityFields.java | 8 +++--- ...oCompleteQueryForLowCardinalityFields.java | 4 +-- .../metadata/dao/search/ESSearchDAO.java | 17 +++++++++--- .../metadata/dao/utils/SearchUtils.java | 4 +-- .../metadata/dao/browse/BrowseDAOTest.java | 27 ++++++++++++++----- .../metadata/dao/search/ESSearchDAOTest.java | 2 +- .../metadata/dao/utils/SearchUtilsTest.java | 2 +- .../asserts/BulkRequestsContainerAssert.java | 3 +-- 11 files changed, 57 insertions(+), 29 deletions(-) diff --git a/dao-api/src/main/java/com/linkedin/metadata/dao/BaseQueryDAO.java b/dao-api/src/main/java/com/linkedin/metadata/dao/BaseQueryDAO.java index 0829afed8..1daee6f5a 100644 --- a/dao-api/src/main/java/com/linkedin/metadata/dao/BaseQueryDAO.java +++ b/dao-api/src/main/java/com/linkedin/metadata/dao/BaseQueryDAO.java @@ -143,7 +143,7 @@ public static final class TraversalPath { * Finds a list of entities based on the given traversing paths. * * @param sourceEntityClass the source entity class as the starting point for the query. Must be a type defined in - * com.linkedin.metadata.entity. + * com.linkedin.metadata.entity. * @param sourceEntityFilter the filter to apply to the source entity when querying * @param traversePaths specify the traverse paths via a list of (relationship type, relationship filter, * intermediate entities) diff --git a/dao-api/src/main/java/com/linkedin/metadata/dao/utils/ModelUtils.java b/dao-api/src/main/java/com/linkedin/metadata/dao/utils/ModelUtils.java index 95de7f70d..9ca236b41 100644 --- a/dao-api/src/main/java/com/linkedin/metadata/dao/utils/ModelUtils.java +++ b/dao-api/src/main/java/com/linkedin/metadata/dao/utils/ModelUtils.java @@ -44,7 +44,6 @@ private ModelUtils() { * Gets the corresponding aspect name for a specific aspect type. * * @param aspectClass the aspect type - * @param must be a valid aspect type * @return the corresponding aspect name, which is actually the FQCN of type */ public static String getAspectName(@Nonnull Class> aspectClass) { diff --git a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/browse/ESBrowseDAO.java b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/browse/ESBrowseDAO.java index f2c6e4bd8..afe074366 100644 --- a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/browse/ESBrowseDAO.java +++ b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/browse/ESBrowseDAO.java @@ -47,9 +47,9 @@ @Slf4j public class ESBrowseDAO extends BaseBrowseDAO { private final RestHighLevelClient _client; - private final BaseBrowseConfig _config; + private final BaseBrowseConfig _config; - public ESBrowseDAO(@Nonnull RestHighLevelClient esClient, @Nonnull BaseBrowseConfig config) { + public ESBrowseDAO(@Nonnull RestHighLevelClient esClient, @Nonnull BaseBrowseConfig config) { this._client = esClient; this._config = config; } @@ -235,7 +235,7 @@ List extractEntitiesResponse(@Nonnull SearchResponse entitie final List entityMetadataArray = new ArrayList<>(); Arrays.stream(entitiesResponse.getHits().getHits()).forEach(hit -> { try { - final List allPaths = (List) hit.getSourceAsMap().get(_config.getBrowsePathFieldName()); + final List allPaths = (List) hit.getSourceAsMap().get(_config.getBrowsePathFieldName()); final String nextLevelPath = getNextLevelPath(allPaths, currentPath); if (nextLevelPath != null) { entityMetadataArray.add(new BrowseResultEntity().setName(getSimpleName(nextLevelPath)) @@ -263,10 +263,12 @@ private String getSimpleName(@Nonnull String path) { @VisibleForTesting @Nullable - static String getNextLevelPath(@Nonnull List paths, @Nonnull String currentPath) { + static String getNextLevelPath(@Nonnull List paths, @Nonnull String currentPath) { final String normalizedCurrentPath = currentPath.toLowerCase(); final int pathDepth = getPathDepth(currentPath); return paths.stream() + .filter(x -> x instanceof String) + .map(x -> (String) x) .filter(x -> x.toLowerCase().startsWith(normalizedCurrentPath) && getPathDepth(x) == (pathDepth + 1)) .findFirst() .orElse(null); @@ -298,10 +300,12 @@ public List getBrowsePaths(@Nonnull Urn urn) { if (searchHits.length == 0) { return Collections.emptyList(); } - final Map sourceMap = searchHits[0].getSourceAsMap(); + final Map sourceMap = searchHits[0].getSourceAsMap(); if (!sourceMap.containsKey(_config.getBrowsePathFieldName())) { return Collections.emptyList(); } - return (List) sourceMap.get(_config.getBrowsePathFieldName()); + @SuppressWarnings("unchecked") + final List paths = (List) sourceMap.get(_config.getBrowsePathFieldName()); + return paths; } } diff --git a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForHighCardinalityFields.java b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForHighCardinalityFields.java index b22563f9f..82523d140 100644 --- a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForHighCardinalityFields.java +++ b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForHighCardinalityFields.java @@ -22,9 +22,9 @@ @Slf4j public class ESAutoCompleteQueryForHighCardinalityFields extends BaseESAutoCompleteQuery { private static final Integer DEFAULT_AUTOCOMPLETE_QUERY_SIZE = 100; - private BaseSearchConfig _config; + private final BaseSearchConfig _config; - ESAutoCompleteQueryForHighCardinalityFields(BaseSearchConfig config) { + ESAutoCompleteQueryForHighCardinalityFields(BaseSearchConfig config) { this._config = config; } @@ -63,7 +63,7 @@ StringArray getSuggestionList(@Nonnull SearchResponse searchResponse, @Nonnull S @Nonnull String input, int limit) { Set autoCompletionList = new LinkedHashSet<>(); SearchHit[] hits = searchResponse.getHits().getHits(); - Integer count = 0; + int count = 0; for (SearchHit hit : hits) { Map source = hit.getSourceAsMap(); if (count >= limit) { @@ -97,7 +97,7 @@ static List decoupleArrayToGetSubstringMatch(@Nonnull Object fieldVal, @ if (!(fieldVal instanceof List)) { return Collections.singletonList(fieldVal.toString()); } - List stringVals = (List) fieldVal; + List stringVals = (List) fieldVal; return stringVals.stream() .map(Object::toString) .filter(x -> x.toLowerCase().contains(input.toLowerCase())) diff --git a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForLowCardinalityFields.java b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForLowCardinalityFields.java index 917b6012f..a5a0ce437 100644 --- a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForLowCardinalityFields.java +++ b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESAutoCompleteQueryForLowCardinalityFields.java @@ -23,9 +23,9 @@ public class ESAutoCompleteQueryForLowCardinalityFields extends BaseESAutoCompleteQuery { private static final String DEFAULT_QUERY_ANALYZER = "lowercase_keyword"; - private BaseSearchConfig _config; + private BaseSearchConfig _config; - ESAutoCompleteQueryForLowCardinalityFields(BaseSearchConfig config) { + ESAutoCompleteQueryForLowCardinalityFields(BaseSearchConfig config) { this._config = config; } diff --git a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESSearchDAO.java b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESSearchDAO.java index d817ef716..5d006853d 100644 --- a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESSearchDAO.java +++ b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/search/ESSearchDAO.java @@ -19,7 +19,6 @@ import com.linkedin.metadata.query.SearchResultMetadata; import com.linkedin.metadata.query.SortCriterion; import java.net.URISyntaxException; -import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -202,6 +201,7 @@ SearchRequest getFilteredSearchQuery(@Nullable Filter filters, @Nullable SortCri * @return a valid search request * @deprecated please use {@link #constructSearchQuery(String, Filter, SortCriterion, String, int, int)} instead */ + @Deprecated @Nonnull public SearchRequest constructSearchQuery(@Nonnull String input, @Nullable Filter filter, @Nullable SortCriterion sortCriterion, int from, int size) { @@ -319,8 +319,19 @@ DataMap buildDocumentsDataMap(@Nonnull Map objectMap) { final DataMap dataMap = new DataMap(); for (Map.Entry entry : objectMap.entrySet()) { - if (entry.getValue() instanceof ArrayList) { - dataMap.put(entry.getKey(), new DataList((ArrayList) entry.getValue())); + if (entry.getValue() instanceof List) { + final List values = (List) entry.getValue(); + final DataList dataList = new DataList(); + + for (Object value : values) { + if (value instanceof String) { + dataList.add(value); + } else { + log.error("Expected all values to be Strings, but found `{}`", value.getClass().getSimpleName()); + } + } + + dataMap.put(entry.getKey(), dataList); } else if (entry.getValue() != null) { dataMap.put(entry.getKey(), entry.getValue()); } diff --git a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/utils/SearchUtils.java b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/utils/SearchUtils.java index 769517660..0a32910c6 100644 --- a/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/utils/SearchUtils.java +++ b/dao-impl/elasticsearch-dao-7/src/main/java/com/linkedin/metadata/dao/utils/SearchUtils.java @@ -84,7 +84,7 @@ public static QueryBuilder getQueryBuilderFromCriterion(@Nonnull Criterion crite } @Nonnull - public static String toEntityType(@Nonnull Class c) { + public static String toEntityType(@Nonnull Class c) { String result = c.getSimpleName().toLowerCase(); if (result.endsWith("entity")) { result = result.substring(0, result.length() - 6); @@ -93,7 +93,7 @@ public static String toEntityType(@Nonnull Class c) { } @Nonnull - public static String readResourceFile(@Nonnull Class clazz, @Nonnull String filePath) { + public static String readResourceFile(@Nonnull Class clazz, @Nonnull String filePath) { try (InputStream inputStream = clazz.getClassLoader().getResourceAsStream(filePath)) { return IOUtils.toString(inputStream); } catch (IOException e) { diff --git a/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/browse/BrowseDAOTest.java b/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/browse/BrowseDAOTest.java index 433124b23..08431465e 100644 --- a/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/browse/BrowseDAOTest.java +++ b/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/browse/BrowseDAOTest.java @@ -20,7 +20,7 @@ public class BrowseDAOTest { - private BaseBrowseConfig _browseConfig; + private BaseBrowseConfig _browseConfig; private RestHighLevelClient _mockClient; private ESBrowseDAO _browseDAO; @@ -70,29 +70,44 @@ public void testMatchingPaths() { } @Test - public void testGetBrowsePath() throws Exception { + public void testEmptyGetBrowsePaths() throws Exception { SearchResponse mockSearchResponse = mock(SearchResponse.class); SearchHits mockSearchHits = mock(SearchHits.class); - SearchHit mockSearchHit = mock(SearchHit.class); Urn dummyUrn = TestUtils.makeUrn(0); - Map sourceMap = new HashMap<>(); // Test when there is no search hit for getBrowsePaths when(mockSearchHits.getHits()).thenReturn(new SearchHit[0]); when(mockSearchResponse.getHits()).thenReturn(mockSearchHits); when(_mockClient.search(any(), eq(RequestOptions.DEFAULT))).thenReturn(mockSearchResponse); assertEquals(_browseDAO.getBrowsePaths(dummyUrn).size(), 0); + } + + @Test + public void testGetBrowsePathMissingField() throws Exception { + SearchResponse mockSearchResponse = mock(SearchResponse.class); + SearchHits mockSearchHits = mock(SearchHits.class); + SearchHit mockSearchHit = mock(SearchHit.class); + Urn dummyUrn = TestUtils.makeUrn(0); + Map sourceMap = new HashMap<>(); // Test the case of single search hit & browsePaths field doesn't exist - sourceMap.remove(_browseConfig.getBrowsePathFieldName()); when(mockSearchHit.getSourceAsMap()).thenReturn(sourceMap); when(mockSearchHits.getHits()).thenReturn(new SearchHit[]{mockSearchHit}); when(mockSearchResponse.getHits()).thenReturn(mockSearchHits); when(_mockClient.search(any(), eq(RequestOptions.DEFAULT))).thenReturn(mockSearchResponse); assertEquals(_browseDAO.getBrowsePaths(dummyUrn).size(), 0); + } - // Test the case of single search hit & browsePaths field exists + @Test + public void testGetBrowsePathFoundField() throws Exception { + SearchResponse mockSearchResponse = mock(SearchResponse.class); + SearchHits mockSearchHits = mock(SearchHits.class); + SearchHit mockSearchHit = mock(SearchHit.class); + Urn dummyUrn = TestUtils.makeUrn(0); + Map sourceMap = new HashMap<>(); sourceMap.put(_browseConfig.getBrowsePathFieldName(), Collections.singletonList("foo")); + + // Test the case of single search hit & browsePaths field exists when(mockSearchHit.getSourceAsMap()).thenReturn(sourceMap); when(mockSearchHits.getHits()).thenReturn(new SearchHit[]{mockSearchHit}); when(mockSearchResponse.getHits()).thenReturn(mockSearchHits); diff --git a/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/search/ESSearchDAOTest.java b/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/search/ESSearchDAOTest.java index 2b5a09a5f..111c6cb2e 100644 --- a/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/search/ESSearchDAOTest.java +++ b/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/search/ESSearchDAOTest.java @@ -53,7 +53,7 @@ private static String loadJsonFromResource(String resourceName) throws IOExcepti @BeforeMethod public void setup() throws Exception { _testSearchConfig = new TestSearchConfig(); - _searchDAO = new ESSearchDAO(null, EntityDocument.class, _testSearchConfig); + _searchDAO = new ESSearchDAO<>(null, EntityDocument.class, _testSearchConfig); _esAutoCompleteQuery = new ESAutoCompleteQueryForHighCardinalityFields(_testSearchConfig); } diff --git a/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/utils/SearchUtilsTest.java b/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/utils/SearchUtilsTest.java index 79f1788dc..ac265db3b 100644 --- a/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/utils/SearchUtilsTest.java +++ b/dao-impl/elasticsearch-dao-7/src/test/java/com/linkedin/metadata/dao/utils/SearchUtilsTest.java @@ -21,7 +21,7 @@ public void testGetRequestMap() { assertTrue(actual1.isEmpty()); // Filter with criteria with default condition - final Map requestParams = Collections.unmodifiableMap(new HashMap() { + final Map requestParams = Collections.unmodifiableMap(new HashMap() { { put("key1", "value1"); put("key2", "value2"); diff --git a/testing/elasticsearch-dao-integ-testing/src/main/java/com/linkedin/metadata/testing/asserts/BulkRequestsContainerAssert.java b/testing/elasticsearch-dao-integ-testing/src/main/java/com/linkedin/metadata/testing/asserts/BulkRequestsContainerAssert.java index 2f835831f..940c32d38 100644 --- a/testing/elasticsearch-dao-integ-testing/src/main/java/com/linkedin/metadata/testing/asserts/BulkRequestsContainerAssert.java +++ b/testing/elasticsearch-dao-integ-testing/src/main/java/com/linkedin/metadata/testing/asserts/BulkRequestsContainerAssert.java @@ -9,7 +9,6 @@ import org.assertj.core.api.Assertions; import org.assertj.core.api.IterableAssert; import org.assertj.core.api.MapAssert; -import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.bulk.BulkRequest; @@ -28,7 +27,7 @@ public static BulkRequestsContainerAssert assertThat(@Nonnull BulkRequestsContai private static Collection getIds(Collection requests) { return requests.stream() .flatMap(request -> request.requests().stream()) - .map(DocWriteRequest::id) + .map(request -> request.id()) .collect(Collectors.toList()); }