Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

Commit

Permalink
fix: turn on werror for elasticsearch-dao (linkedin#59)
Browse files Browse the repository at this point in the history
  • Loading branch information
John Plaisted committed Dec 4, 2020
1 parent 36c7248 commit 3515061
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 31 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def wErrorProjects = [
project(':core-models'),
project(':dao-api'),
project(':dao-impl:ebean-dao'),
// project(':dao-impl:elasticsearch-dao'),
project(':dao-impl:elasticsearch-dao'),
// project(':dao-impl:neo4j-dao'),
project(':restli-resources'),
project(':testing'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,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;
}
Expand Down Expand Up @@ -233,7 +233,7 @@ List<BrowseResultEntity> extractEntitiesResponse(@Nonnull SearchResponse entitie
final List<BrowseResultEntity> entityMetadataArray = new ArrayList<>();
Arrays.stream(entitiesResponse.getHits().getHits()).forEach(hit -> {
try {
final List<String> allPaths = (List<String>) 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))
Expand Down Expand Up @@ -261,10 +261,12 @@ private String getSimpleName(@Nonnull String path) {

@VisibleForTesting
@Nullable
static String getNextLevelPath(@Nonnull List<String> 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);
Expand Down Expand Up @@ -296,10 +298,13 @@ public List<String> getBrowsePaths(@Nonnull Urn urn) {
if (searchHits.length == 0) {
return Collections.emptyList();
}
final Map sourceMap = searchHits[0].getSourceAsMap();
final Map<String, Object> sourceMap = searchHits[0].getSourceAsMap();
if (!sourceMap.containsKey(_config.getBrowsePathFieldName())) {
return Collections.emptyList();
}
return (List<String>) sourceMap.get(_config.getBrowsePathFieldName());

@SuppressWarnings("unchecked")
final List<String> paths = (List<String>) sourceMap.get(_config.getBrowsePathFieldName());
return paths;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -63,7 +63,7 @@ StringArray getSuggestionList(@Nonnull SearchResponse searchResponse, @Nonnull S
@Nonnull String input, int limit) {
Set<String> autoCompletionList = new LinkedHashSet<>();
SearchHit[] hits = searchResponse.getHits().getHits();
Integer count = 0;
int count = 0;
for (SearchHit hit : hits) {
Map<String, Object> source = hit.getSource();
if (count >= limit) {
Expand Down Expand Up @@ -97,7 +97,7 @@ static List<String> decoupleArrayToGetSubstringMatch(@Nonnull Object fieldVal, @
if (!(fieldVal instanceof List)) {
return Collections.singletonList(fieldVal.toString());
}
List<Object> stringVals = (List<Object>) fieldVal;
final List<?> stringVals = (List<?>) fieldVal;
return stringVals.stream()
.map(Object::toString)
.filter(x -> x.toLowerCase().contains(input.toLowerCase()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
public class ESAutoCompleteQueryForLowCardinalityFields extends BaseESAutoCompleteQuery {

private static final String DEFAULT_QUERY_ANALYZER = "lowercase_keyword";
private BaseSearchConfig _config;
private final BaseSearchConfig<?> _config;

ESAutoCompleteQueryForLowCardinalityFields(BaseSearchConfig config) {
ESAutoCompleteQueryForLowCardinalityFields(BaseSearchConfig<?> config) {
this._config = config;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -201,6 +200,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) {
Expand Down Expand Up @@ -315,11 +315,21 @@ List<DOCUMENT> getDocuments(@Nonnull SearchResponse searchResponse) {
*/
@Nonnull
DataMap buildDocumentsDataMap(@Nonnull Map<String, Object> objectMap) {

final DataMap dataMap = new DataMap();
for (Map.Entry<String, Object> entry : objectMap.entrySet()) {
if (entry.getValue() instanceof ArrayList) {
dataMap.put(entry.getKey(), new DataList((ArrayList<String>) 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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,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);
Expand All @@ -103,11 +103,10 @@ 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) {
log.error("Can't read file: " + filePath);
throw new RuntimeException("Can't read file: " + filePath);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.linkedin.testing.TestUtils;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.elasticsearch.action.search.SearchResponse;
Expand All @@ -18,7 +19,7 @@


public class BrowseDAOTest {
private BaseBrowseConfig _browseConfig;
private BaseBrowseConfig<?> _browseConfig;
private RestHighLevelClient _mockClient;
private ESBrowseDAO _browseDAO;

Expand Down Expand Up @@ -68,31 +69,45 @@ 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 mockSourceMap = mock(Map.class);

// 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())).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<String, Object> sourceMap = new HashMap<>();

// Test the case of single search hit & browsePaths field doesn't exist
when(mockSourceMap.containsKey(_browseConfig.getBrowsePathFieldName())).thenReturn(false);
when(mockSearchHit.getSourceAsMap()).thenReturn(mockSourceMap);
when(mockSearchHit.getSourceAsMap()).thenReturn(sourceMap);
when(mockSearchHits.getHits()).thenReturn(new SearchHit[]{mockSearchHit});
when(mockSearchResponse.getHits()).thenReturn(mockSearchHits);
when(_mockClient.search(any())).thenReturn(mockSearchResponse);
assertEquals(_browseDAO.getBrowsePaths(dummyUrn).size(), 0);
}

@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<String, Object> sourceMap = new HashMap<>();
sourceMap.put(_browseConfig.getBrowsePathFieldName(), Collections.singletonList("foo"));

// Test the case of single search hit & browsePaths field exists
when(mockSourceMap.containsKey(_browseConfig.getBrowsePathFieldName())).thenReturn(true);
when(mockSourceMap.get(_browseConfig.getBrowsePathFieldName())).thenReturn(Collections.singletonList("foo"));
when(mockSearchHit.getSourceAsMap()).thenReturn(mockSourceMap);
when(mockSearchHit.getSourceAsMap()).thenReturn(sourceMap);
when(mockSearchHits.getHits()).thenReturn(new SearchHit[]{mockSearchHit});
when(mockSearchResponse.getHits()).thenReturn(mockSearchHits);
when(_mockClient.search(any())).thenReturn(mockSearchResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> requestParams = Collections.unmodifiableMap(new HashMap<String, String>() {
{
put("key1", "value1");
put("key2", "value2");
Expand Down

0 comments on commit 3515061

Please sign in to comment.