Skip to content

Commit

Permalink
CDD-1548: Simplify Exports config service by hard coding config items… (
Browse files Browse the repository at this point in the history
#32)

* CDD-1548: Simplify Exports config service by hard coding config items that are not going to change.

* CDD-1548: Review feedback.
  • Loading branch information
googley42 authored May 23, 2018
1 parent 9aa7582 commit de5dc68
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ class DocumentationController @Inject()(httpErrorHandler: HttpErrorHandler, conf
extends uk.gov.hmrc.api.controllers.DocumentationController(httpErrorHandler) {

override def definition(): Action[AnyContent] = Action {
Ok(txt.definition(configService.apiDefinitionConfig)).withHeaders(CONTENT_TYPE -> JSON)
Ok(txt.definition()).withHeaders(CONTENT_TYPE -> JSON)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ class HeaderValidator @Inject()(logger: ExportsLogger) {
def validateHeaders[A](implicit conversationIdRequest: ConversationIdRequest[A]): Either[ErrorResponse, ExtractedHeadersImpl] = {
implicit val headers: Headers = conversationIdRequest.headers

def hasAccept = validateHeader(ACCEPT, validAcceptHeaders.contains(_), ErrorAcceptHeaderInvalid)
def hasAccept: Either[ErrorResponse, String] = validateHeader(ACCEPT, validAcceptHeaders.contains(_), ErrorAcceptHeaderInvalid)

def hasContentType = validateHeader(CONTENT_TYPE, s => validContentTypeHeaders.contains(s.toLowerCase()), ErrorContentTypeHeaderInvalid)
def hasContentType: Either[ErrorResponse, String] = validateHeader(CONTENT_TYPE, s => validContentTypeHeaders.contains(s.toLowerCase()), ErrorContentTypeHeaderInvalid)

def hasXClientId = validateHeader(XClientIdHeaderName, xClientIdRegex.findFirstIn(_).nonEmpty, ErrorInternalServerError)
def hasXClientId: Either[ErrorResponse, String] = validateHeader(XClientIdHeaderName, xClientIdRegex.findFirstIn(_).nonEmpty, ErrorInternalServerError)

val theResult: Either[ErrorResponse, ExtractedHeadersImpl] = for {
acceptValue <- hasAccept.right
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@

package uk.gov.hmrc.customs.inventorylinking.export.model

case class ApiDefinitionConfig(apiContext: String, apiScope: String, whitelistedApplicationIds: Seq[String])

case class ExportsEnrolmentConfig(customsEnrolmentName: String, eoriIdentifierName: String)

trait ExportsConfig {
val apiDefinitionConfig: ApiDefinitionConfig
val exportsEnrolmentConfig: ExportsEnrolmentConfig
val apiSubscriptionFieldsBaseUrl: String
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class BusinessService @Inject()(logger: ExportsLogger,
uniqueIdsService: UniqueIdsService,
customsConfigService: ExportsConfigService) {

private val apiContextEncoded = URLEncoder.encode(customsConfigService.apiDefinitionConfig.apiContext, "UTF-8")
private val apiContextEncoded = URLEncoder.encode("customs/inventory-linking/exports", "UTF-8")

def send[A](implicit vpr: ValidatedPayloadRequest[A], hc: HeaderCarrier): Future[Either[Result, Unit]] = {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,17 @@ import javax.inject.{Inject, Singleton}
import play.api.Configuration
import uk.gov.hmrc.customs.api.common.config.ConfigValidationNelAdaptor
import uk.gov.hmrc.customs.inventorylinking.export.logging.ExportsLogger
import uk.gov.hmrc.customs.inventorylinking.export.model.{ApiDefinitionConfig, ExportsEnrolmentConfig, ExportsConfig}
import uk.gov.hmrc.customs.inventorylinking.export.model.ExportsConfig

import scalaz.ValidationNel
import scalaz.syntax.apply._
import scalaz.syntax.traverse._

@Singleton
class ExportsConfigService @Inject()(configuration: Configuration,
configValidationNel: ConfigValidationNelAdaptor,
logger: ExportsLogger) extends ExportsConfig {

private val root = configValidationNel.root

private val validatedDefinitionConfig: ValidationNel[String, ApiDefinitionConfig] = (
root.string("inventory-linking.definition.api-context") |@|
root.string("inventory-linking.definition.api-scope") |@|
getStringSeq("api.access.version-1.0.whitelistedApplicationIds")
) (ApiDefinitionConfig.apply)

private val validatedCustomsEnrolmentConfig: ValidationNel[String, ExportsEnrolmentConfig] = (
root.string("inventory-linking.enrolment.name") |@|
root.string("inventory-linking.enrolment.eori-identifier")
) (ExportsEnrolmentConfig.apply)

private val exportsConfig = (
validatedDefinitionConfig |@|
validatedCustomsEnrolmentConfig |@|
configValidationNel.service("api-subscription-fields").serviceUrl
) (ExportsConfigImpl.apply) fold(
private val exportsConfig =
configValidationNel.service("api-subscription-fields").serviceUrl.map(ExportsConfigImpl.apply) fold(
fail = { nel =>
val errorMsg = nel.toList.mkString("\n", "\n", "")
logger.errorWithoutRequestContext(errorMsg)
Expand All @@ -58,17 +40,8 @@ class ExportsConfigService @Inject()(configuration: Configuration,
succ = identity
)

val apiDefinitionConfig: ApiDefinitionConfig = exportsConfig.apiDefinitionConfig

val exportsEnrolmentConfig: ExportsEnrolmentConfig = exportsConfig.exportsEnrolmentConfig

val apiSubscriptionFieldsBaseUrl: String = exportsConfig.apiSubscriptionFieldsBaseUrl

private case class ExportsConfigImpl(apiDefinitionConfig: ApiDefinitionConfig,
exportsEnrolmentConfig: ExportsEnrolmentConfig,
apiSubscriptionFieldsBaseUrl: String)

private def getStringSeq(configKey: String): ValidationNel[String, Seq[String]] =
scalaz.Success(configuration.getStringSeq(configKey).getOrElse(Nil))
private case class ExportsConfigImpl(apiSubscriptionFieldsBaseUrl: String) extends ExportsConfig

}
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
@import play.api.libs.json.Json
@import uk.gov.hmrc.customs.inventorylinking.export.model.ApiDefinitionConfig
@(apiDefinitionConfig: ApiDefinitionConfig)
@()
{
"scopes": [
{
"key": "@apiDefinitionConfig.apiScope",
"key": "write:customs-inventory-linking-exports",
"name": "Inventory Exports Movement Request",
"description": "Submit an Inventory Exports Movement Request"
}
],
"api": {
"name": "Customs Inventory Linking Exports",
"description": "Customs Inventory Linking Exports",
"context": "@apiDefinitionConfig.apiContext",
"context": "customs/inventory-linking/exports",
"versions": [
{
"version": "1.0",
Expand Down
5 changes: 0 additions & 5 deletions conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ application.logger.name = ${appName}
httpHeadersWhitelist += "X-Client-ID"
httpHeadersWhitelist += "X-Badge-Identifier"

inventory-linking.definition.api-context = "customs/inventory-linking/exports"
inventory-linking.definition.api-scope = "write:customs-inventory-linking-exports"
inventory-linking.enrolment.name = "HMRC-CUS-ORG"
inventory-linking.enrolment.eori-identifier = "EORINumber"

xsd.locations += "/api/conf/1.0/schemas/exports/request/inventoryLinkingRequestExternal.xsd"

xml.max-errors = 25
Expand Down
11 changes: 1 addition & 10 deletions test/unit/controllers/InventoryLinkingExportControllerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ import uk.gov.hmrc.customs.api.common.controllers.ErrorResponse.errorBadRequest
import uk.gov.hmrc.customs.inventorylinking.export.controllers.actionbuilders._
import uk.gov.hmrc.customs.inventorylinking.export.controllers.{HeaderValidator, InventoryLinkingExportController}
import uk.gov.hmrc.customs.inventorylinking.export.logging.ExportsLogger
import uk.gov.hmrc.customs.inventorylinking.export.model._
import uk.gov.hmrc.customs.inventorylinking.export.model.actionbuilders.ValidatedPayloadRequest
import uk.gov.hmrc.customs.inventorylinking.export.services.{BusinessService, ExportsConfigService, XmlValidationService}
import uk.gov.hmrc.customs.inventorylinking.export.services.{BusinessService, XmlValidationService}
import uk.gov.hmrc.http.HeaderCarrier
import uk.gov.hmrc.play.test.UnitSpec
import util.AuthConnectorStubbing
Expand Down Expand Up @@ -69,19 +68,11 @@ class InventoryLinkingExportControllerSpec extends UnitSpec
controller.post().apply(request)
}

when(mockApiDefinitionConfig.apiScope).thenReturn(apiScope)
when(mockXmlValidationService.validate(any[NodeSeq])(any[ExecutionContext])).thenReturn(Future.successful(()))
when(mockBusinessService.send(any[ValidatedPayloadRequest[_]], any[HeaderCarrier])).thenReturn(Future.successful(Right(())))
}


private val apiScope = "write:customs-inventory-linking-exports"
// "scope-in-api-definition"
private val customsEnrolmentName = "HMRC-CUS-ORG"
private val eoriIdentifier = "EORINumber"
private val mockApiDefinitionConfig = mock[ApiDefinitionConfig]
private val exportsEnrolmentConfig = ExportsEnrolmentConfig(customsEnrolmentName, eoriIdentifier)

private val errorResultEoriNotFoundInCustomsEnrolment = ErrorResponse(UNAUTHORIZED, errorCode = "UNAUTHORIZED",
message = "EORI number not found in Customs Enrolment").XmlResult.withHeaders(X_CONVERSATION_ID_HEADER)

Expand Down
3 changes: 0 additions & 3 deletions test/unit/services/BusinessServiceSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class BusinessServiceSpec extends UnitSpec with MockitoSugar with BeforeAndAfter
protected val mockPayloadDecorator: MdgPayloadDecorator = mock[MdgPayloadDecorator]
protected val mockDateTimeProvider: DateTimeService = mock[DateTimeService]
protected val mockCustomsConfigService: ExportsConfigService = mock[ExportsConfigService]
protected val mockApiDefinitionConfig: ApiDefinitionConfig = mock[ApiDefinitionConfig]
protected val mockHttpResponse: HttpResponse = mock[HttpResponse]

protected lazy val service: BusinessService = new BusinessService(mockLogger, mockMdgExportsConnector, mockApiSubscriptionFieldsConnector,
Expand All @@ -71,8 +70,6 @@ class BusinessServiceSpec extends UnitSpec with MockitoSugar with BeforeAndAfter
// type of the value contained in the value class i.e. for CorrelationId the value is UUID so needs to meq type of UUID
when(mockPayloadDecorator.decorate(meq(TestXmlPayload), meq[String](TestSubscriptionFieldsId.value).asInstanceOf[SubscriptionFieldsId], meq[UUID](correlationIdUuid).asInstanceOf[CorrelationId], any[DateTime])(any[ValidatedPayloadRequest[_]])).thenReturn(wrappedValidXML)
when(mockDateTimeProvider.getUtcNow).thenReturn(dateTime)
when(mockApiDefinitionConfig.apiContext).thenReturn("customs/inventory-linking/exports")
when(mockCustomsConfigService.apiDefinitionConfig).thenReturn(mockApiDefinitionConfig)
when(mockMdgExportsConnector.send(any[NodeSeq], meq(dateTime), any[UUID])(any[ValidatedPayloadRequest[_]])).thenReturn(mockHttpResponse)
when(mockApiSubscriptionFieldsConnector.getSubscriptionFields(any[ApiSubscriptionKey])(any[ValidatedPayloadRequest[_]], any[HeaderCarrier])).thenReturn(Future.successful(apiSubscriptionFieldsResponse))
}
Expand Down
12 changes: 1 addition & 11 deletions test/unit/services/ExportsConfigServiceSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,13 @@ import org.scalatest.mockito.MockitoSugar
import play.api.{Configuration, Environment}
import uk.gov.hmrc.customs.api.common.config.{ConfigValidationNelAdaptor, ServicesConfig}
import uk.gov.hmrc.customs.inventorylinking.export.logging.ExportsLogger
import uk.gov.hmrc.customs.inventorylinking.export.model.{ApiDefinitionConfig, ExportsConfig, ExportsEnrolmentConfig}
import uk.gov.hmrc.customs.inventorylinking.export.model.ExportsConfig
import uk.gov.hmrc.customs.inventorylinking.export.services.ExportsConfigService
import uk.gov.hmrc.play.test.UnitSpec

class ExportsConfigServiceSpec extends UnitSpec with MockitoSugar {
private val validAppConfig: Config = ConfigFactory.parseString(
"""
|inventory-linking.definition.api-context = some-api-context
|inventory-linking.definition.api-scope = some-api-scope
|inventory-linking.enrolment.name = some-enrolment-name
|inventory-linking.enrolment.eori-identifier = some-eori
|api.access.version-1.0.whitelistedApplicationIds.0 = someId-1
|api.access.version-1.0.whitelistedApplicationIds.1 = someId-2
|microservice.services.api-subscription-fields.host=some-host
Expand All @@ -52,17 +48,11 @@ class ExportsConfigServiceSpec extends UnitSpec with MockitoSugar {
"return config as object model when configuration is valid" in {
val configService = customsConfigService(validServicesConfiguration)

configService.apiDefinitionConfig shouldBe ApiDefinitionConfig("some-api-context", "some-api-scope", Seq("someId-1", "someId-2"))
configService.exportsEnrolmentConfig shouldBe ExportsEnrolmentConfig("some-enrolment-name", "some-eori")
configService.apiSubscriptionFieldsBaseUrl shouldBe "http://some-host:1111/some-context"
}

"throw an exception when configuration is invalid, that contains AGGREGATED error messages" in {
val expectedErrorMessage =
"\nCould not find config key 'inventory-linking.definition.api-context'" +
"\nCould not find config key 'inventory-linking.definition.api-scope'" +
"\nCould not find config key 'inventory-linking.enrolment.name'" +
"\nCould not find config key 'inventory-linking.enrolment.eori-identifier'" +
"\nCould not find config api-subscription-fields.host" +
"\nService configuration not found for key: api-subscription-fields.context"

Expand Down

0 comments on commit de5dc68

Please sign in to comment.