natenho / Mockaco

🐵 HTTP mock server, useful to stub services and simulate dynamic API responses, leveraging ASP.NET Core features, built-in fake data generation and pure C# scripting
https://natenho.github.io/Mockaco/
Other
335 stars 39 forks source link

Routes are determined on first match, not the most complete match #144

Open lanegoolsby opened 3 weeks ago

lanegoolsby commented 3 weeks ago

Prerequisites

Description

If you have multiple mocks setup that are similar, but one is fully formed while the others have wildcards (i.e. {}'s), Mockaco returns the first match it finds, not the mock with the best match.

The order in which Mockaco searches appears to be file-name driven. Meaning it will match a.json before z.json, even if z.json has a fully formed route whereas a.json has wildcards.

For example:

a.json
route: /api/{foo}/{bar}
b.json
route: /api/path/{bar}
c.json
route: /api/{foo}/file
z.json
route: /api/path/file

When calling /api/path/file Mockaco currently matches a.json, b.json, or c.json first because their file names are first alphabetically even though z.json is a complete match.

Proposed solution

Mockaco should look at all defined route matches and choose the route with the most complete match, not the first match it finds.

I propose changing this logic to look something like this:

//snip
            Mock bestMatch = null;
            var bestScore = - 1;

            foreach (var mock in mockProvider.GetMocks())
            {
                if (await requestMatchers.AllAsync(e => e.IsMatch(httpContext.Request, mock)))
                {
                    var score = ScoreRouteTemplate(mock.Route);

                    if (score > bestScore)
                    {
                        bestMatch = mock;
                        bestScore = score;
                    }
                }
                else
                {
                    _logger.LogDebug("Incoming request didn't match {mock}", mock);
                }
            }

            if (bestMatch != null)
            {
                cache.Set($"{nameof(RequestMatchingMiddleware)} {httpContext.Request.Path.Value}",new
                {
                    Route = httpContext.Request.Path.Value,
                    Timestamp = $"{DateTime.Now:t}",
                    Headers = LoadHeaders(httpContext, options.Value.VerificationIgnoredHeaders),
                    Body = await httpContext.Request.ReadBodyStream()
                }, DateTime.Now.AddMinutes(options.Value.MatchedRoutesCacheDuration));

                _logger.LogInformation("Incoming request matched {mock}", bestMatch);

                await scriptContext.AttachRouteParameters(httpContext.Request, bestMatch);

                var template = await templateTransformer.TransformAndSetVariables(bestMatch.RawTemplate, scriptContext);

                mockacoContext.Mock = bestMatch;
                mockacoContext.TransformedTemplate = template;

                await _next(httpContext);

                return;
            }
//snip

    private int ScoreRouteTemplate(string routeTemplate)
    {
        var segments = routeTemplate.Split('/');
        int score = 0;

        foreach (var segment in segments)
        {
            if (segment.StartsWith("{") && segment.EndsWith("}"))
            {
                // This is a wildcard or route parameter, lower score
                score += 1;
            }
            else
            {
                // Explicit path segment, higher score
                score += 10;
            }
        }

        return score;
    }

Note that ScoreRouteTemplate doesn't really handle b.json and c.json in the problem description. It would need a little more work to rank the higher cardinality paths (i.e. /api/foo/{bar}) higher than lower cardinatity (i.e. /api/{foo}/bar). Before I spend more time on it I wanted to get feedback on the approach.

Alternatives

I've tried using file names in creative ways but its becoming difficult to manage.

Additional context

No response

lanegoolsby commented 3 weeks ago

@natenho any thoughts on this?

natenho commented 2 weeks ago

Hi @lanegoolsby, I understand the problem you're facing, but why not use conditions for matching instead of relying solely on the route?

You could have a single route for a, b, c, and z: /api/{foo}/{bar}, and test foo and bar as conditions for each of your cases.

The docs have examples here: https://natenho.github.io/Mockaco/docs/request-matching/

Please let me know if that works for you or give me more details. Dealiing with ambigous routes may be challeging. Your solution seems to be interesting but I'm not sure if it would be intuitive enough. Let's keep the discussion.

lanegoolsby commented 2 weeks ago

Thanks for the feedback @natenho.

For additional context, we're using Mockaco as part of our automated integration tests for a rather complicated application. We run three instances of Mockaco (among other services) mocking different APIs we use in a docker composition.

9 times out of 10 the wildcard routes we setup that produce random values work fine for us. This is ideal most of the time because of how this application works.

However, we want to setup specific test scenarios that mimic edge cases where the random values do not work for us. For example, a reproduction of a specific bug. In those cases we want to have fully defined routes for just that one scenario.

If we have two mock JSON files in the same folder, one with and one without wildcards, Mockaco will grab the first matching mock file. As I said in the OP, we've been managing this with creative file naming schemes so far, but its getting a bit ugly to manage.

natenho commented 2 weeks ago

@lanegoolsby, can you provide the mock files as exact or as close as possible to your usage?

We can work on providing alternative mock matching options as you suggest, but first I would like to try to help you finding a suitable solution with Mockaco as-is.

See this example. You can create as many copies as you want and script the specific conditions you need. Notice that both use the same route, but z.json is prioritized because of the conditions.

file z.json - Very specific scenario (prioritized by Mockaco)

{
  "request": {
    "method": "GET",
    "route": "/api/{foo}/{bar}"
    "condition": "<#= Request.Route["foo"] == "xxxxxxxxxx" && Request.Route["bar"] == "yyyyyyyyyy" #>"
  },
  "response": {
    "status": "OK",
    "body": {
      "foo": "<#= Request.Route["foo"] #>",
      "bar": "<#= Request.Route["bar"] #>",
     "msg": "THIS IS A VERY SPECIFIC ENDPOINT"
    }
  }
}

file a.json - General scenario

{
  "request": {
    "method": "GET",
    "route": "/api/{foo}/{bar}"
  },
  "response": {
    "status": "OK",
    "body": {
      "foo": "<#= Request.Route["foo"] #>",
      "bar": "<#= Request.Route["bar"] #>"
    }
  }
}

Calling the endpoint:

$ curl localhost:5000/api/xxxxxxxxxx/yyyyyyyyyy
{
  "foo": "xxxxxxxxxx",
  "bar": "yyyyyyyyyy",
  "msg": "THIS IS A VERY SPECIFIC ENDPOINT"
}

$ curl localhost:5000/api/any/other
{
  "foo": "any",
  "bar": "other"
}
lanegoolsby commented 2 weeks ago

Oh we're okay for now, this request will just make our lives easier in the long run.

I went ahead and submitted a PR for this. I was mostly done with the code anyways after writing up the OP, the only thing I had left was the unit test and validation.

lanegoolsby commented 2 weeks ago

Thanks for the suggestion!