diff --git a/src/javaRestTest/java/com/o19s/es/ltr/action/BaseIntegrationTest.java b/src/javaRestTest/java/com/o19s/es/ltr/action/BaseIntegrationTest.java index 219e2318..463e0db3 100644 --- a/src/javaRestTest/java/com/o19s/es/ltr/action/BaseIntegrationTest.java +++ b/src/javaRestTest/java/com/o19s/es/ltr/action/BaseIntegrationTest.java @@ -107,7 +107,7 @@ public E getElement(Class clazz, String type, Str } public E getElement(Class clazz, String type, String name, String store) throws IOException { - return new IndexFeatureStore(store, this::client, parserFactory()).getAndParse(name, clazz, type); + return new IndexFeatureStore(store, this::client, parserFactory()).getAndParse(name, clazz, type).get(); } protected LtrRankerParserFactory parserFactory() { diff --git a/src/main/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStore.java b/src/main/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStore.java index 24a7fe45..da66e1a8 100644 --- a/src/main/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStore.java +++ b/src/main/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStore.java @@ -28,6 +28,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.util.BytesRef; +import org.opensearch.ResourceNotFoundException; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.get.GetResponse; import org.opensearch.client.Client; @@ -53,6 +54,7 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Objects; +import java.util.Optional; import java.util.function.Supplier; import java.util.regex.Pattern; @@ -108,13 +110,21 @@ public String getStoreName() { } @Override - public Feature load(String name) throws IOException { - return getAndParse(name, StoredFeature.class, StoredFeature.TYPE).optimize(); + public Feature load(final String name) throws IOException { + return getAndParse(name, StoredFeature.class, StoredFeature.TYPE) + .orElseThrow( + () -> new ResourceNotFoundException("Unknown feature [" + name + "]") + ) + .optimize(); } @Override - public FeatureSet loadSet(String name) throws IOException { - return getAndParse(name, StoredFeatureSet.class, StoredFeatureSet.TYPE).optimize(); + public FeatureSet loadSet(final String name) throws IOException { + return getAndParse(name, StoredFeatureSet.class, StoredFeatureSet.TYPE) + .orElseThrow( + () -> new ResourceNotFoundException("Unknown featureset [" + name + "]") + ) + .optimize(); } /** @@ -165,19 +175,21 @@ public static boolean isIndexStore(String indexName) { @Override public CompiledLtrModel loadModel(String name) throws IOException { - StoredLtrModel model = getAndParse(name, StoredLtrModel.class, StoredLtrModel.TYPE); - if (model == null) { - throw new IllegalArgumentException("Unknown model [" + name + "]"); - } + StoredLtrModel model = getAndParse(name, StoredLtrModel.class, StoredLtrModel.TYPE) + .orElseThrow( + () -> new ResourceNotFoundException("Unknown model [" + name + "]") + ); + return model.compile(parserFactory); } - public E getAndParse(String name, Class eltClass, String type) throws IOException { + public Optional getAndParse(String name, Class eltClass, String type) throws IOException { GetResponse response = internalGet(generateId(type, name)).get(); if (response.isExists()) { - return parse(eltClass, type, response.getSourceAsBytes()); + return Optional.of(parse(eltClass, type, response.getSourceAsBytes())); + } else { - return null; + return Optional.empty(); } } diff --git a/src/test/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStoreTests.java b/src/test/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStoreTests.java index ecc7d328..597a1f1f 100644 --- a/src/test/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStoreTests.java +++ b/src/test/java/com/o19s/es/ltr/feature/store/index/IndexFeatureStoreTests.java @@ -22,20 +22,26 @@ import com.o19s.es.ltr.feature.store.StoredFeatureNormalizers; import com.o19s.es.ltr.feature.store.StoredFeatureSet; import com.o19s.es.ltr.feature.store.StoredLtrModel; +import com.o19s.es.ltr.ranker.parser.LtrRankerParserFactory; import org.apache.lucene.util.BytesRef; import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.lucene.tests.util.TestUtil; +import org.hamcrest.MatcherAssert; +import org.opensearch.ResourceNotFoundException; +import org.opensearch.action.get.GetRequestBuilder; +import org.opensearch.action.get.GetResponse; +import org.opensearch.client.Client; import org.opensearch.client.Requests; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.xcontent.XContentParser; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.function.Supplier; import static com.o19s.es.ltr.feature.store.index.IndexFeatureStore.STORE_PREFIX; import static com.o19s.es.ltr.feature.store.index.IndexFeatureStore.indexName; @@ -44,9 +50,68 @@ import static org.apache.lucene.tests.util.TestUtil.randomRealisticUnicodeString; import static org.apache.lucene.tests.util.TestUtil.randomSimpleString; import static org.hamcrest.CoreMatchers.equalTo; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class IndexFeatureStoreTests extends LuceneTestCase { + private Client clientMock; + private GetRequestBuilder getRequestBuilder; + private GetResponse getResponseMock; + + private Supplier clientSupplier = () -> clientMock; + + private void setupMocks() { + clientMock = mock(Client.class); + getRequestBuilder = mock(GetRequestBuilder.class); + getResponseMock = mock(GetResponse.class); + when(clientMock.prepareGet(anyString(), anyString())).thenReturn(getRequestBuilder); + when(getRequestBuilder.get()).thenReturn(getResponseMock); + } + + public void testThat_exceptionIsThrown_forNonExistingFeature() { + setupMocks(); + when(getResponseMock.isExists()).thenReturn(false); + IndexFeatureStore store = new IndexFeatureStore("index", clientSupplier, mock(LtrRankerParserFactory.class)); + + MatcherAssert.assertThat( + expectThrows( + ResourceNotFoundException.class, + () -> store.load("my_feature") + ).getMessage(), + equalTo("Unknown feature [my_feature]") + ); + } + + public void testThat_exceptionIsThrown_forNonExistingFeatureSet() { + setupMocks(); + when(getResponseMock.isExists()).thenReturn(false); + IndexFeatureStore store = new IndexFeatureStore("index", clientSupplier, mock(LtrRankerParserFactory.class)); + + MatcherAssert.assertThat( + expectThrows( + ResourceNotFoundException.class, + () -> store.loadSet("my_feature_set") + ).getMessage(), + equalTo("Unknown featureset [my_feature_set]") + ); + } + + public void testThat_exceptionIsThrown_forNonExistingModel() { + setupMocks(); + when(getResponseMock.isExists()).thenReturn(false); + IndexFeatureStore store = new IndexFeatureStore("index", clientSupplier, mock(LtrRankerParserFactory.class)); + + MatcherAssert.assertThat( + expectThrows( + ResourceNotFoundException.class, + () -> store.loadModel("my_model") + ).getMessage(), + equalTo("Unknown model [my_model]") + ); + } + public void testParse() throws Exception { parseAssertions(LtrTestUtils.randomFeature()); parseAssertions(LtrTestUtils.randomFeatureSet()); @@ -102,10 +167,16 @@ public void testBadValues() throws IOException { builder = XContentBuilder.builder(Requests.INDEX_CONTENT_TYPE.xContent()); map.put("featureset", LtrTestUtils.randomFeatureSet()); BytesReference bytes2 = BytesReference.bytes(builder.map(map)); - assertThat(expectThrows(IllegalArgumentException.class, - () -> IndexFeatureStore.parse(StoredFeature.class, StoredFeature.TYPE, bytes2)) - .getMessage(), equalTo("Expected an element of type [" + StoredFeature.TYPE + "] but" + - " got [" + StoredFeatureSet.TYPE + "].")); + assertThat( + expectThrows( + IllegalArgumentException.class, + () -> IndexFeatureStore.parse(StoredFeature.class, StoredFeature.TYPE, bytes2) + ).getMessage(), + equalTo( + "Expected an element of type [" + StoredFeature.TYPE + "] but" + + " got [" + StoredFeatureSet.TYPE + "]." + ) + ); } private void parseAssertions(StorableElement elt) throws IOException {