redskap / swagger-brake

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

The attempt to fix recursive schemas causes NPE #15

Closed dalewking closed 5 years ago

dalewking commented 5 years ago

Imagine some schemas in components like these where I whittle it down to only enough properties to demonstrate the problem:

Pageable:
  type: object
  properties:
    sort:
      $ref: '#/components/schemas/Sort'
  title: Pageable
Sort:
  type: object
  properties:
    sorted:
      type: boolean
  title: Sort
Page_Foo_:
  type: object
  properties:
    pageable:
      $ref: '#/components/schemas/Pageable'
    sort:
      $ref: '#/components/schemas/Sort'

This causes an NPE because the sort property $ref is not expanded, because #/components/schemas/Sort was seen when expanding Pageable.

dalewking commented 5 years ago

Here is a diff that should fix the problem:

diff --git a/src/main/java/io/redskap/swagger/brake/core/model/transformer/SchemaTransformer.java b/src/main/java/io/redskap/swagger/brake/core/model/transformer/SchemaTransformer.java
index eeb1524..2fa60cd 100644
--- a/src/main/java/io/redskap/swagger/brake/core/model/transformer/SchemaTransformer.java
+++ b/src/main/java/io/redskap/swagger/brake/core/model/transformer/SchemaTransformer.java
@@ -46,7 +46,9 @@ public class SchemaTransformer implements Transformer<io.swagger.v3.oas.models.m
         if (isNotBlank(ref) && SeenRefHolder.isNotSeen(ref)) {
             io.swagger.v3.oas.models.media.Schema resolvedSchema = getSchemaForRef(ref);
             SeenRefHolder.store(ref);
-            return internalTransform(resolvedSchema);
+            Schema schema = internalTransform(resolvedSchema);
+            SeenRefHolder.remove(ref);
+            return schema;
         }

         Schema.Builder schemaBuilder = new Schema.Builder(swSchema.getType());
@@ -112,6 +114,10 @@ public class SchemaTransformer implements Transformer<io.swagger.v3.oas.models.m
             get().add(refName);
         }

+        static void remove(String refName) {
+            get().remove(refName);
+        }
+
         static void clear() {
             HOLDER.remove();
         }
galovics commented 5 years ago

Fixed.