From 4e3d514e2aabaf4eb2aa022feeebc305b8b90ff0 Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Wed, 21 Feb 2024 16:41:18 +0000 Subject: [PATCH 1/2] fix(bigtable/bttest): Error when applying a mutation with an empty row key Against a real version of Bigtable, an attempt to apply a mutation with an empty row key leads to the following error being produced ``` unable to apply: rpc error: code = InvalidArgument desc = Row keys must be non-empty ``` bttest does not conform to this behaviour. This commit: - adds error-handling when given a mutation with an empty row key, and - adds a regression test. --- bigtable/bttest/inmem.go | 7 ++++++ bigtable/bttest/inmem_test.go | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/bigtable/bttest/inmem.go b/bigtable/bttest/inmem.go index 4114901d8e3b..6850d53df7ec 100644 --- a/bigtable/bttest/inmem.go +++ b/bigtable/bttest/inmem.go @@ -1079,6 +1079,13 @@ func (s *server) PingAndWarm(ctx context.Context, req *btpb.PingAndWarmRequest) // fam should be a snapshot of the keys of tbl.families. // It assumes r.mu is locked. func applyMutations(tbl *table, r *row, muts []*btpb.Mutation, fs map[string]*columnFamily) error { + if len(r.key) == 0 { + return status.Errorf( + codes.InvalidArgument, + "Row keys must be non-empty", + ) + } + for _, mut := range muts { switch mut := mut.Mutation.(type) { default: diff --git a/bigtable/bttest/inmem_test.go b/bigtable/bttest/inmem_test.go index 1072d7e91844..b6294a8c8121 100644 --- a/bigtable/bttest/inmem_test.go +++ b/bigtable/bttest/inmem_test.go @@ -2334,6 +2334,48 @@ func TestMutateRowsEmptyMutationErrors(t *testing.T) { } } +func TestMutateRowEmptyRowKeyErrors(t *testing.T) { + srv := &server{tables: make(map[string]*table)} + ctx := context.Background() + + const tableID = "mytable" + tblReq := &btapb.CreateTableRequest{ + Parent: "cluster", + TableId: tableID, + Table: &btapb.Table{ + ColumnFamilies: map[string]*btapb.ColumnFamily{"cf": {}}, + }, + } + if _, err := srv.CreateTable(ctx, tblReq); err != nil { + t.Fatalf("Failed to create the table: %v", err) + } + + const name = "cluster/tables/" + tableID + req := &btpb.MutateRowRequest{ + TableName: name, + RowKey: []byte(""), + Mutations: []*btpb.Mutation{ + { + Mutation: &btpb.Mutation_SetCell_{ + SetCell: &btpb.Mutation_SetCell{ + FamilyName: "cf", + ColumnQualifier: []byte("col"), + TimestampMicros: 1000, + Value: []byte("hello, world!"), + }, + }, + }, + }, + } + + resp, err := srv.MutateRow(ctx, req) + if resp != nil || err == nil || err.Error() != + "rpc error: code = InvalidArgument"+ + " desc = Row keys must be non-empty" { + t.Fatalf("Failed to produce the expected error: %s", err) + } +} + func TestFilterRowCellsPerRowLimitFilterTruthiness(t *testing.T) { row := &row{ key: "row", From 8ef1a73c5f2c5eb34415cfc307c35dd66ca9d0d8 Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Wed, 21 Feb 2024 16:27:12 +0000 Subject: [PATCH 2/2] chore(bigtable/bttest): Remove a deprecated option from the example grpc.WithInsecure is deprecated. It was marked as such by v1.43.0 of the library. See here: https://github.com/grpc/grpc-go/pull/4718 This commit removes a reference to this from the example comment. --- bigtable/bttest/inmem.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bigtable/bttest/inmem.go b/bigtable/bttest/inmem.go index 6850d53df7ec..9301962adb12 100644 --- a/bigtable/bttest/inmem.go +++ b/bigtable/bttest/inmem.go @@ -22,7 +22,9 @@ To use a Server, create it, and then connect to it with no security: srv, err := bttest.NewServer("localhost:0") ... - conn, err := grpc.Dial(srv.Addr, grpc.WithInsecure()) + conn, err := grpc.Dial( + srv.Addr, + grpc.WithTransportCredentials(insecure.NewCredentials())) ... client, err := bigtable.NewClient(ctx, proj, instance, option.WithGRPCConn(conn))