From 3342ff17f1237181a36af235e67757df652b680c Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 4 Jul 2023 10:00:10 +0200 Subject: [PATCH] refactor: use `LimitedFilesManager` for reports --- .../meta/internal/metals/LoggerContext.scala | 6 ++- .../meta/internal/metals/ReportContext.scala | 53 ++++++------------- .../metals/utils/LimitedFilesManager.scala | 9 ++-- .../src/test/scala/tests/ReportsSuite.scala | 5 +- 4 files changed, 26 insertions(+), 47 deletions(-) rename {metals => mtags-shared}/src/main/scala/scala/meta/internal/metals/utils/LimitedFilesManager.scala (82%) diff --git a/metals/src/main/scala/scala/meta/internal/metals/LoggerContext.scala b/metals/src/main/scala/scala/meta/internal/metals/LoggerContext.scala index 4ce49912289..20dee8c73a2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/LoggerContext.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/LoggerContext.scala @@ -2,6 +2,8 @@ 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] = { @@ -9,10 +11,10 @@ object LoggerReporter extends Reporter { 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 index c483720c843..fe9d7b98e2d 100644 --- 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/metals/src/main/scala/scala/meta/internal/metals/utils/LimitedFilesManager.scala b/mtags-shared/src/main/scala/scala/meta/internal/metals/utils/LimitedFilesManager.scala similarity index 82% rename from metals/src/main/scala/scala/meta/internal/metals/utils/LimitedFilesManager.scala rename to mtags-shared/src/main/scala/scala/meta/internal/metals/utils/LimitedFilesManager.scala index 6e15069c056..89b26bce10e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/utils/LimitedFilesManager.scala +++ b/mtags-shared/src/main/scala/scala/meta/internal/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 index 94b60d3a295..4f83ba5e63c 100644 --- 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") {