diff --git a/client/openapi/doc.go b/client/openapi/doc.go index fc22e067..a610df5f 100644 --- a/client/openapi/doc.go +++ b/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 diff --git a/client/openapi/media_type.go b/client/openapi/media_type.go index 2ede4cc0..6845798b 100644 --- a/client/openapi/media_type.go +++ b/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 diff --git a/client/openapi/object.go b/client/openapi/object.go index 0593d6b2..34ca2337 100644 --- a/client/openapi/object.go +++ b/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}) +} diff --git a/client/openapi/spec.go b/client/openapi/spec.go index 67d2fe62..bb1bcd1c 100644 --- a/client/openapi/spec.go +++ b/client/openapi/spec.go @@ -60,13 +60,26 @@ 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) } @@ -74,14 +87,13 @@ func Parse(r io.Reader) (Spec, error) { } } 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, ¶meters[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 { diff --git a/cmd/internal/testacc/api_docs_test.go b/cmd/internal/testacc/api_docs_test.go new file mode 100644 index 00000000..d34b2f06 --- /dev/null +++ b/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) + } + }) +} diff --git a/cmd/internal/testacc/api_get_test.go b/cmd/internal/testacc/api_get_test.go index cb5b5232..f10e1de7 100644 --- a/cmd/internal/testacc/api_get_test.go +++ b/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 diff --git a/internal/apidocs/apidocs.go b/internal/apidocs/apidocs.go index 45bd30ba..938ccf30 100644 --- a/internal/apidocs/apidocs.go +++ b/internal/apidocs/apidocs.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "os" + "strings" "github.com/meshcloud/meshstack-cli/client/openapi" "github.com/meshcloud/meshstack-cli/client/types/xurl" @@ -23,7 +24,8 @@ var ( devUrl = xurl.MustParsef("https://docs.dev.meshcloud.io/api/meshstack-openapi-docs.json") ) -// urlEnv names the document of the fake meshStack that the tests and the demo serve. It is no +// urlEnv names the document of the fake meshStack that the tests and the demo serve, or as a file:// +// URL the one the acceptance suite generates from the meshStack it runs against. It is no // setting.Setting, and no help names it, so that nobody else comes to rely on it. const urlEnv = "MESHSTACK_API_DOCS_URL" @@ -44,6 +46,17 @@ var ErrNoConfigDir = errors.New("no writable config directory to keep the API do // download can finish. func Load(ctx context.Context, opts Options) (spec openapi.Spec, wait func(), err error) { wait = func() {} + customEnv := os.Getenv(urlEnv) + if path, isFile := strings.CutPrefix(customEnv, "file://"); isFile { + document, readErr := os.ReadFile(path) //nolint:gosec // G304: the path is the one the acceptance suite sets + if readErr != nil { + return spec, wait, fmt.Errorf("%s: %w", urlEnv, readErr) + } + if spec, err = openapi.Parse(bytes.NewReader(document)); err != nil { + return spec, wait, fmt.Errorf("%s: %w", path, err) + } + return spec, wait, nil + } dir, err := opts.ResolveSetting(ctx, config.DirectorySetting) if err != nil { return spec, wait, err @@ -57,7 +70,7 @@ func Load(ctx context.Context, opts Options) (spec openapi.Spec, wait func(), er if cliVersion, parseErr := version.Parse(opts.Version); opts.Dev || parseErr != nil || !cliVersion.IsRelease() { url, path, conditionalGet = devUrl, dir.Join("api-docs-dev.json"), true } - if customEnv := os.Getenv(urlEnv); customEnv != "" { + if customEnv != "" { var custom xurl.URL if err = custom.UnmarshalText([]byte(customEnv)); err != nil { return spec, wait, fmt.Errorf("%s: %w", urlEnv, err)