Repository: apigee/registry (go)

Issue:
Registry server: CreateTime being updated on Update actions
When updating resources, the CreateTime time property is being updated also. I don't know if this is the expected behavior, but this was not happening before. Looking into the source code, I've found that the actions related to update are calling a function models.New(). I verified that this happens for Project, Artifact, Api, and Version.

![image](https://user-images.githubusercontent.com/26742294/197047135-6f72fcae-8f88-45f7-bd4c-3daed4d5ee17.png)
![image](https://user-images.githubusercontent.com/26742294/197047276-35386d49-d666-438e-96c6-0837a860bde1.png)
  

Pull request description:
Enhance UpdateSequence tests to verify create and update timestamps.
This fixes #792 and verifies with tests. Projects, APIs, and versions were already setting timestamps correctly, and artifacts weren't (and are fixed). Although specs and deployments weren't mentioned in #792, this also adds similar tests for them, too.

Patch:
diff --git a/server/registry/actions_artifacts.go b/server/registry/actions_artifacts.go
index aab721a..b192019 100644
--- a/server/registry/actions_artifacts.go
+++ b/server/registry/actions_artifacts.go
@@ -304,6 +304,7 @@ func (s *RegistryServer) ReplaceArtifact(ctx context.Context, req *rpc.ReplaceAr
 		if err != nil {
 			return err
 		}
+		artifact.CreateTime = art.CreateTime // preserve creation time
 		artifact.RevisionID = art.RevisionID // revision is optional in request
 		if err := db.SaveArtifact(ctx, artifact); err != nil {
 			return err


Test patch:
diff --git a/server/registry/actions_apis_test.go b/server/registry/actions_apis_test.go
index 05a132c..905d2bb 100644
--- a/server/registry/actions_apis_test.go
+++ b/server/registry/actions_apis_test.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/test/seeder"
@@ -1142,7 +1143,7 @@ func TestUpdateApiSequence(t *testing.T) {
 				Api: &rpc.Api{
 					Name: "projects/my-project/locations/global/apis/a",
 				},
-				AllowMissing: true,
+				AllowMissing: false,
 			},
 			want: codes.OK,
 		},
@@ -1153,11 +1154,32 @@ func TestUpdateApiSequence(t *testing.T) {
 	if err := seeder.SeedProjects(ctx, server, seed); err != nil {
 		t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
 	}
-	for _, test := range tests {
+	var createTime time.Time
+	var updateTime time.Time
+	// NOTE: in the following sequence of tests, each test depends on its predecessor.
+	// Resources are successively created and updated using the "Update" RPC and the
+	// tests verify that CreateTime/UpdateTime fields are modified appropriately.
+	for i, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			if _, err := server.UpdateApi(ctx, test.req); status.Code(err) != test.want {
+			var result *rpc.Api
+			var err error
+			if result, err = server.UpdateApi(ctx, test.req); status.Code(err) != test.want {
 				t.Errorf("UpdateApi(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
 			}
+			if result != nil {
+				if i == 1 {
+					createTime = result.CreateTime.AsTime()
+					updateTime = result.UpdateTime.AsTime()
+				} else {
+					if !createTime.Equal(result.CreateTime.AsTime()) {
+						t.Errorf("UpdateApi create time changed after update (%v %v)", createTime, result.CreateTime.AsTime())
+					}
+					if !updateTime.Before(result.UpdateTime.AsTime()) {
+						t.Errorf("UpdateApi update time did not increase after update (%v %v)", updateTime, result.UpdateTime.AsTime())
+					}
+					updateTime = result.UpdateTime.AsTime()
+				}
+			}
 		})
 	}
 }
diff --git a/server/registry/actions_artifacts_test.go b/server/registry/actions_artifacts_test.go
index bb53ff6..96b3865 100644
--- a/server/registry/actions_artifacts_test.go
+++ b/server/registry/actions_artifacts_test.go
@@ -1274,6 +1274,68 @@ func TestReplaceArtifactResponseCodes(t *testing.T) {
 	}
 }
 
+func TestReplaceArtifactSequence(t *testing.T) {
+	tests := []struct {
+		desc string
+		req  *rpc.ReplaceArtifactRequest
+		want codes.Code
+	}{
+		{
+			desc: "first replacement",
+			req: &rpc.ReplaceArtifactRequest{
+				Artifact: &rpc.Artifact{
+					Name: "projects/my-project/locations/global/artifacts/a",
+				},
+			},
+			want: codes.OK,
+		},
+		{
+			desc: "second replacement",
+			req: &rpc.ReplaceArtifactRequest{
+				Artifact: &rpc.Artifact{
+					Name: "projects/my-project/locations/global/artifacts/a",
+				},
+			},
+			want: codes.OK,
+		},
+	}
+	ctx := context.Background()
+	server := defaultTestServer(t)
+	seed := &rpc.Artifact{Name: "projects/my-project/locations/global/artifacts/a"}
+	if err := seeder.SeedArtifacts(ctx, server, seed); err != nil {
+		t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+	}
+	a, err := server.GetArtifact(ctx, &rpc.GetArtifactRequest{
+		Name: "projects/my-project/locations/global/artifacts/a",
+	})
+	if err != nil {
+		t.Fatalf("Setup/Seeding: Failed to get seeded artifact: %s", err)
+	}
+	createTime := a.CreateTime.AsTime()
+	updateTime := a.UpdateTime.AsTime()
+	// NOTE: in the following sequence of tests, each test depends on its predecessor.
+	// Resources are successively updated using the "Replace" RPC and the
+	// tests verify that CreateTime/UpdateTime fields are modified appropriately.
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			var result *rpc.Artifact
+			var err error
+			if result, err = server.ReplaceArtifact(ctx, test.req); status.Code(err) != test.want {
+				t.Errorf("UpdateApi(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
+			}
+			if result != nil {
+				if !createTime.Equal(result.CreateTime.AsTime()) {
+					t.Errorf("ReplaceArtifact create time changed after replace (%v %v)", createTime, result.CreateTime.AsTime())
+				}
+				if !updateTime.Before(result.UpdateTime.AsTime()) {
+					t.Errorf("ReplaceArtifact update time did not increase after replace (%v %v)", updateTime, result.UpdateTime.AsTime())
+				}
+				updateTime = result.UpdateTime.AsTime()
+			}
+		})
+	}
+}
+
 func TestDeleteArtifact(t *testing.T) {
 	tests := []struct {
 		desc string
diff --git a/server/registry/actions_deployments_test.go b/server/registry/actions_deployments_test.go
index 5037e1f..febcf9b 100644
--- a/server/registry/actions_deployments_test.go
+++ b/server/registry/actions_deployments_test.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/test/seeder"
@@ -1177,6 +1178,94 @@ func TestUpdateApiDeploymentResponseCodes(t *testing.T) {
 	}
 }
 
+func TestUpdateApiDeploymentSequence(t *testing.T) {
+	tests := []struct {
+		desc string
+		req  *rpc.UpdateApiDeploymentRequest
+		want codes.Code
+	}{
+		{
+			desc: "create using update with allow_missing=false",
+			req: &rpc.UpdateApiDeploymentRequest{
+				ApiDeployment: &rpc.ApiDeployment{
+					Name: "projects/my-project/locations/global/apis/a/deployments/d",
+				},
+				AllowMissing: false,
+			},
+			want: codes.NotFound,
+		},
+		{
+			desc: "create using update with allow_missing=true",
+			req: &rpc.UpdateApiDeploymentRequest{
+				ApiDeployment: &rpc.ApiDeployment{
+					Name: "projects/my-project/locations/global/apis/a/deployments/d",
+				},
+				AllowMissing: true,
+			},
+			want: codes.OK,
+		},
+		{
+			desc: "update existing resource with allow_missing=true",
+			req: &rpc.UpdateApiDeploymentRequest{
+				ApiDeployment: &rpc.ApiDeployment{
+					Name: "projects/my-project/locations/global/apis/a/deployments/d",
+				},
+				AllowMissing: true,
+			},
+			want: codes.OK,
+		},
+		{
+			desc: "update existing resource with allow_missing=false",
+			req: &rpc.UpdateApiDeploymentRequest{
+				ApiDeployment: &rpc.ApiDeployment{
+					Name: "projects/my-project/locations/global/apis/a/deployments/d",
+				},
+				AllowMissing: false,
+			},
+			want: codes.OK,
+		},
+	}
+	ctx := context.Background()
+	server := defaultTestServer(t)
+	seed := &rpc.Api{Name: "projects/my-project/locations/global/apis/a"}
+	if err := seeder.SeedApis(ctx, server, seed); err != nil {
+		t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+	}
+	var createTime time.Time
+	var revisionCreateTime time.Time
+	var revisionUpdateTime time.Time
+	// NOTE: in the following sequence of tests, each test depends on its predecessor.
+	// Resources are successively created and updated using the "Update" RPC and the
+	// tests verify that CreateTime/UpdateTime fields are modified appropriately.
+	for i, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			var result *rpc.ApiDeployment
+			var err error
+			if result, err = server.UpdateApiDeployment(ctx, test.req); status.Code(err) != test.want {
+				t.Errorf("UpdateApiDeployment(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
+			}
+			if result != nil {
+				if i == 1 {
+					createTime = result.CreateTime.AsTime()
+					revisionCreateTime = result.RevisionCreateTime.AsTime()
+					revisionUpdateTime = result.RevisionUpdateTime.AsTime()
+				} else {
+					if !createTime.Equal(result.CreateTime.AsTime()) {
+						t.Errorf("UpdateApiDeployment create time changed after update (%v %v)", createTime, result.CreateTime.AsTime())
+					}
+					if !revisionCreateTime.Equal(result.RevisionCreateTime.AsTime()) {
+						t.Errorf("UpdateApiDeployment revision create time changed after update (%v %v)", revisionCreateTime, result.RevisionCreateTime.AsTime())
+					}
+					if !revisionUpdateTime.Before(result.RevisionUpdateTime.AsTime()) {
+						t.Errorf("UpdateApiDeployment update time did not increase after update (%v %v)", revisionUpdateTime, result.RevisionUpdateTime.AsTime())
+					}
+					revisionUpdateTime = result.RevisionUpdateTime.AsTime()
+				}
+			}
+		})
+	}
+}
+
 func TestDeleteApiDeployment(t *testing.T) {
 	tests := []struct {
 		desc string
diff --git a/server/registry/actions_projects_test.go b/server/registry/actions_projects_test.go
index f450875..49af1f0 100644
--- a/server/registry/actions_projects_test.go
+++ b/server/registry/actions_projects_test.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/test/seeder"
@@ -1036,18 +1037,39 @@ func TestUpdateProjectSequence(t *testing.T) {
 				Project: &rpc.Project{
 					Name: "projects/my-project",
 				},
-				AllowMissing: true,
+				AllowMissing: false,
 			},
 			want: codes.OK,
 		},
 	}
 	ctx := context.Background()
 	server := defaultTestServer(t)
-	for _, test := range tests {
+	var createTime time.Time
+	var updateTime time.Time
+	// NOTE: in the following sequence of tests, each test depends on its predecessor.
+	// Resources are successively created and updated using the "Update" RPC and the
+	// tests verify that CreateTime/UpdateTime fields are modified appropriately
+	for i, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			if _, err := server.UpdateProject(ctx, test.req); status.Code(err) != test.want {
+			var result *rpc.Project
+			var err error
+			if result, err = server.UpdateProject(ctx, test.req); status.Code(err) != test.want {
 				t.Errorf("UpdateProject(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
 			}
+			if result != nil {
+				if i == 1 {
+					createTime = result.CreateTime.AsTime()
+					updateTime = result.UpdateTime.AsTime()
+				} else {
+					if !createTime.Equal(result.CreateTime.AsTime()) {
+						t.Errorf("UpdateProject create time changed after update (%v %v)", createTime, result.CreateTime.AsTime())
+					}
+					if !updateTime.Before(result.UpdateTime.AsTime()) {
+						t.Errorf("UpdateProject update time did not increase after update (%v %v)", updateTime, result.UpdateTime.AsTime())
+					}
+					updateTime = result.UpdateTime.AsTime()
+				}
+			}
 		})
 	}
 }
diff --git a/server/registry/actions_specs_test.go b/server/registry/actions_specs_test.go
index 21722c3..cb52625 100644
--- a/server/registry/actions_specs_test.go
+++ b/server/registry/actions_specs_test.go
@@ -21,6 +21,7 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/test/seeder"
@@ -1484,6 +1485,94 @@ func TestUpdateApiSpecResponseCodes(t *testing.T) {
 	}
 }
 
+func TestUpdateApiSpecSequence(t *testing.T) {
+	tests := []struct {
+		desc string
+		req  *rpc.UpdateApiSpecRequest
+		want codes.Code
+	}{
+		{
+			desc: "create using update with allow_missing=false",
+			req: &rpc.UpdateApiSpecRequest{
+				ApiSpec: &rpc.ApiSpec{
+					Name: "projects/my-project/locations/global/apis/a/versions/v/specs/s",
+				},
+				AllowMissing: false,
+			},
+			want: codes.NotFound,
+		},
+		{
+			desc: "create using update with allow_missing=true",
+			req: &rpc.UpdateApiSpecRequest{
+				ApiSpec: &rpc.ApiSpec{
+					Name: "projects/my-project/locations/global/apis/a/versions/v/specs/s",
+				},
+				AllowMissing: true,
+			},
+			want: codes.OK,
+		},
+		{
+			desc: "update existing resource with allow_missing=true",
+			req: &rpc.UpdateApiSpecRequest{
+				ApiSpec: &rpc.ApiSpec{
+					Name: "projects/my-project/locations/global/apis/a/versions/v/specs/s",
+				},
+				AllowMissing: true,
+			},
+			want: codes.OK,
+		},
+		{
+			desc: "update existing resource with allow_missing=false",
+			req: &rpc.UpdateApiSpecRequest{
+				ApiSpec: &rpc.ApiSpec{
+					Name: "projects/my-project/locations/global/apis/a/versions/v/specs/s",
+				},
+				AllowMissing: false,
+			},
+			want: codes.OK,
+		},
+	}
+	ctx := context.Background()
+	server := defaultTestServer(t)
+	seed := &rpc.ApiVersion{Name: "projects/my-project/locations/global/apis/a/versions/v"}
+	if err := seeder.SeedVersions(ctx, server, seed); err != nil {
+		t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
+	}
+	var createTime time.Time
+	var revisionCreateTime time.Time
+	var revisionUpdateTime time.Time
+	// NOTE: in the following sequence of tests, each test depends on its predecessor.
+	// Resources are successively created and updated using the "Update" RPC and the
+	// tests verify that CreateTime/UpdateTime fields are modified appropriately.
+	for i, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			var result *rpc.ApiSpec
+			var err error
+			if result, err = server.UpdateApiSpec(ctx, test.req); status.Code(err) != test.want {
+				t.Errorf("UpdateApiSpec(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
+			}
+			if result != nil {
+				if i == 1 {
+					createTime = result.CreateTime.AsTime()
+					revisionCreateTime = result.RevisionCreateTime.AsTime()
+					revisionUpdateTime = result.RevisionUpdateTime.AsTime()
+				} else {
+					if !createTime.Equal(result.CreateTime.AsTime()) {
+						t.Errorf("UpdateApiSpec create time changed after update (%v %v)", createTime, result.CreateTime.AsTime())
+					}
+					if !revisionCreateTime.Equal(result.RevisionCreateTime.AsTime()) {
+						t.Errorf("UpdateApiSpec revision create time changed after update (%v %v)", revisionCreateTime, result.RevisionCreateTime.AsTime())
+					}
+					if !revisionUpdateTime.Before(result.RevisionUpdateTime.AsTime()) {
+						t.Errorf("UpdateApiSpec update time did not increase after update (%v %v)", revisionUpdateTime, result.RevisionUpdateTime.AsTime())
+					}
+					revisionUpdateTime = result.RevisionUpdateTime.AsTime()
+				}
+			}
+		})
+	}
+}
+
 func TestDeleteApiSpec(t *testing.T) {
 	tests := []struct {
 		desc string
diff --git a/server/registry/actions_versions_test.go b/server/registry/actions_versions_test.go
index a3ab789..e682219 100644
--- a/server/registry/actions_versions_test.go
+++ b/server/registry/actions_versions_test.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/test/seeder"
@@ -1167,7 +1168,7 @@ func TestUpdateApiVersionSequence(t *testing.T) {
 				ApiVersion: &rpc.ApiVersion{
 					Name: "projects/my-project/locations/global/apis/a/versions/v",
 				},
-				AllowMissing: true,
+				AllowMissing: false,
 			},
 			want: codes.OK,
 		},
@@ -1178,11 +1179,32 @@ func TestUpdateApiVersionSequence(t *testing.T) {
 	if err := seeder.SeedApis(ctx, server, seed); err != nil {
 		t.Fatalf("Setup/Seeding: Failed to seed registry: %s", err)
 	}
-	for _, test := range tests {
+	var createTime time.Time
+	var updateTime time.Time
+	// NOTE: in the following sequence of tests, each test depends on its predecessor.
+	// Resources are successively created and updated using the "Update" RPC and the
+	// tests verify that CreateTime/UpdateTime fields are modified appropriately.
+	for i, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			if _, err := server.UpdateApiVersion(ctx, test.req); status.Code(err) != test.want {
+			var result *rpc.ApiVersion
+			var err error
+			if result, err = server.UpdateApiVersion(ctx, test.req); status.Code(err) != test.want {
 				t.Errorf("UpdateApiVersion(%+v) returned status code %q, want %q: %v", test.req, status.Code(err), test.want, err)
 			}
+			if result != nil {
+				if i == 1 {
+					createTime = result.CreateTime.AsTime()
+					updateTime = result.UpdateTime.AsTime()
+				} else {
+					if !createTime.Equal(result.CreateTime.AsTime()) {
+						t.Errorf("UpdateApiVersion create time changed after update (%v %v)", createTime, result.CreateTime.AsTime())
+					}
+					if !updateTime.Before(result.UpdateTime.AsTime()) {
+						t.Errorf("UpdateApiVersion update time did not increase after update (%v %v)", updateTime, result.UpdateTime.AsTime())
+					}
+					updateTime = result.UpdateTime.AsTime()
+				}
+			}
 		})
 	}
 }