Merge pull request #4355 from guardian/an/scala2.13-syntax

Upgrades to syntax and types to prepare for scala 2.13 upgrade

build.sbt

@@ -34,6 +34,7 @@ val commonSettings = Seq(
 
   Test / testOptions ++= Seq(Tests.Argument(TestFrameworks.ScalaTest, "-o"), Tests.Argument(TestFrameworks.ScalaTest, "-u", "logs/test-reports")),
   libraryDependencies ++= Seq(
+    "org.scalatest" %% "scalatest" % "3.2.19" % Test,
     "org.scalatestplus.play" %% "scalatestplus-play" % "5.1.0" % Test,
     "org.scalatestplus" %% "mockito-3-4" % "3.1.4.0" % Test,
     "org.mockito" % "mockito-core" % "2.18.0" % Test,
@@ -74,6 +75,9 @@ val maybeBBCLib: Option[sbt.ProjectReference] = if(bbcBuildProcess) Some(bbcProj
 
 lazy val commonLib = project("common-lib").settings(
   libraryDependencies ++= Seq(
+    // FIXME - added temporarily to assist code compatible with scala 2.12 and 2.13
+    // remove ASAP after completing 2.13 upgrade!!!
+    "org.scala-lang.modules" %% "scala-collection-compat" % "2.12.0",
     "com.gu" %% "editorial-permissions-client" % "3.0.0",
     "com.gu" %% "pan-domain-auth-play_2-8" % "7.0.0",
     "com.amazonaws" % "aws-java-sdk-iam" % awsSdkVersion,

collections/app/store/CollectionsStore.scala

@@ -12,7 +12,7 @@ import scala.concurrent.Future
 class CollectionsStore(config: CollectionsConfig) {
   val dynamo = new DynamoDB[Collection](config, config.collectionsTable)
 
-  def getAll: Future[List[Collection]] = dynamo.scan map { jsonList =>
+  def getAll: Future[List[Collection]] = dynamo.scan() map { jsonList =>
     jsonList.flatMap(json => (json \ "collection").asOpt[Collection])
   } recover {
     case e => throw CollectionsStoreError(e)

common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala

@@ -8,9 +8,10 @@ import org.joda.time.DateTime
 
 import java.util.concurrent.atomic.AtomicReference
 import java.io.InputStream
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.ExecutionContext
 import scala.concurrent.duration._
+import scala.util.control.NonFatal
 
 
 abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonConfig)(implicit ec: ExecutionContext)
@@ -48,8 +49,7 @@ abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonCon
         update()
         lastUpdated.set(DateTime.now())
       } catch {
-        case e: Exception => logger.error("Store update failed", e)
-        case e: RuntimeException => logger.error("Store update failed", e)
+        case NonFatal(e) => logger.error("Store update failed", e)
       }
     }))
   }

common-lib/src/main/scala/com/gu/mediaservice/lib/DateTimeUtils.scala

@@ -20,7 +20,7 @@ object DateTimeUtils {
   // TODO move this to a LocalDateTime
   def fromValueOrNow(value: Option[String]): DateTime = Try{new DateTime(value.get)}.getOrElse(DateTime.now)
 
-  def timeUntilNextInterval(interval: FiniteDuration, now: ZonedDateTime = now): FiniteDuration = {
+  def timeUntilNextInterval(interval: FiniteDuration, now: ZonedDateTime = DateTimeUtils.now()): FiniteDuration = {
     val nowRoundedDownToTheHour = now.truncatedTo(ChronoUnit.HOURS)
     val millisSinceTheHour = ChronoUnit.MILLIS.between(nowRoundedDownToTheHour, now).toDouble
     val numberOfIntervals = (millisSinceTheHour / interval.toMillis).ceil.toLong

common-lib/src/main/scala/com/gu/mediaservice/lib/Files.scala

@@ -4,13 +4,12 @@ import java.io.{File, FileOutputStream}
 import java.net.URL
 import java.nio.channels.Channels
 import java.util.concurrent.Executors
-
-import scala.concurrent.{ExecutionContext, Future}
+import scala.concurrent.{ExecutionContext, ExecutionContextExecutor, Future}
 
 
 object Files {
 
-  private implicit val ctx = ExecutionContext.fromExecutor(Executors.newCachedThreadPool)
+  private implicit val ctx: ExecutionContextExecutor = ExecutionContext.fromExecutor(Executors.newCachedThreadPool)
 
   def createTempFile(prefix: String, suffix: String, tempDir: File): Future[File] =
     Future {

common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala

@@ -10,8 +10,7 @@ import com.gu.mediaservice.model.{MimeType, Png}
 import org.joda.time.DateTime
 
 import scala.concurrent.Future
-
-import scala.jdk.CollectionConverters.iterableAsScalaIterableConverter
+import scala.jdk.CollectionConverters._
 
 object ImageIngestOperations {
   def fileKeyFromId(id: String): String = id.take(6).mkString("/") + "/" + id

common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala

@@ -7,7 +7,7 @@ import com.gu.mediaservice.model.MimeType
 import org.slf4j.LoggerFactory
 
 import java.io.File
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.Future
 
 // TODO: If deleteObject fails - we should be catching the errors here to avoid them bubbling to the application

common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala

@@ -3,7 +3,7 @@ package com.gu.mediaservice.lib.auth
 import com.gu.mediaservice.lib.BaseStore
 import com.gu.mediaservice.lib.config.CommonConfig
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.ExecutionContext
 
 class KeyStore(bucket: String, config: CommonConfig)(implicit ec: ExecutionContext)

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala

@@ -14,7 +14,7 @@ import org.joda.time.DateTime
 import play.api.libs.json._
 
 import scala.annotation.tailrec
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.{ExecutionContext, Future}
 
 object NoItemFound extends Throwable("item not found")
@@ -343,7 +343,7 @@ object DynamoDB {
         case v: JsArray   => valueMap.withList(key, v.value.map {
           case i: JsString => i.value
           case i: JsValue => i.toString
-        }: _*)
+        }.asJava)
         case _ => valueMap
       }
     }

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/Kinesis.scala

@@ -28,7 +28,7 @@ class Kinesis(config: KinesisSenderConfig) extends GridLogging{
 
   private lazy val kinesisClient: AmazonKinesis = getKinesisClient
 
-  def publish[T <: LogMarker](message: T)(implicit messageWrites: Writes[T]) {
+  def publish[T <: LogMarker](message: T)(implicit messageWrites: Writes[T]): Unit = {
     val partitionKey = UUID.randomUUID().toString
 
     implicit val yourJodaDateWrites: Writes[DateTime] = JodaWrites.JodaDateTimeWrites

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala

@@ -11,7 +11,7 @@ import org.joda.time.{DateTime, Duration}
 
 import java.io.File
 import java.net.URI
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.{ExecutionContext, Future}
 
 case class S3Object(uri: URI, size: Long, metadata: S3Metadata)

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/SNS.scala

@@ -9,7 +9,7 @@ import play.api.libs.json.{JsValue, Json}
 class SNS(config: CommonConfig, topicArn: String) extends GridLogging {
   lazy val client: AmazonSNS = config.withAWSCredentials(AmazonSNSClientBuilder.standard()).build()
 
-  def publish(message: JsValue, subject: String) {
+  def publish(message: JsValue, subject: String): Unit = {
     val result = client.publish(new PublishRequest(topicArn, Json.stringify(message), subject))
     logger.info(s"Published message: $result")
   }

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/SimpleSqsMessageConsumer.scala

@@ -5,7 +5,7 @@ import com.amazonaws.services.sqs.AmazonSQS
 import com.amazonaws.services.sqs.AmazonSQSClientBuilder
 import com.amazonaws.services.sqs.model.{DeleteMessageRequest, ReceiveMessageRequest, Message => SQSMessage}
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 
 class SimpleSqsMessageConsumer (queueUrl: String, config: CommonConfig) {
 

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/ThrallMessageSender.scala

@@ -5,8 +5,8 @@ import com.gu.mediaservice.model._
 import com.gu.mediaservice.model.leases.MediaLease
 import com.gu.mediaservice.model.usage.UsageNotice
 import org.joda.time.{DateTime, DateTimeZone}
-import play.api.libs.functional.syntax.{toFunctionalBuilderOps}
-import play.api.libs.json.{JodaReads, JodaWrites, Json, __}
+import play.api.libs.functional.syntax.toFunctionalBuilderOps
+import play.api.libs.json.{JodaReads, JodaWrites, Json, OWrites, Reads, Writes, __}
 
 // TODO MRB: replace this with the simple Kinesis class once we migrate off SNS
 class ThrallMessageSender(config: KinesisSenderConfig) {
@@ -27,17 +27,17 @@ case class BulkIndexRequest(
 )
 
 object BulkIndexRequest {
-  implicit val reads = Json.reads[BulkIndexRequest]
-  implicit val writes = Json.writes[BulkIndexRequest]
+  implicit val reads: Reads[BulkIndexRequest] = Json.reads[BulkIndexRequest]
+  implicit val writes: OWrites[BulkIndexRequest] = Json.writes[BulkIndexRequest]
 }
 
 object UpdateMessage extends GridLogging {
-  implicit val yourJodaDateReads = JodaReads.DefaultJodaDateTimeReads.map(d => d.withZone(DateTimeZone.UTC))
-  implicit val yourJodaDateWrites = JodaWrites.JodaDateTimeWrites
-  implicit val unw = Json.writes[UsageNotice]
-  implicit val unr = Json.reads[UsageNotice]
-  implicit val writes = Json.writes[UpdateMessage]
-  implicit val reads =
+  implicit val yourJodaDateReads: Reads[DateTime] = JodaReads.DefaultJodaDateTimeReads.map(d => d.withZone(DateTimeZone.UTC))
+  implicit val yourJodaDateWrites: Writes[DateTime] = JodaWrites.JodaDateTimeWrites
+  implicit val unw: OWrites[UsageNotice] = Json.writes[UsageNotice]
+  implicit val unr: Reads[UsageNotice] = Json.reads[UsageNotice]
+  implicit val writes: OWrites[UpdateMessage] = Json.writes[UpdateMessage]
+  implicit val reads: Reads[UpdateMessage] =
     (
       (__ \ "subject").read[String] ~
         (__ \ "image").readNullable[Image] ~

common-lib/src/main/scala/com/gu/mediaservice/lib/cleanup/SupplierProcessors.scala

@@ -139,7 +139,7 @@ trait CanonicalisingImageProcessor extends ImageProcessor {
     }
   }
 
-  def getCanonicalName(): String
+  def getCanonicalName: String
   lazy val canonicalName = getCanonicalName
 
   private def matches(image: Image):Boolean = {
@@ -150,7 +150,7 @@ trait CanonicalisingImageProcessor extends ImageProcessor {
 
   lazy val agencyName = getAgencyName
 
-  def getAgencyName(): String
+  def getAgencyName: String
 
   // Rules for slash delimited strings: byline, credit and supplier collection.
   def apply(image: Image): Image = image match {
@@ -174,7 +174,7 @@ trait CanonicalisingImageProcessor extends ImageProcessor {
             val (creditList, foundFlag) = acc
             getPrefixAndSuffix(Some(s)) match {
               case Some(_) if !foundFlag => (creditList :+ canonicalName, true)
-              case Some(_) if foundFlag => acc
+              case Some(_) => acc
               case None => (creditList :+ s, foundFlag)
             }
           })

common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala

@@ -8,7 +8,7 @@ import play.api.{ConfigLoader, Configuration}
 
 import java.net.URI
 import java.util.UUID
-import scala.collection.JavaConverters.collectionAsScalaIterableConverter
+import scala.jdk.CollectionConverters._
 import scala.util.Try
 
 abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1BuilderUtils with AwsClientV2BuilderUtils with StrictLogging {

common-lib/src/main/scala/com/gu/mediaservice/lib/config/DomainMetadataSpec.scala

@@ -3,7 +3,7 @@ package com.gu.mediaservice.lib.config
 import play.api.ConfigLoader
 import play.api.libs.json.{Json, Writes}
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 
 case class DomainMetadataField(
   name: String,
@@ -39,14 +39,14 @@ object DomainMetadataSpec {
             config.getString("type"),
             fieldOptions
           )
-        })
+        }).toSeq
 
         DomainMetadataSpec(
           config.getString("name"),
           config.getString("label"),
           description,
           fields
         )
-      })
+      }).toSeq
     )
 }

common-lib/src/main/scala/com/gu/mediaservice/lib/config/FieldAlias.scala

@@ -3,7 +3,7 @@ package com.gu.mediaservice.lib.config
 import play.api.ConfigLoader
 import play.api.libs.json._
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 
 case class FieldAlias(elasticsearchPath: String,
                       label: String,
@@ -36,6 +36,6 @@ object FieldAlias {
             searchHintOptions
           )
         }
-      )
+      ).toSeq
     )
 }

common-lib/src/main/scala/com/gu/mediaservice/lib/config/Properties.scala

@@ -3,7 +3,7 @@ package com.gu.mediaservice.lib.config
 import java.io.{File, FileInputStream, InputStream}
 import java.net.URL
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 
 object Properties {
 

common-lib/src/main/scala/com/gu/mediaservice/lib/config/ProviderLoader.scala

@@ -7,7 +7,7 @@ import play.api.inject.ApplicationLifecycle
 import play.api.{ConfigLoader, Configuration}
 
 import java.lang.reflect.{Constructor, InvocationTargetException}
-import scala.collection.JavaConverters.asScalaIteratorConverter
+import scala.jdk.CollectionConverters._
 import scala.concurrent.Future
 import scala.reflect.ClassTag
 import scala.util.Try

common-lib/src/main/scala/com/gu/mediaservice/lib/config/UsageRightsConfig.scala

@@ -4,7 +4,7 @@ import com.gu.mediaservice.model.{ContractPhotographer, Photographer, StaffPhoto
 import com.typesafe.config.Config
 import play.api.{ConfigLoader, Configuration}
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.Future
 
 case class PublicationPhotographers(name: String, photographers: List[String])

common-lib/src/main/scala/com/gu/mediaservice/lib/discovery/EC2.scala

@@ -4,7 +4,7 @@ import com.amazonaws.services.ec2.AmazonEC2
 import com.amazonaws.services.ec2.model.{DescribeInstancesRequest, Filter, InstanceStateName}
 import com.gu.mediaservice.lib.logging.GridLogging
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.util.Random
 
 object EC2 extends GridLogging {

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/ElasticSearchClient.scala

@@ -45,7 +45,7 @@ trait ElasticSearchClient extends ElasticSearchExecutions with GridLogging {
   }
 
   //TODO: this function should fail and cause healthcheck fails
-  def ensureIndexExistsAndAliasAssigned() {
+  def ensureIndexExistsAndAliasAssigned(): Unit = {
     logger.info(s"Checking alias $imagesCurrentAlias is assigned to index…")
     val indexForCurrentAlias = Await.result(getIndexForAlias(imagesCurrentAlias), tenSeconds)
     if (indexForCurrentAlias.isEmpty) {
@@ -65,7 +65,7 @@ trait ElasticSearchClient extends ElasticSearchExecutions with GridLogging {
   }
 
   def healthCheck(): Future[Boolean] = {
-    implicit val logMarker = MarkerMap()
+    implicit val logMarker: MarkerMap = MarkerMap()
     val request = search(imagesCurrentAlias) limit 0
     executeAndLog(request, "Healthcheck").map { _ => true}.recover { case _ => false}
   }
@@ -81,7 +81,7 @@ trait ElasticSearchClient extends ElasticSearchExecutions with GridLogging {
   }
 
   def countImages(indexName: String = imagesCurrentAlias): Future[ElasticSearchImageCounts] = {
-    implicit val logMarker = MarkerMap()
+    implicit val logMarker: MarkerMap = MarkerMap()
     val queryCatCount = catCount(indexName) // document count only of index including live documents, not deleted documents which have not yet been removed by the merge process
     val queryImageSearch = search(indexName) trackTotalHits true limit 0 // hits that match the query defined in the request
     val uploadedInLastFiveMinutes = count(indexName) query rangeQuery("uploadTime").gte("now-5m")
@@ -165,24 +165,6 @@ trait ElasticSearchClient extends ElasticSearchExecutions with GridLogging {
     }
   }
 
-  def getCurrentIndices: List[String] = {
-    Await.result(client.execute( {
-      catIndices()
-    }) map { response =>
-      response.result.toList.map(_.index)
-    }, tenSeconds)
-  }
-
-  def getCurrentAliases(): Map[String, Seq[String]] = {
-    Await.result(client.execute( {
-        getAliases()
-    }) map {response =>
-      response.result.mappings.toList.flatMap { case (index, aliases) =>
-        aliases.map(index.name -> _.name)
-      }.groupBy(_._2).mapValues(_.map(_._1))
-    }, tenSeconds)
-  }
-
   def assignAliasTo(index: String, alias: String): Either[ElasticError, Boolean] = {
     logger.info(s"Assigning alias $alias to $index")
     val aliasActionResponse = Await.result(client.execute {

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/PersistedQueries.scala

@@ -22,16 +22,16 @@ object PersistedQueries extends ImageFields {
     CommissionedAgency.category
   )
 
-  val hasCrops = filters.bool.must(filters.existsOrMissing("exports", exists = true))
+  val hasCrops = filters.bool().must(filters.existsOrMissing("exports", exists = true))
   val usedInContent = filters.nested("usages", filters.exists(NonEmptyList("usages")))
 
   def existedPreGrid(persistenceIdentifier: String) = filters.exists(NonEmptyList(identifierField(persistenceIdentifier)))
 
-  val addedToLibrary = filters.bool.must(filters.boolTerm(editsField("archived"), value = true))
+  val addedToLibrary = filters.bool().must(filters.boolTerm(editsField("archived"), value = true))
   val hasUserEditsToImageMetadata = filters.exists(NonEmptyList(editsField("metadata")))
-  val hasPhotographerUsageRights = filters.bool.must(filters.terms(usageRightsField("category"), photographerCategories))
-  val hasIllustratorUsageRights = filters.bool.must(filters.terms(usageRightsField("category"), illustratorCategories))
-  val hasAgencyCommissionedUsageRights = filters.bool.must(filters.terms(usageRightsField("category"), agencyCommissionedCategories))
+  val hasPhotographerUsageRights = filters.bool().must(filters.terms(usageRightsField("category"), photographerCategories))
+  val hasIllustratorUsageRights = filters.bool().must(filters.terms(usageRightsField("category"), illustratorCategories))
+  val hasAgencyCommissionedUsageRights = filters.bool().must(filters.terms(usageRightsField("category"), agencyCommissionedCategories))
 
   def isInPersistedCollection(maybePersistOnlyTheseCollections: Option[Set[String]]) =
     maybePersistOnlyTheseCollections.map(_.toList) match {

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/filters.scala

@@ -81,7 +81,7 @@ object filters {
   }
 
   def mustWithMustNot(mustClause: Query, mustNotClause: Query): Query = {
-    bool.must(
+    bool().must(
       mustClause
     ).withNot(
       mustNotClause

common-lib/src/main/scala/com/gu/mediaservice/lib/imaging/im4jwrapper/ImageMagick.scala

@@ -5,7 +5,7 @@ import java.io.File
 import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker}
 import org.im4java.process.ArrayListOutputConsumer
 
-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
 import scala.concurrent.{ExecutionContext, Future}
 import org.im4java.core.{ConvertCmd, IMOperation, IdentifyCmd}
 import com.gu.mediaservice.model.{Bounds, Dimensions}