Commit b6aec1f8 authored by Imre Farkas's avatar Imre Farkas Committed by Ash McKenzie

Remove expired GroupGroupLinks

Adds Groups::GroupLinks::DestroyService and extends
RemoveExpiredGroupLinksWorker to also delete expired GroupGroupLinks,
not only ProjectGroupLinks.
parent 08d34252
# frozen_string_literal: true
module Groups
module GroupLinks
class DestroyService < BaseService
def execute(one_or_more_links)
links = Array(one_or_more_links)
GroupGroupLink.transaction do
GroupGroupLink.delete(links)
groups_to_refresh = links.map(&:shared_with_group)
groups_to_refresh.uniq.each do |group|
group.refresh_members_authorized_projects
end
Gitlab::AppLogger.info("GroupGroupLinks with ids: #{links.map(&:id)} have been deleted.")
rescue => ex
Gitlab::AppLogger.error(ex)
raise
end
end
end
end
end
...@@ -8,5 +8,9 @@ class RemoveExpiredGroupLinksWorker ...@@ -8,5 +8,9 @@ class RemoveExpiredGroupLinksWorker
def perform def perform
ProjectGroupLink.expired.destroy_all # rubocop: disable DestroyAll ProjectGroupLink.expired.destroy_all # rubocop: disable DestroyAll
GroupGroupLink.expired.find_in_batches do |link_batch|
Groups::GroupLinks::DestroyService.new(nil, nil).execute(link_batch)
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Groups::GroupLinks::DestroyService, '#execute' do
let(:user) { create(:user) }
let_it_be(:group) { create(:group, :private) }
let_it_be(:shared_group) { create(:group, :private) }
let_it_be(:project) { create(:project, group: shared_group) }
subject { described_class.new(nil, nil) }
context 'single link' do
let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
it 'destroys link' do
expect { subject.execute(link) }.to change { GroupGroupLink.count }.from(1).to(0)
end
it 'revokes project authorization' do
group.add_developer(user)
expect { subject.execute(link) }.to(
change { Ability.allowed?(user, :read_project, project) }.from(true).to(false))
end
end
context 'multiple links' do
let_it_be(:another_group) { create(:group, :private) }
let_it_be(:another_shared_group) { create(:group, :private) }
let!(:links) do
[
create(:group_group_link, shared_group: shared_group, shared_with_group: group),
create(:group_group_link, shared_group: shared_group, shared_with_group: another_group),
create(:group_group_link, shared_group: another_shared_group, shared_with_group: group),
create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group)
]
end
it 'updates project authorization once per group' do
expect(GroupGroupLink).to receive(:delete)
expect(group).to receive(:refresh_members_authorized_projects).once
expect(another_group).to receive(:refresh_members_authorized_projects).once
subject.execute(links)
end
it 'rolls back changes when error happens' do
group.add_developer(user)
expect(group).to receive(:refresh_members_authorized_projects).once.and_call_original
expect(another_group).to(
receive(:refresh_members_authorized_projects).and_raise('boom'))
expect { subject.execute(links) }.to raise_error('boom')
expect(GroupGroupLink.count).to eq(links.length)
expect(Ability.allowed?(user, :read_project, project)).to be_truthy
end
end
end
...@@ -4,6 +4,7 @@ require 'spec_helper' ...@@ -4,6 +4,7 @@ require 'spec_helper'
describe RemoveExpiredGroupLinksWorker do describe RemoveExpiredGroupLinksWorker do
describe '#perform' do describe '#perform' do
context 'ProjectGroupLinks' do
let!(:expired_project_group_link) { create(:project_group_link, expires_at: 1.hour.ago) } let!(:expired_project_group_link) { create(:project_group_link, expires_at: 1.hour.ago) }
let!(:project_group_link_expiring_in_future) { create(:project_group_link, expires_at: 10.days.from_now) } let!(:project_group_link_expiring_in_future) { create(:project_group_link, expires_at: 10.days.from_now) }
let!(:non_expiring_project_group_link) { create(:project_group_link, expires_at: nil) } let!(:non_expiring_project_group_link) { create(:project_group_link, expires_at: nil) }
...@@ -23,4 +24,34 @@ describe RemoveExpiredGroupLinksWorker do ...@@ -23,4 +24,34 @@ describe RemoveExpiredGroupLinksWorker do
expect(non_expiring_project_group_link.reload).to be_present expect(non_expiring_project_group_link.reload).to be_present
end end
end end
context 'GroupGroupLinks' do
let(:mock_destroy_service) { instance_double(Groups::GroupLinks::DestroyService) }
before do
allow(Groups::GroupLinks::DestroyService).to(
receive(:new).and_return(mock_destroy_service))
end
context 'expired GroupGroupLink exists' do
before do
create(:group_group_link, expires_at: 1.hour.ago)
end
it 'calls Groups::GroupLinks::DestroyService' do
expect(mock_destroy_service).to receive(:execute).once
subject.perform
end
end
context 'expired GroupGroupLink does not exist' do
it 'does not call Groups::GroupLinks::DestroyService' do
expect(mock_destroy_service).not_to receive(:execute)
subject.perform
end
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment