there's no need for Options to be a pointer

This commit is contained in:
Will Norris 2014-11-19 21:59:52 -08:00
parent c5b279b947
commit 4a5f24e4ec
4 changed files with 49 additions and 49 deletions

View file

@ -51,7 +51,7 @@ type Options struct {
FlipHorizontal bool FlipHorizontal bool
} }
var emptyOptions = new(Options) var emptyOptions = Options{}
func (o Options) String() string { func (o Options) String() string {
buf := new(bytes.Buffer) buf := new(bytes.Buffer)
@ -71,8 +71,8 @@ func (o Options) String() string {
return buf.String() return buf.String()
} }
func ParseOptions(str string) *Options { func ParseOptions(str string) Options {
o := new(Options) o := Options{}
parts := strings.Split(str, ",") parts := strings.Split(str, ",")
for _, part := range parts { for _, part := range parts {
@ -125,7 +125,7 @@ func ParseOptions(str string) *Options {
type Request struct { type Request struct {
URL *url.URL // URL of the image to proxy 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. // NewRequest parses an http.Request into an image request.

View file

@ -16,18 +16,20 @@ package imageproxy
import ( import (
"net/http" "net/http"
"reflect"
"testing" "testing"
) )
func TestOptions_String(t *testing.T) { func TestOptions_String(t *testing.T) {
tests := []struct { tests := []struct {
Options *Options Options Options
String string 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", "1x2,fit,r90,fv,fh",
}, },
} }
@ -42,7 +44,7 @@ func TestOptions_String(t *testing.T) {
func TestParseOptions(t *testing.T) { func TestParseOptions(t *testing.T) {
tests := []struct { tests := []struct {
Input string Input string
Options *Options Options Options
}{ }{
{"", emptyOptions}, {"", emptyOptions},
{"x", emptyOptions}, {"x", emptyOptions},
@ -50,34 +52,34 @@ func TestParseOptions(t *testing.T) {
{",,,,", emptyOptions}, {",,,,", emptyOptions},
// size variations // size variations
{"1x", &Options{Width: 1}}, {"1x", Options{Width: 1}},
{"x1", &Options{Height: 1}}, {"x1", Options{Height: 1}},
{"1x2", &Options{Width: 1, Height: 2}}, {"1x2", Options{Width: 1, Height: 2}},
{"0.1x0.2", &Options{Width: 0.1, Height: 0.2}}, {"0.1x0.2", Options{Width: 0.1, Height: 0.2}},
// additional flags // additional flags
{"fit", &Options{Fit: true}}, {"fit", Options{Fit: true}},
{"r90", &Options{Rotate: 90}}, {"r90", Options{Rotate: 90}},
{"fv", &Options{FlipVertical: true}}, {"fv", Options{FlipVertical: true}},
{"fh", &Options{FlipHorizontal: true}}, {"fh", Options{FlipHorizontal: true}},
// duplicate flags (last one wins) // duplicate flags (last one wins)
{"1x2,3x4", &Options{Width: 3, Height: 4}}, {"1x2,3x4", Options{Width: 3, Height: 4}},
{"1x2,3", &Options{Width: 3, Height: 3}}, {"1x2,3", Options{Width: 3, Height: 3}},
{"1x2,0x3", &Options{Width: 0, Height: 3}}, {"1x2,0x3", Options{Width: 0, Height: 3}},
{"1x,x2", &Options{Width: 1, Height: 2}}, {"1x,x2", Options{Width: 1, Height: 2}},
{"r90,r270", &Options{Rotate: 270}}, {"r90,r270", Options{Rotate: 270}},
// mix of valid and invalid flags // 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}}, {"1x2,fit,r90,fv,fh", Options{1, 2, true, 90, true, true}},
{"r90,fh,1x2,fv,fit", &Options{1, 2, true, 90, true, true}}, {"r90,fh,1x2,fv,fit", Options{1, 2, true, 90, true, true}},
} }
for i, tt := range tests { for _, tt := range tests {
if got, want := ParseOptions(tt.Input), tt.Options; !reflect.DeepEqual(got, want) { if got, want := ParseOptions(tt.Input), tt.Options; got != want {
t.Errorf("%d. ParseOptions returned %#v, want %#v", i, 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 { tests := []struct {
URL string URL string
RemoteURL string RemoteURL string
Options *Options Options Options
ExpectError bool ExpectError bool
}{ }{
// invalid URLs // 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 // 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://localhost/1xs/http://example.com/",
"http://example.com/", &Options{Width: 1}, false, "http://example.com/", Options{Width: 1}, false,
}, },
// valid URLs // valid URLs
{ {
"http://localhost/http://example.com/foo", "http://localhost/http://example.com/foo",
"http://example.com/foo", nil, false, "http://example.com/foo", emptyOptions, false,
}, },
{ {
"http://localhost//http://example.com/foo", "http://localhost//http://example.com/foo",
@ -132,7 +134,7 @@ func TestNewRequest(t *testing.T) {
}, },
{ {
"http://localhost/1x2/http://example.com/foo", "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", "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) req, err := http.NewRequest("GET", tt.URL, nil)
if err != 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 continue
} }
r, err := NewRequest(req) r, err := NewRequest(req)
if tt.ExpectError { if tt.ExpectError {
if err == nil { if err == nil {
t.Errorf("%d. Expected parsing error", i) t.Errorf("NewRequest(%v) did not return expected error", req)
} }
continue continue
} else if err != nil { } else if err != nil {
t.Errorf("%d. Error parsing request: %v", i, err) t.Errorf("NewRequest(%v) return unexpected error: %v", req, err)
continue continue
} }
if got := r.URL.String(); tt.RemoteURL != got { if got, want := r.URL.String(), tt.RemoteURL; got != want {
t.Errorf("%d. Request URL = %v, want %v", i, got, tt.RemoteURL) t.Errorf("NewRequest(%q) request URL = %v, want %v", tt.URL, got, want)
} }
if !reflect.DeepEqual(tt.Options, r.Options) { if got, want := r.Options, tt.Options; got != want {
t.Errorf("%d. Request Options = %v, want %v", i, r.Options, tt.Options) t.Errorf("NewRequest(%q) request options = %v, want %v", tt.URL, got, want)
} }
} }
} }

View file

@ -23,7 +23,6 @@ import (
"io/ioutil" "io/ioutil"
"net/http" "net/http"
"net/url" "net/url"
"reflect"
"strings" "strings"
"time" "time"
@ -79,7 +78,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
} }
u := req.URL.String() u := req.URL.String()
if req.Options != nil && !reflect.DeepEqual(req.Options, emptyOptions) { if req.Options != emptyOptions {
u += "#" + req.Options.String() u += "#" + req.Options.String()
} }
resp, err := p.Client.Get(u) resp, err := p.Client.Get(u)

View file

@ -20,7 +20,6 @@ import (
"image/gif" "image/gif"
"image/jpeg" "image/jpeg"
"image/png" "image/png"
"reflect"
"github.com/disintegration/imaging" "github.com/disintegration/imaging"
) )
@ -29,8 +28,8 @@ import (
const jpegQuality = 95 const jpegQuality = 95
// Transform the provided image. // Transform the provided image.
func Transform(img []byte, opt *Options) ([]byte, error) { func Transform(img []byte, opt Options) ([]byte, error) {
if opt == nil || reflect.DeepEqual(opt, emptyOptions) { if opt == emptyOptions {
// bail if no transformation was requested // bail if no transformation was requested
return img, nil return img, nil
} }