Merge branch '348176-dp-delete-api-update' into 'master'

Update dependency proxy API to use cleanup worker

See merge request gitlab-org/gitlab!76696

app/workers/purge_dependency_proxy_cache_worker.rb

@@ -12,14 +12,21 @@ class PurgeDependencyProxyCacheWorker
   queue_namespace :dependency_proxy
   feature_category :dependency_proxy
 
+  UPDATE_BATCH_SIZE = 100
+
   def perform(current_user_id, group_id)
     @current_user = User.find_by_id(current_user_id)
     @group = Group.find_by_id(group_id)
 
     return unless valid?
 
-    @group.dependency_proxy_blobs.destroy_all # rubocop:disable Cop/DestroyAll
-    @group.dependency_proxy_manifests.destroy_all # rubocop:disable Cop/DestroyAll
+    @group.dependency_proxy_blobs.each_batch(of: UPDATE_BATCH_SIZE) do |batch|
+      batch.update_all(status: :expired)
+    end
+
+    @group.dependency_proxy_manifests.each_batch(of: UPDATE_BATCH_SIZE) do |batch|
+      batch.update_all(status: :expired)
+    end
   end
 
   private

lib/api/dependency_proxy.rb

@@ -6,15 +6,6 @@ module API
 
     feature_category :dependency_proxy
 
-    helpers do
-      def obtain_new_purge_cache_lease
-        Gitlab::ExclusiveLease
-          .new("dependency_proxy:delete_group_blobs:#{user_group.id}",
-               timeout: 1.hour)
-          .try_obtain
-      end
-    end
-
     after_validation do
       authorize! :admin_group, user_group
     end
@@ -29,9 +20,6 @@ module API
       delete ':id/dependency_proxy/cache' do
         not_found! unless user_group.dependency_proxy_feature_available?
 
-        message = 'This request has already been made. It may take some time to purge the cache. You can run this at most once an hour for a given group'
-        render_api_error!(message, 409) unless obtain_new_purge_cache_lease
-
         # rubocop:disable CodeReuse/Worker
         PurgeDependencyProxyCacheWorker.perform_async(current_user.id, user_group.id)
         # rubocop:enable CodeReuse/Worker

spec/requests/api/dependency_proxy_spec.rb

@@ -3,8 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe API::DependencyProxy, api: true do
-  include ExclusiveLeaseHelpers
-
   let_it_be(:user) { create(:user) }
   let_it_be(:blob) { create(:dependency_proxy_blob )}
   let_it_be(:group, reload: true) { blob.group }
@@ -20,11 +18,8 @@ RSpec.describe API::DependencyProxy, api: true do
 
     shared_examples 'responding to purge requests' do
       context 'with feature available and enabled' do
-        let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" }
-
         context 'an admin user' do
           it 'deletes the blobs and returns no content' do
-            stub_exclusive_lease(lease_key, timeout: 1.hour)
             expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async)
 
             subject
@@ -32,23 +27,6 @@ RSpec.describe API::DependencyProxy, api: true do
             expect(response).to have_gitlab_http_status(:accepted)
             expect(response.body).to eq('202')
           end
-
-          context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do
-            it 'returns 409 with an error message' do
-              stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
-
-              subject
-
-              expect(response).to have_gitlab_http_status(:conflict)
-              expect(response.body).to include('This request has already been made.')
-            end
-
-            it 'executes service only for the first time' do
-              expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async).once
-
-              2.times { subject }
-            end
-          end
         end
 
         context 'a non-admin' do

spec/workers/purge_dependency_proxy_cache_worker_spec.rb

@@ -4,18 +4,18 @@ require 'spec_helper'
 
 RSpec.describe PurgeDependencyProxyCacheWorker do
   let_it_be(:user) { create(:admin) }
-  let_it_be(:blob) { create(:dependency_proxy_blob )}
-  let_it_be(:group, reload: true) { blob.group }
-  let_it_be(:manifest) { create(:dependency_proxy_manifest, group: group )}
+  let_it_be_with_refind(:blob) { create(:dependency_proxy_blob )}
+  let_it_be_with_reload(:group) { blob.group }
+  let_it_be_with_refind(:manifest) { create(:dependency_proxy_manifest, group: group )}
   let_it_be(:group_id) { group.id }
 
   subject { described_class.new.perform(user.id, group_id) }
 
   describe '#perform' do
-    shared_examples 'not removing blobs and manifests' do
-      it 'does not remove blobs and manifests', :aggregate_failures do
-        expect { subject }.not_to change { group.dependency_proxy_blobs.size }
-        expect { subject }.not_to change { group.dependency_proxy_manifests.size }
+    shared_examples 'not expiring blobs and manifests' do
+      it 'does not expire blobs and manifests', :aggregate_failures do
+        expect { subject }.not_to change { blob.status }
+        expect { subject }.not_to change { manifest.status }
         expect(subject).to be_nil
       end
     end
@@ -25,39 +25,36 @@ RSpec.describe PurgeDependencyProxyCacheWorker do
         include_examples 'an idempotent worker' do
           let(:job_args) { [user.id, group_id] }
 
-          it 'deletes the blobs and returns ok', :aggregate_failures do
-            expect(group.dependency_proxy_blobs.size).to eq(1)
-            expect(group.dependency_proxy_manifests.size).to eq(1)
-
+          it 'expires the blobs and returns ok', :aggregate_failures do
             subject
 
-            expect(group.dependency_proxy_blobs.size).to eq(0)
-            expect(group.dependency_proxy_manifests.size).to eq(0)
+            expect(blob).to be_expired
+            expect(manifest).to be_expired
           end
         end
       end
 
       context 'when admin mode is disabled' do
-        it_behaves_like 'not removing blobs and manifests'
+        it_behaves_like 'not expiring blobs and manifests'
       end
     end
 
     context 'a non-admin user' do
       let(:user) { create(:user) }
 
-      it_behaves_like 'not removing blobs and manifests'
+      it_behaves_like 'not expiring blobs and manifests'
     end
 
     context 'an invalid user id' do
       let(:user) { double('User', id: 99999 ) }
 
-      it_behaves_like 'not removing blobs and manifests'
+      it_behaves_like 'not expiring blobs and manifests'
     end
 
     context 'an invalid group' do
       let(:group_id) { 99999 }
 
-      it_behaves_like 'not removing blobs and manifests'
+      it_behaves_like 'not expiring blobs and manifests'
     end
   end
 end