Skip to content
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

Enhance handler customization. #50

Closed
wants to merge 1 commit into from

Conversation

ldez
Copy link

@ldez ldez commented Jul 12, 2017

I would propose to you this PR fixing #43, #40, #34:

This changes:

  • don't expose previous unexported methods or struct. (like acceptsGzip or others)
  • don't break current behaviors.
example
package gziphandler

import (
    "bufio"
    "log"
    "net"
    "net/http"
)

type GzipResponseWriterLogger struct {
    GzipResponseWriter
}

func (g *GzipResponseWriterLogger) Write(b []byte) (int, error) {
    log.Printf("Write: %v\n", b)
    log.Printf("ResponseWriter Header: %v\n", g.ResponseWriter.Header())
    return g.GzipResponseWriter.Write(b)
}

func (g *GzipResponseWriterLogger) WriteHeader(code int) {
    log.Printf("WriteHeader: %v\n", code)
    g.GzipResponseWriter.WriteHeader(code)
}

func (g *GzipResponseWriterLogger) Hijack() (net.Conn, *bufio.ReadWriter, error) {
    log.Println("Hijack")
    return g.GzipResponseWriter.Hijack()
}

func (g *GzipResponseWriterLogger) Close() error {
    log.Println("Close")
    return g.GzipResponseWriter.Close()
}

func (g *GzipResponseWriterLogger) Header() http.Header {
    log.Println("Header")
    return g.GzipResponseWriter.Header()
}

func GzipHandlerLogger(h http.Handler) http.Handler {
    wrapper, _ := NewHandler(&GzipResponseWriterLogger{}, CompressionLevel(2), MinSize(1024))
    return wrapper(h)
}

Fixes #43, #40, #34

- functional options
- GzipWriter interface
@jprobinson
Copy link
Contributor

Hi there, @ldez! Sorry for the delay, I was at GopherCon last week.

While we're interested in using functional options for this package, I'm not quite sure the addition of this interface is really needed. Do you think you could implement this without it?

@jprobinson jprobinson closed this Jul 17, 2017
@jprobinson jprobinson reopened this Jul 17, 2017
@jprobinson
Copy link
Contributor

Sorry, meant to just comment, not close!

@ldez
Copy link
Author

ldez commented Jul 17, 2017

The interface GzipWriter (the name is not the best but GzipResponseWriter is already used) allow to add some custom behaviors without needing to add them to this middleware.

I'm not really happy what I did with this interface (unexported methods) but I don't see a cleaner way to do that, if you have an idea I can implement.

But if you think it's not a interesting idea, I can drop this part.

WDYT?

@ldez
Copy link
Author

ldez commented Aug 11, 2017

With the merge of #51, functional options have been added.
And without any response I suppose you don't want my PR.

@ldez ldez closed this Aug 11, 2017
@ldez ldez deleted the feature/enhance-customization branch August 11, 2017 22:42
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.

Use functional options
2 participants