Improve api path matching

[pipermerriam]

The current api path matching leaves room for improvement in the case where multiple matches are found for a target path. This happens in pretty simple situations like nested resources.

• /get/{id} => /get/(?P<id>.+)
• /get/{id}/nested/{other_id} => /get/(?P<id>.+)/nested/(?P<other_id>.+)

Both of these path regexes will match /get/1234/nested/5678.

The current logic looks at the match with the most matched groups and assumes it is the correct path. I think this is a bit naive and may have false positives, or potentially pick the wrong path in specialized use cases.

I don't know that the correct approach is, but I think a start would be to use other factors in the definition of the parameter to generate stricter regexes. Fox example, path parameters that are of integer type could generate /get/(?P<id>\d+) so that it will only match paths that have numeric values in the location of id.

Another thing to take into account, is that adding this kind matching based on type will turn what should be a type error in the actual extracted parameter value into an error matching the path. I don't know which is the correct error to raise. Maybe there is a way to do both?

[pipermerriam]

@nicolastrres if you have any brilliant idea in this area, I'm all ears.

[alvarocavalcanti]

Just to let you know that I'm currently facing this problem. We used to have this path only:

/v1/accounts/{account_id}:

But now I added this other one:

/v1/accounts/{account_id}/subscriptions:

If I place the latter after the former, I get a wrong match, stating that 1/subscriptions is a string instead of an integer. But if I switch them, placing the latter before the former, I get this error:

flex.exceptions.ValidationError: 'request':
    - 'path':
        - 'tuple index out of range'

Oh, and btw, I work in the same team as @nicolastrres :)

[pipermerriam]

Out of town on vacation for two more days. I'll take a look into this when I get back. Feel free to start up a pull request, even if it only creates a test that shows this failure.

[pipermerriam]

3.4.1 is up on pypi https://pypi.python.org/pypi/flex/3.4.1 with a fix for @alvarocavalcanti 's issue.

This issue however is not resolved, as the regexes are still too greedy.

[alvarocavalcanti]

:(

Em sex, 27 de mar de 2015 às 11:35, Piper Merriam [email protected]
escreveu:

Reopened #58 #58.

—
Reply to this email directly or view it on GitHub
#58 (comment).

[pipermerriam]

Ah, I see I may have been mis-understood. @alvarocavalcanti you're issue is fixed.

I was referencing that the overall issue detailed in this issue about making the regex's more specific is not implemented, so I expect there still may be other bugs found related to this.

[alvarocavalcanti]

Awesome! Thanks.

Em sex, 27 de mar de 2015 11:50, Piper Merriam [email protected]
escreveu:

Ah, I see I may have been mis-understood. @alvarocavalcanti
https://github.com/alvarocavalcanti you're issue is fixed.

I was referencing that the overall issue detailed in this issue about
making the regex's more specific is not implemented, so I expect there
still may be other bugs found related to this.

—
Reply to this email directly or view it on GitHub
#58 (comment).

[kelvan]

just stumbled over this after debugging an error while extending an api,
I found several problems/restrictions

the first is that flex does not recognize the type number of a parameter, e.g.:

posNr:
    name: posNr
    in: path
    description: Position number of entry
    required: true
    type: number

therefore /list/{listId}/{posNr} also matches /list/{listId}/share

the second is the way it selects the "better" match if there are multiple, it just picks the longer path, leading to the strange behaviour that e.g. /list/{listId}/activate chooses the correct match /list/{listId}/share does not.
Another thing is that my request is POST where /list/{listId}/{posNr} allows only patch and delete.

would it be a solution to just select the first match in the spec?

I didn't find a workaround, besides disabling the check for the new paths