Changed path_item_finder to detect more complex path template patterns

[makdad]

No description provided.

[ota42y]

(from interagent/committee#261)

Hi!
I think there is no problem with the method interface, style, etc... 👍

But there is many bugs
And I think we need to make a big change to fix this problem...
(It seems like a more difficult problem than we expected)

f = OpenAPIParser::PathItemFinder.new([])
f.parse_path_parameters('{p1}.json', 'foo-.jso')
# Should be return nil but {"p1"=>"foo-"}

f.parse_path_parameters('{p1}schedule', 'adminsschedule')
# Should be return 'admins' but {"p1"=>"admin"}

f.parse_path_parameters('{p1}schedule', 'usersschedule')
# Should be return 'usres' but {"p1"=>"u"}

f.parse_path_parameters('{p1}-{p2}.json', 'foo.json')
# Should be return nil but got errors

[makdad]

@ota42y thanks a lot, I have added a commit with failing specs to cover your feedback. Will work on the implementation for now. I'll close this PR until we have it covered.

[ota42y]

I think we can convert path parameter to regexp 🤔
( {p1}-{p2}.json -> (.+)-(.+)\.json )

[makdad]

@ota42y your hunch was correct, this code is so much simpler by relying on the underlying Ruby regex engine. Thank you for the idea! I'm ready for review again; if you think this is reasonable, I can open a PR to the main repository.

[makdad]

@ota42y The only thing I am not certain about is if we need to Regex.escape the non-parameter portions of the path. I did some various tests via unit testing, but it all seemed to work as-expected. I think the more "invalid" combinations that might generate the situation would not be valid URLs, so it is probably OK?

[ota42y]

I think there is very good code.

But your concerns need to be checked.
Please check these test case.

• {id}.json and 123-json
  • dose not match.
• {id>}.json and 1.json
  • Parameter name is "id>" and that value is 1
  • The OpenAPI dosen't define valid character so I think any characters without {} is allowed
    • ( Please tell me if I misread.. I'm not good at English)
  • https://github.com/OAI/OpenAPI-Specification/blob/3.0.3/versions/3.0.3.md#path-templating

[ota42y]

👍 👍 👍

[makdad]

@ota42y Thanks for pointing this out; I've updated the code to escape the non-parameter components. Also, don't worry about your English - it's great! In the worst case, we can also speak Japanese 👍

[ota42y]

Sorry for my late reply, I think this PR is perfect !!! 🎉 🎉 🎉
Thank you for your kind words 👍