Merge pull request #12 from adamreese/dm-client-refactor

ref(client): refactor url parsing

cmd/helm/list.go

@@ -22,7 +22,6 @@ func listCmd() cli.Command {
 }
 
 func list(host string) error {
-	client := dm.NewClient(host)
-	client.Protocol = "http"
+	client := dm.NewClient(host).SetDebug(true)
 	return client.ListDeployments()
 }

dm/client.go

@@ -6,8 +6,10 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/ghodss/yaml"
@@ -16,6 +18,9 @@ import (
 // The default HTTP timeout
 var DefaultHTTPTimeout = time.Second * 10
 
+// The default HTTP Protocol
+var DefaultHTTPProtocol = "http"
+
 // Client is a DM client.
 type Client struct {
 	// Timeout on HTTP connections.
@@ -26,29 +31,61 @@ type Client struct {
 	Protocol string
 	// Transport
 	Transport http.RoundTripper
+	// Debug enables http logging
+	Debug bool
+
+	// Base URL for remote service
+	baseURL *url.URL
 }
 
 // NewClient creates a new DM client. Host name is required.
 func NewClient(host string) *Client {
+	url, _ := DefaultServerURL(host)
+
 	return &Client{
 		HTTPTimeout: DefaultHTTPTimeout,
-		Protocol:    "https",
-		Host:        host,
-		Transport:   NewDebugTransport(nil),
+		baseURL:     url,
+		Transport:   http.DefaultTransport,
 	}
 }
 
+// SetDebug enables debug mode which logs http
+func (c *Client) SetDebug(enable bool) *Client {
+	c.Debug = enable
+	return c
+}
+
+// transport wraps client transport if debug is enabled
+func (c *Client) transport() http.RoundTripper {
+	if c.Debug {
+		return NewDebugTransport(c.Transport)
+	}
+	return c.Transport
+}
+
+// SetTransport sets a custom Transport. Defaults to http.DefaultTransport
+func (c *Client) SetTransport(tr http.RoundTripper) *Client {
+	c.Transport = tr
+	return c
+}
+
 // url constructs the URL.
-func (c *Client) url(path string) string {
-	// TODO: Switch to net.URL
-	return c.Protocol + "://" + c.Host + "/" + path
+func (c *Client) url(rawurl string) (string, error) {
+	u, err := url.Parse(rawurl)
+	if err != nil {
+		return "", err
+	}
+	return c.baseURL.ResolveReference(u).String(), nil
 }
 
 // CallService is a low-level function for making an API call.
 //
 // This calls the service and then unmarshals the returned data into dest.
 func (c *Client) CallService(path, method, action string, dest interface{}, reader io.ReadCloser) error {
-	u := c.url(path)
+	u, err := c.url(path)
+	if err != nil {
+		return err
+	}
 
 	resp, err := c.callHTTP(u, method, action, reader)
 	if err != nil {
@@ -76,7 +113,7 @@ func (c *Client) callHTTP(path, method, action string, reader io.ReadCloser) (st
 
 	client := http.Client{
 		Timeout:   time.Duration(time.Duration(DefaultHTTPTimeout) * time.Second),
-		Transport: c.Transport,
+		Transport: c.transport(),
 	}
 
 	response, err := client.Do(request)
@@ -155,3 +192,27 @@ func (c *Client) DeployChart(filename, deployname string) error {
 
 	return nil
 }
+
+// DefaultServerURL converts a host, host:port, or URL string to the default base server API path
+// to use with a Client
+func DefaultServerURL(host string) (*url.URL, error) {
+	if host == "" {
+		return nil, fmt.Errorf("host must be a URL or a host:port pair")
+	}
+	base := host
+	hostURL, err := url.Parse(base)
+	if err != nil {
+		return nil, err
+	}
+	if hostURL.Scheme == "" {
+		hostURL, err = url.Parse(DefaultHTTPProtocol + "://" + base)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if len(hostURL.Path) > 0 && !strings.HasSuffix(hostURL.Path, "/") {
+		hostURL.Path = hostURL.Path + "/"
+	}
+
+	return hostURL, nil
+}

dm/client_test.go

+package dm
+
+import (
+	"testing"
+)
+
+func TestDefaultServerURL(t *testing.T) {
+	tt := []struct {
+		host string
+		url  string
+	}{
+		{"127.0.0.1", "http://127.0.0.1"},
+		{"127.0.0.1:8080", "http://127.0.0.1:8080"},
+		{"foo.bar.com", "http://foo.bar.com"},
+		{"foo.bar.com/prefix", "http://foo.bar.com/prefix/"},
+		{"http://host/prefix", "http://host/prefix/"},
+		{"https://host/prefix", "https://host/prefix/"},
+		{"http://host", "http://host"},
+		{"http://host/other", "http://host/other/"},
+	}
+
+	for _, tc := range tt {
+		u, err := DefaultServerURL(tc.host)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if tc.url != u.String() {
+			t.Errorf("%s, expected host %s, got %s", tc.host, tc.url, u.String())
+		}
+	}
+}
+
+func TestURL(t *testing.T) {
+	tt := []struct {
+		host string
+		path string
+		url  string
+	}{
+		{"127.0.0.1", "foo", "http://127.0.0.1/foo"},
+		{"127.0.0.1:8080", "foo", "http://127.0.0.1:8080/foo"},
+		{"foo.bar.com", "foo", "http://foo.bar.com/foo"},
+		{"foo.bar.com/prefix", "foo", "http://foo.bar.com/prefix/foo"},
+		{"http://host/prefix", "foo", "http://host/prefix/foo"},
+		{"http://host", "foo", "http://host/foo"},
+		{"http://host/other", "/foo", "http://host/foo"},
+	}
+
+	for _, tc := range tt {
+		c := NewClient(tc.host)
+		p, err := c.url(tc.path)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if tc.url != p {
+			t.Errorf("expected %s, got %s", tc.url, p)
+		}
+	}
+}

glide.lock

-hash: fce0581223b80f7a04fbb4ad4bd7ff8fa3d12e879dba894ba448770933731887
-updated: 2016-02-02T17:30:13.283644703-07:00
+hash: 71e9a5a2d1f27a0ecfa70595154f87fee9f5519ba6bdea4d01834bab5aa29074
+updated: 2016-02-06T17:26:31.257079434-08:00
 imports:
 - name: github.com/codegangsta/cli
   version: cf1f63a7274872768d4037305d572b70b1199397
@@ -13,8 +13,6 @@ imports:
   version: 45bba206dd5270d96bac4942dcfe515726613249
 - name: github.com/google/go-github
   version: b8b4ac742977310ff6e75140a403a38dab109977
-  subpackages:
-  - /github
 - name: github.com/google/go-querystring
   version: 2a60fc2ba6c19de80291203597d752e9ba58e4c0
 - name: github.com/gorilla/context
@@ -27,8 +25,6 @@ imports:
   version: 14c555599c2a4f493c1e13fd1ea6fdf721739028
 - name: github.com/kubernetes/deployment-manager
   version: ""
-  repo: https://github.com/technosophos/deployment-manager
-  vcs: git
   subpackages:
   - /common
 - name: github.com/Masterminds/semver
@@ -43,6 +39,15 @@ imports:
   version: 8a57ed94ffd43444c0879fe75701732a38afc985
 - name: golang.org/x/text
   version: 5aaa1a807bf8a2f763540b140e7805973476eb88
+- name: google.golang.com/appengine/datastore
+  version: ""
+  repo: https://google.golang.com/appengine/datastore
+- name: google.golang.com/appengine/memcache
+  version: ""
+  repo: https://google.golang.com/appengine/memcache
+- name: google.golang.com/appengine/user
+  version: ""
+  repo: https://google.golang.com/appengine/user
 - name: google.golang.org/api
   version: 8fa1015948e6fc21c025050624e4c4e2f4f405c4
 - name: google.golang.org/appengine

glide.yaml

@@ -4,9 +4,6 @@ ignore:
 import:
 - package: github.com/codegangsta/cli
 - package: github.com/kubernetes/deployment-manager
-  version: feat/chartfile
-  repo: https://github.com/technosophos/deployment-manager
-  vcs: git
   subpackages:
   - /common
 - package: github.com/ghodss/yaml