SnapshotStubMappingPostProcessor correctly handle scenarios with transformed stubs

Revises SnapshotStubMappingPostProcessor to run stub transformers first. That way, if the transformer results in StubMapping requests being equal they are properly deduplicated or turned into scenarios. Cleans up process method to make clearer the three phases of processing. Additionally: - Converts SnapshotStubMappingPostProcessorTest.TEST_STUB_MAPPINGS to instance variable to avoid sharing mutable state between test cases which caused the suite to be order dependent. Fixes: wiremock#2137

Author: gsmith85

src/main/java/com/github/tomakehurst/wiremock/recording/SnapshotStubMappingPostProcessor.java

@@ -18,16 +18,17 @@
 import com.github.tomakehurst.wiremock.matching.RequestPattern;
 import com.github.tomakehurst.wiremock.stubbing.StubMapping;
 import com.google.common.collect.HashMultiset;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Multiset;
 import java.util.ArrayList;
 import java.util.List;
 
 /**
- * Performs stateful post-processing tasks on stub mappings generated from ServeEvents: 1. Detect
- * duplicate requests and either discard them or turn them into scenarios 2. Extract response bodies
- * to a separate file, if applicable 3. Run any applicable StubMappingTransformers against the stub
- * mappings
+ * Performs stateful post-processing tasks on stub mappings generated from ServeEvents:
+ * <ol>
+ * <li>Run any applicable StubMappingTransformers against the stub mappings.</li>
+ * <li>Detect duplicate requests and either discard them or turn them into scenarios.</li>
+ * <li>Extract response bodies to a separate file, if applicable.</li>
+ * </ol>
  */
 public class SnapshotStubMappingPostProcessor {
  private final boolean shouldRecordRepeatsAsScenarios;
@@ -47,30 +48,45 @@ public SnapshotStubMappingPostProcessor(
  }
 
  public List<StubMapping> process(Iterable<StubMapping> stubMappings) {
- final Multiset<RequestPattern> requestCounts = HashMultiset.create();
- final List<StubMapping> processedStubMappings = new ArrayList<>();
-
+ // 1. Run any applicable StubMappingTransformers against the stub mappings.
+ ArrayList<StubMapping> transformedStubMappings = new ArrayList<>();
  for (StubMapping stubMapping : stubMappings) {
- requestCounts.add(stubMapping.getRequest());
+ transformedStubMappings.add(transformerRunner.apply(stubMapping));
+ }
+ 
+ // 2. Detect duplicate requests and either discard them or turn them into scenarios.
+ Multiset<RequestPattern> requestCounts = HashMultiset.create();
+ List<StubMapping> processedStubMappings = new ArrayList<>();
+ for (StubMapping transformedStubMapping : transformedStubMappings) {
+ requestCounts.add(transformedStubMapping.getRequest());
 
  // Skip duplicate requests if shouldRecordRepeatsAsScenarios is not enabled
- if (requestCounts.count(stubMapping.getRequest()) > 1 && !shouldRecordRepeatsAsScenarios) {
+ if (requestCounts.count(transformedStubMapping.getRequest()) > 1 && !shouldRecordRepeatsAsScenarios) {
  continue;
  }
 
- if (bodyExtractMatcher != null
- && bodyExtractMatcher.match(stubMapping.getResponse()).isExactMatch()) {
- bodyExtractor.extractInPlace(stubMapping);
- }
-
- processedStubMappings.add(stubMapping);
+ processedStubMappings.add(transformedStubMapping);
  }
 
  if (shouldRecordRepeatsAsScenarios) {
  new ScenarioProcessor().putRepeatedRequestsInScenarios(processedStubMappings);
  }
 
- // Run any stub mapping transformer extensions
- return Lists.transform(processedStubMappings, transformerRunner);
+ // 3. Extract response bodies to a separate file, if applicable.
+ extractStubMappingBodies(processedStubMappings);
+
+ return processedStubMappings;
+ }
+ 
+ private void extractStubMappingBodies(List<StubMapping> stubMappings) {
+ if (bodyExtractMatcher == null) {
+ return;
+ }
+ 
+ for (StubMapping stubMapping : stubMappings) {
+ if (bodyExtractMatcher.match(stubMapping.getResponse()).isExactMatch()) {
+ bodyExtractor.extractInPlace(stubMapping);
+ }
+ }
  }
 }

src/test/java/com/github/tomakehurst/wiremock/recording/SnapshotStubMappingPostProcessorTest.java

@@ -18,33 +18,38 @@
 import static com.github.tomakehurst.wiremock.matching.RequestPatternBuilder.newRequestPattern;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.http.ResponseDefinition;
 import com.github.tomakehurst.wiremock.matching.MatchResult;
 import com.github.tomakehurst.wiremock.stubbing.StubMapping;
 import com.google.common.collect.ImmutableList;
 import java.util.List;
+
 import org.junit.jupiter.api.Test;
 
 public class SnapshotStubMappingPostProcessorTest {
- private static final List<StubMapping> TEST_STUB_MAPPINGS =
+ // NOTE: testStubMappings is not deeply immutable, as StubMappings are mutable, and to preserve hermeticity must be
+ // an instance rather than a class variable.
+ private final List<StubMapping> testStubMappings =
  ImmutableList.of(
  WireMock.get("/foo").build(), WireMock.get("/bar").build(), WireMock.get("/foo").build());
 
  @Test
- public void processFiltersRepeatedRequestsWhenNotRecordingScenarios() {
+ public void process_notRecordingScenarios_filtersRepeatedRequests() {
  final List<StubMapping> actual =
  new SnapshotStubMappingPostProcessor(false, noopTransformerRunner(), null, null)
- .process(TEST_STUB_MAPPINGS);
+ .process(testStubMappings);
 
  assertThat(actual, hasSize(2));
  assertThat(actual.get(0).getRequest().getUrl(), equalTo("/foo"));
  assertThat(actual.get(1).getRequest().getUrl(), equalTo("/bar"));
  }
 
  @Test
- public void processRunsTransformers() {
+ public void process_withTransformer() {
  SnapshotStubMappingTransformerRunner transformerRunner =
  new SnapshotStubMappingTransformerRunner(null) {
  @Override
@@ -58,21 +63,48 @@ public StubMapping apply(StubMapping stubMapping) {
 
  final List<StubMapping> actual =
  new SnapshotStubMappingPostProcessor(false, transformerRunner, null, null)
- .process(TEST_STUB_MAPPINGS);
+ .process(testStubMappings);
 
  assertThat(actual, hasSize(2));
  assertThat(actual.get(0).getRequest().getUrl(), equalTo("/foo/transformed"));
  assertThat(actual.get(1).getRequest().getUrl(), equalTo("/bar/transformed"));
  }
 
+ @Test
+ public void process_withShouldRecordRepeatsAsScenariosAndTransformer_runsTransformerBeforeScenarioProcessor() {
+ SnapshotStubMappingTransformerRunner transformerRunner =
+ new SnapshotStubMappingTransformerRunner(null) {
+ @Override
+ public StubMapping apply(StubMapping stubMapping) {
+ // Return StubMapping with "/transformed" at the end of the original URL
+ String url = stubMapping.getRequest().getUrl();
+ return new StubMapping(
+ newRequestPattern().withUrl(url + "/transformed").build(), ResponseDefinition.ok());
+ }
+ };
+
+ final List<StubMapping> actual =
+ new SnapshotStubMappingPostProcessor(true, transformerRunner, null, null)
+ .process(testStubMappings);
+
+ assertThat(actual, hasSize(3));
+ assertThat(actual.get(0).getRequest().getUrl(), equalTo("/foo/transformed"));
+ assertThat(actual.get(1).getRequest().getUrl(), equalTo("/bar/transformed"));
+ assertThat(actual.get(2).getRequest().getUrl(), equalTo("/foo/transformed"));
+
+ assertTrue(actual.get(0).isInScenario());
+ assertFalse(actual.get(1).isInScenario());
+ assertTrue(actual.get(2).isInScenario());
+ }
+
  @Test
- public void processExtractsBodiesWhenMatched() {
+ public void process_withBodyExtractMatcherAndBodyExtractor_extractsBodiesWhenMatched() {
  final ResponseDefinitionBodyMatcher bodyMatcher =
  new ResponseDefinitionBodyMatcher(0, 0) {
  @Override
  public MatchResult match(ResponseDefinition responseDefinition) {
  // Only match the second stub mapping
- return responseDefinition == TEST_STUB_MAPPINGS.get(1).getResponse()
+ return responseDefinition == testStubMappings.get(1).getResponse()
  ? MatchResult.exactMatch()
  : MatchResult.noMatch();
  }
@@ -89,7 +121,7 @@ public void extractInPlace(StubMapping stubMapping) {
  final List<StubMapping> actual =
  new SnapshotStubMappingPostProcessor(
  false, noopTransformerRunner(), bodyMatcher, bodyExtractor)
- .process(TEST_STUB_MAPPINGS);
+ .process(testStubMappings);
 
  assertThat(actual, hasSize(2));
  // Should've only modified second stub mapping