LiveSync: Correctly handling cat blacklist during moves

Delete events from search when moving to a blacklisted category, and
create then when moving out of such a category.
This commit is contained in:
Adrian Moennich 2021-06-08 18:46:13 +02:00
parent 8f2d8114a3
commit 13225a548a
5 changed files with 90 additions and 23 deletions

View File

@ -84,9 +84,20 @@ def connect_signals(plugin):
plugin.connect(signals.acl.entry_changed, _acl_entry_changed, sender=Attachment)
def _is_category_excluded(category):
excluded_categories = get_excluded_categories()
return any(c.id in excluded_categories for c in category.chain_query)
def _moved(obj, old_parent, **kwargs):
_register_change(obj, ChangeType.moved)
new_category = obj if isinstance(obj, Category) else obj.category
old_excluded = _is_category_excluded(old_parent)
new_excluded = _is_category_excluded(new_category)
if old_excluded != new_excluded:
_register_change(obj, ChangeType.unpublished if new_excluded else ChangeType.published)
if obj.is_inheriting:
# If protection is inherited, check whether it changed
category_protection = old_parent.effective_protection_mode

View File

@ -0,0 +1,30 @@
"""Add published/unpublished change types
Revision ID: ff1323696f67
Revises: 330e32d26232
Create Date: 2021-06-08 17:13:48.935771
"""
from alembic import op
# revision identifiers, used by Alembic.
revision = 'ff1323696f67'
down_revision = '330e32d26232'
branch_labels = None
depends_on = None
def upgrade():
op.execute('''
ALTER TABLE plugin_livesync.queues DROP CONSTRAINT "ck_queues_valid_enum_change";
ALTER TABLE plugin_livesync.queues ADD CONSTRAINT "ck_queues_valid_enum_change" CHECK ((change = ANY (ARRAY[1, 2, 3, 4, 5, 6, 7, 8, 9])));
''')
def downgrade():
op.execute('DELETE FROM plugin_livesync.queues WHERE change IN (8, 9)')
op.execute('''
ALTER TABLE plugin_livesync.queues DROP CONSTRAINT "ck_queues_valid_enum_change";
ALTER TABLE plugin_livesync.queues ADD CONSTRAINT "ck_queues_valid_enum_change" CHECK ((change = ANY (ARRAY[1, 2, 3, 4, 5, 6, 7])));
''')

View File

@ -26,6 +26,8 @@ class ChangeType(int, IndicoEnum):
protection_changed = 5
location_changed = 6
undeleted = 7
published = 8
unpublished = 9
class EntryType(int, IndicoEnum):
@ -278,15 +280,17 @@ class LiveSyncQueueEntry(db.Model):
ref = dict(ref)
obj = obj_deref(ref)
if isinstance(obj, Category):
if any(c.id in excluded_categories for c in obj.chain_query):
return
else:
event = obj.folder.event if isinstance(obj, Attachment) else obj.event
if event.category not in g.setdefault('livesync_excluded_categories_checked', {}):
g.livesync_excluded_categories_checked[event.category] = excluded_categories & set(event.category_chain)
if g.livesync_excluded_categories_checked[event.category]:
return
if ChangeType.published not in changes and ChangeType.unpublished not in changes:
if isinstance(obj, Category):
if any(c.id in excluded_categories for c in obj.chain_query):
return
else:
event = obj.folder.event if isinstance(obj, Attachment) else obj.event
if event.category not in g.setdefault('livesync_excluded_categories_checked', {}):
g.livesync_excluded_categories_checked[event.category] = \
excluded_categories & set(event.category_chain)
if g.livesync_excluded_categories_checked[event.category]:
return
try:
agents = g.livesync_agents

View File

@ -53,6 +53,8 @@ def process_records(records):
"""
changes = defaultdict(int)
cascaded_create_records = set()
cascaded_publish_records = set()
cascaded_unpublish_records = set()
cascaded_undelete_records = set()
cascaded_update_records = set()
cascaded_delete_records = set()
@ -66,6 +68,10 @@ def process_records(records):
if record.change == ChangeType.created:
assert record.type != EntryType.category
cascaded_create_records.add(record)
elif record.change == ChangeType.published:
cascaded_publish_records.add(record)
elif record.change == ChangeType.unpublished:
cascaded_unpublish_records.add(record)
elif record.change == ChangeType.undeleted:
assert record.type != EntryType.category
cascaded_undelete_records.add(record)
@ -89,6 +95,12 @@ def process_records(records):
for obj in _process_cascaded_category_contents(cascaded_update_records):
changes[obj] |= SimpleChange.updated
for obj in _process_cascaded_category_contents(cascaded_unpublish_records):
changes[obj] |= SimpleChange.deleted
for obj in _process_cascaded_category_contents(cascaded_publish_records):
changes[obj] |= SimpleChange.created
for obj in _process_cascaded_event_contents(cascaded_delete_records):
changes[obj] |= SimpleChange.deleted
@ -130,10 +142,13 @@ def _process_cascaded_category_contents(records):
and rec.change == ChangeType.protection_changed}
category_move_records = {rec.category_id for rec in records if rec.type == EntryType.category
and rec.change == ChangeType.moved}
category_publishing_records = {rec.category_id for rec in records if rec.type == EntryType.category
and rec.change in (ChangeType.published, ChangeType.unpublished)}
changed_events = set()
category_prot_records -= category_move_records # A move already implies sending the whole record
category_prot_records -= category_publishing_records # A publish/unpublish already implies sending the whole record
# Protection changes are handled differently, as there may not be the need to re-generate the record
if category_prot_records:
@ -156,6 +171,10 @@ def _process_cascaded_category_contents(records):
changed_events.update(Event.query.filter(Event.category_chain_overlaps(category_move_records),
~Event.is_deleted,
excluded_categories_filter))
if category_publishing_records:
changed_events.update(Event.query.filter(Event.category_chain_overlaps(category_publishing_records),
~Event.is_deleted,
excluded_categories_filter))
yield from _process_cascaded_event_contents(records, additional_events=changed_events)

View File

@ -22,26 +22,27 @@ def queue_entry_dummy_object(monkeypatch):
monkeypatch.setattr(LiveSyncQueueEntry, 'object', object)
@pytest.mark.parametrize(('change', 'invalid'), (
(ChangeType.created, True),
(ChangeType.deleted, True),
(ChangeType.data_changed, True),
(ChangeType.protection_changed, False),
(ChangeType.moved, False),
@pytest.mark.parametrize(('change', 'invalid', 'simplified'), (
(ChangeType.created, True, None),
(ChangeType.deleted, True, None),
(ChangeType.data_changed, True, None),
(ChangeType.location_changed, True, None),
(ChangeType.undeleted, True, None),
(ChangeType.published, False, SimpleChange.created),
(ChangeType.unpublished, False, SimpleChange.deleted),
(ChangeType.protection_changed, False, SimpleChange.updated),
(ChangeType.moved, False, SimpleChange.updated),
))
@pytest.mark.usefixtures('queue_entry_dummy_object', 'db')
def test_process_records_category_ignored(mocker, change, invalid):
@pytest.mark.usefixtures('queue_entry_dummy_object')
def test_process_records_category_ignored(dummy_agent, dummy_category, dummy_event, change, invalid, simplified):
"""Test if categories are only kept for certain changes."""
cascade = mocker.patch('indico_livesync.simplify._process_cascaded_category_contents')
cascade.return_value = [object()]
records = [LiveSyncQueueEntry(change=change, type=EntryType.category)]
records = [LiveSyncQueueEntry(agent=dummy_agent, change=change, type=EntryType.category, category=dummy_category)]
if invalid:
with pytest.raises(AssertionError):
process_records(records)
else:
result = process_records(records)
assert len(result) == 1
assert list(result.values())[0] == SimpleChange.updated
assert result == {dummy_event: simplified}
@pytest.mark.parametrize(('change', 'cascade'), (
@ -57,7 +58,9 @@ def test_process_records_cascade(mocker, change, cascade):
cascade_mock = mocker.patch('indico_livesync.simplify._process_cascaded_category_contents')
records = [LiveSyncQueueEntry(change=change)]
process_records(records)
assert cascade_mock.call_args == (({records[0]} if cascade else set(),),)
assert cascade_mock.call_args_list[0] == (({records[0]} if cascade else set(),),)
assert cascade_mock.call_args_list[1] == ((set(),),)
assert cascade_mock.call_args_list[2] == ((set(),),)
@pytest.mark.parametrize('changes', bool_matrix('......'))