Move URL building

4 files changed, 153 insertions, 151 deletions

diff --git a/client/go/internal/vespa/document/document.go b/client/go/internal/vespa/document/document.go
index b327011be4b..0b23df18735 100644
--- a/client/go/internal/vespa/document/document.go
+++ b/client/go/internal/vespa/document/document.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"net/url"
 	"strconv"
 	"strings"
 )
@@ -33,32 +32,6 @@ type Document struct {
 	Fields    json.RawMessage `json:"fields"`
 }
 
-// FeedURL returns the HTTP method and URL to use for feeding this document.
-func (d Document) FeedURL(baseurl string, queryParams url.Values) (string, *url.URL, error) {
-	u, err := url.Parse(baseurl)
-	if err != nil {
-		return "", nil, fmt.Errorf("invalid base url: %w", err)
-	}
-	httpMethod := ""
-	switch d.Operation {
-	case OperationPut:
-		httpMethod = "POST"
-	case OperationUpdate:
-		httpMethod = "PUT"
-	case OperationRemove:
-		httpMethod = "DELETE"
-	}
-	if d.Condition != "" {
-		queryParams.Set("condition", d.Condition)
-	}
-	if d.Create {
-		queryParams.Set("create", "true")
-	}
-	u.Path = d.Id.URLPath()
-	u.RawQuery = queryParams.Encode()
-	return httpMethod, u, nil
-}
-
 // Body returns the body part of this document, suitable for sending to the /document/v1 API.
 func (d Document) Body() []byte {
 	jsonObject := `{"fields":`
@@ -214,28 +187,6 @@ func (d DocumentId) String() string {
 	return sb.String()
 }
 
-// URLPaths returns the feeding path for this document.
-func (d DocumentId) URLPath() string {
-	var sb strings.Builder
-	sb.WriteString("/document/v1/")
-	sb.WriteString(url.PathEscape(d.Namespace))
-	sb.WriteString("/")
-	sb.WriteString(url.PathEscape(d.Type))
-	if d.Number != nil {
-		sb.WriteString("/number/")
-		n := uint64(*d.Number)
-		sb.WriteString(strconv.FormatUint(n, 10))
-	} else if d.Group != "" {
-		sb.WriteString("/group/")
-		sb.WriteString(url.PathEscape(d.Group))
-	} else {
-		sb.WriteString("/docid")
-	}
-	sb.WriteString("/")
-	sb.WriteString(url.PathEscape(d.UserSpecific))
-	return sb.String()
-}
-
 func parseError(value string) error {
 	return fmt.Errorf("invalid document: expected id:<namespace>:<document-type>:[n=<number>|g=<group>]:<user-specific>, got %q", value)
 }
diff --git a/client/go/internal/vespa/document/document_test.go b/client/go/internal/vespa/document/document_test.go
index 03df7b62045..4bd79cf0796 100644
--- a/client/go/internal/vespa/document/document_test.go
+++ b/client/go/internal/vespa/document/document_test.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"io"
-	"net/url"
 	"reflect"
 	"strings"
 	"testing"
@@ -19,7 +18,7 @@ func mustParseDocument(d Document) Document {
 	return d
 }
 
-func TestParseDocumentId(t *testing.T) {
+func TestParseId(t *testing.T) {
 	tests := []struct {
 		in   string
 		out  DocumentId
@@ -164,105 +163,6 @@ func TestDocumentDecoder(t *testing.T) {
 	}
 }
 
-func TestDocumentIdURLPath(t *testing.T) {
-	tests := []struct {
-		in  DocumentId
-		out string
-	}{
-		{
-			DocumentId{
-				Namespace:    "ns-with-/",
-				Type:         "type-with-/",
-				UserSpecific: "user",
-			},
-			"/document/v1/ns-with-%2F/type-with-%2F/docid/user",
-		},
-		{
-			DocumentId{
-				Namespace:    "ns",
-				Type:         "type",
-				Number:       ptr(int64(123)),
-				UserSpecific: "user",
-			},
-			"/document/v1/ns/type/number/123/user",
-		},
-		{
-			DocumentId{
-				Namespace:    "ns",
-				Type:         "type",
-				Group:        "foo",
-				UserSpecific: "user",
-			},
-			"/document/v1/ns/type/group/foo/user",
-		},
-		{
-			DocumentId{
-				Namespace:    "ns",
-				Type:         "type",
-				UserSpecific: "user::specific",
-			},
-			"/document/v1/ns/type/docid/user::specific",
-		},
-		{
-			DocumentId{
-				Namespace:    "ns",
-				Type:         "type",
-				UserSpecific: ":",
-			},
-			"/document/v1/ns/type/docid/:",
-		},
-	}
-	for i, tt := range tests {
-		path := tt.in.URLPath()
-		if path != tt.out {
-			t.Errorf("#%d: documentPath(%q) = %s, want %s", i, tt.in, path, tt.out)
-		}
-	}
-}
-
-func TestDocumentURL(t *testing.T) {
-	tests := []struct {
-		in     Document
-		method string
-		url    string
-	}{
-		{
-			mustParseDocument(Document{
-				IdString: "id:ns:type::user",
-			}),
-			"POST",
-			"https://example.com/document/v1/ns/type/docid/user?foo=ba%2Fr",
-		},
-		{
-			mustParseDocument(Document{
-				UpdateId:  "id:ns:type::user",
-				Create:    true,
-				Condition: "false",
-			}),
-			"PUT",
-			"https://example.com/document/v1/ns/type/docid/user?condition=false&create=true&foo=ba%2Fr",
-		},
-		{
-			mustParseDocument(Document{
-				RemoveId: "id:ns:type::user",
-			}),
-			"DELETE",
-			"https://example.com/document/v1/ns/type/docid/user?foo=ba%2Fr",
-		},
-	}
-	for i, tt := range tests {
-		moreParams := url.Values{}
-		moreParams.Set("foo", "ba/r")
-		method, u, err := tt.in.FeedURL("https://example.com", moreParams)
-		if err != nil {
-			t.Errorf("#%d: got unexpected error = %s, want none", i, err)
-		}
-		if u.String() != tt.url || method != tt.method {
-			t.Errorf("#%d: URL() = (%s, %s), want (%s, %s)", i, method, u.String(), tt.method, tt.url)
-		}
-	}
-}
-
 func TestDocumentBody(t *testing.T) {
 	doc := Document{Fields: json.RawMessage([]byte(`{"foo": "123"}`))}
 	got := doc.Body()
diff --git a/client/go/internal/vespa/document/http.go b/client/go/internal/vespa/document/http.go
index 1d0248c35f9..eed92c8cbcf 100644
--- a/client/go/internal/vespa/document/http.go
+++ b/client/go/internal/vespa/document/http.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -65,6 +66,52 @@ func (c *Client) queryParams() url.Values {
 	return params
 }
 
+func urlPath(id DocumentId) string {
+	var sb strings.Builder
+	sb.WriteString("/document/v1/")
+	sb.WriteString(url.PathEscape(id.Namespace))
+	sb.WriteString("/")
+	sb.WriteString(url.PathEscape(id.Type))
+	if id.Number != nil {
+		sb.WriteString("/number/")
+		n := uint64(*id.Number)
+		sb.WriteString(strconv.FormatUint(n, 10))
+	} else if id.Group != "" {
+		sb.WriteString("/group/")
+		sb.WriteString(url.PathEscape(id.Group))
+	} else {
+		sb.WriteString("/docid")
+	}
+	sb.WriteString("/")
+	sb.WriteString(url.PathEscape(id.UserSpecific))
+	return sb.String()
+}
+
+func (c *Client) feedURL(d Document, queryParams url.Values) (string, *url.URL, error) {
+	u, err := url.Parse(c.options.BaseURL)
+	if err != nil {
+		return "", nil, fmt.Errorf("invalid base url: %w", err)
+	}
+	httpMethod := ""
+	switch d.Operation {
+	case OperationPut:
+		httpMethod = "POST"
+	case OperationUpdate:
+		httpMethod = "PUT"
+	case OperationRemove:
+		httpMethod = "DELETE"
+	}
+	if d.Condition != "" {
+		queryParams.Set("condition", d.Condition)
+	}
+	if d.Create {
+		queryParams.Set("create", "true")
+	}
+	u.Path = urlPath(d.Id)
+	u.RawQuery = queryParams.Encode()
+	return httpMethod, u, nil
+}
+
 // Send given document the URL configured in this client.
 func (c *Client) Send(document Document) Result {
 	start := c.now()
@@ -77,7 +124,7 @@ func (c *Client) Send(document Document) Result {
 		stats.MaxLatency = latency
 		c.addStats(&stats)
 	}()
-	method, url, err := document.FeedURL(c.options.BaseURL, c.queryParams())
+	method, url, err := c.feedURL(document, c.queryParams())
 	if err != nil {
 		stats.Errors = 1
 		return Result{Status: StatusError, Err: err}
diff --git a/client/go/internal/vespa/document/http_test.go b/client/go/internal/vespa/document/http_test.go
index 2bc7feb21ff..fe0ad28e86e 100644
--- a/client/go/internal/vespa/document/http_test.go
+++ b/client/go/internal/vespa/document/http_test.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"reflect"
 	"testing"
 	"time"
@@ -84,3 +85,106 @@ func TestClientSend(t *testing.T) {
 		t.Errorf("got %+v, want %+v", stats, want)
 	}
 }
+
+func TestURLPath(t *testing.T) {
+	tests := []struct {
+		in  DocumentId
+		out string
+	}{
+		{
+			DocumentId{
+				Namespace:    "ns-with-/",
+				Type:         "type-with-/",
+				UserSpecific: "user",
+			},
+			"/document/v1/ns-with-%2F/type-with-%2F/docid/user",
+		},
+		{
+			DocumentId{
+				Namespace:    "ns",
+				Type:         "type",
+				Number:       ptr(int64(123)),
+				UserSpecific: "user",
+			},
+			"/document/v1/ns/type/number/123/user",
+		},
+		{
+			DocumentId{
+				Namespace:    "ns",
+				Type:         "type",
+				Group:        "foo",
+				UserSpecific: "user",
+			},
+			"/document/v1/ns/type/group/foo/user",
+		},
+		{
+			DocumentId{
+				Namespace:    "ns",
+				Type:         "type",
+				UserSpecific: "user::specific",
+			},
+			"/document/v1/ns/type/docid/user::specific",
+		},
+		{
+			DocumentId{
+				Namespace:    "ns",
+				Type:         "type",
+				UserSpecific: ":",
+			},
+			"/document/v1/ns/type/docid/:",
+		},
+	}
+	for i, tt := range tests {
+		path := urlPath(tt.in)
+		if path != tt.out {
+			t.Errorf("#%d: documentPath(%q) = %s, want %s", i, tt.in, path, tt.out)
+		}
+	}
+}
+
+func TestClientFeedURL(t *testing.T) {
+	tests := []struct {
+		in     Document
+		method string
+		url    string
+	}{
+		{
+			mustParseDocument(Document{
+				IdString: "id:ns:type::user",
+			}),
+			"POST",
+			"https://example.com/document/v1/ns/type/docid/user?foo=ba%2Fr",
+		},
+		{
+			mustParseDocument(Document{
+				UpdateId:  "id:ns:type::user",
+				Create:    true,
+				Condition: "false",
+			}),
+			"PUT",
+			"https://example.com/document/v1/ns/type/docid/user?condition=false&create=true&foo=ba%2Fr",
+		},
+		{
+			mustParseDocument(Document{
+				RemoveId: "id:ns:type::user",
+			}),
+			"DELETE",
+			"https://example.com/document/v1/ns/type/docid/user?foo=ba%2Fr",
+		},
+	}
+	httpClient := mock.HTTPClient{}
+	client := NewClient(ClientOptions{
+		BaseURL: "https://example.com",
+	}, &httpClient)
+	for i, tt := range tests {
+		moreParams := url.Values{}
+		moreParams.Set("foo", "ba/r")
+		method, u, err := client.feedURL(tt.in, moreParams)
+		if err != nil {
+			t.Errorf("#%d: got unexpected error = %s, want none", i, err)
+		}
+		if u.String() != tt.url || method != tt.method {
+			t.Errorf("#%d: URL() = (%s, %s), want (%s, %s)", i, method, u.String(), tt.method, tt.url)
+		}
+	}
+}