From 4a5f24e4ec1d6b5f648cdc360485ff3e6fbd84f7 Mon Sep 17 00:00:00 2001 From: Will Norris Date: Wed, 19 Nov 2014 21:59:52 -0800 Subject: [PATCH] there's no need for Options to be a pointer --- data.go | 8 ++--- data_test.go | 82 +++++++++++++++++++++++++++------------------------- proxy.go | 3 +- transform.go | 5 ++-- 4 files changed, 49 insertions(+), 49 deletions(-) diff --git a/data.go b/data.go index 100a773..e2e1353 100644 --- a/data.go +++ b/data.go @@ -51,7 +51,7 @@ type Options struct { FlipHorizontal bool } -var emptyOptions = new(Options) +var emptyOptions = Options{} func (o Options) String() string { buf := new(bytes.Buffer) @@ -71,8 +71,8 @@ func (o Options) String() string { return buf.String() } -func ParseOptions(str string) *Options { - o := new(Options) +func ParseOptions(str string) Options { + o := Options{} parts := strings.Split(str, ",") for _, part := range parts { @@ -125,7 +125,7 @@ func ParseOptions(str string) *Options { type Request struct { URL *url.URL // URL of the image to proxy - Options *Options // Image transformation to perform + Options Options // Image transformation to perform } // NewRequest parses an http.Request into an image request. diff --git a/data_test.go b/data_test.go index 0e29f18..cfe4892 100644 --- a/data_test.go +++ b/data_test.go @@ -16,18 +16,20 @@ package imageproxy import ( "net/http" - "reflect" "testing" ) func TestOptions_String(t *testing.T) { tests := []struct { - Options *Options + Options Options String string }{ - {emptyOptions, "0x0"}, { - &Options{1, 2, true, 90, true, true}, + emptyOptions, + "0x0", + }, + { + Options{1, 2, true, 90, true, true}, "1x2,fit,r90,fv,fh", }, } @@ -42,7 +44,7 @@ func TestOptions_String(t *testing.T) { func TestParseOptions(t *testing.T) { tests := []struct { Input string - Options *Options + Options Options }{ {"", emptyOptions}, {"x", emptyOptions}, @@ -50,34 +52,34 @@ func TestParseOptions(t *testing.T) { {",,,,", emptyOptions}, // size variations - {"1x", &Options{Width: 1}}, - {"x1", &Options{Height: 1}}, - {"1x2", &Options{Width: 1, Height: 2}}, - {"0.1x0.2", &Options{Width: 0.1, Height: 0.2}}, + {"1x", Options{Width: 1}}, + {"x1", Options{Height: 1}}, + {"1x2", Options{Width: 1, Height: 2}}, + {"0.1x0.2", Options{Width: 0.1, Height: 0.2}}, // additional flags - {"fit", &Options{Fit: true}}, - {"r90", &Options{Rotate: 90}}, - {"fv", &Options{FlipVertical: true}}, - {"fh", &Options{FlipHorizontal: true}}, + {"fit", Options{Fit: true}}, + {"r90", Options{Rotate: 90}}, + {"fv", Options{FlipVertical: true}}, + {"fh", Options{FlipHorizontal: true}}, // duplicate flags (last one wins) - {"1x2,3x4", &Options{Width: 3, Height: 4}}, - {"1x2,3", &Options{Width: 3, Height: 3}}, - {"1x2,0x3", &Options{Width: 0, Height: 3}}, - {"1x,x2", &Options{Width: 1, Height: 2}}, - {"r90,r270", &Options{Rotate: 270}}, + {"1x2,3x4", Options{Width: 3, Height: 4}}, + {"1x2,3", Options{Width: 3, Height: 3}}, + {"1x2,0x3", Options{Width: 0, Height: 3}}, + {"1x,x2", Options{Width: 1, Height: 2}}, + {"r90,r270", Options{Rotate: 270}}, // mix of valid and invalid flags - {"FOO,1,BAR,r90,BAZ", &Options{Width: 1, Height: 1, Rotate: 90}}, + {"FOO,1,BAR,r90,BAZ", Options{Width: 1, Height: 1, Rotate: 90}}, - {"1x2,fit,r90,fv,fh", &Options{1, 2, true, 90, true, true}}, - {"r90,fh,1x2,fv,fit", &Options{1, 2, true, 90, true, true}}, + {"1x2,fit,r90,fv,fh", Options{1, 2, true, 90, true, true}}, + {"r90,fh,1x2,fv,fit", Options{1, 2, true, 90, true, true}}, } - for i, tt := range tests { - if got, want := ParseOptions(tt.Input), tt.Options; !reflect.DeepEqual(got, want) { - t.Errorf("%d. ParseOptions returned %#v, want %#v", i, got, want) + for _, tt := range tests { + if got, want := ParseOptions(tt.Input), tt.Options; got != want { + t.Errorf("ParseOptions(%q) returned %#v, want %#v", tt.Input, got, want) } } } @@ -90,21 +92,21 @@ func TestNewRequest(t *testing.T) { tests := []struct { URL string RemoteURL string - Options *Options + Options Options ExpectError bool }{ // invalid URLs { - "http://localhost/", "", nil, true, + "http://localhost/", "", emptyOptions, true, }, { - "http://localhost/1/", "", nil, true, + "http://localhost/1/", "", emptyOptions, true, }, { - "http://localhost//example.com/foo", "", nil, true, + "http://localhost//example.com/foo", "", emptyOptions, true, }, { - "http://localhost//ftp://example.com/foo", "", nil, true, + "http://localhost//ftp://example.com/foo", "", emptyOptions, true, }, // invalid options. These won't return errors, but will not fully parse the options @@ -114,13 +116,13 @@ func TestNewRequest(t *testing.T) { }, { "http://localhost/1xs/http://example.com/", - "http://example.com/", &Options{Width: 1}, false, + "http://example.com/", Options{Width: 1}, false, }, // valid URLs { "http://localhost/http://example.com/foo", - "http://example.com/foo", nil, false, + "http://example.com/foo", emptyOptions, false, }, { "http://localhost//http://example.com/foo", @@ -132,7 +134,7 @@ func TestNewRequest(t *testing.T) { }, { "http://localhost/1x2/http://example.com/foo", - "http://example.com/foo", &Options{Width: 1, Height: 2}, false, + "http://example.com/foo", Options{Width: 1, Height: 2}, false, }, { "http://localhost//http://example.com/foo?bar", @@ -140,29 +142,29 @@ func TestNewRequest(t *testing.T) { }, } - for i, tt := range tests { + for _, tt := range tests { req, err := http.NewRequest("GET", tt.URL, nil) if err != nil { - t.Errorf("%d. Error parsing request: %v", i, err) + t.Errorf("http.NewRequest(%q) returned error: %v", tt.URL, err) continue } r, err := NewRequest(req) if tt.ExpectError { if err == nil { - t.Errorf("%d. Expected parsing error", i) + t.Errorf("NewRequest(%v) did not return expected error", req) } continue } else if err != nil { - t.Errorf("%d. Error parsing request: %v", i, err) + t.Errorf("NewRequest(%v) return unexpected error: %v", req, err) continue } - if got := r.URL.String(); tt.RemoteURL != got { - t.Errorf("%d. Request URL = %v, want %v", i, got, tt.RemoteURL) + if got, want := r.URL.String(), tt.RemoteURL; got != want { + t.Errorf("NewRequest(%q) request URL = %v, want %v", tt.URL, got, want) } - if !reflect.DeepEqual(tt.Options, r.Options) { - t.Errorf("%d. Request Options = %v, want %v", i, r.Options, tt.Options) + if got, want := r.Options, tt.Options; got != want { + t.Errorf("NewRequest(%q) request options = %v, want %v", tt.URL, got, want) } } } diff --git a/proxy.go b/proxy.go index f29eb69..6430471 100644 --- a/proxy.go +++ b/proxy.go @@ -23,7 +23,6 @@ import ( "io/ioutil" "net/http" "net/url" - "reflect" "strings" "time" @@ -79,7 +78,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } u := req.URL.String() - if req.Options != nil && !reflect.DeepEqual(req.Options, emptyOptions) { + if req.Options != emptyOptions { u += "#" + req.Options.String() } resp, err := p.Client.Get(u) diff --git a/transform.go b/transform.go index 666c1b4..f0e0399 100644 --- a/transform.go +++ b/transform.go @@ -20,7 +20,6 @@ import ( "image/gif" "image/jpeg" "image/png" - "reflect" "github.com/disintegration/imaging" ) @@ -29,8 +28,8 @@ import ( const jpegQuality = 95 // Transform the provided image. -func Transform(img []byte, opt *Options) ([]byte, error) { - if opt == nil || reflect.DeepEqual(opt, emptyOptions) { +func Transform(img []byte, opt Options) ([]byte, error) { + if opt == emptyOptions { // bail if no transformation was requested return img, nil }