Feature/39 path matching bugfix

[mwadams]

This PR adds support for Server resolution at the Path and Root levels to address this issue, and updated the specs accordingly.

It now passes the whole URI from the Request down the stack to support this. Your YAML will not work if you do not set the servers correctly for your particular deployment.

Resolves #39

[idg10]

I have been unable to work out why UriTemplate has both CreateMatchingRegex and CreateMatchingRegex2. I have been able to find only two differences at https://github.com/tavis-software/Tavis.UriTemplates/blob/master/src/UriTemplates/UriTemplate.cs#L384-L445

1. A comment after the first statement in the method (absent in the 2 version)
2. Meaninless whilespace in the case "#": body (absent in the 2 version)

[idg10]

So...what is this? I think it's a lazily populated dictionary of regular expressions intended to match the url of a Server Object in an API spec. However, I think there may be a subtle problem here.

I presume the reason you're using RegEx here, and not just matching with string.StartsWith is that the server url property supports Server Variables, so it might look like https://{appname}.azurewebsites.net/{apiVersion}. The OpenAPI spec apparently makes a decision of convenience here: in order to support using variables in this way, the URL is, syntactically speaking, a URL template. (They don't explicitly say that in the url property docs, but they do mention it in the Server Object's variables docs.)

And so you've used Tavis.UriTemplates to build a RegEx that matches strings against URL templates. Sounds perfectly sensible. Except I don't think it'll do what you meant.

Take an even simpler example than the one I gave: https://{appname}.azurewebsites.net/. That will match literally any Azure web app. I don't think that's what you actually mean. And let me give an example of why.

In OpenApiDocumentProvider.feature you have this:

Scenario Outline: Match requests to operation path templates - failure

and you include this line in the examples:

| No matching path | https://duff.server.io/v2/pet/65 | GET |

I venture to suggest that if I were to add this similar line:

| No matching path | https://duff.swagger.io/v2/pet/65 | GET |

you would also expect the test to pass (i.e., it the code under test should fail in the way the test says it should fail) for exactly the same reason the previous example fails: the server name is wrong here.

And indeed it does fail. But only because your example OpenAPI spec doesn't hit the problem I'm describing. Now consider changing the OpenAPI spec by changing the first server's url thus:

servers:
  - url: 'https://{servername}.swagger.io/v2'
    variables:
      servername:
        default: petstore
        enum: [petstore, petstoredev, petstoretest]

(And this is exactly how this feature of OpenAPI is meant to be used as far as I can tell.) The presence of that enum line constrains this to be one of three possible values, meaning that there are exactly three possible roots for the https form here. That means that while https://petstore.swagger.io/v2/pet/65, https://petstoredev.swagger.io/v2/pet/65, and https://petstoretest.swagger.io/v2/pet/65 test are all valid, https://duff.swagger.io/v2/pet/65 definitely isn't.

However, with the YAML set up in that way, and with the extra test scenario example added as shown above, the test now fails, because the Regex you end up creating here is effectively matching https://*.swagger.io/v2.

That is the correct logic for cases where a URI Template is being used in the most usual way, i.e., to match anything that conforms to the template. But it's not what's required here.

It's not entirely clear to me what is really required here. I'm not sure whether the intention is that individual environments can say what value they want to plug in for each server variable, or whether it may actually be valid for a single environment to be able to specify multiple values for each variable, and for things to match any time the URL matches for any combination of the variables.

But what does seem clear is that the current behaviour is demonstrably wrong: it will accept any value in a template placeholder even when the relevant Open API spec clearly states that this is not allowed (as in my example above).

So my suggestion for now would be simply not to support variable-based server URLs, and to revisit this if we ever turn out to need it.

[mwadams]

Right - this is exactly what I didn't fully understand about the intended use of those variables.

[mwadams]

I think it would be better just to support matching the possible variable substitutions. I also note that I have missed the fact that there are Operation overrides as well as Path overrides for the servers and so I need to support both of those cases too.

[mwadams]

I agree with your suggestion to defer this to "a later date" - the later date being imminent on the type generation branch.

[idg10]

...so you've found the need to change to the 2 form so you must have spotted a substantial difference! But what?

[mwadams]

"2"