Move get_file_content logic to revision scope

Author: vrigal

bot/code_review_bot/__init__.py

@@ -198,34 +198,9 @@ def hash(self):
         #  format `obj-x86_64-pc-linux-gnu`
         file_content = None
         if "/obj-" not in self.path:
-            if settings.mercurial_cache_checkout:
-                logger.debug("Using the local repository to build issue's hash")
-                try:
-                    with (settings.mercurial_cache_checkout / self.path).open() as f:
-                        file_content = f.read()
-                except (FileNotFoundError, IsADirectoryError):
-                    logger.warning(
-                        "Failed to find issue's related file", path=self.path
-                    )
-                    file_content = None
-            else:
-                try:
-                    # Load all the lines affected by the issue
-                    file_content = self.revision.load_file(self.path)
-                except ValueError:
-                    # Build the hash with an empty content in case the path is erroneous
-                    file_content = None
-                except requests.exceptions.HTTPError as e:
-                    if e.response.status_code == 404:
-                        logger.warning(
-                            "Failed to download a file with an issue", path=self.path
-                        )
-
-                        # We still build the hash with empty content
-                        file_content = None
-                    else:
-                        # When encountering another HTTP error, raise the issue
-                        raise
+            file_content = self.revision.get_file_content(
+                self.path, settings.mercurial_cache_checkout
+            )
 
         if file_content is None:
             self._hash = None

bot/code_review_bot/revisions/base.py

@@ -6,6 +6,7 @@
 import random
 from abc import ABC
 from datetime import timedelta
+from pathlib import Path
 
 import rs_parsepatch
 import structlog
@@ -165,6 +166,25 @@ def contains(self, issue):
         lines = set(range(issue.line, issue.line + issue.nb_lines))
         return not lines.isdisjoint(modified_lines)
 
+    def get_file_content(
+        self, file_path: str, local_cache_repository: Path | None = None
+    ):
+        if local_cache_repository:
+            logger.debug("Using the local repository to build issue's hash")
+            try:
+                with (local_cache_repository / file_path).open() as f:
+                    file_content = f.read()
+            except (FileNotFoundError, IsADirectoryError):
+                logger.warning("Failed to find issue's related file", path=file_path)
+                file_content = None
+        else:
+            try:
+                file_content = self.load_file(file_path)
+            except ValueError:
+                # The path is erroneous, consider as empty content
+                file_content = None
+        return file_content
+
     @property
     def has_clang_files(self):
         """

bot/code_review_bot/revisions/phabricator.py

@@ -362,8 +362,17 @@ def load_file(self, path):
         )
         logger.info("Downloading HGMO file", url=url)
 
-        response = requests.get(url, headers=GetAppUserAgent())
-        response.raise_for_status()
+        try:
+            response = requests.get(url, headers=GetAppUserAgent())
+            response.raise_for_status()
+        except requests.exceptions.HTTPError as e:
+            if e.response.status_code == 404:
+                logger.warning("Failed to download file", path=self.path)
+                # Consider as empty content if the file is not found
+                return None
+            else:
+                # When encountering another HTTP error, raise the issue
+                raise e
 
         # Store in cache
         content = response.content.decode("utf-8")