From f161ec29679ed94b692a6f8d63ab8a111e5df2e7 Mon Sep 17 00:00:00 2001 From: Konstantin Lapkov Date: Tue, 10 Dec 2024 09:16:24 +0200 Subject: [PATCH] Address comments --- api/handlers/service_plan_test.go | 15 --------- .../service_plan_repository_test.go | 25 ++++++--------- tests/e2e/e2e_suite_test.go | 4 ++- tests/e2e/service_plans_test.go | 31 +++++++++++++++++++ 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index ffb93e0ec..4d9eccb26 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -5,7 +5,6 @@ import ( "net/http" "strings" - apierrors "code.cloudfoundry.org/korifi/api/errors" . "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" "code.cloudfoundry.org/korifi/api/payloads" @@ -353,10 +352,6 @@ var _ = Describe("ServicePlan", func() { }) Describe("DELETE /v3/service_plans/{guid}/visibility/{org-guid}", func() { - BeforeEach(func() { - servicePlanRepo.DeletePlanVisibilityReturns(nil) - }) - JustBeforeEach(func() { req, err := http.NewRequestWithContext(ctx, "DELETE", "/v3/service_plans/my-service-plan/visibility/org-guid", nil) Expect(err).NotTo(HaveOccurred()) @@ -373,16 +368,6 @@ var _ = Describe("ServicePlan", func() { Expect(rr).To(HaveHTTPStatus(http.StatusNoContent)) }) - When("deleting the visibility fails with not found", func() { - BeforeEach(func() { - servicePlanRepo.DeletePlanVisibilityReturns(apierrors.NewNotFoundError(nil, repositories.ServicePlanVisibilityResourceType)) - }) - - It("returns 404 Not Found", func() { - expectNotFoundError("Service Plan Visibility") - }) - }) - When("deleting the visibility fails with an error", func() { BeforeEach(func() { servicePlanRepo.DeletePlanVisibilityReturns(errors.New("visibility-err")) diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index dec3ba4af..6dc8bb316 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -621,14 +621,14 @@ var _ = Describe("ServicePlanRepo", func() { })).To(Succeed()) }) - When("the user is not authorized", func() { - BeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ - PlanGUID: planGUID, - OrgGUID: anotherOrg.Name, - }) + JustBeforeEach(func() { + visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ + PlanGUID: planGUID, + OrgGUID: anotherOrg.Name, }) + }) + When("the user is not authorized", func() { It("returns unauthorized error", func() { Expect(visibilityErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) }) @@ -640,13 +640,6 @@ var _ = Describe("ServicePlanRepo", func() { }) When("the plan and org visibility exist", func() { - BeforeEach(func() { - visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ - PlanGUID: planGUID, - OrgGUID: anotherOrg.Name, - }) - }) - It("deletes the plan visibility in kubernetes", func() { Expect(visibilityErr).ToNot(HaveOccurred()) @@ -657,7 +650,7 @@ var _ = Describe("ServicePlanRepo", func() { }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) - Expect(servicePlan.Spec.Visibility.Organizations).To(Equal([]string{cfOrg.Name})) + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) }) }) @@ -675,7 +668,7 @@ var _ = Describe("ServicePlanRepo", func() { }) When("the org does not exist", func() { - BeforeEach(func() { + JustBeforeEach(func() { visibilityErr = repo.DeletePlanVisibility(ctx, authInfo, repositories.DeleteServicePlanVisibilityMessage{ PlanGUID: planGUID, OrgGUID: "does-not-exist", @@ -692,7 +685,7 @@ var _ = Describe("ServicePlanRepo", func() { }, } Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)).To(Succeed()) - Expect(servicePlan.Spec.Visibility.Organizations).To(Equal([]string{cfOrg.Name, anotherOrg.Name})) + Expect(servicePlan.Spec.Visibility.Organizations).To(ConsistOf(cfOrg.Name)) }) }) }) diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index cd23ac3c3..65465e543 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "code.cloudfoundry.org/korifi/model/services" "code.cloudfoundry.org/korifi/tests/helpers" "code.cloudfoundry.org/korifi/tests/helpers/fail_handler" @@ -293,7 +294,8 @@ type cfErr struct { } type planVisibilityResource struct { - Type string `json:"type"` + Type string `json:"type"` + Organizations []services.VisibilityOrganization `json:"organizations"` } func TestE2E(t *testing.T) { diff --git a/tests/e2e/service_plans_test.go b/tests/e2e/service_plans_test.go index 9ff2d064e..45e282aca 100644 --- a/tests/e2e/service_plans_test.go +++ b/tests/e2e/service_plans_test.go @@ -5,6 +5,7 @@ import ( "net/http" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model/services" "code.cloudfoundry.org/korifi/tests/helpers/broker" "github.com/BooleanCat/go-functional/v2/it/itx" "github.com/go-resty/resty/v2" @@ -121,6 +122,36 @@ var _ = Describe("Service Plans", func() { )) }) }) + + Describe("Delete Visibility", func() { + JustBeforeEach(func() { + var err error + + resp, err = adminClient.R(). + SetBody(planVisibilityResource{ + Type: "organization", + Organizations: []services.VisibilityOrganization{{ + GUID: "org-guid", + }}, + }). + Post(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID)) + Expect(err).NotTo(HaveOccurred()) + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusOK), + )) + + resp, err = adminClient.R(). + SetResult(&result). + Delete(fmt.Sprintf("/v3/service_plans/%s/visibility/%s", planGUID, "org-guid")) + Expect(err).NotTo(HaveOccurred()) + }) + + It("deletes the plan visibility", func() { + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusNoContent), + )) + }) + }) }) Describe("Delete", func() {