fix: implement findRelativeRessourcePath function to fix PatchResourc… #714
Conversation
froz42
force-pushed
the
main
branch
2 times, most recently
from
cecce60 to
899633b
Compare
| func findRelativeRessourcePath(requestPath string, putPath string) string { | ||
| putPathParts := strings.Split(putPath, "/") | ||
| // if the path is not deep enough, we return the original path | ||
| if len(putPathParts) < 2 { |
There was a problem hiding this comment.
What determines this magic number here? Why 2? Are there any cases where this could fail?
There was a problem hiding this comment.
The number 2 is here to handle this case:
if putPath is /{id} -> len(putPathParts == 1 that mean that we don't have a ressource name to find,
A case that could be not covered by this could be:
GET {prefix}/singleRessource
PUT {prefix}/singleRessource
it also protect the table access wantedPrefix := putPathParts[1]
There was a problem hiding this comment.
In the case that is not supported we just fall back to the default behavior
…e when prefix That fix a bug that when the path had prefix like "/api" a 404 error occured
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #714 +/- ##
==========================================
- Coverage 92.79% 92.77% -0.03%
==========================================
Files 22 22
Lines 5027 5051 +24
==========================================
+ Hits 4665 4686 +21
- Misses 312 314 +2
- Partials 50 51 +1 ☔ View full report in Codecov by Sentry. |
This merge request fix issue #713 "Autopatch not working when a prefix is present in the path"
Technical details
I implemented a new function
findRelativeRessourcePathin theautopatchpackage that removes the prefix from the path before searching for the resource in the router by comparing thePUTpath and request path.It will strip the request path until the prefix of the
PUTpath prefix is found.It think a better solution could be found if with a proper rework.
I wrote unit tests for the new function.
Fixes #713.