mjolnir
mjolnir copied to clipboard
Refactor syncing protected rooms with ban lists
While working on #221 it became clear there's a lot of overlap and going back and forwards between Mjolnir.syncLists()
, Mjolnir.syncWithBanList()
and BanList.updateList()
Basically we have Mjolnir that has all the BanLists, Mjolnir currently is in control of calling updateList() on them. Each time it gets an event from sync from a ban list it calls BanList.updateForEvent()
to be batched by the batcher BanList
has, which will later call BanList.updateList()
.
Unfortunately when you let BanList control when updateList()
is called (because of new events from sync), you need some way of going back to Mjolnir to call Mjolnir.syncWithBanList()
or an equivalent. We currently have the BanList.batch
event on the BanList event emitter to do this job because:
-
Using a callback and giving it each time to BanList.updateForEvent() would be strange as the callback is only called when there is a batch, and we are always giving the same callback and only want it to be called once. So we would have a really unintuitive interface where even though we provide a callback for every event, we only call one of them once each batch.
-
Using the existing
BanList.update
event (instead of the newBanList.batch
event) requires refactoringMjolnir.syncLists()
because when Mjolnir callssyncsList
(to initialize the banLists for the first time or because someone used themjolnir! sync
command) we would also be calling the listener for when the batcher callsBanList.updateList
(that is replacingMjolnir.syncWithBanList()
and both of them apply bans, acl and do printing. We don't want that. So we would have had to refactorMjolnir.syncLists
to also not do those things and we would have to consider thatsyncLists
reports all the errors at the same time for all lists (synchronously from the perspective of the command executor), and with theBanList.update
listener would instead be reporting each individually (there are also different uses of the verbosity config setting between the two aswell just to complicate things further) which is a change of behaviour.