Don't request all the blocks for liveblog main page

article/app/controllers/ArticleController.scala

@@ -1,13 +1,14 @@
 package controllers
 
-import liveblog.LiveBlogCurrentPage
-import com.gu.contentapi.client.model.v1.{ItemResponse, Content => ApiContent}
+import _root_.liveblog._
+import com.gu.contentapi.client.model.v1.{Content => ApiContent, ItemResponse}
 import common._
 import conf.switches.Switches
 import contentapi.ContentApiClient
+import controllers.ParseBlockId.{InvalidFormat, ParsedBlockId}
 import model.Cached.WithoutRevalidationResult
 import model._
-import model.liveblog.{BodyBlock, KeyEventData}
+import model.liveblog.BodyBlock
 import org.joda.time.DateTime
 import org.scala_tools.time.Imports._
 import play.api.libs.functional.syntax._
@@ -32,11 +33,15 @@ class ArticleController extends Controller with RendersItemResponse with Logging
 
   private def isSupported(c: ApiContent) = c.isArticle || c.isLiveBlog || c.isSudoku
   override def canRender(i: ItemResponse): Boolean = i.content.exists(isSupported)
-  override def renderItem(path: String)(implicit request: RequestHeader): Future[Result] = mapModel(path, blocks = true)(render(path, _))
+  override def renderItem(path: String)(implicit request: RequestHeader): Future[Result] = mapModel(path, Some(Canonical))(render(path, _))
 
 
-  private def renderNewerUpdates(page: PageWithStoryPackage, lastUpdateBlockId: String, isLivePage: Option[Boolean])(implicit request: RequestHeader): Result = {
-    val newBlocks = page.article.fields.blocks.takeWhile(block => s"block-${block.id}" != lastUpdateBlockId)
+  private def renderNewerUpdates(page: PageWithStoryPackage, lastUpdateBlockId: SinceBlockId, isLivePage: Option[Boolean])(implicit request: RequestHeader): Result = {
+    val newBlocks = page.article.fields.blocks.toSeq.flatMap {
+      _.requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq())
+    }.takeWhile { block =>
+      block.id != lastUpdateBlockId.lastUpdate
+    }
     val blocksHtml = views.html.liveblog.liveBlogBlocks(newBlocks, page.article, Edition(request).timezone)
     val timelineHtml = views.html.liveblog.keyEvents("", KeyEventData(newBlocks, Edition(request).timezone))
     val allPagesJson = Seq(
@@ -46,7 +51,7 @@ class ArticleController extends Controller with RendersItemResponse with Logging
     val livePageJson = isLivePage.filter(_ == true).map { _ =>
       "html" -> blocksHtml
     }
-    val mostRecent = page.article.fields.blocks.headOption.map { block =>
+    val mostRecent = newBlocks.headOption.map { block =>
       "mostRecentBlockId" -> s"block-${block.id}"
     }
     Cached(page)(JsonComponent((allPagesJson ++ livePageJson ++ mostRecent): _*))
@@ -70,9 +75,14 @@ class ArticleController extends Controller with RendersItemResponse with Logging
 
   private def blockText(page: PageWithStoryPackage, number: Int)(implicit request: RequestHeader): Result = page match {
     case LiveBlogPage(liveBlog, _, _) =>
-      val blocks = liveBlog.blocks.collect {
-        case BodyBlock(id, html, _, title, _, _, _, publishedAt, _, updatedAt, _, _) if html.trim.nonEmpty =>
-          TextBlock(id, title, publishedAt, updatedAt, html)
+      val blocks =
+        liveBlog.blocks.toSeq.flatMap { blocks =>
+        blocks.requestedBodyBlocks.get(Canonical.firstPage).toSeq.flatMap { bodyBlocks: Seq[BodyBlock] =>
+          bodyBlocks.collect {
+            case BodyBlock(id, html, _, title, _, _, _, publishedAt, _, updatedAt, _, _) if html.trim.nonEmpty =>
+              TextBlock(id, title, publishedAt, updatedAt, html)
+          }
+        }
       }.take(number)
       Cached(page)(JsonComponent("blocks" -> Json.toJson(blocks)))
     case _ => Cached(600)(WithoutRevalidationResult(NotFound("Can only return block text for a live blog")))
@@ -118,19 +128,34 @@ class ArticleController extends Controller with RendersItemResponse with Logging
 
   def renderLiveBlog(path: String, page: Option[String] = None) =
     Action.async { implicit request =>
-      mapModel(path, blocks = true, page) {// temporarily only ask for blocks too for things we know are new live blogs until until the migration is done and we can always use blocks
-        render(path, _)
+
+      def renderWithRange(range: BlockRange) =
+        mapModel(path, range = Some(range)) {// temporarily only ask for blocks too for things we know are new live blogs until until the migration is done and we can always use blocks
+          render(path, _)
+        }
+
+      page.map(ParseBlockId.fromPageParam) match {
+        case Some(ParsedBlockId(id)) => renderWithRange(PageWithBlock(id)) // we know the id of a block
+        case Some(InvalidFormat) => Future.successful(Cached(10)(WithoutRevalidationResult(NotFound))) // page param there but couldn't extract a block id
+        case None => renderWithRange(Canonical) // no page param
       }
     }
 
   def renderLiveBlogJson(path: String, lastUpdate: Option[String], rendered: Option[Boolean], isLivePage: Option[Boolean]) = {
     Action.async { implicit request =>
-      mapModel(path, blocks = true) { model =>
-        (lastUpdate, rendered) match {
-          case (Some(lastUpdate), _) => renderNewerUpdates(model, lastUpdate, isLivePage)
-          case (None, Some(false)) => blockText(model, 6)
-          case (_, _) => render(path, model)
+
+      def renderWithRange(range: BlockRange) =
+        mapModel(path, Some(range)) { model =>
+          range match {
+            case SinceBlockId(lastBlockId) => renderNewerUpdates(model, SinceBlockId(lastBlockId), isLivePage)
+            case _ => render(path, model)
+          }
         }
+
+      lastUpdate.map(ParseBlockId.fromBlockId) match {
+        case Some(ParsedBlockId(id)) => renderWithRange(SinceBlockId(id))
+        case Some(InvalidFormat) => Future.successful(Cached(10)(WithoutRevalidationResult(NotFound))) // page param there but couldn't extract a block id
+        case None => if (rendered.contains(false)) mapModel(path) { model => blockText(model, 6) } else renderWithRange(Canonical) // no page param
       }
     }
   }
@@ -145,20 +170,22 @@ class ArticleController extends Controller with RendersItemResponse with Logging
 
   def renderArticle(path: String) = {
     Action.async { implicit request =>
-      mapModel(path, blocks = request.isEmail) {
+      mapModel(path, range = if (request.isEmail) Some(ArticleBlocks) else None) {
         render(path, _)
       }
     }
   }
 
-  def mapModel(path: String, blocks: Boolean = false, pageParam: Option[String] = None)(render: PageWithStoryPackage => Result)(implicit request: RequestHeader): Future[Result] = {
-    lookup(path, blocks) map responseToModelOrResult(pageParam) recover convertApiExceptions map {
+  // range: None means the url didn't include /live/, Some(...) means it did.  Canonical just means no url parameter
+  // if we switch to using blocks instead of body for articles, then it no longer needs to be Optional
+  def mapModel(path: String, range: Option[BlockRange] = None)(render: PageWithStoryPackage => Result)(implicit request: RequestHeader): Future[Result] = {
+    lookup(path, range) map responseToModelOrResult(range) recover convertApiExceptions map {
       case Left(model) => render(model)
       case Right(other) => RenderOtherStatus(other)
     }
   }
 
-  private def lookup(path: String, blocks: Boolean)(implicit request: RequestHeader): Future[ItemResponse] = {
+  private def lookup(path: String, range: Option[BlockRange])(implicit request: RequestHeader): Future[ItemResponse] = {
     val edition = Edition(request)
 
     log.info(s"Fetching article: $path for edition ${edition.id}: ${RequestLog(request)}")
@@ -168,7 +195,10 @@ class ArticleController extends Controller with RendersItemResponse with Logging
       .showReferences("all")
       .showAtoms("all")
 
-    val capiItemWithBlocks = if (blocks) capiItem.showBlocks("body") else capiItem
+    val capiItemWithBlocks = range.map { blockRange =>
+      val blocksParam = blockRange.query.map(_.mkString(",")).getOrElse("body")
+      capiItem.showBlocks(blocksParam)
+    }.getOrElse(capiItem)
     ContentApiClient.getResponse(capiItemWithBlocks)
 
   }
@@ -180,35 +210,34 @@ class ArticleController extends Controller with RendersItemResponse with Logging
     * @param response
    * @return Either[PageWithStoryPackage, Result]
    */
-  def responseToModelOrResult(pageParam: Option[String])(response: ItemResponse)(implicit request: RequestHeader): Either[PageWithStoryPackage, Result] = {
+  def responseToModelOrResult(range: Option[BlockRange])(response: ItemResponse)(implicit request: RequestHeader): Either[PageWithStoryPackage, Result] = {
     val supportedContent = response.content.filter(isSupported).map(Content(_))
-    val page = pageParam.map(ParseBlockId.apply)
     val supportedContentResult = ModelOrResult(supportedContent, response)
-    val content: Either[PageWithStoryPackage, Result] = supportedContentResult.left.flatMap { content =>
-      (content, page) match {
-        case (minute: Article, None) if minute.isUSMinute =>
-          Left(MinutePage(minute, StoryPackages(minute, response)))
-        case (liveBlog: Article, None/*no page param*/) if liveBlog.isLiveBlog =>
-          createLiveBlogModel(liveBlog, response, None)
-        case (liveBlog: Article, Some(Some(requiredBlockId))/*page param specified and valid format*/) if liveBlog.isLiveBlog =>
-          createLiveBlogModel(liveBlog, response, Some(requiredBlockId))
-        case (article: Article, None) => Left(ArticlePage(article, StoryPackages(article, response)))
-        case _ =>
-          Right(NotFound)
-      }
+    val content: Either[PageWithStoryPackage, Result] = supportedContentResult.left.flatMap {
+      case minute: Article if minute.isUSMinute =>
+        Left(MinutePage(minute, StoryPackages(minute, response)))
+      case liveBlog: Article if liveBlog.isLiveBlog =>
+        range.map {
+          createLiveBlogModel(liveBlog, response, _)
+        }.getOrElse(Right(NotFound))
+      case article: Article =>
+        Left(ArticlePage(article, StoryPackages(article, response)))
     }
 
     content
   }
 
-  def createLiveBlogModel(liveBlog: Article, response: ItemResponse, maybeRequiredBlockId: Option[String]) = {
+  def createLiveBlogModel(liveBlog: Article, response: ItemResponse, range: BlockRange) = {
 
     val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10
-    val liveBlogPageModel = LiveBlogCurrentPage(
-      pageSize = pageSize,
-      liveBlog.content.fields.blocks,
-      maybeRequiredBlockId
-    )
+    val liveBlogPageModel =
+      liveBlog.content.fields.blocks.map { blocks =>
+        LiveBlogCurrentPage(
+          pageSize = pageSize,
+          blocks,
+          range
+        )
+      } getOrElse None
     liveBlogPageModel match {
       case Some(pageModel) =>
 
@@ -235,15 +264,29 @@ class ArticleController extends Controller with RendersItemResponse with Logging
 }
 
 object ParseBlockId extends RegexParsers {
-  def apply(input: String): Option[String] = {
-    def withParser: Parser[Unit] = "with:" ^^ { _ => () }
-    def block: Parser[Unit] = "block-" ^^ { _ => () }
-    def id: Parser[String] = "[a-zA-Z0-9]+".r
-    def expr: Parser[String] = withParser ~> block ~> id
+
+  sealed trait ParseResult { def toOption: Option[String] }
+  case object InvalidFormat extends ParseResult { val toOption = None }
+  case class ParsedBlockId(blockId: String) extends ParseResult { val toOption = Some(blockId) }
+
+  private def withParser: Parser[Unit] = "with:" ^^ { _ => () }
+  private def block: Parser[Unit] = "block-" ^^ { _ => () }
+  private def id: Parser[String] = "[a-zA-Z0-9]+".r
+  private def blockId = block ~> id
+
+  def fromPageParam(input: String): ParseResult = {
+    def expr: Parser[String] = withParser ~> blockId
 
     parse(expr, input) match {
-      case Success(matched, _) => Some(matched)
-      case _ => None
+      case Success(matched, _) => ParsedBlockId(matched)
+      case _ => InvalidFormat
+    }
+  }
+
+  def fromBlockId(input: String): ParseResult = {
+    parse(blockId, input) match {
+      case Success(matched, _) => ParsedBlockId(matched)
+      case _ => InvalidFormat
     }
   }
 }

article/app/liveblog/KeyEventData.scala

@@ -0,0 +1,32 @@
+package liveblog
+
+import model.liveblog.BodyBlock.{KeyEvent, SummaryEvent}
+import model.liveblog.{Blocks, BodyBlock, LiveBlogDate}
+import org.scala_tools.time.Imports._
+
+object KeyEventData {
+
+  // just for convenience for use from the templates
+  def apply(maybeBlocks: Option[Blocks], timezone: DateTimeZone): Seq[KeyEventData] = {
+
+    val blocks = maybeBlocks.toSeq.flatMap(blocks => blocks.requestedBodyBlocks.getOrElse(Canonical.timeline, blocks.body))
+
+    apply(blocks, timezone)
+  }
+
+  def apply(blocks: Seq[BodyBlock], timezone: DateTimeZone): Seq[KeyEventData] = {
+
+    val TimelineMaxEntries = 7
+
+    val latestSummary = blocks.find(_.eventType == SummaryEvent)
+    val keyEvents = blocks.filter(_.eventType == KeyEvent)
+    val bodyBlocks = (latestSummary.toSeq ++ keyEvents).sortBy(_.lastModifiedDate.getOrElse(new DateTime(0)).millis).reverse.take(TimelineMaxEntries)
+
+    bodyBlocks.map { bodyBlock =>
+      KeyEventData(bodyBlock.id, bodyBlock.publishedCreatedDate(timezone), bodyBlock.title)
+    }
+  }
+
+}
+
+case class KeyEventData(id: String, time: Option[LiveBlogDate], title: Option[String])

article/app/liveblog/LiveBlogCurrentPage.scala

@@ -1,10 +1,71 @@
 package liveblog
 
-import model.liveblog.BodyBlock
+import model.liveblog.{Blocks, BodyBlock}
+
+sealed trait BlockRange { def query: Option[Seq[String]] }
+case object Canonical extends BlockRange {
+  val firstPage = "body:latest:30"
+  val oldestPage = "body:oldest:1"
+  val timeline = "body:key-events"
+  // this only makes sense for liveblogs at the moment, but article use field body not blocks anyway
+  val query = Some(Seq(firstPage, oldestPage, timeline))
+}
+case class PageWithBlock(page: String) extends BlockRange {
+  // just get them all, the caching should prevent too many requests, could use "around"
+  val query = None
+}
+case object ArticleBlocks extends BlockRange {
+  // we currently only use this for emails, we just want all the blocks
+  val query = None
+}
+case class SinceBlockId(lastUpdate: String) extends BlockRange {
+  val around = s"body:around:$lastUpdate:5"
+  // more than 5 could come in (in one go), but unlikely and won't matter as it'll just fetch again soon
+  val query = Some(Seq(around))
+}
 
 object LiveBlogCurrentPage {
 
-  def apply(pageSize: Int, blocks: Seq[BodyBlock], isRequestedBlock: Option[String]): Option[LiveBlogCurrentPage] = {
+  def apply(pageSize: Int, blocks: Blocks, range: BlockRange): Option[LiveBlogCurrentPage] = {
+    range match {
+      case Canonical => firstPage(pageSize, blocks)
+      case PageWithBlock(isRequestedBlock) => findPageWithBlock(pageSize, blocks.body, isRequestedBlock)
+      case SinceBlockId(blockId) => updates(blocks, SinceBlockId(blockId))
+      case ArticleBlocks => None
+    }
+  }
+
+  // filters newer blocks out of the list
+  def updates(blocks: Blocks, sinceBlockId: SinceBlockId) = {
+    val bodyBlocks = blocks.requestedBodyBlocks.get(sinceBlockId.around).toSeq.flatMap { bodyBlocks =>
+      bodyBlocks.takeWhile(_.id != sinceBlockId.lastUpdate)
+    }
+    Some(LiveBlogCurrentPage(FirstPage(bodyBlocks), None))// just pretend to be the first page, it'll be ignored
+  }
+
+  // turns the slimmed down (to save bandwidth) capi response into a first page model object
+  def firstPage(pageSize: Int, blocks: Blocks): Option[LiveBlogCurrentPage] = {
+    val remainder = blocks.totalBodyBlocks % pageSize
+    val numPages = blocks.totalBodyBlocks / pageSize
+    blocks.requestedBodyBlocks.get(Canonical.firstPage).map { requestedBodyBlocks =>
+      val (firstPageBlocks, startOfSecondPageBlocks) = requestedBodyBlocks.splitAt(remainder + pageSize)
+      val oldestPage = blocks.requestedBodyBlocks.get(Canonical.oldestPage).flatMap(_.headOption.map { block =>
+        BlockPage(blocks = Nil, blockId = block.id, pageNumber = numPages)
+      } )
+      val olderPage = startOfSecondPageBlocks.headOption.map { block => BlockPage(blocks = Nil, blockId = block.id, pageNumber = 2) }
+      val pagination = if (blocks.totalBodyBlocks > firstPageBlocks.size) Some(Pagination(
+        newest = None,
+        newer = None,
+        oldest = oldestPage,
+        older = olderPage,
+        numberOfPages = numPages
+      )) else None
+      LiveBlogCurrentPage(FirstPage(firstPageBlocks), pagination)
+    }
+  }
+
+  // turns a full capi blocks list into a page model of the page with a specific block in it
+  def findPageWithBlock(pageSize: Int, blocks: Seq[BodyBlock], isRequestedBlock: String) = {
     val (mainPageBlocks, restPagesBlocks) = getPages(pageSize, blocks)
     val newestPage = FirstPage(mainPageBlocks)
     val pages = newestPage :: restPagesBlocks
@@ -18,7 +79,7 @@ object LiveBlogCurrentPage {
     val endedPages = None :: (pages.map(Some.apply) :+ None)
 
     def hasRequestedBlock(page: LiveBlogCurrentPage): Boolean = {
-      isRequestedBlock.map(isRequestedBlock => page.currentPage.blocks.exists(_.id == isRequestedBlock)).getOrElse(true)
+      page.currentPage.blocks.exists(_.id == isRequestedBlock)
     }
 
 
@@ -77,3 +138,11 @@ case class BlockPage(blocks: Seq[BodyBlock], blockId: String, pageNumber: Int) e
   val suffix = s"?page=with:block-$blockId"
   val isArchivePage = true
 }
+
+object LatestBlock {
+  def apply(maybeBlocks: Option[Blocks]): Option[String] = {
+    maybeBlocks.flatMap { blocks =>
+      blocks.requestedBodyBlocks.getOrElse(Canonical.firstPage, blocks.body).headOption.map(_.id)
+    }
+  }
+}