fix(bib): self-exclusion in resend dedup + alias tag merge (refs #665)
This commit is contained in:
kert
@@ -131,27 +131,31 @@ def _ingest_message(
|
||||
|
||||
# Listservs re-send identical content under a fresh Message-ID
|
||||
# (Class B in docs/superpowers/plans/2026-08-26-zotero-dedupe.md).
|
||||
# If an item with this exact title+body already exists, treat this
|
||||
# message as an alias of it rather than creating a new Source.
|
||||
# json_extract is more robust than a LIKE-on-serialized-JSON match
|
||||
# since it doesn't depend on json.dumps's separator spacing.
|
||||
# If an item with this exact title+body already exists under a
|
||||
# DIFFERENT url, treat this message as an alias of it rather than
|
||||
# creating a new Source. ``AND url != ?`` excludes this message's own
|
||||
# row — without it, re-ingesting the identical Message-ID (e.g. IMAP
|
||||
# Seen state was lost, the scenario this module's docstring promises
|
||||
# to survive) would match itself, record its own mid as an alias of
|
||||
# itself, and skip the normal upsert/tag/attachment-refresh path
|
||||
# below. json_extract is more robust than a LIKE-on-serialized-JSON
|
||||
# match since it doesn't depend on json.dumps's separator spacing.
|
||||
con = store._con()
|
||||
dup = con.execute(
|
||||
"SELECT key, extra_json FROM items"
|
||||
"SELECT key, url, extra_json FROM items"
|
||||
" WHERE item_type = 'source' AND title = ?"
|
||||
" AND json_extract(extra_json, '$.body_sha1') = ?",
|
||||
(title, body_sha1),
|
||||
" AND json_extract(extra_json, '$.body_sha1') = ?"
|
||||
" AND url != ?",
|
||||
(title, body_sha1, url),
|
||||
).fetchone()
|
||||
if dup:
|
||||
ej = json.loads(dup["extra_json"] or "{}")
|
||||
aliases = ej.get("alias_mids", [])
|
||||
if raw_mid not in aliases:
|
||||
aliases.append(raw_mid)
|
||||
ej["alias_mids"] = aliases
|
||||
store.update(dup["key"], extra_json=json.dumps(ej))
|
||||
return 0
|
||||
is_alias = dup is not None
|
||||
# An alias re-send targets the keeper's existing url so tags/collections
|
||||
# merge through Store.upsert's normal union-by-url logic below (a
|
||||
# cross-mailbox/cross-list resend must still contribute its
|
||||
# mailbox:/sender:/list:/year: tags to the keeper).
|
||||
target_url = dup["url"] if is_alias else url
|
||||
|
||||
item = Source(title=title, url=url)
|
||||
item = Source(title=title, url=target_url)
|
||||
item.doc_type = "Email"
|
||||
item.institution = sender_domain or "Email"
|
||||
item.date_published = _date_iso(msg.get("Date", ""))
|
||||
@@ -170,23 +174,36 @@ def _ingest_message(
|
||||
if item.date_published:
|
||||
item.add_tag(f"year:{item.date_published[:4]}")
|
||||
|
||||
# Preserve any extra_json already on this exact url (e.g. alias_mids
|
||||
# accumulated from prior re-sends of this same Message-ID) — upsert()
|
||||
# doesn't merge extra_json, it overwrites with item.to_row()'s output.
|
||||
prior = con.execute("SELECT extra_json FROM items WHERE url = ?", (url,)).fetchone()
|
||||
# Preserve any extra_json already on the target row (alias_mids
|
||||
# accumulated from prior re-sends, or this exact item's own
|
||||
# body_sha1/alias_mids on a same-mid re-ingest) — upsert() doesn't
|
||||
# merge extra_json, it overwrites with item.to_row()'s output.
|
||||
prior = con.execute(
|
||||
"SELECT extra_json FROM items WHERE url = ?", (target_url,)
|
||||
).fetchone()
|
||||
prior_ej = json.loads(prior["extra_json"] or "{}") if prior else {}
|
||||
|
||||
key = store.upsert(item)
|
||||
|
||||
# Record the body hash so future re-sends under a fresh Message-ID
|
||||
# can be matched to this item instead of creating a duplicate. Merge
|
||||
# order: prior_ej first (carries forward alias_mids), then this
|
||||
# item's own to_row() extra_json (carries the current doc_type),
|
||||
# then body_sha1 last so it always reflects this ingest's body.
|
||||
fresh_ej = json.loads(item.to_row()["extra_json"] or "{}")
|
||||
store.update(
|
||||
key, extra_json=json.dumps({**prior_ej, **fresh_ej, "body_sha1": body_sha1})
|
||||
)
|
||||
# Merge order: prior_ej first (carries forward alias_mids), then this
|
||||
# item's own to_row() extra_json (carries the current doc_type), then
|
||||
# either a new alias_mid (alias case) or the fresh body_sha1 (new item
|
||||
# or same-mid re-ingest) last so it always wins.
|
||||
merged_ej = {**prior_ej, **json.loads(item.to_row()["extra_json"] or "{}")}
|
||||
if is_alias:
|
||||
aliases = merged_ej.get("alias_mids", [])
|
||||
if raw_mid not in aliases:
|
||||
aliases.append(raw_mid)
|
||||
merged_ej["alias_mids"] = aliases
|
||||
else:
|
||||
merged_ej["body_sha1"] = body_sha1
|
||||
store.update(key, extra_json=json.dumps(merged_ej))
|
||||
|
||||
if is_alias:
|
||||
# Content is identical to the keeper's — tags are merged above via
|
||||
# the normal upsert-by-url path, but attachments aren't reprocessed
|
||||
# for a duplicate-content resend.
|
||||
return 0
|
||||
|
||||
# Attachments → temp dir → attach_file
|
||||
attached = 0
|
||||
|
||||
@@ -295,3 +295,55 @@ class TestResendAliasing:
|
||||
assert len(items) == 1
|
||||
ej = _raw_extra_json(store, items[0].key)
|
||||
assert len(ej.get("body_sha1", "")) == 40
|
||||
|
||||
def test_same_mid_reingest_does_not_self_alias(self, store, mailbox, tmp_path):
|
||||
"""Re-ingesting the identical message (same Message-ID — e.g. IMAP
|
||||
Seen state was lost) must fall through to the normal upsert-by-url
|
||||
update path, not match itself in the resend-dedup lookup and
|
||||
record its own mid as an alias of itself."""
|
||||
m = make_msg(
|
||||
"Same Message Twice",
|
||||
"Body content.",
|
||||
"same-mid@x.example",
|
||||
"Mon, 27 Apr 2026 10:00:00 -0400",
|
||||
)
|
||||
_ingest_message(store, mailbox, m, tmp_path)
|
||||
with patch.object(store, "upsert", wraps=store.upsert) as spy_upsert:
|
||||
_ingest_message(store, mailbox, m, tmp_path)
|
||||
assert spy_upsert.called, (
|
||||
"second ingest must go through the normal upsert-by-url path,"
|
||||
" not the alias short-circuit"
|
||||
)
|
||||
items = store.list_items(query="Same Message Twice")
|
||||
assert len(items) == 1
|
||||
ej = _raw_extra_json(store, items[0].key)
|
||||
assert "same-mid@x.example" not in ej.get("alias_mids", [])
|
||||
|
||||
def test_alias_resend_tags_merge_into_keeper(self, store, tmp_path):
|
||||
"""A re-send arriving via a different mailbox must contribute its
|
||||
mailbox:/sender:/list:/year: tags to the keeper item, not just
|
||||
record its Message-ID as an alias."""
|
||||
mb1 = Mailbox(
|
||||
host="mail.test", port=993, username="cmslists@test.com", password="pw"
|
||||
)
|
||||
mb2 = Mailbox(
|
||||
host="mail.test", port=993, username="otherlist@test.com", password="pw"
|
||||
)
|
||||
m1 = make_msg(
|
||||
"Cross Mailbox Resend",
|
||||
"Same body.",
|
||||
"hhh@x.example",
|
||||
"Mon, 27 Apr 2026 10:00:00 -0400",
|
||||
)
|
||||
m2 = make_msg(
|
||||
"Cross Mailbox Resend",
|
||||
"Same body.",
|
||||
"iii@x.example",
|
||||
"Fri, 10 Jul 2026 10:00:00 -0400",
|
||||
)
|
||||
_ingest_message(store, mb1, m1, tmp_path)
|
||||
_ingest_message(store, mb2, m2, tmp_path)
|
||||
items = store.list_items(query="Cross Mailbox Resend")
|
||||
assert len(items) == 1
|
||||
assert "mailbox:cmslists" in items[0].tags
|
||||
assert "mailbox:otherlist" in items[0].tags
|
||||
|
||||
Reference in New Issue
Block a user