Skip to content

Commit

Permalink
Merge pull request #126 from hmrc/API-8024
Browse files Browse the repository at this point in the history 
API-8024 - Remove subscriptions from retired APIs on publish
  • Loading branch information
@LewisMagri
LewisMagri authored Dec 6, 2024
2 parents a5a6100 + 7dd612e commit 47d41b4
Show file tree
Hide file tree
Showing 7 changed files with 301 additions and 127 deletions.
16 changes: 9 additions & 7 deletions app/uk/gov/hmrc/apipublisher/connectors/TpaConnector.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package uk.gov.hmrc.apipublisher.connectors
import javax.inject.{Inject, Singleton}
import scala.concurrent.{ExecutionContext, Future}

import uk.gov.hmrc.apiplatform.modules.applications.core.domain.models.ApplicationWithCollaborators
import play.api.http.Status.UNPROCESSABLE_ENTITY
import uk.gov.hmrc.http.HttpReads.Implicits._
import uk.gov.hmrc.http._
import uk.gov.hmrc.http.client.HttpClientV2
Expand All @@ -36,11 +36,13 @@ class TpaConnector @Inject() (config: TpaConnector.Config, http: HttpClientV2)(i

protected val serviceBaseUrl: String = config.serviceBaseUrl

// This only uses library code.
// $COVERAGE-OFF$
def fetchApplications(apiContext: String, versionNbr: String)(implicit hc: HeaderCarrier): Future[List[ApplicationWithCollaborators]] = {
http.get(url"$serviceBaseUrl/application/?subscribesTo=${apiContext}&version=${versionNbr}")
.execute[List[ApplicationWithCollaborators]]
def deleteSubscriptions(apiContext: String, versionNbr: String)(implicit hc: HeaderCarrier): Future[Unit] = {
http.delete(url"$serviceBaseUrl/apis/$apiContext/versions/$versionNbr/subscribers")
.execute[Either[UpstreamErrorResponse, HttpResponse]]
.map {
case Right(_) => (())
case Left(UpstreamErrorResponse(message, UNPROCESSABLE_ENTITY, _, _)) => throw new UnprocessableEntityException(message)
case Left(err) => throw err
}
}
// $COVERAGE-ON$
}
35 changes: 9 additions & 26 deletions app/uk/gov/hmrc/apipublisher/services/PublisherService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,17 @@ class PublisherService @Inject() (
for {
_ <- apiDefinitionConnector.publishAPI(apiDetailsWithServiceLocation)
_ <- publishFieldDefinitions(apiAndScopes.fieldDefinitions)
_ <- deleteRetiredSubscriptions()
} yield apiDetailsWithServiceLocation
}

def deleteRetiredSubscriptions() = {
Future.sequence(
apiAndScopes.retiredVersionNumbers.toList
.map { version => tpaConnector.deleteSubscriptions(apiAndScopes.apiContext, version) }
)
}

def publishFieldDefinitions(fieldDefinitions: Seq[ApiFieldDefinitions]): Future[Unit] = {
if (fieldDefinitions.nonEmpty) {
apiSubscriptionFieldsConnector.publishFieldDefinitions(fieldDefinitions)
Expand Down Expand Up @@ -80,43 +88,18 @@ class PublisherService @Inject() (
}
}

def validateStatusAndSubscriptions()(implicit hc: HeaderCarrier) = {
def hasAnySubscribers(apiContext: String, version: String): Future[(String, Boolean)] = {
tpaConnector.fetchApplications(apiAndScopes.apiContext, version).map(as => (version, as.nonEmpty))
}

val vs: Future[List[String]] = Future.sequence(
apiAndScopes.retiredVersionNumbers.toList
.map { version => hasAnySubscribers(apiAndScopes.apiContext, version) }
)
.map {
_.filter(x => x._2).map(_._1)
}

vs.map { versions =>
versions.map { v =>
JsString(s"Version $v cannot be retired as it still has active subscriptions. Talk to SDST ([email protected]).")
} match {
case Nil => None
case h :: t => Some(JsArray(h :: t))
}
}
}

def checkForErrors(): Future[Option[JsObject]] = {
for {
apiErrors <- conditionalValidateApiDefinition(apiAndScopes, validateApiDefinition)
statusErrors <- validateStatusAndSubscriptions()
fieldDefnErrors <- apiSubscriptionFieldsConnector.validateFieldDefinitions(apiAndScopes.fieldDefinitions.flatMap(_.fieldDefinitions))
} yield {
if (apiErrors.isEmpty && fieldDefnErrors.isEmpty && statusErrors.isEmpty) {
if (apiErrors.isEmpty && fieldDefnErrors.isEmpty) {
None
} else {
Some(
JsObject(
Seq.empty[(String, JsValue)] ++
apiErrors.map("apiDefinitionErrors" -> _) ++
statusErrors.map("statusErrors" -> _) ++
fieldDefnErrors.map("fieldDefinitionErrors" -> _)
)
)
Expand Down
10 changes: 10 additions & 0 deletions it/test/uk/gov/hmrc/apipublisher/BaseFeatureSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ abstract class BaseFeatureSpec extends AnyFeatureSpec
val apiSubscriptionFieldsServer = new WireMockServer(WireMockConfiguration.wireMockConfig().port(apiSubscriptionFieldsPort))
var apiSubscriptionFieldsMock: WireMock = _

val tpaPort: Int = sys.env.getOrElse("WIREMOCK", "9607").toInt
val tpaHost = "localhost"
var tpaUrl = s"http://$tpaHost:$tpaPort"
val tpaServer = new WireMockServer(WireMockConfiguration.wireMockConfig().port(tpaPort))
var tpaMock: WireMock = _

override def beforeEach(): Unit = {
apiSubscriptionFieldsServer.start()
apiSubscriptionFieldsMock = new WireMock(apiSubscriptionFieldsHost, apiSubscriptionFieldsPort)
Expand All @@ -69,13 +75,17 @@ abstract class BaseFeatureSpec extends AnyFeatureSpec
apiDefinitionServer.start()
apiDefinitionMock = new WireMock(apiDefinitionHost, apiDefinitionPort)

tpaServer.start()
tpaMock = new WireMock(tpaHost, tpaPort)

}

override def afterEach(): Unit = {
apiSubscriptionFieldsServer.stop()
apiScopeServer.stop()
apiProducerServer.stop()
apiDefinitionServer.stop()
tpaServer.stop()
}

def http(request: => Request[Either[String, String], Any]): Response[Either[String, String]] = {
Expand Down
111 changes: 111 additions & 0 deletions it/test/uk/gov/hmrc/apipublisher/PublisherFeatureSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,67 @@ class PublisherFeatureSpec extends BaseFeatureSpec with EitherValues {
publishResponse.isSuccess shouldBe true
}

Scenario("Publishing successful for an API with a RETIRED version") {

Given("A microservice is running with an API Definition without scopes")
apiProducerMock.register(get(urlEqualTo("/api/definition")).willReturn(aResponse().withBody(definitionJsonWithRetiredVersion)))
apiProducerMock.register(get(urlEqualTo("/api/conf/1.0/application.yaml")).willReturn(aResponse().withBody(oas_1_0)))
apiProducerMock.register(get(urlEqualTo("/api/conf/2.0/application.yaml")).willReturn(aResponse().withBody(oas_2_0)))
apiProducerMock.register(get(urlEqualTo("/api/conf/3.0/application.yaml")).willReturn(aResponse().withBody(oas_3_0)))

And("api definition is running")
// TOOD - restore when api definition no longer rejects updated api
apiDefinitionMock.register(post(urlEqualTo("/api-definition")).willReturn(aResponse()))

And("api subscription fields is running")
apiSubscriptionFieldsMock.register(put(urlEqualTo(apiSubscriptionFieldsUrlVersion_1_0)).willReturn(aResponse()))
apiSubscriptionFieldsMock.register(put(urlEqualTo(apiSubscriptionFieldsUrlVersion_3_0)).willReturn(aResponse()))
apiSubscriptionFieldsMock.register(post(urlEqualTo("/validate")).willReturn(aResponse()))

And("third party application is running")
tpaMock.register(delete(urlEqualTo(tpaVersion_1_0)).willReturn(aResponse()))
tpaMock.register(delete(urlEqualTo(tpaVersion_2_0)).willReturn(aResponse()))
tpaMock.register(delete(urlEqualTo(tpaVersion_3_0)).willReturn(aResponse()))

When("publisher is triggered")
val publishResponse = http(
basicRequest
.post(uri"$serverUrl/publish")
.header(CONTENT_TYPE, JSON)
.header(AUTHORIZATION, encodedPublishingKey)
.body(s"""{"serviceName":"test.example.com", "serviceUrl": "$apiProducerUrl", "metadata": { "third-party-api" : "true" } }""")
)

Then("The field definitions are validated")
apiSubscriptionFieldsMock.verifyThat(postRequestedFor(urlEqualTo("/validate"))
.withHeader(CONTENT_TYPE, containing(JSON)))

Then("The definition is published to the API Definition microservice")
apiDefinitionMock.verifyThat(postRequestedFor(urlEqualTo("/api-definition"))
.withHeader(CONTENT_TYPE, containing(JSON)))

Then("The field definitions are published to the API Subscription Fields microservice")
apiSubscriptionFieldsMock.verifyThat(putRequestedFor(urlEqualTo(apiSubscriptionFieldsUrlVersion_1_0))
.withHeader(CONTENT_TYPE, containing(JSON))
.withRequestBody(equalToJson(fieldDefinitions_1_0)))

apiSubscriptionFieldsMock.verifyThat(0, putRequestedFor(urlEqualTo(apiSubscriptionFieldsUrlVersion_2_0)))

apiSubscriptionFieldsMock.verifyThat(putRequestedFor(urlEqualTo(apiSubscriptionFieldsUrlVersion_3_0))
.withHeader(CONTENT_TYPE, containing(JSON))
.withRequestBody(equalToJson(fieldDefinitions_3_0)))

Then("The subscriptions for RETIRED versions are deleted")
tpaMock.verifyThat(deleteRequestedFor(urlEqualTo(tpaVersion_1_0)))

tpaMock.verifyThat(0, deleteRequestedFor(urlEqualTo(tpaVersion_2_0)))

tpaMock.verifyThat(0, deleteRequestedFor(urlEqualTo(tpaVersion_3_0)))

And("api-publisher responded with status 2xx")
publishResponse.isSuccess shouldBe true
}

Scenario("A validation error occurs during Publish due to scopes in definition") {

Given("A microservice is running with an API Definition")
Expand Down Expand Up @@ -422,6 +483,9 @@ class PublisherFeatureSpec extends BaseFeatureSpec with EitherValues {
val apiSubscriptionFieldsUrlVersion_1_0 = s"/definition/context/$urlEncodedApiContext/version/1.0"
val apiSubscriptionFieldsUrlVersion_2_0 = s"/definition/context/$urlEncodedApiContext/version/2.0"
val apiSubscriptionFieldsUrlVersion_3_0 = s"/definition/context/$urlEncodedApiContext/version/3.0"
val tpaVersion_1_0 = s"/apis/$urlEncodedApiContext/versions/1.0/subscribers"
val tpaVersion_2_0 = s"/apis/$urlEncodedApiContext/versions/2.0/subscribers"
val tpaVersion_3_0 = s"/apis/$urlEncodedApiContext/versions/3.0/subscribers"

val definitionJsonWithInvalidContext =
s"""
Expand Down Expand Up @@ -730,6 +794,53 @@ class PublisherFeatureSpec extends BaseFeatureSpec with EitherValues {
|}
""".stripMargin

val definitionJsonWithRetiredVersion =
s"""
|{
| "api": {
| "name": "Test",
| "description": "Test API",
| "context": "$apiContext",
| "versions": [
| {
| "version": "1.0",
| "status": "RETIRED",
| "fieldDefinitions": [
| {
| "name": "callbackUrl",
| "description": "Callback URL",
| "hint": "Just a hint",
| "type": "URL"
| },
| {
| "name": "token",
| "description": "Secure Token",
| "hint": "Just a hint",
| "type": "SecureToken"
| }
| ]
| },
| {
| "version": "2.0",
| "status": "PUBLISHED"
| },
| {
| "version": "3.0",
| "status": "PUBLISHED",
| "fieldDefinitions": [
| {
| "name": "callbackUrlOnly",
| "description": "Only a callback URL",
| "hint": "Just a hint",
| "type": "URL"
| }
| ]
| }
| ]
| }
|}
""".stripMargin

val fieldDefinitions_1_0 =
"""
|{
Expand Down
61 changes: 6 additions & 55 deletions test/resources/input/api-with-2-retired-status.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,64 +5,15 @@
"versions" : [
{
"version" : "1.0",
"access" : {
"type" : "PRIVATE"
},
"status" : "RETIRED",
"fieldDefinitions": [
{
"name": "callback-url",
"description": "Callback URL",
"hint": "Just a hint",
"type": "URL",
"shortDescription": "short description",
"validation": {
"errorMessage": "error message",
"rules": [
{
"UrlValidationRule": {}
}
]
}
},
{
"name": "token",
"description": "Secure Token",
"hint": "Just a hint",
"type": "SecureToken",
"validation": {
"errorMessage": "error message",
"rules": [
{
"RegexValidationRule": {
"regex": "regex for token"
}
}
]
},
"access": {
"devhub": {
"read": "anyone",
"write":"noOne"
}
}
}
]
"status" : "RETIRED"
},
{
"version" : "2.0",
"access" : {
"type" : "PUBLIC"
},
"status" : "RETIRED",
"fieldDefinitions": [
{
"name": "callback-url-only",
"description": "Only a callback URL",
"hint": "Just a hint",
"type": "URL"
}
]
"status" : "RETIRED"
},
{
"version" : "3.0",
"status" : "STABLE"
}
]
}
Loading

0 comments on commit 47d41b4

Please sign in to comment.