Validation Option to disable strict path parameter uniqueness validation

[dadgar]

Adds a validation option to disable path uniqueness validation behavior where path parameters are ignored. The existing behavior of the validator replaces path parameters with an X and then checks for uniqueness.

As an example, the two paths GET /v1/{book} and GET /v1/{shelve} where book has the pattern shelves/*/book/* and shelve has the pattern shelve/*, get both replaced to /v1/X and an error is returned reporting overlapping paths.

This does not allow the resource name pattern described by Google's AIPs. Further, I do not see it being described in the specification that the path parameters can not have slashes and thus the path can be unique based on the required path parameter.

I will add that I tested other Swagger validators / code generators and they were perfectly happy with paths that are only unique based on unique path parameters.

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (348543c) 90.60% compared to head (99afba0) 90.60%.

Files	Patch %	Lines
spec.go	66.66%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #157   +/-   ##
=======================================
  Coverage   90.60%   90.60%           
=======================================
  Files          22       22           
  Lines        2990     2991    +1     
=======================================
+ Hits         2709     2710    +1     
  Misses        227      227           
  Partials       54       54           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[casualjim]

Wouldn't it make more sense to just turn the error into a warning? And perhaps make this a configurable behavior via an env var or validator config.

Basically, this is a breaking change and while it might not work for your case, others might be depending on the behavior. This tool has been around for a long time

[dadgar]

@casualjim I would be willing to plump it through a config. So you would want the default behavior to be the historic behavior?

As I see it, the validation is enforcing something that is not in the spec and thus didn't warrant keeping the old behavior around; configurable or not.

[casualjim]

Yeah, it's just that we don't know who is depending on this behavior, and since it's been around for so long, this has become a feature for some and a bug for you. I understand where you're coming from, but the issue is not so critical that it warrants a breaking change. We can easily address both use cases with a simple guard.

[dadgar]

@casualjim Please take another look, added a validation option to switch the behavior and kept the existing default.

[dadgar]

@casualjim would appreciate a look at the updated PR when you have the chance. Thank you.

[dadgar]

Thank you!