Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Header Object missing attributes #4608

Merged
merged 13 commits into from Feb 1, 2024
Merged

Conversation

micryc
Copy link
Contributor

@micryc micryc commented Jan 29, 2024

Refs #4196

@micryc micryc requested a review from frantuma January 29, 2024 10:51
Copy link
Member

@frantuma frantuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems great, only missing thing on top of unused import and new lines in testwould be - as you mentioned - support for schema.implementation / components.

As a test you could use something like:

    @Test
    public void testOperationWithResponseArraySchemaImplementation() {
        Reader reader = new Reader(new OpenAPI());
        OpenAPI openAPI = reader.read(GetOperationResponseHeaderWithArraySchemaImplementation.class);
        Yaml.prettyPrint(openAPI);
        // would expect
        String expectedYAML = "openapi: 3.0.1\n" +
                "paths:\n" +
                "  /path:\n" +
                "    get:\n" +
                "      summary: Simple get operation\n" +
                "      description: Defines a simple get operation with no inputs and a complex output\n" +
                "      operationId: getWithPayloadResponse\n" +
                "      responses:\n" +
                "        \"200\":\n" +
                "          description: voila!\n" +
                "          headers:\n" +
                "            Rate-Limit-Limit:\n" +
                "              description: The number of allowed requests in the current period\n" +
                "              style: simple\n" +
                "              schema:\n" +
                "                maxItems: 10\n" +
                "                minItems: 1\n" +
                "                type: array\n" +
                "                items:\n" +
                "                  $ref: '#/components/schemas/SampleResponseSchema'\n" +
                "      deprecated: true\n" +
                "components:\n" +
                "  schemas:\n" +
                "    SampleResponseSchema:\n" +
                "      type: object\n" +
                "      properties:\n" +
                "        id:\n" +
                "          type: string\n" +
                "          description: the user id";
        assertEquals(expectedYAML, Yaml.pretty(openAPI));
    }

    static class GetOperationResponseHeaderWithArraySchemaImplementation {
        @Operation(
                summary = "Simple get operation",
                description = "Defines a simple get operation with no inputs and a complex output",
                operationId = "getWithPayloadResponse",
                deprecated = true,
                responses = {
                        @ApiResponse(
                                responseCode = "200",
                                description = "voila!",
                                headers = {@Header(
                                        name = "Rate-Limit-Limit",
                                        description = "The number of allowed requests in the current period",
                                        array = @ArraySchema(maxItems = 10, minItems = 1,schema = @Schema(implementation = SampleResponseSchema.class)))})})
        @GET
        @Path("/path")
        public void simpleGet() {
        }
    }

this would mean passing components from OperationParser to the AnnotationUtils methods pipeline, as done in other cases. We must maintain backward compatibility, therefore new methods with new components param must be invoked from existing ones (passing null as param)

@@ -33,6 +34,7 @@
import io.swagger.v3.oas.models.media.JsonSchema;
import io.swagger.v3.oas.models.media.MediaType;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.parameters.Parameter;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used import?

@@ -359,6 +362,139 @@ static class GetOperationWithResponseMultipleHeaders {
public void simpleGet() {
}
}
@Test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a new line (applies to all added methods)

@micryc micryc requested a review from frantuma February 1, 2024 09:23
@@ -1287,18 +1287,18 @@ public static Map<String, String> getLinkParameters(LinkParameter[] linkParamete
return linkParametersMap;
}

public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotations.headers.Header[] annotationHeaders, JsonView jsonViewAnnotation) {
return getHeaders(annotationHeaders, jsonViewAnnotation, false);
public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotations.headers.Header[] annotationHeaders, Components components, JsonView jsonViewAnnotation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned we need to maintain APIs backward compatibility, keeping previous method signature calling new one:

public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotations.headers.Header[] annotationHeaders, JsonView jsonViewAnnotation) {
    return getHeaders(annotationHeaders, null, jsonViewAnnotation, false);
}

}

public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotations.headers.Header[] annotationHeaders, JsonView jsonViewAnnotation, boolean openapi31) {
public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotations.headers.Header[] annotationHeaders, Components components, JsonView jsonViewAnnotation, boolean openapi31) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIs backward compatibility, keeping previous method signature calling new one:

public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotations.headers.Header[] annotationHeaders, JsonView jsonViewAnnotation, boolean openapi31) {
    return getHeaders(annotationHeaders, null, jsonViewAnnotation, openapi31);
}

@@ -1307,13 +1307,13 @@ public static Optional<Map<String, Header>> getHeaders(io.swagger.v3.oas.annotat
return Optional.of(headers);
}

public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.Header header, JsonView jsonViewAnnotation) {
return getHeader(header, jsonViewAnnotation, false);
public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.Header header, Components components, JsonView jsonViewAnnotation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIs backward compatibility, keeping previous method signature calling new one:

public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.Header header, JsonView jsonViewAnnotation) {
    return getHeader(header, null, jsonViewAnnotation);
}

}

public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.Header header, JsonView jsonViewAnnotation, boolean openapi31) {
public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.Header header, Components components, JsonView jsonViewAnnotation, boolean openapi31) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIs backward compatibility, keeping previous method signature calling new one:

public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.Header header, JsonView jsonViewAnnotation, boolean openapi31) {
    return getHeader(header, null, jsonViewAnnotation, openapi31);
}

@@ -46,7 +46,7 @@ public void extensionsTest(String methodName,
.flatMap(response -> Arrays.stream(response.headers())).toArray(Header[]::new);

final Optional<Map<String, io.swagger.v3.oas.models.headers.Header>> optionalMap =
AnnotationsUtils.getHeaders(headers, null);
AnnotationsUtils.getHeaders(headers,null, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a space after comma

}
}
if (hasArrayAnnotation(header.array())){
AnnotationsUtils.getArraySchema(header.array(), components, jsonViewAnnotation, openapi31,null, true).ifPresent(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a space after comma

@micryc micryc requested a review from frantuma February 1, 2024 11:04
@micryc
Copy link
Contributor Author

micryc commented Feb 1, 2024

Fixes #2965 also.

@micryc micryc merged commit 0d067d0 into master Feb 1, 2024
6 checks passed
@micryc micryc deleted the issue-4196-HeaderObject-attributes branch February 1, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants