From 23d1183290a55a8f8c3bf39914b6c7dfa7a9d51a Mon Sep 17 00:00:00 2001 From: lcian <17258265+lcian@users.noreply.github.com> Date: Mon, 3 Aug 2026 11:30:03 +0200 Subject: [PATCH 1/3] fix(difs): Clone ProGuard DIFs through Objectstore Use the stored Objectstore copy when available so cloned DIFs follow the active double-write and exclusive-write modes. --- src/sentry/api/endpoints/debug_files.py | 4 +- src/sentry/models/debugfile.py | 28 ++++-- .../sentry/api/endpoints/test_dif_assemble.py | 87 +++++++++++++++++++ 3 files changed, 108 insertions(+), 11 deletions(-) diff --git a/src/sentry/api/endpoints/debug_files.py b/src/sentry/api/endpoints/debug_files.py index fb6e190fd102..063405dd7ac0 100644 --- a/src/sentry/api/endpoints/debug_files.py +++ b/src/sentry/api/endpoints/debug_files.py @@ -858,11 +858,11 @@ def _clone_proguard_debug_file_for_reupload( } meta = build_proguard_reupload_dif_meta(debug_file, requested_debug_id) - if not debug_file.uses_objectstore_for_read(): + if not debug_file.uses_objectstore_for_write(): assert debug_file.file is not None dif, created = create_dif_from_id(project, meta, file=debug_file.file) else: - source_fileobj = debug_file.get_file() + source_fileobj = debug_file.get_objectstore_file() try: # Spool into a temporary file to get a seekable stream. with tempfile.TemporaryFile() as tmp: diff --git a/src/sentry/models/debugfile.py b/src/sentry/models/debugfile.py index f786dcf2021e..2de1416413eb 100644 --- a/src/sentry/models/debugfile.py +++ b/src/sentry/models/debugfile.py @@ -211,6 +211,10 @@ def uses_objectstore_for_read(self) -> bool: organization = Project.objects.get_from_cache(id=self.project_id).organization return features.has("organizations:objectstore-debugfiles-read", organization) + def uses_objectstore_for_write(self) -> bool: + """Return whether this DIF has an Objectstore copy, regardless of the read rollout.""" + return self.storage_path is not None + def get_checksum(self) -> str: if self.uses_objectstore_for_read(): assert self.checksum is not None @@ -283,20 +287,26 @@ def get_file(self) -> IO[bytes]: """Returns the underlying contents as a file-like object. The caller is responsible for closing it.""" if self.uses_objectstore_for_read(): - assert self.storage_path is not None - try: - response = self._get_objectstore_session().get(self.storage_path) - if response is None: - raise FileNotFoundError("Debug file does not exist in objectstore") - return response.payload - except Exception: - logger.exception("Failed to read debug file from Objectstore") - raise + return self.get_objectstore_file() if self.file is not None: with measure_storage_operation("get", "debug_files", self.get_file_size()): return self.file.getfile() raise ValueError("ProjectDebugFile has neither file nor storage_path") + def get_objectstore_file(self) -> IO[bytes]: + """Return the Objectstore copy, even if Objectstore reads are not enabled.""" + if self.storage_path is None: + raise ValueError("debug file is not stored in Objectstore") + + try: + response = self._get_objectstore_session().get(self.storage_path) + if response is None: + raise FileNotFoundError("Debug file does not exist in objectstore") + return response.payload + except Exception: + logger.exception("Failed to read debug file from Objectstore") + raise + def get_objectstore_presigned_url(self, request: HttpRequest) -> str: """ Returns the URL that `request` should be redirected to in order to download this debug file diff --git a/tests/sentry/api/endpoints/test_dif_assemble.py b/tests/sentry/api/endpoints/test_dif_assemble.py index 1865d61a0085..75aebb239cc5 100644 --- a/tests/sentry/api/endpoints/test_dif_assemble.py +++ b/tests/sentry/api/endpoints/test_dif_assemble.py @@ -437,6 +437,59 @@ def test_reuses_existing_proguard_file_with_new_debug_id(self) -> None: assert first_dif.file_id == second_dif.file_id assert File.objects.filter(type="project.dif", checksum=checksum).count() == 1 + def test_objectstore_assemble_reuses_existing_proguard_without_file(self) -> None: + file_contents = b"proguard mapping" + checksum = sha1(file_contents).hexdigest() + get_chunk_upload_session(self.organization.id).put(file_contents, key=checksum) + chunks = [checksum] + + with self.feature( + { + "organizations:objectstore-debugfiles-exclusive-write": True, + "organizations:objectstore-debugfiles-write": False, + "organizations:objectstore-debugfiles-read": True, + } + ): + assemble_dif( + project_id=self.project.id, + name="/proguard/mapping-00000000-0000-0000-0000-000000000000.txt", + checksum=checksum, + chunks=chunks, + use_objectstore=True, + ) + + first_dif = ProjectDebugFile.objects.get( + project_id=self.project.id, + debug_id="00000000-0000-0000-0000-000000000000", + ) + + response = self.client.post( + self.url, + data={ + checksum: { + "name": "/proguard/mapping-11111111-1111-1111-1111-111111111111.txt", + "chunks": chunks, + } + }, + HTTP_AUTHORIZATION=f"Bearer {self.token.token}", + ) + + assert response.status_code == 200, response.content + assert response.data[checksum]["state"] == ChunkFileState.OK + assert response.data[checksum]["dif"]["uuid"] == "11111111-1111-1111-1111-111111111111" + + second_dif = ProjectDebugFile.objects.get( + project_id=self.project.id, + debug_id="11111111-1111-1111-1111-111111111111", + ) + + assert first_dif.file_id is None + assert second_dif.file_id is None + assert first_dif.storage_path is not None + assert second_dif.storage_path is not None + assert first_dif.storage_path != second_dif.storage_path + assert File.objects.filter(type="project.dif", checksum=checksum).count() == 0 + def test_reupload_proguard_with_same_debug_id_is_idempotent(self) -> None: file_contents = b"proguard mapping" checksum = sha1(file_contents).hexdigest() @@ -590,6 +643,8 @@ def test_clone_dual_written_source_to_file(self) -> None: ) assert first_dif.file_id is not None assert first_dif.storage_path is not None + assert first_dif.uses_objectstore_for_write() + assert not first_dif.uses_objectstore_for_read() with self.feature({"organizations:objectstore-debugfiles-write": False}): response = self._clone_request(checksum, chunks) @@ -604,5 +659,37 @@ def test_clone_dual_written_source_to_file(self) -> None: ) # The source stays Objectstore-backed; the clone is written as a File. assert second_dif.file_id is not None + assert second_dif.file_id != first_dif.file_id assert second_dif.storage_path is None assert second_dif.get_file().read() == file_contents + + def test_clone_file_backed_source_to_objectstore_exclusive(self) -> None: + file_contents = b"proguard mapping" + checksum = sha1(file_contents).hexdigest() + blob = FileBlob.from_file_with_organization(ContentFile(file_contents), self.organization) + chunks = [blob.checksum] + + with self.feature({"organizations:objectstore-debugfiles-write": False}): + self._assemble_source(checksum, chunks) + + first_dif = ProjectDebugFile.objects.get( + project_id=self.project.id, + debug_id="00000000-0000-0000-0000-000000000000", + ) + assert first_dif.file_id is not None + assert first_dif.storage_path is None + + with self.feature("organizations:objectstore-debugfiles-exclusive-write"): + response = self._clone_request(checksum, chunks) + + assert response.status_code == 200, response.content + assert response.data[checksum]["state"] == ChunkFileState.OK + + second_dif = ProjectDebugFile.objects.get( + project_id=self.project.id, + debug_id="11111111-1111-1111-1111-111111111111", + ) + assert second_dif.file_id is None + assert second_dif.storage_path is not None + assert second_dif.get_file().read() == file_contents + assert File.objects.filter(type="project.dif", checksum=checksum).count() == 1 From feded77613e998ca590361bb8c9a2f3cd8532951 Mon Sep 17 00:00:00 2001 From: lcian <17258265+lcian@users.noreply.github.com> Date: Mon, 3 Aug 2026 11:56:48 +0200 Subject: [PATCH 2/3] test(difs): Clarify Objectstore ProGuard clone coverage --- tests/sentry/api/endpoints/test_dif_assemble.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/api/endpoints/test_dif_assemble.py b/tests/sentry/api/endpoints/test_dif_assemble.py index 75aebb239cc5..163f3874925f 100644 --- a/tests/sentry/api/endpoints/test_dif_assemble.py +++ b/tests/sentry/api/endpoints/test_dif_assemble.py @@ -437,7 +437,7 @@ def test_reuses_existing_proguard_file_with_new_debug_id(self) -> None: assert first_dif.file_id == second_dif.file_id assert File.objects.filter(type="project.dif", checksum=checksum).count() == 1 - def test_objectstore_assemble_reuses_existing_proguard_without_file(self) -> None: + def test_objectstore_assemble_clones_existing_proguard_to_new_objectstore_file(self) -> None: file_contents = b"proguard mapping" checksum = sha1(file_contents).hexdigest() get_chunk_upload_session(self.organization.id).put(file_contents, key=checksum) From 255125e5575beb70f061660376db95c2d0c8a8b4 Mon Sep 17 00:00:00 2001 From: lcian <17258265+lcian@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:07:15 +0200 Subject: [PATCH 3/3] ref(difs): Inline ProGuard clone Objectstore access --- src/sentry/api/endpoints/debug_files.py | 8 ++++-- src/sentry/models/debugfile.py | 28 ++++++------------- .../sentry/api/endpoints/test_dif_assemble.py | 1 - 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/sentry/api/endpoints/debug_files.py b/src/sentry/api/endpoints/debug_files.py index 063405dd7ac0..4d9f736bdce7 100644 --- a/src/sentry/api/endpoints/debug_files.py +++ b/src/sentry/api/endpoints/debug_files.py @@ -858,11 +858,15 @@ def _clone_proguard_debug_file_for_reupload( } meta = build_proguard_reupload_dif_meta(debug_file, requested_debug_id) - if not debug_file.uses_objectstore_for_write(): + if debug_file.storage_path is None: assert debug_file.file is not None dif, created = create_dif_from_id(project, meta, file=debug_file.file) else: - source_fileobj = debug_file.get_objectstore_file() + response = debug_file._get_objectstore_session().get(debug_file.storage_path) + if response is None: + raise FileNotFoundError("Debug file does not exist in Objectstore") + + source_fileobj = response.payload try: # Spool into a temporary file to get a seekable stream. with tempfile.TemporaryFile() as tmp: diff --git a/src/sentry/models/debugfile.py b/src/sentry/models/debugfile.py index 2de1416413eb..f786dcf2021e 100644 --- a/src/sentry/models/debugfile.py +++ b/src/sentry/models/debugfile.py @@ -211,10 +211,6 @@ def uses_objectstore_for_read(self) -> bool: organization = Project.objects.get_from_cache(id=self.project_id).organization return features.has("organizations:objectstore-debugfiles-read", organization) - def uses_objectstore_for_write(self) -> bool: - """Return whether this DIF has an Objectstore copy, regardless of the read rollout.""" - return self.storage_path is not None - def get_checksum(self) -> str: if self.uses_objectstore_for_read(): assert self.checksum is not None @@ -287,26 +283,20 @@ def get_file(self) -> IO[bytes]: """Returns the underlying contents as a file-like object. The caller is responsible for closing it.""" if self.uses_objectstore_for_read(): - return self.get_objectstore_file() + assert self.storage_path is not None + try: + response = self._get_objectstore_session().get(self.storage_path) + if response is None: + raise FileNotFoundError("Debug file does not exist in objectstore") + return response.payload + except Exception: + logger.exception("Failed to read debug file from Objectstore") + raise if self.file is not None: with measure_storage_operation("get", "debug_files", self.get_file_size()): return self.file.getfile() raise ValueError("ProjectDebugFile has neither file nor storage_path") - def get_objectstore_file(self) -> IO[bytes]: - """Return the Objectstore copy, even if Objectstore reads are not enabled.""" - if self.storage_path is None: - raise ValueError("debug file is not stored in Objectstore") - - try: - response = self._get_objectstore_session().get(self.storage_path) - if response is None: - raise FileNotFoundError("Debug file does not exist in objectstore") - return response.payload - except Exception: - logger.exception("Failed to read debug file from Objectstore") - raise - def get_objectstore_presigned_url(self, request: HttpRequest) -> str: """ Returns the URL that `request` should be redirected to in order to download this debug file diff --git a/tests/sentry/api/endpoints/test_dif_assemble.py b/tests/sentry/api/endpoints/test_dif_assemble.py index 163f3874925f..a1cd7c4a214d 100644 --- a/tests/sentry/api/endpoints/test_dif_assemble.py +++ b/tests/sentry/api/endpoints/test_dif_assemble.py @@ -643,7 +643,6 @@ def test_clone_dual_written_source_to_file(self) -> None: ) assert first_dif.file_id is not None assert first_dif.storage_path is not None - assert first_dif.uses_objectstore_for_write() assert not first_dif.uses_objectstore_for_read() with self.feature({"organizations:objectstore-debugfiles-write": False}):