From 5eab04c9706a4a7416e6f2bfc47558dfc2093619 Mon Sep 17 00:00:00 2001 From: Dominik Meister Date: Sat, 31 Oct 2020 09:13:33 +0100 Subject: [PATCH] Delete only valid mappings before generating stubs at runtime (#1547) * Delete only valid mappings before generating stubs at runtime Improvements over the changes in 6d44c54c. We now only delete files which contain valid mappings before generating stubs at runtime. This way we ensure that we actually use the newly generated stubs, but still are able to keep additional files around to be included via `body(file(...))`. * Incorporated PR feedback - Removed HttpServerStubFactory - Use SpringFactorieLoader to load HttpServerStub implementations - Added tests for WireMockHttpServerStub#isAccepted(File) --- .../stubrunner/StubRunnerFactory.java | 48 +++++++++++++++++++ .../wiremock/WireMockHttpServerStub.java | 15 +++++- .../contract/stubrunner/StubRunnerSpec.groovy | 1 - .../WireMockHttpServerStubSpec.groovy | 39 +++++++++++++++ .../src/test/resources/arbitrary.json | 12 +++++ .../src/test/resources/broken.json | 7 +++ 6 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-contract-stub-runner/src/test/resources/arbitrary.json create mode 100644 spring-cloud-contract-stub-runner/src/test/resources/broken.json diff --git a/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/StubRunnerFactory.java b/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/StubRunnerFactory.java index 32ec7c0244..eaeb3d39c6 100644 --- a/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/StubRunnerFactory.java +++ b/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/StubRunnerFactory.java @@ -18,18 +18,25 @@ package org.springframework.cloud.contract.stubrunner; import java.io.File; import java.io.IOException; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.cloud.contract.stubrunner.provider.wiremock.WireMockHttpServerStub; import org.springframework.cloud.contract.verifier.converter.RecursiveFilesConverter; import org.springframework.cloud.contract.verifier.messaging.MessageVerifier; import org.springframework.core.io.Resource; +import org.springframework.core.io.support.SpringFactoriesLoader; /** * Factory of StubRunners. Basing on the options and passed collaborators downloads the @@ -88,6 +95,7 @@ class StubRunnerFactory { } private void generateMappingsAtRuntime(Path path) { + removeCurrentMappings(path); generateNewMappings(path); } @@ -106,6 +114,46 @@ class StubRunnerFactory { return path; } + private void removeCurrentMappings(Path path) { + + List httpServerStubs = SpringFactoriesLoader + .loadFactories(HttpServerStub.class, null); + if (httpServerStubs.isEmpty()) { + httpServerStubs.add(new WireMockHttpServerStub()); + } + + try { + Files.walkFileTree(path, new SimpleFileVisitor() { + + private final Log log = LogFactory.getLog(StubRunnerFactory.class); + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { + + if (httpServerStubs.stream() + .anyMatch(h -> h.isAccepted(file.toFile()))) { + if (log.isDebugEnabled()) { + log.debug("Deleting file [" + file.toString() + + "] since it contains a valid mapping."); + } + + try { + Files.delete(file); + } + catch (IOException ex) { + log.warn("Failed to delete file [" + file.toString() + "]", + ex); + } + } + return FileVisitResult.CONTINUE; + } + }); + } + catch (IOException ex) { + log.warn("Exception occurred while trying to delete mappings", ex); + } + } + private void generateNewMappings(Path path) { File unpackedLocation = path.toFile(); RecursiveFilesConverter converter = new RecursiveFilesConverter( diff --git a/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStub.java b/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStub.java index 9e142d1f5b..62a4d9b233 100644 --- a/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStub.java +++ b/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStub.java @@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHashMap; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.common.JsonException; import com.github.tomakehurst.wiremock.common.Slf4jNotifier; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; import com.github.tomakehurst.wiremock.extension.Extension; @@ -237,7 +238,17 @@ public class WireMockHttpServerStub implements HttpServerStub { @Override public boolean isAccepted(File file) { - return file.getName().endsWith(".json"); + return file.getName().endsWith(".json") && validMapping(file); + } + + private boolean validMapping(File file) { + try { + getMapping(file); + return true; + } + catch (IllegalStateException e) { + return false; + } } StubMapping getMapping(File file) { @@ -245,7 +256,7 @@ public class WireMockHttpServerStub implements HttpServerStub { return StubMapping.buildFrom( StreamUtils.copyToString(stream, Charset.forName("UTF-8"))); } - catch (IOException e) { + catch (IOException | JsonException e) { throw new IllegalStateException("Cannot read file", e); } } diff --git a/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/StubRunnerSpec.groovy b/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/StubRunnerSpec.groovy index b2fa019598..d6913e2a17 100644 --- a/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/StubRunnerSpec.groovy +++ b/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/StubRunnerSpec.groovy @@ -57,7 +57,6 @@ class StubRunnerSpec extends Specification { runner.close() } - def 'should generate stubs at runtime'() { given: Arguments args = argumentsWithGenerateStubs() diff --git a/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStubSpec.groovy b/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStubSpec.groovy index 3a75c3cc32..e937068f24 100644 --- a/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStubSpec.groovy +++ b/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/provider/wiremock/WireMockHttpServerStubSpec.groovy @@ -30,6 +30,9 @@ import org.springframework.web.client.RestTemplate class WireMockHttpServerStubSpec extends Specification { public static final File MAPPING_DESCRIPTOR = new File('src/test/resources/transformers.json') + public static final File ARBITRARY_JSON = new File('src/test/resources/sample_response.json') + public static final File PDF = new File('src/test/resources/request.pdf') + public static final File BROKEN_MAPPING = new File('src/test/resources/broken.json') @Rule OutputCaptureRule capture = new OutputCaptureRule() @@ -76,4 +79,40 @@ class WireMockHttpServerStubSpec extends Specification { cleanup: mappingDescriptor?.stop() } + + def 'should accept a valid mapping'() { + given: + WireMockHttpServerStub httpServerStub = new WireMockHttpServerStub() + when: + boolean accepted = httpServerStub.isAccepted(MAPPING_DESCRIPTOR) + then: + accepted + } + + def 'should not accept an arbitrary JSON file'() { + given: + WireMockHttpServerStub httpServerStub = new WireMockHttpServerStub() + when: + boolean accepted = httpServerStub.isAccepted(ARBITRARY_JSON) + then: + !accepted + } + + def 'should not accept a broken mapping file'() { + given: + WireMockHttpServerStub httpServerStub = new WireMockHttpServerStub() + when: + boolean accepted = httpServerStub.isAccepted(BROKEN_MAPPING) + then: + !accepted + } + + def 'should not accept a non-JSON file'() { + given: + WireMockHttpServerStub httpServerStub = new WireMockHttpServerStub() + when: + boolean accepted = httpServerStub.isAccepted(PDF) + then: + !accepted + } } diff --git a/spring-cloud-contract-stub-runner/src/test/resources/arbitrary.json b/spring-cloud-contract-stub-runner/src/test/resources/arbitrary.json new file mode 100644 index 0000000000..35ecab0f56 --- /dev/null +++ b/spring-cloud-contract-stub-runner/src/test/resources/arbitrary.json @@ -0,0 +1,12 @@ +[ + { + "id": "1", + "firstname": "John", + "name": "Doe" + }, + { + "id": "2", + "firstname": "Lisa", + "name": "Smith" + } +] \ No newline at end of file diff --git a/spring-cloud-contract-stub-runner/src/test/resources/broken.json b/spring-cloud-contract-stub-runner/src/test/resources/broken.json new file mode 100644 index 0000000000..3ac85b61ad --- /dev/null +++ b/spring-cloud-contract-stub-runner/src/test/resources/broken.json @@ -0,0 +1,7 @@ +{ + "request": { + "method": "GET", + "url": "/ping" + }, + "response": { + "status": 200,