WIP: Fix scoped buildpack order

Co-authored-by: Nicolas Bender <[email protected]>

Author: 2 people

app/models/runtime/buildpack.rb

@@ -1,11 +1,6 @@
 module VCAP::CloudController
   class Buildpack < Sequel::Model
-    plugin :list
-    plugin :single_table_inheritance, :lifecycle,
-      model_map: {
-        VCAP::CloudController::Lifecycles::BUILDPACK => "VCAP::CloudController::ClassicBuildpack",
-        VCAP::CloudController::Lifecycles::CNB => "VCAP::CloudController::CNBBuildpack"
-      }
+    plugin :list, scope: :lifecycle
 
     export_attributes :name, :stack, :position, :enabled, :locked, :filename
     import_attributes :name, :stack, :position, :enabled, :locked, :filename, :key
@@ -25,6 +20,22 @@ def self.user_visibility_filter(_user)
       full_dataset_filter
     end
 
+    def self.list_admin_buildpacks(stack_name=nil, lifecycle='buildpack')
+      scoped = exclude(key: nil).exclude(key: '')
+      scoped = scoped.filter(:lifecycle => lifecycle)
+      if stack_name.present?
+        scoped = scoped.filter(Sequel.or([
+          [:stack, stack_name],
+          [:stack, nil]
+        ]))
+      end
+      scoped.order(:position).all
+    end
+
+    def self.at_last_position
+      where(position: max(:position)).first
+    end
+
     def validate
       validates_unique %i[name stack]
       validates_format(/\A(\w|-)+\z/, :name, message: 'can only contain alphanumeric characters')

app/models/runtime/classic_buildpack.rb

@@ -91,6 +91,7 @@ def ordered_buildpacks
       let(:buildpack_file_1) { Tempfile.new('admin buildpack 1') }
       let(:buildpack_file_2) { Tempfile.new('admin buildpack 2') }
       let(:buildpack_file_3) { Tempfile.new('admin buildpack 3') }
+      let(:buildpack_file_cnb) { Tempfile.new('admin buildpack cnb') }
 
       let(:buildpack_blobstore) { CloudController::DependencyLocator.instance.buildpack_blobstore }
 
@@ -103,13 +104,17 @@ def ordered_buildpacks
         Timecop.return
       end
 
+      subject(:cnb_buildpacks) { Buildpack.list_admin_buildpacks(nil, 'cnb') }
       subject(:all_buildpacks) { Buildpack.list_admin_buildpacks }
 
       context 'with prioritized buildpacks' do
         before do
           buildpack_blobstore.cp_to_blobstore(buildpack_file_1.path, 'a key')
           Buildpack.make(key: 'a key', position: 2)
 
+          buildpack_blobstore.cp_to_blobstore(buildpack_file_cnb.path, 'cnb key')
+          @cnb_buildpack = Buildpack.make(key: 'cnb key', position: 1, lifecycle: 'cnb')
+
           buildpack_blobstore.cp_to_blobstore(buildpack_file_2.path, 'b key')
           Buildpack.make(key: 'b key', position: 1)
 
@@ -131,13 +136,24 @@ def ordered_buildpacks
           expect(all_buildpacks).to have(2).items
         end
 
+        it 'returns the list of cnb buildpacks' do
+          expect(cnb_buildpacks.collect(&:key)).to eq(['cnb key'])
+        end
+
         it 'randomly orders any buildpacks with the same position (for now we did not want to make clever logic of shifting stuff around: up to the user to get it all correct)' do
           @another_buildpack.position = 1
           @another_buildpack.save
 
           expect(all_buildpacks[2].key).to eq('a key')
         end
 
+        it 'does not reorder cnb buildpacks when classical buildpacks change position)' do
+          @cnb_buildpack.position = 1
+          @cnb_buildpack.save
+
+          expect(cnb_buildpacks[0].key).to eq('cnb key')
+        end
+
         context 'and there are buildpacks with null keys' do
           let!(:null_buildpack) { Buildpack.create(name: 'nil_key_custom_buildpack', stack: Stack.make.name, position: 0) }
 
@@ -175,6 +191,7 @@ def ordered_buildpacks
       end
 
       context 'with a stack' do
+        subject(:cnb_buildpacks) { Buildpack.list_admin_buildpacks('stack2', 'cnb') }
         subject(:all_buildpacks) { Buildpack.list_admin_buildpacks('stack1') }
         let!(:stack1) { Stack.make(name: 'stack1') }
         let!(:stack2) { Stack.make(name: 'stack2') }
@@ -186,6 +203,9 @@ def ordered_buildpacks
           buildpack_blobstore.cp_to_blobstore(buildpack_file_2.path, 'b key')
           Buildpack.make(key: 'b key', position: 1, stack: 'stack2')
 
+          buildpack_blobstore.cp_to_blobstore(buildpack_file_cnb.path, 'cnb key')
+          Buildpack.make(key: 'cnb key', position: 1, stack: 'stack2', lifecycle: 'cnb')
+
           buildpack_blobstore.cp_to_blobstore(buildpack_file_3.path, 'c key')
           @another_buildpack = Buildpack.make(key: 'c key', position: 3, stack: nil)
         end
@@ -195,6 +215,11 @@ def ordered_buildpacks
         it 'returns the list in position order, including buildpacks with null stack or matching stacks' do
           expect(all_buildpacks.collect(&:key)).to eq(['a key', 'c key'])
         end
+
+        it 'returns the list of cnb buildpacks' do
+          expect(cnb_buildpacks.collect(&:key)).to eq(['cnb key'])
+        end
+
       end
     end