From f02605a52821dabbe8d246d2529f4b567579ab90 Mon Sep 17 00:00:00 2001
From: Travis Turner <travis@pilosa.com>
Date: Tue, 23 Oct 2018 17:35:17 -0500
Subject: [PATCH] consolidate ImportOptions setup. use url.Values{}. fix
 comments.

---
 api.go              | 40 +++++++++++++++++++++-------------------
 http/client.go      | 11 +++++++----
 http/client_test.go |  4 ++--
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/api.go b/api.go
index df1265433..af206496b 100644
--- a/api.go
+++ b/api.go
@@ -239,6 +239,17 @@ func (api *API) Field(_ context.Context, indexName, fieldName string) (*Field, e
 	return field, nil
 }
 
+func setUpImportOptions(opts ...ImportOption) (*ImportOptions, error) {
+	options := &ImportOptions{}
+	for _, opt := range opts {
+		err := opt(options)
+		if err != nil {
+			return nil, errors.Wrap(err, "applying option")
+		}
+	}
+	return options, nil
+}
+
 // ImportRoaring is a low level interface for importing data to Pilosa when
 // extremely high throughput is desired. The data must be encoded in a
 // particular way which may be unintuitive (discussed below). The data is merged
@@ -262,12 +273,9 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string,
 	}
 
 	// Set up import options.
-	options := &ImportOptions{}
-	for _, opt := range opts {
-		err := opt(options)
-		if err != nil {
-			return errors.Wrap(err, "applying option")
-		}
+	options, err := setUpImportOptions(opts...)
+	if err != nil {
+		return errors.Wrap(err, "setting up import options")
 	}
 
 	nodes := api.cluster.shardNodes(indexName, shard)
@@ -685,7 +693,7 @@ type ImportOptions struct {
 	Clear bool
 }
 
-// ImportOption is a functional option type for API.Import
+// ImportOption is a functional option type for API.Import.
 type ImportOption func(*ImportOptions) error
 
 func OptImportOptionsClear(c bool) ImportOption {
@@ -702,12 +710,9 @@ func (api *API) Import(_ context.Context, req *ImportRequest, opts ...ImportOpti
 	}
 
 	// Set up import options.
-	options := &ImportOptions{}
-	for _, opt := range opts {
-		err := opt(options)
-		if err != nil {
-			return errors.Wrap(err, "applying option")
-		}
+	options, err := setUpImportOptions(opts...)
+	if err != nil {
+		return errors.Wrap(err, "setting up import options")
 	}
 
 	index := api.holder.Index(req.Index)
@@ -773,12 +778,9 @@ func (api *API) ImportValue(_ context.Context, req *ImportValueRequest, opts ...
 	}
 
 	// Set up import options.
-	options := &ImportOptions{}
-	for _, opt := range opts {
-		err := opt(options)
-		if err != nil {
-			return errors.Wrap(err, "applying option")
-		}
+	options, err := setUpImportOptions(opts...)
+	if err != nil {
+		return errors.Wrap(err, "setting up import options")
 	}
 
 	index := api.holder.Index(req.Index)
diff --git a/http/client.go b/http/client.go
index 17c0b2d63..06bb31479 100644
--- a/http/client.go
+++ b/http/client.go
@@ -433,10 +433,11 @@ func (c *InternalClient) importNode(ctx context.Context, node *pilosa.Node, inde
 	path := fmt.Sprintf("/index/%s/field/%s/import", index, field)
 	u := nodePathToURL(node, path)
 
-	url := u.String()
+	vals := url.Values{}
 	if opts.Clear {
-		url += "?clear=true"
+		vals.Set("clear", "true")
 	}
+	url := fmt.Sprintf("%s?%s", u.String(), vals.Encode())
 
 	req, err := http.NewRequest("POST", url, bytes.NewReader(buf))
 	if err != nil {
@@ -588,10 +589,12 @@ func (c *InternalClient) ImportRoaring(ctx context.Context, uri *pilosa.URI, ind
 		}
 	}
 
-	url := fmt.Sprintf("%s/index/%s/field/%s/import-roaring/%d?remote=%v", uri, index, field, shard, remote)
+	vals := url.Values{}
+	vals.Set("remote", strconv.FormatBool(remote))
 	if options.Clear {
-		url += "&clear=true"
+		vals.Set("clear", "true")
 	}
+	url := fmt.Sprintf("%s/index/%s/field/%s/import-roaring/%d?%s", uri, index, field, shard, vals.Encode())
 
 	// Generate HTTP request.
 	req, err := http.NewRequest("POST", url, bytes.NewBuffer(data))
diff --git a/http/client_test.go b/http/client_test.go
index be6bdb996..4ba076408 100644
--- a/http/client_test.go
+++ b/http/client_test.go
@@ -640,7 +640,7 @@ func TestClient_ImportKeys(t *testing.T) {
 			t.Fatalf("unexpected values: got sum=%v, count=%v; expected sum=50, cnt=3", sum, cnt)
 		}
 
-		// Verify Range
+		// Verify Range.
 		queryRequest := &pilosa.QueryRequest{
 			Query:  fmt.Sprintf(`Range(%s>10)`, fldName),
 			Remote: false,
@@ -671,7 +671,7 @@ func TestClient_ImportKeys(t *testing.T) {
 			t.Fatalf("unexpected values: got sum=%v, count=%v; expected sum=30, cnt=2", sum, cnt)
 		}
 
-		// Verify Range
+		// Verify Range.
 		queryRequest = &pilosa.QueryRequest{
 			Query:  fmt.Sprintf(`Range(%s>10)`, fldName),
 			Remote: false,