feat(bib): Store.upsert_status — identical re-upsert writes nothing (refs #615)
This commit is contained in:
@@ -206,6 +206,17 @@ class Store:
|
|||||||
con.execute("DELETE FROM items WHERE key = ?", (key,))
|
con.execute("DELETE FROM items WHERE key = ?", (key,))
|
||||||
con.commit()
|
con.commit()
|
||||||
|
|
||||||
|
_COMPARE_COLS = (
|
||||||
|
"item_type",
|
||||||
|
"title",
|
||||||
|
"url",
|
||||||
|
"date_published",
|
||||||
|
"abstract",
|
||||||
|
"institution",
|
||||||
|
"extra",
|
||||||
|
"extra_json",
|
||||||
|
)
|
||||||
|
|
||||||
def upsert(
|
def upsert(
|
||||||
self,
|
self,
|
||||||
item: Item,
|
item: Item,
|
||||||
@@ -214,37 +225,68 @@ class Store:
|
|||||||
collection: str = "",
|
collection: str = "",
|
||||||
) -> str:
|
) -> str:
|
||||||
"""Create or update an item, deduplicating by URL."""
|
"""Create or update an item, deduplicating by URL."""
|
||||||
if item.url:
|
return self.upsert_status(item, tags=tags, collection=collection)[0]
|
||||||
con = self._con()
|
|
||||||
existing = con.execute(
|
def upsert_status(
|
||||||
"SELECT key FROM items WHERE url = ?", (item.url,)
|
self,
|
||||||
).fetchone()
|
item: Item,
|
||||||
if existing:
|
*,
|
||||||
ekey = existing["key"]
|
tags: list[Any] | None = None,
|
||||||
|
collection: str = "",
|
||||||
|
) -> tuple[str, str]:
|
||||||
|
"""Like :meth:`upsert` but also report what happened:
|
||||||
|
``"created"``, ``"updated"`` or ``"unchanged"``.
|
||||||
|
|
||||||
|
``unchanged`` means every compared column, the merged tag set and
|
||||||
|
the merged collection set already match the stored row — nothing
|
||||||
|
is written, no ``access_date``/``updated_at`` stamp, no tag row
|
||||||
|
churn. A re-farm over a complete docket is therefore a read-only
|
||||||
|
pass over ``items``.
|
||||||
|
"""
|
||||||
if tags:
|
if tags:
|
||||||
for tag in tags:
|
for tag in tags:
|
||||||
label = tag.label if hasattr(tag, "label") else str(tag)
|
label = tag.label if hasattr(tag, "label") else str(tag)
|
||||||
item.add_tag(label)
|
item.add_tag(label)
|
||||||
if collection and collection not in item.collections:
|
if collection and collection not in item.collections:
|
||||||
item.collections.append(collection)
|
item.collections.append(collection)
|
||||||
item.stamp_access()
|
|
||||||
# Merge with what's already stored — a re-ingest must
|
if not item.url:
|
||||||
# never clobber tags/collections curated on the row
|
return self.create(item), "created"
|
||||||
# since the last ingest (#624: the CY2027 NPRM lost its
|
|
||||||
# sup: registration tag to this and silently vanished
|
con = self._con()
|
||||||
# from the tag-scoped Zotero sync). Deliberate removal
|
existing = con.execute(
|
||||||
# goes through remove_tag, not upsert.
|
"SELECT key FROM items WHERE url = ?", (item.url,)
|
||||||
|
).fetchone()
|
||||||
|
if not existing:
|
||||||
|
return self.create(item), "created"
|
||||||
|
|
||||||
|
ekey = existing["key"]
|
||||||
|
# Merge with what's already stored — a re-ingest must never
|
||||||
|
# clobber tags/collections curated on the row since the last
|
||||||
|
# ingest (#624). Deliberate removal goes through remove_tag.
|
||||||
current = self.get(ekey)
|
current = self.get(ekey)
|
||||||
|
merged_tags = list(dict.fromkeys([*current.tags, *item.tags]))
|
||||||
|
merged_cols = list(dict.fromkeys([*current.collections, *item.collections]))
|
||||||
|
|
||||||
|
new_row = item.to_row()
|
||||||
|
cur_row = current.to_row()
|
||||||
|
same_cols = all(
|
||||||
|
new_row.get(c, "") == cur_row.get(c, "") for c in self._COMPARE_COLS
|
||||||
|
)
|
||||||
|
if (
|
||||||
|
same_cols
|
||||||
|
and set(merged_tags) == set(current.tags)
|
||||||
|
and set(merged_cols) == set(current.collections)
|
||||||
|
):
|
||||||
|
return ekey, "unchanged"
|
||||||
|
|
||||||
|
item.stamp_access()
|
||||||
row = item.to_row()
|
row = item.to_row()
|
||||||
row.pop("key", None)
|
row.pop("key", None)
|
||||||
row["tags"] = list(dict.fromkeys([*current.tags, *item.tags]))
|
row["tags"] = merged_tags
|
||||||
row["collections"] = list(
|
row["collections"] = merged_cols
|
||||||
dict.fromkeys([*current.collections, *item.collections])
|
|
||||||
)
|
|
||||||
self.update(ekey, **row)
|
self.update(ekey, **row)
|
||||||
return ekey
|
return ekey, "updated"
|
||||||
|
|
||||||
return self.create(item, tags=tags, collection=collection)
|
|
||||||
|
|
||||||
# ── Query ────────────────────────────────────────────────────
|
# ── Query ────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|||||||
76
tests/bib/test_store_upsert_noop.py
Normal file
76
tests/bib/test_store_upsert_noop.py
Normal file
@@ -0,0 +1,76 @@
|
|||||||
|
"""Store.upsert must not rewrite rows/tags when nothing changed."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from bib.item import Source
|
||||||
|
from bib.store import Store
|
||||||
|
|
||||||
|
|
||||||
|
def _store() -> Store:
|
||||||
|
return Store(":memory:", storage_dir="/tmp/nope")
|
||||||
|
|
||||||
|
|
||||||
|
def _item(**kw) -> Source:
|
||||||
|
it = Source(title="T", url="https://www.regulations.gov/comment/CMS-2026-2377-1")
|
||||||
|
it.abstract = kw.get("abstract", "body")
|
||||||
|
for t in kw.get("tags", ["a:1", "b:2"]):
|
||||||
|
it.add_tag(t)
|
||||||
|
return it
|
||||||
|
|
||||||
|
|
||||||
|
def _snapshot(s: Store, key: str) -> tuple:
|
||||||
|
con = s._con()
|
||||||
|
row = con.execute(
|
||||||
|
"SELECT access_date, updated_at FROM items WHERE key=?", (key,)
|
||||||
|
).fetchone()
|
||||||
|
tags = con.execute(
|
||||||
|
"SELECT it.rowid FROM item_tags it JOIN items i ON i.id=it.item_id WHERE i.key=? ORDER BY 1",
|
||||||
|
(key,),
|
||||||
|
).fetchall()
|
||||||
|
return (row["access_date"], row["updated_at"], [t[0] for t in tags])
|
||||||
|
|
||||||
|
|
||||||
|
def test_first_upsert_is_created():
|
||||||
|
s = _store()
|
||||||
|
key, status = s.upsert_status(_item())
|
||||||
|
assert status == "created" and key
|
||||||
|
|
||||||
|
|
||||||
|
def test_identical_upsert_is_unchanged_and_writes_nothing():
|
||||||
|
s = _store()
|
||||||
|
key, _ = s.upsert_status(_item())
|
||||||
|
before = _snapshot(s, key)
|
||||||
|
# different Python object, same content
|
||||||
|
key2, status = s.upsert_status(_item())
|
||||||
|
assert key2 == key and status == "unchanged"
|
||||||
|
assert _snapshot(s, key) == before # no access/updated stamp, no tag row churn
|
||||||
|
|
||||||
|
|
||||||
|
def test_changed_abstract_is_updated():
|
||||||
|
s = _store()
|
||||||
|
key, _ = s.upsert_status(_item(abstract="v1"))
|
||||||
|
_, status = s.upsert_status(_item(abstract="v2"))
|
||||||
|
assert status == "updated"
|
||||||
|
assert s.get(key).abstract == "v2"
|
||||||
|
|
||||||
|
|
||||||
|
def test_new_tag_is_updated_and_merged():
|
||||||
|
s = _store()
|
||||||
|
key, _ = s.upsert_status(_item(tags=["a:1"]))
|
||||||
|
_, status = s.upsert_status(_item(tags=["c:3"]))
|
||||||
|
assert status == "updated"
|
||||||
|
assert set(s.get(key).tags) >= {"a:1", "c:3"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_subset_of_existing_tags_is_unchanged():
|
||||||
|
"""Upsert never removes tags (#624); a subset therefore changes nothing."""
|
||||||
|
s = _store()
|
||||||
|
key, _ = s.upsert_status(_item(tags=["a:1", "b:2"]))
|
||||||
|
_, status = s.upsert_status(_item(tags=["a:1"]))
|
||||||
|
assert status == "unchanged"
|
||||||
|
|
||||||
|
|
||||||
|
def test_upsert_keeps_returning_key():
|
||||||
|
s = _store()
|
||||||
|
key = s.upsert(_item())
|
||||||
|
assert s.upsert(_item()) == key
|
||||||
Reference in New Issue
Block a user