From 857cd0e81d369ead10af53aaecb668194b2b11aa Mon Sep 17 00:00:00 2001 From: nttdata-rtorsoli Date: Thu, 2 May 2024 11:20:03 +0200 Subject: [PATCH 1/2] PIN-4845 purpose-process: Verify if exists the check on duplicate title on update --- .../api/impl/PurposeApiServiceImpl.scala | 26 ++- .../api/impl/ResponseHandlers.scala | 8 +- .../PurposeApiServiceSpec.scala | 178 +++++++++++++++++- 3 files changed, 191 insertions(+), 21 deletions(-) diff --git a/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/PurposeApiServiceImpl.scala b/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/PurposeApiServiceImpl.scala index 2f33cd8b..a59751ca 100644 --- a/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/PurposeApiServiceImpl.scala +++ b/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/PurposeApiServiceImpl.scala @@ -103,10 +103,15 @@ final case class PurposeApiServiceImpl( tenantKind <- tenant.kind.toFuture(TenantKindNotFound(tenant.id)) } yield tenantKind - private def checkAgreements(eServiceId: UUID, consumerId: UUID, title: String): Future[Unit] = for { - agreements <- agreementManagementService.getAgreements(eServiceId, consumerId, OPERATIVE_AGREEMENT_STATES) - agreement <- agreements.headOption.toFuture(AgreementNotFound(eServiceId.toString, consumerId.toString)) - maybePurpose <- purposeManagementService + private def checkAgreements( + eServiceId: UUID, + consumerId: UUID, + title: String, + purposeId: Option[UUID] + ): Future[Unit] = for { + agreements <- agreementManagementService.getAgreements(eServiceId, consumerId, OPERATIVE_AGREEMENT_STATES) + agreement <- agreements.headOption.toFuture(AgreementNotFound(eServiceId.toString, consumerId.toString)) + result <- purposeManagementService .listPurposes( consumerId, title.some, @@ -119,9 +124,8 @@ final case class PurposeApiServiceImpl( limit = 1, exactMatchOnTitle = true ) - .map(_.results.headOption) - - _ <- maybePurpose.fold(Future.unit)(_ => Future.failed(DuplicatedPurposeName(title))) + purpose = purposeId.fold(result.results)(id => result.results.filterNot(_.id == id)) + _ <- purpose.headOption.map(_.title).fold(Future.unit)(_ => Future.failed(DuplicatedPurposeName(title))) } yield () override def createPurposeFromEService(seed: EServicePurposeSeed)(implicit @@ -143,7 +147,7 @@ final case class PurposeApiServiceImpl( _ <- checkFreeOfCharge(seed.isFreeOfCharge, seed.freeOfChargeReason) tenantKind <- getTenantKind(eService.producerId) purposeSeed = seed.toManagement(seed.eServiceId, riskAnalysis.riskAnalysisForm.toManagement(seed.riskAnalysisId)) - _ <- checkAgreements(seed.eServiceId, seed.consumerId, seed.title) + _ <- checkAgreements(seed.eServiceId, seed.consumerId, seed.title, None) purpose <- purposeManagementService.createPurpose(purposeSeed) isValidRiskAnalysisForm = isRiskAnalysisFormValid( riskAnalysisForm = purpose.riskAnalysisForm.map(_.toApi), @@ -168,7 +172,7 @@ final case class PurposeApiServiceImpl( _ <- checkFreeOfCharge(seed.isFreeOfCharge, seed.freeOfChargeReason) tenantKind <- getTenantKind(requesterOrgId) purposeSeed <- seed.toManagement(schemaOnlyValidation = true)(tenantKind).toFuture - _ <- checkAgreements(seed.eserviceId, seed.consumerId, seed.title) + _ <- checkAgreements(seed.eserviceId, seed.consumerId, seed.title, None) purpose <- purposeManagementService.createPurpose(purposeSeed) isValidRiskAnalysisForm = isRiskAnalysisFormValid( riskAnalysisForm = purpose.riskAnalysisForm.map(_.toApi), @@ -231,6 +235,7 @@ final case class PurposeApiServiceImpl( eService => if (eService.mode == Deliver) Future.unit else Future.failed(EServiceNotInDeliverMode(eService.id)), seed.isFreeOfCharge, seed.freeOfChargeReason, + seed.title, (_, tenantKind) => seed .toManagement(schemaOnlyValidation = true)(tenantKind) @@ -253,6 +258,7 @@ final case class PurposeApiServiceImpl( eService => if (eService.mode == Receive) Future.unit else Future.failed(EServiceNotInReceiveMode(eService.id)), seed.isFreeOfCharge, seed.freeOfChargeReason, + seed.title, (purpose, tenantKind) => seed .toManagement(schemaOnlyValidation = true, purpose.riskAnalysisForm.map(_.toApi))(tenantKind) @@ -267,11 +273,13 @@ final case class PurposeApiServiceImpl( eServiceModeCheck: CatalogItem => Future[Unit], isFreeOfCharge: Boolean, freeOfChargeReason: Option[String], + title: String, payload: (PersistentPurpose, PersistentTenantKind) => Future[PurposeManagementDependency.PurposeUpdateContent] )(implicit contexts: Seq[(String, String)]): Future[Purpose] = for { requesterOrgId <- getOrganizationIdFutureUUID(contexts) purposeUUID <- purposeId.toFutureUUID purpose <- purposeManagementService.getPurposeById(purposeUUID) + _ <- checkAgreements(purpose.eserviceId, purpose.consumerId, title, Some(purposeUUID)) _ <- assertOrganizationIsAConsumer(requesterOrgId, purpose.consumerId) _ <- assertPurposeIsInDraftState(purpose) eService <- catalogManagementService.getEServiceById(purpose.eserviceId) diff --git a/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/ResponseHandlers.scala b/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/ResponseHandlers.scala index 4bf34db6..93dc88c7 100644 --- a/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/ResponseHandlers.scala +++ b/src/main/scala/it/pagopa/interop/purposeprocess/api/impl/ResponseHandlers.scala @@ -96,8 +96,8 @@ object ResponseHandlers extends AkkaResponses { case Failure(ex: OrganizationIsNotTheConsumer) => forbidden(ex, logMessage) case Failure(ex: PurposeNotInDraftState) => forbidden(ex, logMessage) case Failure(ex: PurposeNotFound) => notFound(ex, logMessage) - - case Failure(ex) => internalServerError(ex, logMessage) + case Failure(ex: DuplicatedPurposeName) => conflict(ex, logMessage) + case Failure(ex) => internalServerError(ex, logMessage) } def updateReversePurposeResponse[T](logMessage: String)( @@ -114,8 +114,8 @@ object ResponseHandlers extends AkkaResponses { case Failure(ex: OrganizationIsNotTheConsumer) => forbidden(ex, logMessage) case Failure(ex: PurposeNotInDraftState) => forbidden(ex, logMessage) case Failure(ex: PurposeNotFound) => notFound(ex, logMessage) - - case Failure(ex) => internalServerError(ex, logMessage) + case Failure(ex: DuplicatedPurposeName) => conflict(ex, logMessage) + case Failure(ex) => internalServerError(ex, logMessage) } def getPurposeResponse[T](logMessage: String)( diff --git a/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala b/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala index f156663b..da674e5d 100644 --- a/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala +++ b/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala @@ -1421,13 +1421,22 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate ) val purpose = - SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersion)) + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) implicit val context: Seq[(String, String)] = Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) + mockEServiceRetrieve(eserviceId, SpecData.eService.copy(id = eserviceId)) mockTenantRetrieve(consumerId, SpecData.tenant.copy(id = consumerId, kind = PersistentTenantKind.PRIVATE.some)) @@ -1438,6 +1447,47 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate status shouldEqual StatusCodes.OK } } + "fail if there is other purposes with the same title" in { + + val purposeId = UUID.randomUUID() + val eserviceId = UUID.randomUUID() + val consumerId = UUID.randomUUID() + val purposeUpdateContent = + PurposeUpdateContent( + title = "A title", + description = "A description", + isFreeOfCharge = false, + riskAnalysisForm = None, + dailyCalls = 100 + ) + + val purpose = + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) + + implicit val context: Seq[(String, String)] = + Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) + + mockPurposeRetrieve(purposeId, purpose) + + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + val other = SpecData.purpose.copy( + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion), + title = "A title" + ) + mockListPurposesRetrieve(Seq(purpose, other)) + + Post() ~> service.updatePurpose(purposeId.toString, purposeUpdateContent) ~> check { + status shouldEqual StatusCodes.Conflict + } + } "fail if case of eService with Receive mode" in { val purposeId = UUID.randomUUID() @@ -1456,11 +1506,19 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) val purpose = - SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersion)) + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) mockEServiceRetrieve(eserviceId, SpecData.eService.copy(id = eserviceId, mode = Receive)) mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) Post() ~> service.updatePurpose(purposeId.toString, purposeUpdateContent) ~> check { status shouldEqual StatusCodes.BadRequest val problem = responseAs[Problem] @@ -1485,11 +1543,19 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) val purpose = - SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersion)) + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) mockPurposeRetrieve(purposeId, purpose) mockEServiceRetrieve(eserviceId, SpecData.eService.copy(id = eserviceId)) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + mockListPurposesRetrieve(Seq(purpose)) + Post() ~> service.updatePurpose(purposeId.toString, purposeUpdateContent) ~> check { status shouldEqual StatusCodes.BadRequest val problem = responseAs[Problem] @@ -1516,8 +1582,19 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate implicit val context: Seq[(String, String)] = Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> requesterId.toString) - mockPurposeRetrieve(purposeId, SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId)) + val purpose = + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) + + mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) Post() ~> service.updatePurpose(purposeId.toString, purposeUpdateContent) ~> check { status shouldEqual StatusCodes.Forbidden val problem = responseAs[Problem] @@ -1540,6 +1617,7 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate ) val purpose = SpecData.purpose.copy( + id = purposeId, eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersionNotInDraftState) @@ -1550,6 +1628,9 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) Post() ~> service.updatePurpose(purposeId.toString, purposeUpdateContent) ~> check { status shouldEqual StatusCodes.Forbidden val problem = responseAs[Problem] @@ -1575,7 +1656,12 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate ) val purpose = - SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersion)) + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) val seed = PurposeManagementDependency.PurposeUpdateContent( title = "A title", @@ -1598,6 +1684,9 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) mockEServiceRetrieve(eserviceId, SpecData.eService.copy(id = eserviceId, mode = Receive, producerId = producerId)) mockTenantRetrieve(producerId, SpecData.tenant.copy(id = producerId, kind = PersistentTenantKind.PRIVATE.some)) @@ -1608,6 +1697,46 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate status shouldEqual StatusCodes.OK } } + "fail if there is other purposes with the same title" in { + val purposeId = UUID.randomUUID() + val eserviceId = UUID.randomUUID() + val consumerId = UUID.randomUUID() + + val reversePurposeUpdateContent = + ReversePurposeUpdateContent( + title = "A title", + description = "A description", + isFreeOfCharge = false, + dailyCalls = 100 + ) + + val purpose = + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) + + implicit val context: Seq[(String, String)] = + Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) + + mockPurposeRetrieve(purposeId, purpose) + + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + val other = SpecData.purpose.copy( + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion), + title = "A title" + ) + mockListPurposesRetrieve(Seq(purpose, other)) + + Post() ~> service.updateReversePurpose(purposeId.toString, reversePurposeUpdateContent) ~> check { + status shouldEqual StatusCodes.Conflict + } + } "fail if case of eService with Deliver mode" in { val purposeId = UUID.randomUUID() @@ -1625,11 +1754,19 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) val purpose = - SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersion)) + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) mockEServiceRetrieve(eserviceId, SpecData.eService.copy(id = eserviceId, mode = Deliver)) mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) Post() ~> service.updateReversePurpose(purposeId.toString, reversePurposeUpdateContent) ~> check { status shouldEqual StatusCodes.BadRequest val problem = responseAs[Problem] @@ -1653,9 +1790,18 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> consumerId.toString) val purpose = - SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersion)) + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) mockPurposeRetrieve(purposeId, purpose) + + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) mockEServiceRetrieve(eserviceId, SpecData.eService.copy(id = eserviceId, mode = Receive)) Post() ~> service.updateReversePurpose(purposeId.toString, reversePurposeUpdateContent) ~> check { @@ -1683,7 +1829,19 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate implicit val context: Seq[(String, String)] = Seq("bearer" -> bearerToken, USER_ROLES -> "admin", ORGANIZATION_ID_CLAIM -> requesterId.toString) - mockPurposeRetrieve(purposeId, SpecData.purpose.copy(eserviceId = eserviceId, consumerId = consumerId)) + val purpose = + SpecData.purpose.copy( + id = purposeId, + eserviceId = eserviceId, + consumerId = consumerId, + versions = Seq(SpecData.purposeVersion) + ) + + mockPurposeRetrieve(purposeId, purpose) + + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) Post() ~> service.updateReversePurpose(purposeId.toString, reversePurposeUpdateContent) ~> check { status shouldEqual StatusCodes.Forbidden @@ -1706,6 +1864,7 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate ) val purpose = SpecData.purpose.copy( + id = purposeId, eserviceId = eserviceId, consumerId = consumerId, versions = Seq(SpecData.purposeVersionNotInDraftState) @@ -1716,6 +1875,9 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate mockPurposeRetrieve(purposeId, purpose) + mockAgreementsRetrieve(eserviceId, consumerId, Seq(AgreementActive)) + + mockListPurposesRetrieve(Seq(purpose)) Post() ~> service.updateReversePurpose(purposeId.toString, reversePurposeUpdateContent) ~> check { status shouldEqual StatusCodes.Forbidden val problem = responseAs[Problem] From 777f6e6561b497a850a6ba7eb68a6b63e1fc3a86 Mon Sep 17 00:00:00 2001 From: nttdata-rtorsoli Date: Thu, 23 May 2024 18:15:04 +0200 Subject: [PATCH 2/2] PIN-4845 Resolved PR issues --- .../pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala b/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala index f6bcf2ee..25e728d5 100644 --- a/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala +++ b/src/test/scala/it/pagopa/interop/purposeprocess/PurposeApiServiceSpec.scala @@ -1447,7 +1447,7 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate status shouldEqual StatusCodes.OK } } - "fail if there is other purposes with the same title" in { + "fail if there is another purpose with the same title" in { val purposeId = UUID.randomUUID() val eserviceId = UUID.randomUUID() @@ -1697,7 +1697,7 @@ class PurposeApiServiceSpec extends AnyWordSpecLike with SpecHelper with Scalate status shouldEqual StatusCodes.OK } } - "fail if there is other purposes with the same title" in { + "fail if there is another purpose with the same title" in { val purposeId = UUID.randomUUID() val eserviceId = UUID.randomUUID() val consumerId = UUID.randomUUID()