From ffcd6af8c9d76347d543cab3f9466d86b08c0720 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Tue, 27 Jun 2023 09:59:09 +0100 Subject: [PATCH] Use pointers for relationships E.g. AppCreate payload has relationships.space.data.guid. If we use pointers all the way down, we get the correct error message if the json only contains some of the path. So providing `relationships.space` would error with `data` being required. Issue: https://github.com/cloudfoundry/korifi/issues/1996 Co-authored-by: Kieron Browne --- api/handlers/app_test.go | 4 ++-- api/payloads/app.go | 8 ++++---- api/payloads/app_test.go | 18 ++++++++++++++---- api/payloads/deployment.go | 6 +++--- api/payloads/deployment_test.go | 22 ++++++++++++++++------ api/payloads/lifecycle.go | 16 +++++++--------- api/payloads/lifecycle_test.go | 14 +++++++------- api/payloads/route.go | 4 ++-- 8 files changed, 55 insertions(+), 37 deletions(-) diff --git a/api/handlers/app_test.go b/api/handlers/app_test.go index 93efe6ec1..fa5d0b28d 100644 --- a/api/handlers/app_test.go +++ b/api/handlers/app_test.go @@ -140,8 +140,8 @@ var _ = Describe("App", func() { BeforeEach(func() { payload = &payloads.AppCreate{ Name: appName, - Relationships: payloads.AppRelationships{ - Space: payloads.Relationship{ + Relationships: &payloads.AppRelationships{ + Space: &payloads.Relationship{ Data: &payloads.RelationshipData{ GUID: spaceGUID, }, diff --git a/api/payloads/app.go b/api/payloads/app.go index a7f276c80..7253b8fa1 100644 --- a/api/payloads/app.go +++ b/api/payloads/app.go @@ -22,7 +22,7 @@ var DefaultLifecycleConfig = config.DefaultLifecycleConfig{ type AppCreate struct { Name string `json:"name"` EnvironmentVariables map[string]string `json:"environment_variables"` - Relationships AppRelationships `json:"relationships"` + Relationships *AppRelationships `json:"relationships"` Lifecycle *Lifecycle `json:"lifecycle"` Metadata Metadata `json:"metadata"` } @@ -30,19 +30,19 @@ type AppCreate struct { func (c AppCreate) Validate() error { return validation.ValidateStruct(&c, validation.Field(&c.Name, payload_validation.StrictlyRequired), - validation.Field(&c.Relationships, payload_validation.StrictlyRequired), + validation.Field(&c.Relationships, validation.NotNil), validation.Field(&c.Lifecycle), validation.Field(&c.Metadata), ) } type AppRelationships struct { - Space Relationship `json:"space"` + Space *Relationship `json:"space"` } func (r AppRelationships) Validate() error { return validation.ValidateStruct(&r, - validation.Field(&r.Space, payload_validation.StrictlyRequired), + validation.Field(&r.Space, validation.NotNil), ) } diff --git a/api/payloads/app_test.go b/api/payloads/app_test.go index e4c619ab5..e446231e1 100644 --- a/api/payloads/app_test.go +++ b/api/payloads/app_test.go @@ -40,8 +40,8 @@ var _ = Describe("App payload validation", func() { BeforeEach(func() { payload = payloads.AppCreate{ Name: "my-app", - Relationships: payloads.AppRelationships{ - Space: payloads.Relationship{ + Relationships: &payloads.AppRelationships{ + Space: &payloads.Relationship{ Data: &payloads.RelationshipData{ GUID: "app-guid", }, @@ -83,11 +83,21 @@ var _ = Describe("App payload validation", func() { When("relationships are not set", func() { BeforeEach(func() { - payload.Relationships = payloads.AppRelationships{} + payload.Relationships = nil }) It("returns an unprocessable entity error", func() { - expectUnprocessableEntityError(validatorErr, "relationships cannot be blank") + expectUnprocessableEntityError(validatorErr, "relationships is required") + }) + }) + + When("relationships space is not set", func() { + BeforeEach(func() { + payload.Relationships.Space = nil + }) + + It("returns an unprocessable entity error", func() { + expectUnprocessableEntityError(validatorErr, "relationships.space is required") }) }) diff --git a/api/payloads/deployment.go b/api/payloads/deployment.go index 61264bf72..1aaa57c09 100644 --- a/api/payloads/deployment.go +++ b/api/payloads/deployment.go @@ -9,12 +9,12 @@ type DropletGUID struct { } type DeploymentCreate struct { - Droplet DropletGUID `json:"droplet"` - Relationships DeploymentRelationships `json:"relationships" validate:"required"` + Droplet DropletGUID `json:"droplet"` + Relationships *DeploymentRelationships `json:"relationships" validate:"required"` } type DeploymentRelationships struct { - App Relationship `json:"app" validate:"required"` + App *Relationship `json:"app" validate:"required"` } func (c *DeploymentCreate) ToMessage() repositories.CreateDeploymentMessage { diff --git a/api/payloads/deployment_test.go b/api/payloads/deployment_test.go index d6a1a1cc4..300cd2146 100644 --- a/api/payloads/deployment_test.go +++ b/api/payloads/deployment_test.go @@ -17,8 +17,8 @@ var _ = Describe("DeploymentCreate", func() { Droplet: payloads.DropletGUID{ Guid: "the-droplet", }, - Relationships: payloads.DeploymentRelationships{ - App: payloads.Relationship{ + Relationships: &payloads.DeploymentRelationships{ + App: &payloads.Relationship{ Data: &payloads.RelationshipData{ GUID: "the-app", }, @@ -57,13 +57,23 @@ var _ = Describe("DeploymentCreate", func() { }) }) - When("the app relationship is not specified", func() { + When("the relationship is not specified", func() { BeforeEach(func() { - createDeployment.Relationships = payloads.DeploymentRelationships{} + createDeployment.Relationships = nil }) - It("says app data is required", func() { - expectUnprocessableEntityError(validatorErr, "Data is a required field") + It("says relationships is required", func() { + expectUnprocessableEntityError(validatorErr, "Relationships is a required field") + }) + }) + + When("the relationship app is not specified", func() { + BeforeEach(func() { + createDeployment.Relationships.App = nil + }) + + It("says app is required", func() { + expectUnprocessableEntityError(validatorErr, "App is a required field") }) }) diff --git a/api/payloads/lifecycle.go b/api/payloads/lifecycle.go index b936d622c..4402f363b 100644 --- a/api/payloads/lifecycle.go +++ b/api/payloads/lifecycle.go @@ -1,30 +1,28 @@ package payloads import ( - payload_validation "code.cloudfoundry.org/korifi/api/payloads/validation" "github.com/jellydator/validation" ) type Lifecycle struct { - Type string `json:"type" validate:"required"` - Data LifecycleData `json:"data" validate:"required"` + Type string `json:"type"` + Data *LifecycleData `json:"data"` } func (l Lifecycle) Validate() error { return validation.ValidateStruct(&l, - validation.Field(&l.Type, payload_validation.StrictlyRequired), - validation.Field(&l.Data, payload_validation.StrictlyRequired), + validation.Field(&l.Type, validation.Required), + validation.Field(&l.Data, validation.NotNil), ) } type LifecycleData struct { - Buildpacks []string `json:"buildpacks" validate:"required"` - Stack string `json:"stack" validate:"required"` + Buildpacks []string `json:"buildpacks"` + Stack string `json:"stack"` } func (d LifecycleData) Validate() error { return validation.ValidateStruct(&d, - validation.Field(&d.Buildpacks, payload_validation.StrictlyRequired), - validation.Field(&d.Stack, payload_validation.StrictlyRequired), + validation.Field(&d.Stack, validation.Required), ) } diff --git a/api/payloads/lifecycle_test.go b/api/payloads/lifecycle_test.go index 400449b50..bd967aa44 100644 --- a/api/payloads/lifecycle_test.go +++ b/api/payloads/lifecycle_test.go @@ -22,7 +22,7 @@ var _ = Describe("Lifecycle", func() { payload = payloads.Lifecycle{ Type: "buildpack", - Data: payloads.LifecycleData{ + Data: &payloads.LifecycleData{ Buildpacks: []string{"foo", "bar"}, Stack: "baz", }, @@ -50,23 +50,23 @@ var _ = Describe("Lifecycle", func() { }) }) - When("stack is not set", func() { + When("lifecycle data is not set", func() { BeforeEach(func() { - payload.Data.Stack = "" + payload.Data = nil }) It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data.stack cannot be blank") + expectUnprocessableEntityError(validatorErr, "data is required") }) }) - When("buildpacks are not set", func() { + When("stack is not set", func() { BeforeEach(func() { - payload.Data.Buildpacks = nil + payload.Data.Stack = "" }) It("returns an appropriate error", func() { - expectUnprocessableEntityError(validatorErr, "data.buildpacks cannot be blank") + expectUnprocessableEntityError(validatorErr, "data.stack cannot be blank") }) }) }) diff --git a/api/payloads/route.go b/api/payloads/route.go index c2fb2134a..a9c2394a9 100644 --- a/api/payloads/route.go +++ b/api/payloads/route.go @@ -39,8 +39,8 @@ func (p RouteCreate) ToMessage(domainNamespace, domainName string) repositories. } type RouteRelationships struct { - Domain Relationship `json:"domain" validate:"required"` - Space Relationship `json:"space" validate:"required"` + Domain Relationship `json:"domain"` + Space Relationship `json:"space"` } func (r RouteRelationships) Validate() error {