From 46fc06099852412071be8c8a02d0d9416fd08817 Mon Sep 17 00:00:00 2001 From: Alex Hong <9397363+hongalex@users.noreply.github.com> Date: Wed, 24 May 2023 09:32:59 -0700 Subject: [PATCH] fix(pubsub): allow clearing of topic schema (#7980) Co-authored-by: Anna Cocuzzo <63511057+acocuzzo@users.noreply.github.com> --- pubsub/pstest/fake.go | 3 +-- pubsub/topic.go | 8 +++++++- pubsub/topic_test.go | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pubsub/pstest/fake.go b/pubsub/pstest/fake.go index f34b46ddbb6..b5bee85ce1b 100644 --- a/pubsub/pstest/fake.go +++ b/pubsub/pstest/fake.go @@ -362,8 +362,7 @@ func (s *GServer) UpdateTopic(_ context.Context, req *pb.UpdateTopicRequest) (*p } t.proto.MessageRetentionDuration = req.Topic.MessageRetentionDuration case "schema_settings": - // Clear this field. - t.proto.SchemaSettings = &pb.SchemaSettings{} + t.proto.SchemaSettings = req.Topic.SchemaSettings case "schema_settings.schema": if t.proto.SchemaSettings == nil { t.proto.SchemaSettings = &pb.SchemaSettings{} diff --git a/pubsub/topic.go b/pubsub/topic.go index 0cc1f95ad94..a17f17b8674 100644 --- a/pubsub/topic.go +++ b/pubsub/topic.go @@ -407,6 +407,11 @@ func (t *Topic) updateRequest(cfg TopicConfigToUpdate) *pb.UpdateTopicRequest { } paths = append(paths, "message_retention_duration") } + // Updating SchemaSettings' field masks are more complicated here + // since each field should be able to be independently edited, while + // preserving the current values for everything else. We also denote + // the zero value SchemaSetting to mean clearing or removing schema + // from the topic. if cfg.SchemaSettings != nil { pt.SchemaSettings = schemaSettingsToProto(cfg.SchemaSettings) clearSchema := true @@ -426,9 +431,10 @@ func (t *Topic) updateRequest(cfg TopicConfigToUpdate) *pb.UpdateTopicRequest { paths = append(paths, "schema_settings.last_revision_id") clearSchema = false } - // Clear the schema if none of it's value changes. + // Clear the schema if all of its values are equal to the zero value. if clearSchema { paths = append(paths, "schema_settings") + pt.SchemaSettings = nil } } return &pb.UpdateTopicRequest{ diff --git a/pubsub/topic_test.go b/pubsub/topic_test.go index 5992d03f2c4..4b7d73da398 100644 --- a/pubsub/topic_test.go +++ b/pubsub/topic_test.go @@ -342,8 +342,8 @@ func TestUpdateTopic_SchemaSettings(t *testing.T) { if err != nil { t.Fatal(err) } - if !testutil.Equal(config3.SchemaSettings, settings, opt) { - t.Errorf("\ngot %+v\nwant %+v", config3.SchemaSettings, settings) + if config3.SchemaSettings != nil { + t.Errorf("got: %+v, want nil", config3.SchemaSettings) } }