Merge branch '334168-keep-changed-cloud-licenses' into 'master'

Create or update a cloud license on sync/activate

See merge request gitlab-org/gitlab!64825

ee/app/models/license.rb

@@ -256,9 +256,8 @@ class License < ApplicationRecord
 
   before_validation :reset_license, if: :data_changed?
 
-  after_create :reset_current
   after_create :update_trial_setting
-  after_destroy :reset_current
+  after_commit :reset_current
   after_commit :reset_future_dated, on: [:create, :destroy]
   after_commit :reset_previous, on: [:create, :destroy]
 
@@ -364,6 +363,13 @@ class License < ApplicationRecord
       yield(current_license) if block_given?
     end
 
+    def current_cloud_license?(key)
+      current_license = License.current
+      return false unless current_license&.cloud_license?
+
+      current_license.data == key
+    end
+
     private
 
     def load_future_dated

ee/app/services/gitlab_subscriptions/activate_service.rb

@@ -17,11 +17,10 @@ module GitlabSubscriptions
 
       return response unless response[:success]
 
-      license = License.new(data: response[:license_key], cloud: true, last_synced_at: Time.current)
+      license = find_or_initialize_cloud_license(response[:license_key])
+      license.last_synced_at = Time.current
 
       if license.save
-        License.cloud.id_not_in(license.id).delete_all
-
         { success: true, license: license }
       else
         error(license.errors.full_messages)
@@ -43,5 +42,11 @@ module GitlabSubscriptions
     def application_settings
       Gitlab::CurrentSettings.current_application_settings
     end
+
+    def find_or_initialize_cloud_license(license_key)
+      return License.current.reset if License.current_cloud_license?(license_key)
+
+      License.new(data: license_key, cloud: true)
+    end
   end
 end

ee/app/workers/sync_seat_link_request_worker.rb

@@ -34,10 +34,11 @@ class SyncSeatLinkRequestWorker
 
   private
 
-  def reset_license!(license_data)
-    License.transaction do
-      License.cloud.delete_all
-      License.create!(data: license_data, cloud: true, last_synced_at: Time.current)
+  def reset_license!(license_key)
+    if License.current_cloud_license?(license_key)
+      License.current.reset.touch(:last_synced_at)
+    else
+      License.create!(data: license_key, cloud: true, last_synced_at: Time.current)
     end
   rescue StandardError => e
     Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)

ee/spec/models/license_spec.rb

@@ -342,6 +342,46 @@ RSpec.describe License do
   end
 
   describe 'Callbacks' do
+    describe '#reset_current', :request_store do
+      def current_license_cached_value
+        License.cache.read(License::CACHE_KEY, License)
+      end
+
+      before do
+        License.current # Set cache up front
+      end
+
+      context 'when a license is created' do
+        it 'expires the current_license cached value' do
+          expect(current_license_cached_value).to be_present
+
+          create(:license)
+
+          expect(current_license_cached_value).to be_nil
+        end
+      end
+
+      context 'when a license is updated' do
+        it 'expires the current_license cached value' do
+          expect(current_license_cached_value).to be_present
+
+          License.last.update!(updated_at: Time.current)
+
+          expect(current_license_cached_value).to be_nil
+        end
+      end
+
+      context 'when a license is destroyed' do
+        it 'expires the current_license cached value' do
+          expect(current_license_cached_value).to be_present
+
+          License.last.destroy!
+
+          expect(current_license_cached_value).to be_nil
+        end
+      end
+    end
+
     describe '#reset_future_dated', :request_store do
       let!(:future_dated_license) { create(:license, data: create(:gitlab_license, starts_at: Date.current + 1.month).export) }
 
@@ -767,6 +807,41 @@ RSpec.describe License do
         end
       end
     end
+
+    describe '.current_cloud_license?' do
+      subject { described_class.current_cloud_license?(license_key) }
+
+      let(:license_key) { 'test-key' }
+
+      before do
+        allow(License).to receive(:current).and_return(current_license)
+      end
+
+      context 'when current license is not set' do
+        let(:current_license) { nil }
+
+        it { is_expected.to be(false) }
+      end
+
+      context 'when current license is not a cloud license' do
+        let(:current_license) { create(:license) }
+
+        it { is_expected.to be(false) }
+      end
+
+      context 'when current license is a cloud license but key does not match current' do
+        let(:current_license) { create_current_license(cloud_licensing_enabled: true) }
+
+        it { is_expected.to be(false) }
+      end
+
+      context 'when current license is a cloud license and key matches current' do
+        let(:current_license) { create_current_license(cloud_licensing_enabled: true) }
+        let(:license_key) { current_license.data }
+
+        it { is_expected.to be(true) }
+      end
+    end
   end
 
   describe "#data_filename" do

ee/spec/services/gitlab_subscriptions/activate_service_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe GitlabSubscriptions::ActivateService do
     create(:application_setting, cloud_license_enabled: cloud_license_enabled)
   end
 
-  let_it_be(:license_key) { build(:gitlab_license).export }
+  let_it_be(:license_key) { build(:gitlab_license, :cloud).export }
 
   let(:cloud_license_enabled) { true }
   let(:activation_code) { 'activation_code' }
@@ -35,7 +35,7 @@ RSpec.describe GitlabSubscriptions::ActivateService do
     it 'persists license' do
       freeze_time do
         result = execute_service
-        created_license = License.last
+        created_license = License.current
 
         expect(result).to eq({ success: true, license: created_license })
 
@@ -47,12 +47,40 @@ RSpec.describe GitlabSubscriptions::ActivateService do
       end
     end
 
-    it 'deletes any existing cloud licenses' do
-      previous_1 = create(:license, cloud: true)
-      previous_2 = create(:license, cloud: true)
+    context 'when the current license key does not match the one returned from activation' do
+      it 'creates a new license' do
+        previous_license = create(:license, cloud: true, last_synced_at: 3.days.ago)
+
+        freeze_time do
+          expect { execute_service }.to change(License.cloud, :count).by(1)
+
+          current_license = License.current
+          expect(current_license.id).not_to eq(previous_license.id)
+          expect(current_license).to have_attributes(
+            data: license_key,
+            cloud: true,
+            last_synced_at: Time.current
+          )
+        end
+      end
+    end
 
-      expect { execute_service }.to change(License.cloud, :count).to(1)
-      expect(License.cloud).not_to include(previous_1, previous_2)
+    context 'when the current license key matches the one returned from activation' do
+      it 'reuses the current license and updates the last_synced_at' do
+        create(:license, cloud: true, last_synced_at: 3.days.ago)
+        current_license = create(:license, cloud: true, data: license_key, last_synced_at: 1.day.ago)
+
+        freeze_time do
+          expect { execute_service }.not_to change(License.cloud, :count)
+
+          expect(License.current).to have_attributes(
+            id: current_license.id,
+            data: license_key,
+            cloud: true,
+            last_synced_at: Time.current
+          )
+        end
+      end
     end
 
     context 'when persisting fails' do
@@ -72,7 +100,7 @@ RSpec.describe GitlabSubscriptions::ActivateService do
 
       expect(execute_service).to eq(customer_dot_response)
 
-      expect(License.last&.data).not_to eq(license_key)
+      expect(License.current&.data).not_to eq(license_key)
     end
   end
 

ee/spec/workers/sync_seat_link_request_worker_spec.rb

@@ -41,14 +41,13 @@ RSpec.describe SyncSeatLinkRequestWorker, type: :worker do
           body: body,
           headers: { content_type: 'application/json' }
         )
-        allow(License).to receive(:current).and_return(current_license)
       end
 
       shared_examples 'successful license creation' do
         it 'persists the new license' do
           freeze_time do
             expect { sync_seat_link }.to change(License, :count).by(1)
-            expect(License.last).to have_attributes(
+            expect(License.current).to have_attributes(
               data: license_key,
               cloud: true,
               last_synced_at: Time.current
@@ -58,53 +57,68 @@ RSpec.describe SyncSeatLinkRequestWorker, type: :worker do
       end
 
       context 'when there is no previous license' do
-        let(:current_license) { nil }
+        before do
+          License.delete_all
+        end
 
         it_behaves_like 'successful license creation'
       end
 
       context 'when there is a previous license' do
         context 'when it is a cloud license' do
-          let(:current_license) { create(:license, cloud: true) }
+          context 'when the current license key does not match the one returned from sync' do
+            it 'creates a new license' do
+              freeze_time do
+                current_license = create(:license, cloud: true, last_synced_at: 1.day.ago)
+
+                expect { sync_seat_link }.to change(License.cloud, :count).by(1)
+
+                new_current_license = License.current
+                expect(new_current_license).not_to eq(current_license.id)
+                expect(new_current_license).to have_attributes(
+                  data: license_key,
+                  cloud: true,
+                  last_synced_at: Time.current
+                )
+              end
+            end
+          end
 
-          it 'persists the new license and deletes any existing cloud licenses' do
-            previous_license = create(:license, cloud: true)
+          context 'when the current license key matches the one returned from sync' do
+            it 'reuses the current license and updates the last_synced_at', :request_store do
+              freeze_time do
+                current_license = create(:license, cloud: true, data: license_key, last_synced_at: 1.day.ago)
 
-            expect { sync_seat_link }.to change(License.cloud, :count).to(1)
+                expect { sync_seat_link }.not_to change(License.cloud, :count)
 
-            expect(License.last).to have_attributes(data: license_key, cloud: true)
-            expect(License.cloud).not_to include(previous_license, current_license)
+                expect(License.current).to have_attributes(
+                  id: current_license.id,
+                  data: license_key,
+                  cloud: true,
+                  last_synced_at: Time.current
+                )
+              end
+            end
           end
 
           context 'when persisting fails' do
             let(:license_key) { 'invalid-key' }
 
             it 'does not delete the current license and logs error' do
+              current_license = License.current
               expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
 
               expect { sync_seat_link }.to raise_error
-              expect(License).to exist(current_license.id)
-            end
-          end
 
-          context 'when deleting fails' do
-            it 'does not create a new license and logs error' do
-              last_license = License.last
-
-              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
-
-              expect { sync_seat_link }.to raise_error
-              expect(License.last).to eq(last_license)
+              expect(License).to exist(current_license.id)
             end
           end
         end
 
         context 'when it is not a cloud license' do
-          let(:current_license) { create(:license) }
+          before do
+            create(:license)
+          end
 
           it_behaves_like 'successful license creation'
         end