Introduce number column for area and iteration and allow searching it

iteration/iteration_test.go

@@ -48,7 +48,7 @@ func (s *TestIterationRepository) TestCreate() {
 		// then
 		require.NoError(t, err)
 		require.NotEqual(t, uuid.Nil, i.ID, "iteration not created, ID is nil")
-		// require.True(t, i.CreatedAt.After(time.Now()), "iteration was not created, CreatedAt after Now()?")
+		require.False(t, i.CreatedAt.After(time.Now()), "iteration was not created, CreatedAt after Now()?")
 		assert.Equal(t, start, *i.StartAt)
 		assert.Equal(t, end, *i.EndAt)
 		assert.Equal(t, name, i.Name)

migration/migration_blackbox_test.go

@@ -1452,6 +1452,9 @@ func testMigration106NumberSequences(t *testing.T) {
 			delete(toBeFound, seq)
 		}
 		require.Empty(t, toBeFound, "failed to find these number sequences: %+v", spew.Sdump(toBeFound))
+
+		require.True(t, dialect.HasIndex("iterations", "iterations_space_id_number_idx"))
+		require.True(t, dialect.HasIndex("areas", "areas_space_id_number_idx"))
 	})
 }
 

migration/sql-files/106-number-sequences.sql

@@ -22,10 +22,13 @@ UPDATE areas SET number = seq.row_number
 FROM (SELECT id, space_id, created_at, row_number() OVER (PARTITION BY space_id ORDER BY created_at ASC) FROM areas) AS seq
 WHERE areas.id = seq.id;
 
--- Make number a required column
+-- Make number a required column and add an index for faster querying over space_id and number
 ALTER TABLE iterations ALTER COLUMN number SET NOT NULL;
 ALTER TABLE areas ALTER COLUMN number SET NOT NULL;
 
+CREATE UNIQUE INDEX iterations_space_id_number_idx ON iterations USING BTREE (space_id, number);
+CREATE UNIQUE INDEX areas_space_id_number_idx ON areas USING BTREE (space_id, number);
+
 -- Update the number_sequences table with the maximum for each table type
 
 INSERT INTO number_sequences (space_id, table_name, current_val)

numbersequence/human_friendly_number.go

@@ -1,6 +1,8 @@
 package numbersequence
 
 import (
+	"fmt"
+
 	"github.com/fabric8-services/fabric8-wit/convert"
 	"github.com/fabric8-services/fabric8-wit/log"
 	"github.com/jinzhu/gorm"
@@ -9,9 +11,11 @@ import (
 )
 
 // The HumanFriendlyNumber struct can be embedded in all model structs that want
-// to have an automatically incremented human friendly number (e.g. 1,10,23).
+// to have an automatically incremented human friendly number (1, 2, 3, 4, ...),
 // Such a number is unique within the space and for the given table name (e.g.
-// 'work_items', 'iterations', 'areas').
+// "work_items", "iterations", "areas"). Once a model has received a number
+// during the creation phase (on database INSERT), any followup call to the
+// model's `Save()` function will prohibit changing the number.
 type HumanFriendlyNumber struct {
 	Number    int       `json:"number,omitempty"`
 	spaceID   uuid.UUID `gorm:"-"`
@@ -79,3 +83,20 @@ func (n *HumanFriendlyNumber) BeforeCreate(scope *gorm.Scope) error {
 	n.Number = currentVal
 	return scope.SetColumn("number", n.Number)
 }
+
+// BeforeUpdate is a GORM callback (see http://doc.gorm.io/callbacks.html) that
+// will be called before updating the model. We use it to check for number
+// compatibility by adding this condition to the WHERE clause of the UPDATE:
+//
+//    AND number=<NUMBER-OF-THE-MODEL>
+//
+// This guarantees that you cannot change the number on the model when you
+// update it. The UPDATE will affect no rows!
+//
+// NOTE(kwk): We could have used scope.Search.Omit("number") in order to avoid
+// setting the number, but there is practically no way to tell back to the
+// model, that we have ignored the number column.
+func (n *HumanFriendlyNumber) BeforeUpdate(scope *gorm.Scope) error {
+	scope.Search.Where(fmt.Sprintf(`"%s"."number"=?`, scope.TableName()), n.Number)
+	return nil
+}

numbersequence/human_friendly_number_test.go

@@ -57,6 +57,19 @@ func TestHumanFriendlyNumberSuite(t *testing.T) {
 const tableName1 = "human_friendly_number_test1"
 const tableName2 = "human_friendly_number_test2"
 
+// SetupTest implements suite.SetupTest
+func (s *humanFriendlyNumberSuite) SetupTest() {
+	s.DBTestSuite.SetupTest()
+	// Prepare a table for our model test structures to persist their data in.
+	db := s.DB.Exec(fmt.Sprintf(`
+		DROP TABLE IF EXISTS %[1]q;
+		DROP TABLE IF EXISTS %[2]q;
+		CREATE TABLE %[1]q ( id uuid PRIMARY KEY, number int, message text );
+		CREATE TABLE %[2]q ( id uuid PRIMARY KEY, number int, message text );
+	`, tableName1, tableName2))
+	require.NoError(s.T(), db.Error)
+}
+
 type testStruct1 struct {
 	numbersequence.HumanFriendlyNumber
 	ID      uuid.UUID `json:"id" gorm:"primary_key"`
@@ -77,19 +90,13 @@ func (s testStruct2) TableName() string {
 	return tableName2
 }
 
-// TestBeforeCreate tests that two model types (testStruct1 and testStruct2) get
-// numbers upon creation of model. Numbers are partitioned by space and table
-// name.
+// TestBeforeCreate tests that two model types (testStruct1 and
+// testStruct2) get numbers upon creation of model. Numbers are partitioned by
+// space and table name.
 func (s *humanFriendlyNumberSuite) TestBeforeCreate() {
 	// given
 	fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(2))
-	db := s.DB.Exec(fmt.Sprintf(`
-		DROP TABLE IF EXISTS %[1]q;
-		DROP TABLE IF EXISTS %[2]q;
-		CREATE TABLE %[1]q ( id uuid PRIMARY KEY, number int, message text );
-		CREATE TABLE %[2]q ( id uuid PRIMARY KEY, number int, message text );
-	`, tableName1, tableName2))
-	require.NoError(s.T(), db.Error)
+	db := s.DB
 
 	// create models of two types in two spaces and check that the numbers are
 	// assigned as expected.
@@ -122,6 +129,36 @@ func (s *humanFriendlyNumberSuite) TestBeforeCreate() {
 	}
 }
 
+// TestBeforeUpdate tests that you cannot change an already given number on a
+// model when you update that model.
+func (s *humanFriendlyNumberSuite) TestBeforeUpdate() {
+	// given
+	fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(1))
+	db := s.DB
+	model := testStruct1{ID: uuid.NewV4(), Message: "first", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[0].ID, tableName1)}
+	db = db.Create(&model)
+	require.NoError(s.T(), db.Error)
+	s.T().Run("update message", func(t *testing.T) {
+		// when updating the message
+		model.Message = "new message"
+		db = db.Save(&model)
+		// then the number should stay the same
+		require.NoError(t, db.Error)
+		require.Equal(t, 1, model.Number)
+		require.Equal(t, "new message", model.Message)
+	})
+	s.T().Run("update number", func(t *testing.T) {
+		// when updating the message and the number
+		model.Message = "new message 2"
+		model.Number = 2
+		db = db.Save(&model)
+		// then there should be no rows affected because we're not supposed to
+		// update the number
+		require.NoError(t, db.Error)
+		require.Equal(t, int64(0), db.RowsAffected)
+	})
+}
+
 func (s *humanFriendlyNumberSuite) TestConcurrentCreate() {
 	// given
 	fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(1))