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

@@ -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(),

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):

socorro/tests/processor/rules/test_mozilla.py

@@ -1273,80 +1273,79 @@ 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()
         ret = rule.predicate(raw_crash, dumps, processed_crash, str(tmp_path), status)
         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: