Skip to content

Commit

Permalink
Smart list bug fixing and lots of other bug fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisbenincasa committed Nov 17, 2019
1 parent 35a44a6 commit dbd9b8f
Show file tree
Hide file tree
Showing 27 changed files with 642 additions and 188 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import com.teletracker.common.db.model.{
DynamicListItemTypeRule,
DynamicListNetworkRule,
DynamicListPersonRule,
DynamicListReleaseYearRule,
DynamicListTagRule,
PersonAssociationType,
TrackedListRow
}
import com.teletracker.common.elasticsearch.EsItemTag.TagFormatter
import com.teletracker.common.util.{IdOrSlug, ListFilters}
import com.teletracker.common.util.{IdOrSlug, ListFilters, OpenDateRange}
import javax.inject.Inject
import com.teletracker.common.util.Functions._
import org.elasticsearch.action.search.{MultiSearchRequest, SearchRequest}
Expand All @@ -33,7 +34,8 @@ import scala.concurrent.{ExecutionContext, Future, Promise}
import scala.util.Try

class DynamicListBuilder @Inject()(
elasticsearchExecutor: ElasticsearchExecutor
elasticsearchExecutor: ElasticsearchExecutor,
personLookup: PersonLookup
)(implicit executionContext: ExecutionContext)
extends ElasticsearchAccess {
def getDynamicListCounts(
Expand Down Expand Up @@ -92,7 +94,7 @@ class DynamicListBuilder @Inject()(
sortMode: SortMode = Popularity(),
bookmark: Option[Bookmark] = None,
limit: Int = 20
): Future[(ElasticsearchItemsResponse, Long)] = {
): Future[(ElasticsearchItemsResponse, Long, List[EsPerson])] = {
val listQuery = getDynamicListQuery(
userId,
list,
Expand All @@ -102,6 +104,16 @@ class DynamicListBuilder @Inject()(
Some(limit)
)

val peopleToFetch = list.rules.toList.flatMap(_.rules).collect {
case personRule: DynamicListPersonRule => personRule.personId
}

val peopleForRulesFut = if (peopleToFetch.nonEmpty) {
personLookup.lookupPeople(peopleToFetch.map(IdOrSlug.fromUUID))
} else {
Future.successful(Nil)
}

val searchRequest = new SearchRequest("items").source(listQuery)

val itemsFut = elasticsearchExecutor
Expand All @@ -114,8 +126,16 @@ class DynamicListBuilder @Inject()(
for {
items <- itemsFut
count <- countFut
people <- peopleForRulesFut
} yield {
items -> count
(
items,
count,
// TODO: gnarly hack to appease the frontend... think about this later
// This is a preemptive fetch to get some light details on a person, but returning credits makes
// future full fetches difficult because cast_credits here would overwrite frontend state...
people.map(_.copy(cast_credits = None, crew_credits = None))
)
}
}

Expand All @@ -140,31 +160,63 @@ class DynamicListBuilder @Inject()(
case personRule: DynamicListPersonRule => personRule
}

val genreRules = rules.rules.collect {
case genreRule: DynamicListGenreRule => genreRule
val releaseYearRules = rules.rules.collect {
case releaseYearRule: DynamicListReleaseYearRule => releaseYearRule
}

val itemTypeRules = rules.rules.collect {
case itemTypeRule: DynamicListItemTypeRule => itemTypeRule
}
require(
releaseYearRules.size < 2,
"Cannot have multiple release year rules!"
)

val clientSpecifiedGenres = listFilters.flatMap(_.genres)
val releaseYearRule =
releaseYearRules.headOption.map(
rule => OpenDateRange.forYearRange(rule.minimum, rule.maximum)
)

val genreIdsToUse = clientSpecifiedGenres.orElse {
Some(genreRules.map(_.genreId)).filter(_.nonEmpty)
val genreIdsToUse = listFilters.flatMap(_.genres).orElse {
Some(
rules.rules
.collect {
case genreRule: DynamicListGenreRule => genreRule
}
.map(_.genreId)
).filter(_.nonEmpty)
}

val clientSpecifiedItemTypes =
listFilters.flatMap(_.itemTypes).filter(_.nonEmpty)

val itemTypesToUse = clientSpecifiedItemTypes.orElse {
Some(itemTypeRules.map(_.itemType).toSet).filter(_.nonEmpty)
val itemTypesToUse = listFilters.flatMap(_.itemTypes).orElse {
Some(
rules.rules
.collect {
case itemTypeRule: DynamicListItemTypeRule => itemTypeRule
}
.map(_.itemType)
.toSet
).filter(_.nonEmpty)
}

val networkRules = rules.rules.collect {
case networkRule: DynamicListNetworkRule => networkRule
val networkIdsToUse = listFilters.flatMap(_.networks).orElse {
Some(
rules.rules
.collect {
case networkRule: DynamicListNetworkRule => networkRule
}
.map(_.networkId)
.toSet
).filter(_.nonEmpty)
}

val peopleIdsToUse = listFilters
.flatMap(_.personIdentifiers)
.map(ids => personIdentifiersToSearch(ids.toList))
.orElse {
Some(
rules.rules.collect {
case personRule: DynamicListPersonRule => personRule
}.distinct
).filter(_.nonEmpty).map(personRulesToSearch)
}

val sourceBuilder = new SearchSourceBuilder()
val baseBoolQuery = QueryBuilders.boolQuery()

Expand All @@ -181,18 +233,21 @@ class DynamicListBuilder @Inject()(
)
})
)
.applyIf(personRules.nonEmpty)(
peopleCreditSearchQuery(_, personRulesToSearch(personRules))
.applyOptional(peopleIdsToUse)(
peopleCreditSearchQuery
)
.applyOptional(itemTypesToUse.filter(_.nonEmpty))(
itemTypesFilter
)
.applyOptional(genreIdsToUse.map(_.toSet))(genreIdsFilter)
.applyOptional(genreIdsToUse.map(_.toSet).filter(_.nonEmpty))(
genreIdsFilter
)
.applyOptional(
Some(networkRules.map(_.networkId)).filter(_.nonEmpty).map(_.toSet)
networkIdsToUse.filter(_.nonEmpty)
)(
availabilityByNetworkIdsOr
)
.applyOptional(releaseYearRule.filter(_.isFinite))(openDateRangeFilter)
.applyOptional(bookmark)(applyBookmark(_, _, Some(dynamicList)))

val defaultSort =
Expand Down Expand Up @@ -241,6 +296,23 @@ class DynamicListBuilder @Inject()(
PeopleCreditSearch(creditSearches, BinaryOperator.Or)
}

private def personIdentifiersToSearch(personIds: List[String]) = {
val creditSearches = personIds.flatMap(id => {
List(
PersonCreditSearch(
IdOrSlug(id),
PersonAssociationType.Cast
),
PersonCreditSearch(
IdOrSlug(id),
PersonAssociationType.Crew
)
)
})

PeopleCreditSearch(creditSearches, BinaryOperator.Or)
}

private def applyNextBookmark(
response: ElasticsearchItemsResponse,
previousBookmark: Option[Bookmark],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,31 +258,35 @@ class PersonLookup @Inject()(
}

def lookupPeopleBySlugs(slugs: List[Slug]): Future[Map[Slug, EsPerson]] = {
val multiSearchRequest = new MultiSearchRequest()
val searches = slugs.map(slug => {
val slugQuery = QueryBuilders
.boolQuery()
.filter(QueryBuilders.termQuery("slug", slug.toString))
if (slugs.isEmpty) {
Future.successful(Map.empty)
} else {
val multiSearchRequest = new MultiSearchRequest()
val searches = slugs.map(slug => {
val slugQuery = QueryBuilders
.boolQuery()
.filter(QueryBuilders.termQuery("slug", slug.toString))

new SearchRequest("people")
.source(new SearchSourceBuilder().query(slugQuery).size(1))
})
new SearchRequest("people")
.source(new SearchSourceBuilder().query(slugQuery).size(1))
})

searches.foreach(multiSearchRequest.add)
searches.foreach(multiSearchRequest.add)

elasticsearchExecutor
.multiSearch(multiSearchRequest)
.map(response => {
response.getResponses
.zip(slugs)
.flatMap {
case (response, slug) =>
// TODO: Properly handle failures
searchResponseToPeople(response.getResponse).items.headOption
.map(slug -> _)
}
.toMap
})
elasticsearchExecutor
.multiSearch(multiSearchRequest)
.map(response => {
response.getResponses
.zip(slugs)
.flatMap {
case (response, slug) =>
// TODO: Properly handle failures
searchResponseToPeople(response.getResponse).items.headOption
.map(slug -> _)
}
.toMap
})
}
}

private def lookupPersonCastCredits(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ object IdOrSlug {
override def idOrSlug: Either[UUID, Slug] = Left(uuid)
}
}

def apply(identifier: String): IdOrSlug = {
new IdOrSlug {
override lazy val idOrSlug: Either[UUID, Slug] =
HasThingIdOrSlug.parse(identifier)
}
}
}

trait IdOrSlug {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,23 @@ import scala.util.Try

case class ListFilters(
itemTypes: Option[Set[ThingType]],
genres: Option[Set[Int]])
genres: Option[Set[Int]],
networks: Option[Set[Int]],
personIdentifiers: Option[Set[String]])

class ListFilterParser @Inject()(genreCache: GenreCache) {
def parseListFilters(
itemTypes: Seq[String],
itemTypes: Option[Seq[String]],
genres: Option[Seq[String]]
)(implicit executionContext: ExecutionContext
): Future[ListFilters] = {
val types = if (itemTypes.nonEmpty) {
Some(
itemTypes
.flatMap(typ => Try(ThingType.fromString(typ)).toOption)
.toSet
)
} else {
None
val types = itemTypes match {
case Some(Seq()) => Some(Set.empty[ThingType])
case Some(value) =>
Some(
value.flatMap(typ => Try(ThingType.fromString(typ)).toOption).toSet
)
case None => None
}

val filteredGenreIdsFut = genres.map(_.map(HasGenreIdOrSlug.parse)) match {
Expand All @@ -49,7 +50,9 @@ class ListFilterParser @Inject()(genreCache: GenreCache) {
filteredGenreIdsFut.map(filteredGenreIds => {
ListFilters(
itemTypes = types,
genres = filteredGenreIds
genres = filteredGenreIds,
networks = None,
personIdentifiers = None
)
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ trait OpenRange[T] {
def isFinite: Boolean = start.isDefined || end.isDefined
}

object OpenDateRange {
def forYearRange(
start: Option[Int],
end: Option[Int]
): OpenDateRange = {
OpenDateRange(start.map(localDateAtYear), end.map(localDateAtYear))
}

private def localDateAtYear(year: Int): LocalDate = LocalDate.of(year, 1, 1)
}

case class OpenDateRange(
start: Option[LocalDate],
end: Option[LocalDate],
Expand Down
Loading

0 comments on commit dbd9b8f

Please sign in to comment.