From 7f3049e573bc880097af6182bc0c5d3c672327f6 Mon Sep 17 00:00:00 2001 From: Nadia Pinaeva Date: Wed, 7 Aug 2024 11:20:37 +0200 Subject: [PATCH] When getting cache values, dereference pointers. This allows using optional pointer fields (e.g. *string) as client indices. Signed-off-by: Nadia Pinaeva --- README.md | 5 +- cache/cache.go | 5 + cache/cache_test.go | 305 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 313 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f35e8613..6c9f6c99 100644 --- a/README.md +++ b/README.md @@ -90,8 +90,9 @@ The table is inferred from the type that the function accepts as only argument. The client will track schema indexes and use them when appropriate in `Get`, `Where`, and `WhereAll` as explained above. Additional indexes can be specified for a client instance to track. Just as schema indexes, client indexes are specified in sets per table. -where each set consists of the columns that compose the index. Unlike schema indexes, a key within a column can be addressed if the column -type is a map. +where each set consists of the columns that compose the index. However, unlike schema indexes, client indexes: +- can be used with columns that are maps, where specific map keys can be indexed (see example below). +- can be used with columns that are optional, where no-value columns are indexed as well. Client indexes are leveraged through `Where`, and `WhereAll`. Since client indexes value uniqueness is not enforced as it happens with schema indexes, conditions based on them can match multiple rows. diff --git a/cache/cache.go b/cache/cache.go index 60182071..0b1e09e7 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -1261,6 +1261,11 @@ func valueFromColumnKey(info *mapper.Info, columnKey model.ColumnKey) (interface return "", fmt.Errorf("can't get key value from map: %v", err) } } + // if the value is a non-nil pointer of an optional, dereference + v := reflect.ValueOf(val) + if v.Kind() == reflect.Ptr && !v.IsNil() { + val = v.Elem().Interface() + } return val, err } diff --git a/cache/cache_test.go b/cache/cache_test.go index abca11d5..769847b3 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -280,6 +280,96 @@ func TestRowCacheCreateClientIndex(t *testing.T) { } } +func getStringPtr(s string) *string { + return &s +} + +var nilString *string + +func TestRowCacheCreateOptionalColumnClientIndex(t *testing.T) { + var schema ovsdb.DatabaseSchema + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db.SetIndexes(map[string][]model.ClientIndex{ + "Open_vSwitch": { + { + Columns: []model.ColumnKey{ + { + Column: "datapath", + }, + }, + }, + }, + }) + require.Nil(t, err) + err = json.Unmarshal(getTestSchema(""), &schema) + require.Nil(t, err) + testData := Data{ + "Open_vSwitch": map[string]model.Model{"bar": &testModel{Datapath: getStringPtr("bar")}}, + } + + dbModel, errs := model.NewDatabaseModel(schema, db) + require.Empty(t, errs) + + tests := []struct { + name string + uuid string + model *testModel + wantErr bool + expected valueToUUIDs + }{ + { + name: "inserts a new row", + uuid: "foo", + model: &testModel{Datapath: getStringPtr("foo")}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("foo"), + "bar": newUUIDSet("bar"), + }, + }, + { + name: "error duplicate uuid", + uuid: "bar", + model: &testModel{Datapath: getStringPtr("foo")}, + wantErr: true, + }, + { + name: "inserts duplicate index", + uuid: "baz", + model: &testModel{Datapath: getStringPtr("bar")}, + wantErr: false, + expected: valueToUUIDs{ + "bar": newUUIDSet("bar", "baz"), + }, + }, + { + name: "inserts nil index", + uuid: "nil", + model: &testModel{Datapath: nil}, + wantErr: false, + expected: valueToUUIDs{ + "bar": newUUIDSet("bar"), + nilString: newUUIDSet("nil"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc, err := NewTableCache(dbModel, testData, nil) + require.Nil(t, err) + rc := tc.Table("Open_vSwitch") + require.NotNil(t, rc) + err = rc.Create(tt.uuid, tt.model, true) + if tt.wantErr { + require.Error(t, err) + } else { + require.Nil(t, err) + require.Equal(t, tt.expected, rc.indexes["datapath"]) + } + }) + } +} + func TestRowCacheCreateMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) @@ -709,6 +799,129 @@ func TestRowCacheUpdateClientIndex(t *testing.T) { } } +func TestRowCacheUpdateOptionalColumnClientIndex(t *testing.T) { + var schema ovsdb.DatabaseSchema + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + require.Nil(t, err) + db.SetIndexes(map[string][]model.ClientIndex{ + "Open_vSwitch": { + { + Columns: []model.ColumnKey{ + { + Column: "datapath", + }, + }, + }, + }, + }) + err = json.Unmarshal(getTestSchema(""), &schema) + require.Nil(t, err) + testData := Data{ + "Open_vSwitch": map[string]model.Model{ + "foo": &testModel{Datapath: getStringPtr("foo"), Bar: "foo"}, + "bar": &testModel{Datapath: getStringPtr("bar"), Bar: "bar"}, + "foobar": &testModel{Datapath: getStringPtr("bar"), Bar: "foobar"}, + "nil": &testModel{Datapath: nilString, Bar: "nil"}, + }, + } + dbModel, errs := model.NewDatabaseModel(schema, db) + require.Empty(t, errs) + + tests := []struct { + name string + uuid string + model *testModel + wantErr bool + expected valueToUUIDs + }{ + { + name: "error if row does not exist", + uuid: "baz", + model: &testModel{Datapath: getStringPtr("baz")}, + wantErr: true, + }, + { + name: "update non-index", + uuid: "foo", + model: &testModel{Datapath: getStringPtr("foo"), Bar: "bar"}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("foo"), + "bar": newUUIDSet("bar", "foobar"), + nilString: newUUIDSet("nil"), + }, + }, + { + name: "update unique index to new index", + uuid: "foo", + model: &testModel{Datapath: getStringPtr("baz")}, + wantErr: false, + expected: valueToUUIDs{ + "baz": newUUIDSet("foo"), + "bar": newUUIDSet("bar", "foobar"), + nilString: newUUIDSet("nil"), + }, + }, + { + name: "update unique index to existing index", + uuid: "foo", + model: &testModel{Datapath: getStringPtr("bar")}, + wantErr: false, + expected: valueToUUIDs{ + "bar": newUUIDSet("foo", "bar", "foobar"), + nilString: newUUIDSet("nil"), + }, + }, + { + name: "update multi index to different index", + uuid: "foobar", + model: &testModel{Datapath: getStringPtr("foo")}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("foo", "foobar"), + "bar": newUUIDSet("bar"), + nilString: newUUIDSet("nil"), + }, + }, + { + name: "update nil index to new index", + uuid: "nil", + model: &testModel{Datapath: getStringPtr("foo")}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("nil", "foo"), + "bar": newUUIDSet("bar", "foobar"), + }, + }, + { + name: "update multi index to nil index", + uuid: "foobar", + model: &testModel{Datapath: nilString}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("foo"), + "bar": newUUIDSet("bar"), + nilString: newUUIDSet("nil", "foobar"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc, err := NewTableCache(dbModel, testData, nil) + require.Nil(t, err) + rc := tc.Table("Open_vSwitch") + require.NotNil(t, rc) + _, err = rc.Update(tt.uuid, tt.model, true) + if tt.wantErr { + require.Error(t, err) + } else { + require.Nil(t, err) + require.Equal(t, tt.expected, rc.indexes["datapath"]) + } + }) + } +} + func TestRowCacheUpdateMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) @@ -1168,6 +1381,98 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { } } +func TestRowCacheDeleteOptionalColumnClientIndex(t *testing.T) { + var schema ovsdb.DatabaseSchema + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + require.Nil(t, err) + + db.SetIndexes(map[string][]model.ClientIndex{ + "Open_vSwitch": { + { + Columns: []model.ColumnKey{ + { + Column: "datapath", + }, + }, + }, + }, + }) + err = json.Unmarshal(getTestSchema(""), &schema) + require.Nil(t, err) + + testData := Data{ + "Open_vSwitch": map[string]model.Model{ + "foo": &testModel{Datapath: getStringPtr("foo"), Bar: "foo"}, + "bar": &testModel{Datapath: getStringPtr("bar"), Bar: "bar"}, + "foobar": &testModel{Datapath: getStringPtr("bar"), Bar: "foobar"}, + "nil": &testModel{Datapath: nilString, Bar: "nil"}, + }, + } + dbModel, errs := model.NewDatabaseModel(schema, db) + require.Empty(t, errs) + + tests := []struct { + name string + uuid string + model *testModel + wantErr bool + expected valueToUUIDs + }{ + { + name: "error if row does not exist", + uuid: "baz", + model: &testModel{Datapath: getStringPtr("baz")}, + wantErr: true, + }, + { + name: "delete a row with unique index", + uuid: "foo", + model: &testModel{Datapath: getStringPtr("foo"), Bar: "foo"}, + wantErr: false, + expected: valueToUUIDs{ + "bar": newUUIDSet("bar", "foobar"), + nilString: newUUIDSet("nil"), + }, + }, + { + name: "delete a row with duplicated index", + uuid: "bar", + model: &testModel{Datapath: getStringPtr("bar"), Bar: "bar"}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("foo"), + "bar": newUUIDSet("foobar"), + nilString: newUUIDSet("nil"), + }, + }, + { + name: "delete a row with nil index", + uuid: "nil", + model: &testModel{Datapath: nilString, Bar: "nil"}, + wantErr: false, + expected: valueToUUIDs{ + "foo": newUUIDSet("foo"), + "bar": newUUIDSet("bar", "foobar"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc, err := NewTableCache(dbModel, testData, nil) + require.Nil(t, err) + rc := tc.Table("Open_vSwitch") + require.NotNil(t, rc) + err = rc.Delete(tt.uuid) + if tt.wantErr { + require.Error(t, err) + } else { + require.Nil(t, err) + require.Equal(t, tt.expected, rc.indexes["datapath"]) + } + }) + } +} + func TestEventHandlerFuncs_OnAdd(t *testing.T) { calls := 0 type fields struct {