Decouple JSON parsing from Document type

5 files changed, 78 insertions, 85 deletions

diff --git a/client/go/internal/vespa/document/dispatcher_test.go b/client/go/internal/vespa/document/dispatcher_test.go
index 384097670e1..04e0928f2a3 100644
--- a/client/go/internal/vespa/document/dispatcher_test.go
+++ b/client/go/internal/vespa/document/dispatcher_test.go
@@ -1,7 +1,6 @@
 package document
 
 import (
-	"encoding/json"
 	"sync"
 	"testing"
 
@@ -43,8 +42,8 @@ func TestDispatcher(t *testing.T) {
 	feeder := &mockFeeder{}
 	dispatcher := NewDispatcher(feeder, 2)
 	docs := []Document{
-		{PutId: "id:ns:type::doc1", Fields: json.RawMessage(`{"foo": "123"}`)},
-		{PutId: "id:ns:type::doc2", Fields: json.RawMessage(`{"bar": "456"}`)},
+		{Id: mustParseId("id:ns:type::doc1"), Operation: OperationPut, Body: []byte(`{"fields":{"foo": "123"}}`)},
+		{Id: mustParseId("id:ns:type::doc2"), Operation: OperationPut, Body: []byte(`{"fields":{"bar": "456"}}`)},
 	}
 	for _, d := range docs {
 		dispatcher.Enqueue(d)
@@ -57,19 +56,19 @@ func TestDispatcher(t *testing.T) {
 
 func TestDispatcherOrdering(t *testing.T) {
 	feeder := &mockFeeder{}
-	commonId := "id:ns:type::doc1"
+	commonId := mustParseId("id:ns:type::doc1")
 	docs := []Document{
-		mustParseDocument(Document{PutId: commonId}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc2"}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc3"}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc4"}),
-		mustParseDocument(Document{UpdateId: commonId}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc5"}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc6"}),
-		mustParseDocument(Document{RemoveId: commonId}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc7"}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc8"}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc9"}),
+		{Id: commonId, Operation: OperationPut},
+		{Id: mustParseId("id:ns:type::doc2"), Operation: OperationPut},
+		{Id: mustParseId("id:ns:type::doc3"), Operation: OperationPut},
+		{Id: mustParseId("id:ns:type::doc4"), Operation: OperationPut},
+		{Id: commonId, Operation: OperationUpdate},
+		{Id: mustParseId("id:ns:type::doc5"), Operation: OperationPut},
+		{Id: mustParseId("id:ns:type::doc6"), Operation: OperationPut},
+		{Id: commonId, Operation: OperationRemove},
+		{Id: mustParseId("id:ns:type::doc7"), Operation: OperationPut},
+		{Id: mustParseId("id:ns:type::doc8"), Operation: OperationPut},
+		{Id: mustParseId("id:ns:type::doc9"), Operation: OperationPut},
 	}
 	dispatcher := NewDispatcher(feeder, len(docs))
 	for _, d := range docs {
@@ -79,13 +78,13 @@ func TestDispatcherOrdering(t *testing.T) {
 
 	var wantDocs []Document
 	for _, d := range docs {
-		if d.Id.String() == commonId {
+		if d.Id.Equal(commonId) {
 			wantDocs = append(wantDocs, d)
 		}
 	}
 	var gotDocs []Document
 	for _, d := range feeder.documents {
-		if d.Id.String() == commonId {
+		if d.Id.Equal(commonId) {
 			gotDocs = append(gotDocs, d)
 		}
 	}
@@ -96,12 +95,12 @@ func TestDispatcherOrdering(t *testing.T) {
 
 func TestDispatcherOrderingWithFailures(t *testing.T) {
 	feeder := &mockFeeder{}
-	commonId := "id:ns:type::doc1"
+	commonId := mustParseId("id:ns:type::doc1")
 	docs := []Document{
-		mustParseDocument(Document{PutId: commonId}),
-		mustParseDocument(Document{PutId: commonId}),
-		mustParseDocument(Document{UpdateId: commonId}), // fails
-		mustParseDocument(Document{RemoveId: commonId}), // fails
+		{Id: commonId, Operation: OperationPut},
+		{Id: commonId, Operation: OperationPut},
+		{Id: commonId, Operation: OperationUpdate}, // fails
+		{Id: commonId, Operation: OperationRemove}, // fails
 	}
 	feeder.failAfterN(2)
 	dispatcher := NewDispatcher(feeder, len(docs))
@@ -116,11 +115,11 @@ func TestDispatcherOrderingWithFailures(t *testing.T) {
 	// Dispatching more documents for same ID fails implicitly
 	feeder.failAfterN(0)
 	dispatcher.start()
-	dispatcher.Enqueue(mustParseDocument(Document{PutId: commonId}))
-	dispatcher.Enqueue(mustParseDocument(Document{RemoveId: commonId}))
+	dispatcher.Enqueue(Document{Id: commonId, Operation: OperationPut})
+	dispatcher.Enqueue(Document{Id: commonId, Operation: OperationRemove})
 	// Other IDs are fine
-	doc2 := mustParseDocument(Document{PutId: "id:ns:type::doc2"})
-	doc3 := mustParseDocument(Document{PutId: "id:ns:type::doc3"})
+	doc2 := Document{Id: mustParseId("id:ns:type::doc2"), Operation: OperationPut}
+	doc3 := Document{Id: mustParseId("id:ns:type::doc3"), Operation: OperationPut}
 	dispatcher.Enqueue(doc2)
 	dispatcher.Enqueue(doc3)
 	dispatcher.Close()
diff --git a/client/go/internal/vespa/document/document.go b/client/go/internal/vespa/document/document.go
index 6aeafd80005..98cb2d1b6c6 100644
--- a/client/go/internal/vespa/document/document.go
+++ b/client/go/internal/vespa/document/document.go
@@ -103,11 +103,16 @@ func ParseId(serialized string) (Id, error) {
 	}, nil
 }
 
-// Document represents a Vespa document, and its operation.
+// Document represents a Vespa document operation.
 type Document struct {
 	Id        Id
 	Operation Operation
+	Condition string
+	Create    bool
+	Body      []byte
+}
 
+type jsonDocument struct {
 	IdString  string          `json:"id"`
 	PutId     string          `json:"put"`
 	UpdateId  string          `json:"update"`
@@ -117,16 +122,6 @@ type Document struct {
 	Fields    json.RawMessage `json:"fields"`
 }
 
-// Body returns the body part of this document, suitable for sending to the /document/v1 API.
-func (d Document) Body() []byte {
-	jsonObject := `{"fields":`
-	body := make([]byte, 0, len(jsonObject)+len(d.Fields)+1)
-	body = append(body, []byte(jsonObject)...)
-	body = append(body, d.Fields...)
-	body = append(body, byte('}'))
-	return body
-}
-
 // Decoder decodes documents from a JSON structure which is either an array of objects, or objects separated by newline.
 type Decoder struct {
 	buf   *bufio.Reader
@@ -193,14 +188,11 @@ func (d *Decoder) decode() (Document, error) {
 		}
 		return Document{}, err
 	}
-	doc := Document{}
+	doc := jsonDocument{}
 	if err := d.dec.Decode(&doc); err != nil {
 		return Document{}, err
 	}
-	if err := parseDocument(&doc); err != nil {
-		return Document{}, err
-	}
-	return doc, nil
+	return parseDocument(&doc)
 }
 
 func NewDecoder(r io.Reader) *Decoder {
@@ -211,29 +203,43 @@ func NewDecoder(r io.Reader) *Decoder {
 	}
 }
 
-func parseDocument(d *Document) error {
+func parseDocument(d *jsonDocument) (Document, error) {
 	id := ""
+	var op Operation
 	if d.IdString != "" {
-		d.Operation = OperationPut
+		op = OperationPut
 		id = d.IdString
 	} else if d.PutId != "" {
-		d.Operation = OperationPut
+		op = OperationPut
 		id = d.PutId
 	} else if d.UpdateId != "" {
-		d.Operation = OperationUpdate
+		op = OperationUpdate
 		id = d.UpdateId
 	} else if d.RemoveId != "" {
-		d.Operation = OperationRemove
+		op = OperationRemove
 		id = d.RemoveId
 	} else {
-		return fmt.Errorf("invalid document: missing operation: %v", d)
+		return Document{}, fmt.Errorf("invalid document: missing operation: %v", d)
 	}
 	docId, err := ParseId(id)
 	if err != nil {
-		return err
+		return Document{}, err
 	}
-	d.Id = docId
-	return nil
+	var body []byte
+	if d.Fields != nil {
+		jsonObject := `{"fields":`
+		body = make([]byte, 0, len(jsonObject)+len(d.Fields)+1)
+		body = append(body, []byte(jsonObject)...)
+		body = append(body, d.Fields...)
+		body = append(body, byte('}'))
+	}
+	return Document{
+		Id:        docId,
+		Operation: op,
+		Condition: d.Condition,
+		Create:    d.Create,
+		Body:      body,
+	}, nil
 }
 
 func parseError(value string) error {
diff --git a/client/go/internal/vespa/document/document_test.go b/client/go/internal/vespa/document/document_test.go
index 478dd795dd8..111b9e37acc 100644
--- a/client/go/internal/vespa/document/document_test.go
+++ b/client/go/internal/vespa/document/document_test.go
@@ -1,8 +1,6 @@
 package document
 
 import (
-	"bytes"
-	"encoding/json"
 	"io"
 	"reflect"
 	"strings"
@@ -11,11 +9,12 @@ import (
 
 func ptr[T any](v T) *T { return &v }
 
-func mustParseDocument(d Document) Document {
-	if err := parseDocument(&d); err != nil {
+func mustParseId(s string) Id {
+	id, err := ParseId(s)
+	if err != nil {
 		panic(err)
 	}
-	return d
+	return id
 }
 
 func TestParseId(t *testing.T) {
@@ -134,9 +133,9 @@ func testDocumentDecoder(t *testing.T, jsonLike string) {
 	t.Helper()
 	r := NewDecoder(strings.NewReader(jsonLike))
 	want := []Document{
-		mustParseDocument(Document{PutId: "id:ns:type::doc1", Fields: json.RawMessage(`{"foo": "123"}`)}),
-		mustParseDocument(Document{PutId: "id:ns:type::doc2", Fields: json.RawMessage(`{"bar": "456"}`)}),
-		mustParseDocument(Document{RemoveId: "id:ns:type::doc1", Fields: json.RawMessage(nil)}),
+		{Id: mustParseId("id:ns:type::doc1"), Operation: OperationPut, Body: []byte(`{"fields":{"foo": "123"}}`)},
+		{Id: mustParseId("id:ns:type::doc2"), Operation: OperationPut, Body: []byte(`{"fields":{"bar": "456"}}`)},
+		{Id: mustParseId("id:ns:type::doc1"), Operation: OperationRemove},
 	}
 	got := []Document{}
 	for {
@@ -179,12 +178,3 @@ func TestDocumentDecoder(t *testing.T) {
 		t.Errorf("want error %q, got %q", wantErr, err.Error())
 	}
 }
-
-func TestDocumentBody(t *testing.T) {
-	doc := Document{Fields: json.RawMessage([]byte(`{"foo": "123"}`))}
-	got := doc.Body()
-	want := []byte(`{"fields":{"foo": "123"}}`)
-	if !bytes.Equal(got, want) {
-		t.Errorf("got %q, want %q", got, want)
-	}
-}
diff --git a/client/go/internal/vespa/document/http.go b/client/go/internal/vespa/document/http.go
index 981bad5140d..2e01d4564ab 100644
--- a/client/go/internal/vespa/document/http.go
+++ b/client/go/internal/vespa/document/http.go
@@ -130,8 +130,7 @@ func (c *Client) Send(document Document) Result {
 		stats.Errors = 1
 		return Result{Status: StatusError, Err: err}
 	}
-	body := document.Body()
-	req, err := http.NewRequest(method, url.String(), bytes.NewReader(body))
+	req, err := http.NewRequest(method, url.String(), bytes.NewReader(document.Body))
 	if err != nil {
 		stats.Errors = 1
 		return Result{Status: StatusError, Err: err}
@@ -146,7 +145,7 @@ func (c *Client) Send(document Document) Result {
 	stats.ResponsesByCode = map[int]int64{
 		resp.StatusCode: 1,
 	}
-	stats.BytesSent = int64(len(body))
+	stats.BytesSent = int64(len(document.Body))
 	return c.createResult(document.Id, &stats, resp)
 }
 
diff --git a/client/go/internal/vespa/document/http_test.go b/client/go/internal/vespa/document/http_test.go
index ca59c4c2af0..f02c87730d5 100644
--- a/client/go/internal/vespa/document/http_test.go
+++ b/client/go/internal/vespa/document/http_test.go
@@ -2,7 +2,6 @@ package document
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -27,9 +26,9 @@ func (c *manualClock) now() time.Time {
 
 func TestClientSend(t *testing.T) {
 	docs := []Document{
-		mustParseDocument(Document{Create: true, UpdateId: "id:ns:type::doc1", Fields: json.RawMessage(`{"foo": "123"}`)}),
-		mustParseDocument(Document{Create: true, UpdateId: "id:ns:type::doc2", Fields: json.RawMessage(`{"foo": "456"}`)}),
-		mustParseDocument(Document{Create: true, UpdateId: "id:ns:type::doc3", Fields: json.RawMessage(`{"baz": "789"}`)}),
+		{Create: true, Id: mustParseId("id:ns:type::doc1"), Operation: OperationUpdate, Body: []byte(`{"fields":{"foo": "123"}}`)},
+		{Create: true, Id: mustParseId("id:ns:type::doc2"), Operation: OperationUpdate, Body: []byte(`{"fields":{"foo": "456"}}`)},
+		{Create: true, Id: mustParseId("id:ns:type::doc3"), Operation: OperationUpdate, Body: []byte(`{"fields":{"baz": "789"}}`)},
 	}
 	httpClient := mock.HTTPClient{}
 	client := NewClient(ClientOptions{
@@ -60,7 +59,7 @@ func TestClientSend(t *testing.T) {
 		if err != nil {
 			t.Fatalf("got unexpected error %q", err)
 		}
-		wantBody := doc.Body()
+		wantBody := doc.Body
 		if !bytes.Equal(body, wantBody) {
 			t.Errorf("got r.Body = %q, want %q", string(body), string(wantBody))
 		}
@@ -149,25 +148,25 @@ func TestClientFeedURL(t *testing.T) {
 		url    string
 	}{
 		{
-			mustParseDocument(Document{
-				IdString: "id:ns:type::user",
-			}),
+			Document{Id: mustParseId("id:ns:type::user")},
 			"POST",
 			"https://example.com/document/v1/ns/type/docid/user?foo=ba%2Fr",
 		},
 		{
-			mustParseDocument(Document{
-				UpdateId:  "id:ns:type::user",
+			Document{
+				Id:        mustParseId("id:ns:type::user"),
+				Operation: OperationUpdate,
 				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",
-			}),
+			Document{
+				Id:        mustParseId("id:ns:type::user"),
+				Operation: OperationRemove,
+			},
 			"DELETE",
 			"https://example.com/document/v1/ns/type/docid/user?foo=ba%2Fr",
 		},