On 17.04.2017 22:21, Matthew Wild wrote: > On 17 April 2017 at 19:34, Kim Alvefur <z...@zash.se> wrote: >> On Mon, Apr 17, 2017 at 02:32:22PM +0100, Matthew Wild wrote: >>> - Treat the request as an atomic change: succeed for all, or fail >>> all (i.e. if one of the JIDs is malformed, don't allow any of the >>> other changes in the request to take place). This would require work >>> to either pre-verify the changes (which we don't have code for) or to >>> roll back changes that were already made (which is a bit hacky, we'd >>> already have sent out notifications, unless we added a new 'batch >>> update' version of :set_affiliation()). >> >> I would prefer this. Possibly by collecting the changed state in a new >> table, then overwriting the old affiliations table as a commit. > > I'd prefer it too, but it's not trivial. > > If we really want to go this route, I have a suggestion - what about > adding a new room method, :set_affiliations() which takes a table of > changes instead of a single jid+affiliation? Then the current > :set_affiliation() can be simplified to just pass a single item to > :set_affiliations(). The current iq handling code can be simplified to > just pass a table of JIDs to :set_affiliations() without any of the > error checking logic (this would be in :set_affiliations()).
As was probably to be expected, when I tried implementing this, snags were hit. :handle_to_room() handles both affiliation and role changes. I checked the MUC XEP and I _think_ that it should be permissible to change an affiliation and a role in the same request. How far do we want to take the atomicity? If it should be on a request level, which would make sense in my opinion, then your first sketch of an implementation does not work. It might be that the affiliations can be cleanly applied but the roles cannot. However, the affiliations would already be applied, the roles would fail, the user would get an error but half of her request would be honored. So I would propose the following: [:handle_to_room] [check for malformed jids] [split up request in affiliation and role changes] [if :can_apply_affiliations() and :can_apply_roles()] [call :set_affiliations()] [call :set_roles()] where :set_roles() would get the same treatment as :set_affiliations(). Does that sound like a plan or am I missing something? Instead of just relying on documentation I'd like to call :can_apply_affiliations() also in :set_affiliations(). However, that would mean that the given list is traversed twice. Is that a performance problem or would that be acceptable? Best Regards Lennart -- You received this message because you are subscribed to the Google Groups "prosody-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to prosody-dev+unsubscr...@googlegroups.com. To post to this group, send email to firstname.lastname@example.org. Visit this group at https://groups.google.com/group/prosody-dev. For more options, visit https://groups.google.com/d/optout.