Replace /contents singleton accessors with custom :getContents method…

…s. (#313) This is a small breaking change (sorry about that!) that makes the API more consistent with AIP-121 https://google.aip.dev/121). Because "contents" is a field of API specs and artifacts, we decided that it's better to not treat it as a full AIP-121 resource, which has complicating implications for some intended hosting platforms and related tools.
apigee · Sep 28, 2021 · f979214 · f979214
1 parent 2f1c779
commit f979214
Show file tree

Hide file tree

Showing 17 changed files with 35 additions and 43 deletions.
diff --git a/cmd/registry/cmd/compute/lintstats.go b/cmd/registry/cmd/compute/lintstats.go
@@ -108,7 +108,7 @@ func computeLintStatsSpecs(ctx context.Context,
 		log.Debug(spec.GetName())
 		// get the lint results
 		request := rpc.GetArtifactContentsRequest{
-			Name: spec.Name + "/artifacts/" + lintRelation(linter) + "/contents",
+			Name: spec.Name + "/artifacts/" + lintRelation(linter),
 		}
 		contents, _ := client.GetArtifactContents(ctx, &request)
 		if contents == nil {
@@ -135,7 +135,7 @@ func computeLintStatsSpecs(ctx context.Context,
 		{
 			// Calculate the operation and schema count
 			request := rpc.GetArtifactContentsRequest{
-				Name: spec.Name + "/artifacts/complexity/contents",
+				Name: spec.Name + "/artifacts/complexity",
 			}
 			contents, _ := client.GetArtifactContents(ctx, &request)
 			if contents == nil {
@@ -249,7 +249,7 @@ func aggregateLintStats(ctx context.Context,
 	aggregateStats *rpc.LintStats) {
 	// Calculate the operation and schema count
 	request := rpc.GetArtifactContentsRequest{
-		Name: name + "/artifacts/" + lintStatsRelation(linter) + "/contents",
+		Name: name + "/artifacts/" + lintStatsRelation(linter),
 	}
 	contents, _ := client.GetArtifactContents(ctx, &request)
 	if contents == nil {

diff --git a/cmd/registry/cmd/export/csv.go b/cmd/registry/cmd/export/csv.go
@@ -68,7 +68,7 @@ func csvCommand(ctx context.Context) *cobra.Command {
 						ApiID:        m[2],
 						VersionID:    m[3],
 						SpecID:       m[4],
-						ContentsPath: fmt.Sprintf("$APG_REGISTRY_ADDRESS/%s/contents", spec.GetName()),
+						ContentsPath: fmt.Sprintf("$APG_REGISTRY_ADDRESS/%s", spec.GetName()),
 					})
 				})
 				if err != nil {

diff --git a/cmd/registry/cmd/export/csv_test.go b/cmd/registry/cmd/export/csv_test.go
@@ -99,7 +99,7 @@ func TestExportCSV(t *testing.T) {
 	// Verify
 	var (
 		expectedHeader = []string{"api_id", "version_id", "spec_id", "contents_path"}
-		expectedRow    = []string{apiID, versionID, specID, fmt.Sprintf("$APG_REGISTRY_ADDRESS/%s/contents", spec.GetName())}
+		expectedRow    = []string{apiID, versionID, specID, fmt.Sprintf("$APG_REGISTRY_ADDRESS/%s", spec.GetName())}
 	)
 
 	r := csv.NewReader(out)

diff --git a/cmd/registry/cmd/upload/csv_test.go b/cmd/registry/cmd/upload/csv_test.go
@@ -150,7 +150,7 @@ func TestUploadCSV(t *testing.T) {
 				}
 
 				body, err := client.GetApiSpecContents(ctx, &rpc.GetApiSpecContentsRequest{
-					Name: fmt.Sprintf("%s/contents", spec.GetName()),
+					Name: spec.GetName(),
 				})
 				if err != nil {
 					t.Fatalf("GetApiSpecContents(%q) returned error: %s", spec.GetName(), err)

diff --git a/cmd/registry/core/get.go b/cmd/registry/core/get.go
@@ -16,7 +16,6 @@ package core
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/apigee/registry/gapic"
 	"github.com/apigee/registry/rpc"
@@ -87,7 +86,7 @@ func GetSpec(ctx context.Context,
 	}
 	if getContents {
 		request := &rpc.GetApiSpecContentsRequest{
-			Name: fmt.Sprintf("%s/contents", spec.GetName()),
+			Name: spec.GetName(),
 		}
 		contents, err := client.GetApiSpecContents(ctx, request)
 		if err != nil {
@@ -125,7 +124,7 @@ func GetArtifact(ctx context.Context,
 	}
 	if getContents {
 		request := &rpc.GetArtifactContentsRequest{
-			Name: fmt.Sprintf("%s/contents", artifact.GetName()),
+			Name: artifact.GetName(),
 		}
 		contents, err := client.GetArtifactContents(ctx, request)
 		if err != nil {

diff --git a/cmd/registry/core/list.go b/cmd/registry/core/list.go
@@ -16,7 +16,6 @@ package core
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/apigee/registry/gapic"
 	"github.com/apigee/registry/rpc"
@@ -196,7 +195,7 @@ func ListArtifacts(ctx context.Context,
 		}
 		if getContents {
 			req := &rpc.GetArtifactContentsRequest{
-				Name: fmt.Sprintf("%s/contents", artifact.GetName()),
+				Name: artifact.GetName(),
 			}
 			resp, err := client.GetArtifactContents(ctx, req)
 			if err != nil {

diff --git a/cmd/registry/core/specs.go b/cmd/registry/core/specs.go
@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"compress/gzip"
 	"context"
-	"fmt"
 
 	"github.com/apex/log"
 	"github.com/apigee/registry/connection"
@@ -37,7 +36,7 @@ func ResourceNameOfSpec(segments []string) string {
 }
 
 func GetBytesForSpec(ctx context.Context, client connection.Client, spec *rpc.ApiSpec) ([]byte, error) {
-	request := &rpc.GetApiSpecContentsRequest{Name: fmt.Sprintf("%s/contents", spec.GetName())}
+	request := &rpc.GetApiSpecContentsRequest{Name: spec.GetName()}
 	contents, err := client.GetApiSpecContents(ctx, request)
 	if err != nil {
 		return nil, err

diff --git a/demos/disco.sh b/demos/disco.sh
@@ -75,7 +75,7 @@ apg registry get-api-spec --name projects/disco/locations/global/apis/translate/
 # You might notice that that didn't return the actual spec. That's because the spec contents
 # are accessed through a separate method that (when transcoded to HTTP) allows direct download
 # of spec contents.
-apg registry get-api-spec-contents --name projects/disco/locations/global/apis/translate/versions/v3/specs/discovery.json/contents
+apg registry get-api-spec-contents --name projects/disco/locations/global/apis/translate/versions/v3/specs/discovery.json
 
 # Another way to get the bytes of the spec is to use `registry get` with the `--contents` flag.
 registry get projects/disco/locations/global/apis/translate/versions/v3/specs/discovery.json --contents

diff --git a/demos/openapi.sh b/demos/openapi.sh
@@ -65,10 +65,10 @@ apg registry get-api-spec --name projects/openapi/locations/global/apis/wordnik.
 # You might notice that that didn't return the actual spec. That's because the spec contents
 # are accessed through a separate method that (when transcoded to HTTP) allows direct download
 # of spec contents.
-apg registry get-api-spec-contents --name projects/openapi/locations/global/apis/wordnik.com/versions/4.0/specs/openapi.yaml/contents
+apg registry get-api-spec-contents --name projects/openapi/locations/global/apis/wordnik.com/versions/4.0/specs/openapi.yaml
 
 # If you have jq and the base64 tool installed, you can get the spec contents from the RPC response.
-# apg registry get-api-spec-contents --name projects/openapi/locations/global/apis/wordnik.com/versions/4.0/specs/openapi.yaml/contents --json | jq .data -r | base64 --decode
+# apg registry get-api-spec-contents --name projects/openapi/locations/global/apis/wordnik.com/versions/4.0/specs/openapi.yaml --json | jq .data -r | base64 --decode
 
 # Another way to get the bytes of the spec is to use `registry get` with the `--contents` flag.
 registry get projects/openapi/locations/global/apis/wordnik.com --contents

diff --git a/demos/protos.sh b/demos/protos.sh
@@ -81,7 +81,7 @@ apg registry get-api-spec --name projects/protos/locations/global/apis/google-cl
 # You might notice that that didn't return the actual spec. That's because the spec contents
 # are accessed through a separate method that (when transcoded to HTTP) allows direct download
 # of spec contents.
-apg registry get-api-spec-contents --name projects/protos/locations/global/apis/google-cloud-translate/versions/v3/specs/protos.zip/contents
+apg registry get-api-spec-contents --name projects/protos/locations/global/apis/google-cloud-translate/versions/v3/specs/protos.zip
 
 # An easier way to get the bytes of the spec is to use `registry get` with the `--contents` flag.
 # This writes the bytes to stdout, so you probably want to redirect this to a file, as follows:

diff --git a/google/cloud/apigee/registry/v1/registry_service.proto b/google/cloud/apigee/registry/v1/registry_service.proto
@@ -211,7 +211,7 @@ service Registry {
   //     aip.dev/not-precedent: Responses are arbitrary blobs of data. --)
   rpc GetApiSpecContents(GetApiSpecContentsRequest) returns (google.api.HttpBody) {
     option (google.api.http) = {
-      get: "/v1/{name=projects/*/locations/*/apis/*/versions/*/specs/*/contents}"
+      get: "/v1/{name=projects/*/locations/*/apis/*/versions/*/specs/*}:getContents"
     };
     option (google.api.method_signature) = "name";
   }
@@ -316,15 +316,15 @@ service Registry {
   //     aip.dev/not-precedent: Responses are arbitrary blobs of data. --)
   rpc GetArtifactContents(GetArtifactContentsRequest) returns (google.api.HttpBody) {
     option (google.api.http) = {
-      get: "/v1/{name=projects/*/locations/*/artifacts/*/contents}"
+      get: "/v1/{name=projects/*/locations/*/artifacts/*}:getContents"
       additional_bindings: { 
-        get: "/v1/{name=projects/*/locations/*/apis/*/artifacts/*/contents}" 
+        get: "/v1/{name=projects/*/locations/*/apis/*/artifacts/*}:getContents" 
       }
       additional_bindings: {
-        get: "/v1/{name=projects/*/locations/*/apis/*/versions/*/artifacts/*/contents}"
+        get: "/v1/{name=projects/*/locations/*/apis/*/versions/*/artifacts/*}:getContents"
       }
       additional_bindings: {
-        get: "/v1/{name=projects/*/locations/*/apis/*/versions/*/specs/*/artifacts/*/contents}"
+        get: "/v1/{name=projects/*/locations/*/apis/*/versions/*/specs/*/artifacts/*}:getContents"
       }
     };
     option (google.api.method_signature) = "name";
@@ -725,7 +725,7 @@ message GetApiSpecRequest {
 // Request message for GetApiSpecContents.
 message GetApiSpecContentsRequest {
   // The name of the spec to retrieve.
-  // Format: projects/*/locations/*/apis/*/versions/*/specs/*/contents
+  // Format: projects/*/locations/*/apis/*/versions/*/specs/*
   string name = 1 [
     (google.api.field_behavior) = REQUIRED,
     (google.api.resource_reference) = {
@@ -919,7 +919,7 @@ message GetArtifactRequest {
 // Request message for GetArtifactContents.
 message GetArtifactContentsRequest {
   // The name of the artifact to retrieve.
-  // Format: {parent}/artifacts/*/contents
+  // Format: {parent}/artifacts/*
   string name = 1 [
     (google.api.field_behavior) = REQUIRED,
     (google.api.resource_reference) = {

diff --git a/openapi.yaml b/openapi.yaml
@@ -718,7 +718,7 @@ paths:
                         application/json:
                             schema:
                                 $ref: '#/components/schemas/ApiSpec'
-    /v1/projects/{project}/locations/{location}/apis/{api}/versions/{version}/specs/{spec}/contents:
+    /v1/projects/{project}/locations/{location}/apis/{api}/versions/{version}/specs/{spec}:getContents:
         get:
             summary: GetApiSpecContents returns the contents of a specified spec.
             operationId: Registry_GetApiSpecContents
@@ -1109,7 +1109,7 @@ paths:
                 "200":
                     description: OK
                     content: {}
-    /v1/projects/{project}/locations/{location}/artifacts/{artifact}/contents:
+    /v1/projects/{project}/locations/{location}/artifacts/{artifact}:getContents:
         get:
             summary: GetArtifactContents returns the contents of a specified artifact.
             operationId: Registry_GetArtifactContents

diff --git a/server/actions_artifacts.go b/server/actions_artifacts.go
@@ -17,7 +17,6 @@ package server
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/internal/storage"
@@ -166,7 +165,7 @@ func (s *RegistryServer) GetArtifactContents(ctx context.Context, req *rpc.GetAr
 	}
 	defer db.Close()
 
-	name, err := names.ParseArtifact(strings.TrimSuffix(req.GetName(), "/contents"))
+	name, err := names.ParseArtifact(req.GetName())
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}

diff --git a/server/actions_specs.go b/server/actions_specs.go
@@ -182,11 +182,7 @@ func (s *RegistryServer) GetApiSpecContents(ctx context.Context, req *rpc.GetApi
 	}
 	defer db.Close()
 
-	if !strings.HasSuffix(req.GetName(), "/contents") {
-		return nil, status.Errorf(codes.InvalidArgument, "invalid resource name %q, must include /contents suffix", req.GetName())
-	}
-
-	var specName = strings.TrimSuffix(req.GetName(), "/contents")
+	var specName = req.GetName()
 	var spec *models.Spec
 	var revisionName names.SpecRevision
 	if name, err := names.ParseSpec(specName); err == nil {

diff --git a/server/actions_specs_test.go b/server/actions_specs_test.go
@@ -427,26 +427,26 @@ func TestGetApiSpecContents(t *testing.T) {
 			desc: "resource not found",
 			seed: &rpc.ApiSpec{Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec"},
 			req: &rpc.GetApiSpecContentsRequest{
-				Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/doesnt-exist/contents",
+				Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/doesnt-exist",
 			},
 			want: codes.NotFound,
 		},
 		{
 			desc: "case insensitive identifiers",
 			seed: &rpc.ApiSpec{Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec"},
 			req: &rpc.GetApiSpecContentsRequest{
-				Name: "projects/My-project/locations/global/apis/My-api/versions/V1/specs/My-Spec/contents",
+				Name: "projects/My-project/locations/global/apis/My-api/versions/V1/specs/My-Spec",
 			},
 			want: codes.OK,
 		},
 		{
-			desc: "missing contents suffix in resource name",
+			desc: "inappropriate contents suffix in resource name",
 			seed: &rpc.ApiSpec{
 				Name:     "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec",
 				Contents: []byte{},
 			},
 			req: &rpc.GetApiSpecContentsRequest{
-				Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec",
+				Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec/contents",
 			},
 			want: codes.InvalidArgument,
 		},
@@ -458,7 +458,7 @@ func TestGetApiSpecContents(t *testing.T) {
 				Contents: []byte{},
 			},
 			req: &rpc.GetApiSpecContentsRequest{
-				Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec/contents",
+				Name: "projects/my-project/locations/global/apis/my-api/versions/v1/specs/my-spec",
 			},
 			want: codes.FailedPrecondition,
 		},

diff --git a/tests/crud/crud_test.go b/tests/crud/crud_test.go
@@ -200,7 +200,7 @@ func TestCRUD(t *testing.T) {
 	// Check the contents of the created spec.
 	{
 		req := &rpc.GetApiSpecContentsRequest{
-			Name: "projects/test/locations/global/apis/sample/versions/1.0.0/specs/openapi.yaml/contents",
+			Name: "projects/test/locations/global/apis/sample/versions/1.0.0/specs/openapi.yaml",
 		}
 		response, err := registryClient.GetApiSpecContents(ctx, req)
 		check(t, "error getting spec contents %s", err)
@@ -217,7 +217,7 @@ func TestCRUD(t *testing.T) {
 	// Check the contents of the created revision.
 	{
 		req := &rpc.GetApiSpecContentsRequest{
-			Name: "projects/test/locations/global/apis/sample/versions/1.0.0/specs/openapi.yaml@" + revision + "/contents",
+			Name: "projects/test/locations/global/apis/sample/versions/1.0.0/specs/openapi.yaml@" + revision,
 		}
 		response, err := registryClient.GetApiSpecContents(ctx, req)
 		check(t, "error getting spec contents %s", err)
@@ -244,7 +244,7 @@ func TestCRUD(t *testing.T) {
 	// Check the contents of the tagged revision.
 	{
 		req := &rpc.GetApiSpecContentsRequest{
-			Name: "projects/test/locations/global/apis/sample/versions/1.0.0/specs/openapi.yaml@" + revisionTag + "/contents",
+			Name: "projects/test/locations/global/apis/sample/versions/1.0.0/specs/openapi.yaml@" + revisionTag,
 		}
 		response, err := registryClient.GetApiSpecContents(ctx, req)
 		check(t, "error getting spec contents %s", err)
@@ -318,7 +318,7 @@ func testArtifacts(ctx context.Context, registryClient connection.Client, t *tes
 	// Check the artifact contents.
 	{
 		req := &rpc.GetArtifactContentsRequest{
-			Name: fmt.Sprintf("%s/artifacts/sample/contents", parent),
+			Name: fmt.Sprintf("%s/artifacts/sample", parent),
 		}
 		resp, err := registryClient.GetArtifactContents(ctx, req)
 		check(t, "error getting artifact contents %s", err)

diff --git a/tests/demo/walkthrough.sh b/tests/demo/walkthrough.sh
@@ -66,7 +66,7 @@ apg registry get-api-spec \
 echo
 echo Get the contents of the API spec.
 apg registry get-api-spec-contents \
-    --name projects/demo/apis/petstore/versions/1.0.0/specs/openapi.yaml/contents \
+    --name projects/demo/apis/petstore/versions/1.0.0/specs/openapi.yaml \
     --json | \
     jq '.data' -r | \
     registry-decode-spec