Remove old cloud licenses upon sync or activation

Fixes a bug in the SyncSeatLinkRequestWorker where the current license
was not being deleted despite being tested.  This appears to be a
problem with the way License.current uses caching.  The .destroy!
command does not actually delete the record from the DB.  The fix
instead deletes all existing cloud licenses uses delete_all.

Applies a similar fix for removing old cloud licenses to the
ActivateService.  Also fixes a minor bug where new licenses created in
the ActivateService did not create a record with the cloud attribute.

ee/app/models/license.rb

@@ -256,6 +256,7 @@ class License < ApplicationRecord
   after_commit :reset_future_dated, on: [:create, :destroy]
   after_commit :reset_previous, on: [:create, :destroy]
 
+  scope :cloud, -> { where(cloud: true) }
   scope :recent, -> { reorder(id: :desc) }
   scope :last_hundred, -> { recent.limit(100) }
 

ee/app/services/gitlab_subscriptions/activate_service.rb

@@ -17,9 +17,11 @@ module GitlabSubscriptions
 
       return response unless response[:success]
 
-      license = License.new(data: response[:license_key])
+      license = License.new(data: response[:license_key], cloud: true)
 
       if license.save
+        License.cloud.id_not_in(license.id).delete_all
+
         { success: true, license: license }
       else
         error(license.errors.full_messages)

ee/app/workers/sync_seat_link_request_worker.rb

@@ -35,10 +35,8 @@ class SyncSeatLinkRequestWorker
   private
 
   def reset_license!(license_data)
-    current_license = License.current if License.current&.cloud_license?
-
     License.transaction do
-      current_license&.destroy!
+      License.cloud.delete_all
       License.create!(data: license_data, cloud: true)
     end
   rescue StandardError => e

ee/spec/models/license_spec.rb

@@ -399,6 +399,20 @@ RSpec.describe License do
     end
   end
 
+  describe 'Scopes' do
+    describe '.cloud' do
+      it 'includes cloud licenses' do
+        create(:license)
+        cloud_license_1 = create(:license, cloud: true)
+        cloud_license_2 = create(:license, cloud: true)
+
+        result = described_class.cloud
+
+        expect(result).to contain_exactly(cloud_license_1, cloud_license_2)
+      end
+    end
+  end
+
   describe "Class methods" do
     before do
       described_class.reset_current

ee/spec/services/gitlab_subscriptions/activate_service_spec.rb

@@ -38,6 +38,15 @@ RSpec.describe GitlabSubscriptions::ActivateService do
       expect(result).to eq({ success: true, license: created_license })
 
       expect(created_license.data).to eq(license_key)
+      expect(created_license.cloud).to eq(true)
+    end
+
+    it 'deletes any existing cloud licenses' do
+      previous_1 = create(:license, cloud: true)
+      previous_2 = create(:license, cloud: true)
+
+      expect { execute_service }.to change(License.cloud, :count).to(1)
+      expect(License.cloud).not_to include(previous_1, previous_2)
     end
 
     context 'when persisting fails' do

ee/spec/workers/sync_seat_link_request_worker_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe SyncSeatLinkRequestWorker, type: :worker do
     end
 
     context 'when response contains a license' do
-      let(:license_key) { build(:gitlab_license).export }
+      let(:license_key) { build(:gitlab_license, :cloud).export }
       let(:body) { { success: true, license: license_key }.to_json }
 
       before do
@@ -64,10 +64,13 @@ RSpec.describe SyncSeatLinkRequestWorker, type: :worker do
         context 'when it is a cloud license' do
           let(:current_license) { create(:license, cloud: true) }
 
-          it 'persists the new license and deletes the current one' do
-            expect { sync_seat_link }.not_to change(License, :count)
+          it 'persists the new license and deletes any existing cloud licenses' do
+            previous_license = create(:license, cloud: true)
+
+            expect { sync_seat_link }.to change(License.cloud, :count).to(1)
+
             expect(License.last).to have_attributes(data: license_key, cloud: true)
-            expect(License).not_to exist(current_license.id)
+            expect(License.cloud).not_to include(previous_license, current_license)
           end
 
           context 'when persisting fails' do
@@ -84,7 +87,10 @@ RSpec.describe SyncSeatLinkRequestWorker, type: :worker do
           context 'when deleting fails' do
             it 'does not create a new license and logs error' do
               last_license = License.last
-              allow(current_license).to receive(:destroy!).and_raise
+
+              relation = instance_double(ActiveRecord::Relation)
+              allow(License).to receive(:cloud).and_return(relation)
+              allow(relation).to receive(:delete_all).and_raise
 
               expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original