feat(difs): Add Objectstore columns to ProjectDebugFile

[lcian]

Adds new columns to house Objectstore-backed difs.
These will lose the file FK and gain the storage_path field.
In addition, we also need to add content_type, file_size and date_created, as those were previously fields of File.
Naturally, during the migration period, both of these need to be nullable, so we need to assert the invariants we know to be true at runtime.

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1115_projectdebugfile_add_objectstore_columns.py

for 1115_projectdebugfile_add_objectstore_columns in sentry

--
-- Alter field file on projectdebugfile
--
SET CONSTRAINTS "sentry_projectdsymfile_file_id_bdf41a41_fk_sentry_file_id" IMMEDIATE; ALTER TABLE "sentry_projectdsymfile" DROP CONSTRAINT "sentry_projectdsymfile_file_id_bdf41a41_fk_sentry_file_id";
ALTER TABLE "sentry_projectdsymfile" ALTER COLUMN "file_id" DROP NOT NULL;
ALTER TABLE "sentry_projectdsymfile" ADD CONSTRAINT "sentry_projectdsymfile_file_id_bdf41a41_fk_sentry_file_id" FOREIGN KEY ("file_id") REFERENCES "sentry_file" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_projectdsymfile" VALIDATE CONSTRAINT "sentry_projectdsymfile_file_id_bdf41a41_fk_sentry_file_id";
--
-- Add field storage_path to projectdebugfile
--
ALTER TABLE "sentry_projectdsymfile" ADD COLUMN "storage_path" text NULL;
--
-- Add field content_type to projectdebugfile
--
ALTER TABLE "sentry_projectdsymfile" ADD COLUMN "content_type" text NULL;
--
-- Add field file_size to projectdebugfile
--
ALTER TABLE "sentry_projectdsymfile" ADD COLUMN "file_size" bigint NULL;
--
-- Add field date_created to projectdebugfile
--
ALTER TABLE "sentry_projectdsymfile" ADD COLUMN "date_created" timestamp with time zone NULL;

[sentry]

Bug: Serializing a DebugFile with a None file attribute (for Objectstore-backed DIFs) raises an AssertionError or AttributeError because the serializer does not handle the null case.
_{Severity: CRITICAL}

Suggested Fix

Update DebugFileSerializer to handle cases where obj.file is None. Use obj.content_type and other direct attributes from the DebugFile model for Objectstore-backed DIFs instead of accessing them through the now-nullable file relation. Remove the assert self.file is not None from the file_format property and add logic to handle the None case.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/models/debugfile.py#L156-L162

Potential issue: The `DebugFileSerializer` attempts to serialize debug information files
(DIFs) by accessing attributes of `obj.file`, such as `obj.file.headers` and
`obj.file.size`. It also calls the `obj.file_format` property. However, for DIFs backed
by Objectstore, the `file` attribute is `None`. The `file_format` property contains an
`assert self.file is not None`, which will raise an `AssertionError`. Direct access to
`obj.file` attributes will raise an `AttributeError`. This causes any API endpoint that
serializes these DIFs, such as the `DebugFilesEndpoint`, to crash.

Also affects:

• src/sentry/models/debugfile.py:130~136

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[sentry]

Bug: The clean_redundant_difs function unconditionally asserts dif.file is not None, which will fail and crash uploads if any existing DIF for the same debug_id is Objectstore-backed.
_{Severity: HIGH}

Suggested Fix

Modify clean_redundant_difs to correctly handle ProjectDebugFile instances where file is None. Instead of asserting dif.file is not None to get the content type, use dif.content_type which is available on the model directly and supports both file-backed and Objectstore-backed DIFs.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/models/debugfile.py#L237-L243

Potential issue: The function `clean_redundant_difs` is called after a new debug
information file (DIF) is uploaded. It queries for all existing `ProjectDebugFile`
records with the same `debug_id` and iterates through them. Inside the loop, it
unconditionally runs `assert dif.file is not None`. If a pre-existing DIF for that
`debug_id` is backed by Objectstore, its `file` attribute will be `None`, causing the
assertion to fail. This will crash the upload process for any new DIF that shares a
`debug_id` with an Objectstore-backed DIF.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[loewenheim]

Is the idea behind making the file field nullable in the model and adding assertions that this way we save ourselves a migration? In principle, it would work just as well to only add the new columns and do the nullability separately, right?

[lcian]

Yeah @loewenheim, we could very well split this into 2 separate migrations, would it be preferable to do it that way?
It's my first time creating a migration, so I don't really know what factors should be considered.

The intention was not really to spare a migration, but to make the File nullable while at the same time introducing the alternative, as if this were an enum you add a variant to.
A ProjectDebugFile with a NULL File doesn't make too much sense to me, unless an alternative way to point to a file exists.

[loewenheim]

If having two separate migrations isn't a concern, I would prefer it if the file field was made nullable at the point where it's actually possible for a debug file to have null there.

[lcian]

Yeah that makes sense, I'll change it.