Expire packages in create/update hooks (#4111)

* Expire packages in create/update hooks

* ensures that Docker packages are also expired

* Mock BitsExpiration.expire_packages! in package_model tests

* Mock BitsExpiration.expire_packages! in stagings_controller tests

* Test package_model hooks in bits_expiration test

* expire_packages! is now implicitly called by hooks

* Run package update hook also for failed state

* Fix RuboCop warning

Author: jochenehret

app/models/runtime/package_model.rb

@@ -30,6 +30,18 @@ class PackageModel < Sequel::Model(:packages)
 
     set_field_as_encrypted :docker_password, salt: :docker_password_salt, column: :encrypted_docker_password
 
+    def after_create
+      super
+      BitsExpiration.new.expire_packages!(app) if ready?
+    end
+
+    def after_update
+      super
+      return unless column_changed?(:state)
+
+      BitsExpiration.new.expire_packages!(app) if ready? || failed?
+    end
+
     def validate
       validates_max_length 5_000, :docker_password, message: 'can be up to 5,000 characters', allow_nil: true
       validates_includes PACKAGE_STATES, :state, allow_missing: true

lib/cloud_controller/packager/package_upload_handler.rb

@@ -21,8 +21,6 @@ def pack
         end
 
         package.succeed_upload!(checksums)
-
-        VCAP::CloudController::BitsExpiration.new.expire_packages!(package.app)
       ensure
         FileUtils.rm_f(@uploaded_files_path) if @uploaded_files_path
       end

spec/unit/controllers/runtime/stagings_controller_spec.rb

@@ -238,6 +238,7 @@ module VCAP::CloudController
       Fog.unmock!
       TestConfig.override(**staging_config)
       set_current_user_as_admin(user: User.make(guid: '1234'), email: '[email protected]', user_name: 'briggs')
+      allow_any_instance_of(BitsExpiration).to receive(:expire_packages!)
     end
 
     after { FileUtils.rm_rf(workspace) }

spec/unit/lib/cloud_controller/bits_expiration_spec.rb

@@ -161,14 +161,12 @@ module VCAP::CloudController
 
       it 'expires old packages' do
         expiration = BitsExpiration.new
-        expiration.expire_packages!(app)
         num_of_packages_to_keep = expiration.packages_storage_count + 1
         remaining_packages      = PackageModel.where(state: PackageModel::READY_STATE, app_guid: app.guid)
         expect(remaining_packages.count).to eq(num_of_packages_to_keep)
       end
 
       it 'does not delete the package related to the current droplet' do
-        BitsExpiration.new.expire_packages!(app)
         app.reload && @current_package.reload
         expect(@current_package.state).to eq(PackageModel::READY_STATE)
       end
@@ -181,7 +179,7 @@ module VCAP::CloudController
       end
 
       it 'enqueues a job to delete the blob' do
-        expect { BitsExpiration.new.expire_packages!(app) }.to change(Delayed::Job, :count).from(0).to(5)
+        expect(Delayed::Job.count).to eq(5)
         expect(Delayed::Job).to(be_all { |j| j.handler.include?('DeleteExpiredPackageBlob') })
       end
     end

spec/unit/models/runtime/package_model_spec.rb

@@ -2,6 +2,10 @@
 
 module VCAP::CloudController
   RSpec.describe PackageModel do
+    before do
+      allow_any_instance_of(BitsExpiration).to receive(:expire_packages!)
+    end
+
     describe 'validations' do
       it { is_expected.to validates_includes PackageModel::PACKAGE_STATES, :state, allow_missing: true }