Repository: scala-steward-org/scala-steward
Pull request #1842 (2020-12-26 22:57:21)

Add caching layer to pull requests store
The `pull_requests` key value store is repeatedly accessed in
`PruningAlg` with the same key. This happens when we're looking for
already existing PRs which means that the same JSON file is repeatedly
read and decoded. This PR adds a caching layer to the `pull_requests`
store that caches the value of last accessed key.

codecov[bot]: # [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=h1) Report
> Merging [#1842](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=desc) (93b5d13) into [master](https://codecov.io/gh/scala-steward-org/scala-steward/commit/bc4e449bee02e0b3361dd4432dc45b120d356ca5?el=desc) (bc4e449) will **decrease** coverage by `0.10%`.
> The diff coverage is `60.00%`.

[![Impacted file tree graph](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842/graphs/tree.svg?width=650&height=150&src=pr&token=udzMsc1Dne)](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1842      +/-   ##
==========================================
- Coverage   74.94%   74.83%   -0.11%     
==========================================
  Files         125      126       +1     
  Lines        2111     2122      +11     
  Branches       54       62       +8     
==========================================
+ Hits         1582     1588       +6     
- Misses        529      534       +5     
```


| [Impacted Files](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...la/org/scalasteward/core/application/Context.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842/diff?src=pr&el=tree#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS9hcHBsaWNhdGlvbi9Db250ZXh0LnNjYWxh) | `0.00% <0.00%> (ø)` | |
| [...teward/core/persistence/CachingKeyValueStore.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842/diff?src=pr&el=tree#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS9wZXJzaXN0ZW5jZS9DYWNoaW5nS2V5VmFsdWVTdG9yZS5zY2FsYQ==) | `83.33% <83.33%> (ø)` | |
| [...in/scala/org/scalasteward/core/vcs/data/Repo.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842/diff?src=pr&el=tree#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS92Y3MvZGF0YS9SZXBvLnNjYWxh) | `83.33% <100.00%> (+3.33%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=footer). Last update [bc4e449...93b5d13](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1842?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
index 3ed53d69..2a7b90a6 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
@@ -21,6 +21,7 @@ import cats.effect._
 import cats.syntax.all._
 import io.chrisdavenport.log4cats.Logger
 import io.chrisdavenport.log4cats.slf4j.Slf4jLogger
+import org.http4s.Uri
 import org.http4s.client.Client
 import org.http4s.client.okhttp.OkHttpBuilder
 import org.scalasteward.core.buildtool.BuildToolDispatcher
@@ -32,8 +33,8 @@ import org.scalasteward.core.edit.EditAlg
 import org.scalasteward.core.edit.hooks.HookExecutor
 import org.scalasteward.core.git.{GenGitAlg, GitAlg}
 import org.scalasteward.core.io.{FileAlg, ProcessAlg, WorkspaceAlg}
-import org.scalasteward.core.nurture.{NurtureAlg, PullRequestRepository}
-import org.scalasteward.core.persistence.JsonKeyValueStore
+import org.scalasteward.core.nurture.{NurtureAlg, PullRequestData, PullRequestRepository}
+import org.scalasteward.core.persistence.{CachingKeyValueStore, JsonKeyValueStore}
 import org.scalasteward.core.repocache.{RefreshErrorAlg, RepoCacheAlg, RepoCacheRepository}
 import org.scalasteward.core.repoconfig.RepoConfigAlg
 import org.scalasteward.core.scalafix.{MigrationAlg, MigrationsLoader}
@@ -41,7 +42,7 @@ import org.scalasteward.core.scalafmt.ScalafmtAlg
 import org.scalasteward.core.update.{ArtifactMigrations, FilterAlg, PruningAlg, UpdateAlg}
 import org.scalasteward.core.util._
 import org.scalasteward.core.util.uri._
-import org.scalasteward.core.vcs.data.AuthenticatedUser
+import org.scalasteward.core.vcs.data.{AuthenticatedUser, Repo}
 import org.scalasteward.core.vcs.github.{GitHubAppApiAlg, GitHubAuthAlg}
 import org.scalasteward.core.vcs.{VCSApiAlg, VCSExtraAlg, VCSRepoAlg, VCSSelection}
 
@@ -63,11 +64,16 @@ object Context {
         Resource.liftF(new MigrationsLoader[F].loadAll(config.scalafixCfg).map(new MigrationAlg(_)))
       implicit0(artifactMigration: ArtifactMigrations) <-
         Resource.liftF(ArtifactMigrations.create[F](config))
+      implicit0(workspaceAlg: WorkspaceAlg[F]) = WorkspaceAlg.create[F](config)
+      kvsPrefix = Some(config.vcsType.asString)
+      pullRequestsStore <- Resource.liftF(
+        CachingKeyValueStore.wrap(
+          new JsonKeyValueStore[F, Repo, Map[Uri, PullRequestData]]("pull_requests", "2", kvsPrefix)
+        )
+      )
     } yield {
-      val kvsPrefix = Some(config.vcsType.asString)
       implicit val dateTimeAlg: DateTimeAlg[F] = DateTimeAlg.create[F]
       implicit val processAlg: ProcessAlg[F] = ProcessAlg.create[F](blocker, config.processCfg)
-      implicit val workspaceAlg: WorkspaceAlg[F] = WorkspaceAlg.create[F](config)
       implicit val repoConfigAlg: RepoConfigAlg[F] = new RepoConfigAlg[F](config)
       implicit val filterAlg: FilterAlg[F] = new FilterAlg[F]
       implicit val gitAlg: GitAlg[F] = GenGitAlg.create[F](config)
@@ -81,7 +87,7 @@ object Context {
       implicit val vcsRepoAlg: VCSRepoAlg[F] = new VCSRepoAlg[F](config)
       implicit val vcsExtraAlg: VCSExtraAlg[F] = VCSExtraAlg.create[F](config)
       implicit val pullRequestRepository: PullRequestRepository[F] =
-        new PullRequestRepository[F](new JsonKeyValueStore("pull_requests", "2", kvsPrefix))
+        new PullRequestRepository[F](pullRequestsStore)
       implicit val scalafmtAlg: ScalafmtAlg[F] = ScalafmtAlg.create[F]
       implicit val selfCheckAlg: SelfCheckAlg[F] = new SelfCheckAlg[F]
       implicit val coursierAlg: CoursierAlg[F] = CoursierAlg.create[F]
diff --git a/modules/core/src/main/scala/org/scalasteward/core/persistence/CachingKeyValueStore.scala b/modules/core/src/main/scala/org/scalasteward/core/persistence/CachingKeyValueStore.scala
new file mode 100644
index 00000000..fd9b5a3c
--- /dev/null
+++ b/modules/core/src/main/scala/org/scalasteward/core/persistence/CachingKeyValueStore.scala
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2018-2020 Scala Steward contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.scalasteward.core.persistence
+
+import cats.effect.Sync
+import cats.effect.concurrent.Ref
+import cats.syntax.all._
+import cats.{Eq, Monad}
+
+final class CachingKeyValueStore[F[_], K, V](
+    underlying: KeyValueStore[F, K, V],
+    cache: Ref[F, Option[(K, Option[V])]]
+)(implicit kEq: Eq[K], F: Monad[F])
+    extends KeyValueStore[F, K, V] {
+  override def get(key: K): F[Option[V]] =
+    cache.get.flatMap {
+      case Some((cachedKey, cachedValue)) if cachedKey === key =>
+        F.pure(cachedValue)
+      case _ =>
+        underlying.get(key).flatTap(value => cache.set(Some(key -> value)))
+    }
+
+  override def set(key: K, value: Option[V]): F[Unit] =
+    underlying.set(key, value) >> cache.set(Some(key -> value))
+}
+
+object CachingKeyValueStore {
+  def wrap[F[_], K, V](
+      underlying: KeyValueStore[F, K, V]
+  )(implicit kEq: Eq[K], F: Sync[F]): F[KeyValueStore[F, K, V]] =
+    Ref[F].of(Option.empty[(K, Option[V])]).map(new CachingKeyValueStore[F, K, V](underlying, _))
+}
diff --git a/modules/core/src/main/scala/org/scalasteward/core/vcs/data/Repo.scala b/modules/core/src/main/scala/org/scalasteward/core/vcs/data/Repo.scala
index d9e9f179..6baab8ec 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/vcs/data/Repo.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/vcs/data/Repo.scala
@@ -16,6 +16,7 @@
 
 package org.scalasteward.core.vcs.data
 
+import cats.Eq
 import io.circe.{KeyDecoder, KeyEncoder}
 
 final case class Repo(
@@ -26,6 +27,9 @@ final case class Repo(
 }
 
 object Repo {
+  implicit val repoEq: Eq[Repo] =
+    Eq.fromUniversalEquals
+
   implicit val repoKeyDecoder: KeyDecoder[Repo] = {
     val / = s"(.+)/([^/]+)".r
     KeyDecoder.instance {


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/persistence/JsonKeyValueStoreTest.scala b/modules/core/src/test/scala/org/scalasteward/core/persistence/JsonKeyValueStoreTest.scala
index 3303d48f..4990d2d6 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/persistence/JsonKeyValueStoreTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/persistence/JsonKeyValueStoreTest.scala
@@ -56,4 +56,25 @@ class JsonKeyValueStoreTest extends FunSuite {
     )
     assertEquals(state, expected)
   }
+
+  test("cached") {
+    val p = for {
+      kvStore <- CachingKeyValueStore.wrap(
+        new JsonKeyValueStore[MockEff, String, String]("test", "0")
+      )
+      _ <- kvStore.put("k1", "v1")
+      v1 <- kvStore.get("k1")
+      v2 <- kvStore.get("k2")
+    } yield (v1, v2)
+    val (state, value) = p.run(MockState.empty).unsafeRunSync()
+    assertEquals(value, (Some("v1"), None))
+
+    val k1File = config.workspace / "store" / "test" / "v0" / "k1" / "test.json"
+    val k2File = config.workspace / "store" / "test" / "v0" / "k2" / "test.json"
+    val expected = MockState.empty.copy(
+      commands = Vector(List("write", k1File.toString), List("read", k2File.toString)),
+      files = Map(k1File -> """"v1"""")
+    )
+    assertEquals(state, expected)
+  }
 }