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

Upgrades to syntax and types to prepare for scala 2.13 upgrade #4355

Merged
merged 26 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2fa65bb
fix "auto-application to () is deprecated"
andrew-nowak Oct 24, 2024
05a2549
remove procedure syntax
andrew-nowak Oct 24, 2024
daab698
make conversion from long to double explicit
andrew-nowak Oct 24, 2024
b4f7993
resolve mismatch between overriding methods missing empty parameter list
andrew-nowak Oct 24, 2024
8e36ca4
resolve confusion between multiple values named `now`
andrew-nowak Oct 24, 2024
4a785f9
import collectionconverters as wildcard to handle renamed conversions
andrew-nowak Oct 24, 2024
2c4e2ea
remove unused alias methods from elasticsearch client
andrew-nowak Oct 24, 2024
353c595
fix type mismatch errors with more explicit conversions
andrew-nowak Oct 24, 2024
dfa0775
Add collection-compat to enable 2.13 map view methods on 2.12
andrew-nowak Oct 24, 2024
67ec3cd
replace JavaConverters with CollectionConverters
andrew-nowak Oct 24, 2024
53dbbfd
better catch statement
andrew-nowak Oct 24, 2024
871036d
help the exhaustiveness checker
andrew-nowak Oct 24, 2024
fc2a1b7
more resolving auto-application to ()
andrew-nowak Oct 24, 2024
0ed42a9
fix yet more type mismatch errors
andrew-nowak Oct 24, 2024
01dbc76
GenIterable is now Iterable
andrew-nowak Oct 24, 2024
5f0ecc5
add some more explicit conversions to avoid deprecation warnings
andrew-nowak Oct 24, 2024
7b488a2
rename "export" controller method
andrew-nowak Oct 24, 2024
8b24b7d
exhaustive pattern matching: throw if failure is "fatal"
andrew-nowak Oct 24, 2024
aacb168
replace deprecated symbol syntax
andrew-nowak Oct 24, 2024
920279d
upgrade scalatest to receive 2.13-compatible EitherValues syntax
andrew-nowak Oct 24, 2024
f0bde5f
update listy syntax
andrew-nowak Oct 24, 2024
7c29725
more methods without params overriding methods with
andrew-nowak Oct 24, 2024
6bcdc20
add exception throwing for unhandled cases in matcher in tests
andrew-nowak Oct 24, 2024
0a1f0ef
TraversableLike -> Iterable
andrew-nowak Oct 24, 2024
5cb7599
final usage of .right, add EitherValues to access nicely
andrew-nowak Oct 24, 2024
2fec085
Add explicit type annotations for implicit definitions
andrew-nowak Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Comment on lines +78 to +80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this removal need adding to #4361

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great cross-PR catch! added in e94c08e

"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,
Expand Down
2 changes: 1 addition & 1 deletion collections/app/store/CollectionsStore.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍

val nowRoundedDownToTheHour = now.truncatedTo(ChronoUnit.HOURS)
val millisSinceTheHour = ChronoUnit.MILLIS.between(nowRoundedDownToTheHour, now).toDouble
val numberOfIntervals = (millisSinceTheHour / interval.toMillis).ceil.toLong
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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] ~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ trait CanonicalisingImageProcessor extends ImageProcessor {
}
}

def getCanonicalName(): String
def getCanonicalName: String
lazy val canonicalName = getCanonicalName

private def matches(image: Image):Boolean = {
Expand All @@ -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 {
Expand All @@ -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)
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -39,14 +39,14 @@ object DomainMetadataSpec {
config.getString("type"),
fieldOptions
)
})
}).toSeq

DomainMetadataSpec(
config.getString("name"),
config.getString("label"),
description,
fields
)
})
}).toSeq
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -36,6 +36,6 @@ object FieldAlias {
searchHintOptions
)
}
)
).toSeq
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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}
}
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ object filters {
}

def mustWithMustNot(mustClause: Query, mustNotClause: Query): Query = {
bool.must(
bool().must(
mustClause
).withNot(
mustNotClause
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading
Loading