8000 feat : excluede paths by regex by standcats · Pull Request #36 · gin-contrib/gzip · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat : excluede paths by regex #36

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

Merged
merged 4 commits into from
Jun 9, 2020
Merged

Conversation

standcats
Copy link

Proposal
Allow regular expressions to exclude paths.
Gin can specify parameters in the URL.
In this case, the action name may be specified below the parameter.
The current exclusion doesn't allow you to specify a lower parameter, so I'd like to add it.
exp:

// In the case of export_zip, i don't want to do gzip.
r.Use(gzip.Gzip(gzip.DefaultCompression, gzip.WithExcludedPathsRegexs([]string{".*export_zip"})))
r.GET("/ping/:variable/export_zip",func(c *gin.Context){ })
r.GET("/ping/:variable/export_text",func(c *gin.Context){ })

handler.go Outdated
@@ -73,5 +73,5 @@ func (g *gzipHandler) shouldCompress(req *http.Request) bool {
return false
}

return !g.ExcludedPaths.Contains(req.URL.Path)
return !g.ExcludedPaths.Contains(req.URL.Path) && !g.ExcludedPathesRegexs.Contains(req.URL.Path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion use if statement

options.go Outdated
@@ -5,6 +5,8 @@ import (
"net/http"
"strings"

Copy link
Member
@thinkerou thinkerou Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the empty line and fmt it

@@ -36,6 +39,11 @@ func WithExcludedPaths(args []string) Option {
o.ExcludedPaths = NewExcludedPaths(args)
}
}
func WithExcludedPathsRegexs(args []string) Option {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add empty line.

}
return result
}
func (e ExcludedPathesRegexs) Contains(requestURI string) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add empty line

@standcats standcats requested a review from thinkerou June 4, 2020 08:57
handler.go Outdated
@@ -72,6 +72,11 @@ func (g *gzipHandler) shouldCompress(req *http.Request) bool {
if g.ExcludedExtensions.Contains(extension) {
return false
}

return !g.ExcludedPaths.Contains(req.URL.Path)
if !g.ExcludedPaths.Contains(req.URL.Path) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add an empty line between if block.

@standcats standcats requested a review from appleboy June 4, 2020 16:39
@appleboy
Copy link
Member
appleboy commented Jun 5, 2020

@standcats build fails?

@codecov-commenter
Copy link
codecov-commenter commented Jun 8, 2020

Codecov Report

Merging #36 into master will decrease coverage by 12.71%.
The diff coverage is 27.77%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #36       +/-   ##
===========================================
- Coverage   97.56%   84.84%   -12.72%     
===========================================
  Files           3        3               
  Lines          82       99       +17     
===========================================
+ Hits           80       84        +4     
- Misses          1       12       +11     
- Partials        1        3        +2     
Impacted Files Coverage Δ
options.go 76.59% <15.38%> (-23.41%) ⬇️
handler.go 90.69% <60.00%> (-4.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af5a1ba...44264cb. Read the comment docs.

@standcats
Copy link
Author

@appleboy
sorry.I fixed it.

Copy link
Member
@appleboy appleboy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@appleboy
Copy link
Member
appleboy commented Jun 8, 2020

Waiting for @thinkerou approval.

Copy link
Member
@thinkerou thinkerou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@thinkerou thinkerou merged commit 6ac096f into gin-contrib:master Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0