refactor: use LimitedFilesManager for reports

scalameta · Jul 11, 2023 · 3342ff1 · 3342ff1
1 parent 2723cdc
commit 3342ff1
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 47 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/LoggerContext.scala b/metals/src/main/scala/scala/meta/internal/metals/LoggerContext.scala
@@ -2,17 +2,19 @@ package scala.meta.internal.metals
 
 import java.nio.file.Path
 
+import scala.meta.internal.metals.utils.TimestampedFile
+
 object LoggerReporter extends Reporter {
 
   override def create(report: => Report, ifVerbose: Boolean): Option[Path] = {
     scribe.info(s"Report ${report.name}: ${report.fullText}")
     None
   }
 
-  override def cleanUpOldReports(maxReportsNumber: Int): List[ReportFile] =
+  override def cleanUpOldReports(maxReportsNumber: Int): List[TimestampedFile] =
     List()
 
-  override def getReports(): List[ReportFile] = List()
+  override def getReports(): List[TimestampedFile] = List()
 
   override def deleteAll(): Unit = {}
 }

diff --git a/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala b/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala
@@ -1,10 +1,11 @@
 package scala.meta.internal.metals
 
-import java.io.File
 import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
 
+import scala.meta.internal.metals.utils.LimitedFilesManager
+import scala.meta.internal.metals.utils.TimestampedFile
 import scala.meta.internal.mtags.CommonMtagsEnrichments._
 
 trait ReportContext {
@@ -23,8 +24,8 @@ trait Reporter {
   def create(report: => Report, ifVerbose: Boolean = false): Option[Path]
   def cleanUpOldReports(
       maxReportsNumber: Int = StdReportContext.MAX_NUMBER_OF_REPORTS
-  ): List[ReportFile]
-  def getReports(): List[ReportFile]
+  ): List[TimestampedFile]
+  def getReports(): List[TimestampedFile]
   def deleteAll(): Unit
 }
 
@@ -68,6 +69,12 @@ class StdReporter(workspace: Path, pathToReports: Path, level: ReportLevel)
     extends Reporter {
   private lazy val reportsDir =
     workspace.resolve(pathToReports).createDirectories()
+  private val limitedFilesManager =
+    new LimitedFilesManager(
+      reportsDir,
+      StdReportContext.MAX_NUMBER_OF_REPORTS,
+      "r_.*_"
+    )
 
   private lazy val userHome = Option(System.getProperty("user.home"))
 
@@ -119,25 +126,10 @@ class StdReporter(workspace: Path, pathToReports: Path, level: ReportLevel)
 
   override def cleanUpOldReports(
       maxReportsNumber: Int = StdReportContext.MAX_NUMBER_OF_REPORTS
-  ): List[ReportFile] = {
-    val reports = getReports()
-    if (reports.length > maxReportsNumber) {
-      val filesToDelete = reports
-        .sortBy(_.timestamp)
-        .slice(0, reports.length - maxReportsNumber)
-      filesToDelete.foreach { f => Files.delete(f.toPath) }
-      filesToDelete
-    } else List()
-  }
+  ): List[TimestampedFile] = limitedFilesManager.deleteOld(maxReportsNumber)
 
-  override def getReports(): List[ReportFile] = {
-    val reportsDir = workspace.resolve(pathToReports)
-    if (reportsDir.exists && Files.isDirectory(reportsDir)) {
-      reportsDir.toFile.listFiles().toList.map(ReportFile.fromFile(_)).collect {
-        case Some(l) => l
-      }
-    } else List()
-  }
+  override def getReports(): List[TimestampedFile] =
+    limitedFilesManager.getAllFiles()
 
   override def deleteAll(): Unit =
     getReports().foreach(r => Files.delete(r.toPath))
@@ -153,30 +145,15 @@ object StdReportContext {
   def reportsDir: Path = Paths.get(".metals").resolve(".reports")
 }
 
-case class ReportFile(file: File, timestamp: Long) {
-  def toPath: Path = file.toPath()
-  def name: String = file.getName()
-}
-
-object ReportFile {
-  def fromFile(file: File): Option[ReportFile] = {
-    val reportRegex = "r_.*_([-+]?[0-9]+)".r
-    file.getName() match {
-      case reportRegex(time) => Some(ReportFile(file, time.toLong))
-      case _: String => None
-    }
-  }
-}
-
 object EmptyReporter extends Reporter {
 
   override def create(report: => Report, ifVerbose: Boolean): Option[Path] =
     None
 
-  override def cleanUpOldReports(maxReportsNumber: Int): List[ReportFile] =
+  override def cleanUpOldReports(maxReportsNumber: Int): List[TimestampedFile] =
     List()
 
-  override def getReports(): List[ReportFile] = List()
+  override def getReports(): List[TimestampedFile] = List()
 
   override def deleteAll(): Unit = {}
 }

diff --git a/...al/metals/utils/LimitedFilesManager.scala → ...al/metals/utils/LimitedFilesManager.scala b/...al/metals/utils/LimitedFilesManager.scala → ...al/metals/utils/LimitedFilesManager.scala
@@ -7,7 +7,7 @@ import java.nio.file.Path
 class LimitedFilesManager(
     directory: Path,
     fileLimit: Int,
-    prefixPattern: String,
+    prefixPattern: String
 ) {
   private val fileNameRegex = s"${prefixPattern}([-+]?[0-9]+)".r
 
@@ -17,12 +17,12 @@ class LimitedFilesManager(
     } else List()
   }
 
-  def deleteOld(): List[TimestampedFile] = {
+  def deleteOld(limit: Int = fileLimit): List[TimestampedFile] = {
     val files = getAllFiles()
-    if (files.length > fileLimit) {
+    if (files.length > limit) {
       val filesToDelete = files
         .sortBy(_.timestamp)
-        .slice(0, files.length - fileLimit)
+        .slice(0, files.length - limit)
       filesToDelete.foreach { f => Files.delete(f.toPath) }
       filesToDelete
     } else List()
@@ -38,4 +38,5 @@ class LimitedFilesManager(
 
 case class TimestampedFile(file: File, timestamp: Long) {
   def toPath: Path = file.toPath()
+  def name: String = file.getName()
 }
diff --git a/tests/unit/src/test/scala/tests/ReportsSuite.scala b/tests/unit/src/test/scala/tests/ReportsSuite.scala
@@ -7,7 +7,6 @@ import java.nio.file.Paths
 import scala.meta.internal.metals.FolderReportsZippper
 import scala.meta.internal.metals.Icons
 import scala.meta.internal.metals.Report
-import scala.meta.internal.metals.ReportFile
 import scala.meta.internal.metals.StdReportContext
 import scala.meta.internal.metals.ZipReportsProvider
 import scala.meta.io.AbsolutePath
@@ -25,7 +24,7 @@ class ReportsSuite extends BaseSuite {
     )
 
   def exampleText(workspaceStr: String = workspace.toString()): String =
-    s"""|An error happend in the file:
+    s"""|An error occurred in the file:
         |${workspaceStr}/WrongFile.scala
         |""".stripMargin
 
@@ -40,7 +39,7 @@ class ReportsSuite extends BaseSuite {
     val obtained =
       new String(Files.readAllBytes(path.get), StandardCharsets.UTF_8)
     assertNoDiff(exampleText(StdReportContext.WORKSPACE_STR), obtained)
-    assert(ReportFile.fromFile(path.get.toFile).nonEmpty)
+    assert(reportsProvider.incognito.getReports().length == 1)
   }
 
   test("delete-old-reports") {