From e2463abf549429f8d795a5236de78d322a02f091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Wed, 20 Dec 2023 15:59:49 +0000 Subject: [PATCH 01/13] updates: fix merge resulting in empty optional values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An optional atomic value is cleared with an empty set instead of a nil set. Thus use an empty set as an original empty value so that when an op that sets a value is merged with an op that clears the value, the comparison with an empty orignal value results in a noop. Signed-off-by: Jaime Caamaño Ruiz --- updates/merge.go | 10 +++++++++ updates/merge_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/updates/merge.go b/updates/merge.go index 903490c1..562f2262 100644 --- a/updates/merge.go +++ b/updates/merge.go @@ -132,6 +132,16 @@ func mergeModifyRow(ts *ovsdb.TableSchema, o, a, b *ovsdb.Row) *ovsdb.Row { // assume zero value if original does not have the column o = reflect.Zero(reflect.TypeOf(v)).Interface() } + if set, ok := o.(ovsdb.OvsSet); ok { + // atomic optional values are cleared out with an empty set + // if the original value was also cleared out, use an empty set + // instead of a nil set so that mergeAtomicDifference notices + // that we are returning to the original value + if set.GoSet == nil { + set.GoSet = []interface{}{} + } + o = set + } result, changed = mergeAtomicDifference(o, aMod[k], v) } diff --git a/updates/merge_test.go b/updates/merge_test.go index 54039f97..1b6318ad 100644 --- a/updates/merge_test.go +++ b/updates/merge_test.go @@ -828,6 +828,53 @@ func Test_merge(t *testing.T) { }, }, }, + { + name: "update optional field to original empty value after update results in no op", + args: args{ + a: modelUpdate{ + old: &test.BridgeType{ + Name: "bridge", + }, + new: &test.BridgeType{ + Name: "bridge", + DatapathID: &newDatapathID, + }, + rowUpdate2: &ovsdb.RowUpdate2{ + Old: &ovsdb.Row{ + "name": "bridge", + }, + New: &ovsdb.Row{ + "name": "bridge", + "datapath_id": ovsdb.OvsSet{GoSet: []interface{}{newDatapathID}}, + }, + Modify: &ovsdb.Row{ + "datapath_id": ovsdb.OvsSet{GoSet: []interface{}{newDatapathID}}, + }, + }, + }, + b: modelUpdate{ + old: &test.BridgeType{ + Name: "bridge", + DatapathID: &newDatapathID, + }, + new: &test.BridgeType{ + Name: "bridge", + }, + rowUpdate2: &ovsdb.RowUpdate2{ + Old: &ovsdb.Row{ + "name": "bridge", + "datapath_id": ovsdb.OvsSet{GoSet: []interface{}{newDatapathID}}, + }, + New: &ovsdb.Row{ + "name": "bridge", + }, + Modify: &ovsdb.Row{ + "datapath_id": ovsdb.OvsSet{GoSet: []interface{}{}}, + }, + }, + }, + }, + }, { name: "update optional field to empty value after update", args: args{ From d03747f26392b4da8ae61c5066655ae19131c6f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Wed, 20 Dec 2023 16:05:03 +0000 Subject: [PATCH 02/13] updates: clean up when merging results in noop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jaime Caamaño Ruiz --- updates/updates.go | 16 ++++++++++++---- updates/updates_test.go | 8 ++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/updates/updates.go b/updates/updates.go index 815cb9e6..f8591560 100644 --- a/updates/updates.go +++ b/updates/updates.go @@ -218,13 +218,21 @@ func (u *ModelUpdates) addUpdate(dbModel model.DatabaseModel, table, uuid string return err } - // If after the merge this amounts to no update, remove it from the list - if update.isEmpty() { - delete(u.updates[table], uuid) + if !update.isEmpty() { + u.updates[table][uuid] = update return nil } - u.updates[table][uuid] = update + // If after the merge this amounts to no update, remove it from the list and + // clean up + delete(u.updates[table], uuid) + if len(u.updates[table]) == 0 { + delete(u.updates, table) + } + if len(u.updates) == 0 { + u.updates = nil + } + return nil } diff --git a/updates/updates_test.go b/updates/updates_test.go index 5d318f3d..6d67fee5 100644 --- a/updates/updates_test.go +++ b/updates/updates_test.go @@ -488,9 +488,7 @@ func TestUpdates_AddOperation(t *testing.T) { }, }, expected: fields{ - updates: map[string]map[string]modelUpdate{ - "Bridge": {}, - }, + updates: nil, }, }, { @@ -1173,9 +1171,7 @@ func TestUpdates_AddOperation(t *testing.T) { }, }, expected: fields{ - updates: map[string]map[string]modelUpdate{ - "Bridge": {}, - }, + updates: nil, }, }, { From f6870ab3df05a245ee0e3a1cc389a1f359ec990d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 9 Jan 2024 13:41:23 +0000 Subject: [PATCH 03/13] transaction: fix results on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The number of results of a transaction should always have a minimum length of the number of operations passed to it even if there is an error. The recent introduction of the named UUID expansion in the transaction broke this requirement. Changed the error handling approach so that this is less likely to happen in the future. Also removed the panic when initializing the transaction cache. Signed-off-by: Jaime Caamaño Ruiz --- database/transaction.go | 71 ++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/database/transaction.go b/database/transaction.go index 97d586a4..7e69a555 100644 --- a/database/transaction.go +++ b/database/transaction.go @@ -20,6 +20,7 @@ type Transaction struct { Model model.DatabaseModel DbName string Database Database + logger *logr.Logger } func NewTransaction(model model.DatabaseModel, dbName string, database Database, logger *logr.Logger) Transaction { @@ -27,24 +28,34 @@ func NewTransaction(model model.DatabaseModel, dbName string, database Database, l := logger.WithName("transaction") logger = &l } - cache, err := cache.NewTableCache(model, nil, logger) - if err != nil { - panic(err) - } + return Transaction{ ID: uuid.New(), - Cache: cache, DeletedRows: make(map[string]struct{}), Model: model, DbName: dbName, Database: database, + logger: logger, } } func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.OperationResult, Update) { - results := []*ovsdb.OperationResult{} + results := make([]*ovsdb.OperationResult, len(operations), len(operations)+1) update := updates.ModelUpdates{} + if !t.Database.Exists(t.DbName) { + r := ovsdb.ResultFromError(fmt.Errorf("database does not exist")) + results[0] = &r + return results, nil + } + + err := t.initializeCache() + if err != nil { + r := ovsdb.ResultFromError(err) + results[0] = &r + return results, nil + } + // Every Insert operation must have a UUID for i := range operations { op := &operations[i] @@ -54,31 +65,15 @@ func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.Operation } // Ensure Named UUIDs are expanded in all operations - var err error operations, err = ovsdb.ExpandNamedUUIDs(operations, &t.Model.Schema) if err != nil { r := ovsdb.ResultFromError(err) - return []*ovsdb.OperationResult{&r}, nil + results[0] = &r + return results, nil } var r ovsdb.OperationResult - for _, op := range operations { - // if we had a previous error, just append a nil result for every op - // after that - if r.Error != "" { - results = append(results, nil) - continue - } - - // simple case: database name does not exist - if !t.Database.Exists(t.DbName) { - r = ovsdb.OperationResult{ - Error: "database does not exist", - } - results = append(results, &r) - continue - } - + for i, op := range operations { var u *updates.ModelUpdates switch op.Op { case ovsdb.OperationInsert: @@ -118,7 +113,12 @@ func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.Operation } result := r - results = append(results, &result) + results[i] = &result + + // if an operation failed, no need to process any further operation + if r.Error != "" { + break + } } // if an operation failed, no need to do any further validation @@ -126,6 +126,11 @@ func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.Operation return results, update } + // if there is no updates, no need to do any further validation + if len(update.GetUpdatedTables()) == 0 { + return results, update + } + // check index constraints if err := t.checkIndexes(); err != nil { if indexExists, ok := err.(*cache.ErrIndexExists); ok { @@ -141,7 +146,21 @@ func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.Operation return results, update } +func (t *Transaction) initializeCache() error { + if t.Cache != nil { + return nil + } + var err error + t.Cache, err = cache.NewTableCache(t.Model, nil, t.logger) + return err +} + func (t *Transaction) rowsFromTransactionCacheAndDatabase(table string, where []ovsdb.Condition) (map[string]model.Model, error) { + err := t.initializeCache() + if err != nil { + return nil, err + } + txnRows, err := t.Cache.Table(table).RowsByCondition(where) if err != nil { return nil, fmt.Errorf("failed getting rows for table %s from transaction cache: %v", table, err) From f3b4525ce1b4524dc559d6928067c581e14bd51c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Wed, 30 Nov 2022 19:00:39 +0000 Subject: [PATCH 04/13] Split database package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main purpose of this change is to have a package for common database types and interfaces that can be used throughout the project without incurring in circular dependencies. To that end, the in-memory database implementation is moved to its own subpackage. The transaction implementation is also moved to its own subpackage given that it can work with any database implementing the Database interface. At the interface level though it should be expected that a transaction implementation could have a more tighter underlying coupling with a database implementation. So it makes sense to introduce a transaction interface, and for transactions to be provided by database implementations rather than being built directly by the server. Signed-off-by: Jaime Caamaño Ruiz --- README.md | 2 +- client/api.go | 6 +- client/client_test.go | 4 +- database/database.go | 109 +---- database/doc.go | 3 +- database/inmemory/doc.go | 4 + database/inmemory/inmemory.go | 125 ++++++ .../inmemory_test.go} | 419 +++++++++--------- database/transaction/doc.go | 4 + database/{ => transaction}/errors.go | 2 +- database/{ => transaction}/transaction.go | 9 +- example/ovsdb-server/main.go | 4 +- server/server.go | 64 ++- server/server_integration_test.go | 4 +- server/server_test.go | 8 +- 15 files changed, 413 insertions(+), 354 deletions(-) create mode 100644 database/inmemory/doc.go create mode 100644 database/inmemory/inmemory.go rename database/{transaction_test.go => inmemory/inmemory_test.go} (70%) create mode 100644 database/transaction/doc.go rename database/{ => transaction}/errors.go (95%) rename database/{ => transaction}/transaction.go (97%) diff --git a/README.md b/README.md index f43869a5..f35e8613 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ This package is divided into several sub-packages. Documentation for each sub-pa * **cache**: model-based cache [![godoc for libovsdb/cache][cachebadge]][cachedoc] * **modelgen**: common code-generator functions [![godoc for libovsdb/modelgen][genbadge]][gendoc] * **server**: ovsdb test server [![godoc for libovsdb/server][serverbadge]][serverdoc] -* **database**: in-memory database for the server [![godoc for libovsdb/database][dbbadge]][dbdoc] +* **database**: database related types, interfaces and implementations [![godoc for libovsdb/database][dbbadge]][dbdoc] * **updates**: common code to handle model updates [![godoc for libovsdb/updates][updatesbadge]][updatesdoc] [doc]: https://pkg.go.dev/ diff --git a/client/api.go b/client/api.go index 9c295638..49775894 100644 --- a/client/api.go +++ b/client/api.go @@ -51,9 +51,9 @@ type API interface { Get(context.Context, model.Model) error // Create returns the operation needed to add the model(s) to the Database - // Only fields with non-default values will be added to the transaction - // If the field associated with column "_uuid" has some content, it will be - // treated as named-uuid + // Only fields with non-default values will be added to the transaction. If + // the field associated with column "_uuid" has some content other than a + // UUID, it will be treated as named-uuid Create(...model.Model) ([]ovsdb.Operation, error) } diff --git a/client/client_test.go b/client/client_test.go index 1f66651a..f6e4da05 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -20,7 +20,7 @@ import ( "github.com/go-logr/stdr" "github.com/google/uuid" "github.com/ovn-org/libovsdb/cache" - db "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/database/inmemory" "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" @@ -956,7 +956,7 @@ func newOVSDBServer(t *testing.T, dbModel model.ClientDBModel, schema ovsdb.Data require.NoError(t, err) serverSchema := serverdb.Schema() - db := db.NewInMemoryDatabase(map[string]model.ClientDBModel{ + db := inmemory.NewDatabase(map[string]model.ClientDBModel{ schema.Name: dbModel, serverSchema.Name: serverDBModel, }) diff --git a/database/database.go b/database/database.go index 47d14505..f41db7d1 100644 --- a/database/database.go +++ b/database/database.go @@ -1,122 +1,31 @@ package database import ( - "fmt" - "sync" - "github.com/google/uuid" - "github.com/ovn-org/libovsdb/cache" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" ) -// Database abstracts database operations from ovsdb +// Database abstracts a database that a server can use to store and transact data type Database interface { CreateDatabase(database string, model ovsdb.DatabaseSchema) error Exists(database string) bool + NewTransaction(database string) Transaction Commit(database string, id uuid.UUID, update Update) error CheckIndexes(database string, table string, m model.Model) error List(database, table string, conditions ...ovsdb.Condition) (map[string]model.Model, error) Get(database, table string, uuid string) (model.Model, error) } -// Update abstacts a database update in both ovsdb and model notation +// Transaction abstracts a database transaction that can generate database +// updates +type Transaction interface { + Transact(operations ...ovsdb.Operation) ([]*ovsdb.OperationResult, Update) +} + +// Update abstracts an update that can be committed to a database type Update interface { GetUpdatedTables() []string ForEachModelUpdate(table string, do func(uuid string, old, new model.Model) error) error ForEachRowUpdate(table string, do func(uuid string, row ovsdb.RowUpdate2) error) error } - -type inMemoryDatabase struct { - databases map[string]*cache.TableCache - models map[string]model.ClientDBModel - mutex sync.RWMutex -} - -func NewInMemoryDatabase(models map[string]model.ClientDBModel) Database { - return &inMemoryDatabase{ - databases: make(map[string]*cache.TableCache), - models: models, - mutex: sync.RWMutex{}, - } -} - -func (db *inMemoryDatabase) CreateDatabase(name string, schema ovsdb.DatabaseSchema) error { - db.mutex.Lock() - defer db.mutex.Unlock() - var mo model.ClientDBModel - var ok bool - if mo, ok = db.models[schema.Name]; !ok { - return fmt.Errorf("no db model provided for schema with name %s", name) - } - dbModel, errs := model.NewDatabaseModel(schema, mo) - if len(errs) > 0 { - return fmt.Errorf("failed to create DatabaseModel: %#+v", errs) - } - database, err := cache.NewTableCache(dbModel, nil, nil) - if err != nil { - return err - } - db.databases[name] = database - return nil -} - -func (db *inMemoryDatabase) Exists(name string) bool { - db.mutex.RLock() - defer db.mutex.RUnlock() - _, ok := db.databases[name] - return ok -} - -func (db *inMemoryDatabase) Commit(database string, id uuid.UUID, update Update) error { - if !db.Exists(database) { - return fmt.Errorf("db does not exist") - } - db.mutex.RLock() - targetDb := db.databases[database] - db.mutex.RUnlock() - - return targetDb.ApplyCacheUpdate(update) -} - -func (db *inMemoryDatabase) CheckIndexes(database string, table string, m model.Model) error { - if !db.Exists(database) { - return nil - } - db.mutex.RLock() - targetDb := db.databases[database] - db.mutex.RUnlock() - targetTable := targetDb.Table(table) - return targetTable.IndexExists(m) -} - -func (db *inMemoryDatabase) List(database, table string, conditions ...ovsdb.Condition) (map[string]model.Model, error) { - if !db.Exists(database) { - return nil, fmt.Errorf("db does not exist") - } - db.mutex.RLock() - targetDb := db.databases[database] - db.mutex.RUnlock() - - targetTable := targetDb.Table(table) - if targetTable == nil { - return nil, fmt.Errorf("table does not exist") - } - - return targetTable.RowsByCondition(conditions) -} - -func (db *inMemoryDatabase) Get(database, table string, uuid string) (model.Model, error) { - if !db.Exists(database) { - return nil, fmt.Errorf("db does not exist") - } - db.mutex.RLock() - targetDb := db.databases[database] - db.mutex.RUnlock() - - targetTable := targetDb.Table(table) - if targetTable == nil { - return nil, fmt.Errorf("table does not exist") - } - return targetTable.Row(uuid), nil -} diff --git a/database/doc.go b/database/doc.go index 8d3bdcb9..c0a858c2 100644 --- a/database/doc.go +++ b/database/doc.go @@ -1,4 +1,5 @@ /* -Package database provides an in-memory database implementation. +Package database collects database related types, interfaces and +implementations. */ package database diff --git a/database/inmemory/doc.go b/database/inmemory/doc.go new file mode 100644 index 00000000..bde3ffc0 --- /dev/null +++ b/database/inmemory/doc.go @@ -0,0 +1,4 @@ +/* +Package inmemory provides a in-memory database implementation +*/ +package inmemory diff --git a/database/inmemory/inmemory.go b/database/inmemory/inmemory.go new file mode 100644 index 00000000..618d99b8 --- /dev/null +++ b/database/inmemory/inmemory.go @@ -0,0 +1,125 @@ +package inmemory + +import ( + "fmt" + "log" + "os" + "sync" + + "github.com/go-logr/logr" + "github.com/go-logr/stdr" + "github.com/google/uuid" + "github.com/ovn-org/libovsdb/cache" + dbase "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/database/transaction" + "github.com/ovn-org/libovsdb/model" + "github.com/ovn-org/libovsdb/ovsdb" +) + +type inMemoryDatabase struct { + databases map[string]*cache.TableCache + models map[string]model.ClientDBModel + logger *logr.Logger + mutex sync.RWMutex +} + +func NewDatabase(models map[string]model.ClientDBModel) dbase.Database { + logger := stdr.NewWithOptions(log.New(os.Stderr, "", log.LstdFlags), stdr.Options{LogCaller: stdr.All}).WithName("database") + return &inMemoryDatabase{ + databases: make(map[string]*cache.TableCache), + models: models, + mutex: sync.RWMutex{}, + logger: &logger, + } +} + +func (db *inMemoryDatabase) NewTransaction(dbName string) dbase.Transaction { + db.mutex.Lock() + defer db.mutex.Unlock() + var model model.DatabaseModel + if database, ok := db.databases[dbName]; ok { + model = database.DatabaseModel() + } + transaction := transaction.NewTransaction(model, dbName, db, db.logger) + return &transaction +} + +func (db *inMemoryDatabase) CreateDatabase(name string, schema ovsdb.DatabaseSchema) error { + db.mutex.Lock() + defer db.mutex.Unlock() + var mo model.ClientDBModel + var ok bool + if mo, ok = db.models[schema.Name]; !ok { + return fmt.Errorf("no db model provided for schema with name %s", name) + } + dbModel, errs := model.NewDatabaseModel(schema, mo) + if len(errs) > 0 { + return fmt.Errorf("failed to create DatabaseModel: %#+v", errs) + } + database, err := cache.NewTableCache(dbModel, nil, nil) + if err != nil { + return err + } + db.databases[name] = database + return nil +} + +func (db *inMemoryDatabase) Exists(name string) bool { + db.mutex.RLock() + defer db.mutex.RUnlock() + _, ok := db.databases[name] + return ok +} + +func (db *inMemoryDatabase) Commit(database string, id uuid.UUID, update dbase.Update) error { + if !db.Exists(database) { + return fmt.Errorf("db does not exist") + } + db.mutex.RLock() + targetDb := db.databases[database] + db.mutex.RUnlock() + + return targetDb.ApplyCacheUpdate(update) +} + +func (db *inMemoryDatabase) CheckIndexes(database string, table string, m model.Model) error { + if !db.Exists(database) { + return nil + } + db.mutex.RLock() + targetDb := db.databases[database] + db.mutex.RUnlock() + targetTable := targetDb.Table(table) + return targetTable.IndexExists(m) +} + +func (db *inMemoryDatabase) List(database, table string, conditions ...ovsdb.Condition) (map[string]model.Model, error) { + if !db.Exists(database) { + return nil, fmt.Errorf("db does not exist") + } + db.mutex.RLock() + targetDb := db.databases[database] + db.mutex.RUnlock() + + targetTable := targetDb.Table(table) + if targetTable == nil { + return nil, fmt.Errorf("table does not exist") + } + + return targetTable.RowsByCondition(conditions) +} + +func (db *inMemoryDatabase) Get(database, table string, uuid string) (model.Model, error) { + if !db.Exists(database) { + return nil, fmt.Errorf("db does not exist") + } + db.mutex.RLock() + targetDb := db.databases[database] + db.mutex.RUnlock() + + targetTable := targetDb.Table(table) + if targetTable == nil { + return nil, fmt.Errorf("table does not exist") + } + return targetTable.Row(uuid), nil +} diff --git a/database/transaction_test.go b/database/inmemory/inmemory_test.go similarity index 70% rename from database/transaction_test.go rename to database/inmemory/inmemory_test.go index 6b4e550e..f2b214ca 100644 --- a/database/transaction_test.go +++ b/database/inmemory/inmemory_test.go @@ -1,4 +1,4 @@ -package database +package inmemory import ( "testing" @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/ovn-org/libovsdb/database" "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" @@ -18,7 +19,7 @@ import ( func TestWaitOpEquals(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) m := mapper.NewMapper(dbModel.Schema) @@ -45,57 +46,57 @@ func TestWaitOpEquals(t *testing.T) { bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) + transaction := db.NewTransaction("Open_vSwitch") - operation := ovsdb.Operation{ - Op: ovsdb.OperationInsert, - Table: "Open_vSwitch", - UUID: ovsUUID, - Row: ovsRow, - } - res, updates := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) - require.Nil(t, err) - - operation = ovsdb.Operation{ - Op: ovsdb.OperationInsert, - Table: "Bridge", - UUID: bridgeUUID, - Row: bridgeRow, + operations := []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Open_vSwitch", + UUIDName: ovsUUID, + Row: ovsRow, + }, + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUIDName: bridgeUUID, + Row: bridgeRow, + }, } - res, update2 := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) - require.Nil(t, err) - - err = updates.Merge(dbModel, *update2) + res, updates := transaction.Transact(operations...) + _, err = checkOperationResults(res, operations...) require.NoError(t, err) - err = db.Commit("Open_vSwitch", uuid.New(), *updates) + + err = db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) timeout := 0 // Attempt to wait for row with name foo to appear - gotResult := transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name"}, - "==", - []ovsdb.Row{{"name": "foo"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.Nil(t, err) + operation := ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name"}, + Until: "==", + Rows: []ovsdb.Row{{"name": "foo"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) // Attempt to wait for 2 rows, where one does not exist - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name"}, - "==", - []ovsdb.Row{{"name": "foo"}, {"name": "blah"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.NotNil(t, err) + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name"}, + Until: "==", + Rows: []ovsdb.Row{{"name": "foo"}, {"name": "blah"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.Error(t, err) extIDs, err := ovsdb.NewOvsMap(map[string]string{ "foo": "bar", @@ -104,48 +105,53 @@ func TestWaitOpEquals(t *testing.T) { }) require.Nil(t, err) // Attempt to wait for a row, with multiple columns specified - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name", "external_ids"}, - "==", - []ovsdb.Row{{"name": "foo", "external_ids": extIDs}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.Nil(t, err) + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name", "external_ids"}, + Until: "==", + Rows: []ovsdb.Row{{"name": "foo", "external_ids": extIDs}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) // Attempt to wait for a row, with multiple columns, but not specified in row filtering - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name", "external_ids"}, - "==", - []ovsdb.Row{{"name": "foo"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.Nil(t, err) + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name", "external_ids"}, + Until: "==", + Rows: []ovsdb.Row{{"name": "foo"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) // Attempt to get something with a non-zero timeout that will fail timeout = 400 - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name", "external_ids"}, - "==", - []ovsdb.Row{{"name": "doesNotExist"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.NotNil(t, err) - + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name", "external_ids"}, + Until: "==", + Rows: []ovsdb.Row{{"name": "doesNotExist"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.Error(t, err) } func TestWaitOpNotEquals(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) m := mapper.NewMapper(dbModel.Schema) @@ -172,57 +178,57 @@ func TestWaitOpNotEquals(t *testing.T) { bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) + transaction := db.NewTransaction("Open_vSwitch") - operation := ovsdb.Operation{ - Op: ovsdb.OperationInsert, - Table: "Open_vSwitch", - UUID: ovsUUID, - Row: ovsRow, - } - res, updates := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) - require.Nil(t, err) - - operation = ovsdb.Operation{ - Op: ovsdb.OperationInsert, - Table: "Bridge", - UUID: bridgeUUID, - Row: bridgeRow, + operations := []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Open_vSwitch", + UUIDName: ovsUUID, + Row: ovsRow, + }, + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUIDName: bridgeUUID, + Row: bridgeRow, + }, } - res, update2 := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) - require.Nil(t, err) - - err = updates.Merge(dbModel, *update2) + res, updates := transaction.Transact(operations...) + _, err = checkOperationResults(res, operations...) require.NoError(t, err) - err = db.Commit("Open_vSwitch", uuid.New(), *updates) + + err = db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) timeout := 0 // Attempt a wait where no entry with name blah should exist - gotResult := transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name"}, - "!=", - []ovsdb.Row{{"name": "blah"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.Nil(t, err) + operation := ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name"}, + Until: "!=", + Rows: []ovsdb.Row{{"name": "blah"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) // Attempt another wait with multiple rows specified, one that would match, and one that doesn't - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name"}, - "!=", - []ovsdb.Row{{"name": "blah"}, {"name": "foo"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.Nil(t, err) + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name"}, + Until: "!=", + Rows: []ovsdb.Row{{"name": "blah"}, {"name": "foo"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) // Attempt another wait where name would match, but ext ids would not match NoMatchExtIDs, err := ovsdb.NewOvsMap(map[string]string{ @@ -230,31 +236,38 @@ func TestWaitOpNotEquals(t *testing.T) { "baz": "quux", "waldo": "is_different", }) - require.Nil(t, err) + require.NoError(t, err) + // Attempt to wait for a row, with multiple columns specified and one is not a match - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name", "external_ids"}, - "!=", - []ovsdb.Row{{"name": "foo", "external_ids": NoMatchExtIDs}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) - require.Nil(t, err) + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name", "external_ids"}, + Until: "!=", + Rows: []ovsdb.Row{{"name": "foo", "external_ids": NoMatchExtIDs}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) // Check to see if a non match takes around the timeout start := time.Now() timeout = 200 - gotResult = transaction.Wait( - "Bridge", - &timeout, - []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, - []string{"name"}, - "!=", - []ovsdb.Row{{"name": "foo"}}, - ) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{gotResult}, []ovsdb.Operation{{Op: "wait"}}) + operation = ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: "Bridge", + Timeout: &timeout, + Where: []ovsdb.Condition{ovsdb.NewCondition("name", ovsdb.ConditionEqual, "foo")}, + Columns: []string{"name"}, + Until: "!=", + Rows: []ovsdb.Row{{"name": "foo"}}, + } + res, _ = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.Error(t, err) + ts := time.Since(start) if ts < time.Duration(timeout)*time.Millisecond { t.Fatalf("Should have taken at least %d milliseconds to return, but it took %d instead", timeout, ts) @@ -265,7 +278,7 @@ func TestWaitOpNotEquals(t *testing.T) { func TestMutateOp(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) m := mapper.NewMapper(dbModel.Schema) @@ -292,42 +305,41 @@ func TestMutateOp(t *testing.T) { bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) + transaction := db.NewTransaction("Open_vSwitch") - operation := ovsdb.Operation{ - Op: ovsdb.OperationInsert, - Table: "Open_vSwitch", - UUID: ovsUUID, - Row: ovsRow, - } - res, updates := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) - require.Nil(t, err) - - operation = ovsdb.Operation{ - Op: ovsdb.OperationInsert, - Table: "Bridge", - UUID: bridgeUUID, - Row: bridgeRow, + operations := []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Open_vSwitch", + UUID: ovsUUID, + Row: ovsRow, + }, + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: bridgeRow, + }, } - res, update2 := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) - require.Nil(t, err) - - err = updates.Merge(dbModel, *update2) + res, updates := transaction.Transact(operations...) + _, err = checkOperationResults(res, operations...) require.NoError(t, err) - err = db.Commit("Open_vSwitch", uuid.New(), *updates) + + err = db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) - operation = ovsdb.Operation{ + operation := ovsdb.Operation{ Op: ovsdb.OperationMutate, Table: "Open_vSwitch", Where: []ovsdb.Condition{ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: ovsUUID})}, Mutations: []ovsdb.Mutation{*ovsdb.NewMutation("bridges", ovsdb.MutateOperationInsert, ovsdb.UUID{GoUUID: bridgeUUID})}, } - gotResult, gotUpdate := transaction.Mutate(&operation) - assert.Equal(t, ovsdb.OperationResult{Count: 1}, gotResult) - err = db.Commit("Open_vSwitch", uuid.New(), *gotUpdate) + res, updates = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) + assert.Equal(t, []*ovsdb.OperationResult{{Count: 1}}, res) + + err = db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) bridgeSet, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: bridgeUUID}}) @@ -349,7 +361,7 @@ func TestMutateOp(t *testing.T) { }, }, }, - }, getTableUpdates(*gotUpdate)) + }, getTableUpdates(updates)) keyDelete, err := ovsdb.NewOvsSet([]string{"foo"}) assert.Nil(t, err) @@ -365,8 +377,10 @@ func TestMutateOp(t *testing.T) { *ovsdb.NewMutation("external_ids", ovsdb.MutateOperationDelete, keyValueDelete), }, } - gotResult, gotUpdate = transaction.Mutate(&operation) - assert.Equal(t, ovsdb.OperationResult{Count: 1}, gotResult) + res, updates = transaction.Transact(operation) + _, err = checkOperationResults(res, operation) + require.NoError(t, err) + assert.Equal(t, []*ovsdb.OperationResult{{Count: 1}}, res) oldExternalIds, _ := ovsdb.NewOvsMap(bridge.ExternalIds) newExternalIds, _ := ovsdb.NewOvsMap(map[string]string{"waldo": "fred"}) @@ -374,9 +388,9 @@ func TestMutateOp(t *testing.T) { assert.Nil(t, err) - gotModify := *getTableUpdates(*gotUpdate)["Bridge"][bridgeUUID].Modify - gotOld := *getTableUpdates(*gotUpdate)["Bridge"][bridgeUUID].Old - gotNew := *getTableUpdates(*gotUpdate)["Bridge"][bridgeUUID].New + gotModify := *getTableUpdates(updates)["Bridge"][bridgeUUID].Modify + gotOld := *getTableUpdates(updates)["Bridge"][bridgeUUID].Old + gotNew := *getTableUpdates(updates)["Bridge"][bridgeUUID].New assert.Equal(t, diffExternalIds, gotModify["external_ids"]) assert.Equal(t, oldExternalIds, gotOld["external_ids"]) assert.Equal(t, newExternalIds, gotNew["external_ids"]) @@ -386,7 +400,7 @@ func TestOvsdbServerInsert(t *testing.T) { t.Skip("need a helper for comparing rows as map elements aren't in same order") dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) m := mapper.NewMapper(dbModel.Schema) @@ -408,7 +422,7 @@ func TestOvsdbServerInsert(t *testing.T) { bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) + transaction := db.NewTransaction("Open_vSwitch") operation := ovsdb.Operation{ Op: ovsdb.OperationInsert, @@ -416,11 +430,11 @@ func TestOvsdbServerInsert(t *testing.T) { UUID: bridgeUUID, Row: bridgeRow, } - res, updates := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) + res, updates := transaction.Transact(operation) + _, err = checkOperationResults(res, operation) require.NoError(t, err) - err = db.Commit("Open_vSwitch", uuid.New(), *updates) + err = db.Commit("Open_vSwitch", uuid.New(), updates) assert.NoError(t, err) bridge.UUID = bridgeUUID @@ -440,7 +454,7 @@ func TestOvsdbServerInsert(t *testing.T) { func TestOvsdbServerUpdate(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) m := mapper.NewMapper(dbModel.Schema) @@ -461,7 +475,7 @@ func TestOvsdbServerUpdate(t *testing.T) { bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) + transaction := db.NewTransaction("Open_vSwitch") operation := ovsdb.Operation{ Op: ovsdb.OperationInsert, @@ -469,11 +483,11 @@ func TestOvsdbServerUpdate(t *testing.T) { UUID: bridgeUUID, Row: bridgeRow, } - res, updates := transaction.Insert(&operation) - _, err = ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "insert"}}) + res, updates := transaction.Transact(operation) + _, err = checkOperationResults(res, operation) require.NoError(t, err) - err = db.Commit("Open_vSwitch", uuid.New(), *updates) + err = db.Commit("Open_vSwitch", uuid.New(), updates) assert.NoError(t, err) halloween, _ := ovsdb.NewOvsSet([]string{"halloween"}) @@ -522,6 +536,7 @@ func TestOvsdbServerUpdate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + transaction := db.NewTransaction("Open_vSwitch") op := ovsdb.Operation{ Op: ovsdb.OperationUpdate, Table: "Bridge", @@ -530,8 +545,8 @@ func TestOvsdbServerUpdate(t *testing.T) { }}, Row: tt.row, } - res, updates := transaction.Update(&op) - errs, err := ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "update"}}) + res, updates := transaction.Transact(op) + errs, err := checkOperationResults(res, op) require.NoErrorf(t, err, "%+v", errs) bridge.UUID = bridgeUUID @@ -539,7 +554,7 @@ func TestOvsdbServerUpdate(t *testing.T) { assert.NoError(t, err) br := row.(*BridgeType) assert.NotEqual(t, br, bridgeRow) - assert.Equal(t, tt.expected.Modify, getTableUpdates(*updates)["Bridge"][bridgeUUID].Modify) + assert.Equal(t, tt.expected.Modify, getTableUpdates(updates)["Bridge"][bridgeUUID].Modify) }) } } @@ -547,7 +562,7 @@ func TestOvsdbServerUpdate(t *testing.T) { func TestMultipleOps(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) @@ -578,8 +593,8 @@ func TestMultipleOps(t *testing.T) { } ops = append(ops, op) - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) - results, _ := transaction.Transact(ops) + transaction := db.NewTransaction("Open_vSwitch") + results, _ := transaction.Transact(ops...) assert.Len(t, results, len(ops)) assert.NotNil(t, results[0]) assert.Empty(t, results[0].Error) @@ -616,7 +631,7 @@ func TestMultipleOps(t *testing.T) { } ops = append(ops, op2) - results, updates := transaction.Transact(ops) + results, updates := transaction.Transact(ops...) require.Len(t, results, len(ops)) for _, result := range results { assert.Empty(t, result.Error) @@ -659,11 +674,9 @@ func TestOvsdbServerDbDoesNotExist(t *testing.T) { if err != nil { t.Fatal(err) } - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": defDB}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": defDB}) err = db.CreateDatabase("Open_vSwitch", schema) require.NoError(t, err) - dbModel, errs := model.NewDatabaseModel(schema, defDB) - require.Empty(t, errs) ops := []ovsdb.Operation{ { @@ -690,8 +703,8 @@ func TestOvsdbServerDbDoesNotExist(t *testing.T) { }, } - transaction := NewTransaction(dbModel, "nonexsitent_db", db, nil) - res, _ := transaction.Transact(ops) + transaction := db.NewTransaction("nonexsitent_db") + res, _ := transaction.Transact(ops...) assert.Len(t, res, len(ops)) assert.Equal(t, "database does not exist", res[0].Error) assert.Nil(t, res[1]) @@ -700,7 +713,7 @@ func TestOvsdbServerDbDoesNotExist(t *testing.T) { func TestCheckIndexes(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - db := NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) require.NoError(t, err) @@ -737,8 +750,8 @@ func TestCheckIndexes(t *testing.T) { }, } - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) - results, updates := transaction.Transact(ops) + transaction := db.NewTransaction("Open_vSwitch") + results, updates := transaction.Transact(ops...) require.Len(t, results, len(ops)) for _, result := range results { assert.Equal(t, "", result.Error) @@ -871,9 +884,9 @@ func TestCheckIndexes(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - transaction := NewTransaction(dbModel, "Open_vSwitch", db, nil) + transaction := db.NewTransaction("Open_vSwitch") ops := tt.ops() - results, _ := transaction.Transact(ops) + results, _ := transaction.Transact(ops...) var err string for _, result := range results { if result.Error != "" { @@ -891,7 +904,7 @@ func TestCheckIndexes(t *testing.T) { } } -func getTableUpdates(update Update) ovsdb.TableUpdates2 { +func getTableUpdates(update database.Update) ovsdb.TableUpdates2 { tus := ovsdb.TableUpdates2{} tables := update.GetUpdatedTables() for _, table := range tables { @@ -904,3 +917,11 @@ func getTableUpdates(update Update) ovsdb.TableUpdates2 { } return tus } + +func checkOperationResults(result []*ovsdb.OperationResult, ops ...ovsdb.Operation) ([]ovsdb.OperationError, error) { + r := make([]ovsdb.OperationResult, len(result)) + for i := range result { + r[i] = *result[i] + } + return ovsdb.CheckOperationResults(r, ops) +} diff --git a/database/transaction/doc.go b/database/transaction/doc.go new file mode 100644 index 00000000..36d35aa7 --- /dev/null +++ b/database/transaction/doc.go @@ -0,0 +1,4 @@ +/* +Package transaction provides a transaction implementation +*/ +package transaction diff --git a/database/errors.go b/database/transaction/errors.go similarity index 95% rename from database/errors.go rename to database/transaction/errors.go index 979752c0..35e47c72 100644 --- a/database/errors.go +++ b/database/transaction/errors.go @@ -1,4 +1,4 @@ -package database +package transaction import ( "fmt" diff --git a/database/transaction.go b/database/transaction/transaction.go similarity index 97% rename from database/transaction.go rename to database/transaction/transaction.go index 7e69a555..fdecdcc0 100644 --- a/database/transaction.go +++ b/database/transaction/transaction.go @@ -1,4 +1,4 @@ -package database +package transaction import ( "fmt" @@ -8,6 +8,7 @@ import ( "github.com/go-logr/logr" "github.com/google/uuid" "github.com/ovn-org/libovsdb/cache" + "github.com/ovn-org/libovsdb/database" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/ovn-org/libovsdb/updates" @@ -19,11 +20,11 @@ type Transaction struct { DeletedRows map[string]struct{} Model model.DatabaseModel DbName string - Database Database + Database database.Database logger *logr.Logger } -func NewTransaction(model model.DatabaseModel, dbName string, database Database, logger *logr.Logger) Transaction { +func NewTransaction(model model.DatabaseModel, dbName string, database database.Database, logger *logr.Logger) Transaction { if logger != nil { l := logger.WithName("transaction") logger = &l @@ -39,7 +40,7 @@ func NewTransaction(model model.DatabaseModel, dbName string, database Database, } } -func (t *Transaction) Transact(operations []ovsdb.Operation) ([]*ovsdb.OperationResult, Update) { +func (t *Transaction) Transact(operations ...ovsdb.Operation) ([]*ovsdb.OperationResult, database.Update) { results := make([]*ovsdb.OperationResult, len(operations), len(operations)+1) update := updates.ModelUpdates{} diff --git a/example/ovsdb-server/main.go b/example/ovsdb-server/main.go index e958280c..5798e7d2 100644 --- a/example/ovsdb-server/main.go +++ b/example/ovsdb-server/main.go @@ -13,7 +13,7 @@ import ( "time" "github.com/ovn-org/libovsdb/client" - "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/database/inmemory" "github.com/ovn-org/libovsdb/example/vswitchd" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" @@ -58,7 +58,7 @@ func main() { log.Fatal(err) } - ovsDB := database.NewInMemoryDatabase(map[string]model.ClientDBModel{ + ovsDB := inmemory.NewDatabase(map[string]model.ClientDBModel{ schema.Name: clientDBModel, }) diff --git a/server/server.go b/server/server.go index 0100d766..ec60ea5d 100644 --- a/server/server.go +++ b/server/server.go @@ -218,11 +218,8 @@ func (o *OvsdbServer) Transact(client *rpc2.Client, args []json.RawMessage, repl } func (o *OvsdbServer) transact(name string, operations []ovsdb.Operation) ([]*ovsdb.OperationResult, database.Update) { - o.modelsMutex.Lock() - dbModel := o.models[name] - o.modelsMutex.Unlock() - transaction := database.NewTransaction(dbModel, name, o.db, &o.logger) - return transaction.Transact(operations) + transaction := o.db.NewTransaction(name) + return transaction.Transact(operations...) } // Cancel cancels the last transaction @@ -255,21 +252,20 @@ func (o *OvsdbServer) Monitor(client *rpc2.Client, args []json.RawMessage, reply } } - o.modelsMutex.Lock() - dbModel := o.models[db] - o.modelsMutex.Unlock() - transaction := database.NewTransaction(dbModel, db, o.db, &o.logger) + transaction := o.db.NewTransaction(db) tableUpdates := make(ovsdb.TableUpdates) for t, request := range request { - rows := transaction.Select(t, nil, request.Columns) - if len(rows.Rows) == 0 { + op := ovsdb.Operation{Op: ovsdb.OperationSelect, Table: t, Columns: request.Columns} + result, _ := transaction.Transact(op) + if len(result) == 0 || len(result[0].Rows) == 0 { continue } - tableUpdates[t] = make(ovsdb.TableUpdate, len(rows.Rows)) - for i := range rows.Rows { - uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID - tableUpdates[t][uuid] = &ovsdb.RowUpdate{New: &rows.Rows[i]} + rows := result[0].Rows + tableUpdates[t] = make(ovsdb.TableUpdate, len(rows)) + for i := range rows { + uuid := rows[i]["_uuid"].(ovsdb.UUID).GoUUID + tableUpdates[t][uuid] = &ovsdb.RowUpdate{New: &rows[i]} } } *reply = tableUpdates @@ -302,21 +298,20 @@ func (o *OvsdbServer) MonitorCond(client *rpc2.Client, args []json.RawMessage, r } } - o.modelsMutex.Lock() - dbModel := o.models[db] - o.modelsMutex.Unlock() - transaction := database.NewTransaction(dbModel, db, o.db, &o.logger) + transaction := o.db.NewTransaction(db) tableUpdates := make(ovsdb.TableUpdates2) for t, request := range request { - rows := transaction.Select(t, nil, request.Columns) - if len(rows.Rows) == 0 { + op := ovsdb.Operation{Op: ovsdb.OperationSelect, Table: t, Columns: request.Columns} + result, _ := transaction.Transact(op) + if len(result) == 0 || len(result[0].Rows) == 0 { continue } - tableUpdates[t] = make(ovsdb.TableUpdate2, len(rows.Rows)) - for i := range rows.Rows { - uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID - tableUpdates[t][uuid] = &ovsdb.RowUpdate2{Initial: &rows.Rows[i]} + rows := result[0].Rows + tableUpdates[t] = make(ovsdb.TableUpdate2, len(rows)) + for i := range rows { + uuid := rows[i]["_uuid"].(ovsdb.UUID).GoUUID + tableUpdates[t][uuid] = &ovsdb.RowUpdate2{Initial: &rows[i]} } } *reply = tableUpdates @@ -349,21 +344,20 @@ func (o *OvsdbServer) MonitorCondSince(client *rpc2.Client, args []json.RawMessa } } - o.modelsMutex.Lock() - dbModel := o.models[db] - o.modelsMutex.Unlock() - transaction := database.NewTransaction(dbModel, db, o.db, &o.logger) + transaction := o.db.NewTransaction(db) tableUpdates := make(ovsdb.TableUpdates2) for t, request := range request { - rows := transaction.Select(t, nil, request.Columns) - if len(rows.Rows) == 0 { + op := ovsdb.Operation{Op: ovsdb.OperationSelect, Table: t, Columns: request.Columns} + result, _ := transaction.Transact(op) + if len(result) == 0 || len(result[0].Rows) == 0 { continue } - tableUpdates[t] = make(ovsdb.TableUpdate2, len(rows.Rows)) - for i := range rows.Rows { - uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID - tableUpdates[t][uuid] = &ovsdb.RowUpdate2{Initial: &rows.Rows[i]} + rows := result[0].Rows + tableUpdates[t] = make(ovsdb.TableUpdate2, len(rows)) + for i := range rows { + uuid := rows[i]["_uuid"].(ovsdb.UUID).GoUUID + tableUpdates[t][uuid] = &ovsdb.RowUpdate2{Initial: &rows[i]} } } *reply = ovsdb.MonitorCondSinceReply{Found: false, LastTransactionID: "00000000-0000-0000-000000000000", Updates: tableUpdates} diff --git a/server/server_integration_test.go b/server/server_integration_test.go index c070ca8f..e9f3d8dd 100644 --- a/server/server_integration_test.go +++ b/server/server_integration_test.go @@ -12,7 +12,7 @@ import ( "github.com/ovn-org/libovsdb/cache" "github.com/ovn-org/libovsdb/client" - "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/database/inmemory" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/assert" @@ -24,7 +24,7 @@ import ( func buildTestServerAndClient(t *testing.T) (client.Client, func()) { dbModel, err := GetModel() require.NoError(t, err) - ovsDB := database.NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + ovsDB := inmemory.NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) schema := dbModel.Schema defDB := dbModel.Client() diff --git a/server/server_test.go b/server/server_test.go index 36cf4d87..77d1a597 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/google/uuid" - "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/database/inmemory" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/assert" @@ -17,7 +17,7 @@ import ( func TestOvsdbServerMonitor(t *testing.T) { dbModel, err := GetModel() require.NoError(t, err) - ovsDB := database.NewInMemoryDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + ovsDB := inmemory.NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) schema := dbModel.Schema o, err := NewOvsdbServer(ovsDB, dbModel) @@ -65,8 +65,8 @@ func TestOvsdbServerMonitor(t *testing.T) { Row: ovsdb.Row{"name": "quux"}, }, } - transaction := database.NewTransaction(dbModel, "Open_vSwitch", o.db, &o.logger) - _, updates := transaction.Transact(operations) + transaction := ovsDB.NewTransaction("Open_vSwitch") + _, updates := transaction.Transact(operations...) err = o.db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) From 68876d1bb710420a0e3dd8c6a426cd723edf5378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Thu, 21 Dec 2023 02:05:05 +0000 Subject: [PATCH 05/13] Remove references from test schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once we introduce referential integrity, existing tests would start to fail as they are not accounting for it. So its better to remove the references from the test schema which is something that current tests are not leveraging, and then expand the schema with other references that the referenctial integrity tests would leverage. Signed-off-by: Jaime Caamaño Ruiz --- database/inmemory/inmemory_test.go | 3 +-- test/test_data.go | 13 +++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/database/inmemory/inmemory_test.go b/database/inmemory/inmemory_test.go index f2b214ca..b26ec607 100644 --- a/database/inmemory/inmemory_test.go +++ b/database/inmemory/inmemory_test.go @@ -283,7 +283,6 @@ func TestMutateOp(t *testing.T) { require.NoError(t, err) m := mapper.NewMapper(dbModel.Schema) - ovsUUID := uuid.NewString() bridgeUUID := uuid.NewString() ovs := OvsType{} @@ -311,7 +310,6 @@ func TestMutateOp(t *testing.T) { { Op: ovsdb.OperationInsert, Table: "Open_vSwitch", - UUID: ovsUUID, Row: ovsRow, }, { @@ -328,6 +326,7 @@ func TestMutateOp(t *testing.T) { err = db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) + ovsUUID := res[0].UUID.GoUUID operation := ovsdb.Operation{ Op: ovsdb.OperationMutate, Table: "Open_vSwitch", diff --git a/test/test_data.go b/test/test_data.go index 6be6b8b2..20686c17 100644 --- a/test/test_data.go +++ b/test/test_data.go @@ -8,6 +8,8 @@ import ( "github.com/ovn-org/libovsdb/ovsdb" ) +// Note that this schema is not strictly a subset of the real OVS schema. It has +// some small variations that allow to effectively test some OVSDB RFC features const schema = ` { "name": "Open_vSwitch", @@ -18,15 +20,13 @@ const schema = ` "bridges": { "type": { "key": { - "type": "uuid", - "refTable": "Bridge" + "type": "uuid" }, "min": 0, "max": "unlimited" } } }, - "isRoot": true, "maxRows": 1 }, "Bridge": { @@ -49,8 +49,7 @@ const schema = ` "ports": { "type": { "key": { - "type": "uuid", - "refTable": "Port" + "type": "uuid" }, "min": 0, "max": "unlimited" @@ -104,8 +103,7 @@ const schema = ` "bridge": { "type": { "key": { - "type": "uuid", - "refTable": "Bridge" + "type": "uuid" }, "min": 1, "max": 1 @@ -120,7 +118,6 @@ const schema = ` } } }, - "isRoot": true, "indexes": [ [ "id", From a6812b2a98e5c0915356b0a02c66d1ed691764c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 9 Jan 2024 15:04:31 +0000 Subject: [PATCH 06/13] schema: add isRoot flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the isRoot flag to the schema which will be used for referential integrity Signed-off-by: Jaime Caamaño Ruiz --- ovsdb/schema.go | 33 +++++++++++++++++++++++++++++---- ovsdb/serverdb/model.go | 3 ++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/ovsdb/schema.go b/ovsdb/schema.go index cf80aa50..99824da7 100644 --- a/ovsdb/schema.go +++ b/ovsdb/schema.go @@ -12,9 +12,10 @@ import ( // DatabaseSchema is a database schema according to RFC7047 type DatabaseSchema struct { - Name string `json:"name"` - Version string `json:"version"` - Tables map[string]TableSchema `json:"tables"` + Name string `json:"name"` + Version string `json:"version"` + Tables map[string]TableSchema `json:"tables"` + allTablesRoot *bool } // UUIDColumn is a static column that represents the _uuid column, common to all tables @@ -30,6 +31,29 @@ func (schema DatabaseSchema) Table(tableName string) *TableSchema { return nil } +// IsRoot whether a table is root or not +func (schema DatabaseSchema) IsRoot(tableName string) (bool, error) { + t := schema.Table(tableName) + if t == nil { + return false, fmt.Errorf("Table %s not in schame", tableName) + } + if schema.allTablesRoot == nil { + allTablesRoot := true + for _, tSchema := range schema.Tables { + // As per RFC7047, for compatibility with schemas created before + // "isRoot" was introduced, if "isRoot" is omitted or false in every + // in a given , then every table is part + // of the root set. + if tSchema.IsRoot { + allTablesRoot = false + break + } + } + schema.allTablesRoot = &allTablesRoot + } + return *schema.allTablesRoot || t.IsRoot, nil +} + // Print will print the contents of the DatabaseSchema func (schema DatabaseSchema) Print(w io.Writer) { fmt.Fprintf(w, "%s, (%s)\n", schema.Name, schema.Version) @@ -104,6 +128,7 @@ func (schema DatabaseSchema) ValidateOperations(operations ...Operation) bool { type TableSchema struct { Columns map[string]*ColumnSchema `json:"columns"` Indexes [][]string `json:"indexes,omitempty"` + IsRoot bool `json:"isRoot,omitempty"` } // Column returns the Column object for a specific column name @@ -124,7 +149,7 @@ of this library, we define an ExtendedType that includes all possible column typ atomic fields). */ -//ExtendedType includes atomic types as defined in the RFC plus Enum, Map and Set +// ExtendedType includes atomic types as defined in the RFC plus Enum, Map and Set type ExtendedType = string // RefType is used to define the possible RefTypes diff --git a/ovsdb/serverdb/model.go b/ovsdb/serverdb/model.go index c8e5cec8..3c117faa 100644 --- a/ovsdb/serverdb/model.go +++ b/ovsdb/serverdb/model.go @@ -83,7 +83,8 @@ var schema = `{ "max": 1 } } - } + }, + "isRoot": true } } }` From e1d2cd1e8d568ec417606f358acc2c7dfd08a094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 9 Jan 2024 15:07:20 +0000 Subject: [PATCH 07/13] server: implement referential integrity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement referential integrity and garbage collection as described in RFC7047. This is achieved keeping track of inverse references. A reference tracker is used in the transaction to keep these references updated, to check for referential integrity related violations and to add additional updates to the transaction resulting from reference garbage collection. The inverse references are stored in the database on commit. The updates resulting from reference garbage collection are also sent to any monitoring clients as expected. Signed-off-by: Jaime Caamaño Ruiz --- database/database.go | 2 + database/inmemory/inmemory.go | 22 +- database/references.go | 71 + database/transaction/transaction.go | 61 +- ovsdb/error.go | 4 + server/monitor_test.go | 4 +- updates/references.go | 798 +++++++++++ updates/references_test.go | 2050 +++++++++++++++++++++++++++ updates/updates.go | 12 +- 9 files changed, 3011 insertions(+), 13 deletions(-) create mode 100644 database/references.go create mode 100644 updates/references.go create mode 100644 updates/references_test.go diff --git a/database/database.go b/database/database.go index f41db7d1..12f1222f 100644 --- a/database/database.go +++ b/database/database.go @@ -15,6 +15,7 @@ type Database interface { CheckIndexes(database string, table string, m model.Model) error List(database, table string, conditions ...ovsdb.Condition) (map[string]model.Model, error) Get(database, table string, uuid string) (model.Model, error) + GetReferences(database, table, row string) (References, error) } // Transaction abstracts a database transaction that can generate database @@ -28,4 +29,5 @@ type Update interface { GetUpdatedTables() []string ForEachModelUpdate(table string, do func(uuid string, old, new model.Model) error) error ForEachRowUpdate(table string, do func(uuid string, row ovsdb.RowUpdate2) error) error + ForReferenceUpdates(do func(references References) error) error } diff --git a/database/inmemory/inmemory.go b/database/inmemory/inmemory.go index 618d99b8..6c1dce9e 100644 --- a/database/inmemory/inmemory.go +++ b/database/inmemory/inmemory.go @@ -19,6 +19,7 @@ import ( type inMemoryDatabase struct { databases map[string]*cache.TableCache models map[string]model.ClientDBModel + references map[string]dbase.References logger *logr.Logger mutex sync.RWMutex } @@ -28,6 +29,7 @@ func NewDatabase(models map[string]model.ClientDBModel) dbase.Database { return &inMemoryDatabase{ databases: make(map[string]*cache.TableCache), models: models, + references: make(map[string]dbase.References), mutex: sync.RWMutex{}, logger: &logger, } @@ -61,6 +63,7 @@ func (db *inMemoryDatabase) CreateDatabase(name string, schema ovsdb.DatabaseSch return err } db.databases[name] = database + db.references[name] = make(dbase.References) return nil } @@ -79,7 +82,15 @@ func (db *inMemoryDatabase) Commit(database string, id uuid.UUID, update dbase.U targetDb := db.databases[database] db.mutex.RUnlock() - return targetDb.ApplyCacheUpdate(update) + err := targetDb.ApplyCacheUpdate(update) + if err != nil { + return err + } + + return update.ForReferenceUpdates(func(references dbase.References) error { + db.references[database].UpdateReferences(references) + return nil + }) } func (db *inMemoryDatabase) CheckIndexes(database string, table string, m model.Model) error { @@ -123,3 +134,12 @@ func (db *inMemoryDatabase) Get(database, table string, uuid string) (model.Mode } return targetTable.Row(uuid), nil } + +func (db *inMemoryDatabase) GetReferences(database, table, row string) (dbase.References, error) { + if !db.Exists(database) { + return nil, fmt.Errorf("db does not exist") + } + db.mutex.RLock() + defer db.mutex.RUnlock() + return db.references[database].GetReferences(table, row), nil +} diff --git a/database/references.go b/database/references.go new file mode 100644 index 00000000..d8181a7a --- /dev/null +++ b/database/references.go @@ -0,0 +1,71 @@ +package database + +// References tracks the references to rows from other rows at specific +// locations in the schema. +type References map[ReferenceSpec]Reference + +// ReferenceSpec specifies details about where in the schema a reference occurs. +type ReferenceSpec struct { + // ToTable is the table of the row to which the reference is made + ToTable string + + // FromTable is the table of the row from which the reference is made + FromTable string + + // FromColumn is the column of the row from which the reference is made + FromColumn string + + // FromValue flags if the reference is made on a map key or map value when + // the column is a map + FromValue bool +} + +// Reference maps the UUIDs of rows to which the reference is made to the +// rows it is made from +type Reference map[string][]string + +// GetReferences gets references to a row +func (rs References) GetReferences(table, uuid string) References { + refs := References{} + for spec, values := range rs { + if spec.ToTable != table { + continue + } + if _, ok := values[uuid]; ok { + refs[spec] = Reference{uuid: values[uuid]} + } + } + return refs +} + +// UpdateReferences updates the references with the provided ones. Dangling +// references, that is, the references of rows that are no longer referenced +// from anywhere, are cleaned up. +func (rs References) UpdateReferences(other References) { + for spec, otherRefs := range other { + for to, from := range otherRefs { + rs.updateReference(spec, to, from) + } + } +} + +// updateReference updates the references to a row at a specific location in the +// schema +func (rs References) updateReference(spec ReferenceSpec, to string, from []string) { + thisRefs, ok := rs[spec] + if !ok && len(from) > 0 { + // add references from a previously untracked location + rs[spec] = Reference{to: from} + return + } + if len(from) > 0 { + // replace references to this row at this specific location + thisRefs[to] = from + return + } + // otherwise remove previously tracked references + delete(thisRefs, to) + if len(thisRefs) == 0 { + delete(rs, spec) + } +} diff --git a/database/transaction/transaction.go b/database/transaction/transaction.go index fdecdcc0..69736d00 100644 --- a/database/transaction/transaction.go +++ b/database/transaction/transaction.go @@ -47,14 +47,14 @@ func (t *Transaction) Transact(operations ...ovsdb.Operation) ([]*ovsdb.Operatio if !t.Database.Exists(t.DbName) { r := ovsdb.ResultFromError(fmt.Errorf("database does not exist")) results[0] = &r - return results, nil + return results, updates.NewDatabaseUpdate(update, nil) } err := t.initializeCache() if err != nil { r := ovsdb.ResultFromError(err) results[0] = &r - return results, nil + return results, updates.NewDatabaseUpdate(update, nil) } // Every Insert operation must have a UUID @@ -70,7 +70,7 @@ func (t *Transaction) Transact(operations ...ovsdb.Operation) ([]*ovsdb.Operatio if err != nil { r := ovsdb.ResultFromError(err) results[0] = &r - return results, nil + return results, updates.NewDatabaseUpdate(update, nil) } var r ovsdb.OperationResult @@ -124,12 +124,29 @@ func (t *Transaction) Transact(operations ...ovsdb.Operation) ([]*ovsdb.Operatio // if an operation failed, no need to do any further validation if r.Error != "" { - return results, update + return results, updates.NewDatabaseUpdate(update, nil) } // if there is no updates, no need to do any further validation if len(update.GetUpdatedTables()) == 0 { - return results, update + return results, updates.NewDatabaseUpdate(update, nil) + } + + // check & update references + update, refUpdates, refs, err := updates.ProcessReferences(t.Model, t.Database, update) + if err != nil { + r = ovsdb.ResultFromError(err) + results = append(results, &r) + return results, updates.NewDatabaseUpdate(update, refs) + } + + // apply updates resulting from referential integrity to the transaction + // caches so they are accounted for when checking index constraints + err = t.applyReferenceUpdates(refUpdates) + if err != nil { + r = ovsdb.ResultFromError(err) + results = append(results, &r) + return results, updates.NewDatabaseUpdate(update, refs) } // check index constraints @@ -142,9 +159,41 @@ func (t *Transaction) Transact(operations ...ovsdb.Operation) ([]*ovsdb.Operatio r := ovsdb.ResultFromError(err) results = append(results, &r) } + + return results, updates.NewDatabaseUpdate(update, refs) } - return results, update + return results, updates.NewDatabaseUpdate(update, refs) +} + +func (t *Transaction) applyReferenceUpdates(update updates.ModelUpdates) error { + tables := update.GetUpdatedTables() + for _, table := range tables { + err := update.ForEachModelUpdate(table, func(uuid string, old, new model.Model) error { + // track deleted rows due to reference updates + if old != nil && new == nil { + t.DeletedRows[uuid] = struct{}{} + } + // warm the cache with updated and deleted rows due to reference + // updates + if old != nil && !t.Cache.Table(table).HasRow(uuid) { + row, err := t.Database.Get(t.DbName, table, uuid) + if err != nil { + return err + } + err = t.Cache.Table(table).Create(uuid, row, false) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return err + } + } + // apply reference updates to the cache + return t.Cache.ApplyCacheUpdate(update) } func (t *Transaction) initializeCache() error { diff --git a/ovsdb/error.go b/ovsdb/error.go index 0803894e..4a85c541 100644 --- a/ovsdb/error.go +++ b/ovsdb/error.go @@ -128,6 +128,10 @@ type ReferentialIntegrityViolation struct { operation *Operation } +func NewReferentialIntegrityViolation(details string) *ReferentialIntegrityViolation { + return &ReferentialIntegrityViolation{details: details} +} + // Error implements the error interface func (e *ReferentialIntegrityViolation) Error() string { msg := referentialIntegrityViolation diff --git a/server/monitor_test.go b/server/monitor_test.go index deda5ecc..27f2ed84 100644 --- a/server/monitor_test.go +++ b/server/monitor_test.go @@ -90,7 +90,7 @@ func TestMonitorFilter(t *testing.T) { assert.NoError(t, err) } } - tu := monitor.filter2(update) + tu := monitor.filter2(updates.NewDatabaseUpdate(update, nil)) assert.Equal(t, tt.expected, tu) }) } @@ -175,7 +175,7 @@ func TestMonitorFilter2(t *testing.T) { assert.NoError(t, err) } } - tu := monitor.filter2(update) + tu := monitor.filter2(updates.NewDatabaseUpdate(update, nil)) assert.Equal(t, tt.expected, tu) }) } diff --git a/updates/references.go b/updates/references.go new file mode 100644 index 00000000..8c0c37ac --- /dev/null +++ b/updates/references.go @@ -0,0 +1,798 @@ +package updates + +import ( + "fmt" + "reflect" + + "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/model" + "github.com/ovn-org/libovsdb/ovsdb" +) + +// ReferenceProvider should be implemented by a database that tracks references +type ReferenceProvider interface { + // GetReferences provides the references to the provided row + GetReferences(database, table, uuid string) (database.References, error) + // Get provides the corresponding model + Get(database, table string, uuid string) (model.Model, error) +} + +// DatabaseUpdate bundles updates together with the updated +// reference information +type DatabaseUpdate struct { + ModelUpdates + referenceUpdates database.References +} + +func (u DatabaseUpdate) ForReferenceUpdates(do func(references database.References) error) error { + refsCopy := database.References{} + // since refsCopy is empty, this will just copy everything + applyReferenceModifications(refsCopy, u.referenceUpdates) + return do(refsCopy) +} + +func NewDatabaseUpdate(updates ModelUpdates, references database.References) DatabaseUpdate { + return DatabaseUpdate{ + ModelUpdates: updates, + referenceUpdates: references, + } +} + +// ProcessReferences tracks referential integrity for the provided set of +// updates. It returns an updated set of updates which includes additional +// updates and updated references as a result of the reference garbage +// collection described in RFC7047. These additional updates resulting from the +// reference garbage collection are also returned separately. Any constraint or +// referential integrity violation is returned as an error. +func ProcessReferences(dbModel model.DatabaseModel, provider ReferenceProvider, updates ModelUpdates) (ModelUpdates, ModelUpdates, database.References, error) { + referenceTracker := newReferenceTracker(dbModel, provider) + return referenceTracker.processReferences(updates) +} + +type referenceTracker struct { + dbModel model.DatabaseModel + provider ReferenceProvider + + // updates that are being processed + updates ModelUpdates + + // references are the updated references by the set of updates processed + references database.References + + // helper maps to track the rows that we are processing and their tables + tracked map[string]string + added map[string]string + deleted map[string]string +} + +func newReferenceTracker(dbModel model.DatabaseModel, provider ReferenceProvider) *referenceTracker { + return &referenceTracker{ + dbModel: dbModel, + provider: provider, + } +} + +func (rt *referenceTracker) processReferences(updates ModelUpdates) (ModelUpdates, ModelUpdates, database.References, error) { + rt.updates = updates + rt.tracked = make(map[string]string) + rt.added = make(map[string]string) + rt.deleted = make(map[string]string) + rt.references = make(database.References) + + referenceUpdates, err := rt.processReferencesLoop(updates) + if err != nil { + return ModelUpdates{}, ModelUpdates{}, nil, err + } + + // merge the updates generated from reference tracking into the main updates + err = updates.Merge(rt.dbModel, referenceUpdates) + if err != nil { + return ModelUpdates{}, ModelUpdates{}, nil, err + } + + return updates, referenceUpdates, rt.references, nil +} + +func (rt *referenceTracker) processReferencesLoop(updates ModelUpdates) (ModelUpdates, error) { + referenceUpdates := ModelUpdates{} + + // references can be transitive and deleting them can lead to further + // references having to be removed so loop until there are no updates to be + // made + for len(updates.updates) > 0 { + // update the references from the updates + err := rt.processModelUpdates(updates) + if err != nil { + return ModelUpdates{}, err + } + + // process strong reference integrity + updates, err = rt.processStrongReferences() + if err != nil { + return ModelUpdates{}, err + } + + // process weak reference integrity + weakUpdates, err := rt.processWeakReferences() + if err != nil { + return ModelUpdates{}, err + } + + // merge strong and weak reference updates + err = updates.Merge(rt.dbModel, weakUpdates) + if err != nil { + return ModelUpdates{}, err + } + + // merge updates from this iteration to the overall reference updates + err = referenceUpdates.Merge(rt.dbModel, updates) + if err != nil { + return ModelUpdates{}, err + } + } + + return referenceUpdates, nil +} + +// processModelUpdates keeps track of the updated references by a set of updates +func (rt *referenceTracker) processModelUpdates(updates ModelUpdates) error { + tables := updates.GetUpdatedTables() + for _, table := range tables { + err := updates.ForEachRowUpdate(table, func(uuid string, row ovsdb.RowUpdate2) error { + return rt.processRowUpdate(table, uuid, &row) + }) + if err != nil { + return err + } + } + return nil +} + +// processRowUpdate keeps track of the updated references by a given row update +func (rt *referenceTracker) processRowUpdate(table, uuid string, row *ovsdb.RowUpdate2) error { + + // getReferencesFromRowModify extracts updated references from the + // modifications. Following the same strategy as the modify field of Update2 + // notification, it will extract a difference, that is, both old removed + // references and new added references are extracted. This difference will + // then be applied to currently tracked references to come up with the + // updated references. + + // For more info on the modify field of Update2 notification and the + // strategy used to apply differences, check + // https://docs.openvswitch.org/en/latest/ref/ovsdb-server.7/#update2-notification + + var updateRefs database.References + switch { + case row.Delete != nil: + rt.deleted[uuid] = table + updateRefs = getReferenceModificationsFromRow(&rt.dbModel, table, uuid, row.Old, row.Old) + case row.Modify != nil: + updateRefs = getReferenceModificationsFromRow(&rt.dbModel, table, uuid, row.Modify, row.Old) + case row.Insert != nil: + if !isRoot(&rt.dbModel, table) { + // track rows added that are not part of the root set, we might need + // to delete those later + rt.added[uuid] = table + rt.tracked[uuid] = table + } + updateRefs = getReferenceModificationsFromRow(&rt.dbModel, table, uuid, row.Insert, nil) + } + + // (lazy) initialize existing references to the same rows from the database + for spec, refs := range updateRefs { + for to := range refs { + err := rt.initReferences(spec.ToTable, to) + if err != nil { + return err + } + } + } + + // apply the reference modifications to the initialized references + applyReferenceModifications(rt.references, updateRefs) + + return nil +} + +// processStrongReferences adds delete operations for rows that are not part of +// the root set and are no longer strongly referenced. Returns a referential +// integrity violation if a nonexistent row is strongly referenced or a strongly +// referenced row has been deleted. +func (rt *referenceTracker) processStrongReferences() (ModelUpdates, error) { + // make sure that we are tracking the references to the deleted rows + err := rt.initReferencesOfDeletedRows() + if err != nil { + return ModelUpdates{}, err + } + + // track if rows are referenced or not + isReferenced := map[string]bool{} + + // go over the updated references + for spec, refs := range rt.references { + + // we only care about strong references + if !isStrong(&rt.dbModel, spec) { + continue + } + + for to, from := range refs { + // check if the referenced row exists + exists, err := rt.rowExists(spec.ToTable, to) + if err != nil { + return ModelUpdates{}, err + } + if !exists { + for _, uuid := range from { + // strong reference to a row that does not exist + return ModelUpdates{}, ovsdb.NewReferentialIntegrityViolation(fmt.Sprintf( + "Table %s column %s row %s references nonexistent or deleted row %s in table %s", + spec.FromTable, spec.FromColumn, uuid, to, spec.ToTable)) + } + // we deleted the row ourselves on a previous loop + continue + } + + // track if this row is referenced from this location spec + isReferenced[to] = isReferenced[to] || len(from) > 0 + } + } + + // inserted rows that are unreferenced and not part of the root set will + // silently be dropped from the updates + for uuid := range rt.added { + if isReferenced[uuid] { + continue + } + isReferenced[uuid] = false + } + + // delete rows that are not referenced + updates := ModelUpdates{} + for uuid, isReferenced := range isReferenced { + if isReferenced { + // row is still referenced, ignore + continue + } + + if rt.deleted[uuid] != "" { + // already deleted, ignore + continue + } + + table := rt.tracked[uuid] + if isRoot(&rt.dbModel, table) { + // table is part of the root set, ignore + continue + } + + // delete row that is not part of the root set and is no longer + // referenced + update, err := rt.deleteRow(table, uuid) + if err != nil { + return ModelUpdates{}, err + } + err = updates.Merge(rt.dbModel, update) + if err != nil { + return ModelUpdates{}, err + } + } + + return updates, nil +} + +// processWeakReferences deletes weak references to rows that were deleted. +// Returns a constraint violation if this results in invalid values +func (rt *referenceTracker) processWeakReferences() (ModelUpdates, error) { + // make sure that we are tracking the references to rows that might have + // been deleted as a result of strong reference garbage collection + err := rt.initReferencesOfDeletedRows() + if err != nil { + return ModelUpdates{}, err + } + + tables := map[string]string{} + originalRows := map[string]ovsdb.Row{} + updatedRows := map[string]ovsdb.Row{} + + for spec, refs := range rt.references { + // fetch some reference information from the schema + extendedType, minLenAllowed, refType, _ := refInfo(&rt.dbModel, spec.FromTable, spec.FromColumn, spec.FromValue) + isEmptyAllowed := minLenAllowed == 0 + + if refType != ovsdb.Weak { + // we only care about weak references + continue + } + + for to, from := range refs { + if len(from) == 0 { + // not referenced from anywhere, ignore + continue + } + + // check if the referenced row exists + exists, err := rt.rowExists(spec.ToTable, to) + if err != nil { + return ModelUpdates{}, err + } + if exists { + // we only care about rows that have been deleted or otherwise + // don't exist + continue + } + + // generate the updates to remove the references to deleted rows + for _, uuid := range from { + if _, ok := updatedRows[uuid]; !ok { + updatedRows[uuid] = ovsdb.NewRow() + } + + if rt.deleted[uuid] != "" { + // already deleted, ignore + continue + } + + // fetch the original rows + if originalRows[uuid] == nil { + originalRow, err := rt.getRow(spec.FromTable, uuid) + if err != nil { + return ModelUpdates{}, err + } + if originalRow == nil { + return ModelUpdates{}, fmt.Errorf("reference from non-existent model with uuid %s", uuid) + } + originalRows[uuid] = *originalRow + } + + var becomesLen int + switch extendedType { + case ovsdb.TypeMap: + // a map referencing the row + // generate the mutation to remove the entry form the map + originalMap := originalRows[uuid][spec.FromColumn].(ovsdb.OvsMap).GoMap + var mutationMap map[interface{}]interface{} + value, ok := updatedRows[uuid][spec.FromColumn] + if !ok { + mutationMap = map[interface{}]interface{}{} + } else { + mutationMap = value.(ovsdb.OvsMap).GoMap + } + // copy the map entries referencing the row from the original map + mutationMap = copyMapKeyValues(originalMap, mutationMap, !spec.FromValue, ovsdb.UUID{GoUUID: to}) + + // track the new length of the map + if !isEmptyAllowed { + becomesLen = len(originalMap) - len(mutationMap) + } + + updatedRows[uuid][spec.FromColumn] = ovsdb.OvsMap{GoMap: mutationMap} + + case ovsdb.TypeSet: + // a set referencing the row + // generate the mutation to remove the entry form the set + var mutationSet []interface{} + value, ok := updatedRows[uuid][spec.FromColumn] + if !ok { + mutationSet = []interface{}{} + } else { + mutationSet = value.(ovsdb.OvsSet).GoSet + } + mutationSet = append(mutationSet, ovsdb.UUID{GoUUID: to}) + + // track the new length of the set + if !isEmptyAllowed { + originalSet := originalRows[uuid][spec.FromColumn].(ovsdb.OvsSet).GoSet + becomesLen = len(originalSet) - len(mutationSet) + } + + updatedRows[uuid][spec.FromColumn] = ovsdb.OvsSet{GoSet: mutationSet} + + case ovsdb.TypeUUID: + // this is an atomic UUID value that needs to be cleared + updatedRows[uuid][spec.FromColumn] = nil + becomesLen = 0 + } + + if becomesLen < minLenAllowed { + return ModelUpdates{}, ovsdb.NewConstraintViolation(fmt.Sprintf( + "Deletion of a weak reference to a deleted (or never-existing) row from column %s in table %s "+ + "row %s caused this column to have an invalid length.", + spec.FromColumn, spec.FromTable, uuid)) + } + + // track the table of the row we are going to update + tables[uuid] = spec.FromTable + } + } + } + + // process the updates + updates := ModelUpdates{} + for uuid, rowUpdate := range updatedRows { + update, err := rt.updateRow(tables[uuid], uuid, rowUpdate) + if err != nil { + return ModelUpdates{}, err + } + err = updates.Merge(rt.dbModel, update) + if err != nil { + return ModelUpdates{}, err + } + } + + return updates, nil +} + +func copyMapKeyValues(from, to map[interface{}]interface{}, isKey bool, keyValue interface{}) map[interface{}]interface{} { + if isKey { + to[keyValue] = from[keyValue] + return to + } + for key, value := range from { + if reflect.DeepEqual(value, keyValue) { + to[key] = from[key] + } + } + return to +} + +// initReferences initializes the references to the provided row from the +// database +func (rt *referenceTracker) initReferences(table, uuid string) error { + if _, ok := rt.tracked[uuid]; ok { + // already initialized + return nil + } + existingRefs, err := rt.provider.GetReferences(rt.dbModel.Client().Name(), table, uuid) + if err != nil { + return err + } + rt.references.UpdateReferences(existingRefs) + rt.tracked[uuid] = table + return nil +} + +func (rt *referenceTracker) initReferencesOfDeletedRows() error { + for uuid, table := range rt.deleted { + err := rt.initReferences(table, uuid) + if err != nil { + return err + } + } + return nil +} + +// deleteRow adds an update to delete the provided row. +func (rt *referenceTracker) deleteRow(table, uuid string) (ModelUpdates, error) { + model, err := rt.getModel(table, uuid) + if err != nil { + return ModelUpdates{}, err + } + row, err := rt.getRow(table, uuid) + if err != nil { + return ModelUpdates{}, err + } + + updates := ModelUpdates{} + update := ovsdb.RowUpdate2{Delete: &ovsdb.Row{}, Old: row} + err = updates.AddRowUpdate2(rt.dbModel, table, uuid, model, update) + + rt.deleted[uuid] = table + + return updates, err +} + +// updateRow generates updates for the provided row +func (rt *referenceTracker) updateRow(table, uuid string, row ovsdb.Row) (ModelUpdates, error) { + model, err := rt.getModel(table, uuid) + if err != nil { + return ModelUpdates{}, err + } + + // In agreement with processWeakReferences, columns with values are assumed + // to be values of sets or maps that need to be mutated for deletion. + // Columns with no values are assumed to be atomic optional values that need + // to be cleared with an update. + + mutations := make([]ovsdb.Mutation, 0, len(row)) + update := ovsdb.Row{} + for column, value := range row { + if value != nil { + mutations = append(mutations, *ovsdb.NewMutation(column, ovsdb.MutateOperationDelete, value)) + continue + } + update[column] = ovsdb.OvsSet{GoSet: []interface{}{}} + } + + updates := ModelUpdates{} + + if len(mutations) > 0 { + err = updates.AddOperation(rt.dbModel, table, uuid, model, &ovsdb.Operation{ + Op: ovsdb.OperationMutate, + Table: table, + Mutations: mutations, + Where: []ovsdb.Condition{ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: uuid})}, + }) + if err != nil { + return ModelUpdates{}, err + } + } + + if len(update) > 0 { + err = updates.AddOperation(rt.dbModel, table, uuid, model, &ovsdb.Operation{ + Op: ovsdb.OperationUpdate, + Table: table, + Row: update, + Where: []ovsdb.Condition{ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: uuid})}, + }) + if err != nil { + return ModelUpdates{}, err + } + } + + return updates, nil +} + +// getModel gets the model from the updates or the database +func (rt *referenceTracker) getModel(table, uuid string) (model.Model, error) { + if _, deleted := rt.deleted[uuid]; deleted { + // model has been deleted + return nil, nil + } + // look for the model in the updates + model := rt.updates.GetModel(table, uuid) + if model != nil { + return model, nil + } + // look for the model in the database + model, err := rt.provider.Get(rt.dbModel.Client().Name(), table, uuid) + if err != nil { + return nil, err + } + return model, nil +} + +// getRow gets the row from the updates or the database +func (rt *referenceTracker) getRow(table, uuid string) (*ovsdb.Row, error) { + if _, deleted := rt.deleted[uuid]; deleted { + // row has been deleted + return nil, nil + } + // look for the row in the updates + row := rt.updates.GetRow(table, uuid) + if row != nil { + return row, nil + } + // look for the model in the database and build the row + model, err := rt.provider.Get(rt.dbModel.Client().Name(), table, uuid) + if err != nil { + return nil, err + } + info, err := rt.dbModel.NewModelInfo(model) + if err != nil { + return nil, err + } + newRow, err := rt.dbModel.Mapper.NewRow(info) + if err != nil { + return nil, err + } + return &newRow, nil +} + +// rowExists returns whether the row exists either in the updates or the database +func (rt *referenceTracker) rowExists(table, uuid string) (bool, error) { + model, err := rt.getModel(table, uuid) + return model != nil, err +} + +func getReferenceModificationsFromRow(dbModel *model.DatabaseModel, table, uuid string, modify, old *ovsdb.Row) database.References { + refs := database.References{} + for column, value := range *modify { + var oldValue interface{} + if old != nil { + oldValue = (*old)[column] + } + crefs := getReferenceModificationsFromColumn(dbModel, table, uuid, column, value, oldValue) + refs.UpdateReferences(crefs) + } + return refs +} + +func getReferenceModificationsFromColumn(dbModel *model.DatabaseModel, table, uuid, column string, modify, old interface{}) database.References { + switch v := modify.(type) { + case ovsdb.UUID: + var oldUUID ovsdb.UUID + if old != nil { + oldUUID = old.(ovsdb.UUID) + } + return getReferenceModificationsFromAtom(dbModel, table, uuid, column, v, oldUUID) + case ovsdb.OvsSet: + var oldSet ovsdb.OvsSet + if old != nil { + oldSet = old.(ovsdb.OvsSet) + } + return getReferenceModificationsFromSet(dbModel, table, uuid, column, v, oldSet) + case ovsdb.OvsMap: + return getReferenceModificationsFromMap(dbModel, table, uuid, column, v) + } + return nil +} + +func getReferenceModificationsFromMap(dbModel *model.DatabaseModel, table, uuid, column string, value ovsdb.OvsMap) database.References { + if len(value.GoMap) == 0 { + return nil + } + + // get the referenced table + keyRefTable := refTable(dbModel, table, column, false) + valueRefTable := refTable(dbModel, table, column, true) + if keyRefTable == "" && valueRefTable == "" { + return nil + } + + from := uuid + keySpec := database.ReferenceSpec{ToTable: keyRefTable, FromTable: table, FromColumn: column, FromValue: false} + valueSpec := database.ReferenceSpec{ToTable: valueRefTable, FromTable: table, FromColumn: column, FromValue: true} + + refs := database.References{} + for k, v := range value.GoMap { + if keyRefTable != "" { + switch to := k.(type) { + case ovsdb.UUID: + if _, ok := refs[keySpec]; !ok { + refs[keySpec] = database.Reference{to.GoUUID: []string{from}} + } else if _, ok := refs[keySpec][to.GoUUID]; !ok { + refs[keySpec][to.GoUUID] = append(refs[keySpec][to.GoUUID], from) + } + } + } + if valueRefTable != "" { + switch to := v.(type) { + case ovsdb.UUID: + if _, ok := refs[valueSpec]; !ok { + refs[valueSpec] = database.Reference{to.GoUUID: []string{from}} + } else if _, ok := refs[valueSpec][to.GoUUID]; !ok { + refs[valueSpec][to.GoUUID] = append(refs[valueSpec][to.GoUUID], from) + } + } + } + } + + return refs +} + +func getReferenceModificationsFromSet(dbModel *model.DatabaseModel, table, uuid, column string, modify, old ovsdb.OvsSet) database.References { + // if the modify set is empty, it means the op is clearing an atomic value + // so pick the old value instead + value := modify + if len(modify.GoSet) == 0 { + value = old + } + + if len(value.GoSet) == 0 { + return nil + } + + // get the referenced table + refTable := refTable(dbModel, table, column, false) + if refTable == "" { + return nil + } + + spec := database.ReferenceSpec{ToTable: refTable, FromTable: table, FromColumn: column} + from := uuid + refs := database.References{spec: database.Reference{}} + for _, v := range value.GoSet { + switch to := v.(type) { + case ovsdb.UUID: + refs[spec][to.GoUUID] = append(refs[spec][to.GoUUID], from) + } + } + return refs +} + +func getReferenceModificationsFromAtom(dbModel *model.DatabaseModel, table, uuid, column string, modify, old ovsdb.UUID) database.References { + // get the referenced table + refTable := refTable(dbModel, table, column, false) + if refTable == "" { + return nil + } + spec := database.ReferenceSpec{ToTable: refTable, FromTable: table, FromColumn: column} + from := uuid + to := modify.GoUUID + refs := database.References{spec: {to: {from}}} + if old.GoUUID != "" { + // extract the old value as well + refs[spec][old.GoUUID] = []string{from} + } + return refs +} + +// applyReferenceModifications updates references in 'a' from those in 'b' +func applyReferenceModifications(a, b database.References) { + for spec, bv := range b { + for to, bfrom := range bv { + if av, ok := a[spec]; ok { + if afrom, ok := av[to]; ok { + r, _ := applyDifference(afrom, bfrom) + av[to] = r.([]string) + } else { + // this reference is not in 'a', so add it + av[to] = bfrom + } + } else { + // this reference is not in 'a', so add it + a[spec] = database.Reference{to: bfrom} + } + } + } +} + +func refInfo(dbModel *model.DatabaseModel, table, column string, mapValue bool) (ovsdb.ExtendedType, int, ovsdb.RefType, string) { + tSchema := dbModel.Schema.Table(table) + if tSchema == nil { + panic(fmt.Sprintf("unexpected schema error: no schema for table %s", table)) + } + + cSchema := tSchema.Column(column) + if cSchema == nil { + panic(fmt.Sprintf("unexpected schema error: no schema for column %s", column)) + } + + cType := cSchema.TypeObj + if cType == nil { + // this is not a reference + return "", 0, "", "" + } + + var bType *ovsdb.BaseType + switch { + case !mapValue && cType.Key != nil: + bType = cType.Key + case mapValue && cType.Value != nil: + bType = cType.Value + default: + panic(fmt.Sprintf("unexpected schema error: no schema for map value on column %s", column)) + } + if bType.Type != ovsdb.TypeUUID { + // this is not a reference + return "", 0, "", "" + } + + // treat optional values represented with sets as atomic UUIDs + extendedType := cSchema.Type + if extendedType == ovsdb.TypeSet && cType.Min() == 0 && cType.Max() == 1 { + extendedType = ovsdb.TypeUUID + } + + rType, err := bType.RefType() + if err != nil { + panic(fmt.Sprintf("unexpected schema error: %v", err)) + } + + rTable, err := bType.RefTable() + if err != nil { + panic(fmt.Sprintf("unexpected schema error: %v", err)) + } + + return extendedType, cType.Min(), rType, rTable +} + +func refTable(dbModel *model.DatabaseModel, table, column string, mapValue bool) ovsdb.RefType { + _, _, _, refTable := refInfo(dbModel, table, column, mapValue) + return refTable +} + +func isRoot(dbModel *model.DatabaseModel, table string) bool { + isRoot, err := dbModel.Schema.IsRoot(table) + if err != nil { + panic(fmt.Sprintf("unexpected schema error: %v", err)) + } + return isRoot +} + +func isStrong(dbModel *model.DatabaseModel, spec database.ReferenceSpec) bool { + _, _, refType, _ := refInfo(dbModel, spec.FromTable, spec.FromColumn, spec.FromValue) + return refType == ovsdb.Strong +} diff --git a/updates/references_test.go b/updates/references_test.go new file mode 100644 index 00000000..4ef645b9 --- /dev/null +++ b/updates/references_test.go @@ -0,0 +1,2050 @@ +package updates + +import ( + "encoding/json" + "fmt" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ovn-org/libovsdb/database" + "github.com/ovn-org/libovsdb/model" + "github.com/ovn-org/libovsdb/ovsdb" +) + +const referencesTestSchema = ` +{ + "name": "References_Test", + "version": "0.0.1", + "tables": { + "Parent": { + "columns": { + "strong_atomic_required_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child" + }, + "min": 1, + "max": 1 + } + }, + "strong_atomic_optional_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child" + }, + "min": 0, + "max": 1 + } + }, + "strong_set_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child" + }, + "min": 0, + "max": "unlimited" + } + }, + "strong_map_key_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child" + }, + "value": { + "type": "string" + }, + "min": 0, + "max": "unlimited" + } + }, + "strong_map_value_reference": { + "type": { + "key": { + "type": "string" + }, + "value": { + "type": "uuid", + "refTable": "Child" + }, + "min": 1, + "max": "unlimited" + } + }, + "weak_atomic_required_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child", + "refType": "weak" + }, + "min": 1, + "max": 1 + } + }, + "weak_atomic_optional_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child", + "refType": "weak" + }, + "min": 0, + "max": 1 + } + }, + "weak_set_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child", + "refType": "weak" + }, + "min": 2, + "max": "unlimited" + } + }, + "weak_map_key_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child", + "refType": "weak" + }, + "value": { + "type": "string" + }, + "min": 1, + "max": "unlimited" + } + }, + "weak_map_value_reference": { + "type": { + "key": { + "type": "string" + }, + "value": { + "type": "uuid", + "refTable": "Child", + "refType": "weak" + }, + "min": 1, + "max": "unlimited" + } + }, + "map_key_value_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Child", + "refType": "weak" + }, + "value": { + "type": "uuid", + "refTable": "Child", + "refType": "strong" + }, + "min": 0, + "max": "unlimited" + } + } + }, + "isRoot": true + }, + "Child": { + "columns": { + "name": { + "type": "string", + "mutable": false + }, + "strong_atomic_optional_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Grandchild" + }, + "min": 0, + "max": 1 + } + }, + "weak_atomic_optional_reference": { + "type": { + "key": { + "type": "uuid", + "refTable": "Grandchild", + "refType": "weak" + }, + "min": 0, + "max": 1 + } + } + }, + "indexes": [ + [ + "name" + ] + ] + }, + "Grandchild": { + "columns": { + "name": { + "type": "string", + "mutable": false + } + }, + "indexes": [ + [ + "name" + ] + ] + } + } +} +` + +type Parent struct { + UUID string `ovsdb:"_uuid"` + StrongAtomicRequiredReference string `ovsdb:"strong_atomic_required_reference"` + StrongAtomicOptionalReference *string `ovsdb:"strong_atomic_optional_reference"` + StrongSetReference []string `ovsdb:"strong_set_reference"` + StrongMapKeyReference map[string]string `ovsdb:"strong_map_key_reference"` + StrongMapValueReference map[string]string `ovsdb:"strong_map_value_reference"` + WeakAtomicRequiredReference string `ovsdb:"weak_atomic_required_reference"` + WeakAtomicOptionalReference *string `ovsdb:"weak_atomic_optional_reference"` + WeakSetReference []string `ovsdb:"weak_set_reference"` + WeakMapKeyReference map[string]string `ovsdb:"weak_map_key_reference"` + WeakMapValueReference map[string]string `ovsdb:"weak_map_value_reference"` + MapKeyValueReference map[string]string `ovsdb:"map_key_value_reference"` +} + +type Child struct { + UUID string `ovsdb:"_uuid"` + StrongAtomicOptionalReference *string `ovsdb:"strong_atomic_optional_reference"` + WeakAtomicOptionalReference *string `ovsdb:"weak_atomic_optional_reference"` +} + +type Grandchild struct { + UUID string `ovsdb:"_uuid"` +} + +func getReferencesTestDBModel() (model.DatabaseModel, error) { + client, err := model.NewClientDBModel( + "References_Test", + map[string]model.Model{ + "Parent": &Parent{}, + "Child": &Child{}, + "Grandchild": &Grandchild{}, + }, + ) + if err != nil { + return model.DatabaseModel{}, err + } + schema, err := getReferencesTestSchema() + if err != nil { + return model.DatabaseModel{}, err + } + dbModel, errs := model.NewDatabaseModel(schema, client) + if len(errs) > 0 { + return model.DatabaseModel{}, fmt.Errorf("errors build model: %v", errs) + } + return dbModel, nil +} + +func getReferencesTestSchema() (ovsdb.DatabaseSchema, error) { + var dbSchema ovsdb.DatabaseSchema + err := json.Unmarshal([]byte(referencesTestSchema), &dbSchema) + return dbSchema, err +} + +type testReferenceProvider struct { + models map[string]model.Model + references database.References +} + +func (rp *testReferenceProvider) GetReferences(database, table, uuid string) (database.References, error) { + return rp.references.GetReferences(table, uuid), nil +} + +func (rp *testReferenceProvider) Get(database, table string, uuid string) (model.Model, error) { + return rp.models[uuid], nil +} + +var ( + referencesTestDBModel model.DatabaseModel +) + +func ptr(s string) *string { + return &s +} + +type testData struct { + existingModels []model.Model + updatedModels []model.Model + finalModels []model.Model + existingReferences database.References + wantUpdatedReferences database.References +} + +func TestProcessReferences(t *testing.T) { + var err error + referencesTestDBModel, err = getReferencesTestDBModel() + if err != nil { + t.Errorf("error building DB model: %v", err) + } + + tests := []struct { + name string + testData testData + wantErr bool + }{ + { + // when a strong reference is replaced with another in a required atomic + // field, the referenced row should be deleted + name: "strong atomic required reference garbage collected when replaced", + testData: strongAtomicRequiredReferenceTestData(), + }, + { + // attempting to delete a row that is strongly referenced from a + // required atomic field should fail + name: "constraint violation when strongly referenced row from required field deleted", + testData: strongAtomicRequiredReferenceDeleteConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to add a required strong reference to a nonexistent row should + // fail + name: "constraint violation when strong required reference to nonexistent row added", + testData: strongAtomicRequiredReferenceAddConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // when a strong reference is removed from an optional atomic field, the + // referenced row should be deleted + name: "strong atomic optional reference garbage collected when removed", + testData: strongAtomicOptionalReferenceTestData(), + }, + { + // attempting to delete a row that is strongly referenced from an + // optional atomic field should fail + name: "constraint violation when strongly referenced row from optional field deleted", + testData: strongAtomicOptionalReferenceDeleteConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to add a optional strong reference to a nonexistent + // row should fail + name: "constraint violation when strong optional reference to nonexistent row added", + testData: strongAtomicOptionalReferenceAddConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // when a strong reference is removed from a set, the referenced row should + // be deleted + name: "strong reference garbage collected when removed from set", + testData: strongSetReferenceTestData(), + }, + { + // attempting to remove a row that is still strongly referenced in a set should fail + name: "strong set reference constraint violation when row deleted error", + testData: strongSetReferenceDeleteConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to add strong set reference to non existent row should fail + name: "strong set reference constraint violation when nonexistent reference added error", + testData: strongSetReferenceAddConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // when a strong reference is removed from a map key, the + // referenced row should be deleted + name: "strong reference garbage collected when removed from map key", + testData: strongMapKeyReferenceTestData(), + }, + { + // attempting to remove a row that is still strongly referenced in a + // map key should fail + name: "strong map key reference constraint violation when row deleted error", + testData: strongMapKeyReferenceDeleteConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to add strong map key reference to non existent row should fail + name: "strong map key reference constraint violation when nonexistent reference added error", + testData: strongMapKeyReferenceAddConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // when a strong reference is removed from a map value, the + // referenced row should be deleted + name: "strong reference garbage collected when removed from map value", + testData: strongMapValueReferenceTestData(), + }, + { + // attempting to remove a row that is still strongly referenced in a + // map value should fail + name: "strong map value reference constraint violation when row deleted error", + testData: strongMapValueReferenceDeleteConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to add strong map value reference to non existent row should fail + name: "strong map value reference constraint violation when nonexistent reference added error", + testData: strongMapValueReferenceAddConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // when a weak referenced row is deleted, the reference on an atomic + // optional field is also deleted + name: "weak atomic optional reference deleted when row deleted", + testData: weakAtomicOptionalReferenceTestData(), + }, + { + // when a weak referenced row is deleted, the reference on an set is + // also deleted + name: "weak reference deleted from set when row deleted", + testData: weakSetReferenceTestData(), + }, + { + // when a weak referenced row is deleted, the reference on a map + // key is also deleted + name: "weak reference deleted from map key when row deleted", + testData: weakMapKeyReferenceTestData(), + }, + { + // when a weak referenced row is deleted, the reference on a map + // value is also deleted + name: "weak reference deleted from map value when row deleted", + testData: weakMapValueReferenceTestData(), + }, + { + // attempting to delete a weak referenced row when it is referenced + // from an atomic required field will fail + name: "weak reference constraint violation in required atomic field when row deleted error", + testData: weakAtomicReferenceConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to delete a weak referenced row when it is referenced + // from an set that then becomes smaller than the minimum allowed + // will fail + name: "weak reference constraint violation in set becoming smaller than allowed error", + testData: weakSetReferenceConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to delete a weak referenced row when it is referenced + // from a map key that then becomes smaller than the minimum + // allowed will fail + name: "weak reference constraint violation in map key field becoming smaller than allowed error", + testData: weakMapKeyReferenceConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // attempting to delete a weak referenced row when it is referenced + // from a map value that then becomes smaller than the minimum + // allowed will fail + name: "weak reference constraint violation in map value field becoming smaller than allowed error", + testData: weakMapValueReferenceConstraintViolationErrorTestData(), + wantErr: true, + }, + { + // corner case + // inserting a row in a table that is not part of the root set and + // is not strongly referenced is a noop + name: "insert unreferenced row in non root set table is a noop", + testData: insertNoRootUnreferencedRowTestData(), + }, + { + // corner case + // adding a weak reference to a nonexistent row is a noop + name: "insert weak reference to nonexistent row is a noop", + testData: weakReferenceToNonExistentRowTestData(), + }, + { + // corner case + // for a map holding weak key references to strong value references, when + // the weak reference row is deleted, the map entry and the strongly + // referenced row is also deleted + name: "map with key weak reference and value strong reference, weak reference and strong referenced row deleted", + testData: mapKeyValueReferenceTestData(), + wantErr: false, + }, + { + // corner case + // when a weak referenced row is deleted, multiple references on a map + // value are also deleted + name: "multiple weak references deleted from map value when row deleted", + testData: multipleWeakMapValueReferenceTestData(), + wantErr: false, + }, + { + // corner case when multiple rows are transitively & strongly + // referenced, garbage collection happens transitively as well + name: "transitive strong references garbage collected when removed", + testData: transitiveStrongReferenceTestData(), + }, + { + // corner case + // when a strong referenced is removed, an unreferenced row will be + // garbage collected and weak references to it removed + name: "transitive strong and weak references garbage collected when removed", + testData: transitiveStrongAndWeakReferenceTestData(), + }, + { + // corner case + // a row needs to have a weak reference garbage collected and + // at the same time that row itself is garbage collected due to not + // being strongly referenced + name: "strong and weak garbage collection over the same row doesn't fail", + testData: sameRowStrongAndWeakReferenceTestData(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + td := tt.testData + rp := testReferenceProvider{ + models: indexModels(td.existingModels), + references: td.existingReferences, + } + + onUpdates, err := getUpdates(td.existingModels, td.updatedModels) + require.NoError(t, err, "failed to build updates from existing and updated models") + + // need a copy easiest way to have it is generating the updates all + // over again + onUpdatesCopy, err := getUpdates(td.existingModels, td.updatedModels) + require.NoError(t, err, "failed to build updates copy from existing and updated models") + + gotModelUpdates, gotReferenceModelUpdates, gotReferenceUpdates, err := ProcessReferences(referencesTestDBModel, &rp, onUpdates) + if tt.wantErr { + assert.NotNil(t, err, "expected an error but got none") + return + } + assert.NoError(t, err, "got a different error than expected") + + //gotModelUpdates := gotUpdates.(modelUpdatesWithReferences).ModelUpdates + wantModelUpdates, err := getUpdates(td.existingModels, td.finalModels) + require.NoError(t, err, "failed to build updates from existing and final models") + assert.Equal(t, wantModelUpdates, gotModelUpdates, "got different updates than expected") + + //gotUpdatedReferences := gotUpdates.(modelUpdatesWithReferences).references + assert.Equal(t, td.wantUpdatedReferences, gotReferenceUpdates, "got different reference updates than expected") + + gotMergedModelUpdates := onUpdatesCopy + err = gotMergedModelUpdates.Merge(referencesTestDBModel, gotReferenceModelUpdates) + require.NoError(t, err) + assert.Equal(t, gotModelUpdates, gotMergedModelUpdates, + "the updates are not a result of merging the initial updates with the reference updates") + }) + } +} + +func getUUID(model model.Model) string { + return reflect.ValueOf(model).Elem().FieldByName("UUID").Interface().(string) +} + +func indexModels(models []model.Model) map[string]model.Model { + indexed := map[string]model.Model{} + for _, model := range models { + indexed[getUUID(model)] = model + } + return indexed +} + +// getUpdates returns the updates needed to go from existing to updated +func getUpdates(existing, updated []model.Model) (ModelUpdates, error) { + // index the models by uuid + existingModels := indexModels(existing) + updatedModels := indexModels(updated) + + // helpers + tables := map[string]string{} + getRow := func(model model.Model, fields ...interface{}) (ovsdb.Row, error) { + info, err := referencesTestDBModel.NewModelInfo(model) + if err != nil { + return nil, err + } + row, err := referencesTestDBModel.Mapper.NewRow(info, fields...) + if err != nil { + return nil, err + } + tables[getUUID(model)] = info.Metadata.TableName + return row, nil + } + + getUpdateOp := func(old, new model.Model) (ovsdb.Operation, error) { + var err error + var row ovsdb.Row + + // insert + if old == nil { + row, err := getRow(new) + return ovsdb.Operation{ + Op: ovsdb.OperationInsert, + Table: tables[getUUID(new)], + Row: row, + }, err + } + + // delete + if new == nil { + // lazy, just to cache the table of the row + _, err := getRow(old) + + return ovsdb.Operation{ + Op: ovsdb.OperationDelete, + Table: tables[getUUID(old)], + Where: []ovsdb.Condition{ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: getUUID(old)})}, + }, err + } + + // update, just with the fields that have been changed + fields := []interface{}{} + xv := reflect.ValueOf(new).Elem() + xt := xv.Type() + for i := 0; i < xt.NumField(); i++ { + if !reflect.DeepEqual(xv.Field(i).Interface(), reflect.ValueOf(old).Elem().Field(i).Interface()) { + fields = append(fields, xv.Field(i).Addr().Interface()) + } + } + + row, err = getRow(new, fields...) + return ovsdb.Operation{ + Op: ovsdb.OperationUpdate, + Table: tables[getUUID(new)], + Row: row, + Where: []ovsdb.Condition{ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: getUUID(new)})}, + }, err + + } + + // generate updates + updates := ModelUpdates{} + for uuid, updatedModel := range updatedModels { + op, err := getUpdateOp(existingModels[uuid], updatedModel) + if err != nil { + return updates, err + } + err = updates.AddOperation(referencesTestDBModel, tables[uuid], uuid, existingModels[uuid], &op) + if err != nil { + return updates, err + } + } + + // deletes + for uuid := range existingModels { + if updatedModels[uuid] != nil { + continue + } + op, err := getUpdateOp(existingModels[uuid], nil) + if err != nil { + return updates, err + } + err = updates.AddOperation(referencesTestDBModel, tables[uuid], uuid, existingModels[uuid], &op) + if err != nil { + return updates, err + } + } + + return updates, nil +} + +func strongAtomicRequiredReferenceTestData() testData { + // when a strong reference is replaced with another in a required atomic + // field, the referenced row should be deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicRequiredReference: "child", + }, + &Child{ + UUID: "child", + }, + }, + // newChild is added and parent reference is replaced with newChild + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicRequiredReference: "newChild", + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "newChild", + }, + }, + // child model should be deleted as it is no longer referenced + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicRequiredReference: "newChild", + }, + &Child{ + UUID: "newChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_required_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + // child model is no longer referenced, newChild is + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_required_reference", + }: database.Reference{ + "child": nil, + "newChild": []string{"parent"}, + }, + }, + } +} + +func strongAtomicRequiredReferenceDeleteConstraintViolationErrorTestData() testData { + // attempting to delete a row that is strongly referenced from a required + // atomic field should fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicRequiredReference: "child", + }, + &Child{ + UUID: "child", + }, + }, + // child is removed but will fail as it is still referenced + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicRequiredReference: "child", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_required_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func strongAtomicRequiredReferenceAddConstraintViolationErrorTestData() testData { + // attempting to add a required strong reference to a nonexistent row should + // fail + return testData{ + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicRequiredReference: "child", + }, + }, + } +} + +func strongAtomicOptionalReferenceTestData() testData { + // when a strong reference is removed from an optional atomic field, the + // referenced row should be deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + }, + }, + // parent reference to child is removed + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + &Child{ + UUID: "child", + }, + }, + // child model should be deleted as it is no longer referenced + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func strongAtomicOptionalReferenceDeleteConstraintViolationErrorTestData() testData { + // attempting to delete a row that is strongly referenced from an optional + // atomic field should fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + }, + }, + // child is removed but will fail as it is still referenced + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func strongAtomicOptionalReferenceAddConstraintViolationErrorTestData() testData { + // attempting to add a optional strong reference to a nonexistent row should + // fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + // add reference to child but will fail as it does not exist + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + }, + }, + } +} + +func strongSetReferenceTestData() testData { + // when a strong reference is removed from a set, the referenced row should + // be deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"child", "otherChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child reference is removed from the set + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"otherChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child model should be deleted as it is no longer referenced + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"otherChild"}, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_set_reference", + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_set_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func strongMapKeyReferenceTestData() testData { + // when a strong reference is removed from a map key, the referenced row + // should be deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapKeyReference: map[string]string{ + "child": "value", + }, + }, + &Child{ + UUID: "child", + }, + }, + // child reference is removed from the map + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + &Child{ + UUID: "child", + }, + }, + // child model should be deleted as it is no longer referenced + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_map_key_reference", + FromValue: false, + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_map_key_reference", + FromValue: false, + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func strongMapKeyReferenceDeleteConstraintViolationErrorTestData() testData { + // attempting to remove a row that is still strongly referenced in a map key + // should fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapKeyReference: map[string]string{ + "child": "value", + }, + }, + &Child{ + UUID: "child", + }, + }, + // child is removed but will fail as it is still referenced + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapKeyReference: map[string]string{ + "child": "value", + }, + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_map_key_reference", + FromValue: false, + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func strongMapKeyReferenceAddConstraintViolationErrorTestData() testData { + // attempting to add a map key strong reference to a nonexistent row should + // fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + // child reference is added to the map but wil fail as child does not + // exist + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapKeyReference: map[string]string{"child": "value"}, + }, + }, + } +} + +func strongMapValueReferenceTestData() testData { + // when a strong reference is removed from a map value, the referenced row + // should be deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapValueReference: map[string]string{ + "key1": "child", + "key2": "otherChild", + }, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child reference is removed from the map + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapValueReference: map[string]string{ + "key2": "otherChild", + }, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child model should be deleted as it is no longer referenced + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapValueReference: map[string]string{ + "key2": "otherChild", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func strongMapValueReferenceDeleteConstraintViolationErrorTestData() testData { + // attempting to remove a row that is still strongly referenced in a map value + // should fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapKeyReference: map[string]string{ + "key": "child", + }, + }, + &Child{ + UUID: "child", + }, + }, + // child is removed but will fail as it is still referenced + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapKeyReference: map[string]string{ + "key": "child", + }, + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func strongMapValueReferenceAddConstraintViolationErrorTestData() testData { + // attempting to add a map key strong reference to a nonexistent row should + // fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + // child reference is added to the map but wil fail as is it doesn't exist + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongMapValueReference: map[string]string{"key": "child"}, + }, + }, + } +} + +func strongSetReferenceDeleteConstraintViolationErrorTestData() testData { + // attempting to remove a row that is still strongly referenced should fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"child", "otherChild"}, + }, + &Parent{ + UUID: "otherParent", + StrongSetReference: []string{"child"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child is deleted from parent but will fail as it is still referenced + // from other parent + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"otherChild"}, + }, + &Parent{ + UUID: "otherParent", + StrongSetReference: []string{"child"}, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_set_reference", + }: database.Reference{ + "child": []string{"parent", "otherParent"}, + "otherChild": []string{"parent"}, + }, + }, + } +} + +func strongSetReferenceAddConstraintViolationErrorTestData() testData { + // attempting to add strong reference to non existent row should fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"child"}, + }, + &Child{ + UUID: "child", + }, + }, + // otherChild reference is added to parent but will fail as otherChild + // does not exist + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"child", "otherChild"}, + }, + &Child{ + UUID: "child", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_set_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func weakAtomicOptionalReferenceTestData() testData { + // when a weak referenced row is deleted, the reference on an atomic + // optional field is also deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + }, + }, + // child is deleted + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakAtomicOptionalReference: ptr("child"), + }, + }, + // the reference to child should be removed from parent + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func weakAtomicReferenceConstraintViolationErrorTestData() testData { + // an attempt to delete a weak referenced row when it is referenced from an + // atomic required field will fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakAtomicRequiredReference: "child", + }, + &Child{ + UUID: "child", + }, + }, + // child is deleted, but will fail because that would leave a mandatory + // field empty + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakAtomicRequiredReference: "child", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_atomic_required_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func weakSetReferenceTestData() testData { + // when a weak referenced row is deleted, the reference on an set is also + // deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakSetReference: []string{"child", "otherChild", "thirdChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + &Child{ + UUID: "thirdChild", + }, + }, + // child is deleted + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakSetReference: []string{"child", "otherChild", "thirdChild"}, + }, + &Child{ + UUID: "otherChild", + }, + &Child{ + UUID: "thirdChild", + }, + }, + // the reference to child should be removed from parent + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakSetReference: []string{"otherChild", "thirdChild"}, + }, + &Child{ + UUID: "otherChild", + }, + &Child{ + UUID: "thirdChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_set_reference", + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + "thirdChild": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_set_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func weakSetReferenceConstraintViolationErrorTestData() testData { + // an attempt to delete a weak referenced row when it is referenced from a + // set that then becomes smaller than the minimum allowed will fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakSetReference: []string{"child", "otherChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child is deleted but will fail because the set becomes empty and + // that is not allowed by the schema + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakSetReference: []string{"child", "otherChild"}, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_set_reference", + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + }, + } +} + +func weakMapKeyReferenceTestData() testData { + // when a weak referenced row is deleted, the reference on a map + // value is also deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapKeyReference: map[string]string{ + "child": "value1", + "otherChild": "value2", + }, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child is deleted + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapKeyReference: map[string]string{ + "child": "value1", + "otherChild": "value2", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + // the reference to child should be removed from parent + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapKeyReference: map[string]string{ + "otherChild": "value2", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_key_reference", + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_key_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func weakMapValueReferenceTestData() testData { + // when a weak referenced row is deleted, the reference on a map + // value is also deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key1": "child", + "key2": "otherChild", + }, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child is deleted + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key1": "child", + "key2": "otherChild", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + // the reference to child should be removed from parent + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key2": "otherChild", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + }, + // child model is no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": nil, + }, + }, + } +} +func weakMapKeyReferenceConstraintViolationErrorTestData() testData { + // an attempt to delete a weak referenced row when it is referenced from a + // map key that then becomes smaller than the minimum allowed will fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapKeyReference: map[string]string{ + "child": "value", + }, + }, + &Child{ + UUID: "child", + }, + }, + // child is deleted but will fail because the map becomes empty and + // that is not allowed by the schema + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapKeyReference: map[string]string{ + "child": "value", + }, + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_key_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func weakMapValueReferenceConstraintViolationErrorTestData() testData { + // an attempt to delete a weak referenced row when it is referenced from a + // map value that then becomes smaller than the minimum allowed will fail + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key1": "child", + }, + }, + &Child{ + UUID: "child", + }, + }, + // child is deleted but will fail because the map becomes empty and + // that is not allowed by the schema + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key1": "child", + }, + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + } +} + +func mapKeyValueReferenceTestData() testData { + // for a map holding weak key references to strong value references, when + // the weak reference row is deleted, the map entry and the strongly + // referenced row is also deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + MapKeyValueReference: map[string]string{ + "weakChild": "strongChild", + }, + }, + &Child{ + UUID: "weakChild", + }, + &Child{ + UUID: "strongChild", + }, + }, + // weak child is deleted + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + MapKeyValueReference: map[string]string{ + "weakChild": "strongChild", + }, + }, + &Child{ + UUID: "strongChild", + }, + }, + // the reference to weak child should be removed from parent + // and strong child should be deleted + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "map_key_value_reference", + FromValue: false, + }: database.Reference{ + "weakChild": []string{"parent"}, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "map_key_value_reference", + FromValue: true, + }: database.Reference{ + "strongChild": []string{"parent"}, + }, + }, + // neither weak or strong child are referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "map_key_value_reference", + FromValue: false, + }: database.Reference{ + "weakChild": nil, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "map_key_value_reference", + FromValue: true, + }: database.Reference{ + "strongChild": nil, + }, + }, + } +} + +func multipleWeakMapValueReferenceTestData() testData { + // when a weak referenced row is deleted, multiple references on a map + // value are also deleted + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key1": "child", + "key2": "otherChild", + "key3": "child", + }, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + }, + // child is deleted + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key1": "child", + "key2": "otherChild", + "key3": "child", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + // the reference to child should be removed from parent + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakMapValueReference: map[string]string{ + "key2": "otherChild", + }, + }, + &Child{ + UUID: "otherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + }, + // child model is no longer referenced, newChild is + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func transitiveStrongReferenceTestData() testData { + // when multiple rows are transitively referenced, garbage collection + // happens transitively as well + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + StrongAtomicOptionalReference: ptr("grandchild"), + }, + &Grandchild{ + UUID: "grandchild", + }, + }, + // parent reference to child is removed + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + &Child{ + UUID: "child", + StrongAtomicOptionalReference: ptr("grandchild"), + }, + &Grandchild{ + UUID: "grandchild", + }, + }, + // child and grandchild models should be deleted as it is no longer referenced + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + database.ReferenceSpec{ + ToTable: "Grandchild", + FromTable: "Child", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "grandchild": []string{"child"}, + }, + }, + // child and grandchild models are no longer referenced + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + database.ReferenceSpec{ + ToTable: "Grandchild", + FromTable: "Child", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "grandchild": nil, + }, + }, + } +} + +func transitiveStrongAndWeakReferenceTestData() testData { + // when a strong referenced is removed, an unreferenced row will be garbage + // collected and transitively, weak references to it removed + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + WeakAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + }, + }, + // parent strong reference to child is removed + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + }, + }, + // as a result, child and and the weak reference to it is removed + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + }, + // child is no longer referenced at all + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func insertNoRootUnreferencedRowTestData() testData { + return testData{ + // new child is inserted + updatedModels: []model.Model{ + &Child{ + UUID: "newChild", + }, + }, + // but is removed since is not referenced from anywhere and the table is + // not part of the root set + finalModels: nil, + wantUpdatedReferences: database.References{}, + } +} + +func weakReferenceToNonExistentRowTestData() testData { + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + // a weak reference is added no nonexistent row + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + WeakAtomicOptionalReference: ptr("child"), + }, + }, + // but is removed since the row does not exist + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + }, + } +} + +func sameRowStrongAndWeakReferenceTestData() testData { + // a row needs to have a weak reference garbage collected and + // at the same time that row itself is garbage collected due to not + // being strongly referenced + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + }, + &Child{ + UUID: "child", + WeakAtomicOptionalReference: ptr("grandchild"), + }, + &Grandchild{ + UUID: "grandchild", + }, + }, + // parent strong reference to child is removed + // grand child is removed as well + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + &Child{ + UUID: "child", + WeakAtomicOptionalReference: ptr("grandchild"), + }, + }, + // as a result, child is removed + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + database.ReferenceSpec{ + ToTable: "Grandchild", + FromTable: "Child", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "grandchild": []string{"child"}, + }, + }, + // neither child nor grandchild are referenced at all + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": nil, + }, + database.ReferenceSpec{ + ToTable: "Grandchild", + FromTable: "Child", + FromColumn: "weak_atomic_optional_reference", + }: database.Reference{ + "grandchild": nil, + }, + }, + } +} diff --git a/updates/updates.go b/updates/updates.go index f8591560..4ff2363a 100644 --- a/updates/updates.go +++ b/updates/updates.go @@ -71,8 +71,10 @@ func (u ModelUpdates) GetModel(table, uuid string) model.Model { if u.updates == nil { return nil } - if table, found := u.updates[table]; found { - return table[uuid].new + if t, found := u.updates[table]; found { + if update, found := t[uuid]; found { + return update.new + } } return nil } @@ -83,8 +85,10 @@ func (u ModelUpdates) GetRow(table, uuid string) *ovsdb.Row { if u.updates == nil { return nil } - if table, found := u.updates[table]; found { - return table[uuid].rowUpdate2.New + if t, found := u.updates[table]; found { + if update, found := t[uuid]; found { + return update.rowUpdate2.New + } } return nil } From 1c3007dd69fa129f5d958458e380a23f9c3db3ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Thu, 21 Dec 2023 11:36:49 +0000 Subject: [PATCH 08/13] Add referential integrity transaction tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provides coverage for the interaction of reference garbage collection with index constraint checks. Signed-off-by: Jaime Caamaño Ruiz --- database/inmemory/inmemory_test.go | 119 +++++++++++++++++++++++++++++ test/test_data.go | 32 +++++++- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/database/inmemory/inmemory_test.go b/database/inmemory/inmemory_test.go index b26ec607..322c021f 100644 --- a/database/inmemory/inmemory_test.go +++ b/database/inmemory/inmemory_test.go @@ -924,3 +924,122 @@ func checkOperationResults(result []*ovsdb.OperationResult, ops ...ovsdb.Operati } return ovsdb.CheckOperationResults(r, ops) } + +func TestCheckIndexesWithReferentialIntegrity(t *testing.T) { + dbModel, err := GetModel() + require.NoError(t, err) + db := NewDatabase(map[string]model.ClientDBModel{"Open_vSwitch": dbModel.Client()}) + err = db.CreateDatabase("Open_vSwitch", dbModel.Schema) + require.NoError(t, err) + + ovsUUID := uuid.NewString() + managerUUID := uuid.NewString() + managerUUID2 := uuid.NewString() + ops := []ovsdb.Operation{ + { + Table: "Open_vSwitch", + Op: ovsdb.OperationInsert, + UUID: ovsUUID, + Row: ovsdb.Row{ + "manager_options": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: managerUUID}}}, + }, + }, + { + Table: "Manager", + Op: ovsdb.OperationInsert, + UUID: managerUUID, + Row: ovsdb.Row{ + "target": "target", + }, + }, + } + + transaction := db.NewTransaction("Open_vSwitch") + results, updates := transaction.Transact(ops...) + require.Len(t, results, len(ops)) + for _, result := range results { + assert.Equal(t, "", result.Error) + } + err = db.Commit("Open_vSwitch", uuid.New(), updates) + require.NoError(t, err) + + tests := []struct { + desc string + ops func() []ovsdb.Operation + wantUpdates int + }{ + { + // As a row is deleted due to garbage collection, that row's index + // should be available for use by a different row + desc: "Replacing a strong reference should garbage collect and account for indexes", + ops: func() []ovsdb.Operation { + return []ovsdb.Operation{ + { + Table: "Open_vSwitch", + Op: ovsdb.OperationUpdate, + Row: ovsdb.Row{ + "manager_options": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: managerUUID2}}}, + }, + Where: []ovsdb.Condition{ + ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: ovsUUID}), + }, + }, + { + Table: "Manager", + Op: ovsdb.OperationInsert, + UUID: managerUUID2, + Row: ovsdb.Row{ + "target": "target", + }, + }, + } + }, + // the update and insert above plus the delete from the garbage + // collection + wantUpdates: 3, + }, + { + desc: "A row that is not root and not strongly referenced should not cause index collisions", + ops: func() []ovsdb.Operation { + return []ovsdb.Operation{ + { + Table: "Manager", + Op: ovsdb.OperationInsert, + UUID: managerUUID2, + Row: ovsdb.Row{ + "target": "target", + }, + }, + } + }, + // no updates as the row is not strongly referenced + wantUpdates: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + transaction := db.NewTransaction("Open_vSwitch") + ops := tt.ops() + results, update := transaction.Transact(ops...) + var err string + for _, result := range results { + if result.Error != "" { + err = result.Error + break + } + } + require.Empty(t, err, "got an unexpected error") + + tables := update.GetUpdatedTables() + var gotUpdates int + for _, table := range tables { + _ = update.ForEachRowUpdate(table, func(uuid string, row ovsdb.RowUpdate2) error { + gotUpdates++ + return nil + }) + } + assert.Equal(t, tt.wantUpdates, gotUpdates, "got a different number of updates than expected") + }) + } +} diff --git a/test/test_data.go b/test/test_data.go index 20686c17..562e6d92 100644 --- a/test/test_data.go +++ b/test/test_data.go @@ -17,6 +17,16 @@ const schema = ` "tables": { "Open_vSwitch": { "columns": { + "manager_options": { + "type": { + "key": { + "type": "uuid", + "refTable": "Manager" + }, + "min": 0, + "max": "unlimited" + } + }, "bridges": { "type": { "key": { @@ -27,6 +37,7 @@ const schema = ` } } }, + "isRoot": true, "maxRows": 1 }, "Bridge": { @@ -81,6 +92,7 @@ const schema = ` } } }, + "isRoot": true, "indexes": [ [ "name" @@ -118,12 +130,21 @@ const schema = ` } } }, + "isRoot": true, "indexes": [ [ "id", "bridge" ] ] + }, + "Manager": { + "columns": { + "target": { + "type": "string" + } + }, + "indexes": [["target"]] } } } @@ -143,8 +164,9 @@ type BridgeType struct { // OvsType is the simplified ORM model of the Bridge table type OvsType struct { - UUID string `ovsdb:"_uuid"` - Bridges []string `ovsdb:"bridges"` + UUID string `ovsdb:"_uuid"` + Bridges []string `ovsdb:"bridges"` + ManagerOptions []string `ovsdb:"manager_options"` } type FlowSampleCollectorSetType struct { @@ -155,6 +177,11 @@ type FlowSampleCollectorSetType struct { IPFIX *string // `ovsdb:"ipfix"` } +type Manager struct { + UUID string `ovsdb:"_uuid"` + Target string `ovsdb:"target"` +} + func GetModel() (model.DatabaseModel, error) { client, err := model.NewClientDBModel( "Open_vSwitch", @@ -162,6 +189,7 @@ func GetModel() (model.DatabaseModel, error) { "Open_vSwitch": &OvsType{}, "Bridge": &BridgeType{}, "Flow_Sample_Collector_Set": &FlowSampleCollectorSetType{}, + "Manager": &Manager{}, }, ) if err != nil { From 09997db8bc94841bb9ffe087a5dc514b714525f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 9 Jan 2024 11:08:24 +0000 Subject: [PATCH 09/13] Add referential integrity integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While these tests could have been purely server tests, setting them up as integration tests against both the test server and the real OVSDB server allows us to contrast the implemented behavior. Also provides coverage for how the references are updated in the database as well as for how referential integrity updates are sent back to the client. Signed-off-by: Jaime Caamaño Ruiz --- server/server_integration_test.go | 346 +++++++++++++++++++++++++++ test/ovs/ovs_integration_test.go | 380 ++++++++++++++++++++++++++++++ test/test_data.go | 56 ++++- 3 files changed, 780 insertions(+), 2 deletions(-) diff --git a/server/server_integration_test.go b/server/server_integration_test.go index e9f3d8dd..e4273df9 100644 --- a/server/server_integration_test.go +++ b/server/server_integration_test.go @@ -10,11 +10,13 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/ovn-org/libovsdb/cache" "github.com/ovn-org/libovsdb/client" "github.com/ovn-org/libovsdb/database/inmemory" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" + "github.com/ovn-org/libovsdb/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -516,3 +518,347 @@ func TestMultipleOpsSameRow(t *testing.T) { require.Equal(t, map[string]string{"key1": "value1", "keyA": "valueA"}, br.ExternalIds) require.Nil(t, br.DatapathID) } + +func TestReferentialIntegrity(t *testing.T) { + // UUIDs to use throughout the tests + ovsUUID := uuid.New().String() + bridgeUUID := uuid.New().String() + port1UUID := uuid.New().String() + port2UUID := uuid.New().String() + mirrorUUID := uuid.New().String() + + // the test adds an additional op to initialOps to set a reference to + // the bridge in OVS table + // the test deletes expectModels at the end + tests := []struct { + name string + initialOps []ovsdb.Operation + testOps func(client.Client) ([]ovsdb.Operation, error) + expectModels []model.Model + dontExpectModels []model.Model + expectErr bool + }{ + { + name: "strong reference is garbage collected", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // remove the mirror reference + b := &test.BridgeType{UUID: bridgeUUID} + return c.Where(b).Update(b, &b.Mirrors) + }, + expectModels: []model.Model{ + &test.BridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port1UUID}}, + &test.PortType{UUID: port1UUID, Name: port1UUID}, + }, + dontExpectModels: []model.Model{ + // mirror should have been garbage collected + &test.MirrorType{UUID: mirrorUUID}, + }, + }, + { + name: "adding non-root row that is not strongly reference is a noop", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // add a mirror + m := &test.MirrorType{UUID: mirrorUUID, Name: mirrorUUID} + return c.Create(m) + }, + expectModels: []model.Model{ + &test.BridgeType{UUID: bridgeUUID, Name: bridgeUUID}, + }, + dontExpectModels: []model.Model{ + // mirror should have not been added as is not referenced from anywhere + &test.MirrorType{UUID: mirrorUUID}, + }, + }, + { + name: "adding non-existent strong reference fails", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // add a mirror + b := &test.BridgeType{UUID: bridgeUUID, Mirrors: []string{mirrorUUID}} + return c.Where(b).Update(b, &b.Mirrors) + }, + expectModels: []model.Model{ + &test.BridgeType{UUID: bridgeUUID, Name: bridgeUUID}, + }, + expectErr: true, + }, + { + name: "weak reference is garbage collected", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}, ovsdb.UUID{GoUUID: port2UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port2UUID, + Row: ovsdb.Row{ + "name": port2UUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}, ovsdb.UUID{GoUUID: port2UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // remove port1 + p := &test.PortType{UUID: port1UUID} + ops, err := c.Where(p).Delete() + if err != nil { + return nil, err + } + b := &test.BridgeType{UUID: bridgeUUID, Ports: []string{port2UUID}} + op, err := c.Where(b).Update(b, &b.Ports) + if err != nil { + return nil, err + } + return append(ops, op...), nil + }, + expectModels: []model.Model{ + &test.BridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port2UUID}, Mirrors: []string{mirrorUUID}}, + &test.PortType{UUID: port2UUID, Name: port2UUID}, + // mirror reference to port1 should have been garbage collected + &test.MirrorType{UUID: mirrorUUID, Name: mirrorUUID, SelectSrcPort: []string{port2UUID}}, + }, + dontExpectModels: []model.Model{ + &test.PortType{UUID: port1UUID}, + }, + }, + { + name: "adding a weak reference to a non-existent row is a noop", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // add reference to non-existent port2 + m := &test.MirrorType{UUID: mirrorUUID, SelectSrcPort: []string{port1UUID, port2UUID}} + return c.Where(m).Update(m, &m.SelectSrcPort) + }, + expectModels: []model.Model{ + &test.BridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port1UUID}, Mirrors: []string{mirrorUUID}}, + &test.PortType{UUID: port1UUID, Name: port1UUID}, + // mirror reference to port2 should have been garbage collected resulting in noop + &test.MirrorType{UUID: mirrorUUID, Name: mirrorUUID, SelectSrcPort: []string{port1UUID}}, + }, + }, + { + name: "garbage collecting a weak reference on a column lowering it below the min length fails", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // remove port 1 + return c.Where(&test.PortType{UUID: port1UUID}).Delete() + }, + expectModels: []model.Model{ + &test.BridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port1UUID}, Mirrors: []string{mirrorUUID}}, + &test.PortType{UUID: port1UUID, Name: port1UUID}, + &test.MirrorType{UUID: mirrorUUID, Name: mirrorUUID, SelectSrcPort: []string{port1UUID}}, + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, close := buildTestServerAndClient(t) + defer close() + _, err := c.MonitorAll(context.Background()) + require.NoError(t, err) + + // add the bridge reference to the initial ops + ops := append(tt.initialOps, ovsdb.Operation{ + Op: ovsdb.OperationInsert, + Table: "Open_vSwitch", + UUID: ovsUUID, + Row: ovsdb.Row{ + "bridges": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: bridgeUUID}}}, + }, + }) + + results, err := c.Transact(context.Background(), ops...) + require.NoError(t, err) + require.Len(t, results, len(ops)) + + errors, err := ovsdb.CheckOperationResults(results, ops) + require.Nil(t, errors) + require.NoError(t, err) + + ops, err = tt.testOps(c) + require.NoError(t, err) + + results, err = c.Transact(context.Background(), ops...) + require.NoError(t, err) + + errors, err = ovsdb.CheckOperationResults(results, ops) + require.Nil(t, errors) + if tt.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + for _, m := range tt.expectModels { + actual := model.Clone(m) + err := c.Get(context.Background(), actual) + require.NoError(t, err, "when expecting model %v", m) + require.Equal(t, m, actual) + } + + for _, m := range tt.dontExpectModels { + err := c.Get(context.Background(), m) + require.ErrorIs(t, err, client.ErrNotFound, "when not expecting model %v", m) + } + + ops = []ovsdb.Operation{} + for _, m := range tt.expectModels { + op, err := c.Where(m).Delete() + require.NoError(t, err) + require.Len(t, op, 1) + ops = append(ops, op...) + } + + // remove the bridge reference + ops = append(ops, ovsdb.Operation{ + Op: ovsdb.OperationDelete, + Table: "Open_vSwitch", + Where: []ovsdb.Condition{ + { + Column: "_uuid", + Function: ovsdb.ConditionEqual, + Value: ovsdb.UUID{GoUUID: ovsUUID}, + }, + }, + }) + + results, err = c.Transact(context.Background(), ops...) + require.NoError(t, err) + require.Len(t, results, len(ops)) + + errors, err = ovsdb.CheckOperationResults(results, ops) + require.Nil(t, errors) + require.NoError(t, err) + }) + } +} diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index daafc926..88da6419 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/cenkalti/backoff/v4" + "github.com/google/uuid" "github.com/ory/dockertest/v3" "github.com/ory/dockertest/v3/docker" "github.com/ovn-org/libovsdb/cache" @@ -164,6 +165,7 @@ type bridgeType struct { BridgeFailMode *BridgeFailMode `ovsdb:"fail_mode"` IPFIX *string `ovsdb:"ipfix"` DatapathID *string `ovsdb:"datapath_id"` + Mirrors []string `ovsdb:"mirrors"` } // ovsType is the ORM model of the OVS table @@ -200,11 +202,31 @@ type queueType struct { DSCP *int `ovsdb:"dscp"` } +type portType struct { + UUID string `ovsdb:"_uuid"` + Name string `ovsdb:"name"` + Interfaces []string `ovsdb:"interfaces"` +} + +type interfaceType struct { + UUID string `ovsdb:"_uuid"` + Name string `ovsdb:"name"` +} + +type mirrorType struct { + UUID string `ovsdb:"_uuid"` + Name string `ovsdb:"name"` + SelectSrcPort []string `ovsdb:"select_src_port"` +} + var defDB, _ = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}, "IPFIX": &ipfixType{}, "Queue": &queueType{}, + "Port": &portType{}, + "Mirror": &mirrorType{}, + "Interface": &interfaceType{}, }) func (suite *OVSIntegrationSuite) TestConnectReconnect() { @@ -1243,3 +1265,361 @@ func (suite *OVSIntegrationSuite) TestMultipleOpsSameRow() { require.Equal(suite.T(), map[string]string{"key1": "value1", "keyA": "valueA"}, br.ExternalIds) require.Nil(suite.T(), br.DatapathID) } + +func (suite *OVSIntegrationSuite) TestReferentialIntegrity() { + t := suite.Suite.T() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + // fetch the OVS UUID + var ovs []*ovsType + err := suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }).List(ctx, &ovs) + require.NoError(t, err) + require.Len(t, ovs, 1) + + // UUIDs to use throughout the tests + ovsUUID := ovs[0].UUID + bridgeUUID := uuid.New().String() + port1UUID := uuid.New().String() + port2UUID := uuid.New().String() + interfaceUUID := uuid.New().String() + mirrorUUID := uuid.New().String() + + // monitor additional table specific to this test + _, err = suite.clientWithoutInactvityCheck.Monitor(ctx, + suite.clientWithoutInactvityCheck.NewMonitor( + client.WithTable(&portType{}), + client.WithTable(&interfaceType{}), + client.WithTable(&mirrorType{}), + ), + ) + require.NoError(t, err) + + // the test adds an additional op to initialOps to set a reference to + // the bridge in OVS table + // the test deletes expectModels at the end + tests := []struct { + name string + initialOps []ovsdb.Operation + testOps func(client.Client) ([]ovsdb.Operation, error) + expectModels []model.Model + dontExpectModels []model.Model + expectErr bool + }{ + { + name: "strong reference is garbage collected", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + "interfaces": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: interfaceUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Interface", + UUID: interfaceUUID, + Row: ovsdb.Row{ + "name": interfaceUUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // remove the mirror reference + b := &bridgeType{UUID: bridgeUUID} + return c.Where(b).Update(b, &b.Mirrors) + }, + expectModels: []model.Model{ + &bridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port1UUID}}, + &portType{UUID: port1UUID, Name: port1UUID, Interfaces: []string{interfaceUUID}}, + &interfaceType{UUID: interfaceUUID, Name: interfaceUUID}, + }, + dontExpectModels: []model.Model{ + // mirror should have been garbage collected + &mirrorType{UUID: mirrorUUID}, + }, + }, + { + name: "adding non-root row that is not strongly reference is a noop", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // add a mirror + m := &mirrorType{UUID: mirrorUUID, Name: mirrorUUID} + return c.Create(m) + }, + expectModels: []model.Model{ + &bridgeType{UUID: bridgeUUID, Name: bridgeUUID}, + }, + dontExpectModels: []model.Model{ + // mirror should have not been added as is not referenced from anywhere + &mirrorType{UUID: mirrorUUID}, + }, + }, + { + name: "adding non-existent strong reference fails", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // add a mirror + b := &bridgeType{UUID: bridgeUUID, Mirrors: []string{mirrorUUID}} + return c.Where(b).Update(b, &b.Mirrors) + }, + expectModels: []model.Model{ + &bridgeType{UUID: bridgeUUID, Name: bridgeUUID}, + }, + expectErr: true, + }, + { + name: "weak reference is garbage collected", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}, ovsdb.UUID{GoUUID: port2UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + "interfaces": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: interfaceUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port2UUID, + Row: ovsdb.Row{ + "name": port2UUID, + "interfaces": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: interfaceUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Interface", + UUID: interfaceUUID, + Row: ovsdb.Row{ + "name": interfaceUUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}, ovsdb.UUID{GoUUID: port2UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // remove port1 + b := &bridgeType{UUID: bridgeUUID, Ports: []string{port2UUID}} + return c.Where(b).Update(b, &b.Ports) + }, + expectModels: []model.Model{ + &bridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port2UUID}, Mirrors: []string{mirrorUUID}}, + &portType{UUID: port2UUID, Name: port2UUID, Interfaces: []string{interfaceUUID}}, + // mirror reference to port1 should have been garbage collected + &mirrorType{UUID: mirrorUUID, Name: mirrorUUID, SelectSrcPort: []string{port2UUID}}, + }, + dontExpectModels: []model.Model{ + &portType{UUID: port1UUID}, + }, + }, + { + name: "adding a weak reference to a non-existent row is a noop", + initialOps: []ovsdb.Operation{ + { + Op: ovsdb.OperationInsert, + Table: "Bridge", + UUID: bridgeUUID, + Row: ovsdb.Row{ + "name": bridgeUUID, + "ports": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + "mirrors": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: mirrorUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Port", + UUID: port1UUID, + Row: ovsdb.Row{ + "name": port1UUID, + "interfaces": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: interfaceUUID}}}, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Interface", + UUID: interfaceUUID, + Row: ovsdb.Row{ + "name": interfaceUUID, + }, + }, + { + Op: ovsdb.OperationInsert, + Table: "Mirror", + UUID: mirrorUUID, + Row: ovsdb.Row{ + "name": mirrorUUID, + "select_src_port": ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: port1UUID}}}, + }, + }, + }, + testOps: func(c client.Client) ([]ovsdb.Operation, error) { + // add reference to non-existent port2 + m := &mirrorType{UUID: mirrorUUID, SelectSrcPort: []string{port1UUID, port2UUID}} + return c.Where(m).Update(m, &m.SelectSrcPort) + }, + expectModels: []model.Model{ + &bridgeType{UUID: bridgeUUID, Name: bridgeUUID, Ports: []string{port1UUID}, Mirrors: []string{mirrorUUID}}, + &portType{UUID: port1UUID, Name: port1UUID, Interfaces: []string{interfaceUUID}}, + &interfaceType{UUID: interfaceUUID, Name: interfaceUUID}, + // mirror reference to port2 should have been garbage collected resulting in noop + &mirrorType{UUID: mirrorUUID, Name: mirrorUUID, SelectSrcPort: []string{port1UUID}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := suite.clientWithoutInactvityCheck + + // add the bridge reference to the initial ops + ops := append(tt.initialOps, ovsdb.Operation{ + Op: ovsdb.OperationMutate, + Table: "Open_vSwitch", + Mutations: []ovsdb.Mutation{ + { + Mutator: ovsdb.MutateOperationInsert, + Column: "bridges", + Value: ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: bridgeUUID}}}, + }, + }, + Where: []ovsdb.Condition{ + { + Column: "_uuid", + Function: ovsdb.ConditionEqual, + Value: ovsdb.UUID{GoUUID: ovsUUID}, + }, + }, + }) + + results, err := c.Transact(ctx, ops...) + require.NoError(t, err) + require.Len(t, results, len(ops)) + + errors, err := ovsdb.CheckOperationResults(results, ops) + require.Nil(t, errors) + require.NoError(t, err) + + ops, err = tt.testOps(c) + require.NoError(t, err) + + results, err = c.Transact(ctx, ops...) + require.NoError(t, err) + + errors, err = ovsdb.CheckOperationResults(results, ops) + require.Nil(t, errors) + if tt.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + for _, m := range tt.expectModels { + actual := model.Clone(m) + err := c.Get(ctx, actual) + require.NoError(t, err, "when expecting model %v", m) + require.Equal(t, m, actual) + } + + for _, m := range tt.dontExpectModels { + err := c.Get(ctx, m) + require.ErrorIs(t, err, client.ErrNotFound, "when expecting model %v", m) + } + + ops = []ovsdb.Operation{} + for _, m := range tt.expectModels { + op, err := c.Where(m).Delete() + require.NoError(t, err) + require.Len(t, op, 1) + ops = append(ops, op...) + } + + // remove the bridge reference + ops = append(ops, ovsdb.Operation{ + Op: ovsdb.OperationMutate, + Table: "Open_vSwitch", + Mutations: []ovsdb.Mutation{ + { + Mutator: ovsdb.MutateOperationDelete, + Column: "bridges", + Value: ovsdb.OvsSet{GoSet: []interface{}{ovsdb.UUID{GoUUID: bridgeUUID}}}, + }, + }, + Where: []ovsdb.Condition{ + { + Column: "_uuid", + Function: ovsdb.ConditionEqual, + Value: ovsdb.UUID{GoUUID: ovsUUID}, + }, + }, + }) + + results, err = c.Transact(context.Background(), ops...) + require.NoError(t, err) + require.Len(t, results, len(ops)) + + errors, err = ovsdb.CheckOperationResults(results, ops) + require.Nil(t, errors) + require.NoError(t, err) + }) + } +} diff --git a/test/test_data.go b/test/test_data.go index 562e6d92..e4435108 100644 --- a/test/test_data.go +++ b/test/test_data.go @@ -66,6 +66,16 @@ const schema = ` "max": "unlimited" } }, + "mirrors": { + "type": { + "key": { + "type": "uuid", + "refTable": "Mirror" + }, + "min": 0, + "max": "unlimited" + } + }, "status": { "type": { "key": "string", @@ -145,6 +155,34 @@ const schema = ` } }, "indexes": [["target"]] + }, + "Mirror": { + "columns": { + "name": { + "type": "string" + }, + "select_src_port": { + "type": { + "key": { + "type": "uuid", + "refTable": "Port", + "refType": "weak" + }, + "min": 1, + "max": "unlimited" + } + } + } + }, + "Port": { + "columns": { + "name": { + "type": "string", + "mutable": false + } + }, + "isRoot": true, + "indexes": [["name"]] } } } @@ -160,6 +198,7 @@ type BridgeType struct { ExternalIds map[string]string `ovsdb:"external_ids"` Ports []string `ovsdb:"ports"` Status map[string]string `ovsdb:"status"` + Mirrors []string `ovsdb:"mirrors"` } // OvsType is the simplified ORM model of the Bridge table @@ -177,11 +216,22 @@ type FlowSampleCollectorSetType struct { IPFIX *string // `ovsdb:"ipfix"` } -type Manager struct { +type ManagerType struct { UUID string `ovsdb:"_uuid"` Target string `ovsdb:"target"` } +type PortType struct { + UUID string `ovsdb:"_uuid"` + Name string `ovsdb:"name"` +} + +type MirrorType struct { + UUID string `ovsdb:"_uuid"` + Name string `ovsdb:"name"` + SelectSrcPort []string `ovsdb:"select_src_port"` +} + func GetModel() (model.DatabaseModel, error) { client, err := model.NewClientDBModel( "Open_vSwitch", @@ -189,7 +239,9 @@ func GetModel() (model.DatabaseModel, error) { "Open_vSwitch": &OvsType{}, "Bridge": &BridgeType{}, "Flow_Sample_Collector_Set": &FlowSampleCollectorSetType{}, - "Manager": &Manager{}, + "Manager": &ManagerType{}, + "Mirror": &MirrorType{}, + "Port": &PortType{}, }, ) if err != nil { From f5a6d80f1be96c0fe56c4cdd10ad04c3715bb268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Mon, 15 Jan 2024 14:55:25 +0000 Subject: [PATCH 10/13] Add more reference tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jaime Caamaño Ruiz --- updates/references_test.go | 145 +++++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/updates/references_test.go b/updates/references_test.go index 4ef645b9..2b2a1148 100644 --- a/updates/references_test.go +++ b/updates/references_test.go @@ -454,6 +454,11 @@ func TestProcessReferences(t *testing.T) { testData: weakMapValueReferenceConstraintViolationErrorTestData(), wantErr: true, }, + { + // testing behavior with multiple combinations of references + name: "multiple strong and weak reference changes", + testData: multipleReferencesTestData(), + }, { // corner case // inserting a row in a table that is not part of the root set and @@ -2048,3 +2053,143 @@ func sameRowStrongAndWeakReferenceTestData() testData { }, } } + +func multipleReferencesTestData() testData { + // testing behavior with multiple combinations of references + return testData{ + existingModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongSetReference: []string{"child"}, + StrongAtomicOptionalReference: ptr("child"), + WeakMapValueReference: map[string]string{"key1": "yetAnotherChild", "key2": "otherChild"}, + }, + &Parent{ + UUID: "otherParent", + StrongAtomicOptionalReference: ptr("child"), + StrongSetReference: []string{"otherChild"}, + WeakSetReference: []string{"otherChild", "child", "yetAnotherChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + &Child{ + UUID: "yetAnotherChild", + }, + }, + // all strong references to child except one are removed + // single strong reference to otherChild is removed + updatedModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + WeakMapValueReference: map[string]string{"key1": "yetAnotherChild", "key2": "otherChild"}, + }, + &Parent{ + UUID: "otherParent", + WeakSetReference: []string{"otherChild", "child", "yetAnotherChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "otherChild", + }, + &Child{ + UUID: "yetAnotherChild", + }, + }, + // otherChild is garbage collected and all weak references to it removed + finalModels: []model.Model{ + &Parent{ + UUID: "parent", + StrongAtomicOptionalReference: ptr("child"), + WeakMapValueReference: map[string]string{"key1": "yetAnotherChild"}, + }, + &Parent{ + UUID: "otherParent", + WeakSetReference: []string{"child", "yetAnotherChild"}, + }, + &Child{ + UUID: "child", + }, + &Child{ + UUID: "yetAnotherChild", + }, + }, + existingReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_set_reference", + }: database.Reference{ + "child": []string{"parent"}, + "otherChild": []string{"otherParent"}, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent", "otherParent"}, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "yetAnotherChild": []string{"parent"}, + "otherChild": []string{"parent"}, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_set_reference", + }: database.Reference{ + "otherChild": []string{"otherParent"}, + "child": []string{"otherParent"}, + "yetAnotherChild": []string{"otherParent"}, + }, + }, + // all strong references to child except one are removed + // all references to otherChild are removed + // references to yetAnotherChild are unchanged + wantUpdatedReferences: database.References{ + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_set_reference", + }: database.Reference{ + "child": nil, + "otherChild": nil, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "strong_atomic_optional_reference", + }: database.Reference{ + "child": []string{"parent"}, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_map_value_reference", + FromValue: true, + }: database.Reference{ + "otherChild": nil, + }, + database.ReferenceSpec{ + ToTable: "Child", + FromTable: "Parent", + FromColumn: "weak_set_reference", + }: database.Reference{ + "child": []string{"otherParent"}, // this reference is read by the reference tracker, but not changed + "otherChild": nil, + }, + }, + } +} From 74320563971fc5d86e367f9ac913a0c912300049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 23 Jan 2024 09:54:35 +0000 Subject: [PATCH 11/13] Improve schema isRoot compat check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jaime Caamaño Ruiz --- ovsdb/schema.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ovsdb/schema.go b/ovsdb/schema.go index 99824da7..285d1e02 100644 --- a/ovsdb/schema.go +++ b/ovsdb/schema.go @@ -37,13 +37,16 @@ func (schema DatabaseSchema) IsRoot(tableName string) (bool, error) { if t == nil { return false, fmt.Errorf("Table %s not in schame", tableName) } + if t.IsRoot { + return true, nil + } + // As per RFC7047, for compatibility with schemas created before + // "isRoot" was introduced, if "isRoot" is omitted or false in every + // in a given , then every table is part + // of the root set. if schema.allTablesRoot == nil { allTablesRoot := true for _, tSchema := range schema.Tables { - // As per RFC7047, for compatibility with schemas created before - // "isRoot" was introduced, if "isRoot" is omitted or false in every - // in a given , then every table is part - // of the root set. if tSchema.IsRoot { allTablesRoot = false break @@ -51,7 +54,7 @@ func (schema DatabaseSchema) IsRoot(tableName string) (bool, error) { } schema.allTablesRoot = &allTablesRoot } - return *schema.allTablesRoot || t.IsRoot, nil + return *schema.allTablesRoot, nil } // Print will print the contents of the DatabaseSchema From bb2f6dfae527a76c276f2f6208d5fd5dbf1e59dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 23 Jan 2024 10:02:54 +0000 Subject: [PATCH 12/13] Fix reference test schema indentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jaime Caamaño Ruiz --- updates/references_test.go | 60 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/updates/references_test.go b/updates/references_test.go index 2b2a1148..6e820ae9 100644 --- a/updates/references_test.go +++ b/updates/references_test.go @@ -31,7 +31,7 @@ const referencesTestSchema = ` "max": 1 } }, - "strong_atomic_optional_reference": { + "strong_atomic_optional_reference": { "type": { "key": { "type": "uuid", @@ -41,7 +41,7 @@ const referencesTestSchema = ` "max": 1 } }, - "strong_set_reference": { + "strong_set_reference": { "type": { "key": { "type": "uuid", @@ -51,25 +51,25 @@ const referencesTestSchema = ` "max": "unlimited" } }, - "strong_map_key_reference": { + "strong_map_key_reference": { "type": { "key": { "type": "uuid", - "refTable": "Child" + "refTable": "Child" }, - "value": { + "value": { "type": "string" }, "min": 0, "max": "unlimited" } }, - "strong_map_value_reference": { + "strong_map_value_reference": { "type": { "key": { "type": "string" }, - "value": { + "value": { "type": "uuid", "refTable": "Child" }, @@ -77,78 +77,78 @@ const referencesTestSchema = ` "max": "unlimited" } }, - "weak_atomic_required_reference": { + "weak_atomic_required_reference": { "type": { "key": { "type": "uuid", "refTable": "Child", - "refType": "weak" + "refType": "weak" }, "min": 1, "max": 1 } }, - "weak_atomic_optional_reference": { + "weak_atomic_optional_reference": { "type": { "key": { "type": "uuid", "refTable": "Child", - "refType": "weak" + "refType": "weak" }, "min": 0, "max": 1 } }, - "weak_set_reference": { + "weak_set_reference": { "type": { "key": { "type": "uuid", "refTable": "Child", - "refType": "weak" + "refType": "weak" }, "min": 2, "max": "unlimited" } }, - "weak_map_key_reference": { + "weak_map_key_reference": { "type": { "key": { - "type": "uuid", + "type": "uuid", "refTable": "Child", - "refType": "weak" + "refType": "weak" }, - "value": { - "type": "string" + "value": { + "type": "string" }, "min": 1, "max": "unlimited" } }, - "weak_map_value_reference": { + "weak_map_value_reference": { "type": { "key": { "type": "string" }, - "value": { + "value": { "type": "uuid", "refTable": "Child", - "refType": "weak" + "refType": "weak" }, "min": 1, "max": "unlimited" } }, - "map_key_value_reference": { + "map_key_value_reference": { "type": { "key": { "type": "uuid", - "refTable": "Child", - "refType": "weak" + "refTable": "Child", + "refType": "weak" }, - "value": { + "value": { "type": "uuid", "refTable": "Child", - "refType": "strong" + "refType": "strong" }, "min": 0, "max": "unlimited" @@ -163,7 +163,7 @@ const referencesTestSchema = ` "type": "string", "mutable": false }, - "strong_atomic_optional_reference": { + "strong_atomic_optional_reference": { "type": { "key": { "type": "uuid", @@ -173,12 +173,12 @@ const referencesTestSchema = ` "max": 1 } }, - "weak_atomic_optional_reference": { + "weak_atomic_optional_reference": { "type": { "key": { "type": "uuid", "refTable": "Grandchild", - "refType": "weak" + "refType": "weak" }, "min": 0, "max": 1 @@ -191,7 +191,7 @@ const referencesTestSchema = ` ] ] }, - "Grandchild": { + "Grandchild": { "columns": { "name": { "type": "string", From 690361fc7d14d68b4c01808473d7e61d01f36e1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Tue, 23 Jan 2024 10:17:33 +0000 Subject: [PATCH 13/13] references: avoid using reflect when comparing GoUUID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jaime Caamaño Ruiz --- updates/references.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/updates/references.go b/updates/references.go index 8c0c37ac..938d02aa 100644 --- a/updates/references.go +++ b/updates/references.go @@ -2,7 +2,6 @@ package updates import ( "fmt" - "reflect" "github.com/ovn-org/libovsdb/database" "github.com/ovn-org/libovsdb/model" @@ -424,13 +423,13 @@ func (rt *referenceTracker) processWeakReferences() (ModelUpdates, error) { return updates, nil } -func copyMapKeyValues(from, to map[interface{}]interface{}, isKey bool, keyValue interface{}) map[interface{}]interface{} { +func copyMapKeyValues(from, to map[interface{}]interface{}, isKey bool, keyValue ovsdb.UUID) map[interface{}]interface{} { if isKey { to[keyValue] = from[keyValue] return to } for key, value := range from { - if reflect.DeepEqual(value, keyValue) { + if value.(ovsdb.UUID) == keyValue { to[key] = from[key] } }