Repository: apigee/registry
Pull request #615 (2022-06-17 20:35:03)

Add new low-level db.create() method that properly returns "AlreadyExists"
This adds a new `create()` method to the `storage` package that recognizes error codes that are returned when attempts are made to create entries that already exist, i.e. they have the same primary key value as an existing entry. Unfortunately this requires checking database-specific response codes, as gorm does not currently include this in its list of [cross-database error codes](https://github.com/go-gorm/gorm/blob/master/errors.go).

With this change, we can rewrite the create actions to remove the pre-save "get" checks and we can rewrite update actions to speculatively create entities (when "allow missing" is set) and continue with updates when these speculative creates fail because entities already exist. This removes multiple race conditions and makes it possible to remove the mutexes in the update methods. _Update: additional commits in this PR make these changes for projects, apis, and versions._

codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/615?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#615](https://codecov.io/gh/apigee/registry/pull/615?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (3af0896) into [main](https://codecov.io/gh/apigee/registry/commit/0961ea50d347a88827a5f0a197cb300189791916?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (0961ea5) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.

> :exclamation: Current head 3af0896 differs from pull request most recent head f8a867d. Consider uploading reports for the commit f8a867d to get more accurate results

```diff
@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
+ Coverage   52.30%   52.32%   +0.02%     
==========================================
  Files          71       71              
  Lines        6976     6979       +3     
==========================================
+ Hits         3649     3652       +3     
  Misses       2922     2922              
  Partials      405      405              
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/615?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [cmd/registry/cmd/upload/styleguide.go](https://codecov.io/gh/apigee/registry/pull/615/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NtZC91cGxvYWQvc3R5bGVndWlkZS5nbw==) | `57.14% <0.00%> (-1.48%)` | :arrow_down: |
| [cmd/registry/conformance/conformance-task.go](https://codecov.io/gh/apigee/registry/pull/615/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NvbmZvcm1hbmNlL2NvbmZvcm1hbmNlLXRhc2suZ28=) | `45.61% <0.00%> (-0.78%)` | :arrow_down: |
| [cmd/registry/cmd/compute/conformance.go](https://codecov.io/gh/apigee/registry/pull/615/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NtZC9jb21wdXRlL2NvbmZvcm1hbmNlLmdv) | `58.20% <0.00%> (ø)` | |
| [cmd/registry/scoring/extensions/extensions.go](https://codecov.io/gh/apigee/registry/pull/615/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3JpbmcvZXh0ZW5zaW9ucy9leHRlbnNpb25zLmdv) | `100.00% <0.00%> (ø)` | |
| [cmd/registry/scoring/extensions/validators.go](https://codecov.io/gh/apigee/registry/pull/615/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3JpbmcvZXh0ZW5zaW9ucy92YWxpZGF0b3JzLmdv) | `82.00% <0.00%> (ø)` | |
| [cmd/registry/cmd/upload/csv.go](https://codecov.io/gh/apigee/registry/pull/615/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NtZC91cGxvYWQvY3N2Lmdv) | `67.90% <0.00%> (+2.46%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apigee/registry/pull/615?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apigee/registry/pull/615?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee). Last update [0961ea5...f8a867d](https://codecov.io/gh/apigee/registry/pull/615?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee).


Patch:
diff --git a/server/registry/actions_apis.go b/server/registry/actions_apis.go
index 32b3e67..46829e7 100644
--- a/server/registry/actions_apis.go
+++ b/server/registry/actions_apis.go
@@ -16,10 +16,7 @@ package registry
 
 import (
 	"context"
-	"sync"
-	"time"
 
-	"github.com/apigee/registry/log"
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/internal/storage"
 	"github.com/apigee/registry/server/registry/internal/storage/models"
@@ -50,12 +47,6 @@ func (s *RegistryServer) createApi(ctx context.Context, name names.Api, body *rp
 	}
 	defer db.Close()
 
-	if _, err := db.GetApi(ctx, name); err == nil {
-		return nil, status.Errorf(codes.AlreadyExists, "API %q already exists", name)
-	} else if !isNotFound(err) {
-		return nil, err
-	}
-
 	if err := name.Validate(); err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
@@ -70,7 +61,7 @@ func (s *RegistryServer) createApi(ctx context.Context, name names.Api, body *rp
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	if err := db.SaveApi(ctx, api); err != nil {
+	if err := db.CreateApi(ctx, api); err != nil {
 		return nil, err
 	}
 
@@ -180,16 +171,8 @@ func (s *RegistryServer) ListApis(ctx context.Context, req *rpc.ListApisRequest)
 	return response, nil
 }
 
-var updateApiMutex sync.Mutex
-
 // UpdateApi handles the corresponding API request.
 func (s *RegistryServer) UpdateApi(ctx context.Context, req *rpc.UpdateApiRequest) (*rpc.Api, error) {
-	db, err := s.getStorageClient(ctx)
-	if err != nil {
-		return nil, status.Error(codes.Unavailable, err.Error())
-	}
-	defer db.Close()
-
 	if req.GetApi() == nil {
 		return nil, status.Errorf(codes.InvalidArgument, "invalid api %v: body must be provided", req.GetApi())
 	} else if err := models.ValidateMask(req.GetApi(), req.GetUpdateMask()); err != nil {
@@ -202,29 +185,29 @@ func (s *RegistryServer) UpdateApi(ctx context.Context, req *rpc.UpdateApiReques
 	}
 
 	if req.GetAllowMissing() {
-		before := time.Now()
-		// Prevent a race condition that can occur when two updates are made
-		// to the same non-existent resource. The db.Get...() call returns
-		// NotFound for both updates, and after one creates the resource,
-		// the other creation fails. The lock() prevents this by serializing
-		// the get and create operations. Future updates could improve this
-		// with improvements closer to the database level.
-		updateApiMutex.Lock()
-		defer updateApiMutex.Unlock()
-		log.Debugf(ctx, "Acquired lock after blocking for %v", time.Since(before))
+		// Speculatively create the api. If it already exists, fall through to the update below.
+		api, err := s.createApi(ctx, name, req.GetApi())
+		if err == nil {
+			return api, nil
+		} else if status.Code(err) != codes.AlreadyExists {
+			return nil, err
+		}
+	}
+
+	db, err := s.getStorageClient(ctx)
+	if err != nil {
+		return nil, status.Error(codes.Unavailable, err.Error())
 	}
+	defer db.Close()
 
+	// TODO: the Get/Update/Save calls below should be a transaction.
 	api, err := db.GetApi(ctx, name)
-	if req.GetAllowMissing() && isNotFound(err) {
-		return s.createApi(ctx, name, req.GetApi())
-	} else if err != nil {
+	if err != nil {
 		return nil, err
 	}
-
 	if err := api.Update(req.GetApi(), models.ExpandMask(req.GetApi(), req.GetUpdateMask())); err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
-
 	if err := db.SaveApi(ctx, api); err != nil {
 		return nil, err
 	}
diff --git a/server/registry/actions_projects.go b/server/registry/actions_projects.go
index bd9ea70..7fd7e00 100644
--- a/server/registry/actions_projects.go
+++ b/server/registry/actions_projects.go
@@ -16,10 +16,7 @@ package registry
 
 import (
 	"context"
-	"sync"
-	"time"
 
-	"github.com/apigee/registry/log"
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/internal/storage"
 	"github.com/apigee/registry/server/registry/internal/storage/models"
@@ -45,18 +42,12 @@ func (s *RegistryServer) createProject(ctx context.Context, name names.Project,
 	}
 	defer db.Close()
 
-	if _, err := db.GetProject(ctx, name); err == nil {
-		return nil, status.Errorf(codes.AlreadyExists, "project %q already exists", name)
-	} else if !isNotFound(err) {
-		return nil, err
-	}
-
 	if err := name.Validate(); err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	project := models.NewProject(name, body)
-	if err := db.SaveProject(ctx, project); err != nil {
+	if err := db.CreateProject(ctx, project); err != nil {
 		return nil, err
 	}
 
@@ -148,16 +139,8 @@ func (s *RegistryServer) ListProjects(ctx context.Context, req *rpc.ListProjects
 	return response, nil
 }
 
-var updateProjectMutex sync.Mutex
-
 // UpdateProject handles the corresponding API request.
 func (s *RegistryServer) UpdateProject(ctx context.Context, req *rpc.UpdateProjectRequest) (*rpc.Project, error) {
-	db, err := s.getStorageClient(ctx)
-	if err != nil {
-		return nil, status.Error(codes.Unavailable, err.Error())
-	}
-	defer db.Close()
-
 	if req.GetProject() == nil {
 		return nil, status.Errorf(codes.InvalidArgument, "invalid project %+v: body must be provided", req.GetProject())
 	} else if err := models.ValidateMask(req.GetProject(), req.GetUpdateMask()); err != nil {
@@ -170,25 +153,26 @@ func (s *RegistryServer) UpdateProject(ctx context.Context, req *rpc.UpdateProje
 	}
 
 	if req.GetAllowMissing() {
-		before := time.Now()
-		// Prevent a race condition that can occur when two updates are made
-		// to the same non-existent resource. The db.Get...() call returns
-		// NotFound for both updates, and after one creates the resource,
-		// the other creation fails. The lock() prevents this by serializing
-		// the get and create operations. Future updates could improve this
-		// with improvements closer to the database level.
-		updateProjectMutex.Lock()
-		defer updateProjectMutex.Unlock()
-		log.Debugf(ctx, "Acquired lock after blocking for %v", time.Since(before))
+		// Speculatively create the project. If it already exists, fall through to the update below.
+		project, err := s.createProject(ctx, name, req.GetProject())
+		if err == nil {
+			return project, nil
+		} else if status.Code(err) != codes.AlreadyExists {
+			return nil, err
+		}
 	}
 
+	db, err := s.getStorageClient(ctx)
+	if err != nil {
+		return nil, status.Error(codes.Unavailable, err.Error())
+	}
+	defer db.Close()
+
+	// TODO: the Get/Update/Save calls below should be a transaction.
 	project, err := db.GetProject(ctx, name)
-	if req.GetAllowMissing() && isNotFound(err) {
-		return s.createProject(ctx, name, req.GetProject())
-	} else if err != nil {
+	if err != nil {
 		return nil, err
 	}
-
 	project.Update(req.GetProject(), models.ExpandMask(req.GetProject(), req.GetUpdateMask()))
 	if err := db.SaveProject(ctx, project); err != nil {
 		return nil, err
diff --git a/server/registry/actions_versions.go b/server/registry/actions_versions.go
index cafdb40..c27fef7 100644
--- a/server/registry/actions_versions.go
+++ b/server/registry/actions_versions.go
@@ -16,10 +16,7 @@ package registry
 
 import (
 	"context"
-	"sync"
-	"time"
 
-	"github.com/apigee/registry/log"
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/internal/storage"
 	"github.com/apigee/registry/server/registry/internal/storage/models"
@@ -50,12 +47,6 @@ func (s *RegistryServer) createApiVersion(ctx context.Context, name names.Versio
 	}
 	defer db.Close()
 
-	if _, err := db.GetVersion(ctx, name); err == nil {
-		return nil, status.Errorf(codes.AlreadyExists, "API version %q already exists", name)
-	} else if !isNotFound(err) {
-		return nil, err
-	}
-
 	if err := name.Validate(); err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
@@ -70,7 +61,7 @@ func (s *RegistryServer) createApiVersion(ctx context.Context, name names.Versio
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	if err := db.SaveVersion(ctx, version); err != nil {
+	if err := db.CreateVersion(ctx, version); err != nil {
 		return nil, err
 	}
 
@@ -180,16 +171,8 @@ func (s *RegistryServer) ListApiVersions(ctx context.Context, req *rpc.ListApiVe
 	return response, nil
 }
 
-var updateVersionMutex sync.Mutex
-
 // UpdateApiVersion handles the corresponding API request.
 func (s *RegistryServer) UpdateApiVersion(ctx context.Context, req *rpc.UpdateApiVersionRequest) (*rpc.ApiVersion, error) {
-	db, err := s.getStorageClient(ctx)
-	if err != nil {
-		return nil, status.Error(codes.Unavailable, err.Error())
-	}
-	defer db.Close()
-
 	if req.GetApiVersion() == nil {
 		return nil, status.Errorf(codes.InvalidArgument, "invalid api_version %+v: body must be provided", req.GetApiVersion())
 	} else if err := models.ValidateMask(req.GetApiVersion(), req.GetUpdateMask()); err != nil {
@@ -202,29 +185,29 @@ func (s *RegistryServer) UpdateApiVersion(ctx context.Context, req *rpc.UpdateAp
 	}
 
 	if req.GetAllowMissing() {
-		before := time.Now()
-		// Prevent a race condition that can occur when two updates are made
-		// to the same non-existent resource. The db.Get...() call returns
-		// NotFound for both updates, and after one creates the resource,
-		// the other creation fails. The lock() prevents this by serializing
-		// the get and create operations. Future updates could improve this
-		// with improvements closer to the database level.
-		updateVersionMutex.Lock()
-		defer updateVersionMutex.Unlock()
-		log.Debugf(ctx, "Acquired lock after blocking for %v", time.Since(before))
+		// Speculatively create the version. If it already exists, fall through to the update below.
+		version, err := s.createApiVersion(ctx, name, req.GetApiVersion())
+		if err == nil {
+			return version, nil
+		} else if status.Code(err) != codes.AlreadyExists {
+			return nil, err
+		}
+	}
+
+	db, err := s.getStorageClient(ctx)
+	if err != nil {
+		return nil, status.Error(codes.Unavailable, err.Error())
 	}
+	defer db.Close()
 
+	// TODO: the Get/Update/Save calls below should be a transaction.
 	version, err := db.GetVersion(ctx, name)
-	if req.GetAllowMissing() && isNotFound(err) {
-		return s.createApiVersion(ctx, name, req.GetApiVersion())
-	} else if err != nil {
+	if err != nil {
 		return nil, err
 	}
-
 	if err := version.Update(req.GetApiVersion(), models.ExpandMask(req.GetApiVersion(), req.GetUpdateMask())); err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
-
 	if err := db.SaveVersion(ctx, version); err != nil {
 		return nil, err
 	}
diff --git a/server/registry/internal/storage/create.go b/server/registry/internal/storage/create.go
new file mode 100644
index 0000000..009417c
--- /dev/null
+++ b/server/registry/internal/storage/create.go
@@ -0,0 +1,43 @@
+// Copyright 2022 Google LLC. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package storage
+
+import (
+	"context"
+
+	"github.com/apigee/registry/server/registry/internal/storage/models"
+)
+
+func (c *Client) CreateProject(ctx context.Context, v *models.Project) error {
+	v.Key = v.Name()
+	return c.create(ctx, v)
+}
+
+func (c *Client) CreateApi(ctx context.Context, v *models.Api) error {
+	v.Key = v.Name()
+	return c.create(ctx, v)
+}
+
+func (c *Client) CreateVersion(ctx context.Context, v *models.Version) error {
+	v.Key = v.Name()
+	return c.create(ctx, v)
+}
+
+func (c *Client) create(ctx context.Context, v interface{}) error {
+	if err := c.db.Create(v).Error; err != nil {
+		return grpcErrorForDBError(err)
+	}
+	return nil
+}
diff --git a/server/registry/internal/storage/errors.go b/server/registry/internal/storage/errors.go
index 94e514f..1eac5d5 100644
--- a/server/registry/internal/storage/errors.go
+++ b/server/registry/internal/storage/errors.go
@@ -15,18 +15,14 @@
 package storage
 
 import (
-	"github.com/lib/pq"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
 // grpcErrorForDBError converts recognized database error codes to grpc error codes.
 func grpcErrorForDBError(err error) error {
-	switch v := err.(type) {
-	case *pq.Error: // Postgres codes are from https://www.postgresql.org/docs/current/errcodes-appendix.html
-		if v.Code == "23505" { // unique_violation
-			return status.Error(codes.AlreadyExists, err.Error())
-		}
+	if alreadyExists(err) {
+		return status.Error(codes.AlreadyExists, err.Error())
 	}
 	// All unrecognized codes fall through to become "Internal" errors.
 	return status.Error(codes.Internal, err.Error())
diff --git a/server/registry/internal/storage/errors_cgo_disabled.go b/server/registry/internal/storage/errors_cgo_disabled.go
new file mode 100644
index 0000000..13e221d
--- /dev/null
+++ b/server/registry/internal/storage/errors_cgo_disabled.go
@@ -0,0 +1,31 @@
+// Copyright 2022 Google LLC. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build !cgo
+
+package storage
+
+import (
+	"github.com/lib/pq"
+)
+
+func alreadyExists(err error) bool {
+	switch v := err.(type) {
+	case *pq.Error:
+		if v.Code.Name() == "unique_violation" {
+			return true
+		}
+	}
+	return false
+}
diff --git a/server/registry/internal/storage/errors_cgo_enabled.go b/server/registry/internal/storage/errors_cgo_enabled.go
new file mode 100644
index 0000000..eecc53e
--- /dev/null
+++ b/server/registry/internal/storage/errors_cgo_enabled.go
@@ -0,0 +1,36 @@
+// Copyright 2022 Google LLC. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build cgo
+
+package storage
+
+import (
+	"github.com/lib/pq"
+	"github.com/mattn/go-sqlite3"
+)
+
+func alreadyExists(err error) bool {
+	switch v := err.(type) {
+	case *pq.Error:
+		if v.Code.Name() == "unique_violation" {
+			return true
+		}
+	case sqlite3.Error:
+		if v.Code == sqlite3.ErrNo(sqlite3.ErrConstraint) && v.ExtendedCode == sqlite3.ErrConstraintPrimaryKey {
+			return true
+		}
+	}
+	return false
+}
diff --git a/server/registry/internal/storage/logger.go b/server/registry/internal/storage/logger.go
index 078e206..5f18c93 100644
--- a/server/registry/internal/storage/logger.go
+++ b/server/registry/internal/storage/logger.go
@@ -58,7 +58,7 @@ func (l gormLogger) Trace(ctx context.Context, begin time.Time, fc func() (sql s
 		"duration": time.Since(begin),
 	})
 
-	if err != nil && err != gorm.ErrRecordNotFound {
+	if err != nil && err != gorm.ErrRecordNotFound && !alreadyExists(err) {
 		logger.WithError(err).Error("Failed database operation.")
 	} else if time.Since(begin) > l.SlowThreshold {
 		logger.Warn("Slow database operation.")


Test patch:
diff --git a/server/registry/actions_apis_test.go b/server/registry/actions_apis_test.go
index c59ada0..635dada 100644
--- a/server/registry/actions_apis_test.go
+++ b/server/registry/actions_apis_test.go
@@ -237,47 +237,32 @@ func TestCreateApiResponseCodes(t *testing.T) {
 }
 
 func TestCreateApiDuplicates(t *testing.T) {
-	tests := []struct {
+	test := struct {
 		desc string
 		seed *rpc.Api
 		req  *rpc.CreateApiRequest
 		want codes.Code
 	}{
-		{
-			desc: "case sensitive",
-			seed: &rpc.Api{Name: "projects/my-project/locations/global/apis/my-api"},
-			req: &rpc.CreateApiRequest{
-				Parent: "projects/my-project/locations/global",
-				ApiId:  "my-api",
-				Api:    &rpc.Api{},
-			},
-			want: codes.AlreadyExists,
-		},
-		{
-			desc: "case insensitive",
-			seed: &rpc.Api{Name: "projects/my-project/locations/global/apis/my-api"},
-			req: &rpc.CreateApiRequest{
-				Parent: "projects/my-project/locations/global",
-				ApiId:  "My-Api",
-				Api:    &rpc.Api{},
-			},
-			want: codes.AlreadyExists,
+		desc: "case sensitive",
+		seed: &rpc.Api{Name: "projects/my-project/locations/global/apis/my-api"},
+		req: &rpc.CreateApiRequest{
+			Parent: "projects/my-project/locations/global",
+			ApiId:  "my-api",
+			Api:    &rpc.Api{},
 		},
+		want: codes.AlreadyExists,
 	}
+	t.Run(test.desc, func(t *testing.T) {
+		ctx := context.Background()
+		server := defaultTestServer(t)
+		if err := seeder.SeedApis(ctx, server, test.seed); err != nil {
+			t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+		}
 
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			ctx := context.Background()
-			server := defaultTestServer(t)
-			if err := seeder.SeedApis(ctx, server, test.seed); err != nil {
-				t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
-			}
-
-			if _, err := server.CreateApi(ctx, test.req); status.Code(err) != test.want {
-				t.Errorf("CreateApi(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
-			}
-		})
-	}
+		if _, err := server.CreateApi(ctx, test.req); status.Code(err) != test.want {
+			t.Errorf("CreateApi(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
+		}
+	})
 }
 
 func TestGetApi(t *testing.T) {
diff --git a/server/registry/actions_projects_test.go b/server/registry/actions_projects_test.go
index b335016..9ca4967 100644
--- a/server/registry/actions_projects_test.go
+++ b/server/registry/actions_projects_test.go
@@ -181,45 +181,31 @@ func TestCreateProjectResponseCodes(t *testing.T) {
 }
 
 func TestCreateProjectDuplicates(t *testing.T) {
-	tests := []struct {
+	test := struct {
 		desc string
 		seed *rpc.Project
 		req  *rpc.CreateProjectRequest
 		want codes.Code
 	}{
-		{
-			desc: "case sensitive",
-			seed: &rpc.Project{Name: "projects/my-project"},
-			req: &rpc.CreateProjectRequest{
-				ProjectId: "my-project",
-				Project:   &rpc.Project{},
-			},
-			want: codes.AlreadyExists,
-		},
-		{
-			desc: "case insensitive",
-			seed: &rpc.Project{Name: "projects/my-project"},
-			req: &rpc.CreateProjectRequest{
-				ProjectId: "My-Project",
-				Project:   &rpc.Project{},
-			},
-			want: codes.AlreadyExists,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			ctx := context.Background()
-			server := defaultTestServer(t)
-			if err := seeder.SeedProjects(ctx, server, test.seed); err != nil {
-				t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
-			}
+		desc: "case sensitive",
+		seed: &rpc.Project{Name: "projects/my-project"},
+		req: &rpc.CreateProjectRequest{
+			ProjectId: "my-project",
+			Project:   &rpc.Project{},
+		},
+		want: codes.AlreadyExists,
+	}
+	t.Run(test.desc, func(t *testing.T) {
+		ctx := context.Background()
+		server := defaultTestServer(t)
+		if err := seeder.SeedProjects(ctx, server, test.seed); err != nil {
+			t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+		}
 
-			if _, err := server.CreateProject(ctx, test.req); status.Code(err) != test.want {
-				t.Errorf("CreateProject(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
-			}
-		})
-	}
+		if _, err := server.CreateProject(ctx, test.req); status.Code(err) != test.want {
+			t.Errorf("CreateProject(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
+		}
+	})
 }
 
 func TestGetProject(t *testing.T) {
diff --git a/server/registry/actions_versions_test.go b/server/registry/actions_versions_test.go
index bd844f2..b093d3e 100644
--- a/server/registry/actions_versions_test.go
+++ b/server/registry/actions_versions_test.go
@@ -233,47 +233,32 @@ func TestCreateApiVersionResponseCodes(t *testing.T) {
 }
 
 func TestCreateApiVersionDuplicates(t *testing.T) {
-	tests := []struct {
+	test := struct {
 		desc string
 		seed *rpc.ApiVersion
 		req  *rpc.CreateApiVersionRequest
 		want codes.Code
 	}{
-		{
-			desc: "case sensitive",
-			seed: &rpc.ApiVersion{Name: "projects/my-project/locations/global/apis/my-api/versions/v1"},
-			req: &rpc.CreateApiVersionRequest{
-				Parent:       "projects/my-project/locations/global/apis/my-api",
-				ApiVersionId: "v1",
-				ApiVersion:   &rpc.ApiVersion{},
-			},
-			want: codes.AlreadyExists,
-		},
-		{
-			desc: "case insensitive",
-			seed: &rpc.ApiVersion{Name: "projects/my-project/locations/global/apis/my-api/versions/v1"},
-			req: &rpc.CreateApiVersionRequest{
-				Parent:       "projects/my-project/locations/global/apis/my-api",
-				ApiVersionId: "V1",
-				ApiVersion:   &rpc.ApiVersion{},
-			},
-			want: codes.AlreadyExists,
+		desc: "case sensitive",
+		seed: &rpc.ApiVersion{Name: "projects/my-project/locations/global/apis/my-api/versions/v1"},
+		req: &rpc.CreateApiVersionRequest{
+			Parent:       "projects/my-project/locations/global/apis/my-api",
+			ApiVersionId: "v1",
+			ApiVersion:   &rpc.ApiVersion{},
 		},
+		want: codes.AlreadyExists,
 	}
+	t.Run(test.desc, func(t *testing.T) {
+		ctx := context.Background()
+		server := defaultTestServer(t)
+		if err := seeder.SeedVersions(ctx, server, test.seed); err != nil {
+			t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+		}
 
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			ctx := context.Background()
-			server := defaultTestServer(t)
-			if err := seeder.SeedVersions(ctx, server, test.seed); err != nil {
-				t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
-			}
-
-			if _, err := server.CreateApiVersion(ctx, test.req); status.Code(err) != test.want {
-				t.Errorf("CreateApiVersion(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
-			}
-		})
-	}
+		if _, err := server.CreateApiVersion(ctx, test.req); status.Code(err) != test.want {
+			t.Errorf("CreateApiVersion(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
+		}
+	})
 }
 
 func TestGetApiVersion(t *testing.T) {
diff --git a/server/registry/storage_test.go b/server/registry/storage_test.go
new file mode 100644
index 0000000..9199b30
--- /dev/null
+++ b/server/registry/storage_test.go
@@ -0,0 +1,75 @@
+// Copyright 2022 Google LLC. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package registry
+
+import (
+	"context"
+	"testing"
+
+	"github.com/apigee/registry/rpc"
+	"github.com/apigee/registry/server/registry/internal/storage/models"
+	"github.com/apigee/registry/server/registry/names"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+func TestDuplicateCreation(t *testing.T) {
+	ctx := context.Background()
+	server := defaultTestServer(t)
+	db, err := server.getStorageClient(ctx)
+	if err != nil {
+		t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+	}
+	defer db.Close()
+
+	// Create a project.
+	project := models.NewProject(names.Project{ProjectID: "duplicate"}, &rpc.Project{})
+	if err := db.CreateProject(ctx, project); err != nil {
+		t.Errorf("error creating project %s", err)
+	}
+	// Create the project again and verify that it already exists.
+	if err := db.CreateProject(ctx, project); status.Code(err) != codes.AlreadyExists {
+		t.Errorf("error creating duplicate project, code should be AlreadyExists but is instead %s", err)
+	}
+
+	// Note that the following creations don't create parents.
+	// Saving to the database does not require that parents exist.
+
+	// Create an API.
+	api, err := models.NewApi(names.Api{ApiID: "duplicate"}, &rpc.Api{})
+	if err != nil {
+		t.Errorf("error creating api %s", err)
+	}
+	if err := db.CreateApi(ctx, api); err != nil {
+		t.Errorf("error creating api %s", err)
+	}
+	// Create the API again and verify that the response shows that it it already exists.
+	if err := db.CreateApi(ctx, api); status.Code(err) != codes.AlreadyExists {
+		t.Errorf("error creating duplicate api, code should be AlreadyExists but is instead %s", err)
+	}
+
+	// Create a version.
+	version, err := models.NewVersion(names.Version{VersionID: "duplicate"}, &rpc.ApiVersion{})
+	if err != nil {
+		t.Errorf("error creating version %s", err)
+	}
+	if err := db.CreateVersion(ctx, version); err != nil {
+		t.Errorf("error creating version %s", err)
+	}
+	// Create the version again and verify that the response shows that it already exists.
+	if err := db.CreateVersion(ctx, version); status.Code(err) != codes.AlreadyExists {
+		t.Errorf("error creating duplicate version, code should be AlreadyExists but is instead %s", err)
+	}
+}