Merge pull request #18911 from vespa-engine/bratseth/documeent-operations

Bratseth/documeent operations

6 files changed, 62 insertions, 24 deletions

diff --git a/client/go/cmd/deploy.go b/client/go/cmd/deploy.go
index 94fb985d575..9c867241d52 100644
--- a/client/go/cmd/deploy.go
+++ b/client/go/cmd/deploy.go
@@ -77,9 +77,9 @@ var prepareCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		resolvedSrc, err := vespa.Prepare(vespa.Deployment{ApplicationSource: applicationSource(args)})
 		if err == nil {
-			log.Printf("Prepared %s successfully", color.Cyan(resolvedSrc))
+			log.Print(color.Green("Success: "), "Prepared ", color.Cyan(resolvedSrc))
 		} else {
-			log.Print(color.Red("Error: "), err)
+			log.Print(color.Red("Error:"), err)
 		}
 	},
 }
@@ -91,7 +91,7 @@ var activateCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		resolvedSrc, err := vespa.Activate(vespa.Deployment{ApplicationSource: applicationSource(args)})
 		if err == nil {
-			log.Print(color.Green("Success: "), "activated ", color.Cyan(resolvedSrc))
+			log.Print(color.Green("Success: "), "Activated ", color.Cyan(resolvedSrc))
 		} else {
 			log.Print(color.Red("Error: "), err)
 		}
diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go
index fdd1d9f5613..d4fddf11f99 100644
--- a/client/go/cmd/deploy_test.go
+++ b/client/go/cmd/deploy_test.go
@@ -54,8 +54,27 @@ func TestDeployApplicationDirectoryWithPomAndEmptyTarget(t *testing.T) {
 		executeCommand(t, client, []string{"deploy", "testdata/applications/withEmptyTarget"}, []string{}))
 }
 
-func TestDeployApplicationPackageError(t *testing.T) {
-	assertApplicationPackageError(t, 401, "Application package error")
+func TestDeployApplicationPackageErrorWithUnexpectedNonJson(t *testing.T) {
+	assertApplicationPackageError(t, 401,
+		"Raw text error",
+		"Raw text error")
+}
+
+func TestDeployApplicationPackageErrorWithUnexpectedJson(t *testing.T) {
+	assertApplicationPackageError(t, 401,
+		`{
+    "some-unexpected-json": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here"
+}`,
+		`{"some-unexpected-json": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here"}`)
+}
+
+func TestDeployApplicationPackageErrorWithExpectedFormat(t *testing.T) {
+	assertApplicationPackageError(t, 400,
+		"Invalid XML, error in services.xml:\nelement \"nosuch\" not allowed here",
+		`{
+         "error-code": "INVALID_APPLICATION_PACKAGE",
+         "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here\n"
+     }`)
 }
 
 func TestDeployError(t *testing.T) {
@@ -81,10 +100,10 @@ func assertDeployRequestMade(target string, client *mockHttpClient, t *testing.T
 	assert.Equal(t, "PK\x03\x04\x14\x00\b", string(buf))
 }
 
-func assertApplicationPackageError(t *testing.T, status int, errorMessage string) {
-	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
+func assertApplicationPackageError(t *testing.T, status int, expectedMessage string, returnBody string) {
+	client := &mockHttpClient{nextStatus: status, nextBody: returnBody}
 	assert.Equal(t,
-		"Error: Invalid application package (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Invalid application package (Status "+strconv.Itoa(status)+")\n\n"+expectedMessage+"\n",
 		executeCommand(t, client, []string{"deploy", "testdata/applications/withTarget/target/application.zip"}, []string{}))
 }
 
diff --git a/client/go/cmd/document.go b/client/go/cmd/document.go
index 9f3754873ad..71114fc8df6 100644
--- a/client/go/cmd/document.go
+++ b/client/go/cmd/document.go
@@ -12,7 +12,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -71,7 +70,8 @@ func post(documentId string, jsonFile string) {
 
 	fileReader, fileError := os.Open(jsonFile)
 	if fileError != nil {
-		log.Printf("Could not open file at %s: %s", color.Cyan(jsonFile), fileError)
+		log.Print(color.Red("Error: "), "Could not open file '", color.Cyan(jsonFile), "'")
+		log.Print(color.Brown(fileError))
 		return
 	}
 
@@ -83,20 +83,20 @@ func post(documentId string, jsonFile string) {
 		if doc["put"] != nil {
 			documentId = doc["put"].(string) // document feeder format
 		} else {
-			log.Print("No document id given neither as argument or as a 'put' key in the json file")
+			log.Print(color.Red("Error: "), "No document id given neither as argument or as a 'put' key in the json file")
 			return
 		}
 	}
 
 	documentPath, documentPathError := vespa.IdToURLPath(documentId)
 	if documentPathError != nil {
-		log.Print("Invalid document id '", color.Red(documentId), "': ", documentPathError)
+		log.Print(color.Red("Error: "), "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)
+		log.Print(color.Red("Error: "), "Invalid request path: '", color.Red(documentTarget()+"/document/v1/"+documentPath), "': ", urlParseError)
 		return
 	}
 
@@ -109,7 +109,7 @@ func post(documentId string, jsonFile string) {
 	serviceDescription := "Container (document API)"
 	response, err := util.HttpDo(request, time.Second*60, serviceDescription)
 	if response == nil {
-		log.Print("Request failed: ", color.Red(err))
+		log.Print(color.Red("Error: "), "Request failed:", err)
 		return
 	}
 
@@ -117,10 +117,10 @@ func post(documentId string, jsonFile string) {
 	if response.StatusCode == 200 {
 		log.Print(color.Green(documentId))
 	} else if response.StatusCode/100 == 4 {
-		log.Printf("Invalid document (%s):", color.Red(response.Status))
+		log.Print(color.Red("Error: "), "Invalid document: ", response.Status, "\n\n")
 		log.Print(util.ReaderToJSON(response.Body))
 	} else {
-		log.Printf("Error from %s at %s (%s):", color.Cyan(strings.ToLower(serviceDescription)), color.Cyan(request.URL.Host), color.Red(response.Status))
+		log.Print(color.Red("Error: "), serviceDescription, " at ", color.Cyan(request.URL.Host), ": ", response.Status, "\n\n")
 		log.Print(util.ReaderToJSON(response.Body))
 	}
 }
diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go
index 061c86e7725..7d6e107e3c2 100644
--- a/client/go/cmd/document_test.go
+++ b/client/go/cmd/document_test.go
@@ -33,7 +33,7 @@ func TestDocumentIdNotSpecified(t *testing.T) {
 	arguments := []string{"document", "post", "testdata/A-Head-Full-of-Dreams-Without-Id.json"}
 	client := &mockHttpClient{}
 	assert.Equal(t,
-		"No document id given neither as argument or as a 'put' key in the json file\n",
+		"Error: No document id given neither as argument or as a 'put' key in the json file\n",
 		executeCommand(t, client, arguments, []string{}))
 }
 
@@ -72,7 +72,7 @@ func assertDocumentPostShortForm(documentId string, jsonFile string, t *testing.
 func assertDocumentError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
 	assert.Equal(t,
-		"Invalid document (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Invalid document: Status "+strconv.Itoa(status)+"\n\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"document", "post",
 			"id:mynamespace:music::a-head-full-of-dreams",
 			"testdata/A-Head-Full-of-Dreams.json"}, []string{}))
@@ -81,7 +81,7 @@ func assertDocumentError(t *testing.T, status int, errorMessage string) {
 func assertDocumentServerError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
 	assert.Equal(t,
-		"Error from container (document api) at 127.0.0.1:8080 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Container (document API) at 127.0.0.1:8080: Status "+strconv.Itoa(status)+"\n\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"document", "post",
 			"id:mynamespace:music::a-head-full-of-dreams",
 			"testdata/A-Head-Full-of-Dreams.json"}, []string{}))
diff --git a/client/go/cmd/query.go b/client/go/cmd/query.go
index 2c25168ca30..97ae416196d 100644
--- a/client/go/cmd/query.go
+++ b/client/go/cmd/query.go
@@ -56,10 +56,10 @@ func query(arguments []string) {
 	if response.StatusCode == 200 {
 		log.Print(util.ReaderToJSON(response.Body))
 	} else if response.StatusCode/100 == 4 {
-		log.Print(color.Red("Error: "), "Invalid query: ", response.Status)
+		log.Print(color.Red("Error: "), "Invalid query: ", response.Status, "\n")
 		log.Print(util.ReaderToJSON(response.Body))
 	} else {
-		log.Print(color.Red("Error: "), response.Status, " from container at ", color.Cyan(url.Host))
+		log.Print(color.Red("Error: "), response.Status, " from container at ", color.Cyan(url.Host), "\n")
 		log.Print(util.ReaderToJSON(response.Body))
 	}
 }
diff --git a/client/go/vespa/deploy.go b/client/go/vespa/deploy.go
index 037c92b5458..9955c3dfbff 100644
--- a/client/go/vespa/deploy.go
+++ b/client/go/vespa/deploy.go
@@ -6,6 +6,8 @@ package vespa
 
 import (
 	"archive/zip"
+	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -126,7 +128,7 @@ func Prepare(deployment Deployment) (string, error) {
 	if deployment.IsCloud() {
 		return "", fmt.Errorf("%s: prepare is not supported", deployment)
 	}
-	// TODO: This doesn't work. A session ID must be explicitly created and then passed to the prepare call
+	// TODO: Save session id in .vespa
 	// https://docs.vespa.ai/en/cloudconfig/deploy-rest-api-v2.html
 	u, err := url.Parse(deployment.TargetURL + "/application/v2/tenant/default/prepare")
 	if err != nil {
@@ -139,7 +141,7 @@ func Activate(deployment Deployment) (string, error) {
 	if deployment.IsCloud() {
 		return "", fmt.Errorf("%s: activate is not supported", deployment)
 	}
-	// TODO: This doesn't work. A session ID must be explicitly created and then passed to the activate call
+	// TODO: Look up session id in .vespa
 	// https://docs.vespa.ai/en/cloudconfig/deploy-rest-api-v2.html
 	u, err := url.Parse(deployment.TargetURL + "/application/v2/tenant/default/activate")
 	if err != nil {
@@ -210,7 +212,7 @@ func postApplicationPackage(url *url.URL, zipReader io.Reader) error {
 	defer response.Body.Close()
 
 	if response.StatusCode/100 == 4 {
-		return fmt.Errorf("Invalid application package (%s):\n%s", response.Status, util.ReaderToJSON(response.Body))
+		return fmt.Errorf("Invalid application package (%s)\n\n%s", response.Status, extractError(response.Body))
 	} else if response.StatusCode != 200 {
 		return fmt.Errorf("Error from %s at %s (%s):\n%s", strings.ToLower(serviceDescription), request.URL.Host, response.Status, util.ReaderToJSON(response.Body))
 	}
@@ -270,3 +272,20 @@ func zipDir(dir string, destination string) error {
 	}
 	return filepath.Walk(dir, walker)
 }
+
+// Returns the error message in the given JSON, or the entire content if it could not be extracted
+func extractError(reader io.Reader) string {
+	responseData := util.ReaderToBytes(reader)
+	var response map[string]interface{}
+	json.Unmarshal(responseData, &response)
+	if response["error-code"] == "INVALID_APPLICATION_PACKAGE" {
+		return strings.ReplaceAll(response["message"].(string), ": ", ":\n")
+	} else {
+		var prettyJSON bytes.Buffer
+		parseError := json.Indent(&prettyJSON, responseData, "", "    ")
+		if parseError != nil { // Not JSON: Print plainly
+			return string(responseData)
+		}
+		return string(prettyJSON.Bytes())
+	}
+}