Skip to content

Commit

Permalink
feat: add new search sanity tests to ensure that query templates are …
Browse files Browse the repository at this point in the history
…valid (#70)


Also fix the ES 7 docker setup, as it was incorrect (which, side note, means that our tests were not being run in GitHub actions? TODO to investigate that)

Also also er comment from Vidya add a createBrowseDao method
  • Loading branch information
John Plaisted authored Jan 21, 2021
1 parent 203ee1b commit f13da1c
Show file tree
Hide file tree
Showing 13 changed files with 531 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import com.linkedin.common.urn.Urn;
import com.linkedin.data.template.RecordTemplate;
import com.linkedin.metadata.dao.SearchResult;
import com.linkedin.metadata.dao.search.BaseSearchConfig;
import javax.annotation.Nonnull;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;

import static com.linkedin.metadata.testing.asserts.SearchIndexAssert.assertThat;
Expand All @@ -18,10 +20,12 @@
public abstract class BaseSearchSanityTests<T extends RecordTemplate> {
private final Urn _urn;
private final T _data;
private final BaseSearchConfig<T> _searchConfig;

protected BaseSearchSanityTests(@Nonnull Urn urn, @Nonnull T data) {
protected BaseSearchSanityTests(@Nonnull Urn urn, @Nonnull T data, @Nonnull BaseSearchConfig<T> searchConfig) {
_urn = urn;
_data = data;
_searchConfig = searchConfig;
}

/**
Expand Down Expand Up @@ -61,4 +65,35 @@ public void canReadDocuments() throws Exception {
assertThat(result).documents().containsExactly(_data);
assertThat(result).urns().containsExactly(_urn);
}

@Test
public void canCreateDaoWithConfig() {
Assertions.assertThatCode(() -> getIndex().createReadDao(_searchConfig)).doesNotThrowAnyException();
}

@Test
public void canUseSearchQueryTemplate() throws Exception {
// given
getIndex().getWriteDao().upsertDocument(_data, _urn.toString());
getIndex().getRequestContainer().flushAndSettle();

Assertions.assertThatCode(() -> {
// when
getIndex().createReadDao(_searchConfig).search("*", null, null, 0, 1);
// then
}).doesNotThrowAnyException();
}

@Test
public void canUseAutocompleteTemplate() throws Exception {
// given
getIndex().getWriteDao().upsertDocument(_data, _urn.toString());
getIndex().getRequestContainer().flushAndSettle();

Assertions.assertThatCode(() -> {
// when
getIndex().createReadDao(_searchConfig).autoComplete("*", null, null, 1);
// then
}).doesNotThrowAnyException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.linkedin.data.template.RecordTemplate;
import com.linkedin.metadata.dao.SearchResult;
import com.linkedin.metadata.dao.browse.BaseBrowseConfig;
import com.linkedin.metadata.dao.browse.ESBrowseDAO;
import com.linkedin.metadata.dao.search.BaseSearchConfig;
import com.linkedin.metadata.dao.search.ESBulkWriterDAO;
import com.linkedin.metadata.dao.search.ESSearchDAO;
Expand Down Expand Up @@ -83,6 +85,14 @@ public ESSearchDAO<DOCUMENT> createReadDao(@Nonnull BaseSearchConfig<DOCUMENT> c
new TestSearchConfig<>(config, _name));
}

/**
* Creates a new {@link ESBrowseDAO} with the given configuration that will talk to this index.
*/
@Nonnull
public ESBrowseDAO createBrowseDao(@Nonnull BaseBrowseConfig<DOCUMENT> config) {
return new ESBrowseDAO(_connection.getRestHighLevelClient(), new TestBrowseConfig<>(config, _name));
}

/**
* Creates a new {@link ESSearchDAO} with will talk to this index, and where all search and autocomplete queries will
* return all documents.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package com.linkedin.metadata.testing;

import com.linkedin.data.template.RecordTemplate;
import com.linkedin.metadata.dao.browse.BaseBrowseConfig;
import javax.annotation.Nonnull;


/**
* Browse config for use in testing, which can wrap another config but use a different index name.
*
* <p>Useful for tests which want to name the index after the test, not the document.
*/
final class TestBrowseConfig<DOCUMENT extends RecordTemplate> extends BaseBrowseConfig<DOCUMENT> {
private final BaseBrowseConfig<DOCUMENT> _wrapped;
private final String _indexName;

/**
* Constructor.
*
* @param wrapped the config to wrap
* @param indexName the ES index to use
*/
TestBrowseConfig(@Nonnull BaseBrowseConfig<DOCUMENT> wrapped, @Nonnull String indexName) {
_wrapped = wrapped;
_indexName = indexName;
}

@Override
public Class<DOCUMENT> getSearchDocument() {
return _wrapped.getSearchDocument();
}

@Nonnull
@Override
public String getBrowseDepthFieldName() {
return _wrapped.getBrowseDepthFieldName();
}

@Nonnull
@Override
public String getBrowsePathFieldName() {
return _wrapped.getBrowsePathFieldName();
}

@Nonnull
@Override
public String getUrnFieldName() {
return _wrapped.getUrnFieldName();
}

@Nonnull
@Override
public String getSortingField() {
return _wrapped.getSortingField();
}

@Nonnull
@Override
public String getRemovedField() {
return _wrapped.getRemovedField();
}

@Override
public boolean hasFieldInSchema(@Nonnull String fieldName) {
return _wrapped.hasFieldInSchema(fieldName);
}

@Nonnull
@Override
public String getIndexName() {
return _indexName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,36 @@
import javax.annotation.Nullable;


/**
* Search config for use in testing, which can wrap another config but use a different index name.
*
* <p>Useful for tests which want to name the index after the test, not the document.
*/
final class TestSearchConfig<DOCUMENT extends RecordTemplate> extends BaseSearchConfig<DOCUMENT> {
private final BaseSearchConfig<DOCUMENT> _wrapper;
private final BaseSearchConfig<DOCUMENT> _wrapped;
private final String _indexName;

TestSearchConfig(BaseSearchConfig<DOCUMENT> wrapper, String indexName) {
_wrapper = wrapper;
/**
* Constructor.
*
* @param wrapped the config to wrap
* @param indexName the ES index to use
*/
TestSearchConfig(@Nonnull BaseSearchConfig<DOCUMENT> wrapped, @Nonnull String indexName) {
_wrapped = wrapped;
_indexName = indexName;
}

@Nonnull
@Override
public Set<String> getFacetFields() {
return _wrapper.getFacetFields();
return _wrapped.getFacetFields();
}

@Nullable
@Override
public Set<String> getLowCardinalityFields() {
return _wrapper.getLowCardinalityFields();
return _wrapped.getLowCardinalityFields();
}

@Nonnull
Expand All @@ -37,24 +48,24 @@ public String getIndexName() {
@Nonnull
@Override
public Class<DOCUMENT> getSearchDocument() {
return _wrapper.getSearchDocument();
return _wrapped.getSearchDocument();
}

@Nonnull
@Override
public String getDefaultAutocompleteField() {
return _wrapper.getDefaultAutocompleteField();
return _wrapped.getDefaultAutocompleteField();
}

@Nonnull
@Override
public String getSearchQueryTemplate() {
return _wrapper.getSearchQueryTemplate();
return _wrapped.getSearchQueryTemplate();
}

@Nonnull
@Override
public String getAutocompleteQueryTemplate() {
return _wrapper.getAutocompleteQueryTemplate();
return _wrapped.getAutocompleteQueryTemplate();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
@ElasticsearchContainerFactory.Implementation
public final class ElasticsearchContainerFactoryDockerImpl implements ElasticsearchContainerFactory {
private static final String IMAGE_NAME = "docker.elastic.co/elasticsearch/elasticsearch:5.6.8";
private static final String IMAGE_NAME = "docker.elastic.co/elasticsearch/elasticsearch:7.9.3";
private static final int HTTP_PORT = 9200;

/**
Expand Down Expand Up @@ -45,7 +45,8 @@ private static RestHighLevelClient buildRestClient(@Nonnull GenericContainerImpl
public ElasticsearchConnection start() throws Exception {
if (_container == null) {
_container = new GenericContainerImpl(IMAGE_NAME).withExposedPorts(HTTP_PORT)
.withEnv("xpack.security.enabled", "false");
.withEnv("xpack.security.enabled", "false")
.withEnv("discovery.type", "single-node");
_container.start();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package com.linkedin.metadata.testing;

import com.linkedin.metadata.dao.exception.ESQueryException;
import com.linkedin.metadata.dao.search.BaseSearchConfig;
import com.linkedin.metadata.dao.search.ESSearchDAO;
import com.linkedin.metadata.testing.annotations.SearchIndexMappings;
import com.linkedin.metadata.testing.annotations.SearchIndexSettings;
import com.linkedin.metadata.testing.annotations.SearchIndexType;
import com.linkedin.testing.BarSearchDocument;
import java.util.Collections;
import java.util.Set;
import javax.annotation.Nonnull;
import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.rest.RestStatus;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.*;


@ElasticsearchIntegrationTest
public class BadSearchConfigTest {
@SearchIndexType(BarSearchDocument.class)
@SearchIndexSettings("/settings.json")
@SearchIndexMappings("/mappings.json")
public SearchIndex<BarSearchDocument> _searchIndex;

private static final class SearchConfig extends BaseSearchConfig<BarSearchDocument> {

@Nonnull
@Override
public Set<String> getFacetFields() {
return Collections.emptySet();
}

@Nonnull
@Override
public Class<BarSearchDocument> getSearchDocument() {
return BarSearchDocument.class;
}

@Nonnull
@Override
public String getDefaultAutocompleteField() {
return "";
}

@Nonnull
@Override
public String getSearchQueryTemplate() {
return "invalid";
}

@Nonnull
@Override
public String getAutocompleteQueryTemplate() {
return "invalid";
}
}

/**
* Tests that invalid query templates cause an exception when searching via the DAO, ensuring that {@link
* BaseSearchSanityTests#canUseSearchQueryTemplate()} ()} is valid test.
*/
@Test
public void invalidSearchQueryTemplateDoesThrowOnSearch() {
// given
final ESSearchDAO<BarSearchDocument> dao = _searchIndex.createReadDao(new SearchConfig());

// then
final Throwable thrown = catchThrowable(() -> dao.search("", null, null, 0, 1));
assertThat(thrown).isInstanceOf(ESQueryException.class).hasCauseInstanceOf(ElasticsearchStatusException.class);
assertThat(((ElasticsearchStatusException) thrown.getCause()).status()).isEqualTo(RestStatus.BAD_REQUEST);
}

/**
* Tests that invalid query templates cause an exception when searching via the DAO, ensuring that {@link
* BaseSearchSanityTests#canUseAutocompleteTemplate()} is valid test.
*/
@Test
public void invalidAutocompleteQueryTemplateDoesThrowOnSearch() {
// given
final ESSearchDAO<BarSearchDocument> dao = _searchIndex.createReadDao(new SearchConfig());

// then
final Throwable thrown = catchThrowable(() -> dao.autoComplete("", null, null, 1));
assertThat(thrown).isInstanceOf(ESQueryException.class).hasCauseInstanceOf(ElasticsearchStatusException.class);
assertThat(((ElasticsearchStatusException) thrown.getCause()).status()).isEqualTo(RestStatus.BAD_REQUEST);
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.linkedin.metadata.testing;

import com.linkedin.metadata.dao.search.BaseSearchConfig;
import com.linkedin.metadata.testing.annotations.SearchIndexMappings;
import com.linkedin.metadata.testing.annotations.SearchIndexSettings;
import com.linkedin.metadata.testing.annotations.SearchIndexType;
import com.linkedin.testing.BarSearchDocument;
import com.linkedin.testing.urn.BarUrn;
import java.util.Collections;
import java.util.Set;
import javax.annotation.Nonnull;


Expand All @@ -17,8 +20,40 @@ public class ExampleSanityTest extends BaseSearchSanityTests<BarSearchDocument>
private static final BarUrn URN = new BarUrn(42);
private static final BarSearchDocument DOCUMENT = new BarSearchDocument().setUrn(URN).setValue("value");

private static final class SearchConfig extends BaseSearchConfig<BarSearchDocument> {
@Nonnull
@Override
public Set<String> getFacetFields() {
return Collections.emptySet();
}

@Nonnull
@Override
public Class<BarSearchDocument> getSearchDocument() {
return BarSearchDocument.class;
}

@Nonnull
@Override
public String getDefaultAutocompleteField() {
return "";
}

@Nonnull
@Override
public String getSearchQueryTemplate() {
return "{ \"query_string\": { \"query\": \"$INPUT\" }}";
}

@Nonnull
@Override
public String getAutocompleteQueryTemplate() {
return "{ \"query_string\": { \"query\": \"~$INPUT\" }}";
}
}

protected ExampleSanityTest() {
super(URN, DOCUMENT);
super(URN, DOCUMENT, new SearchConfig());
}

@Nonnull
Expand Down
Loading

0 comments on commit f13da1c

Please sign in to comment.