Merge pull request #4345 from guardian/an/rm-scalaz-2

Scalaz upgrade & simplifications

build.sbt

@@ -85,7 +85,7 @@ lazy val commonLib = project("common-lib").settings(
     "com.sksamuel.elastic4s" %% "elastic4s-client-esjava" % elastic4sVersion,
     "com.sksamuel.elastic4s" %% "elastic4s-domain" % elastic4sVersion,
     "com.gu" %% "thrift-serializer" % "5.0.2",
-    "org.scalaz.stream" %% "scalaz-stream" % "0.8.6",
+    "org.scalaz" %% "scalaz-core" % "7.3.8",
     "org.im4java" % "im4java" % "1.4.0",
     "com.gu" % "kinesis-logback-appender" % "1.4.4",
     "net.logstash.logback" % "logstash-logback-encoder" % "5.0",

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

@@ -1,24 +1,20 @@
 package com.gu.mediaservice.lib.aws
 
 import java.util.concurrent.Executors
-import java.util.concurrent.atomic.AtomicReference
 
 import _root_.play.api.libs.functional.syntax._
 import _root_.play.api.libs.json._
 import akka.actor.ActorSystem
 import com.amazonaws.services.cloudwatch.model.Dimension
-import com.amazonaws.services.sqs.model.{DeleteMessageRequest, ReceiveMessageRequest, Message => SQSMessage}
-import com.amazonaws.services.sqs.{AmazonSQS, AmazonSQSClientBuilder}
+import com.amazonaws.services.sqs.model.{Message => SQSMessage}
 import com.gu.mediaservice.lib.ImageId
 import com.gu.mediaservice.lib.config.CommonConfig
 import com.gu.mediaservice.lib.json.PlayJsonHelpers._
 import com.gu.mediaservice.lib.metrics.Metric
 import org.joda.time.DateTime
 import org.joda.time.format.ISODateTimeFormat
-import scalaz.syntax.id._
 
 import scala.annotation.tailrec
-import scala.collection.JavaConverters._
 import scala.concurrent.duration._
 import scala.concurrent.{ExecutionContext, Future}
 
@@ -44,7 +40,7 @@ abstract class SqsViaSnsMessageConsumer(queueUrl: String, config: CommonConfig,
             _.apply(message.body))
         _ = recordMessageCount(message)
       } yield ()
-      future |> deleteOnSuccess(msg)
+      deleteOnSuccess(msg)(future)
     }
 
     processMessages()
@@ -61,8 +57,11 @@ abstract class SqsViaSnsMessageConsumer(queueUrl: String, config: CommonConfig,
   private def deleteOnSuccess(msg: SQSMessage)(f: Future[Any]): Unit =
     f.foreach { _ => deleteMessage(msg) }
 
-  private def extractSNSMessage(sqsMessage: SQSMessage): Option[SNSMessage] =
-    Json.fromJson[SNSMessage](Json.parse(sqsMessage.getBody)) <| logParseErrors |> (_.asOpt)
+  private def extractSNSMessage(sqsMessage: SQSMessage): Option[SNSMessage] = {
+    val result = Json.fromJson[SNSMessage](Json.parse(sqsMessage.getBody))
+    logParseErrors(result)
+    result.asOpt
+  }
 
 }
 

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

@@ -4,7 +4,6 @@ import com.gu.mediaservice.lib.ImageFields
 import com.gu.mediaservice.model._
 import com.sksamuel.elastic4s.ElasticApi.matchNoneQuery
 import scalaz.NonEmptyList
-import scalaz.syntax.std.list._
 
 object PersistedQueries extends ImageFields {
   val photographerCategories = NonEmptyList(
@@ -34,15 +33,14 @@ object PersistedQueries extends ImageFields {
   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 match {
-    case None =>
-      filters.exists(NonEmptyList("collections"))
-    case Some(persistedCollections) if persistedCollections.nonEmpty =>
-      filters.bool.must(filters.terms(collectionsField("path"), persistedCollections.toList.toNel.get))
-    case _ =>
-      matchNoneQuery
-  }
-
+  def isInPersistedCollection(maybePersistOnlyTheseCollections: Option[Set[String]]) =
+    maybePersistOnlyTheseCollections.map(_.toList) match {
+      case None =>
+        filters.exists(NonEmptyList("collections"))
+      case Some(Nil) => matchNoneQuery()
+      case Some(head :: tail) =>
+        filters.bool().must(filters.terms(collectionsField("path"), NonEmptyList.fromSeq(head, tail)))
+    }
 
 
   val addedToPhotoshoot = filters.exists(NonEmptyList(editsField("photoshoot")))

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

@@ -17,7 +17,7 @@ object filters {
   def or(queries: Query*): Query = should(queries)
 
   def or(queries: NonEmptyList[Query]): Query = {
-    should(queries.list: _*)
+    should(queries.toList: _*)
   }
 
   def boolTerm(field: String, value: Boolean): TermQuery = termQuery(field, value)
@@ -64,7 +64,7 @@ object filters {
   def term(field: String, term: Int): Query = termQuery(field, term)
 
   def terms(field: String, terms: NonEmptyList[String]): Query = {
-    termsQuery(field, terms.list)
+    termsQuery(field, terms.list.toList)
   }
 
   def existsOrMissing(field: String, exists: Boolean): Query = if (exists) {

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

@@ -5,20 +5,28 @@ import java.util.concurrent.Executors
 import java.io.File
 import scala.concurrent.{Future, ExecutionContext}
 import org.im4java.core.{ETOperation, ExiftoolCmd}
-import scalaz.syntax.id._
 
 
 object ExifTool {
   private implicit val ctx: ExecutionContext =
     ExecutionContext.fromExecutor(Executors.newFixedThreadPool(Config.imagingThreadPoolSize))
 
-  def tagSource(source: File) = (new ETOperation()) <| (_.addImage(source.getAbsolutePath))
+  def tagSource(source: File): ETOperation = {
+    val op = new ETOperation()
+    op.addImage(source.getAbsolutePath)
+    op
+  }
 
   def setTags(ops: ETOperation)(tags: Map[String, String]): ETOperation =  {
-    tags.foldLeft(ops) { case (ops, (key, value)) => ops <| (_.setTags(s"$key=$value")) }
+    tags.foldLeft(ops) { case (ops, (key, value)) =>
+      ops.setTags(s"$key=$value")
+    }
   }
 
-  def overwriteOriginal(ops: ETOperation): ETOperation = ops <| (_.overwrite_original())
+  def overwriteOriginal(ops: ETOperation): ETOperation = {
+    ops.overwrite_original()
+    ops
+  }
 
   def runExiftoolCmd(ops: ETOperation): Future[Unit] = {
     // Set overwrite original to ensure temporary file deletion

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

@@ -8,28 +8,78 @@ import org.im4java.process.ArrayListOutputConsumer
 import scala.collection.JavaConverters._
 import scala.concurrent.{ExecutionContext, Future}
 import org.im4java.core.{ConvertCmd, IMOperation, IdentifyCmd}
-import scalaz.syntax.id._
 import com.gu.mediaservice.model.{Bounds, Dimensions}
+
+
 object ImageMagick extends GridLogging {
   implicit val ctx: ExecutionContext =
     ExecutionContext.fromExecutor(Executors.newFixedThreadPool(Config.imagingThreadPoolSize))
 
-  def addImage(source: File) = (new IMOperation()) <| { op => { op.addImage(source.getAbsolutePath) }}
-  def quality(op: IMOperation)(qual: Double) = op <| (_.quality(qual))
-  def unsharp(op: IMOperation)(radius: Double, sigma: Double, amount: Double) = op <| (_.unsharp(radius, sigma, amount))
-  def stripMeta(op: IMOperation) = op <| (_.strip())
-  def stripProfile(op: IMOperation)(profile: String) = op <| (_.p_profile(profile))
-  def addDestImage(op: IMOperation)(dest: File) = op <| (_.addImage(dest.getAbsolutePath))
-  def crop(op: IMOperation)(b: Bounds): IMOperation = op <| (_.crop(b.width, b.height, b.x, b.y))
-  def profile(op: IMOperation)(profileFileLocation: String): IMOperation = op <| (_.profile(profileFileLocation))
-  def thumbnail(op: IMOperation)(width: Int): IMOperation = op <| (_.thumbnail(width))
-  def resize(op: IMOperation)(maxSize: Int): IMOperation = op <| (_.resize(maxSize, maxSize))
-  def scale(op: IMOperation)(dimensions: Dimensions): IMOperation = op <| (_.scale(dimensions.width, dimensions.height))
-  def format(op: IMOperation)(definition: String): IMOperation = op <| (_.format(definition))
-  def depth(op: IMOperation)(depth: Int): IMOperation = op <| (_.depth(depth))
-  def interlace(op: IMOperation)(interlacedHow: String): IMOperation = op <| (_.interlace(interlacedHow))
-  def setBackgroundColour(op: IMOperation)(backgroundColour: String): IMOperation = op <| (_.background(backgroundColour))
-  def flatten(op: IMOperation): IMOperation = op <| (_.flatten())
+  def addImage(source: File): IMOperation = {
+    val op = new IMOperation
+    op.addImage(source.getAbsolutePath)
+    op
+  }
+  def quality(op: IMOperation)(qual: Double): IMOperation = {
+    op.quality(qual)
+    op
+  }
+  def unsharp(op: IMOperation)(radius: Double, sigma: Double, amount: Double): IMOperation = {
+    op.unsharp(radius, sigma, amount)
+    op
+  }
+  def stripMeta(op: IMOperation): IMOperation = {
+    op.strip()
+    op
+  }
+  def stripProfile(op: IMOperation)(profile: String): IMOperation = {
+    op.p_profile(profile)
+    op
+  }
+  def addDestImage(op: IMOperation)(dest: File): IMOperation = {
+    op.addImage(dest.getAbsolutePath)
+    op
+  }
+  def crop(op: IMOperation)(b: Bounds): IMOperation = {
+    op.crop(b.width, b.height, b.x, b.y)
+    op
+  }
+  def profile(op: IMOperation)(profileFileLocation: String): IMOperation = {
+    op.profile(profileFileLocation)
+    op
+  }
+  def thumbnail(op: IMOperation)(width: Int): IMOperation = {
+    op.thumbnail(width)
+    op
+  }
+  def resize(op: IMOperation)(maxSize: Int): IMOperation = {
+    op.resize(maxSize, maxSize)
+    op
+  }
+  def scale(op: IMOperation)(dimensions: Dimensions): IMOperation = {
+    op.scale(dimensions.width, dimensions.height)
+    op
+  }
+  def format(op: IMOperation)(definition: String): IMOperation = {
+    op.format(definition)
+    op
+  }
+  def depth(op: IMOperation)(depth: Int): IMOperation = {
+    op.depth(depth)
+    op
+  }
+  def interlace(op: IMOperation)(interlacedHow: String): IMOperation = {
+    op.interlace(interlacedHow)
+    op
+  }
+  def setBackgroundColour(op: IMOperation)(backgroundColour: String): IMOperation = {
+    op.background(backgroundColour)
+    op
+  }
+  def flatten(op: IMOperation): IMOperation = {
+    op.flatten()
+    op
+  }
 
   def runConvertCmd(op: IMOperation, useImageMagick: Boolean)(implicit logMarker: LogMarker): Future[Unit] = {
     logger.info(logMarker, s"Using ${if(useImageMagick) { "imagemagick" } else { "graphicsmagick" }} for imaging conversion operation $op")

common-lib/src/main/scala/com/gu/mediaservice/lib/logging/LogConfig.scala

@@ -14,7 +14,6 @@ import org.slf4j.{LoggerFactory, Logger => SLFLogger}
 import play.api.ApplicationLoader.Context
 import play.api.LoggerConfigurator
 import play.api.libs.json._
-import scalaz.syntax.id._
 
 import scala.util.Try
 
@@ -39,37 +38,45 @@ object LogConfig {
     )).toString()
   }
 
-  private def makeLayout(customFields: String) = new LogstashLayout() <| (_.setCustomFields(customFields))
+  private def makeLayout(customFields: String) = {
+    val layout = new LogstashLayout()
+    layout.setCustomFields(customFields)
+    layout
+  }
+
+  private def makeKinesisAppender(layout: LogstashLayout, context: LoggerContext, appenderConfig: KinesisAppenderConfig): KinesisAppender[ILoggingEvent] = {
+    val appender = new KinesisAppender[ILoggingEvent]()
+    appender.setStreamName(appenderConfig.stream)
+    appender.setRegion(appenderConfig.region)
+    appender.setRoleToAssumeArn(appenderConfig.roleArn)
+    appender.setBufferSize(appenderConfig.bufferSize)
 
-  private def makeKinesisAppender(layout: LogstashLayout, context: LoggerContext, appenderConfig: KinesisAppenderConfig) =
-    new KinesisAppender[ILoggingEvent]() <| { a =>
-      a.setStreamName(appenderConfig.stream)
-      a.setRegion(appenderConfig.region)
-      a.setRoleToAssumeArn(appenderConfig.roleArn)
-      a.setBufferSize(appenderConfig.bufferSize)
+    appender.setContext(context)
+    appender.setLayout(layout)
 
-      a.setContext(context)
-      a.setLayout(layout)
+    layout.start()
+    appender.start()
 
-      layout.start()
-      a.start()
+    appender
   }
 
   private def makeLogstashAppender(config: CommonConfig, context: LoggerContext): LogstashTcpSocketAppender = {
     val customFields = makeCustomFields(config)
 
-    new LogstashTcpSocketAppender() <| { appender =>
-      appender.setContext(context)
-      appender.addDestinations(new InetSocketAddress("localhost", 5000))
-      appender.setWriteBufferSize(BUFFER_SIZE)
+    val appender = new LogstashTcpSocketAppender()
 
-      appender.setEncoder(new LogstashEncoder() <| { encoder =>
-        encoder.setCustomFields(customFields)
-        encoder.start()
-      })
+    appender.setContext(context)
+    appender.addDestinations(new InetSocketAddress("localhost", 5000))
+    appender.setWriteBufferSize(BUFFER_SIZE)
 
-      appender.start()
-    }
+    val encoder = new LogstashEncoder
+    encoder.setCustomFields(customFields)
+    encoder.start()
+
+    appender.setEncoder(encoder)
+    appender.start()
+
+    appender
   }
 
   def initLocalLogShipping(config: CommonConfig): Unit = {

common-lib/src/main/scala/com/gu/mediaservice/model/UsageRights.scala

@@ -6,6 +6,7 @@ import com.typesafe.config.ConfigException.BadValue
 import com.typesafe.scalalogging.StrictLogging
 import play.api.ConfigLoader
 import play.api.libs.json._
+import scalaz.NonEmptyList
 
 import scala.collection.JavaConverters.asScalaIteratorConverter
 import scala.reflect.runtime.universe
@@ -52,9 +53,9 @@ object UsageRightsSpec extends StrictLogging {
 
 object UsageRights {
 
-  val photographer: List[UsageRightsSpec] = List(StaffPhotographer, ContractPhotographer, CommissionedPhotographer)
-  val illustrator: List[UsageRightsSpec] = List(StaffIllustrator, ContractIllustrator, CommissionedIllustrator)
-  val whollyOwned: List[UsageRightsSpec] = photographer ++ illustrator
+  val photographer: NonEmptyList[UsageRightsSpec] = NonEmptyList(StaffPhotographer, ContractPhotographer, CommissionedPhotographer)
+  val illustrator: NonEmptyList[UsageRightsSpec] = NonEmptyList(StaffIllustrator, ContractIllustrator, CommissionedIllustrator)
+  val whollyOwned: NonEmptyList[UsageRightsSpec] = photographer append illustrator
 
   // this is a convenience method so that we use the same formatting for all subtypes
   // i.e. use the standard `Json.writes`. I still can't find a not have to pass the `f:Format[T]`

media-api/app/controllers/MediaApi.scala

@@ -477,8 +477,7 @@ class MediaApi(
     SearchParams.validate(searchParams).fold(
       // TODO: respondErrorCollection?
       errors => Future.successful(respondError(UnprocessableEntity, InvalidUriParams.errorKey,
-        // Annoyingly `NonEmptyList` and `IList` don't have `mkString`
-        errors.map(_.message).list.reduce(_+ ", " +_), List(searchLink))
+        errors.map(_.message).mkString(", "))
       ),
       params => respondSuccess(params)
     )

media-api/app/lib/MediaApiConfig.scala

@@ -3,6 +3,7 @@ package lib
 import com.amazonaws.services.cloudfront.util.SignerUtils
 import com.gu.mediaservice.lib.config.{CommonConfigWithElastic, GridConfigResources}
 import org.joda.time.DateTime
+import scalaz.NonEmptyList
 
 import java.security.PrivateKey
 import scala.util.Try
@@ -43,7 +44,7 @@ class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithEla
   val loginUriTemplate: String = services.loginUriTemplate
   val collectionsUri: String = services.collectionsBaseUri
 
-  val requiredMetadata = List("credit", "description", "usageRights")
+  val requiredMetadata = NonEmptyList("credit", "description", "usageRights")
 
   val syndicationStartDate: Option[DateTime] = Try {
     stringOpt("syndication.start").map(d => DateTime.parse(d).withTimeAtStartOfDay())