From 6031c9d9ac44c14827e4f079ab3e699191f57fc5 Mon Sep 17 00:00:00 2001 From: Arnold Galovics Date: Sun, 9 Dec 2018 16:18:00 +0100 Subject: [PATCH] #8 Stack overflow on recursive schemas fix --- .../model/transformer/SchemaTransformer.java | 50 ++++++- .../NoBreakingChangeIntTest.java} | 5 +- .../RecursiveSchemaIntTest.java | 26 ++++ .../RecursiveTypeAttributeRemovedIntTest.java | 32 +++++ .../nobreakingchange/recursive/schema.json | 128 ++++++++++++++++++ .../nobreakingchange/recursive/schema_v2.json | 128 ++++++++++++++++++ .../schema.json | 128 ++++++++++++++++++ .../schema_v2.json | 125 +++++++++++++++++ 8 files changed, 613 insertions(+), 9 deletions(-) rename src/test/java/io/redskap/swagger/brake/integration/{NoBreakingChangeTest.java => nobreakingchange/NoBreakingChangeIntTest.java} (75%) create mode 100644 src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/RecursiveSchemaIntTest.java create mode 100644 src/test/java/io/redskap/swagger/brake/integration/response/RecursiveTypeAttributeRemovedIntTest.java create mode 100644 src/test/resources/nobreakingchange/recursive/schema.json create mode 100644 src/test/resources/nobreakingchange/recursive/schema_v2.json create mode 100644 src/test/resources/response/recursiveresponseattributeremoved/schema.json create mode 100644 src/test/resources/response/recursiveresponseattributeremoved/schema_v2.json 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 ef79d53a..eeb1524b 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 @@ -1,10 +1,9 @@ package io.redskap.swagger.brake.core.model.transformer; import static java.util.stream.Collectors.toList; +import static org.apache.commons.lang3.StringUtils.isNotBlank; -import java.util.Collections; -import java.util.List; -import java.util.Map; +import java.util.*; import io.redskap.swagger.brake.core.model.Schema; import io.redskap.swagger.brake.core.model.SchemaAttribute; @@ -14,7 +13,6 @@ import io.swagger.v3.oas.models.media.ArraySchema; import lombok.RequiredArgsConstructor; import org.apache.commons.collections4.CollectionUtils; -import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Component; @Component @@ -24,7 +22,11 @@ public class SchemaTransformer implements Transformer new IllegalStateException("Reference not found for " + refName)); } + + /* + * The purpose of this class is to keep track of already seen schema references to avoid recursive schemas breaking the functionality. + */ + private static class SeenRefHolder { + private static final ThreadLocal> HOLDER = new ThreadLocal<>(); + + private static Collection get() { + Collection seenRefs = HOLDER.get(); + if (seenRefs == null) { + seenRefs = new HashSet<>(); + HOLDER.set(seenRefs); + } + return seenRefs; + } + + static boolean isSeen(String refName) { + return get().contains(refName); + } + + static boolean isNotSeen(String refName) { + return !isSeen(refName); + } + + static void store(String refName) { + get().add(refName); + } + + static void clear() { + HOLDER.remove(); + } + } } diff --git a/src/test/java/io/redskap/swagger/brake/integration/NoBreakingChangeTest.java b/src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/NoBreakingChangeIntTest.java similarity index 75% rename from src/test/java/io/redskap/swagger/brake/integration/NoBreakingChangeTest.java rename to src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/NoBreakingChangeIntTest.java index 70829882..88c549bc 100644 --- a/src/test/java/io/redskap/swagger/brake/integration/NoBreakingChangeTest.java +++ b/src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/NoBreakingChangeIntTest.java @@ -1,16 +1,17 @@ -package io.redskap.swagger.brake.integration; +package io.redskap.swagger.brake.integration.nobreakingchange; import static org.assertj.core.api.Assertions.assertThat; import java.util.Collection; import io.redskap.swagger.brake.core.BreakingChange; +import io.redskap.swagger.brake.integration.AbstractSwaggerBrakeIntTest; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.test.context.junit4.SpringRunner; @RunWith(SpringRunner.class) -public class NoBreakingChangeTest extends AbstractSwaggerBrakeIntTest { +public class NoBreakingChangeIntTest extends AbstractSwaggerBrakeIntTest { @Test public void testNoBreakingChangeWorksCorrectly() { // given diff --git a/src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/RecursiveSchemaIntTest.java b/src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/RecursiveSchemaIntTest.java new file mode 100644 index 00000000..cf94ec29 --- /dev/null +++ b/src/test/java/io/redskap/swagger/brake/integration/nobreakingchange/RecursiveSchemaIntTest.java @@ -0,0 +1,26 @@ +package io.redskap.swagger.brake.integration.nobreakingchange; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Collection; + +import io.redskap.swagger.brake.core.BreakingChange; +import io.redskap.swagger.brake.integration.AbstractSwaggerBrakeIntTest; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.test.context.junit4.SpringRunner; + +@RunWith(SpringRunner.class) +public class RecursiveSchemaIntTest extends AbstractSwaggerBrakeIntTest { + @Test + public void testNoBreakingChangeWorksCorrectly() { + // given + String oldApiPath = "nobreakingchange/recursive/schema.json"; + String newApiPath = "nobreakingchange/recursive/schema_v2.json"; + // when + Collection result = execute(oldApiPath, newApiPath); + // then + assertThat(result).isEmpty(); + } +} + diff --git a/src/test/java/io/redskap/swagger/brake/integration/response/RecursiveTypeAttributeRemovedIntTest.java b/src/test/java/io/redskap/swagger/brake/integration/response/RecursiveTypeAttributeRemovedIntTest.java new file mode 100644 index 00000000..403d1801 --- /dev/null +++ b/src/test/java/io/redskap/swagger/brake/integration/response/RecursiveTypeAttributeRemovedIntTest.java @@ -0,0 +1,32 @@ +package io.redskap.swagger.brake.integration.response; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Collection; +import java.util.Collections; + +import io.redskap.swagger.brake.core.BreakingChange; +import io.redskap.swagger.brake.core.model.HttpMethod; +import io.redskap.swagger.brake.core.rule.response.ResponseTypeAttributeRemovedBreakingChange; +import io.redskap.swagger.brake.integration.AbstractSwaggerBrakeIntTest; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.test.context.junit4.SpringRunner; + +@RunWith(SpringRunner.class) +public class RecursiveTypeAttributeRemovedIntTest extends AbstractSwaggerBrakeIntTest { + @Test + public void testResponseTypeChangeIsBreakingChangeWhenExistingAttributeRemoved() { + // given + String oldApiPath = "response/recursiveresponseattributeremoved/schema.json"; + String newApiPath = "response/recursiveresponseattributeremoved/schema_v2.json"; + ResponseTypeAttributeRemovedBreakingChange bc1 = + new ResponseTypeAttributeRemovedBreakingChange("/api/v1/audits/summary/{businessId}", HttpMethod.GET, "200", "unverifiedPayoffBreakdown.amountApplied"); + Collection expected = Collections.singletonList(bc1); + // when + Collection result = execute(oldApiPath, newApiPath); + // then + assertThat(result).hasSize(1); + assertThat(result).hasSameElementsAs(expected); + } +} diff --git a/src/test/resources/nobreakingchange/recursive/schema.json b/src/test/resources/nobreakingchange/recursive/schema.json new file mode 100644 index 00000000..81755657 --- /dev/null +++ b/src/test/resources/nobreakingchange/recursive/schema.json @@ -0,0 +1,128 @@ +{ + "swagger": "2.0", + "host": "localhost", + "basePath": "/", + "tags": [ + { + "name": "audit-controller", + "description": "Audit Controller" + }, + { + "name": "audit-fee-controller", + "description": "Audit Fee Controller" + }, + { + "name": "operation-handler", + "description": "Operation Handler" + } + ], + "paths": { + "/api/v1/audits/summary/{businessId}": { + "get": { + "tags": [ + "audit-controller" + ], + "summary": "fetchAuditSummaryByBusinessId", + "operationId": "fetchAuditSummaryByBusinessIdUsingGET", + "produces": [ + "*/*" + ], + "parameters": [ + { + "name": "businessId", + "in": "path", + "description": "businessId", + "required": true, + "type": "string", + "format": "uuid" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/AuditSummaryDTO" + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + }, + "404": { + "description": "Not Found" + } + }, + "security": [ + { + "jwt": [ + "global" + ] + } + ], + "deprecated": false + } + } + }, + "securityDefinitions": { + "jwt": { + "type": "apiKey", + "name": "Authorization", + "in": "header" + } + }, + "definitions": { + "AuditSummaryDTO": { + "type": "object", + "properties": { + "payoffSavings": { + "type": "number" + }, + "unverifiedPayoff": { + "type": "number" + }, + "unverifiedPayoffBreakdown": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "unverifiedVehicles": { + "type": "integer", + "format": "int64" + } + }, + "title": "AuditSummaryDTO" + }, + "PaymentItemDTO": { + "type": "object", + "properties": { + "amountApplied": { + "type": "number" + }, + "children": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "description": { + "type": "string" + }, + "recordType": { + "type": "string", + "enum": [ + "PRINCIPAL", + "INTEREST", + "CPP", + "TRANSPORTATION_FEE", + "AUDIT_FEE", + "OTHER_FEE" + ] + } + }, + "title": "PaymentItemDTO" + } + } +} diff --git a/src/test/resources/nobreakingchange/recursive/schema_v2.json b/src/test/resources/nobreakingchange/recursive/schema_v2.json new file mode 100644 index 00000000..81755657 --- /dev/null +++ b/src/test/resources/nobreakingchange/recursive/schema_v2.json @@ -0,0 +1,128 @@ +{ + "swagger": "2.0", + "host": "localhost", + "basePath": "/", + "tags": [ + { + "name": "audit-controller", + "description": "Audit Controller" + }, + { + "name": "audit-fee-controller", + "description": "Audit Fee Controller" + }, + { + "name": "operation-handler", + "description": "Operation Handler" + } + ], + "paths": { + "/api/v1/audits/summary/{businessId}": { + "get": { + "tags": [ + "audit-controller" + ], + "summary": "fetchAuditSummaryByBusinessId", + "operationId": "fetchAuditSummaryByBusinessIdUsingGET", + "produces": [ + "*/*" + ], + "parameters": [ + { + "name": "businessId", + "in": "path", + "description": "businessId", + "required": true, + "type": "string", + "format": "uuid" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/AuditSummaryDTO" + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + }, + "404": { + "description": "Not Found" + } + }, + "security": [ + { + "jwt": [ + "global" + ] + } + ], + "deprecated": false + } + } + }, + "securityDefinitions": { + "jwt": { + "type": "apiKey", + "name": "Authorization", + "in": "header" + } + }, + "definitions": { + "AuditSummaryDTO": { + "type": "object", + "properties": { + "payoffSavings": { + "type": "number" + }, + "unverifiedPayoff": { + "type": "number" + }, + "unverifiedPayoffBreakdown": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "unverifiedVehicles": { + "type": "integer", + "format": "int64" + } + }, + "title": "AuditSummaryDTO" + }, + "PaymentItemDTO": { + "type": "object", + "properties": { + "amountApplied": { + "type": "number" + }, + "children": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "description": { + "type": "string" + }, + "recordType": { + "type": "string", + "enum": [ + "PRINCIPAL", + "INTEREST", + "CPP", + "TRANSPORTATION_FEE", + "AUDIT_FEE", + "OTHER_FEE" + ] + } + }, + "title": "PaymentItemDTO" + } + } +} diff --git a/src/test/resources/response/recursiveresponseattributeremoved/schema.json b/src/test/resources/response/recursiveresponseattributeremoved/schema.json new file mode 100644 index 00000000..81755657 --- /dev/null +++ b/src/test/resources/response/recursiveresponseattributeremoved/schema.json @@ -0,0 +1,128 @@ +{ + "swagger": "2.0", + "host": "localhost", + "basePath": "/", + "tags": [ + { + "name": "audit-controller", + "description": "Audit Controller" + }, + { + "name": "audit-fee-controller", + "description": "Audit Fee Controller" + }, + { + "name": "operation-handler", + "description": "Operation Handler" + } + ], + "paths": { + "/api/v1/audits/summary/{businessId}": { + "get": { + "tags": [ + "audit-controller" + ], + "summary": "fetchAuditSummaryByBusinessId", + "operationId": "fetchAuditSummaryByBusinessIdUsingGET", + "produces": [ + "*/*" + ], + "parameters": [ + { + "name": "businessId", + "in": "path", + "description": "businessId", + "required": true, + "type": "string", + "format": "uuid" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/AuditSummaryDTO" + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + }, + "404": { + "description": "Not Found" + } + }, + "security": [ + { + "jwt": [ + "global" + ] + } + ], + "deprecated": false + } + } + }, + "securityDefinitions": { + "jwt": { + "type": "apiKey", + "name": "Authorization", + "in": "header" + } + }, + "definitions": { + "AuditSummaryDTO": { + "type": "object", + "properties": { + "payoffSavings": { + "type": "number" + }, + "unverifiedPayoff": { + "type": "number" + }, + "unverifiedPayoffBreakdown": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "unverifiedVehicles": { + "type": "integer", + "format": "int64" + } + }, + "title": "AuditSummaryDTO" + }, + "PaymentItemDTO": { + "type": "object", + "properties": { + "amountApplied": { + "type": "number" + }, + "children": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "description": { + "type": "string" + }, + "recordType": { + "type": "string", + "enum": [ + "PRINCIPAL", + "INTEREST", + "CPP", + "TRANSPORTATION_FEE", + "AUDIT_FEE", + "OTHER_FEE" + ] + } + }, + "title": "PaymentItemDTO" + } + } +} diff --git a/src/test/resources/response/recursiveresponseattributeremoved/schema_v2.json b/src/test/resources/response/recursiveresponseattributeremoved/schema_v2.json new file mode 100644 index 00000000..2e143595 --- /dev/null +++ b/src/test/resources/response/recursiveresponseattributeremoved/schema_v2.json @@ -0,0 +1,125 @@ +{ + "swagger": "2.0", + "host": "localhost", + "basePath": "/", + "tags": [ + { + "name": "audit-controller", + "description": "Audit Controller" + }, + { + "name": "audit-fee-controller", + "description": "Audit Fee Controller" + }, + { + "name": "operation-handler", + "description": "Operation Handler" + } + ], + "paths": { + "/api/v1/audits/summary/{businessId}": { + "get": { + "tags": [ + "audit-controller" + ], + "summary": "fetchAuditSummaryByBusinessId", + "operationId": "fetchAuditSummaryByBusinessIdUsingGET", + "produces": [ + "*/*" + ], + "parameters": [ + { + "name": "businessId", + "in": "path", + "description": "businessId", + "required": true, + "type": "string", + "format": "uuid" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/AuditSummaryDTO" + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + }, + "404": { + "description": "Not Found" + } + }, + "security": [ + { + "jwt": [ + "global" + ] + } + ], + "deprecated": false + } + } + }, + "securityDefinitions": { + "jwt": { + "type": "apiKey", + "name": "Authorization", + "in": "header" + } + }, + "definitions": { + "AuditSummaryDTO": { + "type": "object", + "properties": { + "payoffSavings": { + "type": "number" + }, + "unverifiedPayoff": { + "type": "number" + }, + "unverifiedPayoffBreakdown": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "unverifiedVehicles": { + "type": "integer", + "format": "int64" + } + }, + "title": "AuditSummaryDTO" + }, + "PaymentItemDTO": { + "type": "object", + "properties": { + "children": { + "type": "array", + "items": { + "$ref": "#/definitions/PaymentItemDTO" + } + }, + "description": { + "type": "string" + }, + "recordType": { + "type": "string", + "enum": [ + "PRINCIPAL", + "INTEREST", + "CPP", + "TRANSPORTATION_FEE", + "AUDIT_FEE", + "OTHER_FEE" + ] + } + }, + "title": "PaymentItemDTO" + } + } +}