diff --git a/area/area.go b/area/area.go index a98b5f6ecc..7202a349ea 100644 --- a/area/area.go +++ b/area/area.go @@ -8,7 +8,6 @@ import ( "github.com/fabric8-services/fabric8-wit/errors" "github.com/fabric8-services/fabric8-wit/gormsupport" "github.com/fabric8-services/fabric8-wit/log" - "github.com/fabric8-services/fabric8-wit/numbersequence" "github.com/fabric8-services/fabric8-wit/path" "fmt" @@ -24,7 +23,6 @@ const APIStringTypeAreas = "areas" // Area describes a single Area type Area struct { gormsupport.Lifecycle - numbersequence.HumanFriendlyNumber 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 @@ -83,8 +81,6 @@ type GormAreaRepository struct { func (m *GormAreaRepository) Create(ctx context.Context, u *Area) error { defer goa.MeasureSince([]string{"goa", "db", "area", "create"}, time.Now()) - u.HumanFriendlyNumber = numbersequence.NewHumanFriendlyNumber(u.SpaceID, u.TableName()) - if u.ID == uuid.Nil { u.ID = uuid.NewV4() u.Path = path.Path{u.ID} diff --git a/area/area_blackbox_test.go b/area/area_blackbox_test.go index 4592717b71..d9476b935c 100644 --- a/area/area_blackbox_test.go +++ b/area/area_blackbox_test.go @@ -49,11 +49,11 @@ func (s *TestAreaRepository) TestCreateAreaWithSameNameFail() { assert.IsType(s.T(), errors.DataConflictError{}, errs.Cause(err)) } -func (s *TestAreaRepository) TestCreate() { +func (s *TestAreaRepository) TestCreateArea() { // given repo := area.NewAreaRepository(s.DB) name := "TestCreateArea" - fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(2)) + fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(1)) a := area.Area{ Name: name, SpaceID: fxt.Spaces[0].ID, @@ -65,19 +65,6 @@ func (s *TestAreaRepository) TestCreate() { require.NotEqual(s.T(), uuid.Nil, a.ID) assert.True(s.T(), !a.CreatedAt.After(time.Now()), "Area was not created, CreatedAt after Now()?") assert.Equal(s.T(), name, a.Name) - assert.Equal(s.T(), 1, a.Number) - s.T().Run("second area in same space gets a sequential number", func(t *testing.T) { - a := area.Area{Name: "second area", SpaceID: fxt.Spaces[0].ID} - err := repo.Create(context.Background(), &a) - require.NoError(t, err) - require.Equal(t, 2, a.Number) - }) - s.T().Run("first area in another space starts numbering at 1", func(t *testing.T) { - a := area.Area{Name: "first area", SpaceID: fxt.Spaces[1].ID} - err := repo.Create(context.Background(), &a) - require.NoError(t, err) - require.Equal(t, 1, a.Number) - }) } func (s *TestAreaRepository) TestExistsArea() { diff --git a/controller/area.go b/controller/area.go index 68f0ddabdf..b19456f1b1 100644 --- a/controller/area.go +++ b/controller/area.go @@ -212,7 +212,6 @@ func ConvertArea(db application.DB, request *http.Request, ar area.Area, options UpdatedAt: &ar.UpdatedAt, Version: &ar.Version, ParentPath: ptr.String(ar.Path.ParentPath().String()), - Number: &ar.Number, }, Relationships: &app.AreaRelations{ Space: &app.RelationGeneric{ diff --git a/controller/iteration.go b/controller/iteration.go index 7f63c45463..22697d4e9d 100644 --- a/controller/iteration.go +++ b/controller/iteration.go @@ -475,7 +475,6 @@ func ConvertIteration(request *http.Request, itr iteration.Iteration, additional ParentPath: &pathToTopMostParent, UserActive: &itr.UserActive, ActiveStatus: &activeStatus, - Number: &itr.Number, }, Relationships: &app.IterationRelations{ Space: &app.RelationGeneric{ diff --git a/controller/test-files/area/create/ok.child1_ok.res.payload.golden.json b/controller/test-files/area/create/ok.child1_ok.res.payload.golden.json index 50d30f3695..4d7ed964ad 100755 --- a/controller/test-files/area/create/ok.child1_ok.res.payload.golden.json +++ b/controller/test-files/area/create/ok.child1_ok.res.payload.golden.json @@ -3,7 +3,6 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "name": "TestSuccessCreateMultiChildArea-0", - "number": 3, "parent_path": "/00000000-0000-0000-0000-000000000001", "parent_path_resolved": "/area 00000000-0000-0000-0000-000000000002", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/area/create/ok.res.payload.golden.json b/controller/test-files/area/create/ok.res.payload.golden.json index d1de29dfff..758794f9ae 100755 --- a/controller/test-files/area/create/ok.res.payload.golden.json +++ b/controller/test-files/area/create/ok.res.payload.golden.json @@ -3,7 +3,6 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "name": "TestSuccessCreateChildArea", - "number": 2, "parent_path": "/00000000-0000-0000-0000-000000000001", "parent_path_resolved": "/area 00000000-0000-0000-0000-000000000002", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json b/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json index 1a37d5035f..8615eef112 100755 --- a/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json +++ b/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json @@ -3,7 +3,6 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "name": "root", - "number": 1, "parent_path": "/", "parent_path_resolved": "/", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json b/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json index 1a37d5035f..8615eef112 100755 --- a/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json +++ b/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json @@ -3,7 +3,6 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "name": "root", - "number": 1, "parent_path": "/", "parent_path_resolved": "/", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/area/show/ok-root-area.res.payload.golden.json b/controller/test-files/area/show/ok-root-area.res.payload.golden.json index 1a37d5035f..8615eef112 100755 --- a/controller/test-files/area/show/ok-root-area.res.payload.golden.json +++ b/controller/test-files/area/show/ok-root-area.res.payload.golden.json @@ -3,7 +3,6 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "name": "root", - "number": 1, "parent_path": "/", "parent_path_resolved": "/", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/area/showChildren/ok.res.payload.golden.json b/controller/test-files/area/showChildren/ok.res.payload.golden.json index 7477ae9651..9a5e8b3bb7 100755 --- a/controller/test-files/area/showChildren/ok.res.payload.golden.json +++ b/controller/test-files/area/showChildren/ok.res.payload.golden.json @@ -4,7 +4,6 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "name": "TestShowChildrenArea", - "number": 2, "parent_path": "/00000000-0000-0000-0000-000000000001", "parent_path_resolved": "/area 00000000-0000-0000-0000-000000000002", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/iteration/create/ok_create_child.res.iteration.golden.json b/controller/test-files/iteration/create/ok_create_child.res.iteration.golden.json index c8d0d5b42f..09bba8f745 100755 --- a/controller/test-files/iteration/create/ok_create_child.res.iteration.golden.json +++ b/controller/test-files/iteration/create/ok_create_child.res.iteration.golden.json @@ -6,7 +6,6 @@ "description": "Some description", "endAt": "0001-01-01T00:00:00Z", "name": "Sprint #21", - "number": 3, "parent_path": "/00000000-0000-0000-0000-000000000001/00000000-0000-0000-0000-000000000002", "resolved_parent_path": "/root/child", "startAt": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/iteration/create/ok_create_child_with_ID_paylod.res.iteration.golden.json b/controller/test-files/iteration/create/ok_create_child_with_ID_paylod.res.iteration.golden.json index c8d0d5b42f..09bba8f745 100755 --- a/controller/test-files/iteration/create/ok_create_child_with_ID_paylod.res.iteration.golden.json +++ b/controller/test-files/iteration/create/ok_create_child_with_ID_paylod.res.iteration.golden.json @@ -6,7 +6,6 @@ "description": "Some description", "endAt": "0001-01-01T00:00:00Z", "name": "Sprint #21", - "number": 3, "parent_path": "/00000000-0000-0000-0000-000000000001/00000000-0000-0000-0000-000000000002", "resolved_parent_path": "/root/child", "startAt": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json b/controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json index 04411aaa88..1124bfd88f 100755 --- a/controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json +++ b/controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json @@ -6,7 +6,6 @@ "description": "some description (see function github.com/fabric8-services/fabric8-wit/controller_test.(*TestIterationREST).TestShow in controller/iteration_blackbox_test.go)", "endAt": "0001-01-01T00:00:00Z", "name": "child", - "number": 2, "parent_path": "/00000000-0000-0000-0000-000000000001", "resolved_parent_path": "/root", "startAt": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json b/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json index 97e6e184dc..4c1238a3f4 100755 --- a/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json +++ b/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json @@ -5,7 +5,6 @@ "created-at": "0001-01-01T00:00:00Z", "description": "some description (see function github.com/fabric8-services/fabric8-wit/controller_test.(*TestIterationREST).TestShow in controller/iteration_blackbox_test.go)", "name": "root", - "number": 1, "parent_path": "/", "resolved_parent_path": "/", "state": "new", diff --git a/controller/test-files/iteration/update/ok_change_parent.res.iteration.golden.json b/controller/test-files/iteration/update/ok_change_parent.res.iteration.golden.json index e7982334b2..9d160637d0 100755 --- a/controller/test-files/iteration/update/ok_change_parent.res.iteration.golden.json +++ b/controller/test-files/iteration/update/ok_change_parent.res.iteration.golden.json @@ -5,7 +5,6 @@ "created-at": "0001-01-01T00:00:00Z", "description": "some description (see function github.com/fabric8-services/fabric8-wit/controller_test.(*TestIterationREST).TestUpdateIteration.func1 in controller/iteration_blackbox_test.go)", "name": "iteration 3", - "number": 4, "parent_path": "/00000000-0000-0000-0000-000000000001/00000000-0000-0000-0000-000000000002", "resolved_parent_path": "/root/iteration 1", "state": "new", diff --git a/controller/test-files/space_iterations/create/ok.payload.res.golden.json b/controller/test-files/space_iterations/create/ok.payload.res.golden.json index b60cda339b..5756e75b12 100755 --- a/controller/test-files/space_iterations/create/ok.payload.res.golden.json +++ b/controller/test-files/space_iterations/create/ok.payload.res.golden.json @@ -5,7 +5,6 @@ "created-at": "0001-01-01T00:00:00Z", "endAt": "0001-01-01T00:00:00Z", "name": "Sprint #42", - "number": 2, "parent_path": "/00000000-0000-0000-0000-000000000001", "resolved_parent_path": "/space 00000000-0000-0000-0000-000000000002", "startAt": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/space_iterations/create/ok_with_force_active.payload.res.golden.json b/controller/test-files/space_iterations/create/ok_with_force_active.payload.res.golden.json index e4dc9ee514..2809891067 100755 --- a/controller/test-files/space_iterations/create/ok_with_force_active.payload.res.golden.json +++ b/controller/test-files/space_iterations/create/ok_with_force_active.payload.res.golden.json @@ -5,7 +5,6 @@ "created-at": "0001-01-01T00:00:00Z", "endAt": "0001-01-01T00:00:00Z", "name": "Sprint #43", - "number": 2, "parent_path": "/00000000-0000-0000-0000-000000000001", "resolved_parent_path": "/space 00000000-0000-0000-0000-000000000002", "startAt": "0001-01-01T00:00:00Z", diff --git a/design/areas.go b/design/areas.go index 71a26ef35c..5fe9bf69fa 100644 --- a/design/areas.go +++ b/design/areas.go @@ -37,7 +37,6 @@ var areaAttributes = a.Type("AreaAttributes", func() { a.Attribute("parent_path_resolved", d.String, "Path to the topmost area specified by area names", func() { a.Example("/devtools/planner/planner-ui") }) - a.Attribute("number", d.Integer, "Human-friendly number of the area that is unique inside the area's space") }) var areaRelationships = a.Type("AreaRelations", func() { diff --git a/design/iterations.go b/design/iterations.go index d045650f17..250f249960 100644 --- a/design/iterations.go +++ b/design/iterations.go @@ -40,15 +40,16 @@ var iterationAttributes = a.Type("IterationAttributes", func() { a.Attribute("state", d.String, "State of an iteration", func() { a.Enum("new", "start", "close") }) - a.Attribute("user_active", d.Boolean, "Active flag set by user") - a.Attribute("active_status", d.Boolean, "Active status of iteration calculated using user_active, startAt and endAt") + a.Attribute("user_active", d.Boolean, "Active flag set by user", func() { + }) + a.Attribute("active_status", d.Boolean, "Active status of iteration calculated using user_active, startAt and endAt", func() { + }) a.Attribute("parent_path", d.String, "Path string separataed by / having UUIDs of all parent iterations", func() { a.Example("/8ab013be-6477-41e2-b206-53593dac6543/300d9835-fcf7-4d2f-a629-1919de091663/42f0dabd-16bf-40a6-a521-888ec2ad7461") }) a.Attribute("resolved_parent_path", d.String, "Path string separataed by / having names of all parent iterations", func() { a.Example("/beta/Web-App/Sprint 9/Sprint 9.1") }) - a.Attribute("number", d.Integer, "Human-friendly number of the iteration that is unique inside the iteration's space") }) var iterationRelationships = a.Type("IterationRelations", func() { diff --git a/iteration/iteration.go b/iteration/iteration.go index 6f438f471c..d1e8aeced3 100644 --- a/iteration/iteration.go +++ b/iteration/iteration.go @@ -10,7 +10,6 @@ import ( "github.com/fabric8-services/fabric8-wit/errors" "github.com/fabric8-services/fabric8-wit/gormsupport" "github.com/fabric8-services/fabric8-wit/log" - "github.com/fabric8-services/fabric8-wit/numbersequence" "github.com/fabric8-services/fabric8-wit/path" "github.com/goadesign/goa" "github.com/jinzhu/gorm" @@ -30,7 +29,6 @@ const ( // Iteration describes a single iteration type Iteration struct { gormsupport.Lifecycle - numbersequence.HumanFriendlyNumber 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 @@ -139,7 +137,6 @@ func (m *GormIterationRepository) LoadMultiple(ctx context.Context, ids []uuid.U func (m *GormIterationRepository) Create(ctx context.Context, u *Iteration) error { defer goa.MeasureSince([]string{"goa", "db", "iteration", "create"}, time.Now()) - u.HumanFriendlyNumber = numbersequence.NewHumanFriendlyNumber(u.SpaceID, u.TableName()) if u.ID == uuid.Nil { u.ID = uuid.NewV4() u.Path = path.Path{u.ID} diff --git a/iteration/iteration_test.go b/iteration/iteration_test.go index 9bbda6586b..05e79f2578 100644 --- a/iteration/iteration_test.go +++ b/iteration/iteration_test.go @@ -25,7 +25,7 @@ func TestRunIterationRepository(t *testing.T) { suite.Run(t, &TestIterationRepository{DBTestSuite: gormtestsupport.NewDBTestSuite()}) } -func (s *TestIterationRepository) TestCreate() { +func (s *TestIterationRepository) TestCreateIteration() { t := s.T() resource.Require(t, resource.Database) repo := iteration.NewIterationRepository(s.DB) @@ -35,7 +35,7 @@ func (s *TestIterationRepository) TestCreate() { end := start.Add(time.Hour * (24 * 8 * 3)) name := "Sprint #24" // given - fxt := tf.NewTestFixture(t, s.DB, tf.Spaces(2)) + fxt := tf.NewTestFixture(t, s.DB, tf.Spaces(1)) i := iteration.Iteration{ Name: name, @@ -47,24 +47,15 @@ func (s *TestIterationRepository) TestCreate() { err := repo.Create(context.Background(), &i) // then require.NoError(t, err) - require.NotEqual(t, uuid.Nil, i.ID, "iteration not created, ID is nil") - require.False(t, i.CreatedAt.After(time.Now()), "iteration was not created, CreatedAt after Now()?") + if i.ID == uuid.Nil { + t.Errorf("Iteration was not created, ID nil") + } + if i.CreatedAt.After(time.Now()) { + t.Errorf("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) - assert.Equal(t, 1, i.Number) - t.Run("second iteration in space gets sequential number", func(t *testing.T) { - i := iteration.Iteration{Name: "second iteration", SpaceID: fxt.Spaces[0].ID} - err := repo.Create(context.Background(), &i) - require.NoError(t, err) - assert.Equal(t, 2, i.Number) - }) - t.Run("first iteration in another space starts numbering at 1", func(t *testing.T) { - i := iteration.Iteration{Name: "first iteration", SpaceID: fxt.Spaces[1].ID} - err := repo.Create(context.Background(), &i) - require.NoError(t, err) - assert.Equal(t, 1, i.Number) - }) }) t.Run("success - create child", func(t *testing.T) { @@ -82,7 +73,8 @@ func (s *TestIterationRepository) TestCreate() { EndAt: &end, } // when - repo.Create(context.Background(), &i) + err := repo.Create(context.Background(), &i) + require.NoError(t, err) parentPath := append(i.Path, i.ID) require.NotNil(t, parentPath) i2 := iteration.Iteration{ @@ -92,8 +84,9 @@ func (s *TestIterationRepository) TestCreate() { EndAt: &end, Path: parentPath, } - repo.Create(context.Background(), &i2) + err = repo.Create(context.Background(), &i2) // then + require.NoError(t, err) i2L, err := repo.Load(context.Background(), i2.ID) require.NoError(t, err) assert.NotEmpty(t, i2.Path) @@ -211,8 +204,8 @@ func (s *TestIterationRepository) TestLoad() { EndAt: &end, } // when - repo.Create(context.Background(), &i) - + err := repo.Create(context.Background(), &i) + require.NoError(t, err) i2 := iteration.Iteration{ Name: name2, SpaceID: fxt.Spaces[0].ID, @@ -220,8 +213,9 @@ func (s *TestIterationRepository) TestLoad() { EndAt: &end, } i2.MakeChildOf(i) - repo.Create(context.Background(), &i2) + err = repo.Create(context.Background(), &i2) // then + require.NoError(t, err) res, err := repo.Root(context.Background(), fxt.Spaces[0].ID) require.NoError(t, err) assert.Equal(t, i.Name, res.Name) diff --git a/migration/migration.go b/migration/migration.go index 36d34d5fb0..500f593658 100644 --- a/migration/migration.go +++ b/migration/migration.go @@ -454,10 +454,7 @@ func GetMigrations() Migrations { m = append(m, steps{ExecuteSQLFile("105-update-root-area-and-iteration-path-field.sql")}) // Version 106 - m = append(m, steps{ExecuteSQLFile("106-number-sequences.sql")}) - - // Version 107 - m = append(m, steps{ExecuteSQLFile("107-remove-link-category-concept.sql")}) + m = append(m, steps{ExecuteSQLFile("106-remove-link-category-concept.sql")}) // Version N // diff --git a/migration/migration_blackbox_test.go b/migration/migration_blackbox_test.go index 3513e0723e..3d30ff6a7b 100644 --- a/migration/migration_blackbox_test.go +++ b/migration/migration_blackbox_test.go @@ -155,8 +155,7 @@ func TestMigrations(t *testing.T) { t.Run("TestMigration103", testMigration103NotNullNotEmptyonEmail) t.Run("TestMirgraion104", testMigration104IndexOnWIRevisionTable) t.Run("TestMirgraion105", testMigration105UpdateRootIterationAreaPathField) - t.Run("TestMirgraion106", testMigration106NumberSequences) - t.Run("TestMirgraion107", testMigration107RemoveLinkCategoryConcept) + t.Run("TestMirgraion106", testMigration106RemoveLinkCategoryConcept) // Perform the migration err = migration.Migrate(sqlDB, databaseName) @@ -1322,153 +1321,16 @@ func testMigration105UpdateRootIterationAreaPathField(t *testing.T) { }) } -// testMigration106NumberSequences tests that we can migrate existing work -// items, iterations and areas to use the new number_sequences table -func testMigration106NumberSequences(t *testing.T) { +func testMigration106RemoveLinkCategoryConcept(t *testing.T) { t.Run("migrate to previous version", func(t *testing.T) { migrateToVersion(t, sqlDB, migrations[:106], 106) }) - spaceTemplateID := uuid.NewV4() - space1ID := uuid.NewV4() - space2ID := uuid.NewV4() - witID := uuid.NewV4() - // two work items in space 1 - wi1ID := uuid.NewV4() - wi2ID := uuid.NewV4() - // two work items in space 2 - wi3ID := uuid.NewV4() - wi4ID := uuid.NewV4() - // two iterations in space 1 - iter1ID := uuid.NewV4() - iter2ID := uuid.NewV4() - // two iterations in space 2 - iter3ID := uuid.NewV4() - iter4ID := uuid.NewV4() - // two areas in space 1 - area1ID := uuid.NewV4() - area2ID := uuid.NewV4() - // two areas in space 2 - area3ID := uuid.NewV4() - area4ID := uuid.NewV4() - - t.Run("setup test data to migrate", func(t *testing.T) { - require.Nil(t, runSQLscript(sqlDB, "106-number-sequences.sql", - spaceTemplateID.String(), - space1ID.String(), - space2ID.String(), - witID.String(), - wi1ID.String(), - wi2ID.String(), - wi3ID.String(), - wi4ID.String(), - iter1ID.String(), - iter2ID.String(), - iter3ID.String(), - iter4ID.String(), - area1ID.String(), - area2ID.String(), - area3ID.String(), - area4ID.String(), - )) - }) - - // Helper functions - - getNumberOf := func(t *testing.T, table string, id uuid.UUID) int { - q := fmt.Sprintf("SELECT number FROM %s WHERE id = $1", table) - row := sqlDB.QueryRow(q, id) - require.NotNil(t, row) - var p int32 - err := row.Scan(&p) - require.NoError(t, err, "%+v", err) - return int(p) - } - getNumberOfWorkItem := func(t *testing.T, wiID uuid.UUID) int { return getNumberOf(t, "work_items", wiID) } - getNumberOfIteration := func(t *testing.T, iterID uuid.UUID) int { return getNumberOf(t, "iterations", iterID) } - getNumberOfArea := func(t *testing.T, areaID uuid.UUID) int { return getNumberOf(t, "areas", areaID) } - - t.Run("checks before migration", func(t *testing.T) { - require.Equal(t, 1, getNumberOfWorkItem(t, wi1ID)) - require.Equal(t, 2, getNumberOfWorkItem(t, wi2ID)) - require.Equal(t, 1, getNumberOfWorkItem(t, wi3ID)) - require.Equal(t, 2, getNumberOfWorkItem(t, wi4ID)) - }) - - t.Run("migrate to current version", func(t *testing.T) { - migrateToVersion(t, sqlDB, migrations[:107], 107) - }) - - t.Run("checks after migration", func(t *testing.T) { - oldTable := "work_item_number_sequences" - require.False(t, dialect.HasTable(oldTable), "the old number sequences table %q should be deleted", oldTable) - - // work item numbering must not be touched - assert.Equal(t, 1, getNumberOfWorkItem(t, wi1ID)) - assert.Equal(t, 2, getNumberOfWorkItem(t, wi2ID)) - assert.Equal(t, 1, getNumberOfWorkItem(t, wi3ID)) - assert.Equal(t, 2, getNumberOfWorkItem(t, wi4ID)) - - // check for (newly added) "number" columns - assert.True(t, dialect.HasColumn("work_items", "number"), "work_items table should still have the number column") - assert.True(t, dialect.HasColumn("iterations", "number")) - assert.True(t, dialect.HasColumn("areas", "number")) - - // check that the newest iteration/area has the smaller number and that - // the numbering is partitioned by space_id - assert.Equal(t, 2, getNumberOfIteration(t, iter1ID)) - assert.Equal(t, 1, getNumberOfIteration(t, iter2ID)) - assert.Equal(t, 2, getNumberOfIteration(t, iter3ID)) - assert.Equal(t, 1, getNumberOfIteration(t, iter4ID)) - - assert.Equal(t, 2, getNumberOfArea(t, area1ID)) - assert.Equal(t, 1, getNumberOfArea(t, area2ID)) - assert.Equal(t, 2, getNumberOfArea(t, area3ID)) - assert.Equal(t, 1, getNumberOfArea(t, area4ID)) - - // check that the new sequences table exists and has the expected values - newTable := "number_sequences" - require.True(t, dialect.HasTable(newTable), "the new number sequences table %q has to exist", newTable) - - type numberSequence struct { - spaceID uuid.UUID - tableName string - currentVal int - } - rows, err := sqlDB.Query("SELECT space_id, table_name, current_val FROM number_sequences WHERE space_id IN ($1, $2)", space1ID, space2ID) - require.NoError(t, err) - defer rows.Close() - toBeFound := map[numberSequence]struct{}{ - {space1ID, "work_items", 2}: {}, - {space2ID, "work_items", 2}: {}, - {space1ID, "iterations", 2}: {}, - {space2ID, "iterations", 2}: {}, - {space1ID, "areas", 2}: {}, - {space2ID, "areas", 2}: {}, - } - for rows.Next() { - seq := numberSequence{} - err := rows.Scan(&seq.spaceID, &seq.tableName, &seq.currentVal) - require.NoError(t, err) - 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")) - }) -} - -func testMigration107RemoveLinkCategoryConcept(t *testing.T) { - t.Run("migrate to previous version", func(t *testing.T) { - migrateToVersion(t, sqlDB, migrations[:107], 107) - }) - require.True(t, dialect.HasTable("work_item_link_categories")) require.True(t, dialect.HasForeignKey("work_item_link_types", "work_item_link_types_link_category_id_fkey")) t.Run("migrate to current version", func(t *testing.T) { - migrateToVersion(t, sqlDB, migrations[:108], 108) + migrateToVersion(t, sqlDB, migrations[:107], 107) }) require.True(t, dialect.HasTable("work_item_link_categories")) @@ -1479,7 +1341,7 @@ func testMigration107RemoveLinkCategoryConcept(t *testing.T) { linkCategoryID := uuid.NewV4() linkType1ID := uuid.NewV4() linkType2ID := uuid.NewV4() - require.Nil(t, runSQLscript(sqlDB, "107-remove-link-category-concept.sql", + require.Nil(t, runSQLscript(sqlDB, "106-remove-link-category-concept.sql", spaceTemplateID.String(), linkCategoryID.String(), linkType1ID.String(), @@ -1501,7 +1363,6 @@ func testMigration107RemoveLinkCategoryConcept(t *testing.T) { require.True(t, exists(t, "work_item_link_types", linkType1ID)) require.True(t, exists(t, "work_item_link_types", linkType2ID)) }) - } // runSQLscript loads the given filename from the packaged SQL test files and diff --git a/migration/sql-files/106-number-sequences.sql b/migration/sql-files/106-number-sequences.sql deleted file mode 100644 index 163a1d4207..0000000000 --- a/migration/sql-files/106-number-sequences.sql +++ /dev/null @@ -1,51 +0,0 @@ -LOCK spaces, iterations, areas, work_item_number_sequences IN EXCLUSIVE MODE; - --- Create the number sequences table -CREATE TABLE number_sequences ( - space_id uuid REFERENCES spaces(id) ON DELETE CASCADE, - table_name text CHECK (trim(table_name::text) <> ''), - current_val INTEGER NOT NULL, - PRIMARY KEY (space_id, table_name) -); - --- Update existing iterations and areas with new "number" column and fill in the --- numbers in the order iterations and areas have been created. -ALTER TABLE iterations ADD COLUMN number INTEGER; -ALTER TABLE areas ADD COLUMN number INTEGER; - -UPDATE iterations 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 iterations) AS seq -WHERE iterations.id = seq.id; - -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 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; - -ALTER TABLE iterations ADD CONSTRAINT iterations_space_id_number_idx UNIQUE (space_id, number); -ALTER TABLE areas ADD CONSTRAINT areas_space_id_number_idx UNIQUE (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) - SELECT space_id, 'work_items' "table_name", current_val - FROM work_item_number_sequences - GROUP BY 1,2; - --- Delete old number table -DROP TABLE work_item_number_sequences; - -INSERT INTO number_sequences (space_id, table_name, current_val) - SELECT space_id, 'iterations' "table_name", MAX(number) - FROM iterations - WHERE number IS NOT NULL - GROUP BY 1,2; - -INSERT INTO number_sequences (space_id, table_name, current_val) - SELECT space_id, 'areas' "table_name", MAX(number) - FROM areas - WHERE number IS NOT NULL - GROUP BY 1,2; \ No newline at end of file diff --git a/migration/sql-files/107-remove-link-category-concept.sql b/migration/sql-files/106-remove-link-category-concept.sql similarity index 100% rename from migration/sql-files/107-remove-link-category-concept.sql rename to migration/sql-files/106-remove-link-category-concept.sql diff --git a/migration/sql-test-files/106-number-sequences.sql b/migration/sql-test-files/106-number-sequences.sql deleted file mode 100644 index 49b52c9e32..0000000000 --- a/migration/sql-test-files/106-number-sequences.sql +++ /dev/null @@ -1,59 +0,0 @@ -SET sp_template.id = '{{index . 0}}'; -SET sp1.id = '{{index . 1}}'; -SET sp2.id = '{{index . 2}}'; - -SET wit.id = '{{index . 3}}'; - -SET wi1.id = '{{index . 4}}'; -SET wi2.id = '{{index . 5}}'; -SET wi3.id = '{{index . 6}}'; -SET wi4.id = '{{index . 7}}'; - -SET iter1.id = '{{index . 8}}'; -SET iter2.id = '{{index . 9}}'; -SET iter3.id = '{{index . 10}}'; -SET iter4.id = '{{index . 11}}'; - -SET area1.id = '{{index . 12}}'; -SET area2.id = '{{index . 13}}'; -SET area3.id = '{{index . 14}}'; -SET area4.id = '{{index . 15}}'; - --- create space template -INSERT INTO space_templates (id,name,description) - VALUES(current_setting('sp_template.id')::uuid, current_setting('sp_template.id'), 'test template'); - --- create two spaces -INSERT INTO spaces (id,name,space_template_id) - VALUES - (current_setting('sp1.id')::uuid, current_setting('sp1.id'), current_setting('sp_template.id')::uuid), - (current_setting('sp2.id')::uuid, current_setting('sp2.id'), current_setting('sp_template.id')::uuid); - -INSERT INTO work_item_types (id,name,can_construct,space_template_id,fields,description,icon) - VALUES (current_setting('wit.id')::uuid, 'Custom WIT', 'true', current_setting('sp_template.id')::uuid, '{"system.title": {"type": {"kind": "string"}, "label": "Title", "required": true, "read_only": false, "description": "The title text of the work item"}}', 'Description for Impediment', 'fa fa-bookmark'); - -INSERT INTO work_items (id, type, space_id, fields, number, created_at) -VALUES - (current_setting('wi1.id')::uuid, current_setting('wit.id')::uuid, current_setting('sp1.id')::uuid, '{"system.title":"WI1"}'::json, 1, '2018-09-17 16:01'), - (current_setting('wi2.id')::uuid, current_setting('wit.id')::uuid, current_setting('sp1.id')::uuid, '{"system.title":"WI2"}'::json, 2, '2018-09-17 18:01'), - (current_setting('wi3.id')::uuid, current_setting('wit.id')::uuid, current_setting('sp2.id')::uuid, '{"system.title":"WI3"}'::json, 1, '2018-09-17 12:01'), - (current_setting('wi4.id')::uuid, current_setting('wit.id')::uuid, current_setting('sp2.id')::uuid, '{"system.title":"WI4"}'::json, 2, '2018-09-17 17:01'); - -INSERT INTO work_item_number_sequences (space_id, current_val) -VALUES - (current_setting('sp1.id')::uuid, 2), - (current_setting('sp2.id')::uuid, 2); - -INSERT INTO iterations (id, name, path, space_id, created_at) -VALUES - (current_setting('iter1.id')::uuid, 'iteration 1', replace(current_setting('iter1.id'), '-', '_')::ltree, current_setting('sp1.id')::uuid, '2018-09-17 16:01'), - (current_setting('iter2.id')::uuid, 'iteration 2', replace(current_setting('iter2.id'), '-', '_')::ltree, current_setting('sp1.id')::uuid, '2018-09-17 15:01'), - (current_setting('iter3.id')::uuid, 'iteration 3', replace(current_setting('iter3.id'), '-', '_')::ltree, current_setting('sp2.id')::uuid, '2018-09-17 16:01'), - (current_setting('iter4.id')::uuid, 'iteration 4', replace(current_setting('iter4.id'), '-', '_')::ltree, current_setting('sp2.id')::uuid, '2018-09-17 15:01'); - -INSERT INTO areas (id, name, path, space_id, created_at) -VALUES - (current_setting('area1.id')::uuid, 'area 1', replace(current_setting('area1.id'), '-', '_')::ltree, current_setting('sp1.id')::uuid, '2018-09-17 13:01'), - (current_setting('area2.id')::uuid, 'area 2', replace(current_setting('area2.id'), '-', '_')::ltree, current_setting('sp1.id')::uuid, '2018-09-17 12:01'), - (current_setting('area3.id')::uuid, 'area 3', replace(current_setting('area3.id'), '-', '_')::ltree, current_setting('sp2.id')::uuid, '2018-09-17 13:01'), - (current_setting('area4.id')::uuid, 'area 4', replace(current_setting('area4.id'), '-', '_')::ltree, current_setting('sp2.id')::uuid, '2018-09-17 12:01'); diff --git a/migration/sql-test-files/107-remove-link-category-concept.sql b/migration/sql-test-files/106-remove-link-category-concept.sql similarity index 100% rename from migration/sql-test-files/107-remove-link-category-concept.sql rename to migration/sql-test-files/106-remove-link-category-concept.sql diff --git a/numbersequence/human_friendly_number.go b/numbersequence/human_friendly_number.go deleted file mode 100644 index d78c114007..0000000000 --- a/numbersequence/human_friendly_number.go +++ /dev/null @@ -1,107 +0,0 @@ -package numbersequence - -import ( - "fmt" - - "github.com/fabric8-services/fabric8-wit/convert" - "github.com/fabric8-services/fabric8-wit/log" - "github.com/jinzhu/gorm" - errs "github.com/pkg/errors" - uuid "github.com/satori/go.uuid" -) - -// The HumanFriendlyNumber struct can be embedded in all model structs that want -// 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"). 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:"-"` - tableName string `gorm:"-"` -} - -// NewHumanFriendlyNumber TODO(kwk): document me -func NewHumanFriendlyNumber(spaceID uuid.UUID, tableName string, number ...int) HumanFriendlyNumber { - n := 0 - if len(number) > 0 { - n = number[0] - } - return HumanFriendlyNumber{ - Number: n, - spaceID: spaceID, - tableName: tableName, - } -} - -// Ensure Equaler implements the Equaler interface -var _ convert.Equaler = HumanFriendlyNumber{} -var _ convert.Equaler = (*HumanFriendlyNumber)(nil) - -// Equal implements convert.Equaler -func (n HumanFriendlyNumber) Equal(u convert.Equaler) bool { - other, ok := u.(HumanFriendlyNumber) - if !ok { - return false - } - if n.Number != other.Number { - return false - } - if n.spaceID != other.spaceID { - return false - } - if n.tableName != other.tableName { - return false - } - return true -} - -// EqualValue implements convert.Equaler -func (n HumanFriendlyNumber) EqualValue(u convert.Equaler) bool { - return n.Equal(u) -} - -// BeforeCreate is a GORM callback (see http://doc.gorm.io/callbacks.html) that -// will be called before creating the model. We use it to determine the next -// human readable number for the model and set it automatically in the CREATE -// query. -func (n *HumanFriendlyNumber) BeforeCreate(scope *gorm.Scope) error { - upsertStmt := ` - INSERT INTO number_sequences (space_id, table_name, current_val) - VALUES ($1, $2, 1) - ON CONFLICT (space_id, table_name) - DO UPDATE SET current_val = number_sequences.current_val + EXCLUDED.current_val - RETURNING current_val - ` - var currentVal int - err := scope.NewDB().Debug().CommonDB().QueryRow(upsertStmt, n.spaceID, n.tableName).Scan(¤tVal) - if err != nil { - return errs.Wrapf(err, "failed to obtain next val for space %q and table %q", n.spaceID, n.tableName) - } - log.Debug(nil, map[string]interface{}{ - "space_id": n.spaceID, - "table_name": n.tableName, - "next_val": currentVal, - }, "computed nextVal") - - 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= -// -// 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 -} diff --git a/numbersequence/human_friendly_number_test.go b/numbersequence/human_friendly_number_test.go deleted file mode 100644 index efbdf57f7a..0000000000 --- a/numbersequence/human_friendly_number_test.go +++ /dev/null @@ -1,221 +0,0 @@ -package numbersequence_test - -import ( - "fmt" - "sync" - "testing" - - "github.com/fabric8-services/fabric8-wit/convert" - "github.com/fabric8-services/fabric8-wit/gormtestsupport" - "github.com/fabric8-services/fabric8-wit/numbersequence" - tf "github.com/fabric8-services/fabric8-wit/test/testfixture" - uuid "github.com/satori/go.uuid" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" -) - -func TestHumanFriendlyNumber_Equal(t *testing.T) { - t.Parallel() - spaceID := uuid.NewV4() - a := numbersequence.NewHumanFriendlyNumber(spaceID, "work_items", 1) - t.Run("types", func(t *testing.T) { - t.Parallel() - b := convert.DummyEqualer{} - assert.False(t, a.Equal(b)) - }) - t.Run("equality", func(t *testing.T) { - t.Parallel() - b := a - assert.True(t, a.Equal(b)) - }) - t.Run("number", func(t *testing.T) { - t.Parallel() - b := numbersequence.NewHumanFriendlyNumber(spaceID, "work_items", 567) - assert.False(t, a.Equal(b)) - }) - t.Run("table name", func(t *testing.T) { - t.Parallel() - b := numbersequence.NewHumanFriendlyNumber(spaceID, "iterations", 1) - assert.False(t, a.Equal(b)) - }) - t.Run("space id", func(t *testing.T) { - t.Parallel() - b := numbersequence.NewHumanFriendlyNumber(uuid.NewV4(), "work_items", 1) - assert.False(t, a.Equal(b)) - }) -} - -type humanFriendlyNumberSuite struct { - gormtestsupport.DBTestSuite -} - -func TestHumanFriendlyNumberSuite(t *testing.T) { - suite.Run(t, &humanFriendlyNumberSuite{DBTestSuite: gormtestsupport.NewDBTestSuite()}) -} - -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"` - Message string `json:"message"` -} - -func (s testStruct1) TableName() string { - return tableName1 -} - -type testStruct2 struct { - numbersequence.HumanFriendlyNumber - ID uuid.UUID `json:"id" gorm:"primary_key"` - Message string `json:"message"` -} - -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. -func (s *humanFriendlyNumberSuite) TestBeforeCreate() { - // given - fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(2)) - db := s.DB - - // create models of two types in two spaces and check that the numbers are - // assigned as expected. - data1 := []testStruct1{ - {ID: uuid.NewV4(), Message: "first", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[0].ID, tableName1)}, - {ID: uuid.NewV4(), Message: "second", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[0].ID, tableName1)}, - {ID: uuid.NewV4(), Message: "third", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[1].ID, tableName1)}, - {ID: uuid.NewV4(), Message: "fourth", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[1].ID, tableName1)}, - {ID: uuid.NewV4(), Message: "fifth", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[1].ID, tableName1)}, - } - data2 := []testStruct2{ - {ID: uuid.NewV4(), Message: "first", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[0].ID, tableName2)}, - {ID: uuid.NewV4(), Message: "second", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[0].ID, tableName2)}, - {ID: uuid.NewV4(), Message: "third", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[1].ID, tableName2)}, - {ID: uuid.NewV4(), Message: "fourth", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[1].ID, tableName2)}, - {ID: uuid.NewV4(), Message: "fifth", HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[1].ID, tableName2)}, - } - expectedNumbers := []int{1, 2, 1, 2, 3} - for i := range data1 { - // when - db = db.Create(&data1[i]) - // then - require.NoError(s.T(), db.Error) - require.Equal(s.T(), expectedNumbers[i], data1[i].Number, "data1 item #%d should have number %d but has %d", i, expectedNumbers[i], data1[i].Number) - // when - db = db.Create(&data2[i]) - // then - require.NoError(s.T(), db.Error) - require.Equal(s.T(), expectedNumbers[i], data2[i].Number, "data2 item #%d should have number %d but has %d", i, expectedNumbers[i], data2[i].Number) - } -} - -// 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)) - type Report struct { - id int - total int - failures int - } - routines := 10 - itemsPerRoutine := 50 - reports := make([]Report, routines) - // when running concurrent go routines simultaneously - var wg sync.WaitGroup - for i := 0; i < routines; i++ { - wg.Add(1) - // in each go rountine, run 10 creations - go func(routineID int) { - defer wg.Done() - report := Report{id: routineID} - for j := 0; j < itemsPerRoutine; j++ { - model := testStruct1{ - ID: uuid.NewV4(), - Message: "model " + uuid.NewV4().String(), - HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(fxt.Spaces[0].ID, tableName1), - } - db := s.DB.Create(&model) - if db.Error != nil { - s.T().Logf("creation failed: %+v", db.Error.Error()) - report.failures++ - } - report.total++ - } - reports[routineID] = report - }(i) - } - wg.Wait() - // then - // wait for all items to be created - for _, report := range reports { - fmt.Printf("Routine #%d done: %d creations, including %d failure(s)\n", report.id, report.total, report.failures) - assert.Equal(s.T(), itemsPerRoutine, report.total) - assert.Equal(s.T(), 0, report.failures) - } - - // check that the created models have the correct numbers - toBeFound := map[int]struct{}{} - for i := 0; i < routines*itemsPerRoutine; i++ { - toBeFound[i+1] = struct{}{} - } - models := []testStruct1{} - db := s.DB.Find(&models) - require.NoError(s.T(), db.Error) - - for _, m := range models { - _, ok := toBeFound[m.Number] - assert.True(s.T(), ok, "unexpected number found: %d", m.Number) - delete(toBeFound, m.Number) - } - require.Empty(s.T(), toBeFound, "failed to find these numbers: %+v", toBeFound) -} diff --git a/search/search_repository_blackbox_test.go b/search/search_repository_blackbox_test.go index 0657e8c1c1..421a51133b 100644 --- a/search/search_repository_blackbox_test.go +++ b/search/search_repository_blackbox_test.go @@ -73,20 +73,17 @@ func (s *searchRepositoryBlackboxTest) TestSearchWithJoin() { s.T().Run("join iterations", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, tf.Iterations(2), - tf.Areas(2), tf.WorkItems(10, func(fxt *tf.TestFixture, idx int) error { switch idx { case 0, 1, 2, 3, 4, 5, 6: fxt.WorkItems[idx].Fields[workitem.SystemIteration] = fxt.Iterations[0].ID.String() - fxt.WorkItems[idx].Fields[workitem.SystemArea] = fxt.Areas[0].ID.String() default: fxt.WorkItems[idx].Fields[workitem.SystemIteration] = fxt.Iterations[1].ID.String() - fxt.WorkItems[idx].Fields[workitem.SystemArea] = fxt.Areas[1].ID.String() } return nil }), ) - t.Run("iteration name", func(t *testing.T) { + t.Run("matching name", func(t *testing.T) { // when filter := fmt.Sprintf(`{"iteration.name": "%s"}`, fxt.Iterations[0].Name) res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil) @@ -109,44 +106,6 @@ func (s *searchRepositoryBlackboxTest) TestSearchWithJoin() { } require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound) }) - t.Run("iteration number", func(t *testing.T) { - // when - filter := fmt.Sprintf(`{"iteration.number": "%d"}`, fxt.Iterations[1].Number) - res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil) - // then - require.NoError(t, err) - assert.Equal(t, 3, count) - toBeFound := id.Slice{ - fxt.WorkItems[7].ID, - fxt.WorkItems[8].ID, - fxt.WorkItems[9].ID, - }.ToMap() - for _, wi := range res { - _, ok := toBeFound[wi.ID] - require.True(t, ok, "unknown work item found: %s", wi.ID) - delete(toBeFound, wi.ID) - } - require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound) - }) - t.Run("area number", func(t *testing.T) { - // when - filter := fmt.Sprintf(`{"area.number": "%d"}`, fxt.Areas[1].Number) - res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil) - // then - require.NoError(t, err) - assert.Equal(t, 3, count) - toBeFound := id.Slice{ - fxt.WorkItems[7].ID, - fxt.WorkItems[8].ID, - fxt.WorkItems[9].ID, - }.ToMap() - for _, wi := range res { - _, ok := toBeFound[wi.ID] - require.True(t, ok, "unknown work item found: %s", wi.ID) - delete(toBeFound, wi.ID) - } - require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound) - }) }) s.T().Run("join work item type groups", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, diff --git a/space/space.go b/space/space.go index aa05f4a4c6..e06ccb5ce4 100644 --- a/space/space.go +++ b/space/space.go @@ -13,6 +13,7 @@ import ( "github.com/fabric8-services/fabric8-wit/gormsupport" "github.com/fabric8-services/fabric8-wit/log" "github.com/fabric8-services/fabric8-wit/spacetemplate" + numbersequence "github.com/fabric8-services/fabric8-wit/workitem/number_sequence" "github.com/goadesign/goa" "github.com/jinzhu/gorm" @@ -114,13 +115,15 @@ type Repository interface { // NewRepository creates a new space repo func NewRepository(db *gorm.DB) *GormRepository { return &GormRepository{ - db: db, + db: db, + winr: numbersequence.NewWorkItemNumberSequenceRepository(db), } } // GormRepository implements SpaceRepository using gorm type GormRepository struct { - db *gorm.DB + db *gorm.DB + winr numbersequence.WorkItemNumberSequenceRepository } // Load returns the space for the given id diff --git a/workitem/expression_compiler.go b/workitem/expression_compiler.go index d48f8d3553..d37d118f65 100644 --- a/workitem/expression_compiler.go +++ b/workitem/expression_compiler.go @@ -127,14 +127,14 @@ var DefaultTableJoins = func() TableJoinMap { TableAlias: "iter", On: JoinOnJSONField(SystemIteration, "iter.id") + " AND " + Column("iter", "space_id") + "=" + Column(WorkItemStorage{}.TableName(), "space_id"), PrefixActivators: []string{"iteration."}, - AllowedColumns: []string{"name", "created_at", "number"}, + AllowedColumns: []string{"name", "created_at"}, }, "area": { TableName: "areas", TableAlias: "ar", On: JoinOnJSONField(SystemArea, "ar.id") + " AND " + Column("ar", "space_id") + "=" + Column(WorkItemStorage{}.TableName(), "space_id"), PrefixActivators: []string{"area."}, - AllowedColumns: []string{"name", "number"}, + AllowedColumns: []string{"name"}, }, "codebase": { TableName: "codebases", diff --git a/workitem/number_sequence/work_item_number_sequence_repository_test.go b/workitem/number_sequence/work_item_number_sequence_repository_test.go new file mode 100644 index 0000000000..530175a058 --- /dev/null +++ b/workitem/number_sequence/work_item_number_sequence_repository_test.go @@ -0,0 +1,74 @@ +package numbersequence_test + +import ( + "context" + "fmt" + "sync" + "testing" + + "github.com/fabric8-services/fabric8-wit/application" + "github.com/fabric8-services/fabric8-wit/gormtestsupport" + "github.com/fabric8-services/fabric8-wit/resource" + tf "github.com/fabric8-services/fabric8-wit/test/testfixture" + . "github.com/fabric8-services/fabric8-wit/workitem/number_sequence" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" +) + +type workItemNumberSequenceTest struct { + gormtestsupport.DBTestSuite + repo WorkItemNumberSequenceRepository +} + +func TestWorkItemNumberSequenceTest(t *testing.T) { + resource.Require(t, resource.Database) + suite.Run(t, &workItemNumberSequenceTest{DBTestSuite: gormtestsupport.NewDBTestSuite()}) +} + +func (s *workItemNumberSequenceTest) SetupTest() { + s.DBTestSuite.SetupTest() + s.repo = NewWorkItemNumberSequenceRepository(s.DB) +} + +func (s *workItemNumberSequenceTest) TestConcurrentNextVal() { + // given + fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(1)) + type Report struct { + id int + total int + failures int + } + routines := 10 + itemsPerRoutine := 50 + reports := make([]Report, routines) + // when running concurrent go routines simultaneously + var wg sync.WaitGroup + for i := 0; i < routines; i++ { + wg.Add(1) + // in each go rountine, run 10 creations + go func(routineID int) { + defer wg.Done() + report := Report{id: routineID} + for j := 0; j < itemsPerRoutine; j++ { + if err := application.Transactional(s.GormDB, func(app application.Application) error { + _, err := s.repo.NextVal(context.Background(), fxt.Spaces[0].ID) + return err + }); err != nil { + s.T().Logf("Creation failed: %s", err.Error()) + report.failures++ + } + report.total++ + } + reports[routineID] = report + }(i) + } + wg.Wait() + // then + // wait for all items to be created + for _, report := range reports { + fmt.Printf("Routine #%d done: %d creations, including %d failure(s)\n", report.id, report.total, report.failures) + assert.Equal(s.T(), itemsPerRoutine, report.total) + assert.Equal(s.T(), 0, report.failures) + } + +} diff --git a/workitem/number_sequence/workitem_number_sequence.go b/workitem/number_sequence/workitem_number_sequence.go new file mode 100644 index 0000000000..d8aca1b536 --- /dev/null +++ b/workitem/number_sequence/workitem_number_sequence.go @@ -0,0 +1,26 @@ +package numbersequence + +import ( + "fmt" + + uuid "github.com/satori/go.uuid" +) + +// WorkItemNumberSequence the sequence for work item numbers in a space +type WorkItemNumberSequence struct { + SpaceID uuid.UUID `sql:"type:uuid" gorm:"primary_key"` + CurrentVal int +} + +const ( + workitemNumberTableName = "work_item_number_sequences" +) + +// TableName implements gorm.tabler +func (w WorkItemNumberSequence) TableName() string { + return workitemNumberTableName +} + +func (w *WorkItemNumberSequence) String() string { + return fmt.Sprintf("SpaceID=%s Number=%d", w.SpaceID.String(), w.CurrentVal) +} diff --git a/workitem/number_sequence/workitem_number_sequence_repository.go b/workitem/number_sequence/workitem_number_sequence_repository.go new file mode 100644 index 0000000000..c54ce4d3f3 --- /dev/null +++ b/workitem/number_sequence/workitem_number_sequence_repository.go @@ -0,0 +1,42 @@ +package numbersequence + +import ( + "context" + "fmt" + + "github.com/fabric8-services/fabric8-wit/log" + "github.com/jinzhu/gorm" + errs "github.com/pkg/errors" + uuid "github.com/satori/go.uuid" +) + +// WorkItemNumberSequenceRepository the interface for the work item number sequence repository +type WorkItemNumberSequenceRepository interface { + NextVal(ctx context.Context, spaceID uuid.UUID) (*int, error) +} + +// NewWorkItemNumberSequenceRepository creates a GormWorkItemNumberSequenceRepository +func NewWorkItemNumberSequenceRepository(db *gorm.DB) *GormWorkItemNumberSequenceRepository { + repository := &GormWorkItemNumberSequenceRepository{db} + return repository +} + +// GormWorkItemNumberSequenceRepository implements WorkItemNumberSequenceRepository using gorm +type GormWorkItemNumberSequenceRepository struct { + db *gorm.DB +} + +// NextVal returns the next work item sequence number for the given space ID. Creates an entry in the DB if none was found before +func (r *GormWorkItemNumberSequenceRepository) NextVal(ctx context.Context, spaceID uuid.UUID) (*int, error) { + // upsert the next val, retrieves full row + upsertStmt := fmt.Sprintf(`INSERT INTO %[1]s (space_id, current_val) VALUES ($1,1) + ON CONFLICT (space_id) DO UPDATE SET current_val = %[1]s.current_val + EXCLUDED.current_val + RETURNING current_val`, WorkItemNumberSequence{}.TableName()) + var currentVal int + err := r.db.CommonDB().QueryRow(upsertStmt, spaceID).Scan(¤tVal) + if err != nil { + return nil, errs.Wrapf(err, "failed to obtain next val for space with ID=`%s`", spaceID.String()) + } + log.Debug(nil, map[string]interface{}{"space_id": spaceID, "next_val": currentVal}, "computed nextVal") + return ¤tVal, nil +} diff --git a/workitem/workitem_blackbox_test.go b/workitem/workitem_blackbox_test.go index a24a094398..30de1d8681 100644 --- a/workitem/workitem_blackbox_test.go +++ b/workitem/workitem_blackbox_test.go @@ -6,10 +6,10 @@ import ( "github.com/fabric8-services/fabric8-wit/convert" "github.com/fabric8-services/fabric8-wit/gormsupport" - "github.com/fabric8-services/fabric8-wit/numbersequence" "github.com/fabric8-services/fabric8-wit/ptr" "github.com/fabric8-services/fabric8-wit/resource" "github.com/fabric8-services/fabric8-wit/workitem" + uuid "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" ) @@ -21,10 +21,10 @@ func TestWorkItem_EqualAndEqualValue(t *testing.T) { now := time.Now() spaceID := uuid.NewV4() a := workitem.WorkItemStorage{ - HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(spaceID, workitem.WorkItemStorage{}.TableName(), 1), - ID: uuid.NewV4(), - Type: uuid.NewV4(), - Version: 0, + ID: uuid.NewV4(), + Number: 1, + Type: uuid.NewV4(), + Version: 0, Fields: workitem.Fields{ "foo": "bar", }, @@ -115,11 +115,11 @@ func TestWorkItem_EqualAndEqualValue(t *testing.T) { t.Run("equality", func(t *testing.T) { t.Parallel() b := workitem.WorkItemStorage{ - ID: a.ID, - Type: a.Type, - HumanFriendlyNumber: numbersequence.NewHumanFriendlyNumber(spaceID, workitem.WorkItemStorage{}.TableName(), 1), - ExecutionOrder: 111, - Version: 0, + ID: a.ID, + Type: a.Type, + Number: 1, + ExecutionOrder: 111, + Version: 0, Fields: workitem.Fields{ "foo": "bar", }, diff --git a/workitem/workitem_repository.go b/workitem/workitem_repository.go index f154ed2282..6acf658f9b 100644 --- a/workitem/workitem_repository.go +++ b/workitem/workitem_repository.go @@ -11,7 +11,6 @@ import ( "time" "github.com/fabric8-services/fabric8-wit/label" - "github.com/fabric8-services/fabric8-wit/numbersequence" "github.com/fabric8-services/fabric8-wit/account" "github.com/fabric8-services/fabric8-wit/application/repository" @@ -24,6 +23,7 @@ import ( "github.com/fabric8-services/fabric8-wit/log" "github.com/fabric8-services/fabric8-wit/rendering" "github.com/fabric8-services/fabric8-wit/space" + "github.com/fabric8-services/fabric8-wit/workitem/number_sequence" "github.com/goadesign/goa" "github.com/jinzhu/gorm" errs "github.com/pkg/errors" @@ -110,6 +110,7 @@ type WorkItemRepository interface { func NewWorkItemRepository(db *gorm.DB) *GormWorkItemRepository { repository := &GormWorkItemRepository{ db: db, + winr: numbersequence.NewWorkItemNumberSequenceRepository(db), witr: &GormWorkItemTypeRepository{db}, wirr: &GormRevisionRepository{db}, space: space.NewRepository(db), @@ -120,6 +121,7 @@ func NewWorkItemRepository(db *gorm.DB) *GormWorkItemRepository { // GormWorkItemRepository implements WorkItemRepository using gorm type GormWorkItemRepository struct { db *gorm.DB + winr *numbersequence.GormWorkItemNumberSequenceRepository witr *GormWorkItemTypeRepository wirr *GormRevisionRepository space *space.GormRepository @@ -711,13 +713,17 @@ func (r *GormWorkItemRepository) Create(ctx context.Context, spaceID uuid.UUID, return nil, nil, errors.NewInternalError(ctx, err) } pos = pos + orderValue + number, err := r.winr.NextVal(ctx, spaceID) + if err != nil { + return nil, nil, errors.NewInternalError(ctx, err) + } wi := WorkItemStorage{ Type: typeID, Fields: Fields{}, ExecutionOrder: pos, SpaceID: spaceID, + Number: *number, } - wi.HumanFriendlyNumber = numbersequence.NewHumanFriendlyNumber(spaceID, wi.TableName()) fields[SystemCreator] = creatorID.String() for fieldName, fieldDef := range wiType.Fields { if fieldDef.ReadOnly { diff --git a/workitem/workitem_storage.go b/workitem/workitem_storage.go index bfd8336024..0ed5f788d2 100644 --- a/workitem/workitem_storage.go +++ b/workitem/workitem_storage.go @@ -8,7 +8,6 @@ import ( "github.com/fabric8-services/fabric8-wit/convert" "github.com/fabric8-services/fabric8-wit/errors" "github.com/fabric8-services/fabric8-wit/gormsupport" - "github.com/fabric8-services/fabric8-wit/numbersequence" uuid "github.com/satori/go.uuid" ) @@ -16,9 +15,10 @@ import ( // WorkItemStorage represents a work item as it is stored in the database type WorkItemStorage struct { gormsupport.Lifecycle - numbersequence.HumanFriendlyNumber // unique id per installation (used for references at the DB level) ID uuid.UUID `sql:"type:uuid default uuid_generate_v4()" gorm:"primary_key"` + // unique number per _space_ + Number int // Id of the type of this work item Type uuid.UUID `sql:"type:uuid"` // Version for optimistic concurrency control @@ -55,7 +55,7 @@ func (wi WorkItemStorage) Equal(u convert.Equaler) bool { if !convert.CascadeEqual(wi.Lifecycle, other.Lifecycle) { return false } - if !convert.CascadeEqual(wi.HumanFriendlyNumber, other.HumanFriendlyNumber) { + if wi.Number != other.Number { return false } if wi.Type != other.Type {