From 48963f276426f3e661cb67b4ceea0e0baa9764d2 Mon Sep 17 00:00:00 2001 From: John Duffell Date: Tue, 30 Oct 2018 17:12:43 +0000 Subject: [PATCH 1/8] when linking existing subs, create guest accounts if no identity --- .../gu/cancellation/sf_cases/Handler.scala | 14 ++-- .../sf_cases/EndToEndHandlerEffectsTest.scala | 2 +- .../com/gu/identity/CreateGuestAccount.scala | 61 +++++++++++++++++ .../scala/com/gu/identity/GetByEmail.scala | 45 +++++-------- .../com/gu/identityBackfill/Handler.scala | 63 ++++++++++++++---- .../IdentityBackfillSteps.scala | 40 +++++------- .../com/gu/identityBackfill/PreReqCheck.scala | 30 ++++----- .../CreateGuestAccountEffectsTest.scala | 60 +++++++++++++++++ .../gu/identity/CreateGuestAccountTest.scala | 26 ++++++++ .../gu/identity/GetByEmailEffectsTest.scala | 14 ++-- .../com/gu/identity/GetByEmailTest.scala | 62 ++++-------------- .../EndToEndHandlerTest.scala | 8 ++- .../gu/identityBackfill/PreReqCheckTest.scala | 37 ++++++----- .../com/gu/identityBackfill/StepsTest.scala | 30 +++++---- ...tSFContactSyncCheckFieldsEffectsTest.scala | 2 +- ...pdateSalesforceIdentityIdEffectsTest.scala | 4 +- .../com/gu/sf_contact_merge/Handler.scala | 4 +- .../GetSfAddressEffectsTest.scala | 2 +- ...pdateSalesforceIdentityIdEffectsTest.scala | 4 +- .../scala/com/gu/util/resthttp/HttpOp.scala | 16 +++-- .../gu/util/resthttp/RestRequestMaker.scala | 7 ++ .../scala/com/gu/salesforce/JsonHttp.scala | 65 ++++++++++--------- .../com/gu/salesforce/SalesforceClient.scala | 4 +- 23 files changed, 386 insertions(+), 214 deletions(-) create mode 100644 handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala create mode 100644 handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala create mode 100644 handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountTest.scala diff --git a/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala b/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala index 595e65b6f0..b2c3603c87 100644 --- a/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala +++ b/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala @@ -160,11 +160,11 @@ object Handler extends Logging { (for { identityAndSfRequests <- sfBackendForIdentityCookieHeader(apiGatewayRequest.headers) raiseCaseDetail <- apiGatewayRequest.bodyAsCaseClass[RaiseCaseDetail]() - lookupByIdOp = SalesforceGenericIdLookup(identityAndSfRequests.sfClient.wrap(JsonHttp.get)) - mostRecentCaseOp = SalesforceCase.GetMostRecentCaseByContactId(identityAndSfRequests.sfClient.wrap(JsonHttp.get)) - createCaseOp = SalesforceCase.Create(identityAndSfRequests.sfClient.wrap(JsonHttp.post)) + lookupByIdOp = SalesforceGenericIdLookup(identityAndSfRequests.sfClient.wrapWith(JsonHttp.get)) + mostRecentCaseOp = SalesforceCase.GetMostRecentCaseByContactId(identityAndSfRequests.sfClient.wrapWith(JsonHttp.get)) + createCaseOp = SalesforceCase.Create(identityAndSfRequests.sfClient.wrapWith(JsonHttp.post)) wiredCreateCaseOp = buildWireNewCaseForSalesforce.tupled andThen createCaseOp - sfUpdateOp = SalesforceCase.Update(identityAndSfRequests.sfClient.wrap(JsonHttp.patch)) + sfUpdateOp = SalesforceCase.Update(identityAndSfRequests.sfClient.wrapWith(JsonHttp.patch)) updateReasonOnRecentCaseOp = updateCaseReason(sfUpdateOp)_ newOrResumeCaseOp = newOrResumeCase(wiredCreateCaseOp, updateReasonOnRecentCaseOp, raiseCaseDetail)_ wiredRaiseCase = raiseCase(lookupByIdOp, mostRecentCaseOp, newOrResumeCaseOp)_ @@ -214,11 +214,11 @@ object Handler extends Logging { (for { identityAndSfRequests <- sfBackendForIdentityCookieHeader(apiGatewayRequest.headers) pathParams <- apiGatewayRequest.pathParamsAsCaseClass[CasePathParams]() - lookupByIdOp = SalesforceGenericIdLookup(identityAndSfRequests.sfClient.wrap(JsonHttp.get)) - getCaseByIdOp = SalesforceCase.GetById[CaseWithContactId](identityAndSfRequests.sfClient.wrap(JsonHttp.get))_ + lookupByIdOp = SalesforceGenericIdLookup(identityAndSfRequests.sfClient.wrapWith(JsonHttp.get)) + getCaseByIdOp = SalesforceCase.GetById[CaseWithContactId](identityAndSfRequests.sfClient.wrapWith(JsonHttp.get))_ _ <- verifyCaseBelongsToUser(lookupByIdOp, getCaseByIdOp)(identityAndSfRequests.identityUser.id, pathParams.caseId) requestBody <- apiGatewayRequest.bodyAsCaseClass[JsValue]() - sfUpdateOp = SalesforceCase.Update(identityAndSfRequests.sfClient.wrap(JsonHttp.patch)) + sfUpdateOp = SalesforceCase.Update(identityAndSfRequests.sfClient.wrapWith(JsonHttp.patch)) _ <- sfUpdateOp(pathParams.caseId, requestBody).toApiGatewayOp("update case") } yield ApiGatewayResponse.successfulExecution).apiResponse diff --git a/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala b/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala index 0a68cd3d5c..1cc178ff04 100644 --- a/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala +++ b/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala @@ -29,7 +29,7 @@ class EndToEndHandlerEffectsTest extends FlatSpec with Matchers { (for { identityAndSfRequests <- sfBackendForIdentityCookieHeader(apiGatewayRequest.headers) pathParams <- apiGatewayRequest.pathParamsAsCaseClass[CasePathParams]() - sfGet = SalesforceCase.GetById[JsValue](identityAndSfRequests.sfClient.wrap(JsonHttp.get))_ + sfGet = SalesforceCase.GetById[JsValue](identityAndSfRequests.sfClient.wrapWith(JsonHttp.get))_ getCaseResponse <- sfGet(pathParams.caseId).toApiGatewayOp("get case detail") } yield ApiGatewayResponse("200", getCaseResponse)).apiResponse diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala new file mode 100644 index 0000000000..17682658cf --- /dev/null +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala @@ -0,0 +1,61 @@ +package com.gu.identity + +import com.gu.identityBackfill.Types.EmailAddress +import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId +import com.gu.salesforce.SalesforceClient.StringHttpRequest +import com.gu.util.resthttp.HttpOp.HttpOpWrapper +import com.gu.util.resthttp.RestRequestMaker._ +import com.gu.util.resthttp.{HttpOp, RestRequestMaker} +import okhttp3.{HttpUrl, Request, Response} +import play.api.libs.json.{JsValue, Json, Reads} + +object CreateGuestAccount { + + case class WireGuestRegistrationResponse( + userId: String + ) + implicit val reads = Json.reads[WireGuestRegistrationResponse] + + case class WireGuestRegistrationRequest( + primaryEmailAddress: String //, + // privateFields: ,all optional + // publicFields: req-displayName only + ) + implicit val writes = Json.writes[WireGuestRegistrationRequest] + + case class WireIdentityResponse(status: String, guestRegistrationRequest: WireGuestRegistrationResponse) + implicit val userResponseReads: Reads[WireIdentityResponse] = Json.reads[WireIdentityResponse] + + def toRequest(emailAddress: EmailAddress): PostRequest = + PostRequest(WireGuestRegistrationRequest(emailAddress.value), RelativePath("/guest")) + + def toResponse(wireGuestRegistrationResponse: WireIdentityResponse): IdentityId = + IdentityId(wireGuestRegistrationResponse.guestRegistrationRequest.userId) + + val wrapper: HttpOpWrapper[EmailAddress, PostRequest, JsValue, IdentityId] = + HttpOpWrapper[EmailAddress, PostRequest, JsValue, IdentityId](toRequest, RestRequestMaker.toResult[WireIdentityResponse](_).map(toResponse)) + +} + +object IdentityClient { + + def apply( + response: Request => Response, + config: IdentityConfig + ): HttpOp[StringHttpRequest, BodyAsString] = + HttpOp(response).flatMap { + toClientFailableOp + }.setupRequest[StringHttpRequest] { + withAuth(config) + } + + def withAuth(identityConfig: IdentityConfig)(requestInfo: StringHttpRequest): Request = { + val builder = requestInfo.requestMethod.builder + val authedBuilder = builder.addHeader("X-GU-ID-Client-Access-Token", s"Bearer ${identityConfig.apiToken}") + val url = requestInfo.urlParams.value.foldLeft(HttpUrl.parse(identityConfig.baseUrl + requestInfo.relativePath.value).newBuilder()) { + case (nextBuilder, (key, value)) => nextBuilder.addQueryParameter(key, value) + }.build() + authedBuilder.url(url).build() + } + +} diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala index ea8901cc16..faac3dc40c 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala @@ -4,17 +4,17 @@ import com.gu.identity.GetByEmail.RawWireModel.{User, UserResponse} import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.util.config.ConfigLocation -import okhttp3.{HttpUrl, Request, Response} -import play.api.libs.json.{Json, Reads} -import scalaz.syntax.std.either._ -import scalaz.{-\/, \/, \/-} +import com.gu.util.resthttp.HttpOp.HttpOpWrapper +import com.gu.util.resthttp.RestRequestMaker +import com.gu.util.resthttp.RestRequestMaker.{GetRequestWithParams, RelativePath, UrlParams} +import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess, GenericError} +import play.api.libs.json.{JsValue, Json, Reads} object GetByEmail { - sealed trait ApiError - case class OtherError(message: String) extends ApiError - case object NotFound extends ApiError - case object NotValidated extends ApiError + sealed trait MaybeValidatedEmail + case class ValidatedEmail(identityId: IdentityId) extends MaybeValidatedEmail + case object NotValidated extends MaybeValidatedEmail object RawWireModel { @@ -29,31 +29,22 @@ object GetByEmail { } - def identityIdFromUser(user: User) = - IdentityId(user.id) + val wrapper: HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, MaybeValidatedEmail] = + HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, MaybeValidatedEmail]( + emailAddress => GetRequestWithParams(RelativePath(s"/user"), UrlParams(Map("emailAddress" -> emailAddress.value))), + RestRequestMaker.toResult[UserResponse](_).flatMap(toResponse) + ) - def userFromResponse(userResponse: UserResponse): ApiError \/ User = + def userFromResponse(userResponse: UserResponse): ClientFailableOp[User] = userResponse match { - case UserResponse("ok", user) => \/-(user) - case _ => -\/(OtherError("not an OK response from api")) + case UserResponse("ok", user) => ClientSuccess(user) + case _ => GenericError("not an OK response from api") } - def apply(getResponse: Request => Response, identityConfig: IdentityConfig)(email: EmailAddress): ApiError \/ IdentityId = { - - val url = HttpUrl.parse(identityConfig.baseUrl + "/user").newBuilder().addQueryParameter("emailAddress", email.value).build() - val response = getResponse(new Request.Builder().url(url).addHeader("X-GU-ID-Client-Access-Token", "Bearer " + identityConfig.apiToken).build()) - + def toResponse(userResponse: UserResponse) = { for { - _ <- response.code match { - case 200 => \/-(()) - case 404 => -\/(NotFound) - case code => -\/(OtherError(s"failed http with ${code}")) - } - body = response.body.byteStream - userResponse <- Json.parse(body).validate[UserResponse].asEither.disjunction.leftMap(err => OtherError(err.mkString(", "))) user <- userFromResponse(userResponse) - _ <- if (user.statusFields.userEmailValidated) \/-(()) else -\/(NotValidated) - identityId = identityIdFromUser(user) + identityId = if (user.statusFields.userEmailValidated) ValidatedEmail(IdentityId(user.id)) else NotValidated } yield identityId } diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala index 8f4b516f58..c684222f9f 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala @@ -4,9 +4,11 @@ import java.io.{InputStream, OutputStream} import com.amazonaws.services.lambda.runtime.Context import com.gu.effects.{GetFromS3, RawEffects} -import com.gu.identity.{GetByEmail, IdentityConfig} +import com.gu.identity.{CreateGuestAccount, GetByEmail, IdentityClient, IdentityConfig} +import com.gu.identityBackfill.IdentityBackfillSteps.DomainRequest import com.gu.identityBackfill.TypeConvert._ import com.gu.identityBackfill.Types.EmailAddress +import com.gu.identityBackfill.WireRequestToDomainObject.WireModel.IdentityBackfillRequest import com.gu.identityBackfill.salesforce.ContactSyncCheck.RecordTypeId import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.identityBackfill.salesforce._ @@ -17,17 +19,17 @@ import com.gu.salesforce.TypesForSFEffectsData.SFContactId import com.gu.salesforce.{JsonHttp, SalesforceClient} import com.gu.util.apigateway.ApiGatewayHandler.{LambdaIO, Operation} import com.gu.util.apigateway.ResponseModels.ApiResponse -import com.gu.util.apigateway.{ApiGatewayHandler, ApiGatewayResponse} +import com.gu.util.apigateway.{ApiGatewayHandler, ApiGatewayRequest, ApiGatewayResponse, ResponseModels} import com.gu.util.config.LoadConfigModule.StringFromS3 import com.gu.util.config.{LoadConfigModule, Stage} +import com.gu.util.reader.Types.ApiGatewayOp.{ContinueProcessing, ReturnWithResponse} import com.gu.util.reader.Types._ import com.gu.util.resthttp.RestRequestMaker.{GetRequest, PatchRequest} import com.gu.util.resthttp.Types.ClientFailableOp import com.gu.util.resthttp.{HttpOp, LazyClientFailableOp, RestRequestMaker} import com.gu.util.zuora.{ZuoraQuery, ZuoraRestConfig, ZuoraRestRequestMaker} import okhttp3.{Request, Response} -import play.api.libs.json.JsValue -import scalaz.\/ +import play.api.libs.json.{JsValue, Json, Reads} object Handler { @@ -53,25 +55,28 @@ object Handler { ) = { val zuoraRequests = ZuoraRestRequestMaker(response, zuoraRestConfig) val zuoraQuerier = ZuoraQuery(zuoraRequests) - val getByEmail: EmailAddress => GetByEmail.ApiError \/ IdentityId = GetByEmail(response, identityConfig) + val identityClient = IdentityClient(response, identityConfig) + val createGuestAccount = identityClient.wrapWith(JsonHttp.post).wrapWith(CreateGuestAccount.wrapper) + val getByEmail = identityClient.wrapWith(JsonHttp.getWithParams).wrapWith(GetByEmail.wrapper) val countZuoraAccounts: IdentityId => ClientFailableOp[Int] = CountZuoraAccountsForIdentityId(zuoraQuerier) lazy val sfAuth: LazyClientFailableOp[HttpOp[StringHttpRequest, RestRequestMaker.BodyAsString]] = SalesforceClient(response, sfConfig) - lazy val sfPatch = sfAuth.map(_.wrap(JsonHttp.patch)) - lazy val sfGet = sfAuth.map(_.wrap(JsonHttp.get)) + lazy val sfPatch = sfAuth.map(_.wrapWith(JsonHttp.patch)) + lazy val sfGet = sfAuth.map(_.wrapWith(JsonHttp.get)) Operation( - steps = IdentityBackfillSteps( + steps = WireRequestToDomainObject(IdentityBackfillSteps( PreReqCheck( - getByEmail, + getByEmail.runRequest, GetZuoraAccountsForEmail(zuoraQuerier) _ andThen PreReqCheck.getSingleZuoraAccountForEmail, countZuoraAccounts andThen PreReqCheck.noZuoraAccountsForIdentityId, GetZuoraSubTypeForAccount(zuoraQuerier) _ andThen PreReqCheck.acceptableReaderType, syncableSFToIdentity(sfGet, stage) ), + createGuestAccount.runRequest, AddIdentityIdToAccount(zuoraRequests), updateSalesforceIdentityId(sfPatch) - ), + )), healthcheck = () => Healthcheck( getByEmail, countZuoraAccounts, @@ -129,15 +134,49 @@ object Handler { object Healthcheck { def apply( - getByEmail: EmailAddress => \/[GetByEmail.ApiError, IdentityId], + getByEmail: HttpOp[EmailAddress, GetByEmail.MaybeValidatedEmail], countZuoraAccountsForIdentityId: IdentityId => ClientFailableOp[Int], sfAuth: LazyClientFailableOp[Any] ): ApiResponse = (for { - identityId <- getByEmail(EmailAddress("john.duffell@guardian.co.uk")) + maybeIdentityId <- getByEmail.runRequest(EmailAddress("john.duffell@guardian.co.uk")) .toApiGatewayOp("problem with email").withLogging("healthcheck getByEmail") + identityId <- maybeIdentityId match { + case GetByEmail.ValidatedEmail(identityId) => ContinueProcessing(identityId) + case other => + logger.error(s"failed healthcheck with $other") + ReturnWithResponse(ApiGatewayResponse.internalServerError("test identity id was not present")) + } _ <- countZuoraAccountsForIdentityId(identityId).toApiGatewayOp("get zuora accounts for identity id") _ <- sfAuth.value.toApiGatewayOp("Failed to authenticate with Salesforce") } yield ApiGatewayResponse.successfulExecution).apiResponse } + +object WireRequestToDomainObject { + + object WireModel { + + case class IdentityBackfillRequest( + emailAddress: String, + dryRun: Boolean + ) + implicit val identityBackfillRequest: Reads[IdentityBackfillRequest] = Json.reads[IdentityBackfillRequest] + + } + + def apply( + steps: DomainRequest => ResponseModels.ApiResponse + ): ApiGatewayRequest => ResponseModels.ApiResponse = req => + (for { + wireInput <- req.bodyAsCaseClass[IdentityBackfillRequest]() + mergeRequest = toDomainRequest(wireInput) + } yield steps(mergeRequest)).apiResponse + + def toDomainRequest(request: IdentityBackfillRequest): DomainRequest = + DomainRequest( + EmailAddress(request.emailAddress), + request.dryRun + ) + +} diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/IdentityBackfillSteps.scala b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/IdentityBackfillSteps.scala index 2c8888200c..8bcf9e8b14 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/IdentityBackfillSteps.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/IdentityBackfillSteps.scala @@ -1,57 +1,49 @@ package com.gu.identityBackfill -import com.gu.identityBackfill.IdentityBackfillSteps.WireModel.IdentityBackfillRequest import com.gu.identityBackfill.PreReqCheck.PreReqResult import com.gu.identityBackfill.TypeConvert._ import com.gu.identityBackfill.Types._ import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.salesforce.TypesForSFEffectsData.SFContactId import com.gu.util.Logging +import com.gu.util.apigateway.ApiGatewayResponse import com.gu.util.apigateway.ResponseModels.ApiResponse -import com.gu.util.apigateway.{ApiGatewayRequest, ApiGatewayResponse} import com.gu.util.reader.Types.ApiGatewayOp._ import com.gu.util.reader.Types._ -import com.gu.util.resthttp.Types.ClientFailableOp -import play.api.libs.json.{Json, Reads} +import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess} object IdentityBackfillSteps extends Logging { - object WireModel { - - case class IdentityBackfillRequest( - emailAddress: String, - dryRun: Boolean - ) - implicit val identityBackfillRequest: Reads[IdentityBackfillRequest] = Json.reads[IdentityBackfillRequest] - - } - - def fromRequest(identityBackfillRequest: IdentityBackfillRequest): EmailAddress = { - EmailAddress(identityBackfillRequest.emailAddress) - } + case class DomainRequest( + emailAddress: EmailAddress, + dryRun: Boolean + ) def apply( preReqCheck: EmailAddress => ApiGatewayOp[PreReqResult], + createGuestAccount: EmailAddress => ClientFailableOp[IdentityId], updateZuoraIdentityId: (AccountId, IdentityId) => ClientFailableOp[Unit], updateSalesforceIdentityId: (SFContactId, IdentityId) => ApiGatewayOp[Unit] - )(apiGatewayRequest: ApiGatewayRequest): ApiResponse = { + )(request: DomainRequest): ApiResponse = { (for { - request <- apiGatewayRequest.bodyAsCaseClass[IdentityBackfillRequest]() - preReq <- preReqCheck(fromRequest(request)) + preReq <- preReqCheck(request.emailAddress) _ <- dryRunAbort(request).withLogging("dryrun aborter") - _ <- updateZuoraIdentityId(preReq.zuoraAccountId, preReq.requiredIdentityId).toApiGatewayOp("update zuora identity id field") - _ <- updateSalesforceIdentityId(preReq.sFContactId, preReq.requiredIdentityId) + requiredIdentityId <- (preReq.existingIdentityId match { + case Some(existingIdentityId) => ClientSuccess(existingIdentityId) + case None => createGuestAccount(request.emailAddress) + }).toApiGatewayOp("create guest identity account") + _ <- updateZuoraIdentityId(preReq.zuoraAccountId, requiredIdentityId).toApiGatewayOp("update zuora identity id field") + _ <- updateSalesforceIdentityId(preReq.sFContactId, requiredIdentityId) // need to remember which ones we updated? } yield ApiGatewayResponse.successfulExecution).apiResponse } - def dryRunAbort(request: IdentityBackfillRequest): ApiGatewayOp[Unit] = + def dryRunAbort(request: DomainRequest): ApiGatewayOp[Unit] = if (request.dryRun) ReturnWithResponse(ApiGatewayResponse.noActionRequired("DRY RUN requested! skipping to the end")) else ContinueProcessing(()) } - diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala index 4cefeecabd..646bfc04f4 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala @@ -1,41 +1,41 @@ package com.gu.identityBackfill import com.gu.identity.GetByEmail -import com.gu.identity.GetByEmail.{NotFound, NotValidated, OtherError} +import com.gu.identity.GetByEmail.NotValidated +import com.gu.identityBackfill.TypeConvert._ import com.gu.identityBackfill.Types._ +import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.identityBackfill.zuora.GetZuoraSubTypeForAccount import com.gu.identityBackfill.zuora.GetZuoraSubTypeForAccount.ReaderType.ReaderTypeValue +import com.gu.salesforce.TypesForSFEffectsData.SFContactId import com.gu.util.apigateway.ApiGatewayResponse import com.gu.util.reader.Types.ApiGatewayOp._ import com.gu.util.reader.Types._ -import com.gu.util.resthttp.Types.ClientFailableOp -import scalaz.\/ -import TypeConvert._ -import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId -import com.gu.salesforce.TypesForSFEffectsData.SFContactId import com.gu.util.resthttp.LazyClientFailableOp +import com.gu.util.resthttp.Types.{ClientFailableOp, ClientFailure, ClientSuccess, NotFound} object PreReqCheck { - case class PreReqResult(zuoraAccountId: Types.AccountId, sFContactId: SFContactId, requiredIdentityId: IdentityId) + case class PreReqResult(zuoraAccountId: Types.AccountId, sFContactId: SFContactId, existingIdentityId: Option[IdentityId]) def apply( - getByEmail: EmailAddress => \/[GetByEmail.ApiError, IdentityId], + getByEmail: EmailAddress => ClientFailableOp[GetByEmail.MaybeValidatedEmail], getSingleZuoraAccountForEmail: EmailAddress => ApiGatewayOp[ZuoraAccountIdentitySFContact], noZuoraAccountsForIdentityId: IdentityId => ApiGatewayOp[Unit], zuoraSubType: AccountId => ApiGatewayOp[Unit], syncableSFToIdentity: SFContactId => LazyClientFailableOp[ApiGatewayOp[Unit]] )(emailAddress: EmailAddress): ApiGatewayOp[PreReqResult] = { for { - identityId <- getByEmail(emailAddress).leftMap({ - case NotFound => ApiGatewayResponse.notFound("user doesn't have identity") - case NotValidated => ApiGatewayResponse.notFound("identity email not validated") - case OtherError(unknownError) => ApiGatewayResponse.internalServerError(unknownError) - }).toApiGatewayOp.withLogging("GetByEmail") + maybeExistingIdentityId <- (getByEmail(emailAddress) match { + case ClientSuccess(GetByEmail.ValidatedEmail(identityId)) => ContinueProcessing(Some(identityId)) + case NotFound(_) => ContinueProcessing(None) + case ClientSuccess(NotValidated) => ReturnWithResponse(ApiGatewayResponse.notFound("identity email not validated")) + case other: ClientFailure => ReturnWithResponse(ApiGatewayResponse.internalServerError(other.toString)) + }).withLogging("GetByEmail") zuoraAccountForEmail <- getSingleZuoraAccountForEmail(emailAddress) - _ <- noZuoraAccountsForIdentityId(identityId) + _ <- maybeExistingIdentityId.map(noZuoraAccountsForIdentityId).getOrElse(ContinueProcessing(())) _ <- syncableSFToIdentity(zuoraAccountForEmail.sfContactId).value.toApiGatewayOp("load SF contact").flatMap(identity) - } yield PreReqResult(zuoraAccountForEmail.accountId, zuoraAccountForEmail.sfContactId, identityId) + } yield PreReqResult(zuoraAccountForEmail.accountId, zuoraAccountForEmail.sfContactId, maybeExistingIdentityId) } def noZuoraAccountsForIdentityId( diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala new file mode 100644 index 0000000000..63d09ba5b5 --- /dev/null +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala @@ -0,0 +1,60 @@ +package com.gu.identity + +import com.gu.effects.{GetFromS3, RawEffects} +import com.gu.identityBackfill.Types.EmailAddress +import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId +import com.gu.salesforce.JsonHttp +import com.gu.test.EffectsTest +import com.gu.util.config.{LoadConfigModule, Stage} +import com.gu.util.resthttp.HttpOp.HttpOpWrapper +import com.gu.util.resthttp.RestRequestMaker +import com.gu.util.resthttp.RestRequestMaker.{GetRequestWithParams, RelativePath, UrlParams} +import org.scalatest.{FlatSpec, Matchers} +import play.api.libs.json.{JsValue, Json, Reads} +import scalaz.\/- + +import scala.util.Random + +class CreateGuestAccountEffectsTest extends FlatSpec with Matchers { + + it should "create a guest account" taggedAs EffectsTest in { + + val unique = s"${Random.nextLong.toHexString}" + val testContact = EmailAddress(s"sx.CreateGuestAccountEffectsTest+$unique@gu.com") + + val actual = for { + identityConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[IdentityConfig] + response = RawEffects.response + identityClient = IdentityClient(response, identityConfig) + createGuestAccount = identityClient.wrapWith(JsonHttp.post).wrapWith(CreateGuestAccount.wrapper) + createdId <- createGuestAccount.runRequest(testContact).toDisjunction + fetchedId <- identityClient.wrapWith(JsonHttp.getWithParams).wrapWith(GetByEmailForTesting.wrapper).runRequest(testContact).toDisjunction + } yield (createdId, fetchedId) + + val failure = actual.map { + case (createdId, fetchedId) if createdId == fetchedId => + None + case (createdId, fetchedId) => + Some(s"for email $testContact createdId by email address was $createdId but afterwards fetchedId by email address was $fetchedId") + } + failure should be(\/-(None)) + + } + +} + +object GetByEmailForTesting { + + case class User(id: String) + implicit val userReads: Reads[User] = Json.reads[User] + + case class UserResponse(user: User) + implicit val userResponseReads: Reads[UserResponse] = Json.reads[UserResponse] + + val wrapper: HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityId] = + HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityId]( + emailAddress => GetRequestWithParams(RelativePath(s"/user"), UrlParams(Map("emailAddress" -> emailAddress.value))), + RestRequestMaker.toResult[UserResponse](_).map((wire: UserResponse) => IdentityId(wire.user.id)) + ) + +} diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountTest.scala new file mode 100644 index 0000000000..c03b24e90c --- /dev/null +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountTest.scala @@ -0,0 +1,26 @@ +package com.gu.identity + +import com.gu.identity.CreateGuestAccount.{WireGuestRegistrationResponse, WireIdentityResponse} +import com.gu.identityBackfill.Types.EmailAddress +import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId +import com.gu.util.resthttp.RestRequestMaker.{PostRequest, RelativePath} +import org.scalatest.{FlatSpec, Matchers} +import play.api.libs.json.{JsObject, JsString} + +class CreateGuestAccountTest extends FlatSpec with Matchers { + + it should "create a request ok" in { + val actual = CreateGuestAccount.toRequest(EmailAddress("hello@gu.com")) + + val expected = new PostRequest(JsObject(List("primaryEmailAddress" -> JsString("hello@gu.com"))), RelativePath("/guest")) + actual should be(expected) + } + + it should "crate a response ok" in { + val expected = IdentityId("useridhere") + val testData = WireIdentityResponse("ok", WireGuestRegistrationResponse("useridhere")) + val actual = CreateGuestAccount.toResponse(testData) + actual should be(expected) + } + +} diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala index c088fcb272..be6c68a9db 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala @@ -1,8 +1,10 @@ package com.gu.identity import com.gu.effects.{GetFromS3, RawEffects} +import com.gu.identity.GetByEmail.ValidatedEmail import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId +import com.gu.salesforce.JsonHttp import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} import org.scalatest.{FlatSpec, Matchers} @@ -15,11 +17,13 @@ class GetByEmailEffectsTest extends FlatSpec with Matchers { val actual = for { identityConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[IdentityConfig] - identityId <- GetByEmail(RawEffects.response, identityConfig)(EmailAddress("john.duffell@guardian.co.uk")) - } yield { - identityId - } - actual should be(\/-(IdentityId("21814163"))) + + response = RawEffects.response + identityClient = IdentityClient(response, identityConfig) + getByEmail = identityClient.wrapWith(JsonHttp.getWithParams).wrapWith(GetByEmail.wrapper) + identityId <- getByEmail.runRequest(EmailAddress("john.duffell@guardian.co.uk")).toDisjunction + } yield identityId + actual should be(\/-(ValidatedEmail(IdentityId("21814163")))) } diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala index 1ee4e6ef56..d8869176d1 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala @@ -1,38 +1,32 @@ package com.gu.identity -import com.gu.effects.TestingRawEffects -import com.gu.effects.TestingRawEffects.HTTPResponse -import com.gu.identity.GetByEmail.{NotFound, NotValidated} -import com.gu.identity.GetByEmailTest.{NotFoundTestData, NotValidatedTestData, TestData} +import com.gu.identity.GetByEmail.{NotValidated, ValidatedEmail} +import com.gu.identity.GetByEmailTest.{NotValidatedTestData, TestData} import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId +import com.gu.util.resthttp.RestRequestMaker.{GetRequestWithParams, RelativePath, UrlParams} +import com.gu.util.resthttp.Types.ClientSuccess import org.scalatest.{FlatSpec, Matchers} -import scalaz.{-\/, \/, \/-} +import play.api.libs.json.Json class GetByEmailTest extends FlatSpec with Matchers { - it should "get successful ok" in { - val actual = geyByEmailFromResponses(TestData.responses) - - actual should be(\/-(IdentityId("1234"))) - } - - it should "get not validated with an error" in { - val actual = geyByEmailFromResponses(NotValidatedTestData.responses) + it should "formulate a request" in { + val actual = GetByEmail.wrapper.fromNewParam(EmailAddress("email@address")) - actual should be(-\/(NotValidated)) + actual should be(GetRequestWithParams(RelativePath("/user"), UrlParams(Map("emailAddress" -> "email@address")))) } - it should "get not found" in { - val actual = geyByEmailFromResponses(NotFoundTestData.responses) + it should "get successful ok" in { + val actual = GetByEmail.wrapper.toNewResponse(Json.parse(TestData.dummyIdentityResponse)) - actual should be(-\/(NotFound)) + actual should be(ClientSuccess(ValidatedEmail(IdentityId("1234")))) } - private def geyByEmailFromResponses(responses: Map[String, HTTPResponse]): GetByEmail.ApiError \/ IdentityId = { - val testingRawEffects = new TestingRawEffects(responses = responses) + it should "get not validated with an error" in { + val actual = GetByEmail.wrapper.toNewResponse(Json.parse(NotValidatedTestData.dummyIdentityResponse)) - GetByEmail(testingRawEffects.response, IdentityConfig("http://baseurl", "apitoken"))(EmailAddress("email@address")) + actual should be(ClientSuccess(NotValidated)) } } @@ -41,10 +35,6 @@ object GetByEmailTest { object TestData { - def responses: Map[String, HTTPResponse] = Map( - "/user?emailAddress=email@address" -> HTTPResponse(200, dummyIdentityResponse) - ) - val dummyIdentityResponse: String = """ |{ @@ -73,10 +63,6 @@ object GetByEmailTest { object NotValidatedTestData { - def responses: Map[String, HTTPResponse] = Map( - "/user?emailAddress=email@address" -> HTTPResponse(200, dummyIdentityResponse) - ) - val dummyIdentityResponse: String = """ |{ @@ -103,24 +89,4 @@ object GetByEmailTest { } - object NotFoundTestData { - - def responses: Map[String, HTTPResponse] = Map( - "/user?emailAddress=email@address" -> HTTPResponse(404, dummyIdentityResponse) - ) - val dummyIdentityResponse: String = - """ - |{ - | "status": "error", - | "errors": [ - | { - | "message": "Not found", - | "description": "Resource not found" - | } - | ] - |} - """.stripMargin - - } - } diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/EndToEndHandlerTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/EndToEndHandlerTest.scala index 5f1cd7a561..e3fb748f06 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/EndToEndHandlerTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/EndToEndHandlerTest.scala @@ -4,7 +4,7 @@ import java.io.{ByteArrayInputStream, ByteArrayOutputStream} import com.gu.effects.TestingRawEffects.{BasicRequest, HTTPResponse, POSTRequest} import com.gu.effects.{FakeFetchString, TestingRawEffects} -import com.gu.identity.GetByEmailTest +import com.gu.identity.GetByEmailTest.TestData.dummyIdentityResponse import com.gu.identityBackfill.EndToEndData._ import com.gu.identityBackfill.Runner._ import com.gu.identityBackfill.salesforce.getContact.GetSFContactSyncCheckFieldsTest @@ -109,8 +109,12 @@ object EndToEndData { ) } + def GetByEmailTestresponses: Map[String, HTTPResponse] = Map( + "/user?emailAddress=email@address" -> HTTPResponse(200, dummyIdentityResponse) + ) + def responses: Map[String, HTTPResponse] = - GetByEmailTest.TestData.responses ++ responsesGetSFContactSyncCheckFieldsTest + GetByEmailTestresponses ++ responsesGetSFContactSyncCheckFieldsTest def postResponses: Map[POSTRequest, HTTPResponse] = GetZuoraAccountsForEmailData.postResponses(false) ++ CountZuoraAccountsForIdentityIdData.postResponses(false) ++ diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala index f30b6015f0..f96e095604 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala @@ -1,7 +1,7 @@ package com.gu.identityBackfill import com.gu.identity.GetByEmail -import com.gu.identity.GetByEmail.{NotFound, NotValidated} +import com.gu.identity.GetByEmail.{NotValidated, ValidatedEmail} import com.gu.identityBackfill.PreReqCheck.PreReqResult import com.gu.identityBackfill.Types._ import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId @@ -10,7 +10,7 @@ import com.gu.salesforce.TypesForSFEffectsData.SFContactId import com.gu.util.apigateway.ApiGatewayResponse import com.gu.util.reader.Types.ApiGatewayOp.{ContinueProcessing, ReturnWithResponse} import com.gu.util.resthttp.LazyClientFailableOp -import com.gu.util.resthttp.Types.ClientSuccess +import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess, NotFound} import org.scalatest.{FlatSpec, Matchers} class PreReqCheckTest extends FlatSpec with Matchers { @@ -19,14 +19,29 @@ class PreReqCheckTest extends FlatSpec with Matchers { val result = PreReqCheck( - email => scalaz.\/-(IdentityId("asdf")), + email => ClientSuccess(ValidatedEmail(IdentityId("asdf"))), email => ContinueProcessing(ZuoraAccountIdentitySFContact(AccountId("acc"), None, SFContactId("sf"))), identityId => ContinueProcessing(()), _ => ContinueProcessing(()), _ => LazyClientFailableOp(() => ClientSuccess(ContinueProcessing(()))) )(EmailAddress("email@address")) - val expectedResult = ContinueProcessing(PreReqResult(AccountId("acc"), SFContactId("sf"), IdentityId("asdf"))) + val expectedResult = ContinueProcessing(PreReqResult(AccountId("acc"), SFContactId("sf"), Some(IdentityId("asdf")))) + result should be(expectedResult) + } + + it should "go through a happy case with no existing identity" in { + + val result = + PreReqCheck( + email => NotFound("so we will return None"), + email => ContinueProcessing(ZuoraAccountIdentitySFContact(AccountId("acc"), None, SFContactId("sf"))), + identityId => ContinueProcessing(()), + _ => ContinueProcessing(()), + _ => LazyClientFailableOp(() => ClientSuccess(ContinueProcessing(()))) + )(EmailAddress("email@address")) + + val expectedResult = ContinueProcessing(PreReqResult(AccountId("acc"), SFContactId("sf"), None)) result should be(expectedResult) } @@ -66,25 +81,17 @@ class PreReqCheckTest extends FlatSpec with Matchers { result should be(expectedResult) } - it should "stop processing if it can't find an identity id for the required email" in { - - val result = emailCheckFailure(NotFound) - - val expectedResult = ReturnWithResponse(ApiGatewayResponse.notFound("user doesn't have identity")) - result should be(expectedResult) - } - it should "stop processing if it finds a non validated identity account" in { - val result = emailCheckFailure(NotValidated) + val result = emailCheckFailure(ClientSuccess(NotValidated)) val expectedResult = ReturnWithResponse(ApiGatewayResponse.notFound("identity email not validated")) result should be(expectedResult) } - private def emailCheckFailure(identityError: GetByEmail.ApiError) = { + private def emailCheckFailure(identityError: ClientFailableOp[GetByEmail.MaybeValidatedEmail]) = { PreReqCheck( - email => scalaz.-\/(identityError), + email => identityError, email => fail("shouldn't be called 1"), identityId => fail("shouldn't be called 2"), _ => fail("shouldn't be called 3"), diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/StepsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/StepsTest.scala index 98ee798d16..a5f97f1380 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/StepsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/StepsTest.scala @@ -1,13 +1,13 @@ package com.gu.identityBackfill -import com.gu.identityBackfill.StepsData._ +import com.gu.identityBackfill.IdentityBackfillSteps.DomainRequest import com.gu.identityBackfill.Types._ import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.salesforce.TypesForSFEffectsData.SFContactId +import com.gu.util.apigateway.ApiGatewayResponse import com.gu.util.apigateway.ResponseModels.ApiResponse -import com.gu.util.apigateway.{ApiGatewayRequest, ApiGatewayResponse} import com.gu.util.reader.Types.ApiGatewayOp -import com.gu.util.reader.Types.ApiGatewayOp.{ContinueProcessing, ReturnWithResponse} +import com.gu.util.reader.Types.ApiGatewayOp.ContinueProcessing import com.gu.util.resthttp.Types.ClientSuccess import org.scalatest.{FlatSpec, Matchers} @@ -19,16 +19,17 @@ class StepsTest extends FlatSpec with Matchers { var salesforceUpdate: Option[(SFContactId, IdentityId)] = None // !! var emailToCheck: Option[EmailAddress] = None // !! - def getSteps(succeed: Boolean = true): ApiGatewayRequest => ApiResponse = { + def getSteps(succeed: Boolean = true): DomainRequest => ApiResponse = { val preReqCheck: EmailAddress => ApiGatewayOp[PreReqCheck.PreReqResult] = { email => emailToCheck = Some(email) if (succeed) - ContinueProcessing(PreReqCheck.PreReqResult(AccountId("acc"), SFContactId("sf"), IdentityId("asdf"))) + ContinueProcessing(PreReqCheck.PreReqResult(AccountId("acc"), SFContactId("sf"), Some(IdentityId("existing")))) else - ReturnWithResponse(ApiGatewayResponse.notFound("dummy")) + ContinueProcessing(PreReqCheck.PreReqResult(AccountId("acc"), SFContactId("sf"), None)) } IdentityBackfillSteps( preReqCheck, + createGuestAccount = email => ClientSuccess(IdentityId("created")), updateZuoraIdentityId = (accountId, identityId) => { zuoraUpdate = Some((accountId, identityId)) ClientSuccess(()) @@ -48,12 +49,12 @@ class StepsTest extends FlatSpec with Matchers { import stepsWithMocks._ val result = - getSteps()(ApiGatewayRequest(None, Some(identityBackfillRequest(false)), None)) + getSteps()(DomainRequest(Types.EmailAddress("email@address"), dryRun = false)) val expectedResult = ApiGatewayResponse.successfulExecution result should be(expectedResult) - zuoraUpdate should be(Some((AccountId("acc"), IdentityId("asdf")))) - salesforceUpdate should be(Some((SFContactId("sf"), IdentityId("asdf")))) + zuoraUpdate should be(Some((AccountId("acc"), IdentityId("existing")))) + salesforceUpdate should be(Some((SFContactId("sf"), IdentityId("existing")))) emailToCheck should be(Some(EmailAddress("email@address"))) } @@ -63,7 +64,7 @@ class StepsTest extends FlatSpec with Matchers { import stepsWithMocks._ val result = - getSteps()(ApiGatewayRequest(None, Some(identityBackfillRequest(true)), None)) + getSteps()(DomainRequest(Types.EmailAddress("email@address"), dryRun = true)) val expectedResult = ApiGatewayResponse.noActionRequired("DRY RUN requested! skipping to the end") result should be(expectedResult) @@ -78,12 +79,13 @@ class StepsTest extends FlatSpec with Matchers { import stepsWithMocks._ val result = - getSteps(false)(ApiGatewayRequest(None, Some(identityBackfillRequest(false)), None)) + getSteps(false)(DomainRequest(Types.EmailAddress("email@address"), dryRun = false)) - val expectedResult = ApiGatewayResponse.notFound("dummy") + val expectedResult = ApiGatewayResponse.successfulExecution result should be(expectedResult) - zuoraUpdate should be(None) - salesforceUpdate should be(None) + zuoraUpdate should be(Some((AccountId("acc"), IdentityId("created")))) + salesforceUpdate should be(Some((SFContactId("sf"), IdentityId("created")))) + emailToCheck should be(Some(EmailAddress("email@address"))) } } diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala index 2a42fd2432..072e237701 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala @@ -18,7 +18,7 @@ class GetSFContactSyncCheckFieldsEffectsTest extends FlatSpec with Matchers { val actual = for { sfConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[SFAuthConfig] sfAuth <- SalesforceClient(RawEffects.response, sfConfig).value.toDisjunction - getOp = sfAuth.wrap(JsonHttp.get) + getOp = sfAuth.wrapWith(JsonHttp.get) result <- GetSFContactSyncCheckFields(getOp).apply(SFEffectsData.testContactHasNamePhoneOtherAddress).value.toDisjunction } yield result diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala index e257e68092..e60a352e84 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala @@ -24,9 +24,9 @@ class UpdateSalesforceIdentityIdEffectsTest extends FlatSpec with Matchers { sfConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[SFAuthConfig] response = RawEffects.response auth <- SalesforceClient(response, sfConfig).value.toDisjunction - updateSalesforceIdentityId = UpdateSalesforceIdentityId(auth.wrap(JsonHttp.patch)) + updateSalesforceIdentityId = UpdateSalesforceIdentityId(auth.wrapWith(JsonHttp.patch)) _ <- updateSalesforceIdentityId.runRequestMultiArg(testContact, IdentityId(unique)).toDisjunction - getSalesforceIdentityId = GetSalesforceIdentityId(auth.wrap(JsonHttp.get)) + getSalesforceIdentityId = GetSalesforceIdentityId(auth.wrapWith(JsonHttp.get)) identityId <- getSalesforceIdentityId(testContact).value.toDisjunction } yield identityId diff --git a/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala b/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala index e6411103b4..b2d5d7a95c 100644 --- a/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala +++ b/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala @@ -49,11 +49,11 @@ object Handler { } yield Operation.noHealthcheck { WireRequestToDomainObject { - val sfGet = sfAuth.wrap(JsonHttp.get) + val sfGet = sfAuth.wrapWith(JsonHttp.get) val getSfContact = GetSfContact(sfGet.setupRequest(ToSfContactRequest.apply).parse[WireResult].map(WireContactToSfContact.apply)) DomainSteps( ZuoraSteps(GetIdentityAndZuoraEmailsForAccountsSteps(zuoraQuerier)), - UpdateSFContacts(UpdateSalesforceIdentityId(sfAuth.wrap(JsonHttp.patch))), + UpdateSFContacts(UpdateSalesforceIdentityId(sfAuth.wrapWith(JsonHttp.patch))), UpdateAccountSFLinks(requests.put), SFSteps(getSfContact) ) diff --git a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala index 8cb1c0fcd5..d32dad007e 100644 --- a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala +++ b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala @@ -24,7 +24,7 @@ class GetSfAddressEffectsTest extends FlatSpec with Matchers { sfConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[SFAuthConfig] response = RawEffects.response sfAuth <- SalesforceClient(response, sfConfig).value.toDisjunction - getSfContact = sfAuth.wrap(JsonHttp.get).setupRequest(ToSfContactRequest.apply).parse[WireResult].map(WireContactToSfContact.apply) + getSfContact = sfAuth.wrapWith(JsonHttp.get).setupRequest(ToSfContactRequest.apply).parse[WireResult].map(WireContactToSfContact.apply) address <- getSfContact.runRequest(testContact).toDisjunction } yield address diff --git a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala index e8621000e5..ec47d362b0 100644 --- a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala +++ b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala @@ -47,7 +47,7 @@ class UpdateSalesforceIdentityIdEffectsTest extends FlatSpec with Matchers { sfConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[SFAuthConfig] response = RawEffects.response sfClient <- SalesforceClient(response, sfConfig).value.toDisjunction - patch = sfClient.wrap(JsonHttp.patch) + patch = sfClient.wrapWith(JsonHttp.patch) updateSalesforceIdentityId = UpdateSalesforceIdentityId(patch) sFContactUpdate = SFContactUpdate( Some(testIdentityId), @@ -56,7 +56,7 @@ class UpdateSalesforceIdentityIdEffectsTest extends FlatSpec with Matchers { Some(testEmail) ) _ <- updateSalesforceIdentityId.apply(testContact, sFContactUpdate).toDisjunction - getSalesforceIdentityId = GetSalesforceIdentityId(sfClient.wrap(JsonHttp.get)) _ + getSalesforceIdentityId = GetSalesforceIdentityId(sfClient.wrapWith(JsonHttp.get)) _ updatedIdentityId <- getSalesforceIdentityId(testContact).toDisjunction } yield updatedIdentityId diff --git a/lib/restHttp/src/main/scala/com/gu/util/resthttp/HttpOp.scala b/lib/restHttp/src/main/scala/com/gu/util/resthttp/HttpOp.scala index e3d765266a..dd9ce1eca4 100644 --- a/lib/restHttp/src/main/scala/com/gu/util/resthttp/HttpOp.scala +++ b/lib/restHttp/src/main/scala/com/gu/util/resthttp/HttpOp.scala @@ -1,6 +1,6 @@ package com.gu.util.resthttp -import com.gu.util.resthttp.HttpOp.HttpWrapper +import com.gu.util.resthttp.HttpOp.HttpOpWrapper import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess} import okhttp3.{Request, Response} import play.api.libs.json.{JsValue, Reads} @@ -17,7 +17,7 @@ case class HttpOp[PARAM, RESPONSE]( def flatMap[NEWRESPONSE](toNewResponse: RESPONSE => ClientFailableOp[NEWRESPONSE]): HttpOp[PARAM, NEWRESPONSE] = HttpOp(inputToRequest, effect, responseToOutput.andThen(_.flatMap(toNewResponse))) - def wrap[UPDATEDPARAM, NEWRESPONSE](both: HttpWrapper[UPDATEDPARAM, PARAM, RESPONSE, NEWRESPONSE]): HttpOp[UPDATEDPARAM, NEWRESPONSE] = + def wrapWith[UPDATEDPARAM, NEWRESPONSE](both: HttpOpWrapper[UPDATEDPARAM, PARAM, RESPONSE, NEWRESPONSE]): HttpOp[UPDATEDPARAM, NEWRESPONSE] = setupRequest(both.fromNewParam).flatMap(both.toNewResponse) def runRequest(in: PARAM): ClientFailableOp[RESPONSE] = @@ -35,10 +35,16 @@ case class HttpOp[PARAM, RESPONSE]( object HttpOp { - trait HttpWrapper[UPDATEDPARAM, PARAM, RESPONSE, NEWRESPONSE] { - def fromNewParam(updatedParam: UPDATEDPARAM): PARAM + case class HttpOpWrapper[UPDATEDPARAM, PARAM, RESPONSE, NEWRESPONSE]( + fromNewParam: UPDATEDPARAM => PARAM, + toNewResponse: RESPONSE => ClientFailableOp[NEWRESPONSE] + ) { + + def wrapWith[MOREUPDATEDPARAM, MORENEWRESPONSE]( + wrapper: HttpOpWrapper[MOREUPDATEDPARAM, UPDATEDPARAM, NEWRESPONSE, MORENEWRESPONSE] + ): HttpOpWrapper[MOREUPDATEDPARAM, PARAM, RESPONSE, MORENEWRESPONSE] = + HttpOpWrapper(wrapper.fromNewParam.andThen(fromNewParam), toNewResponse.andThen(_.flatMap(wrapper.toNewResponse))) - def toNewResponse(response: RESPONSE): ClientFailableOp[NEWRESPONSE] } def apply(getResponse: Request => Response): HttpOp[Request, Response] = diff --git a/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala b/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala index 5f0795b3bf..79b441f886 100644 --- a/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala +++ b/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala @@ -66,10 +66,17 @@ object RestRequestMaker extends Logging { case class GetRequest(path: RelativePath) + case class GetRequestWithParams(path: RelativePath, urlParams: UrlParams) + case class JsonResponse(bodyAsJson: JsValue) { def value[RESP: Reads] = toResult[RESP](bodyAsJson) } + case class UrlParams(value: Map[String, String]) + object UrlParams { + val empty = UrlParams(Map.empty) + } + class Requests( headers: Map[String, String], baseUrl: String, diff --git a/lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala b/lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala index e18abb8ebf..bdbe8f4019 100644 --- a/lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala +++ b/lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala @@ -1,7 +1,7 @@ package com.gu.salesforce -import com.gu.salesforce.SalesforceClient.{GetMethod, StringHttpRequest, PatchMethod, PostMethod} -import com.gu.util.resthttp.HttpOp.HttpWrapper +import com.gu.salesforce.SalesforceClient._ +import com.gu.util.resthttp.HttpOp.HttpOpWrapper import com.gu.util.resthttp.RestRequestMaker._ import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess, GenericError} import play.api.libs.json.{JsValue, Json} @@ -11,35 +11,42 @@ import scala.util.Try object JsonHttp { val patch = - new HttpWrapper[PatchRequest, StringHttpRequest, BodyAsString, Unit] { - override def fromNewParam(patchRequest: PatchRequest): StringHttpRequest = - StringHttpRequest(patchRequest.path, PatchMethod(BodyAsString(Json.stringify(patchRequest.body)))) + HttpOpWrapper[PatchRequest, StringHttpRequest, BodyAsString, Unit]( + (patchRequest: PatchRequest) => + StringHttpRequest(PatchMethod(BodyAsString(Json.stringify(patchRequest.body))), patchRequest.path, UrlParams.empty), - override def toNewResponse(response: BodyAsString): ClientFailableOp[Unit] = ClientSuccess(()) - } + (response: BodyAsString) => ClientSuccess(()) + ) val post = - new HttpWrapper[PostRequest, StringHttpRequest, BodyAsString, JsValue] { - override def fromNewParam(postRequest: PostRequest): StringHttpRequest = - StringHttpRequest(postRequest.path, PostMethod(BodyAsString(Json.stringify(postRequest.body)))) - - override def toNewResponse(response: BodyAsString): ClientFailableOp[JsValue] = - Try(Json.parse(response.value)) match { - case scala.util.Success(value) => ClientSuccess(value) - case scala.util.Failure(exception) => GenericError(s"could not deserialise json $response: $exception") - } + HttpOpWrapper[PostRequest, StringHttpRequest, BodyAsString, JsValue]( + (postRequest: PostRequest) => + StringHttpRequest(PostMethod(BodyAsString(Json.stringify(postRequest.body))), postRequest.path, UrlParams.empty), + + deserialiseJsonResponse + ) + + def get = { + HttpOpWrapper[GetRequest, StringHttpRequest, BodyAsString, JsValue]( + (getRequest: GetRequest) => + StringHttpRequest(GetMethod, getRequest.path, UrlParams.empty), + + deserialiseJsonResponse + ) + } + + def getWithParams = { + HttpOpWrapper[GetRequestWithParams, StringHttpRequest, BodyAsString, JsValue]( + (getRequest: GetRequestWithParams) => + StringHttpRequest(GetMethod, getRequest.path, getRequest.urlParams), + + deserialiseJsonResponse + ) + } + + def deserialiseJsonResponse(response: BodyAsString): ClientFailableOp[JsValue] = + Try(Json.parse(response.value)) match { + case scala.util.Success(value) => ClientSuccess(value) + case scala.util.Failure(exception) => GenericError(s"could not deserialise json $response: $exception") } - - def get = - new HttpWrapper[GetRequest, StringHttpRequest, BodyAsString, JsValue] { - override def fromNewParam(getRequest: GetRequest): StringHttpRequest = - StringHttpRequest(getRequest.path, GetMethod) - - override def toNewResponse(response: BodyAsString): ClientFailableOp[JsValue] = - Try(Json.parse(response.value)) match { - case scala.util.Success(value) => ClientSuccess(value) - case scala.util.Failure(exception) => GenericError(s"could not deserialise json $response: $exception") - } - } - } diff --git a/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala b/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala index ae1215c04c..d7b9f5ef98 100644 --- a/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala +++ b/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala @@ -1,7 +1,7 @@ package com.gu.salesforce import com.gu.salesforce.SalesforceAuthenticate.{SFAuthConfig, SalesforceAuth} -import com.gu.util.resthttp.RestRequestMaker.{BodyAsString, RelativePath, createBodyFromString, toClientFailableOp} +import com.gu.util.resthttp.RestRequestMaker._ import com.gu.util.resthttp.{HttpOp, LazyClientFailableOp} import okhttp3.{Request, Response} @@ -39,6 +39,6 @@ object SalesforceClient { override def builder: Request.Builder = new Request.Builder().get() } - case class StringHttpRequest(relativePath: RelativePath, requestMethod: RequestMethod) + case class StringHttpRequest(requestMethod: RequestMethod, relativePath: RelativePath, urlParams: UrlParams) } From 18b4dd5c3e772f76f4d614a085bef8766740acd9 Mon Sep 17 00:00:00 2001 From: John Duffell Date: Tue, 30 Oct 2018 17:25:09 +0000 Subject: [PATCH 2/8] move identity client into a separate file --- .../com/gu/identity/CreateGuestAccount.scala | 36 ++-------------- .../scala/com/gu/identity/GetByEmail.scala | 13 +----- .../com/gu/identity/IdentityClient.scala | 41 +++++++++++++++++++ 3 files changed, 45 insertions(+), 45 deletions(-) create mode 100644 handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala index 17682658cf..207cb53a57 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/CreateGuestAccount.scala @@ -2,25 +2,17 @@ package com.gu.identity import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId -import com.gu.salesforce.SalesforceClient.StringHttpRequest import com.gu.util.resthttp.HttpOp.HttpOpWrapper +import com.gu.util.resthttp.RestRequestMaker import com.gu.util.resthttp.RestRequestMaker._ -import com.gu.util.resthttp.{HttpOp, RestRequestMaker} -import okhttp3.{HttpUrl, Request, Response} import play.api.libs.json.{JsValue, Json, Reads} object CreateGuestAccount { - case class WireGuestRegistrationResponse( - userId: String - ) + case class WireGuestRegistrationResponse(userId: String) implicit val reads = Json.reads[WireGuestRegistrationResponse] - case class WireGuestRegistrationRequest( - primaryEmailAddress: String //, - // privateFields: ,all optional - // publicFields: req-displayName only - ) + case class WireGuestRegistrationRequest(primaryEmailAddress: String) implicit val writes = Json.writes[WireGuestRegistrationRequest] case class WireIdentityResponse(status: String, guestRegistrationRequest: WireGuestRegistrationResponse) @@ -37,25 +29,3 @@ object CreateGuestAccount { } -object IdentityClient { - - def apply( - response: Request => Response, - config: IdentityConfig - ): HttpOp[StringHttpRequest, BodyAsString] = - HttpOp(response).flatMap { - toClientFailableOp - }.setupRequest[StringHttpRequest] { - withAuth(config) - } - - def withAuth(identityConfig: IdentityConfig)(requestInfo: StringHttpRequest): Request = { - val builder = requestInfo.requestMethod.builder - val authedBuilder = builder.addHeader("X-GU-ID-Client-Access-Token", s"Bearer ${identityConfig.apiToken}") - val url = requestInfo.urlParams.value.foldLeft(HttpUrl.parse(identityConfig.baseUrl + requestInfo.relativePath.value).newBuilder()) { - case (nextBuilder, (key, value)) => nextBuilder.addQueryParameter(key, value) - }.build() - authedBuilder.url(url).build() - } - -} diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala index faac3dc40c..dd978b5166 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala @@ -3,7 +3,6 @@ package com.gu.identity import com.gu.identity.GetByEmail.RawWireModel.{User, UserResponse} import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId -import com.gu.util.config.ConfigLocation import com.gu.util.resthttp.HttpOp.HttpOpWrapper import com.gu.util.resthttp.RestRequestMaker import com.gu.util.resthttp.RestRequestMaker.{GetRequestWithParams, RelativePath, UrlParams} @@ -41,7 +40,7 @@ object GetByEmail { case _ => GenericError("not an OK response from api") } - def toResponse(userResponse: UserResponse) = { + def toResponse(userResponse: UserResponse): ClientFailableOp[MaybeValidatedEmail] = { for { user <- userFromResponse(userResponse) identityId = if (user.statusFields.userEmailValidated) ValidatedEmail(IdentityId(user.id)) else NotValidated @@ -50,13 +49,3 @@ object GetByEmail { } } - -case class IdentityConfig( - baseUrl: String, - apiToken: String -) - -object IdentityConfig { - implicit val reads: Reads[IdentityConfig] = Json.reads[IdentityConfig] - implicit val location = ConfigLocation[IdentityConfig](path = "identity", version = 1) -} diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala new file mode 100644 index 0000000000..d3663e298c --- /dev/null +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala @@ -0,0 +1,41 @@ +package com.gu.identity + +import com.gu.salesforce.SalesforceClient.StringHttpRequest +import com.gu.util.config.ConfigLocation +import com.gu.util.resthttp.HttpOp +import com.gu.util.resthttp.RestRequestMaker.{BodyAsString, toClientFailableOp} +import okhttp3.{HttpUrl, Request, Response} +import play.api.libs.json.{Json, Reads} + +object IdentityClient { + + def apply( + response: Request => Response, + config: IdentityConfig + ): HttpOp[StringHttpRequest, BodyAsString] = + HttpOp(response).flatMap { + toClientFailableOp + }.setupRequest[StringHttpRequest] { + withAuth(config) + } + + def withAuth(identityConfig: IdentityConfig)(requestInfo: StringHttpRequest): Request = { + val builder = requestInfo.requestMethod.builder + val authedBuilder = builder.addHeader("X-GU-ID-Client-Access-Token", s"Bearer ${identityConfig.apiToken}") + val url = requestInfo.urlParams.value.foldLeft(HttpUrl.parse(identityConfig.baseUrl + requestInfo.relativePath.value).newBuilder()) { + case (nextBuilder, (key, value)) => nextBuilder.addQueryParameter(key, value) + }.build() + authedBuilder.url(url).build() + } + +} + +case class IdentityConfig( + baseUrl: String, + apiToken: String +) + +object IdentityConfig { + implicit val reads: Reads[IdentityConfig] = Json.reads[IdentityConfig] + implicit val location = ConfigLocation[IdentityConfig](path = "identity", version = 1) +} From 39a207d88ddafabf16ad2fae6c4ab4a99219fedf Mon Sep 17 00:00:00 2001 From: John Duffell Date: Wed, 31 Oct 2018 10:08:29 +0000 Subject: [PATCH 3/8] move StringHttpRequest etc into the restHttp project rather than salesforce --- .../gu/cancellation/sf_cases/Handler.scala | 6 +++--- .../sf_cases/EndToEndHandlerEffectsTest.scala | 2 +- .../com/gu/identity/IdentityClient.scala | 2 +- .../com/gu/identityBackfill/Handler.scala | 6 +++--- .../CreateGuestAccountEffectsTest.scala | 3 +-- .../gu/identity/GetByEmailEffectsTest.scala | 2 +- ...tSFContactSyncCheckFieldsEffectsTest.scala | 3 ++- ...pdateSalesforceIdentityIdEffectsTest.scala | 3 ++- .../com/gu/sf_contact_merge/Handler.scala | 3 ++- .../GetSfAddressEffectsTest.scala | 3 ++- ...pdateSalesforceIdentityIdEffectsTest.scala | 4 ++-- .../com/gu/util/resthttp}/JsonHttp.scala | 19 +++++++++++++++++-- .../com/gu/salesforce/SalesforceClient.scala | 16 +--------------- 13 files changed, 38 insertions(+), 34 deletions(-) rename lib/{salesforce/src/main/scala/com/gu/salesforce => restHttp/src/main/scala/com/gu/util/resthttp}/JsonHttp.scala (70%) diff --git a/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala b/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala index b2c3603c87..a198b93daa 100644 --- a/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala +++ b/handlers/cancellation-sf-cases/src/main/scala/com/gu/cancellation/sf_cases/Handler.scala @@ -9,13 +9,12 @@ import com.gu.effects.{GetFromS3, RawEffects} import com.gu.identity.IdentityCookieToIdentityUser.{CookieValuesToIdentityUser, IdentityId, IdentityUser} import com.gu.identity.{IdentityCookieToIdentityUser, IdentityTestUserConfig, IsIdentityTestUser} import com.gu.salesforce.SalesforceAuthenticate.{SFAuthConfig, SFAuthTestConfig} -import com.gu.salesforce.SalesforceClient.StringHttpRequest import com.gu.salesforce.SalesforceGenericIdLookup.{FieldName, LookupValue, SfObjectType, TSalesforceGenericIdLookup} import com.gu.salesforce.cases.SalesforceCase import com.gu.salesforce.cases.SalesforceCase.Create.WireNewCase import com.gu.salesforce.cases.SalesforceCase.GetMostRecentCaseByContactId.TGetMostRecentCaseByContactId import com.gu.salesforce.cases.SalesforceCase.{CaseId, CaseSubject, CaseWithId, ContactId, SubscriptionId} -import com.gu.salesforce.{JsonHttp, SalesforceClient, SalesforceGenericIdLookup} +import com.gu.salesforce.{SalesforceClient, SalesforceGenericIdLookup} import com.gu.util.Logging import com.gu.util.apigateway.ApiGatewayHandler.{LambdaIO, Operation} import com.gu.util.apigateway.ResponseModels.ApiResponse @@ -23,9 +22,10 @@ import com.gu.util.apigateway.{ApiGatewayHandler, ApiGatewayRequest, ApiGatewayR import com.gu.util.config.LoadConfigModule.StringFromS3 import com.gu.util.config._ import com.gu.util.reader.Types._ +import com.gu.util.resthttp.JsonHttp.StringHttpRequest import com.gu.util.resthttp.RestRequestMaker.BodyAsString import com.gu.util.resthttp.Types.ClientFailableOp -import com.gu.util.resthttp.{HttpOp, Types} +import com.gu.util.resthttp.{HttpOp, JsonHttp, Types} import okhttp3.{Request, Response} import play.api.libs.json._ diff --git a/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala b/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala index 1cc178ff04..9cd239bf00 100644 --- a/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala +++ b/handlers/cancellation-sf-cases/src/test/scala/com/gu/cancellation/sf_cases/EndToEndHandlerEffectsTest.scala @@ -6,12 +6,12 @@ import com.gu.cancellation.sf_cases.Handler.{CasePathParams, SfBackendForIdentit import com.gu.cancellation.sf_cases.TypeConvert._ import com.gu.effects.{GetFromS3, RawEffects} import com.gu.identity.IdentityCookieToIdentityUser.{IdentityId, IdentityUser} -import com.gu.salesforce.JsonHttp import com.gu.salesforce.cases.SalesforceCase import com.gu.salesforce.cases.SalesforceCase.CaseWithId import com.gu.test.EffectsTest import com.gu.util.apigateway.ApiGatewayHandler.LambdaIO import com.gu.util.apigateway.{ApiGatewayRequest, ApiGatewayResponse} +import com.gu.util.resthttp.JsonHttp import org.scalatest.{FlatSpec, Matchers} import play.api.libs.json._ diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala index d3663e298c..9c43fb4551 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/IdentityClient.scala @@ -1,8 +1,8 @@ package com.gu.identity -import com.gu.salesforce.SalesforceClient.StringHttpRequest import com.gu.util.config.ConfigLocation import com.gu.util.resthttp.HttpOp +import com.gu.util.resthttp.JsonHttp.StringHttpRequest import com.gu.util.resthttp.RestRequestMaker.{BodyAsString, toClientFailableOp} import okhttp3.{HttpUrl, Request, Response} import play.api.libs.json.{Json, Reads} diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala index c684222f9f..3c9a391afe 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala @@ -14,9 +14,8 @@ import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.identityBackfill.salesforce._ import com.gu.identityBackfill.zuora.{AddIdentityIdToAccount, CountZuoraAccountsForIdentityId, GetZuoraAccountsForEmail, GetZuoraSubTypeForAccount} import com.gu.salesforce.SalesforceAuthenticate.SFAuthConfig -import com.gu.salesforce.SalesforceClient.StringHttpRequest +import com.gu.salesforce.SalesforceClient import com.gu.salesforce.TypesForSFEffectsData.SFContactId -import com.gu.salesforce.{JsonHttp, SalesforceClient} import com.gu.util.apigateway.ApiGatewayHandler.{LambdaIO, Operation} import com.gu.util.apigateway.ResponseModels.ApiResponse import com.gu.util.apigateway.{ApiGatewayHandler, ApiGatewayRequest, ApiGatewayResponse, ResponseModels} @@ -24,9 +23,10 @@ import com.gu.util.config.LoadConfigModule.StringFromS3 import com.gu.util.config.{LoadConfigModule, Stage} import com.gu.util.reader.Types.ApiGatewayOp.{ContinueProcessing, ReturnWithResponse} import com.gu.util.reader.Types._ +import com.gu.util.resthttp.JsonHttp.StringHttpRequest import com.gu.util.resthttp.RestRequestMaker.{GetRequest, PatchRequest} import com.gu.util.resthttp.Types.ClientFailableOp -import com.gu.util.resthttp.{HttpOp, LazyClientFailableOp, RestRequestMaker} +import com.gu.util.resthttp.{HttpOp, JsonHttp, LazyClientFailableOp, RestRequestMaker} import com.gu.util.zuora.{ZuoraQuery, ZuoraRestConfig, ZuoraRestRequestMaker} import okhttp3.{Request, Response} import play.api.libs.json.{JsValue, Json, Reads} diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala index 63d09ba5b5..4d932133f1 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/CreateGuestAccountEffectsTest.scala @@ -3,11 +3,10 @@ package com.gu.identity import com.gu.effects.{GetFromS3, RawEffects} import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId -import com.gu.salesforce.JsonHttp import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} import com.gu.util.resthttp.HttpOp.HttpOpWrapper -import com.gu.util.resthttp.RestRequestMaker +import com.gu.util.resthttp.{JsonHttp, RestRequestMaker} import com.gu.util.resthttp.RestRequestMaker.{GetRequestWithParams, RelativePath, UrlParams} import org.scalatest.{FlatSpec, Matchers} import play.api.libs.json.{JsValue, Json, Reads} diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala index be6c68a9db..6206d3e46e 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala @@ -4,9 +4,9 @@ import com.gu.effects.{GetFromS3, RawEffects} import com.gu.identity.GetByEmail.ValidatedEmail import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId -import com.gu.salesforce.JsonHttp import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} +import com.gu.util.resthttp.JsonHttp import org.scalatest.{FlatSpec, Matchers} import scalaz.\/- diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala index 072e237701..f2c1452d97 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/getContact/GetSFContactSyncCheckFieldsEffectsTest.scala @@ -3,11 +3,12 @@ package com.gu.identityBackfill.salesforce.getContact import com.gu.effects.{GetFromS3, RawEffects} import com.gu.identityBackfill.salesforce.GetSFContactSyncCheckFields import com.gu.identityBackfill.salesforce.GetSFContactSyncCheckFields.ContactSyncCheckFields -import com.gu.salesforce.{JsonHttp, SalesforceClient} +import com.gu.salesforce.SalesforceClient import com.gu.salesforce.SalesforceAuthenticate.SFAuthConfig import com.gu.salesforce.dev.SFEffectsData import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} +import com.gu.util.resthttp.JsonHttp import org.scalatest.{FlatSpec, Matchers} import scalaz.\/- diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala index e60a352e84..3c45a47ab1 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/salesforce/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala @@ -3,11 +3,12 @@ package com.gu.identityBackfill.salesforce.updateSFIdentityId import com.gu.effects.{GetFromS3, RawEffects} import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId -import com.gu.salesforce.{JsonHttp, SalesforceClient} +import com.gu.salesforce.SalesforceClient import com.gu.salesforce.SalesforceAuthenticate.SFAuthConfig import com.gu.salesforce.dev.SFEffectsData import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} +import com.gu.util.resthttp.JsonHttp import org.scalatest.{FlatSpec, Matchers} import scalaz.\/- diff --git a/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala b/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala index b2d5d7a95c..8b16de9196 100644 --- a/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala +++ b/handlers/sf-contact-merge/src/main/scala/com/gu/sf_contact_merge/Handler.scala @@ -6,7 +6,7 @@ import com.amazonaws.services.lambda.runtime.Context import com.gu.effects.{GetFromS3, RawEffects} import com.gu.salesforce.SalesforceAuthenticate.SFAuthConfig import com.gu.salesforce.TypesForSFEffectsData.SFContactId -import com.gu.salesforce.{JsonHttp, SalesforceClient} +import com.gu.salesforce.SalesforceClient import com.gu.sf_contact_merge.DomainSteps.MergeRequest import com.gu.sf_contact_merge.SFSteps.GetSfContact import com.gu.sf_contact_merge.TypeConvert._ @@ -22,6 +22,7 @@ import com.gu.util.apigateway.{ApiGatewayHandler, ApiGatewayRequest, ApiGatewayR import com.gu.util.config.LoadConfigModule.StringFromS3 import com.gu.util.config.{LoadConfigModule, Stage} import com.gu.util.reader.Types._ +import com.gu.util.resthttp.JsonHttp import com.gu.util.resthttp.RestOp.HttpOpParseOp import com.gu.util.zuora.SafeQueryBuilder.MaybeNonEmptyList import com.gu.util.zuora.{ZuoraQuery, ZuoraRestConfig, ZuoraRestRequestMaker} diff --git a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala index d32dad007e..7cfb8f22da 100644 --- a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala +++ b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/getsfcontacts/GetSfAddressEffectsTest.scala @@ -3,7 +3,7 @@ package com.gu.sf_contact_merge.getsfcontacts import com.gu.effects.{GetFromS3, RawEffects} import com.gu.salesforce.SalesforceAuthenticate.SFAuthConfig import com.gu.salesforce.dev.SFEffectsData -import com.gu.salesforce.{JsonHttp, SalesforceClient} +import com.gu.salesforce.SalesforceClient import com.gu.sf_contact_merge.Types.IdentityId import com.gu.sf_contact_merge.getaccounts.GetZuoraContactDetails.EmailAddress import com.gu.sf_contact_merge.getsfcontacts.ToSfContactRequest.WireResult @@ -11,6 +11,7 @@ import com.gu.sf_contact_merge.getsfcontacts.WireContactToSfContact.Types._ import com.gu.util.resthttp.RestOp._ import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} +import com.gu.util.resthttp.JsonHttp import org.scalatest.{FlatSpec, Matchers} import scalaz.\/- diff --git a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala index ec47d362b0..b19eacd0dd 100644 --- a/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala +++ b/handlers/sf-contact-merge/src/test/scala/com/gu/sf_contact_merge/update/updateSFIdentityId/UpdateSalesforceIdentityIdEffectsTest.scala @@ -4,7 +4,7 @@ import com.gu.effects.{GetFromS3, RawEffects} import com.gu.salesforce.SalesforceAuthenticate.SFAuthConfig import com.gu.salesforce.TypesForSFEffectsData.SFContactId import com.gu.salesforce.dev.SFEffectsData -import com.gu.salesforce.{JsonHttp, SalesforceClient} +import com.gu.salesforce.SalesforceClient import com.gu.sf_contact_merge.Types.IdentityId import com.gu.sf_contact_merge.getaccounts.GetZuoraContactDetails.{EmailAddress, FirstName} import com.gu.sf_contact_merge.getsfcontacts.GetSfAddressOverride.OverrideAddressWith @@ -14,7 +14,7 @@ import com.gu.sf_contact_merge.update.UpdateSalesforceIdentityId.{SFContactUpdat import com.gu.sf_contact_merge.update.updateSFIdentityId.GetSalesforceIdentityId.WireResult import com.gu.test.EffectsTest import com.gu.util.config.{LoadConfigModule, Stage} -import com.gu.util.resthttp.HttpOp +import com.gu.util.resthttp.{HttpOp, JsonHttp} import com.gu.util.resthttp.RestOp.HttpOpParseOp import com.gu.util.resthttp.RestRequestMaker.{GetRequest, RelativePath} import com.gu.util.resthttp.Types.ClientFailableOp diff --git a/lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala b/lib/restHttp/src/main/scala/com/gu/util/resthttp/JsonHttp.scala similarity index 70% rename from lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala rename to lib/restHttp/src/main/scala/com/gu/util/resthttp/JsonHttp.scala index bdbe8f4019..a56291cbd3 100644 --- a/lib/salesforce/src/main/scala/com/gu/salesforce/JsonHttp.scala +++ b/lib/restHttp/src/main/scala/com/gu/util/resthttp/JsonHttp.scala @@ -1,15 +1,30 @@ -package com.gu.salesforce +package com.gu.util.resthttp -import com.gu.salesforce.SalesforceClient._ import com.gu.util.resthttp.HttpOp.HttpOpWrapper import com.gu.util.resthttp.RestRequestMaker._ import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess, GenericError} +import okhttp3.Request import play.api.libs.json.{JsValue, Json} import scala.util.Try object JsonHttp { + sealed trait RequestMethod { + def builder: Request.Builder + } + case class PostMethod(body: BodyAsString) extends RequestMethod { + override def builder: Request.Builder = new Request.Builder().post(createBodyFromString(body)) + } + case class PatchMethod(body: BodyAsString) extends RequestMethod { + override def builder: Request.Builder = new Request.Builder().patch(createBodyFromString(body)) + } + case object GetMethod extends RequestMethod { + override def builder: Request.Builder = new Request.Builder().get() + } + + case class StringHttpRequest(requestMethod: RequestMethod, relativePath: RelativePath, urlParams: UrlParams) + val patch = HttpOpWrapper[PatchRequest, StringHttpRequest, BodyAsString, Unit]( (patchRequest: PatchRequest) => diff --git a/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala b/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala index d7b9f5ef98..1faaa55846 100644 --- a/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala +++ b/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala @@ -1,6 +1,7 @@ package com.gu.salesforce import com.gu.salesforce.SalesforceAuthenticate.{SFAuthConfig, SalesforceAuth} +import com.gu.util.resthttp.JsonHttp.StringHttpRequest import com.gu.util.resthttp.RestRequestMaker._ import com.gu.util.resthttp.{HttpOp, LazyClientFailableOp} import okhttp3.{Request, Response} @@ -26,19 +27,4 @@ object SalesforceClient { authedBuilder.url(url).build() } - sealed trait RequestMethod { - def builder: Request.Builder - } - case class PostMethod(body: BodyAsString) extends RequestMethod { - override def builder: Request.Builder = new Request.Builder().post(createBodyFromString(body)) - } - case class PatchMethod(body: BodyAsString) extends RequestMethod { - override def builder: Request.Builder = new Request.Builder().patch(createBodyFromString(body)) - } - case object GetMethod extends RequestMethod { - override def builder: Request.Builder = new Request.Builder().get() - } - - case class StringHttpRequest(requestMethod: RequestMethod, relativePath: RelativePath, urlParams: UrlParams) - } From 1bd69b72111459df817e1ef83755cca4cc984340 Mon Sep 17 00:00:00 2001 From: John Duffell Date: Wed, 31 Oct 2018 10:09:45 +0000 Subject: [PATCH 4/8] remove zuora references from generic code --- .../scala/com/gu/util/resthttp/RestRequestMaker.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala b/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala index 79b441f886..d43a7ce59c 100644 --- a/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala +++ b/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala @@ -17,10 +17,10 @@ object RestRequestMaker extends Logging { val body = response.body.string val truncated = body.take(500) + (if (body.length > 500) "..." else "") - logger.error(s"Request to Zuora was unsuccessful, response status was ${response.code}, response body: \n $response\n$truncated") + logger.error(s"HTTP request was unsuccessful, response status was ${response.code}, response body: \n $response\n$truncated") if (response.code == 404) { NotFound(response.message) - } else GenericError("Request to Zuora was unsuccessful") + } else GenericError("HTTP request was unsuccessful") } } @@ -29,8 +29,8 @@ object RestRequestMaker extends Logging { case success: JsSuccess[T] => ClientSuccess(success.get) case error: JsError => { - logger.error(s"Failed to convert Zuora response to case case $error. Response body was: \n $bodyAsJson") - GenericError(s"Error when converting Zuora response to case class: $error") + logger.error(s"Failed to convert JSON response to case case $error. Response body was: \n $bodyAsJson") + GenericError(s"Error when converting JSON response to case class: $error") } } } From 0633c52845f21d319221fd282e38a6aab8b0c341 Mon Sep 17 00:00:00 2001 From: John Duffell Date: Wed, 31 Oct 2018 10:28:12 +0000 Subject: [PATCH 5/8] better naming for identity account trait --- .../main/scala/com/gu/identity/GetByEmail.scala | 16 ++++++++-------- .../scala/com/gu/identityBackfill/Handler.scala | 4 ++-- .../com/gu/identityBackfill/PreReqCheck.scala | 8 ++++---- .../com/gu/identity/GetByEmailEffectsTest.scala | 4 ++-- .../scala/com/gu/identity/GetByEmailTest.scala | 6 +++--- .../gu/identityBackfill/PreReqCheckTest.scala | 8 ++++---- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala index dd978b5166..59849617ed 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala @@ -11,9 +11,9 @@ import play.api.libs.json.{JsValue, Json, Reads} object GetByEmail { - sealed trait MaybeValidatedEmail - case class ValidatedEmail(identityId: IdentityId) extends MaybeValidatedEmail - case object NotValidated extends MaybeValidatedEmail + sealed trait IdentityAccount + case class IdentityAccountWithValidatedEmail(identityId: IdentityId) extends IdentityAccount + case object IdentityAccountWithUnvalidatedEmail extends IdentityAccount object RawWireModel { @@ -28,8 +28,8 @@ object GetByEmail { } - val wrapper: HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, MaybeValidatedEmail] = - HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, MaybeValidatedEmail]( + val wrapper: HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityAccount] = + HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityAccount]( emailAddress => GetRequestWithParams(RelativePath(s"/user"), UrlParams(Map("emailAddress" -> emailAddress.value))), RestRequestMaker.toResult[UserResponse](_).flatMap(toResponse) ) @@ -37,13 +37,13 @@ object GetByEmail { def userFromResponse(userResponse: UserResponse): ClientFailableOp[User] = userResponse match { case UserResponse("ok", user) => ClientSuccess(user) - case _ => GenericError("not an OK response from api") + case error => GenericError(s"not an OK response from api: $error") } - def toResponse(userResponse: UserResponse): ClientFailableOp[MaybeValidatedEmail] = { + def toResponse(userResponse: UserResponse): ClientFailableOp[IdentityAccount] = { for { user <- userFromResponse(userResponse) - identityId = if (user.statusFields.userEmailValidated) ValidatedEmail(IdentityId(user.id)) else NotValidated + identityId = if (user.statusFields.userEmailValidated) IdentityAccountWithValidatedEmail(IdentityId(user.id)) else IdentityAccountWithUnvalidatedEmail } yield identityId } diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala index 3c9a391afe..a5d67df953 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/Handler.scala @@ -134,7 +134,7 @@ object Handler { object Healthcheck { def apply( - getByEmail: HttpOp[EmailAddress, GetByEmail.MaybeValidatedEmail], + getByEmail: HttpOp[EmailAddress, GetByEmail.IdentityAccount], countZuoraAccountsForIdentityId: IdentityId => ClientFailableOp[Int], sfAuth: LazyClientFailableOp[Any] ): ApiResponse = @@ -142,7 +142,7 @@ object Healthcheck { maybeIdentityId <- getByEmail.runRequest(EmailAddress("john.duffell@guardian.co.uk")) .toApiGatewayOp("problem with email").withLogging("healthcheck getByEmail") identityId <- maybeIdentityId match { - case GetByEmail.ValidatedEmail(identityId) => ContinueProcessing(identityId) + case GetByEmail.IdentityAccountWithValidatedEmail(identityId) => ContinueProcessing(identityId) case other => logger.error(s"failed healthcheck with $other") ReturnWithResponse(ApiGatewayResponse.internalServerError("test identity id was not present")) diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala index 646bfc04f4..f1ab51214b 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identityBackfill/PreReqCheck.scala @@ -1,7 +1,7 @@ package com.gu.identityBackfill import com.gu.identity.GetByEmail -import com.gu.identity.GetByEmail.NotValidated +import com.gu.identity.GetByEmail.IdentityAccountWithUnvalidatedEmail import com.gu.identityBackfill.TypeConvert._ import com.gu.identityBackfill.Types._ import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId @@ -19,7 +19,7 @@ object PreReqCheck { case class PreReqResult(zuoraAccountId: Types.AccountId, sFContactId: SFContactId, existingIdentityId: Option[IdentityId]) def apply( - getByEmail: EmailAddress => ClientFailableOp[GetByEmail.MaybeValidatedEmail], + getByEmail: EmailAddress => ClientFailableOp[GetByEmail.IdentityAccount], getSingleZuoraAccountForEmail: EmailAddress => ApiGatewayOp[ZuoraAccountIdentitySFContact], noZuoraAccountsForIdentityId: IdentityId => ApiGatewayOp[Unit], zuoraSubType: AccountId => ApiGatewayOp[Unit], @@ -27,9 +27,9 @@ object PreReqCheck { )(emailAddress: EmailAddress): ApiGatewayOp[PreReqResult] = { for { maybeExistingIdentityId <- (getByEmail(emailAddress) match { - case ClientSuccess(GetByEmail.ValidatedEmail(identityId)) => ContinueProcessing(Some(identityId)) + case ClientSuccess(GetByEmail.IdentityAccountWithValidatedEmail(identityId)) => ContinueProcessing(Some(identityId)) case NotFound(_) => ContinueProcessing(None) - case ClientSuccess(NotValidated) => ReturnWithResponse(ApiGatewayResponse.notFound("identity email not validated")) + case ClientSuccess(IdentityAccountWithUnvalidatedEmail) => ReturnWithResponse(ApiGatewayResponse.notFound("identity email not validated")) case other: ClientFailure => ReturnWithResponse(ApiGatewayResponse.internalServerError(other.toString)) }).withLogging("GetByEmail") zuoraAccountForEmail <- getSingleZuoraAccountForEmail(emailAddress) diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala index 6206d3e46e..f592215c01 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailEffectsTest.scala @@ -1,7 +1,7 @@ package com.gu.identity import com.gu.effects.{GetFromS3, RawEffects} -import com.gu.identity.GetByEmail.ValidatedEmail +import com.gu.identity.GetByEmail.IdentityAccountWithValidatedEmail import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId import com.gu.test.EffectsTest @@ -23,7 +23,7 @@ class GetByEmailEffectsTest extends FlatSpec with Matchers { getByEmail = identityClient.wrapWith(JsonHttp.getWithParams).wrapWith(GetByEmail.wrapper) identityId <- getByEmail.runRequest(EmailAddress("john.duffell@guardian.co.uk")).toDisjunction } yield identityId - actual should be(\/-(ValidatedEmail(IdentityId("21814163")))) + actual should be(\/-(IdentityAccountWithValidatedEmail(IdentityId("21814163")))) } diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala index d8869176d1..f159535ac5 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identity/GetByEmailTest.scala @@ -1,6 +1,6 @@ package com.gu.identity -import com.gu.identity.GetByEmail.{NotValidated, ValidatedEmail} +import com.gu.identity.GetByEmail.{IdentityAccountWithUnvalidatedEmail, IdentityAccountWithValidatedEmail} import com.gu.identity.GetByEmailTest.{NotValidatedTestData, TestData} import com.gu.identityBackfill.Types.EmailAddress import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId @@ -20,13 +20,13 @@ class GetByEmailTest extends FlatSpec with Matchers { it should "get successful ok" in { val actual = GetByEmail.wrapper.toNewResponse(Json.parse(TestData.dummyIdentityResponse)) - actual should be(ClientSuccess(ValidatedEmail(IdentityId("1234")))) + actual should be(ClientSuccess(IdentityAccountWithValidatedEmail(IdentityId("1234")))) } it should "get not validated with an error" in { val actual = GetByEmail.wrapper.toNewResponse(Json.parse(NotValidatedTestData.dummyIdentityResponse)) - actual should be(ClientSuccess(NotValidated)) + actual should be(ClientSuccess(IdentityAccountWithUnvalidatedEmail)) } } diff --git a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala index f96e095604..b1984df3f8 100644 --- a/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala +++ b/handlers/identity-backfill/src/test/scala/com/gu/identityBackfill/PreReqCheckTest.scala @@ -1,7 +1,7 @@ package com.gu.identityBackfill import com.gu.identity.GetByEmail -import com.gu.identity.GetByEmail.{NotValidated, ValidatedEmail} +import com.gu.identity.GetByEmail.{IdentityAccountWithUnvalidatedEmail, IdentityAccountWithValidatedEmail} import com.gu.identityBackfill.PreReqCheck.PreReqResult import com.gu.identityBackfill.Types._ import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId @@ -19,7 +19,7 @@ class PreReqCheckTest extends FlatSpec with Matchers { val result = PreReqCheck( - email => ClientSuccess(ValidatedEmail(IdentityId("asdf"))), + email => ClientSuccess(IdentityAccountWithValidatedEmail(IdentityId("asdf"))), email => ContinueProcessing(ZuoraAccountIdentitySFContact(AccountId("acc"), None, SFContactId("sf"))), identityId => ContinueProcessing(()), _ => ContinueProcessing(()), @@ -83,13 +83,13 @@ class PreReqCheckTest extends FlatSpec with Matchers { it should "stop processing if it finds a non validated identity account" in { - val result = emailCheckFailure(ClientSuccess(NotValidated)) + val result = emailCheckFailure(ClientSuccess(IdentityAccountWithUnvalidatedEmail)) val expectedResult = ReturnWithResponse(ApiGatewayResponse.notFound("identity email not validated")) result should be(expectedResult) } - private def emailCheckFailure(identityError: ClientFailableOp[GetByEmail.MaybeValidatedEmail]) = { + private def emailCheckFailure(identityError: ClientFailableOp[GetByEmail.IdentityAccount]) = { PreReqCheck( email => identityError, email => fail("shouldn't be called 1"), From 8b6e02ece2025a8a3b23e23f3e7b1a5ba5d4820a Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Wed, 31 Oct 2018 17:45:34 +0000 Subject: [PATCH 6/8] typo in error message Co-Authored-By: johnduffell --- .../src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala b/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala index d43a7ce59c..3134be64ec 100644 --- a/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala +++ b/lib/restHttp/src/main/scala/com/gu/util/resthttp/RestRequestMaker.scala @@ -29,7 +29,7 @@ object RestRequestMaker extends Logging { case success: JsSuccess[T] => ClientSuccess(success.get) case error: JsError => { - logger.error(s"Failed to convert JSON response to case case $error. Response body was: \n $bodyAsJson") + logger.error(s"Failed to convert JSON response to case class $error. Response body was: \n $bodyAsJson") GenericError(s"Error when converting JSON response to case class: $error") } } From 262905efa89b3d121e934c4445f8dfe123a2274c Mon Sep 17 00:00:00 2001 From: John Duffell Date: Wed, 31 Oct 2018 18:06:22 +0000 Subject: [PATCH 7/8] rejig the http wrapper to be better separated --- .../main/scala/com/gu/identity/GetByEmail.scala | 17 +++++++++++------ .../com/gu/salesforce/SalesforceClient.scala | 6 +++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala index 59849617ed..a9a220f426 100644 --- a/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala +++ b/handlers/identity-backfill/src/main/scala/com/gu/identity/GetByEmail.scala @@ -28,11 +28,10 @@ object GetByEmail { } - val wrapper: HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityAccount] = - HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityAccount]( - emailAddress => GetRequestWithParams(RelativePath(s"/user"), UrlParams(Map("emailAddress" -> emailAddress.value))), - RestRequestMaker.toResult[UserResponse](_).flatMap(toResponse) - ) + def emailAddressToParams(emailAddress: EmailAddress): GetRequestWithParams = + GetRequestWithParams(RelativePath(s"/user"), UrlParams(Map("emailAddress" -> emailAddress.value))) + + val jsToWireModel: JsValue => ClientFailableOp[UserResponse] = RestRequestMaker.toResult[UserResponse] def userFromResponse(userResponse: UserResponse): ClientFailableOp[User] = userResponse match { @@ -40,7 +39,7 @@ object GetByEmail { case error => GenericError(s"not an OK response from api: $error") } - def toResponse(userResponse: UserResponse): ClientFailableOp[IdentityAccount] = { + def wireToDomainModel(userResponse: UserResponse): ClientFailableOp[IdentityAccount] = { for { user <- userFromResponse(userResponse) identityId = if (user.statusFields.userEmailValidated) IdentityAccountWithValidatedEmail(IdentityId(user.id)) else IdentityAccountWithUnvalidatedEmail @@ -48,4 +47,10 @@ object GetByEmail { } + val wrapper: HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityAccount] = + HttpOpWrapper[EmailAddress, GetRequestWithParams, JsValue, IdentityAccount]( + fromNewParam = emailAddressToParams, + toNewResponse = jsToWireModel.andThen(_.flatMap(wireToDomainModel)) + ) + } diff --git a/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala b/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala index 1faaa55846..9ee0c00bc9 100644 --- a/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala +++ b/lib/salesforce/src/main/scala/com/gu/salesforce/SalesforceClient.scala @@ -9,11 +9,11 @@ import okhttp3.{Request, Response} object SalesforceClient { def apply( - response: Request => Response, + getResponse: Request => Response, config: SFAuthConfig ): LazyClientFailableOp[HttpOp[StringHttpRequest, BodyAsString]] = - SalesforceAuthenticate(response)(config).map { sfAuth => - HttpOp(response).flatMap { + SalesforceAuthenticate(getResponse)(config).map { sfAuth => + HttpOp(getResponse).flatMap { toClientFailableOp }.setupRequest[StringHttpRequest] { withAuth(sfAuth) From bf4c07c1d99631cb9b70b0497668a4419b0b5e8a Mon Sep 17 00:00:00 2001 From: John Duffell Date: Wed, 31 Oct 2018 18:23:59 +0000 Subject: [PATCH 8/8] update tests for new wording --- .../src/test/scala/com/gu/util/RestRequestMakerTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/restHttp/src/test/scala/com/gu/util/RestRequestMakerTest.scala b/lib/restHttp/src/test/scala/com/gu/util/RestRequestMakerTest.scala index b3f175d6b7..7f8a59790d 100644 --- a/lib/restHttp/src/test/scala/com/gu/util/RestRequestMakerTest.scala +++ b/lib/restHttp/src/test/scala/com/gu/util/RestRequestMakerTest.scala @@ -72,13 +72,13 @@ class RestRequestMakerTest extends AsyncFlatSpec { "convertResponseToCaseClass" should "return a left[String] for an unsuccessful response code" in { val response = constructTestResponse(500) val either = RestRequestMaker.httpIsSuccessful(response) - assert(either == GenericError("Request to Zuora was unsuccessful")) + assert(either == GenericError("HTTP request was unsuccessful")) } it should "return a left[String] if the body of a successful response cannot be de-serialized to that case class" in { val either = RestRequestMaker.toResult[BasicAccountInfo](validZuoraNoOtherFields) val result = either.mapFailure(first => GenericError(first.message.split(":")(0))) - assert(result == GenericError("Error when converting Zuora response to case class")) + assert(result == GenericError("Error when converting JSON response to case class")) } it should "return a right[T] if the body of a successful response deserializes to T" in {