diff options
Diffstat (limited to 'app/services/members/destroy_service.rb')
-rw-r--r-- | app/services/members/destroy_service.rb | 44 |
1 files changed, 34 insertions, 10 deletions
diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 5afc13701e0..24c5b12b335 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -13,24 +13,48 @@ module Members end @skip_auth = skip_authorization - last_owner = true + + if a_group_owner?(member) + process_destroy_of_group_owner_member(member, skip_subresources, unassign_issuables) + else + destroy_member(member) + destroy_data_related_to_member(member, skip_subresources, unassign_issuables) + end + + member + end + + private + + def process_destroy_of_group_owner_member(member, skip_subresources, unassign_issuables) + # Deleting 2 different group owners via the API in quick succession could lead to + # wrong results for the `last_owner?` check due to race conditions. To prevent this + # we wrap both the last_owner? check and the deletes of owners within a lock. + last_group_owner = true in_lock("delete_members:#{member.source.class}:#{member.source.id}", sleep_sec: 0.1.seconds) do - break if member.is_a?(GroupMember) && member.source.last_owner?(member.user) + break if member.source.last_owner?(member.user) - last_owner = false - member.destroy + last_group_owner = false + destroy_member(member) end - unless last_owner - member.user&.invalidate_cache_counts - delete_member_associations(member, skip_subresources, unassign_issuables) - end + # deletion of related data does not have to be within the lock. + destroy_data_related_to_member(member, skip_subresources, unassign_issuables) unless last_group_owner + end - member + def destroy_member(member) + member.destroy end - private + def destroy_data_related_to_member(member, skip_subresources, unassign_issuables) + member.user&.invalidate_cache_counts + delete_member_associations(member, skip_subresources, unassign_issuables) + end + + def a_group_owner?(member) + member.is_a?(GroupMember) && member.owner? + end def delete_member_associations(member, skip_subresources, unassign_issuables) if member.request? && member.user != current_user |