WIP: Introduce gormsupport.Versioning to auto increment a model's Version on update and set to 0 on create

area/area.go

@@ -23,11 +23,11 @@ const APIStringTypeAreas = "areas"
 // Area describes a single Area
 type Area struct {
 	gormsupport.Lifecycle
+	gormsupport.Versioning
 	ID      uuid.UUID `sql:"type:uuid default uuid_generate_v4()" gorm:"primary_key"` // This is the ID PK field
 	SpaceID uuid.UUID `sql:"type:uuid"`
 	Path    path.Path
 	Name    string
-	Version int
 }
 
 // MakeChildOf does all the path magic to make the current area a child of the

controller/area_blackbox_test.go

@@ -305,8 +305,8 @@ func (rest *TestAreaREST) TestShowChildrenArea() {
 
 func ConvertAreaToModel(appArea app.AreaSingle) area.Area {
 	return area.Area{
-		ID:      *appArea.Data.ID,
-		Version: *appArea.Data.Attributes.Version,
+		ID:         *appArea.Data.ID,
+		Versioning: gormsupport.Versioning{Version: *appArea.Data.Attributes.Version},
 		Lifecycle: gormsupport.Lifecycle{
 			UpdatedAt: *appArea.Data.Attributes.UpdatedAt,
 		},

controller/work_item_link_category_whitebox_test.go

@@ -19,7 +19,6 @@ func TestWorkItemLinkCategory_ConvertLinkCategoryFromModel(t *testing.T) {
 		ID:          uuid.FromStringOrNil("0e671e36-871b-43a6-9166-0c4bd573e231"),
 		Name:        "Example work item link category",
 		Description: &description,
-		Version:     0,
 	}
 
 	expected := app.WorkItemLinkCategorySingle{

controller/workitemtype_blackbox_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/fabric8-services/fabric8-wit/gormsupport"
+
 	"github.com/fabric8-services/fabric8-wit/account"
 	"github.com/fabric8-services/fabric8-wit/app"
 	"github.com/fabric8-services/fabric8-wit/app/test"
@@ -113,8 +115,8 @@ func (s *workItemTypeSuite) TestShow() {
 // used for testing purpose only
 func ConvertWorkItemTypeToModel(data app.WorkItemTypeData) workitem.WorkItemType {
 	return workitem.WorkItemType{
-		ID:      *data.ID,
-		Version: *data.Attributes.Version,
+		ID:         *data.ID,
+		Versioning: gormsupport.Versioning{Version: *data.Attributes.Version},
 	}
 }
 

controller/workitemtype_whitebox_test.go

@@ -39,7 +39,7 @@ func TestConvertTypeFromModel(t *testing.T) {
 		},
 		Name:        "foo",
 		Description: &descFoo,
-		Version:     42,
+		Versioning:  gormsupport.Versioning{Version: 42},
 		Path:        "something",
 		Fields: map[string]workitem.FieldDefinition{
 			"aListType": {

gormsupport/versioning.go

@@ -0,0 +1,68 @@
+package gormsupport
+
+import (
+	"fmt"
+
+	"github.com/fabric8-services/fabric8-wit/convert"
+	"github.com/jinzhu/gorm"
+)
+
+// Versioning can be embedded into model structs that want to have a Version
+// column which will automatically be incremented before each UPDATE, set to 0
+// on CREATE, and checked for compatibility on each UPDATE.
+//
+// For the first creation of a model the initial version will always be
+// overwritten with 0 nomatter what the user specified in the model itself. The
+// model itself is not changed in any cases, just the DB query for INSERT and
+// UPDATE is touched.
+//
+// We also add
+//
+//    AND version=<VERSION-OF-THE-MODEL>
+//
+// to the WHERE conditions of the UPDATE part.
+type Versioning struct {
+	Version int `json:"version"`
+}
+
+// BeforeUpdate is a GORM callback (see http://doc.gorm.io/callbacks.html) that
+// will be called before updating the model. We use it to automatically
+// increment the version number before saving the model and to check for version
+// compatibility by adding this condition to the WHERE clause of the UPDATE:
+//
+//    AND version=<VERSION-OF-THE-MODEL>
+func (v *Versioning) BeforeUpdate(scope *gorm.Scope) error {
+	scope.Search.Where(fmt.Sprintf(`"%s"."version"=?`, scope.TableName()), v.Version)
+	return scope.SetColumn("version", v.Version+1)
+}
+
+// BeforeCreate is a GORM callback (see http://doc.gorm.io/callbacks.html) that
+// will be called before creating the model. We use it to automatically
+// have the first version of the model set to 0.
+func (v *Versioning) BeforeCreate(scope *gorm.Scope) error {
+	return scope.SetColumn("version", 0)
+}
+
+// BeforeDelete is a GORM callback (see http://doc.gorm.io/callbacks.html) that
+// will be called before soft-deleting the model. We use it to check for version
+// compatibility by adding this condition to the WHERE clause of the deletion:
+//
+//    AND version=<VERSION-OF-THE-MODEL>
+func (v *Versioning) BeforeDelete(scope *gorm.Scope) error {
+	scope.Search.Where(fmt.Sprintf(`"%s"."version"=?`, scope.TableName()), v.Version)
+	return nil
+}
+
+// Ensure Versioning implements the Equaler interface
+var _ convert.Equaler = Versioning{}
+var _ convert.Equaler = (*Versioning)(nil)
+
+// Equal returns true if two Versioning objects are equal; otherwise false is
+// returned.
+func (v Versioning) Equal(u convert.Equaler) bool {
+	other, ok := u.(Versioning)
+	if !ok {
+		return false
+	}
+	return v.Version == other.Version
+}

gormsupport/versioning_blackbox_test.go

@@ -0,0 +1,116 @@
+package gormsupport_test
+
+import (
+	"testing"
+
+	"github.com/fabric8-services/fabric8-wit/convert"
+	"github.com/fabric8-services/fabric8-wit/gormsupport"
+	"github.com/fabric8-services/fabric8-wit/gormtestsupport"
+	"github.com/fabric8-services/fabric8-wit/resource"
+	"github.com/fabric8-services/fabric8-wit/workitem/link"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+)
+
+func TestVersioning_Equal(t *testing.T) {
+	t.Parallel()
+	resource.Require(t, resource.UnitTest)
+
+	a := gormsupport.Versioning{
+		Version: 42,
+	}
+
+	t.Run("equality", func(t *testing.T) {
+		b := gormsupport.Versioning{
+			Version: 42,
+		}
+		require.True(t, a.Equal(b))
+	})
+	t.Run("type difference", func(t *testing.T) {
+		b := convert.DummyEqualer{}
+		require.False(t, a.Equal(b))
+	})
+	t.Run("version difference", func(t *testing.T) {
+		b := gormsupport.Versioning{
+			Version: 123,
+		}
+		require.False(t, a.Equal(b))
+	})
+}
+
+type VersioningSuite struct {
+	gormtestsupport.DBTestSuite
+}
+
+func TestVersioningSuite(t *testing.T) {
+	suite.Run(t, &VersioningSuite{DBTestSuite: gormtestsupport.NewDBTestSuite()})
+}
+func (s *VersioningSuite) TestCallbacks() {
+	// given a work item link category that embeds the Versioning struct is a
+	// good way to demonstrate the way the callbacks work.
+	cat := link.WorkItemLinkCategory{
+		Name: "foo",
+	}
+	// set the version to something else than 0 the BeforeCreate callback will
+	// automatically set this to 0 before entering it in the DB.
+	cat.Version = 123
+
+	newName := "new name"
+
+	s.T().Run("before create", func(t *testing.T) {
+		// when
+		err := s.DB.Create(&cat).Error
+		// then
+		require.NoError(t, err)
+		require.Equal(t, 0, cat.Version, "initial version of entity must be 0 nomatter what the given version was")
+	})
+	s.T().Run("before update", func(t *testing.T) {
+		t.Run("allowed because versions match", func(t *testing.T) {
+			// given
+			cat.Name = newName
+			// when
+			db := s.DB.Save(&cat)
+			// then
+			require.NoError(t, db.Error)
+			require.Equal(t, int64(1), db.RowsAffected)
+			require.Equal(t, 1, cat.Version, "followup version of entity must be 1")
+			require.Equal(t, newName, cat.Name)
+		})
+		t.Run("no update because versions mismatch", func(t *testing.T) {
+			// given
+			cat.Name = "not used"
+			cat.Version = 42
+			// when
+			db := s.DB.Save(&cat)
+			// then
+			require.NoError(t, db.Error)
+			require.Equal(t, int64(0), db.RowsAffected)
+			require.Equal(t, newName, cat.Name, "name should not have been updated")
+		})
+	})
+	s.T().Run("before delete", func(t *testing.T) {
+		t.Run("no delete because versions mismatch", func(t *testing.T) {
+			// given
+			cat.Version = 42
+			// when
+			db := s.DB.Delete(&cat)
+			// then
+			require.NoError(t, db.Error)
+			require.Equal(t, int64(0), db.RowsAffected, "the delete should have failed because of wrong version")
+			require.Equal(t, newName, cat.Name, "name should not have been updated")
+		})
+		t.Run("allowed because versions match", func(t *testing.T) {
+			// given
+			cat.Version = 1
+			// when
+			db := s.DB.Delete(&cat)
+			// then
+			require.NoError(t, db.Error)
+			require.Equal(t, int64(1), db.RowsAffected, "the delete should have worked")
+			loadedCat := link.WorkItemLinkCategory{}
+			db = s.DB.Where("id = ?", cat.ID).First(&loadedCat)
+			require.Error(t, db.Error)
+			require.Equal(t, "record not found", db.Error.Error())
+		})
+	})
+}

label/label.go

@@ -22,13 +22,13 @@ const APIStringTypeLabels = "labels"
 // Label describes a single Label
 type Label struct {
 	gormsupport.Lifecycle
+	gormsupport.Versioning
 	ID              uuid.UUID `sql:"type:uuid default uuid_generate_v4()" gorm:"primary_key"` // This is the ID PK field
 	SpaceID         uuid.UUID `sql:"type:uuid"`
 	Name            string
 	TextColor       string `sql:"DEFAULT:#000000"`
 	BackgroundColor string `sql:"DEFAULT:#FFFFFF"`
 	BorderColor     string `sql:"DEFAULT:#000000"`
-	Version         int
 }
 
 // GetETagData returns the field values to use to generate the ETag
@@ -101,8 +101,6 @@ func (m *GormLabelRepository) Save(ctx context.Context, l Label) (*Label, error)
 	}
 	lbl := Label{}
 	tx := m.db.Where("id = ?", l.ID).First(&lbl)
-	oldVersion := l.Version
-	l.Version = lbl.Version + 1
 	if tx.RecordNotFound() {
 		log.Error(ctx, map[string]interface{}{
 			"label_id": l.ID,
@@ -116,7 +114,7 @@ func (m *GormLabelRepository) Save(ctx context.Context, l Label) (*Label, error)
 		}, "unknown error happened when searching the label")
 		return nil, errors.NewInternalError(ctx, err)
 	}
-	tx = tx.Where("Version = ?", oldVersion).Save(&l)
+	tx = tx.Save(&l)
 	if err := tx.Error; err != nil {
 		// combination of name and space ID should be unique
 		if gormsupport.IsUniqueViolation(err, "labels_name_space_id_unique_idx") {

query/query.go

@@ -24,12 +24,12 @@ const APIStringTypeQuery = "queries"
 // Query describes a single Query
 type Query struct {
 	gormsupport.Lifecycle
+	gormsupport.Versioning
 	ID      uuid.UUID `sql:"type:uuid default uuid_generate_v4()" gorm:"primary_key"` // This is the ID PK field
 	SpaceID uuid.UUID `sql:"type:uuid"`
 	Creator uuid.UUID `sql:"type:uuid"`
 	Title   string
 	Fields  string
-	Version int
 }
 
 // QueryTableName constant that holds table name of Queries
@@ -130,8 +130,6 @@ func (r *GormQueryRepository) Save(ctx context.Context, q Query) (*Query, error)
 	}
 	qry := Query{}
 	tx := r.db.Where("id = ?", q.ID).First(&qry)
-	oldVersion := q.Version
-	q.Version = qry.Version + 1
 	if tx.RecordNotFound() {
 		log.Error(ctx, map[string]interface{}{
 			"query_id": q.ID,
@@ -145,7 +143,7 @@ func (r *GormQueryRepository) Save(ctx context.Context, q Query) (*Query, error)
 		}, "unknown error happened when searching the query")
 		return nil, errors.NewInternalError(ctx, err)
 	}
-	tx = tx.Where("Version = ?", oldVersion).Save(&q)
+	tx = tx.Save(&q)
 	if err := tx.Error; err != nil {
 		// combination of name and space ID should be unique
 		if gormsupport.IsUniqueViolation(err, "queries_title_space_id_creator_unique") {

space/space.go

@@ -29,8 +29,8 @@ var (
 // Space represents a Space on the domain and db layer
 type Space struct {
 	gormsupport.Lifecycle
+	gormsupport.Versioning
 	ID              uuid.UUID
-	Version         int
 	Name            string
 	Description     string
 	OwnerID         uuid.UUID `sql:"type:uuid"` // Belongs To Identity
@@ -51,7 +51,7 @@ func (p Space) Equal(u convert.Equaler) bool {
 	if !lfEqual {
 		return false
 	}
-	if p.Version != other.Version {
+	if !p.Versioning.Equal(other.Versioning) {
 		return false
 	}
 	if p.Name != other.Name {
@@ -219,7 +219,6 @@ func (r *GormRepository) Save(ctx context.Context, p *Space) (*Space, error) {
 	pr := Space{}
 	tx := r.db.Where("id=?", p.ID).First(&pr)
 	oldVersion := p.Version
-	p.Version++
 	if tx.RecordNotFound() {
 		// treating this as a not found error: the fact that we're using number internal is implementation detail
 		return nil, errors.NewNotFoundError("space", p.ID.String())
@@ -231,7 +230,7 @@ func (r *GormRepository) Save(ctx context.Context, p *Space) (*Space, error) {
 		}, "unable to find the space by ID")
 		return nil, errors.NewInternalError(ctx, err)
 	}
-	tx = tx.Where("Version = ?", oldVersion).Save(p)
+	tx = tx.Save(p)
 	if err := tx.Error; err != nil {
 		if gormsupport.IsCheckViolation(tx.Error, "spaces_name_check") {
 			return nil, errors.NewBadParameterError("Name", p.Name).Expected("not empty")

space/space_test.go

@@ -172,7 +172,6 @@ func (s *SpaceRepositoryTestSuite) TestSave() {
 		// given a space with a not existing ID
 		p := space.Space{
 			ID:              uuid.NewV4(),
-			Version:         0,
 			Name:            testsupport.CreateRandomValidTestName("some space"),
 			SpaceTemplateID: fxt.SpaceTemplates[0].ID,
 		}

workitem/link/category.go

@@ -17,14 +17,13 @@ var (
 // WorkItemLinkCategory represents the category of a work item link as it is stored in the db
 type WorkItemLinkCategory struct {
 	gormsupport.Lifecycle
+	gormsupport.Versioning
 	// ID
 	ID uuid.UUID `sql:"type:uuid default uuid_generate_v4()" gorm:"primary_key"`
 	// Name is the unique name of this work item link category.
 	Name string
 	// Description is an optional description of the work item link category
 	Description *string
-	// Version for optimistic concurrency control
-	Version int
 }
 
 // Ensure Fields implements the Equaler interface
@@ -43,7 +42,7 @@ func (c WorkItemLinkCategory) Equal(u convert.Equaler) bool {
 	if c.Name != other.Name {
 		return false
 	}
-	if c.Version != other.Version {
+	if !c.Versioning.Equal(other.Versioning) {
 		return false
 	}
 	if !reflect.DeepEqual(c.Description, other.Description) {

workitem/link/category_blackbox_test.go

@@ -20,7 +20,6 @@ func TestWorkItemLinkCategory_Equal(t *testing.T) {
 		ID:          uuid.FromStringOrNil("0e671e36-871b-43a6-9166-0c4bd573e231"),
 		Name:        "Example work item link category",
 		Description: &description,
-		Version:     0,
 	}
 
 	t.Run("types", func(t *testing.T) {

workitem/link/category_repository.go

@@ -143,7 +143,6 @@ func (r *GormWorkItemLinkCategoryRepository) Save(ctx context.Context, linkCat W
 	}
 	newLinkCat := WorkItemLinkCategory{
 		ID:          linkCat.ID,
-		Version:     linkCat.Version + 1,
 		Name:        linkCat.Name,
 		Description: linkCat.Description,
 	}