Skip to content

Commit 21d96be

Browse files
committed
ensure images with the new derivative-of-media-ids and replaces-media-id identifiers are persisted (i.e. not 'reaped')
1 parent f7d86a8 commit 21d96be

File tree

9 files changed

+29
-17
lines changed

9 files changed

+29
-17
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.gu.mediaservice.lib.config
22

33
import com.gu.mediaservice.lib.ImageStorageProps
44
import com.gu.mediaservice.lib.elasticsearch.{ElasticSearchAliases, ElasticSearchConfig}
5+
import scalaz.NonEmptyList
56

67
class CommonConfigWithElastic(resources: GridConfigResources) extends CommonConfig(resources) {
78

@@ -15,7 +16,12 @@ class CommonConfigWithElastic(resources: GridConfigResources) extends CommonConf
1516
replicas = string("es6.replicas").toInt
1617
)
1718

18-
val persistenceIdentifier = string("persistence.identifier")
19+
private val persistenceIdentifier = string("persistence.identifier")
20+
val persistenceIdentifiers = NonEmptyList(
21+
persistenceIdentifier,
22+
ImageStorageProps.derivativeOfMediaIdsIdentifierKey,
23+
ImageStorageProps.replacesMediaIdIdentifierKey
24+
)
1925
val queriableIdentifiers = Seq(
2026
persistenceIdentifier,
2127
ImageStorageProps.derivativeOfMediaIdsIdentifierKey,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ object PersistedQueries extends ImageFields {
2525
val hasCrops = filters.bool().must(filters.existsOrMissing("exports", exists = true))
2626
val usedInContent = filters.nested("usages", filters.exists(NonEmptyList("usages")))
2727

28-
def existedPreGrid(persistenceIdentifier: String) = filters.exists(NonEmptyList(identifierField(persistenceIdentifier)))
28+
def hasPersistedIdentifier(persistenceIdentifiers: NonEmptyList[String]) = filters.exists(persistenceIdentifiers.map(identifierField))
2929

3030
val addedToLibrary = filters.bool().must(filters.boolTerm(editsField("archived"), value = true))
3131
val hasUserEditsToImageMetadata = filters.exists(NonEmptyList(editsField("metadata")))

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package com.gu.mediaservice.lib.elasticsearch
33
import com.sksamuel.elastic4s.ElasticDsl.matchAllQuery
44
import com.sksamuel.elastic4s.requests.searches.queries.Query
55
import org.joda.time.DateTime
6-
import com.gu.mediaservice.lib.config. Provider
6+
import com.gu.mediaservice.lib.config.Provider
7+
import scalaz.NonEmptyList
8+
79
import scala.concurrent.Future
810

911
trait ReapableEligibility extends Provider{
@@ -13,7 +15,7 @@ trait ReapableEligibility extends Provider{
1315

1416

1517
val maybePersistOnlyTheseCollections: Option[Set[String]] // typically from config
16-
val persistenceIdentifier: String // typically from config
18+
val persistenceIdentifiers: NonEmptyList[String] // typically from config
1719

1820
private def moreThanTwentyDaysOld =
1921
filters.date("uploadTime", None, Some(DateTime.now().minusDays(20))).getOrElse(matchAllQuery())
@@ -29,7 +31,7 @@ trait ReapableEligibility extends Provider{
2931
PersistedQueries.addedToPhotoshoot,
3032
PersistedQueries.hasLabels,
3133
PersistedQueries.hasLeases,
32-
PersistedQueries.existedPreGrid(persistenceIdentifier),
34+
PersistedQueries.hasPersistedIdentifier(persistenceIdentifiers),
3335
PersistedQueries.isInPersistedCollection(maybePersistOnlyTheseCollections)
3436
)
3537

media-api/app/lib/ImagePersistenceReasons.scala

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@ package lib
33
import com.gu.mediaservice.lib.elasticsearch.PersistedQueries
44
import com.gu.mediaservice.model.{CommissionedAgency, Illustrator, Image, ImageMetadata, Photographer}
55
import com.sksamuel.elastic4s.requests.searches.queries.Query
6+
import scalaz.NonEmptyList
67

78

8-
case class ImagePersistenceReasons(maybePersistOnlyTheseCollections: Option[Set[String]], persistenceIdentifier: String) {
9+
case class ImagePersistenceReasons(maybePersistOnlyTheseCollections: Option[Set[String]], persistenceIdentifiers: NonEmptyList[String]) {
910
val allReasons: List[PersistenceReason] =
1011
List(
11-
HasPersistenceIdentifier(persistenceIdentifier),
12+
HasPersistenceIdentifier(persistenceIdentifiers),
1213
HasExports,
1314
HasUsages,
1415
IsArchived,
@@ -31,12 +32,12 @@ sealed trait PersistenceReason {
3132
val reason: String
3233
}
3334

34-
case class HasPersistenceIdentifier(persistenceIdentifier: String) extends PersistenceReason {
35-
override def shouldPersist(image: Image): Boolean = image.identifiers.contains(persistenceIdentifier)
35+
case class HasPersistenceIdentifier(persistenceIdentifiers: NonEmptyList[String]) extends PersistenceReason {
36+
override def shouldPersist(image: Image): Boolean = persistenceIdentifiers.list.toList.exists(image.identifiers.contains)
3637

3738
override val reason: String = "persistence-identifier"
3839

39-
override val query: Query = PersistedQueries.existedPreGrid(persistenceIdentifier)
40+
override val query: Query = PersistedQueries.hasPersistedIdentifier(persistenceIdentifiers)
4041
}
4142

4243
object HasExports extends PersistenceReason {

media-api/app/lib/ImageResponse.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3Client, usageQuota: Usag
4747

4848
private val imgPersistenceReasons = ImagePersistenceReasons(
4949
config.maybePersistOnlyTheseCollections,
50-
config.persistenceIdentifier
50+
config.persistenceIdentifiers
5151
)
5252

5353
def imagePersistenceReasons(image: Image): List[String] = imgPersistenceReasons.reasons(image)

media-api/app/lib/elasticsearch/IsQueryFilter.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.sksamuel.elastic4s.ElasticDsl.matchAllQuery
77
import com.sksamuel.elastic4s.requests.searches.queries.Query
88
import lib.MediaApiConfig
99
import org.joda.time.DateTime
10+
import scalaz.NonEmptyList
1011
import scalaz.syntax.std.list._
1112

1213
sealed trait IsQueryFilter extends Query with ImageFields {
@@ -32,7 +33,7 @@ object IsQueryFilter {
3233
case s if s == s"$organisation-owned" => Some(IsOwnedImage(organisation))
3334
case "under-quota" => Some(IsUnderQuota(overQuotaAgencies()))
3435
case "deleted" => Some(IsDeleted(true))
35-
case "reapable" => Some(IsReapable(config.maybePersistOnlyTheseCollections, config.persistenceIdentifier))
36+
case "reapable" => Some(IsReapable(config.maybePersistOnlyTheseCollections, config.persistenceIdentifiers))
3637
case _ => None
3738
}
3839
}
@@ -68,6 +69,6 @@ case class IsDeleted(isDeleted: Boolean) extends IsQueryFilter {
6869
)
6970
}
7071

71-
case class IsReapable(maybePersistOnlyTheseCollections: Option[Set[String]], persistenceIdentifier: String)
72+
case class IsReapable(maybePersistOnlyTheseCollections: Option[Set[String]], persistenceIdentifiers: NonEmptyList[String])
7273
extends IsQueryFilter with ReapableEligibility {
7374
}

media-api/app/lib/elasticsearch/SearchFilters.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class SearchFilters(config: MediaApiConfig) extends ImageFields {
5252
lazy val freeToUseCategories: List[String] =
5353
usageRights.filter(ur => ur.defaultCost.exists(cost => cost == Free || cost == Conditional)).map(ur => ur.category)
5454

55-
val persistedReasons: List[PersistenceReason] = ImagePersistenceReasons(config.maybePersistOnlyTheseCollections, config.persistenceIdentifier).allReasons
55+
val persistedReasons: List[PersistenceReason] = ImagePersistenceReasons(config.maybePersistOnlyTheseCollections, config.persistenceIdentifiers).allReasons
5656

5757
val persistedFilter: Query = filters.or(persistedReasons.map(_.query): _*)
5858

media-api/test/lib/ImagePersistenceReasonsTest.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.gu.mediaservice.model.usage.Usage
66
import org.joda.time.DateTime.now
77
import org.scalatest.funspec.AnyFunSpec
88
import org.scalatest.matchers.should.Matchers
9+
import scalaz.NonEmptyList
910

1011
class ImagePersistenceReasonsTest extends AnyFunSpec with Matchers {
1112

@@ -15,8 +16,8 @@ class ImagePersistenceReasonsTest extends AnyFunSpec with Matchers {
1516

1617
val persistedIdentifier = "test-p-id"
1718
val persistedCollections = Set("coll1", "coll2", "coll3")
18-
val imgPersistenceReasons = ImagePersistenceReasons(Some(persistedCollections), persistedIdentifier)
19-
val imagePersistenceReasonsWithEmptyListOfPersistedCollections = ImagePersistenceReasons(Some(Set.empty), persistedIdentifier)
19+
val imgPersistenceReasons = ImagePersistenceReasons(Some(persistedCollections), NonEmptyList(persistedIdentifier))
20+
val imagePersistenceReasonsWithEmptyListOfPersistedCollections = ImagePersistenceReasons(Some(Set.empty), NonEmptyList(persistedIdentifier))
2021

2122
imgPersistenceReasons.reasons(img) shouldBe Nil
2223
val imgWithPersistenceIdentifier = img.copy(identifiers = Map(persistedIdentifier -> "test-id"))

thrall/app/controllers/ReaperController.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import lib.elasticsearch.ElasticSearch
1414
import org.joda.time.{DateTime, DateTimeZone}
1515
import play.api.libs.json.{JsValue, Json}
1616
import play.api.mvc.{Action, AnyContent, ControllerComponents}
17+
import scalaz.NonEmptyList
1718

1819
import scala.concurrent.{ExecutionContext, Future}
1920
import scala.jdk.CollectionConverters._
@@ -44,7 +45,7 @@ class ReaperController(
4445
private val isReapable = maybeCustomReapableEligibility getOrElse {
4546
new ReapableEligibility {
4647
override val maybePersistOnlyTheseCollections: Option[Set[String]] = config.maybePersistOnlyTheseCollections
47-
override val persistenceIdentifier: String = config.persistenceIdentifier
48+
override val persistenceIdentifiers: NonEmptyList[String] = config.persistenceIdentifiers
4849
}
4950
}
5051

0 commit comments

Comments
 (0)