From 26caa8a751803d7b4212d3c43182ef4f06962d40 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com> Date: Tue, 3 Jun 2025 13:37:07 -0400 Subject: [PATCH] Renaming and flattening request map (#128699) --- .../custom/CustomServiceSettings.java | 19 +---- .../custom/request/CustomRequest.java | 4 +- ...ava => AbstractInferenceServiceTests.java} | 4 +- .../custom/CustomServiceSettingsTests.java | 74 ++++--------------- .../services/custom/CustomServiceTests.java | 6 +- 5 files changed, 23 insertions(+), 84 deletions(-) rename x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/{AbstractServiceTests.java => AbstractInferenceServiceTests.java} (99%) diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettings.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettings.java index d6e58b4517f1b..8e6ed8d446582 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettings.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettings.java @@ -57,7 +57,6 @@ public class CustomServiceSettings extends FilteredXContentObject implements Ser public static final String URL = "url"; public static final String HEADERS = "headers"; public static final String REQUEST = "request"; - public static final String REQUEST_CONTENT = "content"; public static final String RESPONSE = "response"; public static final String JSON_PARSER = "json_parser"; public static final String ERROR_PARSER = "error_parser"; @@ -83,14 +82,7 @@ public static CustomServiceSettings fromMap( removeNullValues(headers); var stringHeaders = validateMapStringValues(headers, HEADERS, validationException, false); - Map requestBodyMap = extractRequiredMap(map, REQUEST, ModelConfigurations.SERVICE_SETTINGS, validationException); - - String requestContentString = extractRequiredString( - Objects.requireNonNullElse(requestBodyMap, new HashMap<>()), - REQUEST_CONTENT, - ModelConfigurations.SERVICE_SETTINGS, - validationException - ); + String requestContentString = extractRequiredString(map, REQUEST, ModelConfigurations.SERVICE_SETTINGS, validationException); Map responseParserMap = extractRequiredMap( map, @@ -125,11 +117,10 @@ public static CustomServiceSettings fromMap( context ); - if (requestBodyMap == null || responseParserMap == null || jsonParserMap == null || errorParserMap == null) { + if (responseParserMap == null || jsonParserMap == null || errorParserMap == null) { throw validationException; } - throwIfNotEmptyMap(requestBodyMap, REQUEST, NAME); throwIfNotEmptyMap(jsonParserMap, JSON_PARSER, NAME); throwIfNotEmptyMap(responseParserMap, RESPONSE, NAME); throwIfNotEmptyMap(errorParserMap, ERROR_PARSER, NAME); @@ -335,11 +326,7 @@ public XContentBuilder toXContentFragmentOfExposedFields(XContentBuilder builder queryParameters.toXContent(builder, params); - builder.startObject(REQUEST); - { - builder.field(REQUEST_CONTENT, requestContentString); - } - builder.endObject(); + builder.field(REQUEST, requestContentString); builder.startObject(RESPONSE); { diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/request/CustomRequest.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/request/CustomRequest.java index 0a50b08163260..eaabdfea472af 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/request/CustomRequest.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/request/CustomRequest.java @@ -32,7 +32,7 @@ import java.util.Objects; import static org.elasticsearch.xpack.inference.common.JsonUtils.toJson; -import static org.elasticsearch.xpack.inference.services.custom.CustomServiceSettings.REQUEST_CONTENT; +import static org.elasticsearch.xpack.inference.services.custom.CustomServiceSettings.REQUEST; import static org.elasticsearch.xpack.inference.services.custom.CustomServiceSettings.URL; public class CustomRequest implements Request { @@ -133,7 +133,7 @@ private void setHeaders(HttpRequestBase httpRequest) { private void setRequestContent(HttpPost httpRequest) { String replacedRequestContentString = jsonPlaceholderReplacer.replace( model.getServiceSettings().getRequestContentString(), - REQUEST_CONTENT + REQUEST ); StringEntity stringEntity = new StringEntity(replacedRequestContentString, StandardCharsets.UTF_8); httpRequest.setEntity(stringEntity); diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/AbstractServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/AbstractInferenceServiceTests.java similarity index 99% rename from x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/AbstractServiceTests.java rename to x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/AbstractInferenceServiceTests.java index 24e0c7cadb73f..5712ff363bb07 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/AbstractServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/AbstractInferenceServiceTests.java @@ -54,7 +54,7 @@ * To use this class, extend it and pass the constructor a configuration. *

*/ -public abstract class AbstractServiceTests extends ESTestCase { +public abstract class AbstractInferenceServiceTests extends ESTestCase { protected final MockWebServer webServer = new MockWebServer(); protected ThreadPool threadPool; @@ -80,7 +80,7 @@ public void tearDown() throws Exception { private final TestConfiguration testConfiguration; - public AbstractServiceTests(TestConfiguration testConfiguration) { + public AbstractInferenceServiceTests(TestConfiguration testConfiguration) { this.testConfiguration = Objects.requireNonNull(testConfiguration); } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettingsTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettingsTests.java index 1bb3d44b897c8..9e1d3a8f4c8f8 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettingsTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettingsTests.java @@ -126,7 +126,7 @@ public void testFromMap() { QueryParameters.QUERY_PARAMETERS, queryParameters, CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -179,7 +179,7 @@ public void testFromMap_WithOptionalsNotSpecified() { CustomServiceSettings.URL, url, CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -243,7 +243,7 @@ public void testFromMap_RemovesNullValues_FromMaps() { CustomServiceSettings.HEADERS, headersWithNulls, CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -304,7 +304,7 @@ public void testFromMap_ReturnsError_IfHeadersContainsNonStringValues() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", 1)), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -353,7 +353,7 @@ public void testFromMap_ReturnsError_IfQueryParamsContainsNonStringValues() { QueryParameters.QUERY_PARAMETERS, List.of(List.of("key", 1)), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -393,7 +393,7 @@ public void testFromMap_ReturnsError_IfRequestMapIsMissing() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", "value")), "invalid_request", - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -413,13 +413,7 @@ public void testFromMap_ReturnsError_IfRequestMapIsMissing() { () -> CustomServiceSettings.fromMap(mapSettings, ConfigurationParseContext.REQUEST, TaskType.TEXT_EMBEDDING, "inference_id") ); - assertThat( - exception.getMessage(), - is( - "Validation Failed: 1: [service_settings] does not contain the required setting [request];" - + "2: [service_settings] does not contain the required setting [content];" - ) - ); + assertThat(exception.getMessage(), is("Validation Failed: 1: [service_settings] does not contain the required setting [request];")); } public void testFromMap_ReturnsError_IfResponseMapIsMissing() { @@ -433,7 +427,7 @@ public void testFromMap_ReturnsError_IfResponseMapIsMissing() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", "value")), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, "invalid_response", new HashMap<>( Map.of( @@ -464,46 +458,6 @@ public void testFromMap_ReturnsError_IfResponseMapIsMissing() { ); } - public void testFromMap_ReturnsError_IfRequestMapIsNotEmptyAfterParsing() { - String url = "http://www.abc.com"; - String requestContentString = "request body"; - - var mapSettings = new HashMap( - Map.of( - CustomServiceSettings.URL, - url, - CustomServiceSettings.HEADERS, - new HashMap<>(Map.of("key", "value")), - CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString, "key", "value")), - CustomServiceSettings.RESPONSE, - new HashMap<>( - Map.of( - CustomServiceSettings.JSON_PARSER, - new HashMap<>( - Map.of(TextEmbeddingResponseParser.TEXT_EMBEDDING_PARSER_EMBEDDINGS, "$.result.embeddings[*].embedding") - ), - CustomServiceSettings.ERROR_PARSER, - new HashMap<>(Map.of(ErrorResponseParser.MESSAGE_PATH, "$.error.message")) - ) - ) - ) - ); - - var exception = expectThrows( - ElasticsearchStatusException.class, - () -> CustomServiceSettings.fromMap(mapSettings, ConfigurationParseContext.REQUEST, TaskType.TEXT_EMBEDDING, "inference_id") - ); - - assertThat( - exception.getMessage(), - is( - "Configuration contains unknown settings [{key=value}] while parsing field [request]" - + " for settings [custom_service_settings]" - ) - ); - } - public void testFromMap_ReturnsError_IfJsonParserMapIsNotEmptyAfterParsing() { String url = "http://www.abc.com"; String requestContentString = "request body"; @@ -515,7 +469,7 @@ public void testFromMap_ReturnsError_IfJsonParserMapIsNotEmptyAfterParsing() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", "value")), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -560,7 +514,7 @@ public void testFromMap_ReturnsError_IfResponseMapIsNotEmptyAfterParsing() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", "value")), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -602,7 +556,7 @@ public void testFromMap_ReturnsError_IfErrorParserMapIsNotEmptyAfterParsing() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", "value")), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -642,7 +596,7 @@ public void testFromMap_ReturnsError_IfTaskTypeIsInvalid() { CustomServiceSettings.HEADERS, new HashMap<>(Map.of("key", "value")), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, requestContentString)), + requestContentString, CustomServiceSettings.RESPONSE, new HashMap<>( Map.of( @@ -687,9 +641,7 @@ public void testXContent() throws IOException { "headers": { "key": "value" }, - "request": { - "content": "string" - }, + "request": "string", "response": { "json_parser": { "text_embeddings": "$.result.embeddings[*].embedding" diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceTests.java index fb6c50f1bd9c4..b9a94e0124183 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceTests.java @@ -25,7 +25,7 @@ import org.elasticsearch.xpack.core.ml.search.WeightedToken; import org.elasticsearch.xpack.inference.external.http.HttpClientManager; import org.elasticsearch.xpack.inference.external.http.sender.HttpRequestSenderTests; -import org.elasticsearch.xpack.inference.services.AbstractServiceTests; +import org.elasticsearch.xpack.inference.services.AbstractInferenceServiceTests; import org.elasticsearch.xpack.inference.services.SenderService; import org.elasticsearch.xpack.inference.services.ServiceFields; import org.elasticsearch.xpack.inference.services.custom.response.CompletionResponseParser; @@ -54,7 +54,7 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; -public class CustomServiceTests extends AbstractServiceTests { +public class CustomServiceTests extends AbstractInferenceServiceTests { public CustomServiceTests() { super(createTestConfiguration()); @@ -150,7 +150,7 @@ private static Map createServiceSettingsMap(TaskType taskType) { QueryParameters.QUERY_PARAMETERS, List.of(List.of("key", "value")), CustomServiceSettings.REQUEST, - new HashMap<>(Map.of(CustomServiceSettings.REQUEST_CONTENT, "request body")), + "request body", CustomServiceSettings.RESPONSE, new HashMap<>( Map.of(