Skip to content

Commit

Permalink
Consider contents when choosing between v1/legacy ReportParser
Browse files Browse the repository at this point in the history
  • Loading branch information
Swatinem committed Aug 16, 2024
1 parent 201323d commit afdef97
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
3 changes: 1 addition & 2 deletions services/report/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,8 +917,7 @@ def parse_raw_report_from_storage(
else:
archive_file = archive_service.read_file(archive_url)

parser = get_proper_parser(upload)

parser = get_proper_parser(upload, archive_file)
upload_version = (
"v1" if isinstance(parser, VersionOneReportParser) else "legacy"
)
Expand Down
13 changes: 11 additions & 2 deletions services/report/parser/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
import sentry_sdk

from database.models.reports import Upload
from services.report.parser.legacy import LegacyReportParser
from services.report.parser.version_one import VersionOneReportParser


def get_proper_parser(upload: Upload):
def get_proper_parser(upload: Upload, contents: bytes):
if upload.upload_extras and upload.upload_extras.get("format_version") == "v1":
return VersionOneReportParser()
contents = contents.strip()
if contents.startswith(b"{") and contents.endswith(b"}"):
return VersionOneReportParser()
else:
with sentry_sdk.configure_scope() as scope:
scope.set_extra("upload_extras", upload.upload_extras)
scope.set_extra("contents", contents[:64])
sentry_sdk.capture_message("Upload `format_version` lied to us")
return LegacyReportParser()
17 changes: 9 additions & 8 deletions services/report/parser/tests/unit/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@
@pytest.mark.parametrize(
"upload_extras, expected_type",
[
(None, LegacyReportParser),
({}, LegacyReportParser),
({"something": 1}, LegacyReportParser),
({"format_version": "v1"}, VersionOneReportParser),
({"format_version": "v1", "something": "else"}, VersionOneReportParser),
({"format_version": None}, LegacyReportParser),
(None, b"", LegacyReportParser),
({}, b"", LegacyReportParser),
({"something": 1}, b"", LegacyReportParser),
({"format_version": "v1"}, b"{}", VersionOneReportParser),
({"format_version": "v1", "something": "else"}, b"{}", VersionOneReportParser),
({"format_version": None}, b"", LegacyReportParser),
({"format_version": "v1"}, b"not/a/v1/format.txt", LegacyReportParser),
],
)
def test_get_proper_parser(dbsession, upload_extras, expected_type):
def test_get_proper_parser(dbsession, upload_extras, contents, expected_type):
upload = UploadFactory.create(upload_extras=upload_extras)
dbsession.add(upload)
dbsession.flush()
assert isinstance(get_proper_parser(upload), expected_type)
assert isinstance(get_proper_parser(upload, contents), expected_type)

0 comments on commit afdef97

Please sign in to comment.