Refactor some of the TUF metadata storage tests to be more general, s…

…o they can be applied to rethink

Signed-off-by: Ying Li <ying.li@docker.com>

buildscripts/integrationtest.sh

@@ -63,4 +63,4 @@ esac
 
 docker-compose -f $composeFile down -v
 
-# docker-compose -f $composeFile up --abort-on-container-exit
+docker-compose -f $composeFile up --abort-on-container-exit

server/storage/memory.go

@@ -65,10 +65,33 @@ func (st *MemStorage) UpdateCurrent(gun string, update MetaUpdate) error {
 
 // UpdateMany updates multiple TUF records
 func (st *MemStorage) UpdateMany(gun string, updates []MetaUpdate) error {
+	st.lock.Lock()
+	defer st.lock.Unlock()
+
+	for _, u := range updates {
+		id := entryKey(gun, u.Role)
+		if space, ok := st.tufMeta[id]; ok {
+			for _, v := range space {
+				if v.version >= u.Version {
+					return &ErrOldVersion{}
+				}
+			}
+		}
+	}
+
 	for _, u := range updates {
-		if err := st.UpdateCurrent(gun, u); err != nil {
-			return err
+		id := entryKey(gun, u.Role)
+
+		version := ver{version: u.Version, data: u.Data, createupdate: time.Now()}
+		st.tufMeta[id] = append(st.tufMeta[id], &version)
+		checksumBytes := sha256.Sum256(u.Data)
+		checksum := hex.EncodeToString(checksumBytes[:])
+
+		_, ok := st.checksums[gun]
+		if !ok {
+			st.checksums[gun] = make(map[string]ver)
 		}
+		st.checksums[gun][checksum] = version
 	}
 	return nil
 }

server/storage/memory_test.go

@@ -9,38 +9,51 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestUpdateCurrent(t *testing.T) {
-	s := NewMemStorage()
-	s.UpdateCurrent("gun", MetaUpdate{"role", 1, []byte("test")})
-
-	k := entryKey("gun", "role")
-	gun, ok := s.tufMeta[k]
-	v := gun[0]
-	require.True(t, ok, "Did not find gun in store")
-	require.Equal(t, 1, v.version, "Version mismatch. Expected 1, found %d", v.version)
-	require.Equal(t, []byte("test"), v.data, "Data was incorrect")
+func assertExpectedMemoryTUFMeta(t *testing.T, expected []tufMeta, s *MemStorage) {
+	counter := make(map[string]int)
+	for _, tufObj := range expected {
+		k := entryKey(tufObj.Gun, tufObj.Role)
+		gun, ok := s.tufMeta[k]
+		require.True(t, len(gun) >= counter[k])
+		v := gun[counter[k]]
+		require.True(t, ok, "Did not find gun in store")
+		require.Equal(t, tufObj.Version, v.version, "Version mismatch. Expected %d, found %d",
+			tufObj.Version, v.version)
+		require.Equal(t, tufObj.Data, v.data, "Data was incorrect")
+		counter[k]++
+	}
 }
 
-func TestUpdateMany(t *testing.T) {
+// UpdateCurrent should succeed if there was no previous metadata of the same
+// gun and role.  They should be gettable.
+func TestMemoryUpdateCurrentEmpty(t *testing.T) {
 	s := NewMemStorage()
-	require.NoError(t, s.UpdateMany("gun", []MetaUpdate{
-		{"role1", 1, []byte("test1")},
-		{"role2", 1, []byte("test2")},
-	}))
+	expected := testUpdateCurrentEmptyStore(t, s)
+	assertExpectedMemoryTUFMeta(t, expected, s)
+}
 
-	_, d, err := s.GetCurrent("gun", "role1")
-	require.Nil(t, err, "Expected error to be nil")
-	require.Equal(t, []byte("test1"), d, "Data was incorrect")
+// UpdateCurrent will successfully add a new (higher) version of an existing TUF file,
+// but will return an error if there is an older version of a TUF file.
+func TestMemoryUpdateCurrentVersionCheck(t *testing.T) {
+	s := NewMemStorage()
+	expected := testUpdateCurrentVersionCheck(t, s)
+	assertExpectedMemoryTUFMeta(t, expected, s)
+}
 
-	_, d, err = s.GetCurrent("gun", "role2")
-	require.Nil(t, err, "Expected error to be nil")
-	require.Equal(t, []byte("test2"), d, "Data was incorrect")
+// UpdateMany succeeds if the updates do not conflict with each other or with what's
+// already in the DB
+func TestMemoryUpdateManyNoConflicts(t *testing.T) {
+	s := NewMemStorage()
+	expected := testUpdateManyNoConflicts(t, s)
+	assertExpectedMemoryTUFMeta(t, expected, s)
+}
 
-	// updating even one with an equal version fails
-	require.IsType(t, &ErrOldVersion{}, s.UpdateMany("gun", []MetaUpdate{
-		{"role1", 1, []byte("test1")},
-		{"role2", 2, []byte("test2")},
-	}))
+// UpdateMany does not insert any rows (or at least rolls them back) if there
+// are any conflicts.
+func TestMemoryUpdateManyConflictRollback(t *testing.T) {
+	s := NewMemStorage()
+	expected := testUpdateManyConflictRollback(t, s)
+	assertExpectedMemoryTUFMeta(t, expected, s)
 }
 
 func TestGetCurrent(t *testing.T) {

server/storage/rethink_realdb_test.go

@@ -28,6 +28,18 @@ func rethinkSessionSetup(t *testing.T) (*gorethink.Session, string) {
 	return sess, rethinkSource
 }
 
+func rethinkDBSetup(t *testing.T) (RethinkDB, func()) {
+	session, _ := rethinkSessionSetup(t)
+	dbName := "testdb"
+	require.NoError(t, rethinkdb.SetupDB(session, dbName, []rethinkdb.Table{
+		TUFFilesRethinkTable,
+		PubKeysRethinkTable,
+	}))
+	return NewRethinkDBStorage(dbName, "", "", session), func() {
+		gorethink.DBDrop(dbName).Exec(session)
+	}
+}
+
 func TestBootstrapSetsUsernamePassword(t *testing.T) {
 	adminSession, source := rethinkSessionSetup(t)
 	dbname, username, password := "testdb", "testuser", "testpassword"
@@ -59,3 +71,41 @@ func TestBootstrapSetsUsernamePassword(t *testing.T) {
 	require.Error(t, err)
 	require.IsType(t, ErrNotFound{}, err)
 }
+
+// UpdateCurrent will add a new TUF file if no previous version of that gun and role existed.
+func TestRethinkUpdateCurrentEmpty(t *testing.T) {
+	dbStore, cleanup := rethinkDBSetup(t)
+	defer cleanup()
+
+	testUpdateCurrentEmptyStore(t, dbStore)
+}
+
+// UpdateCurrent will add a new TUF file if the version is higher than previous, but fail
+// if the version is equal to or less than the current, whether or not that previous
+// version exists
+func TestRethinkUpdateCurrentVersionCheck(t *testing.T) {
+	t.Skip("Currently rethink only errors if the previous version exists - it doesn't check for strictly increasing")
+	dbStore, cleanup := rethinkDBSetup(t)
+	defer cleanup()
+
+	testUpdateCurrentVersionCheck(t, dbStore)
+}
+
+// UpdateMany succeeds if the updates do not conflict with each other or with what's
+// already in the DB
+func TestRethinkUpdateManyNoConflicts(t *testing.T) {
+	dbStore, cleanup := rethinkDBSetup(t)
+	defer cleanup()
+
+	testUpdateManyNoConflicts(t, dbStore)
+}
+
+// UpdateMany does not insert any rows (or at least rolls them back) if there
+// are any conflicts.
+func TestRethinkUpdateManyConflictRollback(t *testing.T) {
+	t.Skip("This doesn't work for some reason, still need to debug")
+	dbStore, cleanup := rethinkDBSetup(t)
+	defer cleanup()
+
+	testUpdateManyConflictRollback(t, dbStore)
+}

server/storage/rethinkdb.go

@@ -8,6 +8,7 @@ import (
 	"sort"
 	"time"
 
+	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary/storage/rethinkdb"
 	"github.com/docker/notary/tuf/data"
 	"gopkg.in/dancannon/gorethink.v2"
@@ -233,7 +234,11 @@ func (rdb RethinkDB) UpdateMany(gun string, updates []MetaUpdate) error {
 	for _, up := range updates {
 		if err := rdb.UpdateCurrentWithTSChecksum(gun, tsChecksum, up); err != nil {
 			// roll back with best-effort deletion, and then error out
-			rdb.deleteByTSChecksum(tsChecksum)
+			rollbackErr := rdb.deleteByTSChecksum(tsChecksum)
+			if rollbackErr != nil {
+				logrus.Errorf("Unable to rollback DB conflict - items with timestamp_checksum %s: %v",
+					tsChecksum, rollbackErr)
+			}
 			return err
 		}
 	}

server/storage/sqldb_test.go

@@ -64,121 +64,79 @@ func SampleUpdate(version int) MetaUpdate {
 	}
 }
 
-// TestSQLUpdateCurrent asserts that UpdateCurrent will add a new TUF file
-// if no previous version existed.
-func TestSQLUpdateCurrentNew(t *testing.T) {
-	dbStore, cleanup := sqldbSetup(t)
-	defer cleanup()
-
-	// Adding a new TUF file should succeed
-	err := dbStore.UpdateCurrent("testGUN", SampleUpdate(0))
-	require.NoError(t, err, "Creating a row in an empty DB failed.")
+func assertExpectedGormTUFMeta(t *testing.T, expected []tufMeta, gormDB gorm.DB) {
+	expectedGorm := make([]TUFFile, len(expected))
+	for i, tufObj := range expected {
+		expectedGorm[i] = TUFFile{
+			Model:   gorm.Model{ID: uint(i + 1)},
+			Gun:     tufObj.Gun,
+			Role:    tufObj.Role,
+			Version: tufObj.Version,
+			Sha256:  tufObj.Sha256,
+			Data:    tufObj.Data,
+		}
+	}
 
 	// There should just be one row
 	var rows []TUFFile
-	query := dbStore.DB.Select("id, gun, role, version, sha256, data").Find(&rows)
+	query := gormDB.Select("id, gun, role, version, sha256, data").Find(&rows)
 	require.NoError(t, query.Error)
-
-	expected := SampleTUF(0)
-	expected.ID = 1
-	require.Equal(t, []TUFFile{expected}, rows)
+	require.Equal(t, expectedGorm, rows)
 }
 
-// TestSQLUpdateCurrentNewVersion asserts that UpdateCurrent will add a
-// new (higher) version of an existing TUF file
-func TestSQLUpdateCurrentNewVersion(t *testing.T) {
+// TestSQLUpdateCurrent asserts that UpdateCurrent will add a new TUF file
+// if no previous version of that gun and role existed.
+func TestSQLUpdateCurrentEmpty(t *testing.T) {
 	dbStore, cleanup := sqldbSetup(t)
 	defer cleanup()
 
-	// insert row
-	oldVersion := SampleTUF(0)
-	query := dbStore.DB.Create(&oldVersion)
-	require.NoError(t, query.Error, "Creating a row in an empty DB failed.")
-
-	// UpdateCurrent with a newer version should succeed
-	update := SampleUpdate(2)
-	err := dbStore.UpdateCurrent("testGUN", update)
-	require.NoError(t, err, "Creating a row in an empty DB failed.")
-
-	// There should just be one row
-	var rows []TUFFile
-	query = dbStore.DB.Select("id, gun, role, version, sha256, data").Find(&rows)
-	require.NoError(t, query.Error)
+	expected := testUpdateCurrentEmptyStore(t, dbStore)
+	assertExpectedGormTUFMeta(t, expected, dbStore.DB)
 
-	oldVersion.Model = gorm.Model{ID: 1}
-	expected := SampleTUF(2)
-	expected.Model = gorm.Model{ID: 2}
-	require.Equal(t, []TUFFile{oldVersion, expected}, rows)
+	dbStore.DB.Close()
 }
 
-// TestSQLUpdateCurrentOldVersionError asserts that an error is raised if
+// TestSQLUpdateCurrentNewVersion asserts that UpdateCurrent will add a
+// new (higher) version of an existing TUF file, and that an error is raised if
 // trying to update to an older version of a TUF file.
-func TestSQLUpdateCurrentOldVersionError(t *testing.T) {
+func TestSQLUpdateCurrentNewVersion(t *testing.T) {
 	dbStore, cleanup := sqldbSetup(t)
 	defer cleanup()
 
-	// insert row
-	newVersion := SampleTUF(3)
-	query := dbStore.DB.Create(&newVersion)
-	require.NoError(t, query.Error, "Creating a row in an empty DB failed.")
+	expected := testUpdateCurrentVersionCheck(t, dbStore)
+	assertExpectedGormTUFMeta(t, expected, dbStore.DB)
 
-	// UpdateCurrent should fail due to the version being lower than the
-	// previous row
-	err := dbStore.UpdateCurrent("testGUN", SampleUpdate(0))
-	require.Error(t, err, "Error should not be nil")
-	require.IsType(t, &ErrOldVersion{}, err,
-		"Expected ErrOldVersion error type, got: %v", err)
+	dbStore.DB.Close()
+}
 
-	// There should just be one row
-	var rows []TUFFile
-	query = dbStore.DB.Select("id, gun, role, version, sha256, data").Find(&rows)
-	require.NoError(t, query.Error)
+// TestSQLUpdateManyNoConflicts asserts that inserting multiple updates succeeds if the
+// updates do not conflict with each other or with the DB, even if there are
+// 2 versions of the same role/gun in a non-monotonic order.
+func TestSQLUpdateManyNoConflicts(t *testing.T) {
+	dbStore, cleanup := sqldbSetup(t)
+	defer cleanup()
 
-	newVersion.Model = gorm.Model{ID: 1}
-	require.Equal(t, []TUFFile{newVersion}, rows)
+	expected := testUpdateManyNoConflicts(t, dbStore)
+	assertExpectedGormTUFMeta(t, expected, dbStore.DB)
 
 	dbStore.DB.Close()
 }
 
-// TestSQLUpdateMany asserts that inserting multiple updates succeeds if the
-// updates do not conflict with each.
-func TestSQLUpdateMany(t *testing.T) {
+// TestSQLUpdateManyConflictRollback asserts that no data ends up in the DB if there is
+// a conflict
+func TestSQLUpdateManyConflictRollback(t *testing.T) {
 	dbStore, cleanup := sqldbSetup(t)
 	defer cleanup()
 
-	err := dbStore.UpdateMany("testGUN", []MetaUpdate{
-		SampleUpdate(0),
-		{
-			Role:    "targets",
-			Version: 1,
-			Data:    []byte("2"),
-		},
-		SampleUpdate(2),
-	})
-	require.NoError(t, err, "UpdateMany errored unexpectedly: %v", err)
-
-	gorm1 := SampleTUF(0)
-	gorm1.ID = 1
-	data := []byte("2")
-	checksum := sha256.Sum256(data)
-	hexChecksum := hex.EncodeToString(checksum[:])
-	gorm2 := TUFFile{
-		Model: gorm.Model{ID: 2}, Gun: "testGUN", Role: "targets",
-		Version: 1, Sha256: hexChecksum, Data: data}
-	gorm3 := SampleTUF(2)
-	gorm3.ID = 3
-	expected := []TUFFile{gorm1, gorm2, gorm3}
-
-	var rows []TUFFile
-	query := dbStore.DB.Select("id, gun, role, version, sha256, data").Find(&rows)
-	require.NoError(t, query.Error)
-	require.Equal(t, expected, rows)
+	expected := testUpdateManyConflictRollback(t, dbStore)
+	assertExpectedGormTUFMeta(t, expected, dbStore.DB)
 
 	dbStore.DB.Close()
 }
 
 // TestSQLUpdateManyVersionOrder asserts that inserting updates with
 // non-monotonic versions still succeeds.
+// TODO: this seems like just wrong behavior
 func TestSQLUpdateManyVersionOrder(t *testing.T) {
 	dbStore, cleanup := sqldbSetup(t)
 	defer cleanup()
@@ -215,8 +173,6 @@ func TestSQLUpdateManyDuplicateRollback(t *testing.T) {
 	require.IsType(t, &ErrOldVersion{}, err,
 		"UpdateMany returned wrong error type")
 
-	// the whole transaction should have rolled back, so there should be
-	// no entries.
 	var count int
 	query := dbStore.DB.Model(&TUFFile{}).Count(&count)
 	require.NoError(t, query.Error)