MTDSA-16033: Remove references of redundant feature switches for indi…

…viduals-disclosures-api (#113) * MTDSA-16033 progress * MTDSA-16033: Completed * MTDSA-16033: Addressed platops comments * MTDSA-16033: Implemented reviewer comments * Remove .DS_Store from everywhere
hmrc · Jul 10, 2023 · e979ae1 · e979ae1
1 parent 3f0f1a2
commit e979ae1
Show file tree

Hide file tree

Showing 23 changed files with 208 additions and 356 deletions.
diff --git a/app/api/controllers/AuthorisedController.scala b/app/api/controllers/AuthorisedController.scala
@@ -17,10 +17,8 @@
 package api.controllers
 
 import api.models.auth.UserDetails
-import api.models.errors
-import api.models.errors.{ClientNotAuthorisedError, InvalidBearerTokenError, NinoFormatError}
+import api.models.errors.MtdError
 import api.services.{EnrolmentsAuthService, MtdIdLookupService}
-import play.api.libs.json.Json
 import play.api.mvc._
 import uk.gov.hmrc.auth.core.Enrolment
 import uk.gov.hmrc.auth.core.authorise.Predicate
@@ -50,24 +48,22 @@ abstract class AuthorisedController(cc: ControllerComponents)(implicit ec: Execu
     def invokeBlockWithAuthCheck[A](mtdId: String, request: Request[A], block: UserRequest[A] => Future[Result])(implicit
         headerCarrier: HeaderCarrier): Future[Result] = {
       authService.authorised(predicate(mtdId)).flatMap[Result] {
-        case Right(userDetails)             => block(UserRequest(userDetails.copy(mtdId = mtdId), request))
-        case Left(ClientNotAuthorisedError) => Future.successful(Forbidden(Json.toJson(ClientNotAuthorisedError)))
-        case Left(_)                        => Future.successful(InternalServerError(Json.toJson(errors.InternalError)))
+        case Right(userDetails) => block(UserRequest(userDetails.copy(mtdId = mtdId), request))
+        case Left(mtdError)     => errorResponse(mtdError)
       }
     }
 
     override def invokeBlock[A](request: Request[A], block: UserRequest[A] => Future[Result]): Future[Result] = {
+
       implicit val headerCarrier: HeaderCarrier = hc(request)
 
       lookupService.lookup(nino).flatMap[Result] {
-        case Right(mtdId)                   => invokeBlockWithAuthCheck(mtdId, request, block)
-        case Left(NinoFormatError)          => Future.successful(BadRequest(Json.toJson(NinoFormatError)))
-        case Left(ClientNotAuthorisedError) => Future.successful(Forbidden(Json.toJson(ClientNotAuthorisedError)))
-        case Left(InvalidBearerTokenError)  => Future.successful(Unauthorized(Json.toJson(InvalidBearerTokenError)))
-        case Left(_)                        => Future.successful(InternalServerError(Json.toJson(errors.InternalError)))
+        case Right(mtdId)   => invokeBlockWithAuthCheck(mtdId, request, block)
+        case Left(mtdError) => errorResponse(mtdError)
       }
     }
 
+    private def errorResponse[A](mtdError: MtdError): Future[Result] = Future.successful(Status(mtdError.httpStatus)(mtdError.asJson))
   }
 
 }
diff --git a/app/config/AppConfig.scala b/app/config/AppConfig.scala
@@ -18,6 +18,7 @@ package config
 
 import com.typesafe.config.Config
 import play.api.{ConfigLoader, Configuration}
+import routing.Version
 import uk.gov.hmrc.auth.core.ConfidenceLevel
 import uk.gov.hmrc.play.bootstrap.config.ServicesConfig
 
@@ -51,9 +52,9 @@ trait AppConfig {
   // API Config
   def apiGatewayContext: String
   def confidenceLevelConfig: ConfidenceLevelConfig
-  def apiStatus(version: String): String
+  def apiStatus(version: Version): String
   def featureSwitches: Configuration
-  def endpointsEnabled(version: String): Boolean
+  def endpointsEnabled(version: Version): Boolean
 }
 
 @Singleton
@@ -79,9 +80,9 @@ class AppConfigImpl @Inject() (config: ServicesConfig, configuration: Configurat
   // API Config
   val apiGatewayContext: String                    = config.getString("api.gateway.context")
   val confidenceLevelConfig: ConfidenceLevelConfig = configuration.get[ConfidenceLevelConfig](s"api.confidence-level-check")
-  def apiStatus(version: String): String           = config.getString(s"api.$version.status")
+  def apiStatus(version: Version): String          = config.getString(s"api.${version.name}.status")
   def featureSwitches: Configuration               = configuration.getOptional[Configuration](s"feature-switch").getOrElse(Configuration.empty)
-  def endpointsEnabled(version: String): Boolean   = config.getBoolean(s"api.$version.endpoints.enabled")
+  def endpointsEnabled(version: Version): Boolean  = config.getBoolean(s"api.${version.name}.endpoints.enabled")
 }
 
 trait FixedConfig {

diff --git a/app/config/FeatureSwitches.scala b/app/config/FeatureSwitches.scala
@@ -18,29 +18,7 @@ package config
 
 import play.api.Configuration
 
-case class FeatureSwitches(featureSwitchConfig: Configuration) {
-
-  private val versionRegex = """(\d)\.\d""".r
-
-  def isVersionEnabled(version: String): Boolean = {
-    val maybeVersion: Option[String] =
-      version match {
-        case versionRegex(v) => Some(v)
-        case _               => None
-      }
-
-    val enabled = for {
-      validVersion <- maybeVersion
-      enabled      <- featureSwitchConfig.getOptional[Boolean](s"version-$validVersion.enabled")
-    } yield enabled
-
-    enabled.getOrElse(false)
-  }
-
-  val isMarriageAllowanceRoutingEnabled: Boolean = isEnabled("marriage-allowance.enabled")
-
-  private def isEnabled(key: String): Boolean = featureSwitchConfig.getOptional[Boolean](key).getOrElse(true)
-}
+case class FeatureSwitches(featureSwitchConfig: Configuration)
 
 object FeatureSwitches {
   def apply()(implicit appConfig: AppConfig): FeatureSwitches = FeatureSwitches(appConfig.featureSwitches)

diff --git a/app/definition/ApiDefinition.scala b/app/definition/ApiDefinition.scala
@@ -17,6 +17,7 @@
 package definition
 
 import play.api.libs.json.{Format, Json, OFormat}
+import routing.Version
 import uk.gov.hmrc.auth.core.ConfidenceLevel
 import utils.enums.Enums
 
@@ -46,10 +47,7 @@ object APIStatus extends Enumeration {
   val parser: PartialFunction[String, APIStatus]   = Enums.parser[APIStatus]
 }
 
-case class APIVersion(version: String, status: APIStatus, endpointsEnabled: Boolean) {
-
-  require(version.nonEmpty, "version is required")
-}
+case class APIVersion(version: Version, status: APIStatus, endpointsEnabled: Boolean)
 
 object APIVersion {
   implicit val formatAPIVersion: OFormat[APIVersion] = Json.format[APIVersion]
@@ -70,8 +68,7 @@ case class APIDefinition(name: String,
   require(uniqueVersions, "version numbers must be unique")
 
   private def uniqueVersions: Boolean = {
-    val foundVersions: Seq[String] = versions.map(_.version)
-    foundVersions.distinct == foundVersions
+    !versions.map(_.version).groupBy(identity).view.mapValues(_.size).exists(_._2 > 1)
   }
 
 }

diff --git a/app/definition/ApiDefinitionFactory.scala b/app/definition/ApiDefinitionFactory.scala
@@ -17,8 +17,8 @@
 package definition
 
 import config.AppConfig
-import definition.Versions._
 import play.api.Logger
+import routing.{Version, Version1}
 import uk.gov.hmrc.auth.core.ConfidenceLevel
 
 import javax.inject.{Inject, Singleton}
@@ -59,16 +59,16 @@ class ApiDefinitionFactory @Inject() (appConfig: AppConfig) {
         categories = Seq("INCOME_TAX_MTD"),
         versions = Seq(
           APIVersion(
-            version = VERSION_1,
-            status = buildAPIStatus(VERSION_1),
-            endpointsEnabled = appConfig.endpointsEnabled(VERSION_1)
+            version = Version1,
+            status = buildAPIStatus(Version1),
+            endpointsEnabled = appConfig.endpointsEnabled(Version1)
           )
         ),
         requiresTrust = None
       )
     )
 
-  private[definition] def buildAPIStatus(version: String): APIStatus = {
+  private[definition] def buildAPIStatus(version: Version): APIStatus = {
     APIStatus.parser
       .lift(appConfig.apiStatus(version))
       .getOrElse {

diff --git a/app/definition/Versions.scala b/app/definition/Versions.scala
diff --git a/app/routing/VersionRoutingMap.scala b/app/routing/VersionRoutingMap.scala
@@ -17,10 +17,8 @@
 package routing
 
 import com.google.inject.ImplementedBy
-import config.{AppConfig, FeatureSwitches}
-import definition.Versions.VERSION_1
+import config.AppConfig
 import play.api.routing.Router
-import utils.Logging
 
 import javax.inject.Inject
 
@@ -31,25 +29,16 @@ import javax.inject.Inject
 trait VersionRoutingMap {
   val defaultRouter: Router
 
-  val map: Map[String, Router]
+  val map: Map[Version, Router]
 
-  final def versionRouter(version: String): Option[Router] = map.get(version)
+  final def versionRouter(version: Version): Option[Router] = map.get(version)
 }
 
 // Add routes corresponding to available versions...
-case class VersionRoutingMapImpl @Inject() (appConfig: AppConfig,
-                                            defaultRouter: Router,
-                                            v1Router: v1.Routes,
-                                            v1RouterWithMarriageAllowance: v1WithMarriageAllowance.Routes)
-    extends VersionRoutingMap
-    with Logging {
-
-  val featureSwitch: FeatureSwitches = FeatureSwitches(appConfig.featureSwitches)
-
-  val map: Map[String, Router] = Map(
-    VERSION_1 -> {
-      if (featureSwitch.isMarriageAllowanceRoutingEnabled) v1RouterWithMarriageAllowance else v1Router
-    }
+case class VersionRoutingMapImpl @Inject() (appConfig: AppConfig, defaultRouter: Router, v1Router: v1.Routes) extends VersionRoutingMap {
+
+  val map: Map[Version, Router] = Map(
+    Version1 -> v1Router
   )
 
 }
diff --git a/app/routing/VersionRoutingRequestHandler.scala b/app/routing/VersionRoutingRequestHandler.scala
@@ -16,9 +16,8 @@
 
 package routing
 
-import api.models.errors.{InvalidAcceptHeaderError, UnsupportedVersionError}
-import config.{AppConfig, FeatureSwitches}
-import definition.Versions
+import api.models.errors.{InvalidAcceptHeaderError, NotFoundError, UnsupportedVersionError}
+import config.AppConfig
 import play.api.http.{DefaultHttpRequestHandler, HttpConfiguration, HttpErrorHandler, HttpFilters}
 import play.api.libs.json.Json
 import play.api.mvc.{DefaultActionBuilder, Handler, RequestHeader, Results}
@@ -43,30 +42,43 @@ class VersionRoutingRequestHandler @Inject() (versionRoutingMap: VersionRoutingM
       filters = filters.filters
     ) {
 
-  private val featureSwitch = FeatureSwitches(config.featureSwitches)
-
   private val unsupportedVersionAction = action(Results.NotFound(Json.toJson(UnsupportedVersionError)))
 
+  private val resourceNotFoundAction = action(Results.NotFound(NotFoundError.asJson))
+
   private val invalidAcceptHeaderError = action(Results.NotAcceptable(Json.toJson(InvalidAcceptHeaderError)))
 
   override def routeRequest(request: RequestHeader): Option[Handler] = {
 
-    def documentHandler: Option[Handler] = routeWith(versionRoutingMap.defaultRouter)(request)
+    def documentHandler: Option[Handler] = routeWith(versionRoutingMap.defaultRouter, request)
 
-    def apiHandler: Option[Handler] = Versions.getFromRequest(request) match {
-      case Some(version) =>
-        versionRoutingMap.versionRouter(version) match {
-          case Some(versionRouter) if featureSwitch.isVersionEnabled(version) => routeWith(versionRouter)(request)
-          case Some(_)                                                        => Some(unsupportedVersionAction)
-          case None                                                           => Some(unsupportedVersionAction)
-        }
-      case None => Some(invalidAcceptHeaderError)
-    }
+    def apiHandler: Option[Handler] = Some(
+      Versions.getFromRequest(request) match {
+        case Left(InvalidHeader)   => invalidAcceptHeaderError
+        case Left(VersionNotFound) => unsupportedVersionAction
+
+        case Right(version) => findRoute(request, version) getOrElse resourceNotFoundAction
+      }
+    )
 
     documentHandler orElse apiHandler
   }
 
-  private def routeWith(router: Router)(request: RequestHeader): Option[Handler] =
+  private def findRoute(request: RequestHeader, version: Version): Option[Handler] = {
+    val found =
+      if (config.endpointsEnabled(version)) {
+        versionRoutingMap
+          .versionRouter(version)
+          .flatMap(router => routeWith(router, request))
+      } else {
+        Some(unsupportedVersionAction)
+      }
+
+    found
+      .orElse(version.maybePrevious.flatMap(previousVersion => findRoute(request, previousVersion)))
+  }
+
+  private def routeWith(router: Router, request: RequestHeader): Option[Handler] =
     router
       .handlerFor(request)
       .orElse {

diff --git a/app/routing/Versions.scala b/app/routing/Versions.scala
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2023 HM Revenue & Customs
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package routing
+
+import play.api.http.HeaderNames.ACCEPT
+import play.api.libs.json._
+import play.api.mvc.RequestHeader
+
+object Version {
+
+  object VersionWrites extends Writes[Version] {
+
+    def writes(version: Version): JsValue = version match {
+      case Version1 => Json.toJson(Version1.name)
+    }
+
+  }
+
+  object VersionReads extends Reads[Version] {
+
+    override def reads(version: JsValue): JsResult[Version] =
+      version.validate[String].flatMap {
+        case Version1.name => JsSuccess(Version1)
+        case _             => JsError("Unrecognised version")
+      }
+
+  }
+
+  implicit val versionFormat: Format[Version] = Format(VersionReads, VersionWrites)
+}
+
+sealed trait Version {
+  val name: String
+  val configName: String
+  val maybePrevious: Option[Version] = None
+
+  override def toString: String = name
+}
+
+case object Version1 extends Version {
+  val name       = "1.0"
+  val configName = "1"
+}
+
+object Versions {
+
+  private val versionsByName: Map[String, Version] = Map(
+    Version1.name -> Version1
+  )
+
+  private val versionRegex = """application/vnd.hmrc.(\d.\d)\+json""".r
+
+  def getFromRequest(request: RequestHeader): Either[GetFromRequestError, Version] =
+    for {
+      str <- getFrom(request.headers.headers)
+      ver <- getFrom(str)
+    } yield ver
+
+  private def getFrom(headers: Seq[(String, String)]): Either[GetFromRequestError, String] =
+    headers.collectFirst { case (ACCEPT, versionRegex(ver)) => ver }.toRight(left = InvalidHeader)
+
+  private def getFrom(name: String): Either[GetFromRequestError, Version] =
+    versionsByName.get(name).toRight(left = VersionNotFound)
+
+}
+
+sealed trait GetFromRequestError
+case object InvalidHeader   extends GetFromRequestError
+case object VersionNotFound extends GetFromRequestError