redskap / swagger-brake

Swagger contract checker for breaking API changes
Apache License 2.0
58 stars 16 forks source link

Need null checks for getContent #6

Closed dalewking closed 5 years ago

dalewking commented 5 years ago

Had a Swagger 2.0 file that I converted to 3.0 and when ran with your tool would fail with null pointer exceptions. These occurred in ApiResponseTransformer and RequestBodyTransformer because getContent() returned null.

I think you need some null checking like this (version for ApiResponseTransformer):

   final Content content = from.getValue().getContent();
   Set<Map.Entry<String, MediaType>> entries = content != null ? content.entrySet() : Collections.emptySet();

When I added the null checks to those classes the check worked as expected

dalewking commented 5 years ago

Or better yet using Java 8 Optional:

Set<Map.Entry<String, MediaType>> entries = Optional.ofNullable(from.getValue().getContent())
        .map(Map::entrySet)
        .orElseGet(Collections::emptySet);

Or even further:

    Map<String, Schema> schemaRefs = Optional.ofNullable(from.getValue().getContent())
            .map(Map::entrySet)
            .map(Set::stream)
            .map(stream -> stream.collect(toMap(Map.Entry::getKey,
                        e-> mediaTypeTransformer.transform(e.getValue()))))
            .orElseGet(Collections::emptyMap);
galovics commented 5 years ago

Thanks for reporting @dalewking. Could you please provide an example specification with which the issue is reproducible? Thanks!

dalewking commented 5 years ago

I am fine with providing it to you personally to investigate, but do not want to publish it publicly. I'll see if I can provide a sanitized stripped down simple example.

dalewking commented 5 years ago

So actually I could pair it down to just the acuator/info route that gets added to the project. It may not be actually valid OpenAPI:

openapi: 3.0.0
info:
  title: EXAMPLE
  version: '1'
paths:
  /actuator/info:
    get:
      tags:
        - operation-handler
      summary: handle
      operationId: handleUsingGET_1
      responses:
        '200':
          description: OK
          responseSchema:
            type: object
          content:
            application/json:
              schema:
                type: object
            application/vnd.spring-boot.actuator.v2+json:
              schema:
                type: object
        '401':
          description: Unauthorized
        '403':
          description: Forbidden
        '404':
          description: Not Found
      deprecated: false
      requestBody:
        $ref: '#/components/requestBodies/handleUsingGETBody'
components:
  requestBodies:
    handleUsingGETBody:
      content:
        application/json:
          schema:
            type: object
            additionalProperties:
              type: string
      description: body
dalewking commented 5 years ago

I wonder if this case the issue is that you are not following the $ref to get the content

dalewking commented 5 years ago

Yes, that appears to be the problem. There is a content entry, but you have to follow the $ref to get to it.

dalewking commented 5 years ago

Since the issue is really that you are not supporting reference objects for anything other than Schema, I will close this and create a new issue for the real problem