diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java index 0413f79312..5e6066d8c1 100644 --- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java +++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java @@ -137,7 +137,8 @@ public String processRefToExternalSchema(String $ref, RefFormat refFormat) { } schemaFullRef = FilenameUtils.separatorsToUnix(schemaFullRef); } - schema.set$ref(processRefToExternalSchema(schemaFullRef, ref)); + // Fix for issue-1865: use internal prefix + schema.set$ref(RefType.SCHEMAS.getInternalPrefix()+ processRefToExternalSchema(schemaFullRef, ref)); } } else { processRefToExternalSchema(file + schema.get$ref(), RefFormat.RELATIVE); diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java index 561a3b5679..d3ca9a7ad6 100644 --- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java +++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java @@ -47,14 +47,11 @@ public static String computeDefinitionName(String ref) { final String[] filePathElements = file.split("/"); plausibleName = filePathElements[filePathElements.length - 1]; - final String[] split = plausibleName.split("\\."); - // Fix for issue-1621 and issue-1865 - //validate number of dots - if(split.length > 2) { - //Remove dot so ref can be interpreted as internal and relative in Swagger-Core schema class 'set$ref' - plausibleName = String.join("", Arrays.copyOf(split, split.length - 1)); - }else{ - plausibleName = split[0]; + // Fix for issue-1621 and issue-2092 + // Remove only the file extension from the plausible name but keep all other dots + int lastDotIndex = plausibleName.lastIndexOf('.'); + if (lastDotIndex > 0) { + plausibleName = plausibleName.substring(0, lastDotIndex); } } @@ -90,7 +87,7 @@ public static RefFormat computeRefFormat(String ref) { } public static String mungedRef(String refString) { - // Ref: IETF RFC 3966, Section 5.2.2 + // Ref: IETF RFC 3986, Section 5.2.2 if (!refString.contains(":") && // No scheme !refString.startsWith("#") && // Path is not empty !refString.startsWith("/") && // Path is not absolute diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/DottedNonSchemaComponentTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/DottedNonSchemaComponentTest.java new file mode 100644 index 0000000000..6b4b643162 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/DottedNonSchemaComponentTest.java @@ -0,0 +1,217 @@ +package io.swagger.v3.parser.processors; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.callbacks.Callback; +import io.swagger.v3.oas.models.examples.Example; +import io.swagger.v3.oas.models.headers.Header; +import io.swagger.v3.oas.models.links.Link; +import io.swagger.v3.oas.models.parameters.Parameter; +import io.swagger.v3.oas.models.parameters.RequestBody; +import io.swagger.v3.oas.models.responses.ApiResponse; +import io.swagger.v3.oas.models.security.SecurityScheme; +import io.swagger.v3.parser.ResolverCache; +import io.swagger.v3.parser.models.RefFormat; +import org.testng.annotations.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertSame; + +/** + * Regression coverage for dotted external non-schema components. Assertions describe the intended resolved output + * and intentionally fail where processors currently leave bare names, lose failed refs, or reuse a colliding key. + */ +public class DottedNonSchemaComponentTest { + + // Wrong: the parser leaves "error.response" as a bare ref. + // It should create the internal ref "#/components/responses/error.response". + @Test + public void testDottedExternalResponseUsesQualifiedInternalRef() { + String ref = "./responses/error.response.yaml"; + OpenAPI openAPI = new OpenAPI(); + ApiResponse resolved = new ApiResponse().description("Bad request"); + ApiResponse response = new ApiResponse().$ref(ref); + + new ResponseProcessor(cacheWith(openAPI, ref, resolved), openAPI).processResponse(response); + + assertEquals(response.get$ref(), "#/components/responses/error.response"); + assertSame(openAPI.getComponents().getResponses().get("error.response"), resolved); + } + + // Wrong: the parser leaves "create.request" as a bare ref. + // It should create the internal ref "#/components/requestBodies/create.request". + @Test + public void testDottedExternalRequestBodyUsesQualifiedInternalRef() { + String ref = "./request-bodies/create.request.yaml"; + OpenAPI openAPI = new OpenAPI(); + RequestBody resolved = new RequestBody().description("Create request"); + RequestBody requestBody = new RequestBody().$ref(ref); + + new RequestBodyProcessor(cacheWith(openAPI, ref, resolved), openAPI).processRequestBody(requestBody); + + assertEquals(requestBody.get$ref(), "#/components/requestBodies/create.request"); + assertSame(openAPI.getComponents().getRequestBodies().get("create.request"), resolved); + } + + // Wrong: the parser leaves "request.id" as a bare ref. + // It should create the internal ref "#/components/parameters/request.id". + @Test + public void testDottedExternalParameterUsesQualifiedInternalRef() { + String ref = "./parameters/request.id.yaml"; + OpenAPI openAPI = new OpenAPI(); + Parameter resolved = new Parameter().name("requestId").in("query"); + Parameter parameter = new Parameter().$ref(ref); + + new ParameterProcessor(cacheWith(openAPI, ref, resolved), openAPI).processParameter(parameter); + + assertEquals(parameter.get$ref(), "#/components/parameters/request.id"); + assertSame(openAPI.getComponents().getParameters().get("request.id"), resolved); + } + + // Wrong: the parser leaves "rate.limit" as a bare ref. + // It should create the internal ref "#/components/headers/rate.limit". + @Test + public void testDottedExternalHeaderUsesQualifiedInternalRef() { + String ref = "./headers/rate.limit.yaml"; + OpenAPI openAPI = new OpenAPI(); + Header resolved = new Header().description("Rate limit"); + Header header = new Header().$ref(ref); + + new HeaderProcessor(cacheWith(openAPI, ref, resolved), openAPI).processHeader(header); + + assertEquals(header.get$ref(), "#/components/headers/rate.limit"); + assertSame(openAPI.getComponents().getHeaders().get("rate.limit"), resolved); + } + + // Wrong: the parser leaves "error.example" as a bare ref. + // It should create the internal ref "#/components/examples/error.example". + @Test + public void testDottedExternalExampleUsesQualifiedInternalRef() { + String ref = "./examples/error.example.yaml"; + OpenAPI openAPI = new OpenAPI(); + Example resolved = new Example().summary("Error example"); + Example example = new Example().$ref(ref); + + new ExampleProcessor(cacheWith(openAPI, ref, resolved), openAPI).processExample(example); + + assertEquals(example.get$ref(), "#/components/examples/error.example"); + assertSame(openAPI.getComponents().getExamples().get("error.example"), resolved); + } + + // Wrong: the parser leaves "account.link" as a bare ref. + // It should create the internal ref "#/components/links/account.link". + @Test + public void testDottedExternalLinkUsesQualifiedInternalRef() { + String ref = "./links/account.link.yaml"; + OpenAPI openAPI = new OpenAPI(); + Link resolved = new Link().operationId("getAccount"); + Link link = new Link().$ref(ref); + + new LinkProcessor(cacheWith(openAPI, ref, resolved), openAPI).processLink(link); + + assertEquals(link.get$ref(), "#/components/links/account.link"); + assertSame(openAPI.getComponents().getLinks().get("account.link"), resolved); + } + + @Test + public void testDottedExternalCallbackUsesQualifiedInternalRef() { + String ref = "./callbacks/payment.callback.yaml"; + OpenAPI openAPI = new OpenAPI(); + Callback resolved = new Callback(); + Callback callback = new Callback(); + callback.set$ref(ref); + + new CallbackProcessor(cacheWith(openAPI, ref, resolved), openAPI).processCallback(callback); + + assertEquals(callback.get$ref(), "#/components/callbacks/payment.callback"); + assertSame(openAPI.getComponents().getCallbacks().get("payment.callback"), resolved); + } + + @Test + public void testDottedExternalSecuritySchemeUsesDottedComponentName() { + String ref = "./security/oauth.scheme.yaml"; + OpenAPI openAPI = new OpenAPI(); + SecurityScheme resolved = new SecurityScheme().type(SecurityScheme.Type.OAUTH2); + StubResolverCache cache = cacheWith(openAPI, ref, resolved); + + String name = new ExternalRefProcessor(cache, openAPI) + .processRefToExternalSecurityScheme(ref, RefFormat.RELATIVE); + + assertEquals(name, "oauth.scheme"); + assertSame(openAPI.getComponents().getSecuritySchemes().get("oauth.scheme"), resolved); + } + + @Test + public void testDottedExternalPathItemCachesDottedDerivedName() { + String ref = "./paths/account.path.yaml"; + OpenAPI openAPI = new OpenAPI(); + PathItem resolved = new PathItem(); + StubResolverCache cache = cacheWith(openAPI, ref, resolved); + + PathItem result = new ExternalRefProcessor(cache, openAPI) + .processRefToExternalPathItem(ref, RefFormat.RELATIVE); + + assertSame(result, resolved); + assertEquals(cache.getRenamedRef(ref), "account.path"); + } + + // Wrong: a missing file causes a NullPointerException. + // For example, "./responses/missing.response.yaml" should stay unchanged when it cannot be loaded. + @Test + public void testMissingExternalResponseRetainsOriginalRef() { + String ref = "./responses/missing.response.yaml"; + OpenAPI openAPI = new OpenAPI(); + ApiResponse response = new ApiResponse().$ref(ref); + + new ResponseProcessor(new StubResolverCache(openAPI), openAPI).processResponse(response); + + assertEquals(response.get$ref(), ref); + } + + // Wrong: both files are given the same name, "error.response", so the second response is lost. + // The second component should use "error.response_1". + @Test + public void testDottedExternalResponseCollisionUsesDeterministicSuffix() { + String firstRef = "./first/error.response.yaml"; + String secondRef = "./second/error.response.yaml"; + OpenAPI openAPI = new OpenAPI(); + StubResolverCache cache = new StubResolverCache(openAPI); + ApiResponse first = new ApiResponse().description("First"); + ApiResponse second = new ApiResponse().description("Second"); + cache.add(firstRef, first); + cache.add(secondRef, second); + ExternalRefProcessor processor = new ExternalRefProcessor(cache, openAPI); + + assertEquals(processor.processRefToExternalResponse(firstRef, RefFormat.RELATIVE), "error.response"); + assertEquals(processor.processRefToExternalResponse(secondRef, RefFormat.RELATIVE), "error.response_1"); + assertSame(openAPI.getComponents().getResponses().get("error.response"), first); + assertSame(openAPI.getComponents().getResponses().get("error.response_1"), second); + } + + private StubResolverCache cacheWith(OpenAPI openAPI, String ref, Object value) { + StubResolverCache cache = new StubResolverCache(openAPI); + cache.add(ref, value); + return cache; + } + + private static class StubResolverCache extends ResolverCache { + private final Map refs = new HashMap<>(); + + StubResolverCache(OpenAPI openAPI) { + super(openAPI, null, null); + } + + void add(String ref, Object value) { + refs.put(ref, value); + } + + @Override + public T loadRef(String ref, RefFormat refFormat, Class expectedType) { + Object value = refs.get(ref); + return value == null ? null : expectedType.cast(value); + } + } +} diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/ExternalRefProcessorTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/ExternalRefProcessorTest.java index 267591ad0c..0881e01756 100644 --- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/ExternalRefProcessorTest.java +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/processors/ExternalRefProcessorTest.java @@ -76,6 +76,29 @@ public void testProcessRefToExternalDefinition_NoNameConflict() throws Exception assertEquals(newRef, "bar"); } + // Error introduced by the PR. + // Wrong: a missing "../models/foo.model.yaml" becomes + // "#/components/schemas/../models/foo.model.yaml" instead of staying unchanged. + @Test + public void testNestedMissingExternalSchemaRetainsOriginalRef() { + final String outerRef = "../schemas/wrapper.yaml"; + final String missingRef = "../models/foo.model.yaml"; + final Schema wrapper = new Schema().$ref(missingRef); + final OpenAPI testedOpenAPI = new OpenAPI(); + + new Expectations() {{ + cache.loadRef(outerRef, RefFormat.RELATIVE, Schema.class); + result = wrapper; + + cache.loadRef(missingRef, RefFormat.RELATIVE, Schema.class); + result = null; + }}; + + new ExternalRefProcessor(cache, testedOpenAPI) + .processRefToExternalSchema(outerRef, RefFormat.RELATIVE); + + assertEquals(wrapper.get$ref(), missingRef); + } @Test diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java index 997f1c23bd..f3bdfff83e 100644 --- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java @@ -112,7 +112,7 @@ public void testIssue1865() throws Exception { Assert.assertNotNull(result); Assert.assertNotNull(result.getOpenAPI()); - Assert.assertEquals(result.getOpenAPI().getComponents().getSchemas().get("Foo").get$ref(), "#/components/schemas/foomodel"); + Assert.assertEquals(result.getOpenAPI().getComponents().getSchemas().get("Foo").get$ref(), "#/components/schemas/foo.model"); } @Test