From 87bf3711e1d4aac790f122ecc6796a96b90bb0b5 Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Thu, 24 May 2018 22:43:07 +0200 Subject: [PATCH 1/3] Validate space name with a regexp in the controller (OSIO#3580) This will prevent creating spaces with a name that cannot be used as a value in pod labels (matching max length and pattern). The regexp and length checks in the controller replace the validation function that was previously used at the design level, which allows for returning proper JSON-API errors to the clients. Fixes openshiftio/openshift.io#3580 Signed-off-by: Xavier Coulon --- controller/space.go | 23 ++ controller/space_blackbox_test.go | 432 +++++++++++++++--------------- design/resources.go | 4 + design/spaces.go | 2 +- errors/errors.go | 4 +- 5 files changed, 242 insertions(+), 223 deletions(-) diff --git a/controller/space.go b/controller/space.go index 9d908a3948..caefef2751 100644 --- a/controller/space.go +++ b/controller/space.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "regexp" "github.com/fabric8-services/fabric8-wit/app" "github.com/fabric8-services/fabric8-wit/application" @@ -546,6 +547,18 @@ func (c *SpaceController) Update(ctx *app.UpdateSpaceContext) error { return ctx.OK(&response) } +const ( + // see https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/identifiers.md + spaceNameMaxLength int = 63 + spaceNamePattern string = `^([A-Za-z0-9][-A-Za-z0-9]*)?[A-Za-z0-9]$` +) + +var nameRegex *regexp.Regexp + +func init() { + nameRegex = regexp.MustCompile(spaceNamePattern) // will panic if the pattern is invalid +} + func validateCreateSpace(ctx *app.CreateSpaceContext) error { if ctx.Payload.Data == nil { return errors.NewBadParameterError("data", nil).Expected("not nil") @@ -556,6 +569,16 @@ func validateCreateSpace(ctx *app.CreateSpaceContext) error { if ctx.Payload.Data.Attributes.Name == nil { return errors.NewBadParameterError("data.attributes.name", nil).Expected("not nil") } + name := *ctx.Payload.Data.Attributes.Name + // now, verify the length and pattern for the space name + if len(name) > spaceNameMaxLength { + return errors.NewBadParameterError("data.attributes.name", name).Expected(fmt.Sprintf("max length: %d (was %d)", spaceNameMaxLength, len(name))) + } + matched := nameRegex.MatchString(name) + if !matched { + return errors.NewBadParameterError("data.attributes.name", name).Expected(fmt.Sprintf("matching '%s' pattern", spaceNamePattern)) + } + // // TODO(kwk): Comment back in once space template is official // if ctx.Payload.Data.Relationships == nil { // return errors.NewBadParameterError("data.relationships", nil).Expected("not nil") diff --git a/controller/space_blackbox_test.go b/controller/space_blackbox_test.go index d0bf72b340..6e3f7926e5 100644 --- a/controller/space_blackbox_test.go +++ b/controller/space_blackbox_test.go @@ -154,241 +154,233 @@ func (s *SpaceControllerTestSuite) SecuredSpaceIterationController(identity acco return svc, NewSpaceIterationsController(svc, s.db, s.Configuration) } -func (s *SpaceControllerTestSuite) TestValidateSpaceName() { +func (s *SpaceControllerTestSuite) TestCreateSpace() { s.T().Run("ok", func(t *testing.T) { - // given - p := newCreateSpacePayload(&testsupport.TestMaxsizedNameObj, nil) - // when - err := p.Validate() - // Validate payload function returns no error - assert.Nil(t, err) - }) - - s.T().Run("Fail - length", func(t *testing.T) { - // given - p := newCreateSpacePayload(&testsupport.TestOversizedNameObj, nil) - // when - err := p.Validate() - // Validate payload function returns an error - assert.NotNil(t, err) - assert.Contains(t, err.Error(), "length of type.name must be less than or equal to 63 but got") - }) - s.T().Run("Fail - prefix", func(t *testing.T) { - // given - invalidSpaceName := "_TestSpace" - p := newCreateSpacePayload(&invalidSpaceName, nil) - // when - err := p.Validate() - // Validate payload function returns an error - assert.NotNil(t, err) - assert.Contains(t, err.Error(), "type.name must match the regexp") - }) -} + t.Run("valid name", func(t *testing.T) { + // given + name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpace-") + p := newCreateSpacePayload(&name, nil) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + // when + compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.payload.req.golden.json"), p) + res, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) + // then + require.NotNil(t, created.Data) + require.NotNil(t, created.Data.Attributes) + assert.NotNil(t, created.Data.Attributes.CreatedAt) + assert.NotNil(t, created.Data.Attributes.UpdatedAt) + require.NotNil(t, created.Data.Attributes.Name) + assert.Equal(t, name, *created.Data.Attributes.Name) + require.NotNil(t, created.Data.Links) + assert.NotNil(t, created.Data.Links.Self) + compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.payload.res.golden.json"), created) + compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.headers.res.golden.json"), res.Header()) + }) -func (s *SpaceControllerTestSuite) TestCreateSpace() { - s.T().Run("Fail - unsecure", func(t *testing.T) { - // given - p := newCreateSpacePayload(nil, nil) - svc, ctrl := s.UnSecuredController() - // when/then - test.CreateSpaceUnauthorized(t, svc.Context, svc, ctrl, p) - }) + t.Run("with explicit template", func(t *testing.T) { + // given + fxt := tf.NewTestFixture(t, s.DB, tf.SpaceTemplates(1)) + name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpace-") + p := newCreateSpacePayload(&name, nil) - s.T().Run("Fail - auth returned 400", func(t *testing.T) { - // given - spaceName := uuid.NewV4().String() - p := newCreateSpacePayload(&spaceName, nil) - r := DummyResourceManager{ - httpResponseCode: 400, - } - svc, ctrl := s.SecuredControllerWithDummyResourceManager(testsupport.TestIdentity, r) - // when/then - test.CreateSpaceBadRequest(t, svc.Context, svc, ctrl, p) - }) - s.T().Run("Fail - auth returned 401", func(t *testing.T) { - // given - spaceName := uuid.NewV4().String() - p := newCreateSpacePayload(&spaceName, nil) - r := DummyResourceManager{ - httpResponseCode: 401, - } - svc, ctrl := s.SecuredControllerWithDummyResourceManager(testsupport.TestIdentity, r) - // when/then - test.CreateSpaceUnauthorized(t, svc.Context, svc, ctrl, p) - }) - s.T().Run("Fail - auth returned 500", func(t *testing.T) { - // given - spaceName := uuid.NewV4().String() - p := newCreateSpacePayload(&spaceName, nil) - r := DummyResourceManager{ - httpResponseCode: 500, - } - svc, ctrl := s.SecuredControllerWithDummyResourceManager(testsupport.TestIdentity, r) - // when/then - test.CreateSpaceInternalServerError(t, svc.Context, svc, ctrl, p) - }) + if p.Data.Relationships == nil { + p.Data.Relationships = &app.SpaceRelationships{} + } + p.Data.Relationships.SpaceTemplate = app.NewSpaceTemplateRelation( + fxt.SpaceTemplates[0].ID, + rest.AbsoluteURL( + &http.Request{Host: "api.service.domain.org"}, + app.SpaceTemplateHref(fxt.SpaceTemplates[0].ID.String()), + ), + ) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + // when + compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok_with_explicit_template.payload.req.golden.json"), p) + res, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) + // then + require.NotNil(t, created.Data) + require.NotNil(t, created.Data.Attributes) + assert.NotNil(t, created.Data.Attributes.CreatedAt) + assert.NotNil(t, created.Data.Attributes.UpdatedAt) + require.NotNil(t, created.Data.Attributes.Name) + assert.Equal(t, name, *created.Data.Attributes.Name) + require.NotNil(t, created.Data.Links) + assert.NotNil(t, created.Data.Links.Self) + compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok_with_explicit_template.payload.res.golden.json"), created) + compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok_with_explicit_template.headers.res.golden.json"), res.Header()) + }) - s.T().Run("ok", func(t *testing.T) { - // given - name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpace-") - p := newCreateSpacePayload(&name, nil) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - // when - compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.payload.req.golden.json"), p) - res, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) - // then - require.NotNil(t, created.Data) - require.NotNil(t, created.Data.Attributes) - assert.NotNil(t, created.Data.Attributes.CreatedAt) - assert.NotNil(t, created.Data.Attributes.UpdatedAt) - require.NotNil(t, created.Data.Attributes.Name) - assert.Equal(t, name, *created.Data.Attributes.Name) - require.NotNil(t, created.Data.Links) - assert.NotNil(t, created.Data.Links.Self) - compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.payload.res.golden.json"), created) - compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.headers.res.golden.json"), res.Header()) - }) + t.Run("with default area", func(t *testing.T) { + // given + name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpaceAndDefaultArea-") + p := newCreateSpacePayload(&name, nil) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + // when + _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) + require.NotNil(t, created.Data) + spaceAreaSvc, spaceAreaCtrl := s.SecuredSpaceAreaController(testsupport.TestIdentity) + _, areaList := test.ListSpaceAreasOK(t, spaceAreaSvc.Context, spaceAreaSvc, spaceAreaCtrl, *created.Data.ID, nil, nil) + // then + // only 1 default gets created. + assert.Len(t, areaList.Data, 1) + assert.Equal(t, name, *areaList.Data[0].Attributes.Name) + + // verify if root iteration is created or not + spaceIterationSvc, spaceIterationCtrl := s.SecuredSpaceIterationController(testsupport.TestIdentity) + _, iterationList := test.ListSpaceIterationsOK(t, spaceIterationSvc.Context, spaceIterationSvc, spaceIterationCtrl, *created.Data.ID, nil, nil) + require.Len(t, iterationList.Data, 1) + assert.Equal(t, name, *iterationList.Data[0].Attributes.Name) + }) - s.T().Run("ok (with explicit template)", func(t *testing.T) { - // given - fxt := tf.NewTestFixture(t, s.DB, tf.SpaceTemplates(1)) - name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpace-") - p := newCreateSpacePayload(&name, nil) + t.Run("with description", func(t *testing.T) { + // given + name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpaceWithDescription-") + description := "Space for TestSuccessCreateSpaceWithDescription" + p := newCreateSpacePayload(&name, &description) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + // when + _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) + // then + assert.NotNil(t, created.Data) + assert.NotNil(t, created.Data.Attributes) + assert.NotNil(t, created.Data.Attributes.CreatedAt) + assert.NotNil(t, created.Data.Attributes.UpdatedAt) + assert.NotNil(t, created.Data.Attributes.Name) + assert.Equal(t, name, *created.Data.Attributes.Name) + assert.NotNil(t, created.Data.Attributes.Description) + assert.Equal(t, description, *created.Data.Attributes.Description) + assert.NotNil(t, created.Data.Links) + assert.NotNil(t, created.Data.Links.Self) + }) - if p.Data.Relationships == nil { - p.Data.Relationships = &app.SpaceRelationships{} - } - p.Data.Relationships.SpaceTemplate = app.NewSpaceTemplateRelation( - fxt.SpaceTemplates[0].ID, - rest.AbsoluteURL( - &http.Request{Host: "api.service.domain.org"}, - app.SpaceTemplateHref(fxt.SpaceTemplates[0].ID.String()), - ), - ) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - // when - compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok_with_explicit_template.payload.req.golden.json"), p) - res, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) - // then - require.NotNil(t, created.Data) - require.NotNil(t, created.Data.Attributes) - assert.NotNil(t, created.Data.Attributes.CreatedAt) - assert.NotNil(t, created.Data.Attributes.UpdatedAt) - require.NotNil(t, created.Data.Attributes.Name) - assert.Equal(t, name, *created.Data.Attributes.Name) - require.NotNil(t, created.Data.Links) - assert.NotNil(t, created.Data.Links.Self) - compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok_with_explicit_template.payload.res.golden.json"), created) - compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok_with_explicit_template.headers.res.golden.json"), res.Header()) - }) + t.Run("same name but different owner", func(t *testing.T) { + // given + name := testsupport.CreateRandomValidTestName("SameName-") + description := "Space for TestSuccessCreateSameSpaceNameDifferentOwners" + newDescription := "Space for TestSuccessCreateSameSpaceNameDifferentOwners2" + a := newCreateSpacePayload(&name, &description) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, a) + // when + b := newCreateSpacePayload(&name, &newDescription) + svc2, ctrl2 := s.SecuredController(testsupport.TestIdentity2) + _, created2 := test.CreateSpaceCreated(t, svc2.Context, svc2, ctrl2, b) + // then + assert.NotNil(t, created.Data) + assert.NotNil(t, created.Data.Attributes) + assert.NotNil(t, created.Data.Attributes.Name) + assert.Equal(t, name, *created.Data.Attributes.Name) + assert.NotNil(t, created2.Data) + assert.NotNil(t, created2.Data.Attributes) + assert.NotNil(t, created2.Data.Attributes.Name) + assert.Equal(t, name, *created2.Data.Attributes.Name) + assert.NotEqual(t, created.Data.Relationships.OwnedBy.Data.ID, created2.Data.Relationships.OwnedBy.Data.ID) + }) - s.T().Run("ok with default area", func(t *testing.T) { - // given - name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpaceAndDefaultArea-") - p := newCreateSpacePayload(&name, nil) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - // when - _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) - require.NotNil(t, created.Data) - spaceAreaSvc, spaceAreaCtrl := s.SecuredSpaceAreaController(testsupport.TestIdentity) - _, areaList := test.ListSpaceAreasOK(t, spaceAreaSvc.Context, spaceAreaSvc, spaceAreaCtrl, *created.Data.ID, nil, nil) - // then - // only 1 default gets created. - assert.Len(t, areaList.Data, 1) - assert.Equal(t, name, *areaList.Data[0].Attributes.Name) - - // verify if root iteration is created or not - spaceIterationSvc, spaceIterationCtrl := s.SecuredSpaceIterationController(testsupport.TestIdentity) - _, iterationList := test.ListSpaceIterationsOK(t, spaceIterationSvc.Context, spaceIterationSvc, spaceIterationCtrl, *created.Data.ID, nil, nil) - require.Len(t, iterationList.Data, 1) - assert.Equal(t, name, *iterationList.Data[0].Attributes.Name) + t.Run("with max length name", func(t *testing.T) { + // given + name := testsupport.TestMaxsizedNameObj + p := newCreateSpacePayload(&name, nil) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + // when + _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) + // then + require.NotNil(t, created.Data) + require.NotNil(t, created.Data.Attributes) + assert.NotNil(t, created.Data.Attributes.CreatedAt) + assert.NotNil(t, created.Data.Attributes.UpdatedAt) + require.NotNil(t, created.Data.Attributes.Name) + assert.Equal(t, name, *created.Data.Attributes.Name) + require.NotNil(t, created.Data.Links) + assert.NotNil(t, created.Data.Links.Self) + }) }) - s.T().Run("ok with description", func(t *testing.T) { - // given - name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpaceWithDescription-") - description := "Space for TestSuccessCreateSpaceWithDescription" - p := newCreateSpacePayload(&name, &description) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - // when - _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) - // then - assert.NotNil(t, created.Data) - assert.NotNil(t, created.Data.Attributes) - assert.NotNil(t, created.Data.Attributes.CreatedAt) - assert.NotNil(t, created.Data.Attributes.UpdatedAt) - assert.NotNil(t, created.Data.Attributes.Name) - assert.Equal(t, name, *created.Data.Attributes.Name) - assert.NotNil(t, created.Data.Attributes.Description) - assert.Equal(t, description, *created.Data.Attributes.Description) - assert.NotNil(t, created.Data.Links) - assert.NotNil(t, created.Data.Links.Self) - }) + s.T().Run("fail", func(t *testing.T) { - s.T().Run("ok same name but different owner", func(t *testing.T) { - // given - name := testsupport.CreateRandomValidTestName("SameName-") - description := "Space for TestSuccessCreateSameSpaceNameDifferentOwners" - newDescription := "Space for TestSuccessCreateSameSpaceNameDifferentOwners2" - a := newCreateSpacePayload(&name, &description) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, a) - // when - b := newCreateSpacePayload(&name, &newDescription) - svc2, ctrl2 := s.SecuredController(testsupport.TestIdentity2) - _, created2 := test.CreateSpaceCreated(t, svc2.Context, svc2, ctrl2, b) - // then - assert.NotNil(t, created.Data) - assert.NotNil(t, created.Data.Attributes) - assert.NotNil(t, created.Data.Attributes.Name) - assert.Equal(t, name, *created.Data.Attributes.Name) - assert.NotNil(t, created2.Data) - assert.NotNil(t, created2.Data.Attributes) - assert.NotNil(t, created2.Data.Attributes.Name) - assert.Equal(t, name, *created2.Data.Attributes.Name) - assert.NotEqual(t, created.Data.Relationships.OwnedBy.Data.ID, created2.Data.Relationships.OwnedBy.Data.ID) - }) + t.Run("same name and same owner", func(t *testing.T) { + // given + name := testsupport.CreateRandomValidTestName("SameName-") + description := "Space for TestSuccessCreateSameSpaceNameDifferentOwners" + newDescription := "Space for TestSuccessCreateSameSpaceNameDifferentOwners2" + // when + a := newCreateSpacePayload(&name, &description) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, a) + // then + assert.NotNil(t, created.Data) + assert.NotNil(t, created.Data.Attributes) + assert.NotNil(t, created.Data.Attributes.Name) + assert.Equal(t, name, *created.Data.Attributes.Name) - s.T().Run("ok with max length name", func(t *testing.T) { - // given - name := testsupport.TestMaxsizedNameObj - p := newCreateSpacePayload(&name, nil) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - // when - _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) - // then - require.NotNil(t, created.Data) - require.NotNil(t, created.Data.Attributes) - assert.NotNil(t, created.Data.Attributes.CreatedAt) - assert.NotNil(t, created.Data.Attributes.UpdatedAt) - require.NotNil(t, created.Data.Attributes.Name) - assert.Equal(t, name, *created.Data.Attributes.Name) - require.NotNil(t, created.Data.Links) - assert.NotNil(t, created.Data.Links.Self) - }) + // when + b := newCreateSpacePayload(&name, &newDescription) + b.Data.Attributes.Name = &name + b.Data.Attributes.Description = &newDescription + test.CreateSpaceConflict(t, svc.Context, svc, ctrl, b) + }) - s.T().Run("fail same name and same owner", func(t *testing.T) { - // given - name := testsupport.CreateRandomValidTestName("SameName-") - description := "Space for TestSuccessCreateSameSpaceNameDifferentOwners" - newDescription := "Space for TestSuccessCreateSameSpaceNameDifferentOwners2" - // when - a := newCreateSpacePayload(&name, &description) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, a) - // then - assert.NotNil(t, created.Data) - assert.NotNil(t, created.Data.Attributes) - assert.NotNil(t, created.Data.Attributes.Name) - assert.Equal(t, name, *created.Data.Attributes.Name) + t.Run("invalid name", func(t *testing.T) { + t.Run("invalid character", func(t *testing.T) { + // given + name := "foo@bar.com" + description := "Space with invalid name" + // when/then + p := newCreateSpacePayload(&name, &description) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + test.CreateSpaceBadRequest(t, svc.Context, svc, ctrl, p) + }) + + t.Run("invalid length", func(t *testing.T) { + // given + name := testsupport.TestOversizedNameObj + description := "Space with invalid name" + // when/then + p := newCreateSpacePayload(&name, &description) + svc, ctrl := s.SecuredController(testsupport.TestIdentity) + test.CreateSpaceBadRequest(t, svc.Context, svc, ctrl, p) + }) + }) - // when - b := newCreateSpacePayload(&name, &newDescription) - b.Data.Attributes.Name = &name - b.Data.Attributes.Description = &newDescription - test.CreateSpaceConflict(t, svc.Context, svc, ctrl, b) + t.Run("auth", func(t *testing.T) { + t.Run("400", func(t *testing.T) { + // given + spaceName := uuid.NewV4().String() + p := newCreateSpacePayload(&spaceName, nil) + r := DummyResourceManager{ + httpResponseCode: 400, + } + svc, ctrl := s.SecuredControllerWithDummyResourceManager(testsupport.TestIdentity, r) + // when/then + test.CreateSpaceBadRequest(t, svc.Context, svc, ctrl, p) + }) + + t.Run("returned 401", func(t *testing.T) { + // given + spaceName := uuid.NewV4().String() + p := newCreateSpacePayload(&spaceName, nil) + r := DummyResourceManager{ + httpResponseCode: 401, + } + svc, ctrl := s.SecuredControllerWithDummyResourceManager(testsupport.TestIdentity, r) + // when/then + test.CreateSpaceUnauthorized(t, svc.Context, svc, ctrl, p) + }) + + s.T().Run("500", func(t *testing.T) { + // given + spaceName := uuid.NewV4().String() + p := newCreateSpacePayload(&spaceName, nil) + r := DummyResourceManager{ + httpResponseCode: 500, + } + svc, ctrl := s.SecuredControllerWithDummyResourceManager(testsupport.TestIdentity, r) + // when/then + test.CreateSpaceInternalServerError(t, svc.Context, svc, ctrl, p) + }) + }) }) } diff --git a/design/resources.go b/design/resources.go index 40a215cc08..e40a15f409 100644 --- a/design/resources.go +++ b/design/resources.go @@ -97,6 +97,10 @@ var _ = a.Resource("trackerquery", func() { }) }) +const ( + spaceNamePattern string = `^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$` +) + var nameValidationFunction = func() { a.MaxLength(63) // maximum name length is 63 characters a.MinLength(1) // minimum name length is 1 characters diff --git a/design/spaces.go b/design/spaces.go index f992731e7d..f3f18afada 100644 --- a/design/spaces.go +++ b/design/spaces.go @@ -63,7 +63,7 @@ var spaceOwnedBy = a.Type("SpaceOwnedBy", func() { }) var spaceAttributes = a.Type("SpaceAttributes", func() { - a.Attribute("name", d.String, "Name for the space", nameValidationFunction) + a.Attribute("name", d.String, "Name for the space") // name validation is performed at the controller level, to return a proper JSON-API response a.Attribute("description", d.String, "Description for the space", func() { a.Example("This is the foobar collaboration space") }) diff --git a/errors/errors.go b/errors/errors.go index 7518667df4..b9a8fa299a 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -179,8 +179,8 @@ func (err BadParameterError) Error() string { } // Expected sets the optional expectedValue parameter on the BadParameterError -func (err BadParameterError) Expected(expexcted interface{}) BadParameterError { - err.expectedValue = expexcted +func (err BadParameterError) Expected(expected interface{}) BadParameterError { + err.expectedValue = expected err.hasExpectedValue = true return err } From 83cce7227e5ff35ffb827c37e051480978df307e Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Fri, 25 May 2018 09:27:25 +0200 Subject: [PATCH 2/3] fix tests Signed-off-by: Xavier Coulon --- controller/namedspaces_test.go | 4 ++-- controller/space_blackbox_test.go | 20 +++++++++---------- .../space/show/ok.payload.res.golden.json | 2 +- controller/workitem_blackbox_test.go | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/controller/namedspaces_test.go b/controller/namedspaces_test.go index 1573cd31ac..f9c052b89c 100644 --- a/controller/namedspaces_test.go +++ b/controller/namedspaces_test.go @@ -68,7 +68,7 @@ func (rest *TestNamedSpaceREST) TestSuccessQuerySpace() { assert.Fail(t, "Failed to create an identity") } - name := testsupport.CreateRandomValidTestName("Test 24") + name := testsupport.CreateRandomValidTestName("Test") p := newCreateSpacePayload(&name, nil) @@ -109,7 +109,7 @@ func (rest *TestNamedSpaceREST) TestSuccessListSpaces() { assert.Fail(t, "Failed to create an identity") } - name := testsupport.CreateRandomValidTestName("Test 24") + name := testsupport.CreateRandomValidTestName("Test") p := newCreateSpacePayload(&name, nil) diff --git a/controller/space_blackbox_test.go b/controller/space_blackbox_test.go index 6e3f7926e5..c43a15268f 100644 --- a/controller/space_blackbox_test.go +++ b/controller/space_blackbox_test.go @@ -714,7 +714,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { s.T().Run("ok", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestShowSpaceOK-") + name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceOK" p := newCreateSpacePayload(&name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) @@ -731,7 +731,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { s.T().Run("conditional request", func(t *testing.T) { t.Run("ok with expired modified-since header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestShowSpaceOKUsingExpiredIfModifiedSinceHeader-") + name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceOKUsingExpiredIfModifiedSinceHeader" p := newCreateSpacePayload(&name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) @@ -747,7 +747,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { t.Run("ok with expired if-none-match header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestShowSpaceOKUsingExpiredIfNoneMatchHeader-") + name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceOKUsingExpiredIfNoneMatchHeader" p := newCreateSpacePayload(&name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) @@ -763,7 +763,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { t.Run("not modified with modified-since header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestShowSpaceNotModifiedUsingIfModifiedSinceHeader-") + name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceNotModifiedUsingIfModifiedSinceHeader" p := newCreateSpacePayload(&name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) @@ -775,7 +775,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { t.Run("not modified with if-none-match header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestShowSpaceNotModifiedUsingIfNoneMatchHeader-") + name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceNotModifiedUsingIfNoneMatchHeader" p := newCreateSpacePayload(&name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) @@ -800,7 +800,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { s.T().Run("ok", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestListSpacesOK-") + name := testsupport.CreateRandomValidTestName("Test-") p := newCreateSpacePayload(&name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -822,7 +822,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("ok with expired modified-since header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestListSpacesOKUsingExpiredIfModifiedSinceHeader-") + name := testsupport.CreateRandomValidTestName("Test-") p := newCreateSpacePayload(&name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, createdSpace := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -838,7 +838,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("ok with expired if-none-match header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestListSpacesOKUsingExpiredIfNoneMatchHeader-") + name := testsupport.CreateRandomValidTestName("Test-") p := newCreateSpacePayload(&name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -852,7 +852,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("not modified with modified-since header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestListSpacesNotModifiedUsingIfModifiedSinceHeader-") + name := testsupport.CreateRandomValidTestName("Test-") p := newCreateSpacePayload(&name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, createdSpace := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -863,7 +863,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("not modified with if-none-match header", func(t *testing.T) { // given - name := testsupport.CreateRandomValidTestName("TestListSpacesNotModifiedUsingIfNoneMatchHeader-") + name := testsupport.CreateRandomValidTestName("Test-") p := newCreateSpacePayload(&name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) diff --git a/controller/test-files/space/show/ok.payload.res.golden.json b/controller/test-files/space/show/ok.payload.res.golden.json index 580077f28d..f8123bce7d 100755 --- a/controller/test-files/space/show/ok.payload.res.golden.json +++ b/controller/test-files/space/show/ok.payload.res.golden.json @@ -3,7 +3,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Space for TestShowSpaceOK", - "name": "TestShowSpaceOK-00000000-0000-0000-0000-000000000001", + "name": "Test-00000000-0000-0000-0000-000000000001", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, diff --git a/controller/workitem_blackbox_test.go b/controller/workitem_blackbox_test.go index 7b771a9f54..29974d364f 100644 --- a/controller/workitem_blackbox_test.go +++ b/controller/workitem_blackbox_test.go @@ -114,7 +114,7 @@ func (s *WorkItemSuite) TestPagingLinks() { pagingTest(0, 4, "page[offset]=0&page[limit]=4", "page[offset]=8&page[limit]=4", "", "page[offset]=4&page[limit]=4") // With only ZERO work items - spaceName := "paging zero space " + uuid.NewV4().String() + spaceName := "paging-" + uuid.NewV4().String() sp := &app.CreateSpacePayload{ Data: &app.Space{ Type: "spaces", @@ -2558,7 +2558,7 @@ func (s *WorkItem2Suite) TestCreateWorkItemWithCustomSpace() { spaceTemplateID := spacetemplate.SystemLegacyTemplateID spaceTemplateSelfURL := rest.AbsoluteURL(reqLong, app.SpaceTemplateHref(spaceTemplateID.String())) - spaceName := "My own Space " + uuid.NewV4().String() + spaceName := "My-Space-" + uuid.NewV4().String() sp := &app.CreateSpacePayload{ Data: &app.Space{ Type: "spaces", From 4ef1e950deacdbbf4f306b3a0c03d0be21b6169f Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Fri, 15 Jun 2018 18:27:41 +0200 Subject: [PATCH 3/3] Move space name validation back in the design Also, enforce name requirement in the creatio operation, which requires its own type in the design also, rename the legacy `system.space` to `system-space` to avoid test failures, and replace all ` ` with `-` in other tests as well. Signed-off-by: Xavier Coulon --- controller/comments_blackbox_test.go | 6 +- controller/namedspaces_test.go | 4 +- controller/search_spaces_blackbox_test.go | 14 +-- controller/space.go | 54 +----------- controller/space_blackbox_test.go | 85 +++++++++---------- .../show/ok.res.iteration.golden.json | 2 +- .../namedspaces/show/ok.payload.golden.json | 2 +- .../search_codebase_per_url_multi_match.json | 10 +-- .../search_codebase_per_url_single_match.json | 2 +- .../create/ok.payload.res.golden.json | 2 +- ..._with_force_active.payload.res.golden.json | 2 +- ...generated_template.res.payload.golden.json | 4 +- controller/workitem_blackbox_test.go | 12 +-- design/spaces.go | 46 +++++++++- migration/migration.go | 3 + .../sql-files/093-rename-system-space.sql | 3 + test/testfixture/make_functions.go | 4 +- 17 files changed, 125 insertions(+), 130 deletions(-) create mode 100644 migration/sql-files/093-rename-system-space.sql diff --git a/controller/comments_blackbox_test.go b/controller/comments_blackbox_test.go index 16d8d0056d..962b3c1044 100644 --- a/controller/comments_blackbox_test.go +++ b/controller/comments_blackbox_test.go @@ -592,10 +592,10 @@ func CreateSecuredSpace(t *testing.T, db application.DB, config SpaceConfigurati spaceCtrl := NewSpaceController(svc, db, config, &DummyResourceManager{}) require.NotNil(t, spaceCtrl) spacePayload := &app.CreateSpacePayload{ - Data: &app.Space{ + Data: &app.CreateSpace{ Type: "spaces", - Attributes: &app.SpaceAttributes{ - Name: ptr.String("TestCollaborators-space-" + uuid.NewV4().String()), + Attributes: &app.CreateSpaceAttributes{ + Name: fmt.Sprintf("TestCollaborators-space-%v", uuid.NewV4()), Description: ptr.String("description"), }, }, diff --git a/controller/namedspaces_test.go b/controller/namedspaces_test.go index f9c052b89c..71f90c84b1 100644 --- a/controller/namedspaces_test.go +++ b/controller/namedspaces_test.go @@ -70,7 +70,7 @@ func (rest *TestNamedSpaceREST) TestSuccessQuerySpace() { name := testsupport.CreateRandomValidTestName("Test") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) _, created := test.CreateSpaceCreated(t, spaceSvc.Context, spaceSvc, spaceCtrl, p) assert.NotNil(t, created.Data) @@ -111,7 +111,7 @@ func (rest *TestNamedSpaceREST) TestSuccessListSpaces() { name := testsupport.CreateRandomValidTestName("Test") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) _, created := test.CreateSpaceCreated(t, spaceSvc.Context, spaceSvc, spaceCtrl, p) assert.NotNil(t, created.Data) diff --git a/controller/search_spaces_blackbox_test.go b/controller/search_spaces_blackbox_test.go index 91a53bf640..e3d4ea94b6 100644 --- a/controller/search_spaces_blackbox_test.go +++ b/controller/search_spaces_blackbox_test.go @@ -67,14 +67,14 @@ func (rest *TestSearchSpacesREST) UnSecuredController() (*goa.Service, *SearchCo func (rest *TestSearchSpacesREST) TestSpacesSearchOK() { // given - prefix := time.Now().Format("2006_Jan_2_15_04_05_") // using a unique prefix to make sure the test data will not collide with existing, older spaces. + prefix := time.Now().Format("2006-Jan-2-15-04-05-") // using a unique prefix to make sure the test data will not collide with existing, older spaces. idents, err := createTestData(rest.db, prefix) require.NoError(rest.T(), err) tests := []okScenario{ - {"With uppercase fullname query", args{offset("0"), limit(10), prefix + "TEST_AB"}, expects{totalCount(1)}}, - {"With lowercase fullname query", args{offset("0"), limit(10), prefix + "TEST_AB"}, expects{totalCount(1)}}, - {"With uppercase description query", args{offset("0"), limit(10), "DESCRIPTION FOR " + prefix + "TEST_AB"}, expects{totalCount(1)}}, - {"With lowercase description query", args{offset("0"), limit(10), "description for " + prefix + "test_ab"}, expects{totalCount(1)}}, + {"With uppercase fullname query", args{offset("0"), limit(10), prefix + "TEST-AB"}, expects{totalCount(1)}}, + {"With lowercase fullname query", args{offset("0"), limit(10), prefix + "TEST-AB"}, expects{totalCount(1)}}, + {"With uppercase description query", args{offset("0"), limit(10), "DESCRIPTION FOR " + prefix + "TEST-AB"}, expects{totalCount(1)}}, + {"With lowercase description query", args{offset("0"), limit(10), "description for " + prefix + "TEST-ab"}, expects{totalCount(1)}}, {"with special chars", args{offset("0"), limit(10), "&:\n!#%?*"}, expects{totalCount(0)}}, {"with * to list all", args{offset("0"), limit(10), "*"}, expects{totalCountAtLeast(len(idents))}}, {"with multi page", args{offset("0"), limit(10), prefix + "TEST"}, expects{hasLinks("Next")}}, @@ -92,9 +92,9 @@ func (rest *TestSearchSpacesREST) TestSpacesSearchOK() { } func createTestData(db application.DB, prefix string) ([]space.Space, error) { - names := []string{prefix + "TEST_A", prefix + "TEST_AB", prefix + "TEST_B", prefix + "TEST_C"} + names := []string{prefix + "TEST-A", prefix + "TEST-AB", prefix + "TEST-B", prefix + "TEST-C"} for i := 0; i < 20; i++ { - names = append(names, prefix+"TEST_"+strconv.Itoa(i)) + names = append(names, prefix+"TEST-"+strconv.Itoa(i)) } spaces := []space.Space{} diff --git a/controller/space.go b/controller/space.go index caefef2751..619afb1210 100644 --- a/controller/space.go +++ b/controller/space.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "net/url" - "regexp" "github.com/fabric8-services/fabric8-wit/app" "github.com/fabric8-services/fabric8-wit/application" @@ -74,14 +73,8 @@ func (c *SpaceController) Create(ctx *app.CreateSpaceContext) error { if err != nil { return jsonapi.JSONErrorResponse(ctx, goa.ErrUnauthorized(err.Error())) } - - err = validateCreateSpace(ctx) - if err != nil { - return jsonapi.JSONErrorResponse(ctx, err) - } - reqSpace := ctx.Payload.Data - spaceName := *reqSpace.Attributes.Name + spaceName := reqSpace.Attributes.Name spaceID := uuid.NewV4() if reqSpace.ID != nil { spaceID = *reqSpace.ID @@ -547,51 +540,6 @@ func (c *SpaceController) Update(ctx *app.UpdateSpaceContext) error { return ctx.OK(&response) } -const ( - // see https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/identifiers.md - spaceNameMaxLength int = 63 - spaceNamePattern string = `^([A-Za-z0-9][-A-Za-z0-9]*)?[A-Za-z0-9]$` -) - -var nameRegex *regexp.Regexp - -func init() { - nameRegex = regexp.MustCompile(spaceNamePattern) // will panic if the pattern is invalid -} - -func validateCreateSpace(ctx *app.CreateSpaceContext) error { - if ctx.Payload.Data == nil { - return errors.NewBadParameterError("data", nil).Expected("not nil") - } - if ctx.Payload.Data.Attributes == nil { - return errors.NewBadParameterError("data.attributes", nil).Expected("not nil") - } - if ctx.Payload.Data.Attributes.Name == nil { - return errors.NewBadParameterError("data.attributes.name", nil).Expected("not nil") - } - name := *ctx.Payload.Data.Attributes.Name - // now, verify the length and pattern for the space name - if len(name) > spaceNameMaxLength { - return errors.NewBadParameterError("data.attributes.name", name).Expected(fmt.Sprintf("max length: %d (was %d)", spaceNameMaxLength, len(name))) - } - matched := nameRegex.MatchString(name) - if !matched { - return errors.NewBadParameterError("data.attributes.name", name).Expected(fmt.Sprintf("matching '%s' pattern", spaceNamePattern)) - } - - // // TODO(kwk): Comment back in once space template is official - // if ctx.Payload.Data.Relationships == nil { - // return errors.NewBadParameterError("data.relationships", nil).Expected("not nil") - // } - // if ctx.Payload.Data.Relationships.SpaceTemplate == nil { - // return errors.NewBadParameterError("data.relationships.spacetemplate", nil).Expected("not nil") - // } - // if ctx.Payload.Data.Relationships.SpaceTemplate.Data == nil { - // return errors.NewBadParameterError("data.relationships.spacetemplate.data", nil).Expected("not nil") - // } - return nil -} - func validateUpdateSpace(ctx *app.UpdateSpaceContext) error { if ctx.Payload.Data == nil { return errors.NewBadParameterError("data", nil).Expected("not nil") diff --git a/controller/space_blackbox_test.go b/controller/space_blackbox_test.go index c43a15268f..e0886c4851 100644 --- a/controller/space_blackbox_test.go +++ b/controller/space_blackbox_test.go @@ -161,7 +161,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { t.Run("valid name", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpace-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) // when compareWithGoldenAgnostic(t, filepath.Join(s.testDir, "create", "ok.payload.req.golden.json"), p) @@ -183,7 +183,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { // given fxt := tf.NewTestFixture(t, s.DB, tf.SpaceTemplates(1)) name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpace-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) if p.Data.Relationships == nil { p.Data.Relationships = &app.SpaceRelationships{} @@ -215,7 +215,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { t.Run("with default area", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpaceAndDefaultArea-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) // when _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -238,7 +238,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { // given name := testsupport.CreateRandomValidTestName("TestSuccessCreateSpaceWithDescription-") description := "Space for TestSuccessCreateSpaceWithDescription" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) // when _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -260,11 +260,11 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { name := testsupport.CreateRandomValidTestName("SameName-") description := "Space for TestSuccessCreateSameSpaceNameDifferentOwners" newDescription := "Space for TestSuccessCreateSameSpaceNameDifferentOwners2" - a := newCreateSpacePayload(&name, &description) + a := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, a) // when - b := newCreateSpacePayload(&name, &newDescription) + b := newCreateSpacePayload(name, &newDescription) svc2, ctrl2 := s.SecuredController(testsupport.TestIdentity2) _, created2 := test.CreateSpaceCreated(t, svc2.Context, svc2, ctrl2, b) // then @@ -282,7 +282,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { t.Run("with max length name", func(t *testing.T) { // given name := testsupport.TestMaxsizedNameObj - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) // when _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) @@ -306,7 +306,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { description := "Space for TestSuccessCreateSameSpaceNameDifferentOwners" newDescription := "Space for TestSuccessCreateSameSpaceNameDifferentOwners2" // when - a := newCreateSpacePayload(&name, &description) + a := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, a) // then @@ -316,8 +316,8 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { assert.Equal(t, name, *created.Data.Attributes.Name) // when - b := newCreateSpacePayload(&name, &newDescription) - b.Data.Attributes.Name = &name + b := newCreateSpacePayload(name, &newDescription) + b.Data.Attributes.Name = name b.Data.Attributes.Description = &newDescription test.CreateSpaceConflict(t, svc.Context, svc, ctrl, b) }) @@ -327,10 +327,11 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { // given name := "foo@bar.com" description := "Space with invalid name" - // when/then - p := newCreateSpacePayload(&name, &description) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - test.CreateSpaceBadRequest(t, svc.Context, svc, ctrl, p) + // when + p := newCreateSpacePayload(name, &description) + err := p.Validate() + // then + require.Error(t, err) }) t.Run("invalid length", func(t *testing.T) { @@ -338,9 +339,10 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { name := testsupport.TestOversizedNameObj description := "Space with invalid name" // when/then - p := newCreateSpacePayload(&name, &description) - svc, ctrl := s.SecuredController(testsupport.TestIdentity) - test.CreateSpaceBadRequest(t, svc.Context, svc, ctrl, p) + p := newCreateSpacePayload(name, &description) + err := p.Validate() + // then + require.Error(t, err) }) }) @@ -348,7 +350,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { t.Run("400", func(t *testing.T) { // given spaceName := uuid.NewV4().String() - p := newCreateSpacePayload(&spaceName, nil) + p := newCreateSpacePayload(spaceName, nil) r := DummyResourceManager{ httpResponseCode: 400, } @@ -360,7 +362,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { t.Run("returned 401", func(t *testing.T) { // given spaceName := uuid.NewV4().String() - p := newCreateSpacePayload(&spaceName, nil) + p := newCreateSpacePayload(spaceName, nil) r := DummyResourceManager{ httpResponseCode: 401, } @@ -372,7 +374,7 @@ func (s *SpaceControllerTestSuite) TestCreateSpace() { s.T().Run("500", func(t *testing.T) { // given spaceName := uuid.NewV4().String() - p := newCreateSpacePayload(&spaceName, nil) + p := newCreateSpacePayload(spaceName, nil) r := DummyResourceManager{ httpResponseCode: 500, } @@ -582,7 +584,7 @@ func (s *SpaceControllerTestSuite) TestUpdateSpace() { description := "Space for TestSuccessUpdateSpace" newName := testsupport.CreateRandomValidTestName("TestSuccessUpdateSpace") newDescription := "Space for TestSuccessUpdateSpace2" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) u := newUpdateSpacePayload() @@ -603,7 +605,7 @@ func (s *SpaceControllerTestSuite) TestUpdateSpace() { description := "Space for TestSuccessUpdateSpace" newName := testsupport.CreateRandomValidTestName("TestSuccessUpdateSpace") newDescription := "Space for TestSuccessUpdateSpace2" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) u := newUpdateSpacePayload() @@ -620,14 +622,14 @@ func (s *SpaceControllerTestSuite) TestUpdateSpace() { s.T().Run("fail - name length", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("TestFailUpdateSpaceNameLength-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when / then u := newUpdateSpacePayload() u.Data.ID = created.Data.ID u.Data.Attributes.Version = created.Data.Attributes.Version - p.Data.Attributes.Name = &testsupport.TestOversizedNameObj + p.Data.Attributes.Name = testsupport.TestOversizedNameObj svc2, ctrl2 := s.SecuredController(testsupport.TestIdentity2) test.UpdateSpaceBadRequest(t, svc2.Context, svc2, ctrl2, *created.Data.ID, u) }) @@ -638,9 +640,7 @@ func (s *SpaceControllerTestSuite) TestUpdateSpace() { description := "Space for TestFailUpdateSpaceDifferentOwner" newName := testsupport.CreateRandomValidTestName("TestFailUpdateSpaceDifferentOwner-") newDescription := "Space for TestFailUpdateSpaceDifferentOwner2" - p := newCreateSpacePayload(&name, &description) - p.Data.Attributes.Name = &name - p.Data.Attributes.Description = &description + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -681,8 +681,7 @@ func (s *SpaceControllerTestSuite) TestUpdateSpace() { s.T().Run("fail - missing name", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("TestFailUpdateSpaceMissingName-") - p := newCreateSpacePayload(&name, nil) - p.Data.Attributes.Name = &name + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) u := newUpdateSpacePayload() @@ -696,7 +695,7 @@ func (s *SpaceControllerTestSuite) TestUpdateSpace() { // given name := testsupport.CreateRandomValidTestName("TestFailUpdateSpaceMissingVersion-") newName := testsupport.CreateRandomValidTestName("TestFailUpdateSpaceMissingVersion-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) u := newUpdateSpacePayload() @@ -716,7 +715,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { // given name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceOK" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -733,7 +732,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { // given name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceOKUsingExpiredIfModifiedSinceHeader" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -749,7 +748,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { // given name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceOKUsingExpiredIfNoneMatchHeader" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -765,7 +764,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { // given name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceNotModifiedUsingIfModifiedSinceHeader" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when/then @@ -777,7 +776,7 @@ func (s *SpaceControllerTestSuite) TestShowSpace() { // given name := testsupport.CreateRandomValidTestName("Test-") description := "Space for TestShowSpaceNotModifiedUsingIfNoneMatchHeader" - p := newCreateSpacePayload(&name, &description) + p := newCreateSpacePayload(name, &description) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, created := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when/then @@ -801,7 +800,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { s.T().Run("ok", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("Test-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -823,7 +822,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("ok with expired modified-since header", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("Test-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, createdSpace := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -839,7 +838,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("ok with expired if-none-match header", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("Test-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when @@ -853,7 +852,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("not modified with modified-since header", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("Test-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) _, createdSpace := test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) // when/then @@ -864,7 +863,7 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { t.Run("not modified with if-none-match header", func(t *testing.T) { // given name := testsupport.CreateRandomValidTestName("Test-") - p := newCreateSpacePayload(&name, nil) + p := newCreateSpacePayload(name, nil) svc, ctrl := s.SecuredController(testsupport.TestIdentity) test.CreateSpaceCreated(t, svc.Context, svc, ctrl, p) _, spaceList := test.ListSpaceOK(t, svc.Context, svc, ctrl, nil, nil, nil, nil) @@ -875,14 +874,14 @@ func (s *SpaceControllerTestSuite) TestListSpaces() { }) } -func newCreateSpacePayload(name, description *string) *app.CreateSpacePayload { +func newCreateSpacePayload(name string, description *string) *app.CreateSpacePayload { //spaceTemplateID := spacetemplate.SystemLegacyTemplateID //req := &http.Request{Host: "api.service.domain.org"} // spaceTemplateRelatedURL := rest.AbsoluteURL(req, app.SpaceTemplateHref(spaceTemplateID.String())) return &app.CreateSpacePayload{ - Data: &app.Space{ + Data: &app.CreateSpace{ Type: "spaces", - Attributes: &app.SpaceAttributes{ + Attributes: &app.CreateSpaceAttributes{ Name: name, Description: description, }, diff --git a/controller/test-files/iteration/show/ok.res.iteration.golden.json b/controller/test-files/iteration/show/ok.res.iteration.golden.json index 2188d06970..5eb2b8eafe 100755 --- a/controller/test-files/iteration/show/ok.res.iteration.golden.json +++ b/controller/test-files/iteration/show/ok.res.iteration.golden.json @@ -7,7 +7,7 @@ "endAt": "0001-01-01T00:00:00Z", "name": "iteration 00000000-0000-0000-0000-000000000001", "parent_path": "/00000000-0000-0000-0000-000000000002", - "resolved_parent_path": "/space 00000000-0000-0000-0000-000000000003", + "resolved_parent_path": "/space-00000000-0000-0000-0000-000000000003", "startAt": "0001-01-01T00:00:00Z", "state": "new", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/namedspaces/show/ok.payload.golden.json b/controller/test-files/namedspaces/show/ok.payload.golden.json index baf5d17dff..096fa568b3 100755 --- a/controller/test-files/namedspaces/show/ok.payload.golden.json +++ b/controller/test-files/namedspaces/show/ok.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000001", + "name": "space-00000000-0000-0000-0000-000000000001", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, diff --git a/controller/test-files/search/search_codebase_per_url_multi_match.json b/controller/test-files/search/search_codebase_per_url_multi_match.json index 0049b8ed13..63861e16c3 100644 --- a/controller/test-files/search/search_codebase_per_url_multi_match.json +++ b/controller/test-files/search/search_codebase_per_url_multi_match.json @@ -176,7 +176,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000011", + "name": "space-00000000-0000-0000-0000-000000000011", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, @@ -273,7 +273,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000014", + "name": "space-00000000-0000-0000-0000-000000000014", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, @@ -370,7 +370,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000015", + "name": "space-00000000-0000-0000-0000-000000000015", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, @@ -467,7 +467,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000016", + "name": "space-00000000-0000-0000-0000-000000000016", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, @@ -564,7 +564,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000017", + "name": "space-00000000-0000-0000-0000-000000000017", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, diff --git a/controller/test-files/search/search_codebase_per_url_single_match.json b/controller/test-files/search/search_codebase_per_url_single_match.json index 8ceba21d59..d9c28fcd68 100755 --- a/controller/test-files/search/search_codebase_per_url_single_match.json +++ b/controller/test-files/search/search_codebase_per_url_single_match.json @@ -40,7 +40,7 @@ "attributes": { "created-at": "0001-01-01T00:00:00Z", "description": "Some description", - "name": "space 00000000-0000-0000-0000-000000000003", + "name": "space-00000000-0000-0000-0000-000000000003", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, 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 5756e75b12..035d9ab3c9 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 @@ -6,7 +6,7 @@ "endAt": "0001-01-01T00:00:00Z", "name": "Sprint #42", "parent_path": "/00000000-0000-0000-0000-000000000001", - "resolved_parent_path": "/space 00000000-0000-0000-0000-000000000002", + "resolved_parent_path": "/space-00000000-0000-0000-0000-000000000002", "startAt": "0001-01-01T00:00:00Z", "state": "new", "updated-at": "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 2809891067..9c50e20518 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 @@ -6,7 +6,7 @@ "endAt": "0001-01-01T00:00:00Z", "name": "Sprint #43", "parent_path": "/00000000-0000-0000-0000-000000000001", - "resolved_parent_path": "/space 00000000-0000-0000-0000-000000000002", + "resolved_parent_path": "/space-00000000-0000-0000-0000-000000000002", "startAt": "0001-01-01T00:00:00Z", "state": "new", "updated-at": "0001-01-01T00:00:00Z", diff --git a/controller/test-files/space_templates/show/ok_generated_template.res.payload.golden.json b/controller/test-files/space_templates/show/ok_generated_template.res.payload.golden.json index 5393e04685..a2f0a16c46 100755 --- a/controller/test-files/space_templates/show/ok_generated_template.res.payload.golden.json +++ b/controller/test-files/space_templates/show/ok_generated_template.res.payload.golden.json @@ -3,8 +3,8 @@ "attributes": { "can-construct": true, "created-at": "0001-01-01T00:00:00Z", - "description": "Description for space template 00000000-0000-0000-0000-000000000001", - "name": "space template 00000000-0000-0000-0000-000000000001", + "description": "Description for space-template-00000000-0000-0000-0000-000000000001", + "name": "space-template-00000000-0000-0000-0000-000000000001", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, diff --git a/controller/workitem_blackbox_test.go b/controller/workitem_blackbox_test.go index 29974d364f..e7ca0c4538 100644 --- a/controller/workitem_blackbox_test.go +++ b/controller/workitem_blackbox_test.go @@ -116,10 +116,10 @@ func (s *WorkItemSuite) TestPagingLinks() { // With only ZERO work items spaceName := "paging-" + uuid.NewV4().String() sp := &app.CreateSpacePayload{ - Data: &app.Space{ + Data: &app.CreateSpace{ Type: "spaces", - Attributes: &app.SpaceAttributes{ - Name: &spaceName, + Attributes: &app.CreateSpaceAttributes{ + Name: spaceName, }, }, } @@ -2560,10 +2560,10 @@ func (s *WorkItem2Suite) TestCreateWorkItemWithCustomSpace() { spaceName := "My-Space-" + uuid.NewV4().String() sp := &app.CreateSpacePayload{ - Data: &app.Space{ + Data: &app.CreateSpace{ Type: "spaces", - Attributes: &app.SpaceAttributes{ - Name: &spaceName, + Attributes: &app.CreateSpaceAttributes{ + Name: spaceName, }, Relationships: &app.SpaceRelationships{ SpaceTemplate: app.NewSpaceTemplateRelation(spaceTemplateID, spaceTemplateSelfURL), diff --git a/design/spaces.go b/design/spaces.go index f3f18afada..1a67b080a2 100644 --- a/design/spaces.go +++ b/design/spaces.go @@ -5,6 +5,19 @@ import ( a "github.com/goadesign/goa/design/apidsl" ) +var createSpace = a.Type("createSpace", func() { + a.Attribute("type", d.String, "The type of the related resource", func() { + a.Enum("spaces") + }) + a.Attribute("id", d.UUID, "ID of the space", func() { + a.Example("40bbdd3d-8b5d-4fd6-ac90-7236b669af04") + }) + a.Attribute("attributes", createSpaceAttributes) + a.Attribute("links", genericLinksForSpace) + a.Required("type", "attributes") + a.Attribute("relationships", spaceRelationships) +}) + var space = a.Type("Space", func() { a.Attribute("type", d.String, "The type of the related resource", func() { a.Enum("spaces") @@ -62,8 +75,25 @@ var spaceOwnedBy = a.Type("SpaceOwnedBy", func() { a.Required("data") }) +var createSpaceAttributes = a.Type("createSpaceAttributes", func() { + a.Attribute("name", d.String, "Name for the space", spacenameValidationFunction) + a.Attribute("description", d.String, "Description for the space", func() { + a.Example("This is the foobar collaboration space") + }) + a.Attribute("version", d.Integer, "Version for optimistic concurrency control (optional during creating)", func() { + a.Example(23) + }) + a.Attribute("created-at", d.DateTime, "When the space was created", func() { + a.Example("2016-11-29T23:18:14Z") + }) + a.Attribute("updated-at", d.DateTime, "When the space was updated", func() { + a.Example("2016-11-29T23:18:14Z") + }) + a.Required("name") +}) + var spaceAttributes = a.Type("SpaceAttributes", func() { - a.Attribute("name", d.String, "Name for the space") // name validation is performed at the controller level, to return a proper JSON-API response + a.Attribute("name", d.String, "Name for the space", spacenameValidationFunction) a.Attribute("description", d.String, "Description for the space", func() { a.Example("This is the foobar collaboration space") }) @@ -78,6 +108,13 @@ var spaceAttributes = a.Type("SpaceAttributes", func() { }) }) +var spacenameValidationFunction = func() { + a.MaxLength(63) // maximum name length is 63 characters + a.MinLength(1) // minimum name length is 1 characters + a.Pattern("^([A-Za-z0-9][-A-Za-z0-9]*)?[A-Za-z0-9]$") + a.Example("Space-1234") +} + var spaceListMeta = a.Type("SpaceListMeta", func() { a.Attribute("totalCount", d.Integer) a.Required("totalCount") @@ -89,6 +126,11 @@ var spaceList = JSONList( pagingLinks, spaceListMeta) +var createSpaceSingle = JSONSingle( + "CreateSpace", "Holds the request to create a space", + createSpace, + nil) + var spaceSingle = JSONSingle( "Space", "Holds a single response to a space request", space, @@ -155,7 +197,7 @@ var _ = a.Resource("space", func() { a.POST(""), ) a.Description("Create a space") - a.Payload(spaceSingle) + a.Payload(createSpaceSingle) a.Response(d.Created, "/spaces/.*", func() { a.Media(spaceSingle) }) diff --git a/migration/migration.go b/migration/migration.go index 86b5df1c09..f0bea647c7 100644 --- a/migration/migration.go +++ b/migration/migration.go @@ -416,6 +416,9 @@ func GetMigrations() Migrations { // Version 92 m = append(m, steps{ExecuteSQLFile("092-comment-revisions-child-comments.sql")}) + // Version 93 + m = append(m, steps{ExecuteSQLFile("093-rename-system-space.sql")}) + // Version N // // In order to add an upgrade, simply append an array of MigrationFunc to the diff --git a/migration/sql-files/093-rename-system-space.sql b/migration/sql-files/093-rename-system-space.sql new file mode 100644 index 0000000000..465c732ede --- /dev/null +++ b/migration/sql-files/093-rename-system-space.sql @@ -0,0 +1,3 @@ +-- rename the `system.space` to `system-space` to comply with the system naming pattern which +-- does not allow the `.` character +update spaces set name = 'system-space' where id = '2e0698d8-753e-4cef-bb7c-f027634824a2'; \ No newline at end of file diff --git a/test/testfixture/make_functions.go b/test/testfixture/make_functions.go index 0822dfbc8c..9cf9af0126 100644 --- a/test/testfixture/make_functions.go +++ b/test/testfixture/make_functions.go @@ -77,7 +77,7 @@ func makeSpaceTemplates(fxt *TestFixture) error { spaceTemplateRepo := spacetemplate.NewRepository(fxt.db) for i := range fxt.SpaceTemplates { fxt.SpaceTemplates[i] = &spacetemplate.SpaceTemplate{ - Name: testsupport.CreateRandomValidTestName("space template "), + Name: testsupport.CreateRandomValidTestName("space-template-"), CanConstruct: true, } fxt.SpaceTemplates[i].Description = ptr.String("Description for " + fxt.SpaceTemplates[i].Name) @@ -102,7 +102,7 @@ func makeSpaces(fxt *TestFixture) error { spaceRepo := space.NewRepository(fxt.db) for i := range fxt.Spaces { fxt.Spaces[i] = &space.Space{ - Name: testsupport.CreateRandomValidTestName("space "), + Name: testsupport.CreateRandomValidTestName("space-"), Description: "Some description", } if !fxt.isolatedCreation {