fix(client): read the OpenAPI document of openapi-spec-for-spring 1.0.2

client/openapi/doc.go

@@ -23,6 +23,9 @@ type Parameter struct {
 	Required    bool
 	Type        string
 	Description string
+	// PathKey names the metadata field that fills a path parameter, such as metadata.uuid, or a
+	// template of several in braces, such as {metadata.ownedByWorkspace}.{metadata.name}.
+	PathKey string
 }
 
 type Response struct {
@@ -76,6 +79,7 @@ func (s Spec) Doc(operation Operation) (OperationDoc, error) {
 			Required    bool   `json:"required"`
 			Description string `json:"description"`
 			Schema      schema `json:"schema"`
+			PathKey     string `json:"x-meshstack-path-key"`
 		} `json:"parameters"`
 		RequestBody struct {
 			Content object `json:"content"`
@@ -93,6 +97,7 @@ func (s Spec) Doc(operation Operation) (OperationDoc, error) {
 			Required:    parameter.Required,
 			Type:        parameter.Schema.typeName(nil),
 			Description: parameter.Description,
+			PathKey:     parameter.PathKey,
 		})
 	}
 	var err error

client/openapi/media_type.go

@@ -14,7 +14,7 @@ import (
 
 // Only a hal+json type carries a meshObject: application/vnd.meshcloud.api.meshobjects.v1+json is
 // a collection of them.
-var versionedMediaTypeRe = regexp.MustCompile(`^application/vnd\.meshcloud\.api\.([a-z]+)\.(v\d+(?:-preview)?)(\.hal)?\+json$`)
+var versionedMediaTypeRe = regexp.MustCompile(`^application/vnd\.meshcloud\.api\.(?:([a-z]+)\.)?(v\d+(?:-preview)?)(\.hal)?\+json$`)
 
 type MediaType struct {
 	Name string
@@ -32,7 +32,7 @@ func newMediaType(name, operationId string) MediaType {
 		return MediaType{Name: name}
 	}
 	mediaType := MediaType{Name: name, ApiVersion: MustParseApiVersion(match[2])}
-	if kind, isMeshObject := match[1], match[3] != ""; isMeshObject {
+	if kind, isMeshObject := match[1], match[1] != "" && match[3] != ""; isMeshObject {
 		mediaType.Kind = kind
 		if prefix := operationId[:min(len(operationId), len(kind))]; strings.EqualFold(prefix, kind) {
 			mediaType.Kind = prefix

client/openapi/object.go

@@ -3,6 +3,7 @@ package openapi
 import (
 	"encoding/json/jsontext"
 	"fmt"
+	"slices"
 )
 
 // object keeps the order of its members, which a map would lose, so that a part of the document
@@ -63,3 +64,12 @@ func (o object) get(name string) (jsontext.Value, bool) {
 	}
 	return nil, false
 }
+
+// set replaces the value of the member, or appends one.
+func (o object) set(name string, value jsontext.Value) object {
+	if i := slices.IndexFunc(o, func(m member) bool { return m.name == name }); i >= 0 {
+		o[i].value = value
+		return o
+	}
+	return append(o, member{name, value})
+}

client/openapi/spec.go

@@ -60,28 +60,40 @@ func Parse(r io.Reader) (Spec, error) {
 		return Spec{}, fmt.Errorf("cannot parse the OpenAPI document: %w", err)
 	}
 	spec := Spec{document: document}
+	for _, kind := range slices.Sorted(maps.Keys(parsed.Components)) {
+		for _, c := range parsed.Components[kind] {
+			spec.components = append(spec.components, component{kind: kind, member: c})
+		}
+	}
 	for _, path := range parsed.Paths {
-		var methods object
-		if err := json.Unmarshal(path.value, &methods); err != nil {
+		var item object
+		if err := json.Unmarshal(path.value, &item); err != nil {
 			return Spec{}, fmt.Errorf("cannot parse path %s: %w", path.name, err)
 		}
-		for _, method := range methods {
-			operation, err := parseOperation(strings.ToUpper(method.name), path.name, method.value)
+		shared, _ := item.get("parameters")
+		for _, method := range item {
+			if !slices.Contains(httpMethods, method.name) {
+				continue
+			}
+			raw, err := spec.inlineOperation(method.value, shared)
+			if err != nil {
+				return Spec{}, fmt.Errorf("cannot parse %s %s: %w", method.name, path.name, err)
+			}
+			operation, err := parseOperation(strings.ToUpper(method.name), path.name, raw)
 			if err != nil {
 				return Spec{}, fmt.Errorf("cannot parse %s %s: %w", method.name, path.name, err)
 			}
 			spec.Operations = append(spec.Operations, operation)
 		}
 	}
 	spec.assignKinds()
-	for _, kind := range slices.Sorted(maps.Keys(parsed.Components)) {
-		for _, c := range parsed.Components[kind] {
-			spec.components = append(spec.components, component{kind: kind, member: c})
-		}
-	}
 	return spec, nil
 }
 
+// httpMethods are the members of a path item that are operations. The others, such as parameters
+// and summary, apply to every operation of the path.
+var httpMethods = []string{"get", "put", "post", "delete", "options", "head", "patch", "trace"}
+
 func parseOperation(method, pathTemplate string, raw jsontext.Value) (Operation, error) {
 	type content struct {
 		Content map[string]jsontext.Value `json:"content"`
@@ -306,6 +318,121 @@ func (s Spec) MarshalJSONTo(enc *jsontext.Encoder) error {
 	return json.MarshalEncode(enc, document)
 }
 
+// inlineOperation replaces the $ref of a parameter, the request body and a response with the
+// component, so that the media types of an operation are its own, and adds the parameters of the
+// path item. Those of the operation win over one of the same name and location, as OpenAPI says.
+func (s Spec) inlineOperation(operation, sharedParameters jsontext.Value) (jsontext.Value, error) {
+	var members object
+	if err := json.Unmarshal(operation, &members); err != nil {
+		return nil, err
+	}
+	ownParameters, _ := members.get("parameters")
+	own, err := s.inlineParameters(ownParameters)
+	if err != nil {
+		return nil, err
+	}
+	shared, err := s.inlineParameters(sharedParameters)
+	if err != nil {
+		return nil, err
+	}
+	var parameters []jsontext.Value
+	for _, parameter := range shared {
+		if !slices.ContainsFunc(own, parameter.sameAs) {
+			parameters = append(parameters, parameter.value)
+		}
+	}
+	for _, parameter := range own {
+		parameters = append(parameters, parameter.value)
+	}
+	if len(parameters) > 0 {
+		merged, marshalErr := json.Marshal(parameters)
+		if marshalErr != nil {
+			return nil, marshalErr
+		}
+		members = members.set("parameters", merged)
+	}
+	if requestBody, ok := members.get("requestBody"); ok {
+		if requestBody, err = s.inline(requestBody); err != nil {
+			return nil, err
+		}
+		members = members.set("requestBody", requestBody)
+	}
+	if value, ok := members.get("responses"); ok {
+		var responses object
+		if err = json.Unmarshal(value, &responses); err != nil {
+			return nil, err
+		}
+		for i, response := range responses {
+			if responses[i].value, err = s.inline(response.value); err != nil {
+				return nil, err
+			}
+		}
+		members = members.set("responses", responses.mustMarshal())
+	}
+	return json.Marshal(members)
+}
+
+type inlinedParameter struct {
+	Name  string `json:"name"`
+	In    string `json:"in"`
+	value jsontext.Value
+}
+
+func (p inlinedParameter) sameAs(other inlinedParameter) bool {
+	return p.Name == other.Name && p.In == other.In
+}
+
+func (s Spec) inlineParameters(value jsontext.Value) ([]inlinedParameter, error) {
+	if value == nil {
+		return nil, nil
+	}
+	var values []jsontext.Value
+	if err := json.Unmarshal(value, &values); err != nil {
+		return nil, err
+	}
+	parameters := make([]inlinedParameter, len(values))
+	for i, value := range values {
+		inlined, err := s.inline(value)
+		if err != nil {
+			return nil, err
+		}
+		if err := json.Unmarshal(inlined, &parameters[i]); err != nil {
+			return nil, fmt.Errorf("parameter %s: %w", value, err)
+		}
+		parameters[i].value = inlined
+	}
+	return parameters, nil
+}
+
+// inline follows the $ref of a value from component to component, and returns the first value that
+// is no $ref, or that refers to no component of the document.
+func (s Spec) inline(value jsontext.Value) (jsontext.Value, error) {
+	var followed []string
+	for {
+		var ref struct {
+			Ref string `json:"$ref"`
+		}
+		if value.Kind() != '{' {
+			return value, nil
+		}
+		if err := json.Unmarshal(value, &ref); err != nil {
+			return nil, err
+		}
+		if ref.Ref == "" {
+			return value, nil
+		}
+		if slices.Contains(followed, ref.Ref) {
+			return nil, fmt.Errorf("$ref %s refers to itself through %s", ref.Ref, strings.Join(followed, ", "))
+		}
+		i := slices.IndexFunc(s.components, func(c component) bool { return c.ref() == ref.Ref })
+		if i < 0 {
+			return value, nil
+		}
+		followed = append(followed, ref.Ref)
+		value = s.components[i].value
+	}
+}
+
 var refRe = regexp.MustCompile(`"\$ref"\s*:\s*"([^"]+)"`)
 
 func (s Spec) referencedComponents(value jsontext.Value) []component {

cmd/internal/testacc/api_docs_test.go

@@ -0,0 +1,48 @@
+package testacc
+
+import (
+	gohttp "net/http"
+	"regexp"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/meshcloud/meshstack-cli/client/openapi"
+)
+
+var pathParameterRe = regexp.MustCompile(`\{([^}]+)}`)
+
+func TestAccApiDocs(t *testing.T) {
+	requireLocalStack(t)
+	spec := devApiDocs(t)
+	require.NotEmpty(t, spec.Operations)
+
+	t.Run("every operation documents the parameters of its path", func(t *testing.T) {
+		for _, operation := range spec.Operations {
+			doc, err := spec.Doc(operation)
+			require.NoError(t, err)
+			var documented []string
+			for _, parameter := range doc.Parameters {
+				if parameter.In == "path" {
+					documented = append(documented, parameter.Name)
+				}
+			}
+			for _, match := range pathParameterRe.FindAllStringSubmatch(string(operation.PathTemplate), -1) {
+				assert.Containsf(t, documented, match[1], "%s %s", operation.Method, operation.PathTemplate)
+			}
+		}
+	})
+
+	t.Run("every meshObject operation has an API version, except a delete and an artifact", func(t *testing.T) {
+		for _, operation := range spec.Operations {
+			if !strings.HasPrefix(string(operation.PathTemplate), "/api/meshobjects/") || operation.Method == gohttp.MethodDelete ||
+				slices.ContainsFunc(operation.MediaTypes, func(m openapi.MediaType) bool { return m.Name == "application/octet-stream" }) {
+				continue
+			}
+			assert.Falsef(t, operation.LatestApiVersion().IsZero(), "%s %s", operation.Method, operation.PathTemplate)
+		}
+	})
+}

cmd/internal/testacc/api_get_test.go

@@ -10,6 +10,7 @@ import (
 	gohttp "net/http"
 	"net/url"
 	"regexp"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
@@ -37,23 +38,14 @@ var getExceptions = map[openapi.PathTemplate]getException{
 	"/api/meshobjects/meshapikeys/self": {
 		apiKeyOnly: "it answers 404 to a request that authenticates with no meshApiKey",
 	},
-	"/api/meshobjects/meshbuildingblockruns/{blockRunUuid}/plan-artifact": {
+	"/api/meshobjects/meshbuildingblockruns/{buildingBlockRun}/plan-artifact": {
 		skip: "the API docs say it is in development, and answers 404 in normal operation",
 	},
-	"/api/meshobjects/meshprojects/{fullProjectIdentifier}": {
-		parameters: map[string]string{"fullProjectIdentifier": "{ownedByWorkspace}.{name}"},
-	},
-	"/api/meshobjects/meshworkspaceusergroups/{fullWorkspaceUserGroupIdentifier}": {
-		parameters: map[string]string{"fullWorkspaceUserGroupIdentifier": "{ownedByWorkspace}.{name}"},
-	},
 }
 
 type getException struct {
 	// query fills a query parameter that the list requires, and the API docs do not mark as required.
 	query map[string]objectField
-	// parameters give the value of a path parameter that fill does not find on its own, with each
-	// metadata field in braces.
-	parameters map[string]string
 	// skip sends no GET, for the reason it gives.
 	skip string
 	// apiKeyOnly sends the GET only for the API key login, for the reason it gives.
@@ -186,6 +178,10 @@ func (o *firstObjects) get(ctx context.Context, r getRequest, operation openapi.
 // fill replaces each path parameter by a field of the first object's metadata. skip says why there
 // is no request to send.
 func (o *firstObjects) fill(ctx context.Context, operation openapi.Operation, exception getException) (r getRequest, skip string, err error) {
+	doc, err := o.spec.Doc(operation)
+	if err != nil {
+		return r, "", err
+	}
 	segments := strings.Split(string(operation.PathTemplate), "/")
 	for i, segment := range segments {
 		parameter, isParameter := strings.CutPrefix(segment, "{")
@@ -197,7 +193,11 @@ func (o *firstObjects) fill(ctx context.Context, operation openapi.Operation, ex
 		if first.metadata == nil {
 			return r, first.none, nil
 		}
-		value, err := parameterValue(parameter, first.metadata, exception)
+		documented := slices.IndexFunc(doc.Parameters, func(p openapi.Parameter) bool { return p.In == "path" && p.Name == parameter })
+		if documented < 0 {
+			return r, "", fmt.Errorf("the API docs do not document %s", segment)
+		}
+		value, err := parameterValue(doc.Parameters[documented], first.metadata)
 		if err != nil {
 			return r, "", fmt.Errorf("%s of %s: %w", segment, operation.Kind, err)
 		}
@@ -217,37 +217,25 @@ func (o *firstObjects) fill(ctx context.Context, operation openapi.Operation, ex
 	return r, "", nil
 }
 
-var metadataFieldRe = regexp.MustCompile(`\{(\w+)\}`)
+var metadataFieldRe = regexp.MustCompile(`\{metadata\.(\w+)\}`)
 
-// parameterValue takes the field of the metadata whose name the parameter ends with, longest first,
-// so that blockRunUuid takes metadata.uuid. A parameter that ends with identifier or id and with no
-// field takes metadata.name, which identifies most kinds.
-func parameterValue(parameter string, metadata map[string]string, exception getException) (string, error) {
-	if template, ok := exception.parameters[parameter]; ok {
-		var err error
-		value := metadataFieldRe.ReplaceAllStringFunc(template, func(field string) string {
-			value := metadata[strings.Trim(field, "{}")]
-			if value == "" {
-				err = fmt.Errorf("the metadata has no %s: %v", field, metadata)
-			}
-			return value
-		})
-		return value, err
+func parameterValue(parameter openapi.Parameter, metadata map[string]string) (string, error) {
+	template := parameter.PathKey
+	if !strings.Contains(template, "{") {
+		template = "{" + template + "}"
 	}
-	lower := strings.ToLower(parameter)
-	var field string
-	for name := range metadata {
-		if strings.HasSuffix(lower, strings.ToLower(name)) && len(name) > len(field) {
-			field = name
+	var err error
+	value := metadataFieldRe.ReplaceAllStringFunc(template, func(field string) string {
+		value := metadata[metadataFieldRe.FindStringSubmatch(field)[1]]
+		if value == "" {
+			err = fmt.Errorf("the metadata has no %s: %v", field, metadata)
 		}
+		return value
+	})
+	if value == template {
+		return "", fmt.Errorf("x-meshstack-path-key names no metadata field: %q", parameter.PathKey)
 	}
-	if field == "" && (strings.HasSuffix(lower, "identifier") || strings.HasSuffix(lower, "id")) {
-		field = "name"
-	}
-	if metadata[field] == "" {
-		return "", fmt.Errorf("no field of the metadata fills it: %v", metadata)
-	}
-	return metadata[field], nil
+	return value, err
 }
 
 // first lists a kind outside the lock, because the list of one kind can take its query from the