Skip to content

Commit

Permalink
Fix search trigger bug when deleting and adding again the same issue.
Browse files Browse the repository at this point in the history
Found during #239.
  • Loading branch information
lemon24 committed Oct 27, 2021
1 parent 8b8b344 commit 3d4d7fb
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ Unreleased
* Fix bug causing
:attr:`~Entry.read_modified` and :attr:`~Entry.important_modified`
to be reset to :const:`None` when an entry is updated.
* Fix bug where deleting an entry and then adding it again
(with the same id) would fail
if search was enabled and :meth:`~Reader.update_search`
was not run before adding the new entry.


Version 2.4
Expand Down
52 changes: 38 additions & 14 deletions src/reader/_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,26 +303,47 @@ def _create_triggers(self) -> None:
# the ON DELETE trigger doesn't fire during REPLACE
# if recursive_triggers are not enabled (they aren't, as of 1.5).
#
# What I don't understand is why the "INSERT into esss" below succeeds
# even when a specific (id, feed) already exists in esss
# (it seems to act as INSERT OR REPLACE -- maybe because it already
# is in one, and that REPLACE is global?).
# Anyway, it seems to do what we need it to do.
# Note the entries_search_entries_insert{,_esss_exists} trigers
# cannot use OR REPLACE, so we must have one trigger for each case.

self.db.execute(
"""
CREATE TRIGGER entries_search_entries_insert
AFTER INSERT ON entries
WHEN
NOT EXISTS (
SELECT *
FROM entries_search_sync_state AS esss
WHERE (esss.id, esss.feed) = (new.id, new.feed)
)
BEGIN
INSERT OR REPLACE INTO entries_search_sync_state (id, feed, es_rowids)
VALUES (
new.id,
new.feed,
coalesce((
SELECT es_rowids
from entries_search_sync_state
where (id, feed) = (new.id, new.feed)
), '[]')
INSERT INTO entries_search_sync_state (id, feed)
VALUES (new.id, new.feed);
END;
"""
)
self.db.execute(
"""
CREATE TRIGGER entries_search_entries_insert_esss_exists
AFTER INSERT ON entries
WHEN
EXISTS (
SELECT *
FROM entries_search_sync_state AS esss
WHERE (esss.id, esss.feed) = (new.id, new.feed)
)
BEGIN
UPDATE entries_search_sync_state
SET
to_update = 1,
to_delete = 0
WHERE (new.id, new.feed) = (
entries_search_sync_state.id,
entries_search_sync_state.feed
);
END;
"""
Expand Down Expand Up @@ -446,6 +467,9 @@ def _drop_triggers(self) -> None:
# Private API, may be called from migrations.
assert self.db.in_transaction
self.db.execute("DROP TRIGGER IF EXISTS entries_search_entries_insert;")
self.db.execute(
"DROP TRIGGER IF EXISTS entries_search_entries_insert_esss_exists;"
)
self.db.execute("DROP TRIGGER IF EXISTS entries_search_entries_update;")
self.db.execute("DROP TRIGGER IF EXISTS entries_search_entries_delete;")
self.db.execute("DROP TRIGGER IF EXISTS entries_search_feeds_update;")
Expand Down
2 changes: 1 addition & 1 deletion src/reader/_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ def make_params() -> Iterable[Mapping[str, Any]]:
if "foreign key constraint failed" in e_msg:
raise FeedNotFoundError(feed_url) from None

elif "unique constraint failed" in e_msg:
elif "unique constraint failed: entries.id, entries.feed" in e_msg:
raise EntryExistsError(feed_url, entry_id) from None

else: # pragma: no cover
Expand Down
16 changes: 16 additions & 0 deletions tests/test_reader_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -1069,3 +1069,19 @@ def strip_html(*args, **kwargs):


# END concurrency tests


def test_add_entry_repeatedly(reader):
# this could have been found with Hypothesis

reader.enable_search()
reader.add_feed('1')

# we should not get
# sqlite3.IntegrityError: UNIQUE constraint failed:
# entries_search_sync_state.id, entries_search_sync_state.feed
# on the second loop

for _ in range(3):
reader.add_entry(dict(feed_url='1', id='1'))
reader.delete_entry(('1', '1'))

0 comments on commit 3d4d7fb

Please sign in to comment.