diff --git a/controller/workitem_blackbox_test.go b/controller/workitem_blackbox_test.go index 0272aa9898..48d25c47e1 100644 --- a/controller/workitem_blackbox_test.go +++ b/controller/workitem_blackbox_test.go @@ -782,12 +782,12 @@ func createOneRandomIteration(ctx context.Context, db *gorm.DB) *iteration.Itera return &itr } -func createOneRandomArea(ctx context.Context, db *gorm.DB, testName string) *area.Area { +func createOneRandomArea(ctx context.Context, db *gorm.DB) *area.Area { areaRepo := area.NewAreaRepository(db) spaceRepo := space.NewRepository(db) newSpace := space.Space{ - Name: fmt.Sprintf("Space area %v %v", testName, uuid.NewV4()), + Name: fmt.Sprintf("Space area %v", uuid.NewV4()), } space, err := spaceRepo.Create(ctx, &newSpace) if err != nil { @@ -1491,7 +1491,7 @@ func (s *WorkItem2Suite) TestWI2ListByStateFilterOKModifiedUsingIfNoneMatchIfMod } func (s *WorkItem2Suite) setupAreaWorkItem(createWorkItem bool) (uuid.UUID, string, *app.WorkItemSingle) { - tempArea := createOneRandomArea(s.svc.Context, s.DB, "TestWI2ListByAreaFilter") + tempArea := createOneRandomArea(s.svc.Context, s.DB) require.NotNil(s.T(), tempArea) areaID := tempArea.ID.String() c := minimumRequiredCreatePayload() diff --git a/migration/migration.go b/migration/migration.go index 7ad5f8267d..12114dbd5e 100644 --- a/migration/migration.go +++ b/migration/migration.go @@ -352,6 +352,9 @@ func GetMigrations() Migrations { // Version 76 m = append(m, steps{ExecuteSQLFile("076-drop-space-resources-and-oauth-state.sql")}) + // Version 77 + m = append(m, steps{ExecuteSQLFile("077-work_item_number_sequence.sql")}) + // Version N // // In order to add an upgrade, simply append an array of MigrationFunc to the diff --git a/migration/sql-files/077-work_item_number_sequence.sql b/migration/sql-files/077-work_item_number_sequence.sql new file mode 100644 index 0000000000..fb41b2879b --- /dev/null +++ b/migration/sql-files/077-work_item_number_sequence.sql @@ -0,0 +1,5 @@ +-- modify the `work_item_number_sequences` to include an independant ID column for the primary key +alter table work_item_number_sequences drop constraint work_item_number_sequences_pkey; +alter table work_item_number_sequences add column id uuid primary key DEFAULT uuid_generate_v4() NOT NULL; + + diff --git a/search/search_repository_whitebox_test.go b/search/search_repository_whitebox_test.go index 879c663fe8..a96e17aab4 100644 --- a/search/search_repository_whitebox_test.go +++ b/search/search_repository_whitebox_test.go @@ -11,16 +11,14 @@ import ( "testing" "github.com/fabric8-services/fabric8-wit/gormtestsupport" - "github.com/fabric8-services/fabric8-wit/models" "github.com/fabric8-services/fabric8-wit/rendering" "github.com/fabric8-services/fabric8-wit/resource" "github.com/fabric8-services/fabric8-wit/space" testsupport "github.com/fabric8-services/fabric8-wit/test" + tf "github.com/fabric8-services/fabric8-wit/test/testfixture" "github.com/fabric8-services/fabric8-wit/workitem" "github.com/goadesign/goa" - "github.com/jinzhu/gorm" _ "github.com/lib/pq" - "github.com/pkg/errors" uuid "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -52,7 +50,6 @@ type SearchTestDescriptor struct { func (s *searchRepositoryWhiteboxTest) TestSearchByText() { wir := workitem.NewWorkItemRepository(s.DB) - testDataSet := []SearchTestDescriptor{ { wi: workitem.WorkItem{ @@ -144,90 +141,82 @@ func (s *searchRepositoryWhiteboxTest) TestSearchByText() { minimumResults: 1, }, } + // create the parent space + fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(1)) + // + for _, testData := range testDataSet { + workItem := testData.wi + searchString := testData.searchString + minimumResults := testData.minimumResults + workItemURLInSearchString := "http://demo.almighty.io/work-item/list/detail/" + req := &http.Request{Host: "localhost"} + params := url.Values{} + ctx := goa.NewContext(context.Background(), nil, req, params) + createdWorkItem, err := wir.Create(ctx, fxt.Spaces[0].ID, workitem.SystemBug, workItem.Fields, s.modifierID) + require.Nil(s.T(), err, "failed to create test data") + + // create the URL and use it in the search string + workItemURLInSearchString = workItemURLInSearchString + strconv.Itoa(createdWorkItem.Number) + + // had to dynamically create this since I didn't now the URL/ID of the workitem + // till the test data was created. + searchString = searchString + workItemURLInSearchString + searchString = fmt.Sprintf("\"%s\"", searchString) + s.T().Log("using search string: " + searchString) + sr := NewGormSearchRepository(s.DB) + var start, limit int = 0, 100 + spaceID := fxt.Spaces[0].ID.String() + workItemList, _, err := sr.SearchFullText(ctx, searchString, &start, &limit, &spaceID) + require.Nil(s.T(), err, "failed to get search result") + searchString = strings.Trim(searchString, "\"") + // Since this test adds test data, whether or not other workitems exist + // there must be at least 1 search result returned. + if len(workItemList) == minimumResults && minimumResults == 0 { + // no point checking further, we got what we wanted. + continue + } else if len(workItemList) < minimumResults { + s.T().Fatalf("At least %d search result(s) was|were expected ", minimumResults) + } - models.Transactional(s.DB, func(tx *gorm.DB) error { - - for _, testData := range testDataSet { - workItem := testData.wi - searchString := testData.searchString - minimumResults := testData.minimumResults - workItemURLInSearchString := "http://demo.almighty.io/work-item/list/detail/" - req := &http.Request{Host: "localhost"} - params := url.Values{} - ctx := goa.NewContext(context.Background(), nil, req, params) - - createdWorkItem, err := wir.Create(ctx, space.SystemSpace, workitem.SystemBug, workItem.Fields, s.modifierID) - require.Nil(s.T(), err, "failed to create test data") - - // create the URL and use it in the search string - workItemURLInSearchString = workItemURLInSearchString + strconv.Itoa(createdWorkItem.Number) - - // had to dynamically create this since I didn't now the URL/ID of the workitem - // till the test data was created. - searchString = searchString + workItemURLInSearchString - searchString = fmt.Sprintf("\"%s\"", searchString) - s.T().Log("using search string: " + searchString) - sr := NewGormSearchRepository(tx) - var start, limit int = 0, 100 - workItemList, _, err := sr.SearchFullText(ctx, searchString, &start, &limit, nil) - require.Nil(s.T(), err, "failed to get search result") - searchString = strings.Trim(searchString, "\"") - // Since this test adds test data, whether or not other workitems exist - // there must be at least 1 search result returned. - if len(workItemList) == minimumResults && minimumResults == 0 { - // no point checking further, we got what we wanted. - continue - } else if len(workItemList) < minimumResults { - s.T().Fatalf("At least %d search result(s) was|were expected ", minimumResults) - } + // These keywords need a match in the textual part. + allKeywords := strings.Fields(searchString) + allKeywords = append(allKeywords, strconv.Itoa(createdWorkItem.Number)) + //[]string{workItemURLInSearchString, createdWorkItem.ID, `"Sbose"`, `"deScription"`, `'12345678asdfgh'`} + + // These keywords need a match optionally either as URL string or ID + optionalKeywords := []string{workItemURLInSearchString, strconv.Itoa(createdWorkItem.Number)} + + // We will now check the legitimacy of the search results. + // Iterate through all search results and see whether they meet the criteria - // These keywords need a match in the textual part. - allKeywords := strings.Fields(searchString) - allKeywords = append(allKeywords, strconv.Itoa(createdWorkItem.Number)) - //[]string{workItemURLInSearchString, createdWorkItem.ID, `"Sbose"`, `"deScription"`, `'12345678asdfgh'`} - - // These keywords need a match optionally either as URL string or ID - optionalKeywords := []string{workItemURLInSearchString, strconv.Itoa(createdWorkItem.Number)} - - // We will now check the legitimacy of the search results. - // Iterate through all search results and see whether they meet the criteria - - for _, workItemValue := range workItemList { - s.T().Log("Found search result ", workItemValue.ID) - - for _, keyWord := range allKeywords { - - workItemTitle := "" - if workItemValue.Fields[workitem.SystemTitle] != nil { - workItemTitle = strings.ToLower(workItemValue.Fields[workitem.SystemTitle].(string)) - } - workItemDescription := "" - if workItemValue.Fields[workitem.SystemDescription] != nil { - descriptionField := workItemValue.Fields[workitem.SystemDescription].(rendering.MarkupContent) - workItemDescription = strings.ToLower(descriptionField.Content) - } - workItemNumber := 0 - if workItemValue.Fields[workitem.SystemNumber] != nil { - workItemNumber = workItemValue.Fields[workitem.SystemNumber].(int) - } - keyWord = strings.ToLower(keyWord) - - if strings.Contains(workItemTitle, keyWord) || strings.Contains(workItemDescription, keyWord) { - // Check if the search keyword is present as text in the title/description - s.T().Logf("Found keyword %s in workitem %s", keyWord, workItemValue.ID) - } else if stringInSlice(keyWord, optionalKeywords) && strings.Contains(keyWord, strconv.Itoa(workItemValue.Number)) { - // If not present in title/description then it should be a URL or ID - s.T().Logf("Found keyword '%s' as number '%s' from the URL", keyWord, strconv.Itoa(workItemValue.Number)) - } else { - s.T().Errorf("'%s' neither found in title '%s' nor in the description: '%s' for workitem number %d", keyWord, workItemTitle, workItemDescription, workItemNumber) - } + for _, workItemValue := range workItemList { + s.T().Log("Found search result ", workItemValue.ID) + + for _, keyWord := range allKeywords { + + workItemTitle := "" + if workItemValue.Fields[workitem.SystemTitle] != nil { + workItemTitle = strings.ToLower(workItemValue.Fields[workitem.SystemTitle].(string)) + } + workItemDescription := "" + if workItemValue.Fields[workitem.SystemDescription] != nil { + descriptionField := workItemValue.Fields[workitem.SystemDescription].(rendering.MarkupContent) + workItemDescription = strings.ToLower(descriptionField.Content) + } + keyWord = strings.ToLower(keyWord) + if strings.Contains(workItemTitle, keyWord) || strings.Contains(workItemDescription, keyWord) { + // Check if the search keyword is present as text in the title/description + s.T().Logf("Found keyword %s in workitem %s", keyWord, workItemValue.ID) + } else if stringInSlice(keyWord, optionalKeywords) && strings.Contains(keyWord, strconv.Itoa(workItemValue.Number)) { + // If not present in title/description then it should be a URL or ID + s.T().Logf("Found keyword '%s' as number '%s' from the URL", keyWord, strconv.Itoa(workItemValue.Number)) + } else { + s.T().Errorf("'%s' neither found in title '%s' nor in the description: '%s' for workitem number %d", keyWord, workItemTitle, workItemDescription, workItemValue.Number) } } - } - return nil - }) + } } func stringInSlice(str string, list []string) bool { @@ -240,55 +229,51 @@ func stringInSlice(str string, list []string) bool { } func (s *searchRepositoryWhiteboxTest) TestSearchByID() { + req := &http.Request{Host: "localhost"} + params := url.Values{} + ctx := goa.NewContext(context.Background(), nil, req, params) - models.Transactional(s.DB, func(tx *gorm.DB) error { - req := &http.Request{Host: "localhost"} - params := url.Values{} - ctx := goa.NewContext(context.Background(), nil, req, params) - - wir := workitem.NewWorkItemRepository(tx) + wir := workitem.NewWorkItemRepository(s.DB) - workItem := workitem.WorkItem{Fields: make(map[string]interface{})} + workItem := workitem.WorkItem{Fields: make(map[string]interface{})} - workItem.Fields = map[string]interface{}{ - workitem.SystemTitle: "Search Test Sbose", - workitem.SystemDescription: rendering.NewMarkupContentFromLegacy("Description"), - workitem.SystemCreator: "sbose78", - workitem.SystemAssignees: []string{"pranav"}, - workitem.SystemState: "closed", - } + workItem.Fields = map[string]interface{}{ + workitem.SystemTitle: "Search Test Sbose", + workitem.SystemDescription: rendering.NewMarkupContentFromLegacy("Description"), + workitem.SystemCreator: "sbose78", + workitem.SystemAssignees: []string{"pranav"}, + workitem.SystemState: "closed", + } - createdWorkItem, err := wir.Create(ctx, space.SystemSpace, workitem.SystemBug, workItem.Fields, s.modifierID) - if err != nil { - s.T().Fatalf("Couldn't create test data: %+v", err) - } + createdWorkItem, err := wir.Create(ctx, space.SystemSpace, workitem.SystemBug, workItem.Fields, s.modifierID) + if err != nil { + s.T().Fatalf("Couldn't create test data: %+v", err) + } - // Create a new workitem to have the ID in it's title. This should not come - // up in search results + // Create a new workitem to have the ID in it's title. This should not come + // up in search results - workItem.Fields[workitem.SystemTitle] = "Search test sbose " + createdWorkItem.ID.String() - _, err = wir.Create(ctx, space.SystemSpace, workitem.SystemBug, workItem.Fields, s.modifierID) - if err != nil { - s.T().Fatalf("Couldn't create test data: %+v", err) - } + workItem.Fields[workitem.SystemTitle] = "Search test sbose " + createdWorkItem.ID.String() + _, err = wir.Create(ctx, space.SystemSpace, workitem.SystemBug, workItem.Fields, s.modifierID) + if err != nil { + s.T().Fatalf("Couldn't create test data: %+v", err) + } - sr := NewGormSearchRepository(tx) + sr := NewGormSearchRepository(s.DB) - var start, limit int = 0, 100 - searchString := "number:" + strconv.Itoa(createdWorkItem.Number) - workItemList, _, err := sr.SearchFullText(ctx, searchString, &start, &limit, nil) - if err != nil { - s.T().Fatal("Error gettig search result ", err) - } + var start, limit int = 0, 100 + searchString := "number:" + strconv.Itoa(createdWorkItem.Number) + workItemList, _, err := sr.SearchFullText(ctx, searchString, &start, &limit, nil) + if err != nil { + s.T().Fatal("Error gettig search result ", err) + } - // ID is unique, hence search result set's length should be 1 - assert.Equal(s.T(), len(workItemList), 1) - for _, workItemValue := range workItemList { - s.T().Log("Found search result for ID Search ", workItemValue.ID) - assert.Equal(s.T(), createdWorkItem.ID, workItemValue.ID) - } - return errors.WithStack(err) - }) + // ID is unique, hence search result set's length should be 1 + assert.Equal(s.T(), len(workItemList), 1) + for _, workItemValue := range workItemList { + s.T().Log("Found search result for ID Search ", workItemValue.ID) + assert.Equal(s.T(), createdWorkItem.ID, workItemValue.ID) + } } func TestGenerateSQLSearchStringText(t *testing.T) { diff --git a/space/space.go b/space/space.go index 8441e06c75..c2b84383f9 100644 --- a/space/space.go +++ b/space/space.go @@ -11,6 +11,7 @@ import ( "github.com/fabric8-services/fabric8-wit/errors" "github.com/fabric8-services/fabric8-wit/gormsupport" "github.com/fabric8-services/fabric8-wit/log" + numbersequence "github.com/fabric8-services/fabric8-wit/workitem/number_sequence" "github.com/goadesign/goa" "github.com/jinzhu/gorm" @@ -93,12 +94,16 @@ type Repository interface { // NewRepository creates a new space repo func NewRepository(db *gorm.DB) *GormRepository { - return &GormRepository{db} + return &GormRepository{ + 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 @@ -225,7 +230,11 @@ func (r *GormRepository) Create(ctx context.Context, space *Space) (*Space, erro } return nil, errors.NewInternalError(ctx, err) } - + // also, initialize the work_item_number_sequence table for this space + _, err := r.winr.Create(ctx, space.ID) + if err != nil { + return nil, errors.NewInternalError(ctx, err) + } log.Info(ctx, map[string]interface{}{ "space_id": space.ID, }, "Space created successfully") diff --git a/workitem/workitem_number.go b/workitem/number_sequence/workitem_number_sequence.go similarity index 58% rename from workitem/workitem_number.go rename to workitem/number_sequence/workitem_number_sequence.go index fd686163a0..5a062e905f 100644 --- a/workitem/workitem_number.go +++ b/workitem/number_sequence/workitem_number_sequence.go @@ -1,12 +1,15 @@ -package workitem +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"` + ID uuid.UUID `sql:"type:uuid" gorm:"primary_key"` + SpaceID uuid.UUID `sql:"type:uuid"` CurrentVal int } @@ -18,3 +21,7 @@ const ( 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..b08b57b305 --- /dev/null +++ b/workitem/number_sequence/workitem_number_sequence_repository.go @@ -0,0 +1,56 @@ +package numbersequence + +import ( + "context" + + "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 { + Create(ctx context.Context, spaceID uuid.UUID) (*WorkItemNumberSequence, error) + NextVal(ctx context.Context, spaceID uuid.UUID) (*WorkItemNumberSequence, 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 +} + +// Create 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) Create(ctx context.Context, spaceID uuid.UUID) (*WorkItemNumberSequence, error) { + // retrieve the current issue number in the given space + numberSequence := WorkItemNumberSequence{SpaceID: spaceID, CurrentVal: 0} + if err := r.db.Save(&numberSequence).Error; err != nil { + return nil, errs.Wrapf(err, "failed to create work item with sequence number: `%s`", numberSequence.String()) + } + log.Warn(nil, map[string]interface{}{"Sequence": numberSequence.String()}, "Creating sequence") + return &numberSequence, nil +} + +// 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) (*WorkItemNumberSequence, error) { + // retrieve the current issue number in the given space + numberSequence := WorkItemNumberSequence{} + tx := r.db.Model(&WorkItemNumberSequence{}).Set("gorm:query_option", "FOR UPDATE").Where("space_id = ?", spaceID).First(&numberSequence) + if tx.RecordNotFound() { + numberSequence.SpaceID = spaceID + numberSequence.CurrentVal = 1 + } else { + numberSequence.CurrentVal++ + } + if err := r.db.Save(&numberSequence).Error; err != nil { + return nil, errs.Wrapf(err, "failed to update work item with sequence number: `%s`", numberSequence.String()) + } + log.Warn(nil, map[string]interface{}{"Sequence": numberSequence.String()}, "computing nextVal") + return &numberSequence, nil +} diff --git a/workitem/workitem_repository.go b/workitem/workitem_repository.go index dfdeaa3582..ed17077efc 100644 --- a/workitem/workitem_repository.go +++ b/workitem/workitem_repository.go @@ -15,6 +15,7 @@ import ( "github.com/fabric8-services/fabric8-wit/log" "github.com/fabric8-services/fabric8-wit/rendering" "github.com/fabric8-services/fabric8-wit/space" + numbersequence "github.com/fabric8-services/fabric8-wit/workitem/number_sequence" "github.com/goadesign/goa" "github.com/jinzhu/gorm" @@ -53,13 +54,19 @@ type WorkItemRepository interface { // NewWorkItemRepository creates a GormWorkItemRepository func NewWorkItemRepository(db *gorm.DB) *GormWorkItemRepository { - repository := &GormWorkItemRepository{db, &GormWorkItemTypeRepository{db}, &GormRevisionRepository{db}} + repository := &GormWorkItemRepository{ + db, + numbersequence.NewWorkItemNumberSequenceRepository(db), + &GormWorkItemTypeRepository{db}, + &GormRevisionRepository{db}, + } return repository } // GormWorkItemRepository implements WorkItemRepository using gorm type GormWorkItemRepository struct { db *gorm.DB + winr *numbersequence.GormWorkItemNumberSequenceRepository witr *GormWorkItemTypeRepository wirr *GormRevisionRepository } @@ -551,18 +558,6 @@ func (r *GormWorkItemRepository) Create(ctx context.Context, spaceID uuid.UUID, if err != nil { return nil, errors.NewBadParameterError("typeID", typeID) } - // retrieve the current issue number in the given space - numberSequence := WorkItemNumberSequence{} - tx := r.db.Model(&WorkItemNumberSequence{}).Set("gorm:query_option", "FOR UPDATE").Where("space_id = ?", spaceID).First(&numberSequence) - if tx.RecordNotFound() { - numberSequence.SpaceID = spaceID - numberSequence.CurrentVal = 1 - } else { - numberSequence.CurrentVal++ - } - if err = r.db.Save(&numberSequence).Error; err != nil { - return nil, errs.Wrapf(err, "failed to create work item") - } // The order of workitems are spaced by a factor of 1000. pos, err := r.LoadHighestOrder(ctx, spaceID) @@ -570,12 +565,16 @@ func (r *GormWorkItemRepository) Create(ctx context.Context, spaceID uuid.UUID, return nil, errors.NewInternalError(ctx, err) } pos = pos + orderValue + wiNumberSequence, err := r.winr.NextVal(ctx, spaceID) + if err != nil { + return nil, errors.NewInternalError(ctx, err) + } wi := WorkItemStorage{ Type: typeID, Fields: Fields{}, ExecutionOrder: pos, SpaceID: spaceID, - Number: numberSequence.CurrentVal, + Number: wiNumberSequence.CurrentVal, } fields[SystemCreator] = creatorID.String() for fieldName, fieldDef := range wiType.Fields { @@ -608,7 +607,7 @@ func (r *GormWorkItemRepository) Create(ctx context.Context, spaceID uuid.UUID, if err != nil { return nil, errs.Wrapf(err, "error while creating work item") } - log.Debug(ctx, map[string]interface{}{"pkg": "workitem", "wi_id": wi.ID}, "Work item created successfully!") + log.Debug(ctx, map[string]interface{}{"pkg": "workitem", "wi_id": wi.ID, "number": wi.Number}, "Work item created successfully!") return witem, nil }