wiremock icon indicating copy to clipboard operation
wiremock copied to clipboard

Fix matcher distance short-circuit biases

Open RafeArnold opened this issue 1 year ago • 0 comments

Proposal

Due to recent changes to the project, when a matching stub mapping cannot be found for a request, the algorithm for calculating the nearest miss currently contains some unexpected biases in certain scenarios. Namely, when a custom matcher is attached to a stub mapping.

Due to a stub mapping's custom matcher's result only being aggregated with the standard matchers' result when the standard matchers return an exact match, there is a good chance that stub mappings that do not match a request's method, url etc. will actually be calculated as a closer match than a stub mapping that does match the request's standard attributes exactly, but whose custom matcher does not match.

As an example, consider the following. Stub mapping "A" expects a request akin to GET /things, with a custom matcher that (for illustrative purposes) returns a no-match result. Stub mapping "B" expects a request akin to GET /stuff, with the same custom matcher as "A". If a GET /things request is received, no matches will be found due to the custom matcher returning a no-match result (as expected). This will cause the nearest stub mapping to be calculated. Mapping "A"'s standard matchers will return an exact match (distance 0.0), so this result will aggregated with its custom matcher result, which will be a no-match (distance 1.0). Currently, this aggregate result will have a distance of 0.5, since equal weight is given to both the standard matcher and custom matcher results. Mapping "B"'s standard matchers will likely return a match result with a distance somewhere close to 0.0 but not exactly, and since this is not an exact match, this result will not be aggregated with "B"'s custom matcher's result. Thus, the mapping "B"'s distance from the request will be closer than mapping "A"'s.

I believe the simplest solution to this issue is to give a stub mapping's standard matcher results a much higher aggregate weighting than the mapping's custom matcher result, thus ensuring that mappings with exact matches on their standard matchers will almost always be calculated with smaller distances than other mappings, regardless of their custom matchers.

A potentially more complex solution would be to filter stub mappings by priorities when calculating the nearest miss (e.g. not consider PUT mappings on a GET request when there are GET mappings available, then not considering mappings with different URLs if mappings with the same URL exist, etc.).

Reproduction steps

The following JUnit test demonstrates the example given in the proprosal. The diff line that's logged by WireMock (and returned in the response body of the request) shows that the GET /stuff stub mapping was calculated as a nearer miss to a GET /things request than the GET /things stub mapping.

import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import com.github.tomakehurst.wiremock.extension.Parameters;
import com.github.tomakehurst.wiremock.http.Request;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import com.github.tomakehurst.wiremock.matching.MatchResult;
import com.github.tomakehurst.wiremock.matching.RequestMatcherExtension;
import org.junit.Rule;
import org.junit.Test;

import java.io.IOException;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;

import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;

public class ReproduceTest {

    public static RequestMatcherExtension customMatcher = new RequestMatcherExtension() {
        @Override
        public String getName() {
            return "custom matcher";
        }

        @Override
        public MatchResult match(Request request, Parameters parameters) {
            return MatchResult.noMatch();
        }
    };

    @Rule
    public WireMockRule wiremock = new WireMockRule(new WireMockConfiguration().extensions(customMatcher));

    @Test
    public void reproduce() throws IOException, InterruptedException {
        wiremock.addStubMapping(wiremock.stubFor(get(urlEqualTo("/things")).andMatching(customMatcher.getName())));
        wiremock.addStubMapping(wiremock.stubFor(get(urlEqualTo("/stuff")).andMatching(customMatcher.getName())));
        HttpRequest request = HttpRequest.newBuilder(URI.create(wiremock.url("/things"))).GET().build();
        HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString());
    }
}

References

No response

RafeArnold avatar Apr 10 '24 14:04 RafeArnold