add docs and cleanup data code

- simplify ParseOptions
- add lots of docs to ParseOptions and NewRequest
- cleanup some data tests

No behavioral changes.

data.go

@@ -33,17 +33,18 @@ func (e URLError) Error() string {
 	return fmt.Sprintf("malformed URL %q: %s", e.URL, e.Message)
 }
 
-// Options specifies transformations that can be performed on a
+// Options specifies transformations to be performed on the requested image.
-// requested image.
 type Options struct {
-	Width  float64 // requested width, in pixels
+	// See ParseOptions for interpretation of Width and Height values
-	Height float64 // requested height, in pixels
+	Width  float64
+	Height float64
 
 	// If true, resize the image to fit in the specified dimensions.  Image
 	// will not be cropped, and aspect ratio will be maintained.
 	Fit bool
 
-	// Rotate image the specified degrees counter-clockwise.  Valid values are 90, 180, 270.
+	// Rotate image the specified degrees counter-clockwise.  Valid values
+	// are 90, 180, 270.
 	Rotate int
 
 	FlipVertical   bool
@@ -70,64 +71,109 @@ func (o Options) String() string {
 	return buf.String()
 }
 
+// ParseOptions parses str as a list of comma separated transformation options.
+// The following options can be specified in any order:
+//
+// Size and Cropping
+//
+// The size option takes the general form "{width}x{height}", where width and
+// height are numbers. Integer values greater than 1 are interpreted as exact
+// pixel values. Floats between 0 and 1 are interpreted as percentages of the
+// original image size. If either value is omitted or set to 0, it will be
+// automatically set to preserve the aspect ratio based on the other dimension.
+// If a single number is provided (with no "x" separator), it will be used for
+// both height and width.
+//
+// Depending on the size options specified, an image may be cropped to fit the
+// requested size. In all cases, the original aspect ratio of the image will be
+// preserved; imageproxy will never stretch the original image.
+//
+// When no explicit crop mode is specified, the following rules are followed:
+//
+// - If both width and height values are specified, the image will be scaled to
+// fill the space, cropping if necessary to fit the exact dimension.
+//
+// - If only one of the width or height values is specified, the image will be
+// resized to fit the specified dimension, scaling the other dimension as
+// needed to maintain the aspect ratio.
+//
+// If the "fit" option is specified together with a width and height value, the
+// image will be resized to fit within a containing box of the specified size.
+// As always, the original aspect ratio will be preserved. Specifying the "fit"
+// option with only one of either width or height does the same thing as if
+// "fit" had not been specified.
+//
+// Rotation and Flips
+//
+// The "r{degrees}" option will rotate the image the specified number of
+// degrees, counter-clockwise. Valid degrees values are 90, 180, and 270.
+//
+// The "fv" option will flip the image vertically. The "fh" option will flip
+// the image horizontally. Images are flipped after being rotated.
+//
+// Examples
+//
+// 	0x0       - no resizing
+// 	200x      - 200 pixels wide, proportional height
+// 	0.15x     - 15% original width, proportional height
+// 	x100      - 100 pixels tall, proportional width
+// 	100x150   - 100 by 150 pixels, cropping as needed
+// 	100       - 100 pixels square, cropping as needed
+// 	150,fit   - scale to fit 150 pixels square, no cropping
+// 	100,r90   - 100 pixels square, rotated 90 degrees
+// 	100,fv,fh - 100 pixels square, flipped horizontal and vertical
 func ParseOptions(str string) Options {
-	o := Options{}
+	options := Options{}
 
-	parts := strings.Split(str, ",")
+	for _, opt := range strings.Split(str, ",") {
-	for _, part := range parts {
+		switch {
-		if part == "fit" {
+		case opt == "fit":
-			o.Fit = true
+			options.Fit = true
-			continue
+		case opt == "fv":
-		}
+			options.FlipVertical = true
-		if part == "fv" {
+		case opt == "fh":
-			o.FlipVertical = true
+			options.FlipHorizontal = true
-			continue
+		case len(opt) > 2 && opt[:1] == "r":
-		}
+			options.Rotate, _ = strconv.Atoi(opt[1:])
-		if part == "fh" {
+		case strings.ContainsRune(opt, 'x'):
-			o.FlipHorizontal = true
+			size := strings.SplitN(opt, "x", 2)
-			continue
+			if w := size[0]; w != "" {
-		}
+				options.Width, _ = strconv.ParseFloat(w, 64)
-
-		if len(part) > 2 && part[:1] == "r" {
-			o.Rotate, _ = strconv.Atoi(part[1:])
-			continue
-		}
-
-		if strings.ContainsRune(part, 'x') {
-			var h, w string
-			size := strings.SplitN(part, "x", 2)
-			w = size[0]
-			if len(size) > 1 {
-				h = size[1]
-			} else {
-				h = w
 			}
-
+			if h := size[1]; h != "" {
-			if w != "" {
+				options.Height, _ = strconv.ParseFloat(h, 64)
-				o.Width, _ = strconv.ParseFloat(w, 64)
 			}
-			if h != "" {
+		default:
-				o.Height, _ = strconv.ParseFloat(h, 64)
+			if size, err := strconv.ParseFloat(opt, 64); err == nil {
+				options.Width = size
+				options.Height = size
 			}
-			continue
-		}
-
-		if size, err := strconv.ParseFloat(part, 64); err == nil {
-			o.Width = size
-			o.Height = size
-			continue
 		}
 	}
 
-	return o
+	return options
 }
 
+// Request is an imageproxy request which includes a remote URL of an image to
+// proxy, and an optional set of transformations to perform.
 type Request struct {
 	URL     *url.URL // URL of the image to proxy
 	Options Options  // Image transformation to perform
 }
 
-// NewRequest parses an http.Request into an image request.
+// NewRequest parses an http.Request into an imageproxy Request.  Options and
+// the remote image URL are specified in the request path, formatted as:
+// /{options}/{remote_url}.  Options may be omitted, so a request path may
+// simply contian /{remote_url}.  The remote URL must be an absolute "http" or
+// "https" URL, should not be URL encoded, and may contain a query string.
+//
+// Assuming an imageproxy server running on localhost, the following are all
+// valid imageproxy requests:
+//
+// 	http://localhost/100x200/http://example.com/image.jpg
+// 	http://localhost/100x200,r90/http://example.com/image.jpg?foo=bar
+// 	http://localhost//http://example.com/image.jpg
+// 	http://localhost/http://example.com/image.jpg
 func NewRequest(r *http.Request) (*Request, error) {
 	var err error
 	req := new(Request)
@@ -135,7 +181,7 @@ func NewRequest(r *http.Request) (*Request, error) {
 	path := r.URL.Path[1:] // strip leading slash
 	req.URL, err = url.Parse(path)
 	if err != nil || !req.URL.IsAbs() {
-		// first segment is likely options
+		// first segment should be options
 		parts := strings.SplitN(path, "/", 2)
 		if len(parts) != 2 {
 			return nil, URLError{"too few path segments", r.URL}

data_test.go

@@ -55,7 +55,10 @@ func TestParseOptions(t *testing.T) {
 		{"1x", Options{Width: 1}},
 		{"x1", Options{Height: 1}},
 		{"1x2", Options{Width: 1, Height: 2}},
+		{"-1x-2", Options{Width: -1, Height: -2}},
 		{"0.1x0.2", Options{Width: 0.1, Height: 0.2}},
+		{"1", Options{Width: 1, Height: 1}},
+		{"0.1", Options{Width: 0.1, Height: 0.1}},
 
 		// additional flags
 		{"fit", Options{Fit: true}},
@@ -73,6 +76,7 @@ func TestParseOptions(t *testing.T) {
 		// mix of valid and invalid flags
 		{"FOO,1,BAR,r90,BAZ", Options{Width: 1, Height: 1, Rotate: 90}},
 
+		// all flags, in different orders
 		{"1x2,fit,r90,fv,fh", Options{1, 2, true, 90, true, true}},
 		{"r90,fh,1x2,fv,fit", Options{1, 2, true, 90, true, true}},
 	}
@@ -90,24 +94,16 @@ func TestParseOptions(t *testing.T) {
 // the various Options that can be specified; see TestParseOptions for that.
 func TestNewRequest(t *testing.T) {
 	tests := []struct {
-		URL         string
+		URL         string  // input URL to parse as an imageproxy request
-		RemoteURL   string
+		RemoteURL   string  // expected URL of remote image parsed from input
-		Options     Options
+		Options     Options // expected options parsed from input
-		ExpectError bool
+		ExpectError bool    // whether an error is expected from NewRequest
 	}{
 		// invalid URLs
-		{
+		{"http://localhost/", "", emptyOptions, true},
-			"http://localhost/", "", emptyOptions, true,
+		{"http://localhost/1/", "", emptyOptions, true},
-		},
+		{"http://localhost//example.com/foo", "", emptyOptions, true},
-		{
+		{"http://localhost//ftp://example.com/foo", "", emptyOptions, true},
-			"http://localhost/1/", "", emptyOptions, true,
-		},
-		{
-			"http://localhost//example.com/foo", "", emptyOptions, true,
-		},
-		{
-			"http://localhost//ftp://example.com/foo", "", emptyOptions, true,
-		},
 
 		// invalid options.  These won't return errors, but will not fully parse the options
 		{