From 4864a708772f47de26ba869faf9d01d5245e6e25 Mon Sep 17 00:00:00 2001
From: Thibault Duplessis <t@lichess.org>
Date: Fri, 5 Feb 2021 11:39:07 +0100
Subject: [PATCH] fix study server analysis

---
 modules/study/src/main/ChapterRepo.scala | 47 ++++++++++++++--------
 modules/study/src/main/ServerEval.scala  | 50 ++++++++++++++----------
 2 files changed, 59 insertions(+), 38 deletions(-)

diff --git a/modules/study/src/main/ChapterRepo.scala b/modules/study/src/main/ChapterRepo.scala
index ce373ab8f3..a27d361541 100644
--- a/modules/study/src/main/ChapterRepo.scala
+++ b/modules/study/src/main/ChapterRepo.scala
@@ -119,20 +119,20 @@ final class ChapterRepo(val coll: AsyncColl)(implicit
   def setTagsFor(chapter: Chapter) =
     coll(_.updateField($id(chapter.id), "tags", chapter.tags)).void
 
-  def setShapes(shapes: lila.tree.Node.Shapes) = setNodeValue("h", shapes.value.nonEmpty option shapes) _
+  def setShapes(shapes: lila.tree.Node.Shapes) =
+    setNodeValue(Node.BsonFields.shapes, shapes.value.nonEmpty option shapes) _
 
   def setComments(comments: lila.tree.Node.Comments) =
-    setNodeValue("co", comments.value.nonEmpty option comments) _
+    setNodeValue(Node.BsonFields.comments, comments.value.nonEmpty option comments) _
 
-  def setGamebook(gamebook: lila.tree.Node.Gamebook) = setNodeValue("ga", gamebook.nonEmpty option gamebook) _
+  def setGamebook(gamebook: lila.tree.Node.Gamebook) =
+    setNodeValue(Node.BsonFields.gamebook, gamebook.nonEmpty option gamebook) _
 
-  def setGlyphs(glyphs: chess.format.pgn.Glyphs) = setNodeValue("g", glyphs.nonEmpty) _
+  def setGlyphs(glyphs: chess.format.pgn.Glyphs) = setNodeValue(Node.BsonFields.glyphs, glyphs.nonEmpty) _
 
-  def setClock(clock: Option[chess.Centis]) = setNodeValue("l", clock) _
+  def setClock(clock: Option[chess.Centis]) = setNodeValue(Node.BsonFields.clock, clock) _
 
-  def forceVariation(force: Boolean) = setNodeValue("fv", force option true) _
-
-  def setScore(score: Option[lila.tree.Eval.Score]) = setNodeValue("e", score) _
+  def forceVariation(force: Boolean) = setNodeValue(Node.BsonFields.forceVariation, force option true) _
 
   // overrides all children sub-nodes in DB! Make the tree merge beforehand.
   def setChildren(children: Node.Children)(chapter: Chapter, path: Path): Funit = {
@@ -162,15 +162,28 @@ final class ChapterRepo(val coll: AsyncColl)(implicit
         $id(chapter.id) ++ $doc(path.toDbField $exists true),
         pathToField(path, field),
         value
-      ).map {
-        case 0 =>
-          logger.warn(
-            s"Can't setNodeValue ${chapter.studyId}/${chapter.id} '$path' $field '${path.toDbField}' / no node matched!"
-          )
-          false
-        case _ => true
-      }
-    }.void
+      ).void
+    }
+
+  private[study] def setNodeValues(
+      chapter: Chapter,
+      path: Path,
+      values: List[(String, Option[BSONValue])]
+  ): Funit =
+    values.collect { case (field, Some(v)) =>
+      pathToField(path, field) -> v
+    } match {
+      case Nil => funit
+      case sets =>
+        coll {
+          _.update
+            .one(
+              $id(chapter.id) ++ $doc(path.toDbField $exists true),
+              $set($doc(sets))
+            )
+            .void
+        }
+    }
 
   // root.path.subField
   private def pathToField(path: Path, subField: String): String = s"${path.toDbField}.$subField"
diff --git a/modules/study/src/main/ServerEval.scala b/modules/study/src/main/ServerEval.scala
index bbf280dd92..becc56a315 100644
--- a/modules/study/src/main/ServerEval.scala
+++ b/modules/study/src/main/ServerEval.scala
@@ -59,32 +59,40 @@ object ServerEval {
               lila.common.Future
                 .fold(chapter.root.mainline.zip(analysis.infoAdvices).toList)(Path.root) {
                   case (path, (node, (info, advOpt))) =>
-                    info.eval.score
-                      .ifTrue {
-                        node.score.isEmpty ||
-                        advOpt.isDefined && node.comments.findBy(Comment.Author.Lichess).isEmpty
-                      }
-                      .?? { score =>
-                        chapterRepo.setScore(score.some)(chapter, path + node) >>
-                          advOpt.?? { adv =>
-                            chapterRepo.setComments(
+                    chapter.root.nodeAt(path).flatMap { parent =>
+                      analysisLine(parent, chapter.setup.variant, info) map parent.addChild
+                    } ?? { parentWithNewChildren =>
+                      chapterRepo.setChildren(parentWithNewChildren.children)(chapter, path)
+                    } >> {
+                      import BSONHandlers._
+                      import Node.{ BsonFields => F }
+                      chapterRepo.setNodeValues(
+                        chapter,
+                        path + node,
+                        List(
+                          F.score -> info.eval.score
+                            .ifTrue {
+                              node.score.isEmpty ||
+                              advOpt.isDefined && node.comments.findBy(Comment.Author.Lichess).isEmpty
+                            }
+                            .flatMap(EvalScoreBSONHandler.writeOpt),
+                          F.comments -> advOpt
+                            .map { adv =>
                               node.comments + Comment(
                                 Comment.Id.make,
                                 Comment.Text(adv.makeComment(withEval = false, withBestMove = true)),
                                 Comment.Author.Lichess
                               )
-                            )(chapter, path + node) >>
-                              chapterRepo.setGlyphs(
-                                node.glyphs merge Glyphs.fromList(List(adv.judgment.glyph))
-                              )(chapter, path + node) >> {
-                                chapter.root.nodeAt(path).flatMap { parent =>
-                                  analysisLine(parent, chapter.setup.variant, info) map parent.addChild
-                                } ?? { parentWithNewChildren =>
-                                  chapterRepo.setChildren(parentWithNewChildren.children)(chapter, path)
-                                }
-                              }
-                          }
-                      } inject path + node
+                            }
+                            .flatMap(CommentsBSONHandler.writeOpt),
+                          F.glyphs -> advOpt
+                            .map { adv =>
+                              node.glyphs merge Glyphs.fromList(List(adv.judgment.glyph))
+                            }
+                            .flatMap(GlyphsBSONHandler.writeOpt)
+                        )
+                      )
+                    } inject path + node
                 } void
             } >>- {
               chapterRepo.byId(Chapter.Id(analysis.id)).foreach {