-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds IgnoreBase parameter to static middleware #1701
Conversation
Adds IgnoreBase parameter to static middleware to support the use case of nested route groups
middleware/static_test.go
Outdated
e.ServeHTTP(rec, req) | ||
|
||
assert.Equal(http.StatusOK, rec.Code) | ||
assert.Contains(rec.Body.String(), "..\\_fixture\\_fixture") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be ../_fixture/_fixture
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I was doing it on a windows machine, and was trying to write tests quickly so I made a rookie mistake, I'll add system path separators instead.
Codecov Report
@@ Coverage Diff @@
## master #1701 +/- ##
==========================================
+ Coverage 84.88% 84.93% +0.04%
==========================================
Files 29 29
Lines 1945 1951 +6
==========================================
+ Hits 1651 1657 +6
Misses 187 187
Partials 107 107
Continue to review full report at Codecov.
|
I am now thinking if maybe this behavior (of trimming duplicate path) should be default, as it seems like a bug that the path is duplicated. |
@lnenad it would have been great as a default in my opinion but that would be a breaking change. it's best left optional. |
@iambenkay Cool, any idea when is this going to be merged, I need this functionality and I don't like using forks if I don't have to :)? |
Looks good and is a common pitfall for some users. Thanks @lnenad . |
Yes, I would be glad to. Thanks |
Adds IgnoreBase parameter to static middleware to support the use case of nested route groups.
I've run into a use case of having a route group that is used with static middleware, but when you access the group's base url the filesystem path is doubled. For example
When an incoming request comes for
/somepath
the actual filesystem request goes tofilesystempath/somepath
instead of onlyfilesystempath
. This is what is being solved with this PR.It includes tests for both branches, with bool value set to true, or default -> false.