Merge pull request #327 from codacy/fetch-outdated-tool-patterns-only

feature: Use patterns from local storage and API as fallback CY-2772

Author: josemiguelmelo

toolRepository-remote/src/main/scala/com/codacy/toolRepository/remote/ToolRepositoryRemote.scala

@@ -9,6 +9,7 @@ import com.codacy.analysis.core.model.{AnalyserError, ParameterSpec, PatternSpec
 import com.codacy.analysis.core.tools.ToolRepository
 import com.codacy.plugins.api.languages.{Language, Languages}
 import com.codacy.toolRepository.remote.storage.{PatternSpecDataStorage, ToolSpecDataStorage}
+import org.log4s.{Logger, getLogger}
 
 import scala.concurrent.duration._
 import scala.concurrent.{Await, ExecutionContext, Future}
@@ -18,6 +19,7 @@ class ToolRepositoryRemote(toolsClient: ToolsClient,
                            patternStorage: String => PatternSpecDataStorage)(implicit val ec: ExecutionContext,
                                                                              implicit val mat: Materializer)
     extends ToolRepository {
+  private val logger: Logger = getLogger
 
   private val toolStorageFilename = "tools"
   private val toolStorageInstance = toolsStorage(toolStorageFilename)
@@ -61,7 +63,7 @@ class ToolRepositoryRemote(toolsClient: ToolsClient,
       }
     }
 
-  override def listPatterns(tool: ToolSpec): Either[AnalyserError, Seq[PatternSpec]] = {
+  private def patternsFromApi(tool: ToolSpec): Either[AnalyserError, Seq[PatternSpec]] = {
     val source = PaginatedApiSourceFactory { cursor =>
       toolsClient.listPatterns(tool.uuid, cursor = cursor).value.map {
         case Right(ListPatternsResponse.OK(PatternListResponse(data, None | Some(PaginationInfo(None, _, _))))) =>
@@ -88,14 +90,33 @@ class ToolRepositoryRemote(toolsClient: ToolsClient,
         case e: Exception => Left(AnalyserError.FailedToListPatterns(tool.uuid, e.getMessage))
       }
 
-    val result = Await.result(patternsF, 1 minute)
-    val patternSpecDataStorage = patternStorage(tool.uuid)
-    result match {
-      case Right(patterns) =>
-        patternSpecDataStorage.save(patterns)
-        Right(patterns)
+    Await.result(patternsF, 1 minute)
+  }
+
+  private def downloadPatternsFromApi(
+    tool: ToolSpec,
+    toolPatternsStorageInstance: PatternSpecDataStorage): Either[AnalyserError, Seq[PatternSpec]] = {
+    patternsFromApi(tool) match {
+      case Right(patternsFromApi) =>
+        logger.info(s"Fetched patterns for ${tool.name} version ${tool.version}")
+        toolPatternsStorageInstance.save(patternsFromApi)
+        Right(patternsFromApi)
       case Left(err) =>
-        patternSpecDataStorage.get().toRight(err)
+        logger.error(s"Failed to fetch patterns for ${tool.name} from API: ${err.message}")
+        Left(err)
+    }
+  }
+
+  override def listPatterns(tool: ToolSpec): Either[AnalyserError, Seq[PatternSpec]] = {
+    val toolPatternsStorageFilename = s"${tool.uuid}-${tool.version}"
+    val toolPatternsStorageInstance = patternStorage(toolPatternsStorageFilename)
+
+    toolPatternsStorageInstance.get() match {
+      case Some(patternsFromStorage) =>
+        logger.info(s"Using patterns from cache for ${tool.name} version ${tool.version}")
+        Right(patternsFromStorage)
+      case None =>
+        downloadPatternsFromApi(tool, toolPatternsStorageInstance)
     }
   }
 

toolRepository-remote/src/test/scala/com/codacy/toolRespository/remote/ToolRepositoryRemoteSpec.scala

@@ -116,12 +116,20 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
     override def get(): Option[Seq[PatternSpec]] = Some(Seq(patternSpec(patternA.id)))
   }
 
+  def eitherListToolsResponse(
+    listToolsResponse: ListToolsResponse): EitherT[Future, Either[Throwable, HttpResponse], ListToolsResponse] = {
+    val responseEither: Either[Either[Throwable, HttpResponse], ListToolsResponse] = Right(listToolsResponse)
+    EitherT(Future.successful(responseEither))
+  }
+
+  def eitherListToolPatternsResponse(listToolPatternsResponse: ListPatternsResponse)
+    : EitherT[Future, Either[Throwable, HttpResponse], ListPatternsResponse] = {
+    val responseEither: Either[Either[Throwable, HttpResponse], ListPatternsResponse] =
+      Right(listToolPatternsResponse)
+    EitherT(Future.successful(responseEither))
+  }
+
   "list" should {
-    def eitherListToolsResponse(
-      listToolsResponse: ListToolsResponse): EitherT[Future, Either[Throwable, HttpResponse], ListToolsResponse] = {
-      val responseEither: Either[Either[Throwable, HttpResponse], ListToolsResponse] = Right(listToolsResponse)
-      EitherT(Future.successful(responseEither))
-    }
 
     "return the list of tools" in {
       val mockedClient = mock[ToolsClient]
@@ -203,17 +211,15 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
   }
 
   "listPatterns" should {
-    def eitherListToolPatternsResponse(listToolPatternsResponse: ListPatternsResponse)
-      : EitherT[Future, Either[Throwable, HttpResponse], ListPatternsResponse] = {
-      val responseEither: Either[Either[Throwable, HttpResponse], ListPatternsResponse] =
-        Right(listToolPatternsResponse)
-      EitherT(Future.successful(responseEither))
-    }
 
-    "return the list of patterns" in {
+    "return the list of patterns from API" in {
       val mockedClient = mock[ToolsClient]
       val toolRepository =
-        new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataWithStorage)
+        new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataEmptyStorage)
+
+      when(mockedClient.listTools(cursor = None)).thenReturn(
+        eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA), None))),
+        eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolB), None))))
 
       when(
         mockedClient.listPatterns(
@@ -232,13 +238,16 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
       patternsEither must beRight((p: Seq[PatternSpec]) => p.head.id must_== patternA.id)
     }
 
-    "return list with multiple patterns" in {
+    "return list with multiple patterns from API" in {
       val mockedClient = mock[ToolsClient]
       val toolRepository =
-        new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataWithStorage)
+        new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataEmptyStorage)
 
       val paginationInfo = PaginationInfo(Some("cursor"), Some(100), Some(1))
 
+      when(mockedClient.listTools(cursor = None))
+        .thenReturn(eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA), None))))
+
       when(
         mockedClient.listPatterns(
           toolId = ArgumentMatchers.any[String],
@@ -263,6 +272,9 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
       val toolRepository =
         new ToolRepositoryRemote(mockedClient, _ => mockToolsDataWithStorage, _ => mockPatternDataWithStorage)
 
+      when(mockedClient.listTools(cursor = None))
+        .thenReturn(eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA), None))))
+
       when(
         mockedClient.listPatterns(
           toolId = ArgumentMatchers.any[String],
@@ -278,11 +290,37 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
       patternsEither must beRight((p: Seq[PatternSpec]) => p.map(_.id) must contain(patternA.id))
     }
 
+    "return list of patterns first from storage" in {
+      val mockedClient = mock[ToolsClient]
+      val toolRepository =
+        new ToolRepositoryRemote(mockedClient, _ => mockToolsDataWithStorage, _ => mockPatternDataWithStorage)
+
+      when(mockedClient.listTools(cursor = None))
+        .thenReturn(eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA, toolB), None))))
+
+      when(
+        mockedClient.listPatterns(
+          toolId = ArgumentMatchers.any[String],
+          cursor = ArgumentMatchers.any[Option[String]],
+          limit = ArgumentMatchers.any[Option[Int]],
+          headers = ArgumentMatchers.any[List[HttpHeader]]))
+        .thenReturn(eitherListToolPatternsResponse(ListPatternsResponse.BadRequest(BadRequest("error"))))
+
+      val patternsEither = toolRepository.listPatterns(toolSpec(toolB.uuid))
+
+      patternsEither must beRight
+      patternsEither must beRight((p: Seq[PatternSpec]) => p must haveLength(1))
+      patternsEither must beRight((p: Seq[PatternSpec]) => p.map(_.id) must contain(patternA.id))
+    }
+
     "throw an exception if API returns BadRequest" in {
       val mockedClient = mock[ToolsClient]
       val toolRepository =
         new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataEmptyStorage)
 
+      when(mockedClient.listTools(cursor = None))
+        .thenReturn(eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA), None))))
+
       when(
         mockedClient.listPatterns(
           toolId = ArgumentMatchers.any[String],
@@ -300,6 +338,9 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
       val toolRepository =
         new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataEmptyStorage)
 
+      when(mockedClient.listTools(cursor = None))
+        .thenReturn(eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA), None))))
+
       when(
         mockedClient.listPatterns(
           toolId = ArgumentMatchers.any[String],
@@ -317,6 +358,9 @@ class ToolRepositoryRemoteSpec extends Specification with Mockito with EitherMat
       val toolRepository =
         new ToolRepositoryRemote(mockedClient, _ => mockToolsDataEmptyStorage, _ => mockPatternDataEmptyStorage)
 
+      when(mockedClient.listTools(cursor = None))
+        .thenReturn(eitherListToolsResponse(ListToolsResponse.OK(ToolListResponse(Vector(toolA), None))))
+
       when(
         mockedClient.listPatterns(
           toolId = ArgumentMatchers.any[String],