ref(auto_config): Use factory to create the right instance

src/sentry/api/endpoints/project_repo_path_parsing.py

@@ -15,7 +15,7 @@
 from sentry.integrations.services.integration import RpcIntegration, integration_service
 from sentry.integrations.source_code_management.repository import RepositoryIntegration
 from sentry.issues.auto_source_code_config.code_mapping import find_roots
-from sentry.issues.auto_source_code_config.frame_info import FrameInfo
+from sentry.issues.auto_source_code_config.frame_info import FrameInfo, create_frame_info
 from sentry.models.project import Project
 from sentry.models.repository import Repository
 
@@ -156,4 +156,4 @@ def get_frame_info_from_request(request: Request) -> FrameInfo:
         "filename": request.data["stackPath"],
         "module": request.data.get("module"),
     }
-    return FrameInfo(frame, request.data.get("platform"))
+    return create_frame_info(frame, request.data.get("platform"))

src/sentry/issues/auto_source_code_config/code_mapping.py

@@ -25,7 +25,7 @@
     UnexpectedPathException,
     UnsupportedFrameInfo,
 )
-from .frame_info import FrameInfo
+from .frame_info import FrameInfo, create_frame_info
 from .integration_utils import InstallationNotFoundError, get_installation
 from .utils.misc import get_straight_path_prefix_end_index
 
@@ -53,7 +53,7 @@ def derive_code_mappings(
     trees = installation.get_trees_for_org()
     trees_helper = CodeMappingTreesHelper(trees)
     try:
-        frame_filename = FrameInfo(frame, platform)
+        frame_filename = create_frame_info(frame, platform)
         return trees_helper.get_file_and_repo_matches(frame_filename)
     except NeedsExtension:
         logger.warning("Needs extension: %s", frame.get("filename"))
@@ -141,7 +141,7 @@ def _stacktrace_buckets(
         buckets: defaultdict[str, list[FrameInfo]] = defaultdict(list)
         for frame in frames:
             try:
-                frame_filename = FrameInfo(frame, self.platform)
+                frame_filename = create_frame_info(frame, self.platform)
                 # Any files without a top directory will be grouped together
                 buckets[frame_filename.stack_root].append(frame_filename)
             except UnsupportedFrameInfo:

src/sentry/issues/auto_source_code_config/derived_code_mappings_endpoint.py

@@ -9,7 +9,7 @@
 from sentry.models.organization import Organization
 
 from .code_mapping import CodeMapping, CodeMappingTreesHelper
-from .frame_info import FrameInfo
+from .frame_info import FrameInfo, create_frame_info
 from .integration_utils import get_installation
 
 logger = logging.getLogger(__name__)
@@ -31,7 +31,7 @@ def get_frame_info_from_request(request: Request) -> FrameInfo:
         "filename": request.GET["stacktraceFilename"],
         "module": request.GET.get("module"),
     }
-    return FrameInfo(frame, request.GET.get("platform"))
+    return create_frame_info(frame, request.GET.get("platform"))
 
 
 def get_code_mapping_from_request(request: Request) -> CodeMapping:

src/sentry/issues/auto_source_code_config/frame_info.py

@@ -1,4 +1,7 @@
+from __future__ import annotations
+
 import re
+from abc import ABC, abstractmethod
 from collections.abc import Mapping
 from typing import Any
 
@@ -20,14 +23,53 @@
 UNSUPPORTED_NORMALIZED_PATH_PATTERN = re.compile(r"^[^/]*$")
 
 
-class FrameInfo:
-    def __init__(self, frame: Mapping[str, Any], platform: str | None = None) -> None:
-        if platform:
-            platform_config = PlatformConfig(platform)
-            if platform_config.extracts_filename_from_module():
-                self.frame_info_from_module(frame)
-                return
+def create_frame_info(frame: Mapping[str, Any], platform: str | None = None) -> FrameInfo:
+    """Factory function to create the appropriate FrameInfo instance."""
+    frame_info: FrameInfo | None = None
+    if platform:
+        platform_config = PlatformConfig(platform)
+        if platform_config.extracts_filename_from_module():
+            frame_info = ModuleBasedFrameInfo()
+            frame_info.process_frame(frame)
+            return frame_info
+
+    frame_info = PathBasedFrameInfo()
+    frame_info.process_frame(frame)
+    return frame_info
+
+
+class FrameInfo(ABC):
+    raw_path: str
+    normalized_path: str
+    stack_root: str
+
+    def __repr__(self) -> str:
+        return f"FrameInfo: {self.raw_path}"
+
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, FrameInfo):
+            return False
+        return self.raw_path == other.raw_path
+
+    @abstractmethod
+    def process_frame(self, frame: Mapping[str, Any]) -> None:
+        """Process the frame and set the necessary attributes."""
+        raise NotImplementedError("Subclasses must implement process_frame")
+
 
+class ModuleBasedFrameInfo(FrameInfo):
+    def process_frame(self, frame: Mapping[str, Any]) -> None:
+        if frame.get("module") and frame.get("abs_path"):
+            stack_root, filepath = get_path_from_module(frame["module"], frame["abs_path"])
+            self.stack_root = stack_root
+            self.raw_path = filepath
+            self.normalized_path = filepath
+        else:
+            raise MissingModuleOrAbsPath("Investigate why the data is missing.")
+
+
+class PathBasedFrameInfo(FrameInfo):
+    def process_frame(self, frame: Mapping[str, Any]) -> None:
         frame_file_path = frame["filename"]
         frame_file_path = self.transformations(frame_file_path)
 
@@ -69,23 +111,6 @@ def transformations(self, frame_file_path: str) -> str:
 
         return frame_file_path
 
-    def frame_info_from_module(self, frame: Mapping[str, Any]) -> None:
-        if frame.get("module") and frame.get("abs_path"):
-            stack_root, filepath = get_path_from_module(frame["module"], frame["abs_path"])
-            self.stack_root = stack_root
-            self.raw_path = filepath
-            self.normalized_path = filepath
-        else:
-            raise MissingModuleOrAbsPath("Investigate why the data is missing.")
-
-    def __repr__(self) -> str:
-        return f"FrameInfo: {self.raw_path}"
-
-    def __eq__(self, other: object) -> bool:
-        if not isinstance(other, FrameInfo):
-            return False
-        return self.raw_path == other.raw_path
-
 
 PREFIXES_TO_REMOVE = ["app:///", "./", "../", "/"]
 

tests/sentry/issues/auto_source_code_config/test_code_mapping.py

@@ -22,7 +22,7 @@
     UnexpectedPathException,
     UnsupportedFrameInfo,
 )
-from sentry.issues.auto_source_code_config.frame_info import FrameInfo
+from sentry.issues.auto_source_code_config.frame_info import create_frame_info
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import assume_test_silo_mode
@@ -86,10 +86,10 @@ def test_buckets_logic() -> None:
     helper = CodeMappingTreesHelper({})
     buckets = helper._stacktrace_buckets(frames)
     assert buckets == {
-        "/cronscripts/": [FrameInfo({"filename": "/cronscripts/monitoringsync.php"})],
-        "./app": [FrameInfo({"filename": "./app/utils/handleXhrErrorResponse.tsx"})],
-        "app:": [FrameInfo({"filename": "app://foo.js"})],
-        "getsentry": [FrameInfo({"filename": "getsentry/billing/tax/manager.py"})],
+        "/cronscripts/": [create_frame_info({"filename": "/cronscripts/monitoringsync.php"})],
+        "./app": [create_frame_info({"filename": "./app/utils/handleXhrErrorResponse.tsx"})],
+        "app:": [create_frame_info({"filename": "app://foo.js"})],
+        "getsentry": [create_frame_info({"filename": "getsentry/billing/tax/manager.py"})],
     }
 
 
@@ -123,7 +123,7 @@ def test_package_also_matches(self) -> None:
         # We create a new tree helper in order to improve the understability of this test
         cmh = CodeMappingTreesHelper({self.foo_repo.name: repo_tree})
         cm = cmh._generate_code_mapping_from_tree(
-            repo_tree=repo_tree, frame_filename=FrameInfo({"filename": "raven/base.py"})
+            repo_tree=repo_tree, frame_filename=create_frame_info({"filename": "raven/base.py"})
         )
         # We should not derive a code mapping since the package name does not match
         assert cm == []
@@ -205,7 +205,7 @@ def test_more_than_one_repo_match(self, logger: Any) -> None:
         logger.warning.assert_called_with("More than one repo matched %s", "sentry/web/urls.py")
 
     def test_get_file_and_repo_matches_single(self) -> None:
-        frame_filename = FrameInfo({"filename": "sentry_plugins/slack/client.py"})
+        frame_filename = create_frame_info({"filename": "sentry_plugins/slack/client.py"})
         matches = self.code_mapping_helper.get_file_and_repo_matches(frame_filename)
         expected_matches = [
             {
@@ -219,7 +219,7 @@ def test_get_file_and_repo_matches_single(self) -> None:
         assert matches == expected_matches
 
     def test_get_file_and_repo_matches_multiple(self) -> None:
-        frame_filename = FrameInfo({"filename": "sentry/web/urls.py"})
+        frame_filename = create_frame_info({"filename": "sentry/web/urls.py"})
         matches = self.code_mapping_helper.get_file_and_repo_matches(frame_filename)
         expected_matches = [
             {
@@ -241,91 +241,93 @@ def test_get_file_and_repo_matches_multiple(self) -> None:
 
     def test_find_roots_starts_with_period_slash(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "./app/foo.tsx"}), "static/app/foo.tsx"
+            create_frame_info({"filename": "./app/foo.tsx"}), "static/app/foo.tsx"
         )
         assert stacktrace_root == "./"
         assert source_path == "static/"
 
     def test_find_roots_starts_with_period_slash_no_containing_directory(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "./app/foo.tsx"}), "app/foo.tsx"
+            create_frame_info({"filename": "./app/foo.tsx"}), "app/foo.tsx"
         )
         assert stacktrace_root == "./"
         assert source_path == ""
 
     def test_find_roots_not_matching(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "sentry/foo.py"}), "src/sentry/foo.py"
+            create_frame_info({"filename": "sentry/foo.py"}), "src/sentry/foo.py"
         )
         assert stacktrace_root == "sentry/"
         assert source_path == "src/sentry/"
 
     def test_find_roots_equal(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "source/foo.py"}), "source/foo.py"
+            create_frame_info({"filename": "source/foo.py"}), "source/foo.py"
         )
         assert stacktrace_root == ""
         assert source_path == ""
 
     def test_find_roots_starts_with_period_slash_two_levels(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "./app/foo.tsx"}), "app/foo/app/foo.tsx"
+            create_frame_info({"filename": "./app/foo.tsx"}), "app/foo/app/foo.tsx"
         )
         assert stacktrace_root == "./"
         assert source_path == "app/foo/"
 
     def test_find_roots_starts_with_app(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "app:///utils/foo.tsx"}), "utils/foo.tsx"
+            create_frame_info({"filename": "app:///utils/foo.tsx"}), "utils/foo.tsx"
         )
         assert stacktrace_root == "app:///"
         assert source_path == ""
 
     def test_find_roots_starts_with_multiple_dot_dot_slash(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "../../../../../../packages/foo.tsx"}),
+            create_frame_info({"filename": "../../../../../../packages/foo.tsx"}),
             "packages/foo.tsx",
         )
         assert stacktrace_root == "../../../../../../"
         assert source_path == ""
 
     def test_find_roots_starts_with_app_dot_dot_slash(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "app:///../services/foo.tsx"}),
+            create_frame_info({"filename": "app:///../services/foo.tsx"}),
             "services/foo.tsx",
         )
         assert stacktrace_root == "app:///../"
         assert source_path == ""
 
     def test_find_roots_bad_stack_path(self) -> None:
         with pytest.raises(UnsupportedFrameInfo):
-            FrameInfo({"filename": "https://yrurlsinyourstackpath.com/"})
+            create_frame_info({"filename": "https://yrurlsinyourstackpath.com/"})
 
     def test_find_roots_bad_source_path(self) -> None:
         with pytest.raises(UnexpectedPathException):
             find_roots(
-                FrameInfo({"filename": "sentry/random.py"}),
+                create_frame_info({"filename": "sentry/random.py"}),
                 "nothing/something.js",
             )
 
     def test_find_roots_windows_path_with_spaces(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "C:\\Program Files\\MyApp\\src\\file.py"}), "src/file.py"
+            create_frame_info({"filename": "C:\\Program Files\\MyApp\\src\\file.py"}), "src/file.py"
         )
         assert stacktrace_root == "C:\\Program Files\\MyApp\\"
         assert source_path == ""
 
     def test_find_roots_windows_path_with_spaces_nested(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "C:\\Program Files\\My Company\\My App\\src\\main\\file.py"}),
+            create_frame_info(
+                {"filename": "C:\\Program Files\\My Company\\My App\\src\\main\\file.py"}
+            ),
             "src/main/file.py",
         )
         assert stacktrace_root == "C:\\Program Files\\My Company\\My App\\"
         assert source_path == ""
 
     def test_find_roots_windows_path_with_spaces_source_match(self) -> None:
         stacktrace_root, source_path = find_roots(
-            FrameInfo({"filename": "C:\\Program Files\\MyApp\\src\\components\\file.py"}),
+            create_frame_info({"filename": "C:\\Program Files\\MyApp\\src\\components\\file.py"}),
             "frontend/src/components/file.py",
         )
         assert stacktrace_root == "C:\\Program Files\\MyApp\\"

tests/sentry/issues/auto_source_code_config/test_frame_info.py

@@ -8,7 +8,7 @@
     NeedsExtension,
     UnsupportedFrameInfo,
 )
-from sentry.issues.auto_source_code_config.frame_info import FrameInfo
+from sentry.issues.auto_source_code_config.frame_info import create_frame_info
 
 UNSUPPORTED_FRAME_FILENAMES = [
     # HTTP/HTTPS URLs
@@ -49,23 +49,23 @@
 class TestFrameInfo:
     def test_frame_filename_repr(self) -> None:
         path = "getsentry/billing/tax/manager.py"
-        assert FrameInfo({"filename": path}).__repr__() == f"FrameInfo: {path}"
+        assert create_frame_info({"filename": path}).__repr__() == f"FrameInfo: {path}"
 
     @pytest.mark.parametrize("filepath", UNSUPPORTED_FRAME_FILENAMES)
     def test_raises_unsupported(self, filepath: str) -> None:
         with pytest.raises(UnsupportedFrameInfo):
-            FrameInfo({"filename": filepath})
+            create_frame_info({"filename": filepath})
 
     @pytest.mark.parametrize("filepath", LEGITIMATE_HTTP_FILENAMES)
     def test_legitimate_http_filenames_accepted(self, filepath: str) -> None:
         # These files contain "http" but should NOT be rejected
-        frame_info = FrameInfo({"filename": filepath})
+        frame_info = create_frame_info({"filename": filepath})
         assert frame_info.raw_path == filepath
 
     def test_raises_no_extension(self) -> None:
         for filepath in NO_EXTENSION_FRAME_FILENAMES:
             with pytest.raises(NeedsExtension):
-                FrameInfo({"filename": filepath})
+                create_frame_info({"filename": filepath})
 
     @pytest.mark.parametrize(
         "frame, expected_exception",
@@ -86,7 +86,7 @@ def test_java_raises_exception(
         self, frame: dict[str, Any], expected_exception: type[Exception]
     ) -> None:
         with pytest.raises(expected_exception):
-            FrameInfo(frame, "java")
+            create_frame_info(frame, "java")
 
     @pytest.mark.parametrize(
         "frame, expected_stack_root, expected_normalized_path",
@@ -120,7 +120,7 @@ def test_java_raises_exception(
     def test_java_valid_frames(
         self, frame: dict[str, Any], expected_stack_root: str, expected_normalized_path: str
     ) -> None:
-        frame_info = FrameInfo(frame, "java")
+        frame_info = create_frame_info(frame, "java")
         assert frame_info.stack_root == expected_stack_root
         assert frame_info.normalized_path == expected_normalized_path
 
@@ -157,6 +157,6 @@ def test_java_valid_frames(
     def test_straight_path_prefix(
         self, frame_filename: str, stack_root: str, normalized_path: str
     ) -> None:
-        frame_info = FrameInfo({"filename": frame_filename})
+        frame_info = create_frame_info({"filename": frame_filename})
         assert frame_info.normalized_path == normalized_path
         assert frame_info.stack_root == stack_root