hivesolutions / appier

Joyful Python Web App development
http://appier.hive.pt
Apache License 2.0
127 stars 22 forks source link

Removed dot from route replace regex #45

Closed BeeMargarida closed 3 years ago

BeeMargarida commented 3 years ago
- -
Issue There was a problem in the route parsing and regex replacement. For example, with the following route endpoints /api/builds/<str:name>/fonts/<str:font>.<str:format> the font and format are wrongly extracted, where the parameter SRGunmetal-Regular.fnt results in font: SRGunmetal-Regular.f and format: t.
Dependencies --
Decisions Removed the . from the REPLACE_REGEX. All other routes with dots are working as supposed and the route mentiond above as well.
Animated GIF --
BeeMargarida commented 3 years ago

@joamag @gcandal Problem found while doing the endpoint in builds

gcandal commented 3 years ago

Why wasn't this a problem for the existing endpoints?

image

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 63.865% when pulling 77d042e79cc0513b1a9d77abde0f51333efc3610 on BeeMargarida:ms/fix-route-regex into ad27856692d6d1938136116ff50cae5eba964e97 on hivesolutions:master.

BeeMargarida commented 3 years ago

Why wasn't this a problem for the existing endpoints?

image

The logo. is not replaced by the REPLACE_REGEX substitution because it does employ the format, so it will be the same.

The expression for the logo: ^/api/builds/(?P<name>[\@\+\:\.\s\w-]+)/logo.(?P<format>[\@\+\:\.\s\w-]+)$ The expression for the the example above before the fix: ^/api/builds/(?P<name>[\@\+\:\.\s\w-]+)/fonts/(?P<font>[\@\+\:\.\s\w-]+).(?P<format>[\@\+\:\.\s\w-]+)$

joamag commented 3 years ago

I can't approve this PR as this would create a huge level of regressions in other projects, we'll need to go with <regex:> instead for this specific situation so something like /api/builds/<str:name>/fonts/<regex:font:[a-zA-Z0-9]>.<str:format>

joamag commented 3 years ago

@BeeMargarida @gcandal Sorry wrong syntax it should be something like /api/builds/<str:name>/fonts/<regex('[a-zA-Z0-9]+'):font>.<str:format>