Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Commit

Permalink
Assign number to new areas/iterations and allow searching by it (#2311)
Browse files Browse the repository at this point in the history
Existing iterations and areas are not touched and won't have a number
assigned to them. This will come in a later change.

This is a code-only change to proof that it is backwards compatible.

See https://openshift.io/openshiftio/Openshift_io/plan/detail/618

Most of the code in this change comes from this PR which was later revered: #2311
  • Loading branch information
kwk authored Oct 10, 2018
1 parent 87bf305 commit 11904c7
Show file tree
Hide file tree
Showing 27 changed files with 434 additions and 18 deletions.
4 changes: 4 additions & 0 deletions area/area.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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"
Expand All @@ -23,6 +24,7 @@ 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
Expand Down Expand Up @@ -81,6 +83,8 @@ 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}
Expand Down
17 changes: 15 additions & 2 deletions area/area_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ func (s *TestAreaRepository) TestCreateAreaWithSameNameFail() {
assert.IsType(s.T(), errors.DataConflictError{}, errs.Cause(err))
}

func (s *TestAreaRepository) TestCreateArea() {
func (s *TestAreaRepository) TestCreate() {
// given
repo := area.NewAreaRepository(s.DB)
name := "TestCreateArea"
fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(1))
fxt := tf.NewTestFixture(s.T(), s.DB, tf.Spaces(2))
a := area.Area{
Name: name,
SpaceID: fxt.Spaces[0].ID,
Expand All @@ -65,6 +65,19 @@ func (s *TestAreaRepository) TestCreateArea() {
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() {
Expand Down
1 change: 1 addition & 0 deletions controller/area.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ 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{
Expand Down
1 change: 1 addition & 0 deletions controller/iteration.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ 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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"attributes": {
"created-at": "0001-01-01T00:00:00Z",
"name": "root",
"number": 1,
"parent_path": "/",
"parent_path_resolved": "/",
"updated-at": "0001-01-01T00:00:00Z",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"attributes": {
"created-at": "0001-01-01T00:00:00Z",
"name": "root",
"number": 1,
"parent_path": "/",
"parent_path_resolved": "/",
"updated-at": "0001-01-01T00:00:00Z",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"attributes": {
"created-at": "0001-01-01T00:00:00Z",
"name": "root",
"number": 1,
"parent_path": "/",
"parent_path_resolved": "/",
"updated-at": "0001-01-01T00:00:00Z",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"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",
Expand Down
1 change: 1 addition & 0 deletions design/areas.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ 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() {
Expand Down
7 changes: 3 additions & 4 deletions design/iterations.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,15 @@ 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", func() {
})
a.Attribute("active_status", d.Boolean, "Active status of iteration calculated using user_active, startAt and endAt", func() {
})
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("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() {
Expand Down
3 changes: 3 additions & 0 deletions iteration/iteration.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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"
Expand All @@ -29,6 +30,7 @@ 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
Expand Down Expand Up @@ -137,6 +139,7 @@ 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}
Expand Down
25 changes: 17 additions & 8 deletions iteration/iteration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestRunIterationRepository(t *testing.T) {
suite.Run(t, &TestIterationRepository{DBTestSuite: gormtestsupport.NewDBTestSuite()})
}

func (s *TestIterationRepository) TestCreateIteration() {
func (s *TestIterationRepository) TestCreate() {
t := s.T()
resource.Require(t, resource.Database)
repo := iteration.NewIterationRepository(s.DB)
Expand All @@ -35,7 +35,7 @@ func (s *TestIterationRepository) TestCreateIteration() {
end := start.Add(time.Hour * (24 * 8 * 3))
name := "Sprint #24"
// given
fxt := tf.NewTestFixture(t, s.DB, tf.Spaces(1))
fxt := tf.NewTestFixture(t, s.DB, tf.Spaces(2))

i := iteration.Iteration{
Name: name,
Expand All @@ -47,15 +47,24 @@ func (s *TestIterationRepository) TestCreateIteration() {
err := repo.Create(context.Background(), &i)
// then
require.NoError(t, err)
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()?")
}
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()?")
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) {
Expand Down
107 changes: 107 additions & 0 deletions numbersequence/human_friendly_number.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
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(&currentVal)
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=<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
}
Loading

0 comments on commit 11904c7

Please sign in to comment.