From f2a0c4eccb93ed4d540515ab24b9845db657e7c3 Mon Sep 17 00:00:00 2001 From: Junwei Dai Date: Wed, 2 Oct 2024 14:56:33 -0700 Subject: [PATCH 1/6] Added ApiSpecFetcher with test Signed-off-by: Junwei Dai --- CHANGELOG.md | 1 + build.gradle | 9 ++ .../exception/ApiSpecParseException.java | 46 +++++++ .../flowframework/util/ApiSpecFetcher.java | 115 ++++++++++++++++++ .../util/ApiSpecFetcherTests.java | 102 ++++++++++++++++ .../workflow/RegisterAgentTests.java | 21 ++++ 6 files changed, 294 insertions(+) create mode 100644 src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java create mode 100644 src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java create mode 100644 src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 167472c68..6a5fdc7b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ## [Unreleased 2.x](https://github.com/opensearch-project/flow-framework/compare/2.17...2.x) ### Features +- Add ApiSpecFetcher for Fetching and Comparing API Specifications ([#651](https://github.com/opensearch-project/flow-framework/issues/651)) ### Enhancements ### Bug Fixes ### Infrastructure diff --git a/build.gradle b/build.gradle index 56d6814e1..236a6a9d9 100644 --- a/build.gradle +++ b/build.gradle @@ -179,6 +179,15 @@ dependencies { implementation "org.glassfish:jakarta.json:2.0.1" implementation "org.eclipse:yasson:3.0.4" implementation "com.google.code.gson:gson:2.11.0" + // Swagger-Parser dependencies for API consistency tests + implementation 'io.swagger.core.v3:swagger-models:2.2.23' + implementation 'io.swagger.parser.v3:swagger-parser-core:2.1.22' + implementation 'io.swagger.parser.v3:swagger-parser:2.1.22' + implementation 'io.swagger.parser.v3:swagger-parser-v3:2.1.22' + implementation 'com.fasterxml.jackson.core:jackson-databind:2.17.0' + implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.13.0' + implementation 'com.fasterxml.jackson.core:jackson-annotations:2.17.0' + implementation 'io.swagger.core.v3:swagger-core:2.2.21' // ZipArchive dependencies used for integration tests zipArchive group: 'org.opensearch.plugin', name:'opensearch-ml-plugin', version: "${opensearch_build}" diff --git a/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java b/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java new file mode 100644 index 000000000..c8b344ef5 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java @@ -0,0 +1,46 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.exception; + +import org.opensearch.OpenSearchException; + +/** + * Custom exception to be thrown when an error occurs during the parsing of an API specification. + */ +public class ApiSpecParseException extends OpenSearchException { + + /** + * Constructor with message. + * + * @param message The detail message. + */ + public ApiSpecParseException(String message) { + super(message); + } + + /** + * Constructor with message and cause. + * + * @param message The detail message. + * @param cause The cause of the exception. + */ + public ApiSpecParseException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructor with message and list of detailed errors. + * + * @param message The detail message. + * @param detailedErrors The list of errors encountered during the parsing process. + */ + public ApiSpecParseException(String message, String detailedErrors) { + super(message + ": " + detailedErrors); + } +} diff --git a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java new file mode 100644 index 000000000..f52ed0665 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java @@ -0,0 +1,115 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.util; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.flowframework.exception.ApiSpecParseException; +import org.opensearch.rest.RestRequest; + +import java.net.URI; +import java.util.HashSet; +import java.util.List; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.media.Content; +import io.swagger.v3.oas.models.media.MediaType; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.parameters.RequestBody; +import io.swagger.v3.parser.OpenAPIV3Parser; +import io.swagger.v3.parser.core.models.ParseOptions; +import io.swagger.v3.parser.core.models.SwaggerParseResult; + +/** + * Utility class for fetching and parsing OpenAPI specifications. + */ +public class ApiSpecFetcher { + private static final Logger LOGGER = LogManager.getLogger(ApiSpecFetcher.class); + private static final ParseOptions PARSE_OPTIONS = new ParseOptions(); + private static final OpenAPIV3Parser PARSER = new OpenAPIV3Parser(); + + /** + * Default constructor for ApiSpecFetcher. + * It sets up default parse options for resolving references. + */ + public ApiSpecFetcher() { + PARSE_OPTIONS.setResolve(true); + PARSE_OPTIONS.setResolveFully(true); + } + + /** + * Parses the OpenAPI specification directly from the URI. + * + * @param apiSpecUri URI to the API specification (can be file path or web URI). + * @return Parsed OpenAPI object. + * @throws ApiSpecParseException If parsing fails. + */ + public OpenAPI fetchApiSpec(URI apiSpecUri) { + LOGGER.info("Parsing API spec from URI: {}", apiSpecUri); + SwaggerParseResult result = PARSER.readLocation(apiSpecUri.toString(), null, PARSE_OPTIONS); + OpenAPI openApi = result.getOpenAPI(); + + if (openApi == null) { + throw new ApiSpecParseException("Unable to parse spec from URI: " + apiSpecUri, String.join(", ", result.getMessages())); + } + + return openApi; + } + + /** + * Compares the required fields in the API spec with the required enum parameters. + * + * @param requiredEnumParams List of required parameters from the enum. + * @param apiSpecUri URI of the API spec to fetch and compare. + * @param path The API path to check. + * @param method The HTTP method (POST, GET, etc.). + * @return boolean indicating if the required fields match. + * @throws Exception If fetching or parsing fails. + */ + public boolean compareRequiredFields(List requiredEnumParams, URI apiSpecUri, String path, RestRequest.Method method) + throws Exception { + OpenAPI openAPI = fetchApiSpec(apiSpecUri); + + PathItem pathItem = openAPI.getPaths().get(path); + Content content = getContent(method, pathItem); + MediaType mediaType = content.get("application/json"); + if (mediaType != null) { + Schema schema = mediaType.getSchema(); + + List requiredApiParams = schema.getRequired(); + if (requiredApiParams != null && !requiredApiParams.isEmpty()) { + return new HashSet<>(requiredEnumParams).equals(new HashSet<>(requiredApiParams)); + } + } + return false; + } + + private static Content getContent(RestRequest.Method method, PathItem pathItem) throws Exception { + Operation operation = switch (method) { + case RestRequest.Method.POST -> pathItem.getPost(); + case RestRequest.Method.GET -> pathItem.getGet(); + case RestRequest.Method.PUT -> pathItem.getPut(); + case RestRequest.Method.DELETE -> pathItem.getDelete(); + default -> throw new Exception("Unsupported HTTP method: " + method); + }; + + if (operation == null) { + throw new Exception("No operation found for the specified method: " + method); + } + + RequestBody requestBody = operation.getRequestBody(); + if (requestBody == null) { + throw new Exception("No requestBody defined for this operation."); + } + + return requestBody.getContent(); + } +} diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java new file mode 100644 index 000000000..7e8b57b8e --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -0,0 +1,102 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.util; + +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +import org.opensearch.flowframework.exception.ApiSpecParseException; +import org.opensearch.rest.RestRequest; +import org.opensearch.test.OpenSearchTestCase; +import org.junit.Before; + +import java.net.URI; +import java.util.Arrays; +import java.util.List; + +import io.swagger.v3.oas.models.OpenAPI; + +public class ApiSpecFetcherTests extends OpenSearchTestCase { + + private ApiSpecFetcher apiSpecFetcher; + + @Before + public void setUp() throws Exception { + super.setUp(); + this.apiSpecFetcher = new ApiSpecFetcher(); // Initialize your fetcher + } + + public void testFetchApiSpecSuccess() throws Exception { + URI validUri = new URI( + "https://raw.githubusercontent.com/junweid62/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + ); + + OpenAPI result = apiSpecFetcher.fetchApiSpec(validUri); + + assertNotNull("The fetched OpenAPI spec should not be null", result); + } + + public void testFetchApiSpecThrowsException() throws Exception { + URI invalidUri = new URI("http://invalid-url.com/fail.yaml"); + + ApiSpecParseException exception = expectThrows(ApiSpecParseException.class, () -> { apiSpecFetcher.fetchApiSpec(invalidUri); }); + + assertNotNull("Exception should be thrown for invalid URL", exception); + assertTrue(exception.getMessage().contains("Unable to parse spec")); + } + + public void testCompareRequiredFieldsSuccess() throws Exception { + URI validUri = new URI( + "https://raw.githubusercontent.com/junweid62/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + ); + String path = "/_plugins/_ml/agents/_register"; + RestRequest.Method method = RestRequest.Method.POST; + + // Assuming REGISTER_AGENT step in the enum has these required fields + List expectedRequiredParams = Arrays.asList("name", "type"); + + boolean comparisonResult = apiSpecFetcher.compareRequiredFields(expectedRequiredParams, validUri, path, method); + + assertTrue("The required fields should match between API spec and enum", comparisonResult); + } + + public void testCompareRequiredFieldsFailure() throws Exception { + URI validUri = new URI( + "https://raw.githubusercontent.com/junweid62/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + ); + String path = "/_plugins/_ml/agents/_register"; + RestRequest.Method method = RestRequest.Method.POST; + + List wrongRequiredParams = Arrays.asList("nonexistent_param"); + + boolean comparisonResult = apiSpecFetcher.compareRequiredFields(wrongRequiredParams, validUri, path, method); + + assertFalse("The required fields should not match for incorrect input", comparisonResult); + } + + public void testCompareRequiredFieldsThrowsException() throws Exception { + URI invalidUri = new URI("http://invalid-url.com/fail.yaml"); + String path = "/_plugins/_ml/agents/_register"; + RestRequest.Method method = RestRequest.Method.POST; + + Exception exception = expectThrows( + Exception.class, + () -> { apiSpecFetcher.compareRequiredFields(List.of(), invalidUri, path, method); } + ); + + assertNotNull("An exception should be thrown for an invalid API spec URI", exception); + assertTrue(exception.getMessage().contains("Unable to parse spec")); + } + +} diff --git a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java index 626dfdfa1..62de25a8c 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java @@ -13,6 +13,7 @@ import org.opensearch.core.rest.RestStatus; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; +import org.opensearch.flowframework.util.ApiSpecFetcher; import org.opensearch.ml.client.MachineLearningNodeClient; import org.opensearch.ml.common.MLAgentType; import org.opensearch.ml.common.agent.LLMSpec; @@ -20,10 +21,13 @@ import org.opensearch.ml.common.agent.MLMemorySpec; import org.opensearch.ml.common.agent.MLToolSpec; import org.opensearch.ml.common.transport.agent.MLRegisterAgentResponse; +import org.opensearch.rest.RestRequest; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.net.URI; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -150,4 +154,21 @@ public void testRegisterAgentFailure() throws IOException { assertTrue(ex.getCause() instanceof FlowFrameworkException); assertEquals("Failed to register the agent", ex.getCause().getMessage()); } + + public void testApiSpecRegisterAgentInputParamComparison() throws Exception { + List requiredEnumParams = WorkflowStepFactory.WorkflowSteps.REGISTER_AGENT.inputs(); + + ApiSpecFetcher apiSpecFetcher = new ApiSpecFetcher(); + boolean isMatch = apiSpecFetcher.compareRequiredFields( + requiredEnumParams, + new URI( + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + ), + "/_plugins/_ml/agents/_register", + RestRequest.Method.POST + ); + + assertTrue("API spec input params do not match enum required params", isMatch); + } + } From 65b5303fb51a23e2be02c0f0d6524f20826c749d Mon Sep 17 00:00:00 2001 From: Junwei Dai Date: Fri, 4 Oct 2024 09:42:52 -0700 Subject: [PATCH 2/6] remove duplication license Signed-off-by: Junwei Dai --- .../flowframework/util/ApiSpecFetcherTests.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java index 7e8b57b8e..33529153b 100644 --- a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -8,14 +8,6 @@ */ package org.opensearch.flowframework.util; -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - import org.opensearch.flowframework.exception.ApiSpecParseException; import org.opensearch.rest.RestRequest; import org.opensearch.test.OpenSearchTestCase; From 87a23a9a3cf76b18f2d87e48f56cd56549ee2d9f Mon Sep 17 00:00:00 2001 From: Junwei Dai Date: Fri, 4 Oct 2024 10:22:06 -0700 Subject: [PATCH 3/6] Add more test to pass test coverage check Signed-off-by: Junwei Dai --- .../exception/ApiSpecParseExceptionTests.java | 36 +++++++++++++++++++ .../util/ApiSpecFetcherTests.java | 18 ++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java diff --git a/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java new file mode 100644 index 000000000..ae39c39f4 --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java @@ -0,0 +1,36 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.exception; + +import org.opensearch.OpenSearchException; +import org.opensearch.test.OpenSearchTestCase; + +public class ApiSpecParseExceptionTests extends OpenSearchTestCase { + + public void testApiSpecParseException() { + ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed"); + assertTrue(exception instanceof OpenSearchException); + assertEquals("API spec parsing failed", exception.getMessage()); + } + + public void testApiSpecParseExceptionWithCause() { + Throwable cause = new RuntimeException("Underlying issue"); + ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed", cause); + assertTrue(exception instanceof OpenSearchException); + assertEquals("API spec parsing failed", exception.getMessage()); + assertEquals(cause, exception.getCause()); + } + + public void testApiSpecParseExceptionWithDetailedErrors() { + ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed", "Missing required field"); + assertTrue(exception instanceof OpenSearchException); + assertEquals("API spec parsing failed: Missing required field", exception.getMessage()); + } + +} diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java index 33529153b..4636508d8 100644 --- a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -22,16 +22,20 @@ public class ApiSpecFetcherTests extends OpenSearchTestCase { private ApiSpecFetcher apiSpecFetcher; + private URI apiSpecUri; @Before public void setUp() throws Exception { super.setUp(); this.apiSpecFetcher = new ApiSpecFetcher(); // Initialize your fetcher + this.apiSpecUri = new URI( + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + ); } public void testFetchApiSpecSuccess() throws Exception { URI validUri = new URI( - "https://raw.githubusercontent.com/junweid62/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" ); OpenAPI result = apiSpecFetcher.fetchApiSpec(validUri); @@ -50,7 +54,7 @@ public void testFetchApiSpecThrowsException() throws Exception { public void testCompareRequiredFieldsSuccess() throws Exception { URI validUri = new URI( - "https://raw.githubusercontent.com/junweid62/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" ); String path = "/_plugins/_ml/agents/_register"; RestRequest.Method method = RestRequest.Method.POST; @@ -65,7 +69,7 @@ public void testCompareRequiredFieldsSuccess() throws Exception { public void testCompareRequiredFieldsFailure() throws Exception { URI validUri = new URI( - "https://raw.githubusercontent.com/junweid62/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" ); String path = "/_plugins/_ml/agents/_register"; RestRequest.Method method = RestRequest.Method.POST; @@ -91,4 +95,12 @@ public void testCompareRequiredFieldsThrowsException() throws Exception { assertTrue(exception.getMessage().contains("Unable to parse spec")); } + public void testNoOperationFoundException() throws Exception { + Exception exception = expectThrows(Exception.class, () -> { + apiSpecFetcher.compareRequiredFields(List.of("name", "type"), apiSpecUri, "/invalid/path", RestRequest.Method.PATCH); + }); + + assertEquals("Unsupported HTTP method: PATCH", exception.getMessage()); + } + } From 8f0010fb6157b4a421800ebe05bb17a200ce1917 Mon Sep 17 00:00:00 2001 From: Junwei Dai Date: Mon, 7 Oct 2024 11:41:57 -0700 Subject: [PATCH 4/6] new commit address all comments Signed-off-by: Junwei Dai --- CHANGELOG.md | 1 + build.gradle | 24 +++++----- .../flowframework/common/CommonValue.java | 8 ++++ .../exception/ApiSpecParseException.java | 8 ++-- .../flowframework/util/ApiSpecFetcher.java | 39 +++++++--------- .../exception/ApiSpecParseExceptionTests.java | 18 +++++--- .../util/ApiSpecFetcherTests.java | 44 ++++++++----------- .../workflow/RegisterAgentTests.java | 8 ++-- 8 files changed, 79 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a5fdc7b3..e71b22c69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ## [Unreleased 2.x](https://github.com/opensearch-project/flow-framework/compare/2.17...2.x) ### Features - Add ApiSpecFetcher for Fetching and Comparing API Specifications ([#651](https://github.com/opensearch-project/flow-framework/issues/651)) + ### Enhancements ### Bug Fixes ### Infrastructure diff --git a/build.gradle b/build.gradle index a3d9825a0..3caef4f90 100644 --- a/build.gradle +++ b/build.gradle @@ -24,7 +24,7 @@ buildscript { opensearch_no_snapshot = opensearch_build.replace("-SNAPSHOT","") System.setProperty('tests.security.manager', 'false') common_utils_version = System.getProperty("common_utils.version", opensearch_build) - + swaggerCoreVersion = "2.2.23" bwcVersionShort = "2.12.0" bwcVersion = bwcVersionShort + ".0" bwcOpenSearchFFDownload = 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/' + bwcVersionShort + '/latest/linux/x64/tar/builds/' + @@ -33,7 +33,11 @@ buildscript { bwcFilePath = "src/test/resources/org/opensearch/flowframework/bwc/" bwcFlowFrameworkPath = bwcFilePath + "flowframework/" - isSameMajorVersion = opensearch_version.split("\\.")[0] == bwcVersionShort.split("\\.")[0] + isSameMajorVersion = opensearch_version.split("\\.")[0] == bwcVersionShort.split("\\.") + swaggerVersion = "2.1.22" + jacksonVersion = "2.18.0" + swaggerCoreVersion = "2.2.23" + } repositories { @@ -180,14 +184,14 @@ dependencies { implementation "org.eclipse:yasson:3.0.4" implementation "com.google.code.gson:gson:2.11.0" // Swagger-Parser dependencies for API consistency tests - implementation 'io.swagger.core.v3:swagger-models:2.2.23' - implementation 'io.swagger.parser.v3:swagger-parser-core:2.1.22' - implementation 'io.swagger.parser.v3:swagger-parser:2.1.22' - implementation 'io.swagger.parser.v3:swagger-parser-v3:2.1.22' - implementation 'com.fasterxml.jackson.core:jackson-databind:2.17.0' - implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.13.0' - implementation 'com.fasterxml.jackson.core:jackson-annotations:2.17.0' - implementation 'io.swagger.core.v3:swagger-core:2.2.21' + implementation "io.swagger.core.v3:swagger-models:${swaggerCoreVersion}" + implementation "io.swagger.core.v3:swagger-core:${swaggerCoreVersion}" + implementation "io.swagger.parser.v3:swagger-parser-core:${swaggerVersion}" + implementation "io.swagger.parser.v3:swagger-parser:${swaggerVersion}" + implementation "io.swagger.parser.v3:swagger-parser-v3:${swaggerVersion}" + implementation "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}" + implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${jacksonVersion}" + implementation "com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}" // ZipArchive dependencies used for integration tests zipArchive group: 'org.opensearch.plugin', name:'opensearch-ml-plugin', version: "${opensearch_build}" diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java index f291cff1c..6011d76ad 100644 --- a/src/main/java/org/opensearch/flowframework/common/CommonValue.java +++ b/src/main/java/org/opensearch/flowframework/common/CommonValue.java @@ -233,4 +233,12 @@ private CommonValue() {} public static final String CREATE_INGEST_PIPELINE_MODEL_ID = "create_ingest_pipeline.model_id"; /** The field name for reindex source index substitution */ public static final String REINDEX_SOURCE_INDEX = "reindex.source_index"; + + /* + * Constants associated with API specification + */ + + public static final String ML_COMMONS_API_SPEC_YAML_URI = + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml"; + } diff --git a/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java b/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java index c8b344ef5..ae77452c7 100644 --- a/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java +++ b/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java @@ -10,6 +10,8 @@ import org.opensearch.OpenSearchException; +import java.util.List; + /** * Custom exception to be thrown when an error occurs during the parsing of an API specification. */ @@ -38,9 +40,9 @@ public ApiSpecParseException(String message, Throwable cause) { * Constructor with message and list of detailed errors. * * @param message The detail message. - * @param detailedErrors The list of errors encountered during the parsing process. + * @param details The list of errors encountered during the parsing process. */ - public ApiSpecParseException(String message, String detailedErrors) { - super(message + ": " + detailedErrors); + public ApiSpecParseException(String message, List details) { + super(message + ": " + String.join(", ", details)); } } diff --git a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java index f52ed0665..94b3ef368 100644 --- a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java +++ b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java @@ -10,10 +10,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.flowframework.exception.ApiSpecParseException; import org.opensearch.rest.RestRequest; -import java.net.URI; import java.util.HashSet; import java.util.List; @@ -32,17 +32,13 @@ * Utility class for fetching and parsing OpenAPI specifications. */ public class ApiSpecFetcher { - private static final Logger LOGGER = LogManager.getLogger(ApiSpecFetcher.class); - private static final ParseOptions PARSE_OPTIONS = new ParseOptions(); + private static final Logger logger = LogManager.getLogger(ApiSpecFetcher.class); + private static final ParseOptions OPENAPI_PARSER = new ParseOptions(); private static final OpenAPIV3Parser PARSER = new OpenAPIV3Parser(); - /** - * Default constructor for ApiSpecFetcher. - * It sets up default parse options for resolving references. - */ - public ApiSpecFetcher() { - PARSE_OPTIONS.setResolve(true); - PARSE_OPTIONS.setResolveFully(true); + static { + OPENAPI_PARSER.setResolve(true); + OPENAPI_PARSER.setResolveFully(true); } /** @@ -52,13 +48,13 @@ public ApiSpecFetcher() { * @return Parsed OpenAPI object. * @throws ApiSpecParseException If parsing fails. */ - public OpenAPI fetchApiSpec(URI apiSpecUri) { - LOGGER.info("Parsing API spec from URI: {}", apiSpecUri); - SwaggerParseResult result = PARSER.readLocation(apiSpecUri.toString(), null, PARSE_OPTIONS); + public static OpenAPI fetchApiSpec(String apiSpecUri) { + logger.info("Parsing API spec from URI: {}", apiSpecUri); + SwaggerParseResult result = PARSER.readLocation(apiSpecUri, null, OPENAPI_PARSER); OpenAPI openApi = result.getOpenAPI(); if (openApi == null) { - throw new ApiSpecParseException("Unable to parse spec from URI: " + apiSpecUri, String.join(", ", result.getMessages())); + throw new ApiSpecParseException("Unable to parse spec from URI: " + apiSpecUri, result.getMessages()); } return openApi; @@ -72,15 +68,14 @@ public OpenAPI fetchApiSpec(URI apiSpecUri) { * @param path The API path to check. * @param method The HTTP method (POST, GET, etc.). * @return boolean indicating if the required fields match. - * @throws Exception If fetching or parsing fails. */ - public boolean compareRequiredFields(List requiredEnumParams, URI apiSpecUri, String path, RestRequest.Method method) - throws Exception { + public static boolean compareRequiredFields(List requiredEnumParams, String apiSpecUri, String path, RestRequest.Method method) + throws IllegalArgumentException, ApiSpecParseException { OpenAPI openAPI = fetchApiSpec(apiSpecUri); PathItem pathItem = openAPI.getPaths().get(path); Content content = getContent(method, pathItem); - MediaType mediaType = content.get("application/json"); + MediaType mediaType = content.get(XContentType.JSON.mediaTypeWithoutParameters()); if (mediaType != null) { Schema schema = mediaType.getSchema(); @@ -92,22 +87,22 @@ public boolean compareRequiredFields(List requiredEnumParams, URI apiSpe return false; } - private static Content getContent(RestRequest.Method method, PathItem pathItem) throws Exception { + private static Content getContent(RestRequest.Method method, PathItem pathItem) throws IllegalArgumentException, ApiSpecParseException { Operation operation = switch (method) { case RestRequest.Method.POST -> pathItem.getPost(); case RestRequest.Method.GET -> pathItem.getGet(); case RestRequest.Method.PUT -> pathItem.getPut(); case RestRequest.Method.DELETE -> pathItem.getDelete(); - default -> throw new Exception("Unsupported HTTP method: " + method); + default -> throw new IllegalArgumentException("Unsupported HTTP method: " + method); }; if (operation == null) { - throw new Exception("No operation found for the specified method: " + method); + throw new IllegalArgumentException("No operation found for the specified method: " + method); } RequestBody requestBody = operation.getRequestBody(); if (requestBody == null) { - throw new Exception("No requestBody defined for this operation."); + throw new ApiSpecParseException("No requestBody defined for this operation."); } return requestBody.getContent(); diff --git a/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java index ae39c39f4..70df0e7ca 100644 --- a/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java +++ b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java @@ -8,29 +8,35 @@ */ package org.opensearch.flowframework.exception; -import org.opensearch.OpenSearchException; import org.opensearch.test.OpenSearchTestCase; +import java.util.Arrays; +import java.util.List; + public class ApiSpecParseExceptionTests extends OpenSearchTestCase { public void testApiSpecParseException() { ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed"); - assertTrue(exception instanceof OpenSearchException); + assertTrue(true); assertEquals("API spec parsing failed", exception.getMessage()); } public void testApiSpecParseExceptionWithCause() { Throwable cause = new RuntimeException("Underlying issue"); ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed", cause); - assertTrue(exception instanceof OpenSearchException); + assertTrue(true); assertEquals("API spec parsing failed", exception.getMessage()); assertEquals(cause, exception.getCause()); } public void testApiSpecParseExceptionWithDetailedErrors() { - ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed", "Missing required field"); - assertTrue(exception instanceof OpenSearchException); - assertEquals("API spec parsing failed: Missing required field", exception.getMessage()); + String message = "API spec parsing failed"; + List details = Arrays.asList("Missing required field", "Invalid type"); + + ApiSpecParseException exception = new ApiSpecParseException(message, details); + + String expectedMessage = "API spec parsing failed: Missing required field, Invalid type"; + assertEquals(expectedMessage, exception.getMessage()); } } diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java index 4636508d8..3b5ab0bed 100644 --- a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -13,91 +13,85 @@ import org.opensearch.test.OpenSearchTestCase; import org.junit.Before; -import java.net.URI; import java.util.Arrays; import java.util.List; import io.swagger.v3.oas.models.OpenAPI; +import static org.opensearch.flowframework.common.CommonValue.ML_COMMONS_API_SPEC_YAML_URI; + public class ApiSpecFetcherTests extends OpenSearchTestCase { private ApiSpecFetcher apiSpecFetcher; - private URI apiSpecUri; @Before public void setUp() throws Exception { super.setUp(); - this.apiSpecFetcher = new ApiSpecFetcher(); // Initialize your fetcher - this.apiSpecUri = new URI( - "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" - ); } public void testFetchApiSpecSuccess() throws Exception { - URI validUri = new URI( - "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" - ); - OpenAPI result = apiSpecFetcher.fetchApiSpec(validUri); + OpenAPI result = ApiSpecFetcher.fetchApiSpec(ML_COMMONS_API_SPEC_YAML_URI); assertNotNull("The fetched OpenAPI spec should not be null", result); } public void testFetchApiSpecThrowsException() throws Exception { - URI invalidUri = new URI("http://invalid-url.com/fail.yaml"); + String invalidUri = "http://invalid-url.com/fail.yaml"; - ApiSpecParseException exception = expectThrows(ApiSpecParseException.class, () -> { apiSpecFetcher.fetchApiSpec(invalidUri); }); + ApiSpecParseException exception = expectThrows(ApiSpecParseException.class, () -> { ApiSpecFetcher.fetchApiSpec(invalidUri); }); - assertNotNull("Exception should be thrown for invalid URL", exception); + assertNotNull("Exception should be thrown for invalid URI", exception); assertTrue(exception.getMessage().contains("Unable to parse spec")); } public void testCompareRequiredFieldsSuccess() throws Exception { - URI validUri = new URI( - "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" - ); + String path = "/_plugins/_ml/agents/_register"; RestRequest.Method method = RestRequest.Method.POST; // Assuming REGISTER_AGENT step in the enum has these required fields List expectedRequiredParams = Arrays.asList("name", "type"); - boolean comparisonResult = apiSpecFetcher.compareRequiredFields(expectedRequiredParams, validUri, path, method); + boolean comparisonResult = ApiSpecFetcher.compareRequiredFields(expectedRequiredParams, ML_COMMONS_API_SPEC_YAML_URI, path, method); assertTrue("The required fields should match between API spec and enum", comparisonResult); } public void testCompareRequiredFieldsFailure() throws Exception { - URI validUri = new URI( - "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" - ); + String path = "/_plugins/_ml/agents/_register"; RestRequest.Method method = RestRequest.Method.POST; List wrongRequiredParams = Arrays.asList("nonexistent_param"); - boolean comparisonResult = apiSpecFetcher.compareRequiredFields(wrongRequiredParams, validUri, path, method); + boolean comparisonResult = ApiSpecFetcher.compareRequiredFields(wrongRequiredParams, ML_COMMONS_API_SPEC_YAML_URI, path, method); assertFalse("The required fields should not match for incorrect input", comparisonResult); } public void testCompareRequiredFieldsThrowsException() throws Exception { - URI invalidUri = new URI("http://invalid-url.com/fail.yaml"); + String invalidUri = "http://invalid-url.com/fail.yaml"; String path = "/_plugins/_ml/agents/_register"; RestRequest.Method method = RestRequest.Method.POST; Exception exception = expectThrows( Exception.class, - () -> { apiSpecFetcher.compareRequiredFields(List.of(), invalidUri, path, method); } + () -> { ApiSpecFetcher.compareRequiredFields(List.of(), invalidUri, path, method); } ); - assertNotNull("An exception should be thrown for an invalid API spec URI", exception); + assertNotNull("An exception should be thrown for an invalid API spec Uri", exception); assertTrue(exception.getMessage().contains("Unable to parse spec")); } public void testNoOperationFoundException() throws Exception { Exception exception = expectThrows(Exception.class, () -> { - apiSpecFetcher.compareRequiredFields(List.of("name", "type"), apiSpecUri, "/invalid/path", RestRequest.Method.PATCH); + ApiSpecFetcher.compareRequiredFields( + List.of("name", "type"), + ML_COMMONS_API_SPEC_YAML_URI, + "/invalid/path", + RestRequest.Method.PATCH + ); }); assertEquals("Unsupported HTTP method: PATCH", exception.getMessage()); diff --git a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java index 62de25a8c..e15c327d9 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java @@ -25,7 +25,6 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; -import java.net.URI; import java.util.Collections; import java.util.List; import java.util.Map; @@ -35,6 +34,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.opensearch.flowframework.common.CommonValue.ML_COMMONS_API_SPEC_YAML_URI; import static org.opensearch.flowframework.common.WorkflowResources.AGENT_ID; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -161,14 +161,12 @@ public void testApiSpecRegisterAgentInputParamComparison() throws Exception { ApiSpecFetcher apiSpecFetcher = new ApiSpecFetcher(); boolean isMatch = apiSpecFetcher.compareRequiredFields( requiredEnumParams, - new URI( - "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml" - ), + ML_COMMONS_API_SPEC_YAML_URI, "/_plugins/_ml/agents/_register", RestRequest.Method.POST ); - assertTrue("API spec input params do not match enum required params", isMatch); + assertTrue("API spec input params do match enum required params", isMatch); } } From f021ef852d47f210756794636020d5b24b39dd61 Mon Sep 17 00:00:00 2001 From: Junwei Dai Date: Mon, 7 Oct 2024 15:54:56 -0700 Subject: [PATCH 5/6] new commit address all comments Signed-off-by: Junwei Dai --- build.gradle | 2 +- .../flowframework/common/CommonValue.java | 5 +--- .../flowframework/util/ApiSpecFetcher.java | 24 ++++++++++----- .../exception/ApiSpecParseExceptionTests.java | 8 ++--- .../util/ApiSpecFetcherTests.java | 30 +++++++++++++++++-- .../workflow/RegisterAgentTests.java | 5 ++-- 6 files changed, 53 insertions(+), 21 deletions(-) diff --git a/build.gradle b/build.gradle index 3caef4f90..e5bc757bf 100644 --- a/build.gradle +++ b/build.gradle @@ -33,7 +33,7 @@ buildscript { bwcFilePath = "src/test/resources/org/opensearch/flowframework/bwc/" bwcFlowFrameworkPath = bwcFilePath + "flowframework/" - isSameMajorVersion = opensearch_version.split("\\.")[0] == bwcVersionShort.split("\\.") + isSameMajorVersion = opensearch_version.split("\\.")[0] == bwcVersionShort.split("\\.")[0] swaggerVersion = "2.1.22" jacksonVersion = "2.18.0" swaggerCoreVersion = "2.2.23" diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java index 6011d76ad..898675d94 100644 --- a/src/main/java/org/opensearch/flowframework/common/CommonValue.java +++ b/src/main/java/org/opensearch/flowframework/common/CommonValue.java @@ -234,10 +234,7 @@ private CommonValue() {} /** The field name for reindex source index substitution */ public static final String REINDEX_SOURCE_INDEX = "reindex.source_index"; - /* - * Constants associated with API specification - */ - + /**URI for the YAML file of the ML Commons API specification.*/ public static final String ML_COMMONS_API_SPEC_YAML_URI = "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml"; diff --git a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java index 94b3ef368..e393587bc 100644 --- a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java +++ b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java @@ -88,13 +88,23 @@ public static boolean compareRequiredFields(List requiredEnumParams, Str } private static Content getContent(RestRequest.Method method, PathItem pathItem) throws IllegalArgumentException, ApiSpecParseException { - Operation operation = switch (method) { - case RestRequest.Method.POST -> pathItem.getPost(); - case RestRequest.Method.GET -> pathItem.getGet(); - case RestRequest.Method.PUT -> pathItem.getPut(); - case RestRequest.Method.DELETE -> pathItem.getDelete(); - default -> throw new IllegalArgumentException("Unsupported HTTP method: " + method); - }; + Operation operation; + switch (method) { + case POST: + operation = pathItem.getPost(); + break; + case GET: + operation = pathItem.getGet(); + break; + case PUT: + operation = pathItem.getPut(); + break; + case DELETE: + operation = pathItem.getDelete(); + break; + default: + throw new IllegalArgumentException("Unsupported HTTP method: " + method); + } if (operation == null) { throw new IllegalArgumentException("No operation found for the specified method: " + method); diff --git a/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java index 70df0e7ca..ab93bd66c 100644 --- a/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java +++ b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java @@ -8,6 +8,7 @@ */ package org.opensearch.flowframework.exception; +import org.opensearch.OpenSearchException; import org.opensearch.test.OpenSearchTestCase; import java.util.Arrays; @@ -17,14 +18,14 @@ public class ApiSpecParseExceptionTests extends OpenSearchTestCase { public void testApiSpecParseException() { ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed"); - assertTrue(true); + assertTrue(exception instanceof OpenSearchException); assertEquals("API spec parsing failed", exception.getMessage()); } public void testApiSpecParseExceptionWithCause() { Throwable cause = new RuntimeException("Underlying issue"); ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed", cause); - assertTrue(true); + assertTrue(exception instanceof OpenSearchException); assertEquals("API spec parsing failed", exception.getMessage()); assertEquals(cause, exception.getCause()); } @@ -32,9 +33,8 @@ public void testApiSpecParseExceptionWithCause() { public void testApiSpecParseExceptionWithDetailedErrors() { String message = "API spec parsing failed"; List details = Arrays.asList("Missing required field", "Invalid type"); - ApiSpecParseException exception = new ApiSpecParseException(message, details); - + assertTrue(exception instanceof OpenSearchException); String expectedMessage = "API spec parsing failed: Missing required field, Invalid type"; assertEquals(expectedMessage, exception.getMessage()); } diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java index 3b5ab0bed..b7d70b580 100644 --- a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -84,12 +84,12 @@ public void testCompareRequiredFieldsThrowsException() throws Exception { assertTrue(exception.getMessage().contains("Unable to parse spec")); } - public void testNoOperationFoundException() throws Exception { + public void testUnsupportedMethodException() throws IllegalArgumentException { Exception exception = expectThrows(Exception.class, () -> { ApiSpecFetcher.compareRequiredFields( List.of("name", "type"), ML_COMMONS_API_SPEC_YAML_URI, - "/invalid/path", + "/_plugins/_ml/agents/_register", RestRequest.Method.PATCH ); }); @@ -97,4 +97,30 @@ public void testNoOperationFoundException() throws Exception { assertEquals("Unsupported HTTP method: PATCH", exception.getMessage()); } + public void testNoOperationFoundException() throws Exception { + Exception exception = expectThrows(IllegalArgumentException.class, () -> { + ApiSpecFetcher.compareRequiredFields( + List.of("name", "type"), + ML_COMMONS_API_SPEC_YAML_URI, + "/_plugins/_ml/agents/_register", + RestRequest.Method.DELETE // PATCH should be unsupported + ); + }); + + assertEquals("No operation found for the specified method: DELETE", exception.getMessage()); + } + + public void testNoRequestBodyDefinedException() throws ApiSpecParseException { + Exception exception = expectThrows(ApiSpecParseException.class, () -> { + ApiSpecFetcher.compareRequiredFields( + List.of("name", "type"), + ML_COMMONS_API_SPEC_YAML_URI, + "/_plugins/_ml/model_groups/{model_group_id}", + RestRequest.Method.GET + ); + }); + + assertEquals("No requestBody defined for this operation.", exception.getMessage()); + } + } diff --git a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java index e15c327d9..c2b3dcca1 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java @@ -158,15 +158,14 @@ public void testRegisterAgentFailure() throws IOException { public void testApiSpecRegisterAgentInputParamComparison() throws Exception { List requiredEnumParams = WorkflowStepFactory.WorkflowSteps.REGISTER_AGENT.inputs(); - ApiSpecFetcher apiSpecFetcher = new ApiSpecFetcher(); - boolean isMatch = apiSpecFetcher.compareRequiredFields( + boolean isMatch = ApiSpecFetcher.compareRequiredFields( requiredEnumParams, ML_COMMONS_API_SPEC_YAML_URI, "/_plugins/_ml/agents/_register", RestRequest.Method.POST ); - assertTrue("API spec input params do match enum required params", isMatch); + assertTrue(isMatch); } } From af15a00c52feb1c193e0b16c6b1772749bbfa10f Mon Sep 17 00:00:00 2001 From: Junwei Dai Date: Mon, 7 Oct 2024 17:42:49 -0700 Subject: [PATCH 6/6] Addressed all comments Signed-off-by: Junwei Dai --- .../flowframework/util/ApiSpecFetcher.java | 10 +++++----- .../flowframework/util/ApiSpecFetcherTests.java | 14 +++++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java index e393587bc..80be71b65 100644 --- a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java +++ b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java @@ -33,12 +33,12 @@ */ public class ApiSpecFetcher { private static final Logger logger = LogManager.getLogger(ApiSpecFetcher.class); - private static final ParseOptions OPENAPI_PARSER = new ParseOptions(); - private static final OpenAPIV3Parser PARSER = new OpenAPIV3Parser(); + private static final ParseOptions PARSE_OPTIONS = new ParseOptions(); + private static final OpenAPIV3Parser OPENAPI_PARSER = new OpenAPIV3Parser(); static { - OPENAPI_PARSER.setResolve(true); - OPENAPI_PARSER.setResolveFully(true); + PARSE_OPTIONS.setResolve(true); + PARSE_OPTIONS.setResolveFully(true); } /** @@ -50,7 +50,7 @@ public class ApiSpecFetcher { */ public static OpenAPI fetchApiSpec(String apiSpecUri) { logger.info("Parsing API spec from URI: {}", apiSpecUri); - SwaggerParseResult result = PARSER.readLocation(apiSpecUri, null, OPENAPI_PARSER); + SwaggerParseResult result = OPENAPI_PARSER.readLocation(apiSpecUri, null, PARSE_OPTIONS); OpenAPI openApi = result.getOpenAPI(); if (openApi == null) { diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java index b7d70b580..fb60ae08d 100644 --- a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -19,6 +19,10 @@ import io.swagger.v3.oas.models.OpenAPI; import static org.opensearch.flowframework.common.CommonValue.ML_COMMONS_API_SPEC_YAML_URI; +import static org.opensearch.rest.RestRequest.Method.DELETE; +import static org.opensearch.rest.RestRequest.Method.PATCH; +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.rest.RestRequest.Method.PUT; public class ApiSpecFetcherTests extends OpenSearchTestCase { @@ -48,7 +52,7 @@ public void testFetchApiSpecThrowsException() throws Exception { public void testCompareRequiredFieldsSuccess() throws Exception { String path = "/_plugins/_ml/agents/_register"; - RestRequest.Method method = RestRequest.Method.POST; + RestRequest.Method method = POST; // Assuming REGISTER_AGENT step in the enum has these required fields List expectedRequiredParams = Arrays.asList("name", "type"); @@ -61,7 +65,7 @@ public void testCompareRequiredFieldsSuccess() throws Exception { public void testCompareRequiredFieldsFailure() throws Exception { String path = "/_plugins/_ml/agents/_register"; - RestRequest.Method method = RestRequest.Method.POST; + RestRequest.Method method = POST; List wrongRequiredParams = Arrays.asList("nonexistent_param"); @@ -73,7 +77,7 @@ public void testCompareRequiredFieldsFailure() throws Exception { public void testCompareRequiredFieldsThrowsException() throws Exception { String invalidUri = "http://invalid-url.com/fail.yaml"; String path = "/_plugins/_ml/agents/_register"; - RestRequest.Method method = RestRequest.Method.POST; + RestRequest.Method method = PUT; Exception exception = expectThrows( Exception.class, @@ -90,7 +94,7 @@ public void testUnsupportedMethodException() throws IllegalArgumentException { List.of("name", "type"), ML_COMMONS_API_SPEC_YAML_URI, "/_plugins/_ml/agents/_register", - RestRequest.Method.PATCH + PATCH ); }); @@ -103,7 +107,7 @@ public void testNoOperationFoundException() throws Exception { List.of("name", "type"), ML_COMMONS_API_SPEC_YAML_URI, "/_plugins/_ml/agents/_register", - RestRequest.Method.DELETE // PATCH should be unsupported + DELETE ); });