diff --git a/app/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationController.scala b/app/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationController.scala index 34f1e9587..73e8025ca 100644 --- a/app/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationController.scala +++ b/app/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationController.scala @@ -234,8 +234,22 @@ class ApplicationController @Inject() ( .isDefined } + private def asNotFoundOrJson[T](notFoundMessage: String): Option[JsValue] => Result = _.fold(handleNotFound(notFoundMessage))(v => Ok(v)) + private def asJsonResult[T](notFoundMessage: String)(maybeApplication: Option[T])(implicit writes: Writes[T]): Result = { - maybeApplication.fold(handleNotFound(notFoundMessage))(t => Ok(toJson(t))) + val js = maybeApplication.map(Json.toJson[T]) + asNotFoundOrJson[T](notFoundMessage)(js) + } + + private def asJsonResultWithServerToken[T](notFoundMessage: String)(maybeApplication: Option[(T, String)])(implicit writes: Writes[T]): Result = { + val js: Option[JsValue] = maybeApplication.map { + case (app, serverToken) => + Json.toJson[T](app) match { + case o: JsObject => o + ("serverToken" -> JsString(serverToken)) + case v => v + } + } + asNotFoundOrJson[T](notFoundMessage)(js) } private def fetchByServerToken(serverToken: String)(implicit hc: HeaderCarrier): Future[Result] = { @@ -244,7 +258,7 @@ class ApplicationController @Inject() ( // If request has originated from an API gateway, record usage of the Application ( if (hasGatewayUserAgent) { - applicationService.findAndRecordServerTokenUsage(serverToken).map(asJsonResult(notFoundMessage)) + applicationService.findAndRecordServerTokenUsage(serverToken).map(asJsonResultWithServerToken(notFoundMessage)) } else { applicationService.fetchByServerToken(serverToken).map(asJsonResult(notFoundMessage)) } @@ -257,7 +271,7 @@ class ApplicationController @Inject() ( // If request has originated from an API gateway, record usage of the Application ( if (hasGatewayUserAgent) { - applicationService.findAndRecordApplicationUsage(clientId).map(asJsonResult(notFoundMessage)) + applicationService.findAndRecordApplicationUsage(clientId).map(asJsonResultWithServerToken(notFoundMessage)) } else { applicationService.fetchByClientId(clientId).map(asJsonResult(notFoundMessage)) } diff --git a/app/uk/gov/hmrc/thirdpartyapplication/services/ApplicationService.scala b/app/uk/gov/hmrc/thirdpartyapplication/services/ApplicationService.scala index 08f31d2aa..a7e6f6b7b 100644 --- a/app/uk/gov/hmrc/thirdpartyapplication/services/ApplicationService.scala +++ b/app/uk/gov/hmrc/thirdpartyapplication/services/ApplicationService.scala @@ -174,14 +174,15 @@ class ApplicationService @Inject() ( } } - def findAndRecordApplicationUsage(clientId: ClientId): Future[Option[ApplicationWithSubscriptions]] = { + def findAndRecordApplicationUsage(clientId: ClientId): Future[Option[(ApplicationWithSubscriptions, String)]] = { timeFuture("Service Find And Record Application Usage", "application.service.findAndRecordApplicationUsage") { ( for { app <- OptionT(applicationRepository.findAndRecordApplicationUsage(clientId)) + serverToken = app.tokens.production.accessToken subscriptions <- OptionT.liftF(subscriptionRepository.getSubscriptions(app.id)) result = StoredApplication.asApplication(app).withSubscriptions(subscriptions.toSet) - } yield result + } yield (result, serverToken) ) .value } @@ -195,14 +196,16 @@ class ApplicationService @Inject() ( } } - def findAndRecordServerTokenUsage(serverToken: String): Future[Option[ApplicationWithSubscriptions]] = { + def findAndRecordServerTokenUsage(serverToken: String): Future[Option[(ApplicationWithSubscriptions, String)]] = { timeFuture("Service Find And Record Server Token Usage", "application.service.findAndRecordServerTokenUsage") { ( for { app <- OptionT(applicationRepository.findAndRecordServerTokenUsage(serverToken)) + // Unlike findAndRecordApplicationUsage(clientId), in this method the serverToken is provided as input and is used as a search term for accessToken + // so it's not necessary create a variable here, such as serverToken = app.tokens.production.accessToken subscriptions <- OptionT.liftF(subscriptionRepository.getSubscriptions(app.id)) result = StoredApplication.asApplication(app).withSubscriptions(subscriptions.toSet) - } yield result + } yield (result, serverToken) ) .value } diff --git a/test/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationControllerSpec.scala b/test/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationControllerSpec.scala index 5630c2ac5..9aae03a19 100644 --- a/test/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationControllerSpec.scala +++ b/test/uk/gov/hmrc/thirdpartyapplication/controllers/ApplicationControllerSpec.scala @@ -444,7 +444,7 @@ class ApplicationControllerSpec forAll(scenarios) { (headers, expectedLastAccessTime, shouldUpdate) => when(underTest.applicationService.fetchByServerToken(serverToken)).thenReturn(Future(Some(applicationResponse))) - when(underTest.applicationService.findAndRecordServerTokenUsage(serverToken)).thenReturn(Future(Some(updatedApplicationResponse))) + when(underTest.applicationService.findAndRecordServerTokenUsage(serverToken)).thenReturn(Future(Some((updatedApplicationResponse, serverToken)))) val result = underTest.queryDispatcher()(request.withHeaders(headers: _*)) @@ -470,7 +470,7 @@ class ApplicationControllerSpec forAll(scenarios) { (headers, expectedLastAccessTime, shouldUpdate) => when(underTest.applicationService.fetchByClientId(clientId)).thenReturn(Future(Some(applicationResponse))) - when(underTest.applicationService.findAndRecordApplicationUsage(clientId)).thenReturn(Future(Some(updatedApplicationResponse))) + when(underTest.applicationService.findAndRecordApplicationUsage(clientId)).thenReturn(Future(Some((updatedApplicationResponse, "aServerToken")))) val result = underTest.queryDispatcher()(FakeRequest("GET", s"?clientId=${clientId.value}").withHeaders(headers: _*)) @@ -479,6 +479,8 @@ class ApplicationControllerSpec (contentAsJson(result) \ "details" \ "lastAccess").as[Instant].toEpochMilli() shouldBe expectedLastAccessTime if (shouldUpdate) { verify(underTest.applicationService).findAndRecordApplicationUsage(eqTo(clientId)) + } else { + verify(underTest.applicationService).fetchByClientId(eqTo(clientId)) } reset(underTest.applicationService) } diff --git a/test/uk/gov/hmrc/thirdpartyapplication/services/ApplicationServiceSpec.scala b/test/uk/gov/hmrc/thirdpartyapplication/services/ApplicationServiceSpec.scala index 9e4cb4917..c96aa5768 100644 --- a/test/uk/gov/hmrc/thirdpartyapplication/services/ApplicationServiceSpec.scala +++ b/test/uk/gov/hmrc/thirdpartyapplication/services/ApplicationServiceSpec.scala @@ -492,16 +492,18 @@ class ApplicationServiceSpec } "recordApplicationUsage" should { - "update the Application and return an ExtendedApplicationResponse" in new Setup { + "update the Application and return an application with subscriptions and accessToken (serverToken)" in new Setup { val subscriptions = Set("myContext".asIdentifier("myVersion")) val clientId = applicationData.tokens.production.clientId + val expectedToken = applicationData.tokens.production.accessToken ApplicationRepoMock.FindAndRecordApplicationUsage.thenReturnWhen(clientId)(applicationData) SubscriptionRepoMock.Fetch.thenReturnWhen(applicationId)(subscriptions.toSeq: _*) val result = await(underTest.findAndRecordApplicationUsage(clientId)) - result.value.id shouldBe applicationId - result.value.subscriptions shouldBe subscriptions + result.value._1.id shouldBe applicationId + result.value._1.subscriptions shouldBe subscriptions + result.value._2 shouldBe expectedToken } } @@ -539,7 +541,7 @@ class ApplicationServiceSpec } "findAndRecordServerTokenUsage" should { - "update the Application and return an ExtendedApplicationResponse" in new Setup { + "update the Application and return an application with subscriptions and server token (accessToken)" in new Setup { val subscriptions = Set("myContext".asIdentifier("myVersion")) val serverToken = applicationData.tokens.production.accessToken ApplicationRepoMock.FindAndRecordServerTokenUsage.thenReturnWhen(serverToken)(applicationData) @@ -547,8 +549,9 @@ class ApplicationServiceSpec val result = await(underTest.findAndRecordServerTokenUsage(serverToken)) - result.value.id shouldBe applicationId - result.value.subscriptions shouldBe subscriptions + result.value._1.id shouldBe applicationId + result.value._1.subscriptions shouldBe subscriptions + result.value._2 shouldBe serverToken ApplicationRepoMock.FindAndRecordServerTokenUsage.verifyCalledWith(serverToken) } }