From 1af2fcdbd05cbf5bb36b1672f1521a3558119dfd Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Fri, 1 Nov 2024 15:05:37 -0400 Subject: [PATCH] bug-1851708: fix ESRVersionRewriteRule and FenixVersionRewriteRule (#6781) This fixes these two rules to stop editing the raw crash--that's soo bad! Now they run after we copy everything from the raw crash to the processed crash (validated and normalized), use data from the processed crash to figure out what to do, and edit data in the processed crash if necessary. --- socorro/mozilla_rulesets.py | 5 +- socorro/processor/rules/mozilla.py | 20 +++--- socorro/tests/processor/rules/test_mozilla.py | 69 +++++++++---------- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/socorro/mozilla_rulesets.py b/socorro/mozilla_rulesets.py index 2e990c910c..f8db9e73e2 100644 --- a/socorro/mozilla_rulesets.py +++ b/socorro/mozilla_rulesets.py @@ -68,11 +68,10 @@ CollectorMetadataRule(), # fix ModuleSignatureInfo if it needs fixing ConvertModuleSignatureInfoRule(), - # rules to change the internals of the raw crash - FenixVersionRewriteRule(), - ESRVersionRewrite(), # rules to transform a raw crash into a processed crash CopyFromRawCrashRule(schema=get_schema("processed_crash.schema.yaml")), + FenixVersionRewriteRule(), + ESRVersionRewrite(), SubmittedFromRule(), IdentifierRule(), MinidumpSha256HashRule(), diff --git a/socorro/processor/rules/mozilla.py b/socorro/processor/rules/mozilla.py index 938fc190ee..a931b14377 100644 --- a/socorro/processor/rules/mozilla.py +++ b/socorro/processor/rules/mozilla.py @@ -571,23 +571,25 @@ class FenixVersionRewriteRule(Rule): """ def predicate(self, raw_crash, dumps, processed_crash, tmpdir, status): - is_nightly = (raw_crash.get("Version") or "").startswith("Nightly ") - return raw_crash.get("ProductName") == "Fenix" and is_nightly + is_nightly = (processed_crash.get("version") or "").startswith("Nightly ") + return processed_crash.get("product_name") == "Fenix" and is_nightly def action(self, raw_crash, dumps, processed_crash, tmpdir, status): - status.add_note("Changed version from %r to 0.0a1" % raw_crash.get("Version")) - raw_crash["Version"] = "0.0a1" + if "version" in processed_crash: + version = processed_crash["version"] + status.add_note(f"Changed version from {version!r} to 0.0a1") + processed_crash["version"] = "0.0a1" class ESRVersionRewrite(Rule): def predicate(self, raw_crash, dumps, processed_crash, tmpdir, status): - return raw_crash.get("ReleaseChannel", "") == "esr" + return processed_crash.get("release_channel", "") == "esr" def action(self, raw_crash, dumps, processed_crash, tmpdir, status): - try: - raw_crash["Version"] += "esr" - except KeyError: - status.add_note('"Version" missing from esr release raw_crash') + if "version" in processed_crash: + processed_crash["version"] = processed_crash["version"] + "esr" + else: + status.add_note("'version' missing from esr release processed_crash") class TopMostFilesRule(Rule): diff --git a/socorro/tests/processor/rules/test_mozilla.py b/socorro/tests/processor/rules/test_mozilla.py index ac36529e99..85701b7063 100644 --- a/socorro/tests/processor/rules/test_mozilla.py +++ b/socorro/tests/processor/rules/test_mozilla.py @@ -1273,12 +1273,12 @@ class TestFenixVersionRewriteRule: ], ) def test_predicate(self, tmp_path, product, version, expected): - raw_crash = { - "ProductName": product, - "Version": version, - } + raw_crash = {} dumps = {} - processed_crash = {} + processed_crash = { + "product_name": product, + "version": version, + } status = Status() rule = FenixVersionRewriteRule() @@ -1286,67 +1286,66 @@ def test_predicate(self, tmp_path, product, version, expected): assert ret == expected def test_act(self, tmp_path): - raw_crash = { - "ProductName": "Fenix", - "Version": "Nightly 200315 05:05", - } + raw_crash = {} dumps = {} - processed_crash = {} + processed_crash = { + "product_name": "Fenix", + "version": "Nightly 200315 05:05", + } status = Status() rule = FenixVersionRewriteRule() rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status) - assert raw_crash["Version"] == "0.0a1" + assert processed_crash["version"] == "0.0a1" assert status.notes == ["Changed version from 'Nightly 200315 05:05' to 0.0a1"] class TestESRVersionRewrite: def test_everything_we_hoped_for(self, tmp_path): - raw_crash = copy.deepcopy(canonical_standard_raw_crash) - raw_crash["ReleaseChannel"] = "esr" + raw_crash = {} dumps = {} - processed_crash = {} + processed_crash = { + "release_channel": "esr", + "version": "120.0", + } status = Status() rule = ESRVersionRewrite() rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status) - assert raw_crash["Version"] == "12.0esr" - - # processed_crash should be unchanged - assert processed_crash == {} + assert raw_crash == {} + assert processed_crash["version"] == "120.0esr" def test_this_is_not_the_crash_you_are_looking_for(self, tmp_path): - raw_crash = copy.deepcopy(canonical_standard_raw_crash) - raw_crash["ReleaseChannel"] = "not_esr" + raw_crash = {} dumps = {} - processed_crash = {} + processed_crash = { + "release_channel": "release", + "version": "120.0", + } status = Status() rule = ESRVersionRewrite() rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status) - assert raw_crash["Version"] == "12.0" - - # processed_crash should be unchanged - assert processed_crash == {} + assert raw_crash == {} + assert processed_crash["version"] == "120.0" - def test_this_is_really_broken(self, tmp_path): - raw_crash = copy.deepcopy(canonical_standard_raw_crash) - raw_crash["ReleaseChannel"] = "esr" - del raw_crash["Version"] + def test_no_version(self, tmp_path): + raw_crash = {} dumps = {} - processed_crash = {} + processed_crash = { + "release_channel": "esr", + # no "version" + } status = Status() rule = ESRVersionRewrite() rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status) - assert "Version" not in raw_crash - assert status.notes == ['"Version" missing from esr release raw_crash'] - - # processed_crash should be unchanged - assert processed_crash == {} + assert raw_crash == {} + assert "version" not in processed_crash + assert status.notes == ["'version' missing from esr release processed_crash"] class TestTopMostFilesRule: