Merge pull request #18888 from vespa-engine/bratseth/docids

Expect ids on the standard Vespa format

6 files changed, 90 insertions, 11 deletions

diff --git a/client/go/cmd/document.go b/client/go/cmd/document.go
index cf820d624a4..48b300db964 100644
--- a/client/go/cmd/document.go
+++ b/client/go/cmd/document.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/spf13/cobra"
 	"github.com/vespa-engine/vespa/util"
+	"github.com/vespa-engine/vespa/vespa"
 )
 
 func init() {
@@ -89,7 +90,17 @@ func post(documentId string, jsonFile string) {
 		}
 	}
 
-	url, _ := url.Parse(documentTarget() + "/document/v1/" + documentId)
+	documentPath, documentPathError := vespa.IdToUrlPath(documentId)
+	if documentPathError != nil {
+		log.Print("Invalid document id '", color.Red(documentId), "': ", documentPathError)
+		return
+	}
+
+	url, urlParseError := url.Parse(documentTarget() + "/document/v1/" + documentPath)
+	if urlParseError != nil {
+		log.Print("Invalid request path: '", color.Red(documentTarget()+"/document/v1/"+documentPath), "': ", urlParseError)
+		return
+	}
 
 	request := &http.Request{
 		URL:    url,
@@ -101,6 +112,7 @@ func post(documentId string, jsonFile string) {
 	response, err := util.HttpDo(request, time.Second*60, serviceDescription)
 	if response == nil {
 		log.Print("Request failed: ", color.Red(err))
+		return
 	}
 
 	defer response.Body.Close()
diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go
index 93532cdd69e..789747e989d 100644
--- a/client/go/cmd/document_test.go
+++ b/client/go/cmd/document_test.go
@@ -11,26 +11,27 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/vespa-engine/vespa/util"
+	"github.com/vespa-engine/vespa/vespa"
 )
 
 func TestDocumentPostWithIdArg(t *testing.T) {
-	assertDocumentPost([]string{"document", "post", "mynamespace/music/docid/1", "testdata/A-Head-Full-of-Dreams.json"},
-		"mynamespace/music/docid/1", "testdata/A-Head-Full-of-Dreams.json", t)
+	assertDocumentPost([]string{"document", "post", "id:mynamespace:music::head", "testdata/A-Head-Full-of-Dreams.json"},
+		"id:mynamespace:music::head", "testdata/A-Head-Full-of-Dreams.json", t)
 }
 
 func TestDocumentPostWithIdInDocument(t *testing.T) {
 	assertDocumentPost([]string{"document", "post", "testdata/A-Head-Full-of-Dreams-With-Id.json"},
-		"mynamespace/music/docid/1", "testdata/A-Head-Full-of-Dreams-With-Id.json", t)
+		"id:mynamespace:music::head", "testdata/A-Head-Full-of-Dreams-With-Id.json", t)
 }
 
 func TestDocumentPostWithIdInDocumentShortForm(t *testing.T) {
 	assertDocumentPost([]string{"document", "testdata/A-Head-Full-of-Dreams-With-Id.json"},
-		"mynamespace/music/docid/1", "testdata/A-Head-Full-of-Dreams-With-Id.json", t)
+		"id:mynamespace:music::head", "testdata/A-Head-Full-of-Dreams-With-Id.json", t)
 }
 
 func TestDocumentPostWithIdAsPutInDocument(t *testing.T) {
 	assertDocumentPost([]string{"document", "post", "testdata/A-Head-Full-of-Dreams-With-Put.json"},
-		"mynamespace/music/docid/1", "testdata/A-Head-Full-of-Dreams-With-Put.json", t)
+		"id:mynamespace:music::head", "testdata/A-Head-Full-of-Dreams-With-Put.json", t)
 }
 
 func TestDocumentIdNotSpecified(t *testing.T) {
@@ -55,7 +56,8 @@ func assertDocumentPost(arguments []string, documentId string, jsonFile string,
 		documentId+"\n",
 		executeCommand(t, client, arguments, []string{}))
 	target := getTarget(documentContext).document
-	assert.Equal(t, target+"/document/v1/"+documentId, client.lastRequest.URL.String())
+	expectedPath, _ := vespa.IdToUrlPath(documentId)
+	assert.Equal(t, target+"/document/v1/"+expectedPath, client.lastRequest.URL.String())
 	assert.Equal(t, "application/json", client.lastRequest.Header.Get("Content-Type"))
 	assert.Equal(t, "POST", client.lastRequest.Method)
 
@@ -77,7 +79,7 @@ func assertDocumentError(t *testing.T, status int, errorMessage string) {
 	assert.Equal(t,
 		"Invalid document (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"document", "post",
-			"mynamespace/music/docid/1",
+			"id:mynamespace:music::head",
 			"testdata/A-Head-Full-of-Dreams.json"}, []string{}))
 }
 
@@ -86,6 +88,6 @@ func assertDocumentServerError(t *testing.T, status int, errorMessage string) {
 	assert.Equal(t,
 		"Error from container (document api) at 127.0.0.1:8080 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"document", "post",
-			"mynamespace/music/docid/1",
+			"id:mynamespace:music::head",
 			"testdata/A-Head-Full-of-Dreams.json"}, []string{}))
 }
diff --git a/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Id.json b/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Id.json
index fddbbf94916..33c0cc88b04 100644
--- a/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Id.json
+++ b/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Id.json
@@ -1,5 +1,5 @@
 {
-    "id": "mynamespace/music/docid/1",
+    "id": "id:mynamespace:music::head",
     "fields": {
         "album": "A Head Full of Dreams",
         "artist": "Coldplay",
diff --git a/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Put.json b/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Put.json
index d6467073816..c7541210581 100644
--- a/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Put.json
+++ b/client/go/cmd/testdata/A-Head-Full-of-Dreams-With-Put.json
@@ -1,5 +1,5 @@
 {
-    "put": "mynamespace/music/docid/1",
+    "put": "id:mynamespace:music::head",
     "fields": {
         "album": "A Head Full of Dreams",
         "artist": "Coldplay",
diff --git a/client/go/vespa/id.go b/client/go/vespa/id.go
new file mode 100644
index 00000000000..70570188d93
--- /dev/null
+++ b/client/go/vespa/id.go
@@ -0,0 +1,46 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// vespa document ids
+// Author: bratseth
+
+package vespa
+
+import (
+	"errors"
+	"strings"
+)
+
+func IdToUrlPath(documentId string) (string, error) {
+	parts := strings.Split(documentId, ":")
+	formatAdvice := "Id should be on the form id:<namespace>:<document-type>:<attribute>?:<local-id-string>"
+	if len(parts) < 5 {
+		return "", errors.New(formatAdvice)
+	}
+
+	scheme := parts[0]
+	namespace := parts[1]
+	documentType := parts[2]
+	attribute := parts[3]
+	localId := strings.Join(parts[4:], "")
+
+	var group string
+	var number string
+	if strings.HasPrefix(attribute, "g=") {
+		group = attribute[2:]
+	} else if strings.HasPrefix(attribute, "n=") {
+		number = attribute[2:]
+	} else if attribute != "" {
+		return "", errors.New(formatAdvice + ": Attribute must be g=<string> or n=<integer>")
+	}
+
+	if scheme != "id" {
+		return "", errors.New(formatAdvice)
+	}
+
+	var attributeAsPath string
+	if group != "" {
+		attributeAsPath = "group/" + group + "/"
+	} else if number != "" {
+		attributeAsPath = "number/" + number + "/"
+	}
+	return namespace + "/" + documentType + "/" + attributeAsPath + "docid/" + localId, nil
+}
diff --git a/client/go/vespa/id_test.go b/client/go/vespa/id_test.go
new file mode 100644
index 00000000000..2e4a9a80dd9
--- /dev/null
+++ b/client/go/vespa/id_test.go
@@ -0,0 +1,19 @@
+package vespa
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIdToUrlPath(t *testing.T) {
+	assertIdToUrlPath("ns/type/docid/local", "id:ns:type::local", t)
+	assertIdToUrlPath("ns/type/number/123/docid/local", "id:ns:type:n=123:local", t)
+	assertIdToUrlPath("ns/type/group/mygroup/docid/local", "id:ns:type:g=mygroup:local", t)
+}
+
+func assertIdToUrlPath(expectedPath string, id string, t *testing.T) {
+	path, err := IdToUrlPath(id)
+	assert.Nil(t, err)
+	assert.Equal(t, expectedPath, path)
+}