Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear Up Scala Warnings #6383

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions .scalafix.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
rules = [
ExplicitResultTypes,
]
ExplicitResultTypes.skipSimpleDefinitions = false
2 changes: 2 additions & 0 deletions project/plugins.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ libraryDependencies += "org.vafer" % "jdeb" % "1.10" artifacts (Artifact("jdeb",

addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.2")

addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.13.0")

addSbtPlugin("com.gu" % "sbt-riffraff-artifact" % "1.1.18")

addSbtPlugin("com.github.sbt" % "sbt-native-packager" % "1.9.16")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import com.gu.supporterdata.model.Stage
case class PatronsIdentityConfig(apiUrl: String, apiClientToken: String)

object PatronsIdentityConfig extends ConfigService {
def fromParameterStoreSync(stage: Stage) = {
def fromParameterStoreSync(stage: Stage): PatronsIdentityConfig = {
logger.info(s"Loading Identity config")
val params = ParameterStoreService(stage).getParametersByPathSync("identity-config")
PatronsIdentityConfig(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration.{Duration, DurationInt, MINUTES}
import scala.jdk.CollectionConverters.MapHasAsScala
import scala.util.{Failure, Success, Try}
import okhttp3.{Request, Response}

class PatronCancelledEventLambda extends StrictLogging {
val runner = configurableFutureRunner(60.seconds)
val runner: Request => Future[Response] = configurableFutureRunner(60.seconds)

implicit val stage: Stage = StageConstructors.fromEnvironment
private val stripeConfig = PatronsStripeConfig.fromParameterStoreSync(stage)
Expand Down Expand Up @@ -76,7 +77,7 @@ class PatronCancelledEventLambda extends StrictLogging {

def cancelSubscriptionForEmail(email: String, subscriptionId: String, identityConfig: PatronsIdentityConfig)(implicit
stage: Stage,
) = for {
): EitherT[Future, CancelError, Unit] = for {
identityId <- getIdentityId(identityConfig, email)
_ <- cancelSubscription(stage, identityId, subscriptionId)
} yield ()
Expand All @@ -96,7 +97,7 @@ class PatronCancelledEventLambda extends StrictLogging {
EitherT.pure(())
}

def getAPIGatewayResult(result: Either[CancelError, Unit]) = {
def getAPIGatewayResult(result: Either[CancelError, Unit]): APIGatewayProxyResponseEvent = {
val response = new APIGatewayProxyResponseEvent()
result match {
case Left(error @ (ConfigLoadingError(_) | DynamoDbError(_))) =>
Expand Down Expand Up @@ -211,8 +212,8 @@ class PatronCancelledEventLambda extends StrictLogging {
}

case class PatronCancelledEvent(data: PatronCancelledData, `type`: String) {
def customerId = data.`object`.customer
def subscriptionId = data.`object`.id
def customerId: String = data.`object`.customer
def subscriptionId: String = data.`object`.id
}
object PatronCancelledEvent {
implicit val decoder: Decoder[PatronCancelledEvent] = deriveDecoder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration.{Duration, DurationInt, MINUTES}
import scala.jdk.CollectionConverters.MapHasAsScala
import scala.util.{Failure, Success, Try}
import okhttp3.{Request, Response}

class PatronSignUpEventLambda extends StrictLogging {
val runner = configurableFutureRunner(60.seconds)
val runner: Request => Future[Response] = configurableFutureRunner(60.seconds)

implicit val stage: Stage = StageConstructors.fromEnvironment
private val stripeConfig = PatronsStripeConfig.fromParameterStoreSync(stage)
Expand Down Expand Up @@ -77,7 +78,7 @@ class PatronSignUpEventLambda extends StrictLogging {
} yield unit).value.map(getAPIGatewayResult(_))
}

def getAPIGatewayResult(result: Either[SignUpError, Unit]) = {
def getAPIGatewayResult(result: Either[SignUpError, Unit]): APIGatewayProxyResponseEvent = {
val response = new APIGatewayProxyResponseEvent()
result match {
case Left(error @ (ConfigLoadingError(_) | SubscriptionProcessingError(_) | StripeGetCustomerFailedError(_))) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ import com.gu.supporterdata.services.SupporterDataDynamoService
import scala.concurrent.Await
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration.{Duration, DurationInt, MILLISECONDS}
import scala.concurrent.Future

class ProcessStripeSubscriptionsLambda extends RequestHandler[Unit, Unit] {

private val stage = StageConstructors.fromEnvironment
private val stripeConfig = PatronsStripeConfig.fromParameterStoreSync(stage)
private val identityConfig = PatronsIdentityConfig.fromParameterStoreSync(stage)

override def handleRequest(input: Unit, context: Context) = {
override def handleRequest(input: Unit, context: Context): Unit = {
val account = GnmPatronScheme // TODO: allow this to be set by the caller
Await.result(
processSubscriptions(account, stage, stripeConfig, identityConfig),
Expand All @@ -42,7 +43,7 @@ object ProcessStripeSubscriptionsLambda {
stage: Stage,
stripeConfig: PatronsStripeConfig,
identityConfig: PatronsIdentityConfig,
) = {
): Future[Unit] = {
val runner = configurableFutureRunner(60.seconds)
val stripeService = new PatronsStripeService(stripeConfig, runner)
val identityService = new PatronsIdentityService(identityConfig, runner)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ sealed trait StripeCustomer
@ConfiguredJsonCodec
case class ExpandedStripeCustomer(id: String, name: Option[String], email: String, metadata: Metadata)
extends StripeCustomer {
val jointPatronEmail = metadata.jointPatronEmail
val jointPatronName = metadata.jointPatronName
val jointPatronEmail: Option[String] = metadata.jointPatronEmail
val jointPatronName: Option[String] = metadata.jointPatronName
}

case class UnexpandedStripeCustomer(id: String) extends StripeCustomer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ object IdentityErrorResponse {

object IdentityError {
object InvalidEmailAddress {
val message = "Registration Error"
val description = "Please sign up using an email address from a different provider"
val errorReasonCode = "invalid_email_address"
val message: String = "Registration Error"
val description: String = "Please sign up using an email address from a different provider"
val errorReasonCode: String = "invalid_email_address"
}

def isDisallowedEmailError(identityError: IdentityError): Boolean =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import com.typesafe.scalalogging.StrictLogging

trait ConfigService extends StrictLogging {

protected def findParameterOrThrow(name: String, params: List[Parameter]) =
protected def findParameterOrThrow(name: String, params: List[Parameter]): String =
findParameterValue(name, params).getOrElse {
val error = s"Missing config value for parameter $name"
logger.error(error)
throw new RuntimeException(error)
}

protected def findParameterValue(name: String, params: List[Parameter]) =
protected def findParameterValue(name: String, params: List[Parameter]): Option[String] =
params
.find(_.getName.split('/').last == name)
.map(_.getValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ import com.gu.supporterdata.model.Stage

import scala.concurrent.ExecutionContext
import scala.jdk.CollectionConverters._
import com.amazonaws.services.simplesystemsmanagement.model.{Parameter, PutParameterResult}
import scala.concurrent.Future

class ParameterStoreService(client: AWSSimpleSystemsManagementAsync, stage: Stage) {
val configRoot = s"/$stage/support/stripe-patrons-data"
val configRoot: String = s"/$stage/support/stripe-patrons-data"

def getParametersByPath(path: String)(implicit executionContext: ExecutionContext) = {
def getParametersByPath(path: String)(implicit executionContext: ExecutionContext): Future[List[Parameter]] = {
val request: GetParametersByPathRequest = new GetParametersByPathRequest()
.withPath(s"$configRoot/$path/")
.withRecursive(false)
Expand All @@ -37,7 +39,7 @@ class ParameterStoreService(client: AWSSimpleSystemsManagementAsync, stage: Stag
AwsAsync(client.getParametersByPathAsync, request).map(_.getParameters.asScala.toList)
}

def getParametersByPathSync(path: String) = {
def getParametersByPathSync(path: String): List[Parameter] = {
val request: GetParametersByPathRequest = new GetParametersByPathRequest()
.withPath(s"$configRoot/$path/")
.withRecursive(false)
Expand All @@ -46,15 +48,19 @@ class ParameterStoreService(client: AWSSimpleSystemsManagementAsync, stage: Stag
client.getParametersByPath(request).getParameters.asScala.toList
}

def getParameter(name: String)(implicit executionContext: ExecutionContext) = {
def getParameter(name: String)(implicit executionContext: ExecutionContext): Future[String] = {
val request = new GetParameterRequest()
.withName(s"$configRoot/$name")
.withWithDecryption(true)

AwsAsync(client.getParameterAsync, request).map(_.getParameter.getValue)
}

def putParameter(name: String, value: String, parameterType: ParameterType = ParameterType.String) = {
def putParameter(
name: String,
value: String,
parameterType: ParameterType = ParameterType.String,
): Future[PutParameterResult] = {

val putParameterRequest = new PutParameterRequest()
.withName(s"$configRoot/$name")
Expand All @@ -70,18 +76,18 @@ class ParameterStoreService(client: AWSSimpleSystemsManagementAsync, stage: Stag
object ParameterStoreService {

// please update to AWS SDK 2 and use com.gu.aws.CredentialsProvider
lazy val CredentialsProviderDEPRECATEDV1 = new AWSCredentialsProviderChain(
lazy val CredentialsProviderDEPRECATEDV1: AWSCredentialsProviderChain = new AWSCredentialsProviderChain(
new ProfileCredentialsProvider(ProfileName),
new InstanceProfileCredentialsProvider(false),
new EnvironmentVariableCredentialsProvider(),
new EC2ContainerCredentialsProviderWrapper(), // for use with lambda snapstart
)

lazy val client = AWSSimpleSystemsManagementAsyncClientBuilder
lazy val client: AWSSimpleSystemsManagementAsync = AWSSimpleSystemsManagementAsyncClientBuilder
.standard()
.withRegion(Regions.EU_WEST_1)
.withCredentials(CredentialsProviderDEPRECATEDV1)
.build()

def apply(stage: Stage) = new ParameterStoreService(client, stage)
def apply(stage: Stage): ParameterStoreService = new ParameterStoreService(client, stage)
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import scala.concurrent.{ExecutionContext, Future}

class PatronsIdentityService(val config: PatronsIdentityConfig, client: FutureHttpClient)
extends WebServiceHelper[IdentityErrorResponse] {
override val wsUrl = config.apiUrl
override val httpClient = client
val authHeader = Map(
override val wsUrl: String = config.apiUrl
override val httpClient: FutureHttpClient = client
val authHeader: Map[String, String] = Map(
"Authorization" -> s"Bearer ${config.apiClientToken}",
)

Expand Down Expand Up @@ -45,7 +45,7 @@ class PatronsIdentityService(val config: PatronsIdentityConfig, client: FutureHt
def createUserIdFromEmailUser(
email: String,
firstName: Option[String],
)(implicit ec: ExecutionContext) = {
)(implicit ec: ExecutionContext): Future[String] = {
logger.info(s"Attempting to create guest identity account for user $email")
val body = CreateGuestAccountRequestBody(
email,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import scala.concurrent.{ExecutionContext, Future}
class PatronsStripeService(val config: PatronsStripeConfig, client: FutureHttpClient)(implicit
ec: ExecutionContext,
) extends WebServiceHelper[StripeError] {
override val wsUrl = "https://api.stripe.com/v1"
override val httpClient = client
override val wsUrl: String = "https://api.stripe.com/v1"
override val httpClient: FutureHttpClient = client

def authHeader(account: PatronsStripeAccount): Map[String, String] = Map(
"Authorization" -> s"Bearer ${account match {
Expand All @@ -38,7 +38,7 @@ class PatronsStripeService(val config: PatronsStripeConfig, client: FutureHttpCl
)
}

def getCustomer(account: PatronsStripeAccount, customerId: String) =
def getCustomer(account: PatronsStripeAccount, customerId: String): Future[ExpandedStripeCustomer] =
get[ExpandedStripeCustomer](
s"customers/$customerId",
authHeader(account),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class StripeSubscriptionsProcessor(
executionContext: ExecutionContext,
) extends SafeLogging {

def processSubscriptions(account: PatronsStripeAccount, pageSize: Int) =
def processSubscriptions(account: PatronsStripeAccount, pageSize: Int): Future[Unit] =
processFutureResponse(account, pageSize, None)

final def processFutureResponse(
Expand All @@ -33,7 +33,7 @@ class StripeSubscriptionsProcessor(
Future.successful(())
} yield result

def processSubs(list: List[StripeSubscription[ExpandedStripeCustomer]]) = {
def processSubs(list: List[StripeSubscription[ExpandedStripeCustomer]]): Future[List[Unit]] = {
val unknownEmailViaCSR = "[email protected]"
Future.sequence(
list
Expand All @@ -56,7 +56,7 @@ abstract class DynamoProcessor(
with SafeLogging {
import scala.concurrent.ExecutionContext.Implicits.global

def writeToDynamo(identityId: String, sub: StripeSubscription[ExpandedStripeCustomer]) = {
def writeToDynamo(identityId: String, sub: StripeSubscription[ExpandedStripeCustomer]): Future[UpdateItemResponse] = {
logger.info(
s"Attempting to write subscription (${sub.customer.email}, $identityId) to Dynamo",
)
Expand All @@ -75,7 +75,7 @@ abstract class DynamoProcessor(
)
}

def logDynamoResult(email: String, addSubscriptionsToQueueState: UpdateItemResponse) =
def logDynamoResult(email: String, addSubscriptionsToQueueState: UpdateItemResponse): Unit =
if (addSubscriptionsToQueueState.sdkHttpResponse().statusCode() == 200)
logger.info(s"Dynamo record successfully written for $email")
else
Expand All @@ -90,7 +90,7 @@ class CreateMissingIdentityProcessor(
) extends DynamoProcessor(supporterDataDynamoService) {
import scala.concurrent.ExecutionContext.Implicits.global

override def processSubscription(subscription: StripeSubscription[ExpandedStripeCustomer]) =
override def processSubscription(subscription: StripeSubscription[ExpandedStripeCustomer]): Future[Unit] =
for {
_ <- processCustomerEmail(subscription.customer.email, subscription.customer.name, subscription)
_ <- maybeAddJointPatron(subscription)
Expand All @@ -110,7 +110,7 @@ class CreateMissingIdentityProcessor(
()
}

def maybeAddJointPatron(subscription: StripeSubscription[ExpandedStripeCustomer]) =
def maybeAddJointPatron(subscription: StripeSubscription[ExpandedStripeCustomer]): Future[Unit] =
subscription.customer.jointPatronEmail match {
case Some(email) =>
logger.info(s"Customer ${subscription.customer.email} has an associated joint patron - $email")
Expand All @@ -127,7 +127,7 @@ class SkipMissingIdentityProcessor(
) extends DynamoProcessor(supporterDataDynamoService) {
import scala.concurrent.ExecutionContext.Implicits.global

override def processSubscription(subscription: StripeSubscription[ExpandedStripeCustomer]) =
override def processSubscription(subscription: StripeSubscription[ExpandedStripeCustomer]): Future[Unit] =
identityService.getUserIdFromEmail(subscription.customer.email).map {
case Some(identityId) =>
writeToDynamo(identityId, subscription).map(response => logDynamoResult(subscription.customer.email, response))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.gu.patrons.services

object Fixtures {
val subscriptionJson = """
val subscriptionJson: String = """
{
"id": "sub_HHdivtqWAZRZLT",
"object": "subscription",
Expand Down Expand Up @@ -363,7 +363,7 @@ object Fixtures {
}
"""

val patronCancelledEventJson =
val patronCancelledEventJson: String =
"""
{
"id": "evt_1LudpBJETvkRwpwqmTN4BiHf",
Expand Down Expand Up @@ -492,7 +492,7 @@ object Fixtures {
}
"""

val patronSignUpEventJson = """
val patronSignUpEventJson: String = """
{
"id": "evt_1MX3M1JETvkRwpwqAQ6JAXKW",
"object": "event",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.scalatest.flatspec.AsyncFlatSpec
import org.scalatest.matchers.should.Matchers

import scala.concurrent.duration.DurationInt
import scala.concurrent.Future

@IntegrationTest
class StripeSubscriptionsProcessorSpec extends AsyncFlatSpec with Matchers {
Expand All @@ -28,7 +29,7 @@ class StripeSubscriptionsProcessorSpec extends AsyncFlatSpec with Matchers {
}

class LoggingSubscriptionProcessor(identityService: PatronsIdentityService) extends SubscriptionProcessor {
override def processSubscription(subscription: StripeSubscription[ExpandedStripeCustomer]) =
override def processSubscription(subscription: StripeSubscription[ExpandedStripeCustomer]): Future[Unit] =
identityService
.getUserIdFromEmail(subscription.customer.email)
.map(maybeIdentityId =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ object SafeLogger {
val sanitizedLogMessage: Marker = MarkerFactory.getMarker("SENTRY")

case class LogMessage(withPersonalData: String, withoutPersonalData: String) {
override val toString = withoutPersonalData
override val toString: String = withoutPersonalData
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import com.typesafe.config.Config
case class PriceSummaryConfig(catalogConfig: CatalogConfig, promotionsConfig: PromotionsConfig)
class PriceSummaryConfigProvider(config: Config, defaultStage: Stage)
extends TouchpointConfigProvider[PriceSummaryConfig](config, defaultStage) {
override protected def fromConfig(config: Config) = {
override protected def fromConfig(config: Config): PriceSummaryConfig = {
PriceSummaryConfig(
CatalogConfig.fromConfig(config),
PromotionsConfig.fromConfig(config),
Expand Down
2 changes: 1 addition & 1 deletion support-frontend/app/actions/CacheControl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import HttpHeaders._

object CacheControl {

val defaultStaleIfErrors = 10.days
val defaultStaleIfErrors: FiniteDuration = 10.days

def browser(maxAge: FiniteDuration): (String, String) = {
"Cache-Control" -> standardDirectives(maxAge, 1.second max (maxAge / 10), defaultStaleIfErrors)
Expand Down
Loading