APIS-6765: Play 3.0 upgrade (#113)

* APIS-6765: Play 3.0 upgrade

* APIS-6765 - Refined json and map of refined

* APIS-6765 - Context/VersionNbr/CLientId from library

* APIS-6765 - Replace derived-play-json

* APIS-6765 - Test URL validation with length validation

* APIS-6765 - PR Bot comments

* APIS-6765 - PR comments

---------

Co-authored-by: Andy Spaven <16537059+AndySpaven@users.noreply.github.com>

acceptance/uk/gov/hmrc/apisubscriptionfields/AcceptanceTestSpec.scala

@@ -27,7 +27,6 @@ import play.api.test.FakeRequest
 import play.api.test.Helpers._
 import uk.gov.hmrc.apisubscriptionfields.model._
 
-import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.duration._
 import scala.concurrent.{Await, Future}
 import cats.data.NonEmptyList
@@ -56,12 +55,12 @@ trait AcceptanceTestSpec extends AnyFeatureSpec
 
   protected val InvalidNonJsonPayload = "##INVALID_JSON_PAYLOAD##"
 
-  protected def subscriptionFieldsEndpoint(clientId: String, apiContext: String, apiVersion: String) =
-    s"/field/application/$clientId/context/$apiContext/version/$apiVersion"
+  protected def subscriptionFieldsEndpoint(clientId: String, apiContext: String, apiVersionNbr: String) =
+    s"/field/application/$clientId/context/$apiContext/version/$apiVersionNbr"
 
   protected def byClientIdEndpoint(clientId: String) = s"/field/application/$clientId"
 
-  protected def definitionEndpoint(apiContext: String, apiVersion: String) = s"/definition/context/$apiContext/version/$apiVersion"
+  protected def definitionEndpoint(apiContext: String, apiVersionNbr: String) = s"/definition/context/$apiContext/version/$apiVersionNbr"
 
   protected def fieldsIdEndpoint(fieldsId: UUID) = s"/field/$fieldsId"
 
@@ -78,8 +77,8 @@ trait AcceptanceTestSpec extends AnyFeatureSpec
     FakeRequest().withHeaders(RequestHeaders.ACCEPT_HMRC_JSON_HEADER, RequestHeaders.CONTENT_TYPE_HEADER)
   }
 
-  protected def fieldsEndpoint(clientId: String, apiContext: String, apiVersion: String) =
-    s"/field/application/$clientId/context/$apiContext/version/$apiVersion"
+  protected def fieldsEndpoint(clientId: String, apiContext: String, apiVersionNbr: String) =
+    s"/field/application/$clientId/context/$apiContext/version/$apiVersionNbr"
 
   override def fakeApplication(): Application = new GuiceApplicationBuilder().configure(Map(
     "metrics.jvm" -> false,

acceptance/uk/gov/hmrc/apisubscriptionfields/ApiSubscriptionFieldsHappySpec.scala

@@ -28,6 +28,7 @@ import scala.concurrent.Future
 import scala.concurrent.Await
 import scala.concurrent.duration._
 import uk.gov.hmrc.apisubscriptionfields.utils.ApplicationLogger
+import uk.gov.hmrc.apiplatform.modules.common.domain.models._
 
 class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
   with OptionValues
@@ -78,7 +79,7 @@ class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
       val fieldsId = sfr.get.fieldsId
 
       sfr.isSuccess shouldBe true
-      sfr.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersion("1.0.2"), fieldsId, SampleFields1)
+      sfr.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersionNbr("1.0.2"), fieldsId, SampleFields1)
     }
 
     def createSubscriptionFieldsRequest(): FakeRequest[AnyContentAsJson] = {
@@ -111,7 +112,7 @@ class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
       val fieldsId = sfr.get.fieldsId
 
       sfr.isSuccess shouldBe true
-      sfr.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersion("1.0.2"), fieldsId, SampleFields1)
+      sfr.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersionNbr("1.0.2"), fieldsId, SampleFields1)
     }
 
     Scenario("the API is called to GET all existing subscription fields") {
@@ -135,7 +136,7 @@ class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
       val fieldsId = sfr.get.subscriptions.head.fieldsId
 
       sfr.isSuccess shouldBe true
-      sfr.get shouldBe BulkSubscriptionFieldsResponse(Seq(SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersion("1.0.2"), fieldsId, SampleFields1)))
+      sfr.get shouldBe BulkSubscriptionFieldsResponse(Seq(SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersionNbr("1.0.2"), fieldsId, SampleFields1)))
     }
 
     Scenario("the API is called to GET with a known fieldsId") {
@@ -175,7 +176,7 @@ class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
       val sfrFieldsId = contentAsJson(resultFuture).validate[SubscriptionFields]
 
       sfrFieldsId.isSuccess shouldBe true
-      sfrFieldsId.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersion("1.0.2"), fieldsId, SampleFields1)
+      sfrFieldsId.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersionNbr("1.0.2"), fieldsId, SampleFields1)
     }
 
     Scenario("the API is called to GET existing subscription fields by application clientId") {
@@ -200,7 +201,7 @@ class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
       val fieldsId = sfr.get.subscriptions.head.fieldsId
 
       sfr.isSuccess shouldBe true
-      sfr.get shouldBe BulkSubscriptionFieldsResponse(Seq(SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersion("1.0.2"), fieldsId, SampleFields1)))
+      sfr.get shouldBe BulkSubscriptionFieldsResponse(Seq(SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersionNbr("1.0.2"), fieldsId, SampleFields1)))
     }
 
     Scenario("the API is called to update existing subscription fields") {
@@ -224,7 +225,7 @@ class ApiSubscriptionFieldsHappySpec extends AcceptanceTestSpec
       val fieldsId = sfr.get.fieldsId
 
       sfr.isSuccess shouldBe true
-      sfr.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersion("1.0.2"), fieldsId, SampleFields2)
+      sfr.get shouldBe SubscriptionFields(FakeClientId, ApiContext("acontext"), ApiVersionNbr("1.0.2"), fieldsId, SampleFields2)
     }
 
     Scenario("the API is called to DELETE existing subscription fields") {

app/uk/gov/hmrc/apisubscriptionfields/connector/JsonFormatters.scala

@@ -17,8 +17,9 @@
 package uk.gov.hmrc.apisubscriptionfields.connector
 
 import play.api.libs.json._
+import uk.gov.hmrc.apiplatform.modules.common.domain.models._
 
-import uk.gov.hmrc.apisubscriptionfields.model.{BoxId, ClientId, SubscriptionFieldsId}
+import uk.gov.hmrc.apisubscriptionfields.model.{BoxId, SubscriptionFieldsId}
 
 trait JsonFormatters {
   implicit val clientIdJF: Format[ClientId]                         = Json.valueFormat[ClientId]

app/uk/gov/hmrc/apisubscriptionfields/connector/Model.scala

@@ -16,7 +16,9 @@
 
 package uk.gov.hmrc.apisubscriptionfields.connector
 
-import uk.gov.hmrc.apisubscriptionfields.model.{BoxId, ClientId}
+import uk.gov.hmrc.apiplatform.modules.common.domain.models.ClientId
+
+import uk.gov.hmrc.apisubscriptionfields.model.BoxId
 
 private[connector] case class CreateBoxRequest(boxName: String, clientId: ClientId)
 

app/uk/gov/hmrc/apisubscriptionfields/connector/PushPullNotificationServiceConnector.scala

@@ -20,6 +20,7 @@ import javax.inject.{Inject, Singleton}
 import scala.concurrent.{ExecutionContext, Future}
 import scala.util.control.NonFatal
 
+import uk.gov.hmrc.apiplatform.modules.common.domain.models.ClientId
 import uk.gov.hmrc.http.HttpReads.Implicits._
 import uk.gov.hmrc.http.{HeaderCarrier, HttpClient}
 import uk.gov.hmrc.play.http.metrics.common._

app/uk/gov/hmrc/apisubscriptionfields/controller/ApiFieldDefinitionsController.scala

@@ -23,6 +23,7 @@ import scala.util.{Failure, Success, Try}
 
 import play.api.libs.json.{JsError, JsSuccess, JsValue, Json}
 import play.api.mvc._
+import uk.gov.hmrc.apiplatform.modules.common.domain.models._
 
 import uk.gov.hmrc.apisubscriptionfields.model._
 import uk.gov.hmrc.apisubscriptionfields.service.ApiFieldDefinitionsService
@@ -41,8 +42,8 @@ class ApiFieldDefinitionsController @Inject() (cc: ControllerComponents, service
     BadRequest(s"""{"tag": "$errorTag"}""")
   }
 
-  private def notFoundResponse(apiContext: ApiContext, apiVersion: ApiVersion) =
-    NotFound(JsErrorResponse(ErrorCode.NOT_FOUND, s"Fields definition not found for (${apiContext.value}, ${apiVersion.value})"))
+  private def notFoundResponse(apiContext: ApiContext, apiVersionNbr: ApiVersionNbr) =
+    NotFound(JsErrorResponse(ErrorCode.NOT_FOUND, s"Fields definition not found for (${apiContext.value}, ${apiVersionNbr.value})"))
 
   def validateFieldsDefinition(): Action[JsValue] = Action(parse.json) { request =>
     Try(request.body.validate[FieldDefinitionsRequest]) match {
@@ -56,9 +57,9 @@ class ApiFieldDefinitionsController @Inject() (cc: ControllerComponents, service
     }
   }
 
-  def upsertFieldsDefinition(apiContext: ApiContext, apiVersion: ApiVersion): Action[JsValue] = Action.async(parse.json) { implicit request =>
+  def upsertFieldsDefinition(apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): Action[JsValue] = Action.async(parse.json) { implicit request =>
     withJsonBody[FieldDefinitionsRequest] { payload =>
-      service.upsert(apiContext, apiVersion, payload.fieldDefinitions) map {
+      service.upsert(apiContext, apiVersionNbr, payload.fieldDefinitions) map {
         case (response, true)  => Created(Json.toJson(response))
         case (response, false) => Ok(Json.toJson(response))
       }
@@ -69,22 +70,22 @@ class ApiFieldDefinitionsController @Inject() (cc: ControllerComponents, service
     service.getAll map (defs => Ok(Json.toJson(defs))) recover recovery
   }
 
-  def getFieldsDefinition(apiContext: ApiContext, apiVersion: ApiVersion): Action[AnyContent] = Action.async { _ =>
-    val eventualMaybeResponse = service.get(apiContext, apiVersion)
-    asActionResult(eventualMaybeResponse, apiContext, apiVersion)
+  def getFieldsDefinition(apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): Action[AnyContent] = Action.async { _ =>
+    val eventualMaybeResponse = service.get(apiContext, apiVersionNbr)
+    asActionResult(eventualMaybeResponse, apiContext, apiVersionNbr)
   }
 
-  def deleteFieldsDefinition(apiContext: ApiContext, apiVersion: ApiVersion): Action[AnyContent] = Action.async { _ =>
-    service.delete(apiContext, apiVersion) map {
+  def deleteFieldsDefinition(apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): Action[AnyContent] = Action.async { _ =>
+    service.delete(apiContext, apiVersionNbr) map {
       case true  => NoContent
-      case false => notFoundResponse(apiContext, apiVersion)
+      case false => notFoundResponse(apiContext, apiVersionNbr)
     } recover recovery
   }
 
-  private def asActionResult(eventualMaybeResponse: Future[Option[ApiFieldDefinitions]], apiContext: ApiContext, apiVersion: ApiVersion) = {
+  private def asActionResult(eventualMaybeResponse: Future[Option[ApiFieldDefinitions]], apiContext: ApiContext, apiVersionNbr: ApiVersionNbr) = {
     eventualMaybeResponse map {
       case Some(subscriptionFields) => Ok(Json.toJson(subscriptionFields))
-      case None                     => notFoundResponse(apiContext, apiVersion)
+      case None                     => notFoundResponse(apiContext, apiVersionNbr)
     } recover recovery
   }
 

app/uk/gov/hmrc/apisubscriptionfields/controller/Binders.scala

@@ -19,8 +19,9 @@ package uk.gov.hmrc.apisubscriptionfields.controller
 import java.{util => ju}
 
 import play.api.mvc.PathBindable
+import uk.gov.hmrc.apiplatform.modules.common.domain.models._
 
-import uk.gov.hmrc.apisubscriptionfields.model.{ApiContext, ApiVersion, ClientId, SubscriptionFieldsId}
+import uk.gov.hmrc.apisubscriptionfields.model.SubscriptionFieldsId
 
 object Binders {
 
@@ -32,10 +33,10 @@ object Binders {
       )
 
   implicit object apiVersionPathBindable
-      extends PathBindable.Parsing[ApiVersion](
-        ApiVersion.apply,
+      extends PathBindable.Parsing[ApiVersionNbr](
+        ApiVersionNbr.apply,
         _.value,
-        (key: String, e: Exception) => "Cannot parse parameter %s as ApiVersion: %s".format(key, e.getMessage)
+        (key: String, e: Exception) => "Cannot parse parameter %s as ApiVersionNbr: %s".format(key, e.getMessage)
       )
 
   implicit object clientIdPathBindable

app/uk/gov/hmrc/apisubscriptionfields/controller/Requests.scala

@@ -18,17 +18,16 @@ package uk.gov.hmrc.apisubscriptionfields.controller
 
 import cats.data.NonEmptyList
 
-import uk.gov.hmrc.apisubscriptionfields.model.FieldDefinition
 import uk.gov.hmrc.apisubscriptionfields.model.Types._
+import uk.gov.hmrc.apisubscriptionfields.model.{FieldDefinition, RefinedJson}
 
 private[controller] case class SubscriptionFieldsRequest(fields: Fields)
 
 private[controller] case class FieldDefinitionsRequest(fieldDefinitions: NonEmptyList[FieldDefinition])
 
 object SubscriptionFieldsRequest {
+  import RefinedJson.formatRefined
   import play.api.libs.json._
-  import be.venneborg.refined.play.RefinedJsonFormats._
-  import eu.timepit.refined.auto._
 
   implicit val SubscriptionFieldsRequestJF: OFormat[SubscriptionFieldsRequest] = Json.format[SubscriptionFieldsRequest]
 }

app/uk/gov/hmrc/apisubscriptionfields/controller/SubscriptionFieldsController.scala

@@ -21,6 +21,7 @@ import scala.concurrent.{ExecutionContext, Future}
 
 import play.api.libs.json._
 import play.api.mvc._
+import uk.gov.hmrc.apiplatform.modules.common.domain.models._
 
 import uk.gov.hmrc.apisubscriptionfields.model.ErrorCode._
 import uk.gov.hmrc.apisubscriptionfields.model._
@@ -35,8 +36,8 @@ class SubscriptionFieldsController @Inject() (cc: ControllerComponents, service:
     NotFound(JsErrorResponse(ErrorCode.NOT_FOUND, message))
   }
 
-  private def notFoundMessage(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): String = {
-    s"Subscription fields not found for (${clientId.value}, ${apiContext.value}, ${apiVersion.value})"
+  private def notFoundMessage(clientId: ClientId, apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): String = {
+    s"Subscription fields not found for (${clientId.value}, ${apiContext.value}, ${apiVersionNbr.value})"
   }
 
   private def fieldIdNotFoundMessage(subscriptionFieldsId: SubscriptionFieldsId): String = {
@@ -47,9 +48,9 @@ class SubscriptionFieldsController @Inject() (cc: ControllerComponents, service:
     s"ClientId (${clientId.value.toString}) was not found"
   }
 
-  def getSubscriptionFields(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Action[AnyContent] = Action.async { _ =>
-    val eventualMaybeResponse = service.get(clientId, apiContext, apiVersion)
-    asActionResult(eventualMaybeResponse, notFoundMessage(clientId, apiContext, apiVersion))
+  def getSubscriptionFields(clientId: ClientId, apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): Action[AnyContent] = Action.async { _ =>
+    val eventualMaybeResponse = service.get(clientId, apiContext, apiVersionNbr)
+    asActionResult(eventualMaybeResponse, notFoundMessage(clientId, apiContext, apiVersionNbr))
   }
 
   def getSubscriptionFieldsByFieldsId(subscriptionFieldsId: SubscriptionFieldsId): Action[AnyContent] = Action.async { _ =>
@@ -80,15 +81,15 @@ class SubscriptionFieldsController @Inject() (cc: ControllerComponents, service:
     } recover recovery
   }
 
-  def upsertSubscriptionFields(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Action[JsValue] = Action.async(parse.json) { implicit request =>
+  def upsertSubscriptionFields(clientId: ClientId, apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): Action[JsValue] = Action.async(parse.json) { implicit request =>
     import JsonFormatters._
 
     withJsonBody[SubscriptionFieldsRequest] { payload =>
       if (payload.fields.isEmpty) {
         Future.successful(UnprocessableEntity(JsErrorResponse(INVALID_REQUEST_PAYLOAD, "At least one field must be specified")))
       } else {
         service
-          .upsert(clientId, apiContext, apiVersion, payload.fields)
+          .upsert(clientId, apiContext, apiVersionNbr, payload.fields)
           .map(_ match {
             case NotFoundSubsFieldsUpsertResponse                             => BadRequest(Json.toJson("reason" -> "field definitions not found")) // TODO
             case FailedValidationSubsFieldsUpsertResponse(fieldErrorMessages) =>
@@ -102,10 +103,10 @@ class SubscriptionFieldsController @Inject() (cc: ControllerComponents, service:
     }
   }
 
-  def deleteSubscriptionFields(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Action[AnyContent] = Action.async { _ =>
-    service.delete(clientId, apiContext, apiVersion) map {
+  def deleteSubscriptionFields(clientId: ClientId, apiContext: ApiContext, apiVersionNbr: ApiVersionNbr): Action[AnyContent] = Action.async { _ =>
+    service.delete(clientId, apiContext, apiVersionNbr) map {
       case true  => NoContent
-      case false => notFoundResponse(notFoundMessage(clientId, apiContext, apiVersion))
+      case false => notFoundResponse(notFoundMessage(clientId, apiContext, apiVersionNbr))
     } recover recovery
   }