fix regex matching on revisions with deleted text or comments
RegexPair.matchmake crashed with a TypeError when a capture-group pattern was applied to a revision whose text or comment was deleted or suppressed (content is None). Guard both matching paths against None, and make the no-capture-group path emit a None column for such revisions instead of omitting the key entirely. This carries forward the fix Kaylea Champion and Mako Hill made on the mako_changes-20230429 branch (7e6cd5b), which predated the rewrite. Adds a unit test for matchmake(None) and an end-to-end test against the ikwiki dump, which contains revisions with deleted text and comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commitdbcae5c64e)
This commit is contained in:
@@ -463,7 +463,8 @@ class RegexPair(object):
|
|||||||
# if there are named capture groups in the regex
|
# if there are named capture groups in the regex
|
||||||
if self.has_groups:
|
if self.has_groups:
|
||||||
# if there are matches of some sort in this revision content, fill the lists for each cap_group
|
# if there are matches of some sort in this revision content, fill the lists for each cap_group
|
||||||
if self.pattern.search(content) is not None:
|
# content can be None when the text or comment was deleted/suppressed
|
||||||
|
if content is not None and self.pattern.search(content) is not None:
|
||||||
m = self.pattern.finditer(content)
|
m = self.pattern.finditer(content)
|
||||||
matchobjects = list(m)
|
matchobjects = list(m)
|
||||||
|
|
||||||
@@ -491,12 +492,11 @@ class RegexPair(object):
|
|||||||
# there are no capture groups, we just search for all the matches of the regex
|
# there are no capture groups, we just search for all the matches of the regex
|
||||||
else:
|
else:
|
||||||
# given that there are matches to be made
|
# given that there are matches to be made
|
||||||
if type(content) in (str, bytes):
|
if content is not None and self.pattern.search(content) is not None:
|
||||||
if self.pattern.search(content) is not None:
|
m = self.pattern.findall(content)
|
||||||
m = self.pattern.findall(content)
|
temp_dict[self.label] = ", ".join(m)
|
||||||
temp_dict[self.label] = ", ".join(m)
|
else:
|
||||||
else:
|
temp_dict[self.label] = None
|
||||||
temp_dict[self.label] = None
|
|
||||||
|
|
||||||
return temp_dict
|
return temp_dict
|
||||||
|
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ import pytest
|
|||||||
from pandas import DataFrame
|
from pandas import DataFrame
|
||||||
from pandas.testing import assert_frame_equal, assert_series_equal
|
from pandas.testing import assert_frame_equal, assert_series_equal
|
||||||
|
|
||||||
from wikiq import build_table, build_schema
|
from wikiq import build_table, build_schema, RegexPair
|
||||||
from wikiq_test_utils import (
|
from wikiq_test_utils import (
|
||||||
BASELINE_DIR,
|
BASELINE_DIR,
|
||||||
IKWIKI,
|
IKWIKI,
|
||||||
@@ -391,6 +391,35 @@ def test_capturegroup_regex():
|
|||||||
baseline = pd.read_table(tester.baseline_file)
|
baseline = pd.read_table(tester.baseline_file)
|
||||||
assert_frame_equal(test, baseline, check_like=True)
|
assert_frame_equal(test, baseline, check_like=True)
|
||||||
|
|
||||||
|
def test_regex_none_content():
|
||||||
|
# deleted or suppressed revisions yield None for text and comments, and
|
||||||
|
# matchmake must tolerate that in both the capture-group and plain paths
|
||||||
|
pair = RegexPair(r"(?P<letter>\b[a-zA-Z]{3}\b)|(?P<number>\b\d+\b)", "cap")
|
||||||
|
assert pair.matchmake(None) == {"cap_letter": None, "cap_number": None}
|
||||||
|
|
||||||
|
pair = RegexPair(r"\b\d{3}\b", "digits")
|
||||||
|
assert pair.matchmake(None) == {"digits": None}
|
||||||
|
|
||||||
|
def test_regex_deleted_revisions():
|
||||||
|
# the ikwiki dump contains revisions with deleted text and deleted
|
||||||
|
# comments; regex matching must handle them rather than crashing
|
||||||
|
tester = WikiqTester(wiki=IKWIKI, case_name="regex_deleted")
|
||||||
|
|
||||||
|
try:
|
||||||
|
tester.call_wikiq(
|
||||||
|
"-RP '(?P<npov>npov|NPOV)' -RPl npov",
|
||||||
|
"-CP '(?P<talk>[Tt]alk)' -CPl talk",
|
||||||
|
)
|
||||||
|
except subprocess.CalledProcessError as exc:
|
||||||
|
pytest.fail(exc.stderr.decode("utf8"))
|
||||||
|
|
||||||
|
test = pd.read_table(tester.output)
|
||||||
|
|
||||||
|
deleted = test[test["deleted"]]
|
||||||
|
assert len(deleted) > 0
|
||||||
|
assert deleted["npov_npov"].isna().all()
|
||||||
|
assert deleted["talk_talk"].isna().all()
|
||||||
|
|
||||||
def test_external_links_only():
|
def test_external_links_only():
|
||||||
"""Test that --external-links extracts external links correctly."""
|
"""Test that --external-links extracts external links correctly."""
|
||||||
import mwparserfromhell
|
import mwparserfromhell
|
||||||
|
|||||||
Reference in New Issue
Block a user