Fix default_order_by_id for association loading (#5089)

Co-authored-by: johha <johannes.haass@sap.com>

Author: philippthun

lib/sequel/extensions/default_order_by_id.rb

@@ -2,7 +2,8 @@
 
 # Sequel extension that adds default ORDER BY id to model queries.
 #
-# Hooks into fetch methods (all, each, first) to add ORDER BY id just before
+# Hooks into fetch methods (all, each, first) and placeholder_literalizer_loader
+# (used for optimized association loading) to add ORDER BY id just before
 # execution. This ensures ordering is only added to the final query, not to
 # subqueries or compound query parts.
 #
@@ -47,6 +48,13 @@ def first(*, &)
         ds.first(*, &)
       end
 
+      def placeholder_literalizer_loader(&block)
+        super do |pl, ds|
+          result_ds = block.call(pl, ds)
+          result_ds.send(:default_order_by_id) || result_ds
+        end
+      end
+
       private
 
       def default_order_by_id

spec/unit/lib/sequel/extensions/default_order_by_id_spec.rb

@@ -153,4 +153,32 @@ def capture_sql(&)
       end
     end
   end
+
+  describe 'association loading (placeholder_literalizer_loader)' do
+    let(:child_class) { Class.new(Sequel::Model(db[:test_default_order_join])) }
+
+    it 'adds ORDER BY id to association queries' do
+      child = child_class
+      parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
+        define_singleton_method(:name) { 'TestParent' } # vcap_relations plugin derives reciprocal from self.name
+        one_to_many :children, class: child, key: :main_id
+      end
+      parent = parent_class.new
+      parent.values[:id] = 1
+      sql = capture_sql { parent.children }
+      expect(sql).to match(/ORDER BY .id./)
+    end
+
+    it 'does not override explicit association order' do
+      child = child_class
+      parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
+        define_singleton_method(:name) { 'TestParent' }
+        one_to_many :children, class: child, key: :main_id, order: Sequel.desc(:id)
+      end
+      parent = parent_class.new
+      parent.values[:id] = 1
+      sql = capture_sql { parent.children }
+      expect(sql).to match(/ORDER BY .id. DESC/)
+    end
+  end
 end

spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb

@@ -11,6 +11,24 @@ module VCAP::CloudController
       let(:attr_salt) { :encrypted_buildpack_url_salt }
     end
 
+    describe 'buildpack_lifecycle_buildpacks association' do
+      it 'orders by id via the default_order_by_id extension' do
+        lifecycle_data.save
+        lifecycle_data.reload
+
+        sqls = []
+        logger = Logger.new(StringIO.new)
+        logger.define_singleton_method(:info) { |msg| sqls << msg }
+        BuildpackLifecycleDataModel.db.loggers << logger
+
+        lifecycle_data.buildpack_lifecycle_buildpacks
+
+        BuildpackLifecycleDataModel.db.loggers.delete(logger)
+        sql = sqls.find { |s| s.include?('buildpack_lifecycle_buildpacks') }
+        expect(sql).to match(/ORDER BY .id./)
+      end
+    end
+
     describe '#stack' do
       it 'persists the stack' do
         lifecycle_data.stack = 'cflinuxfs4'

spec/unit/models/runtime/cnb_lifecycle_data_model_spec.rb

@@ -4,6 +4,23 @@ module VCAP::CloudController
   RSpec.describe CNBLifecycleDataModel do
     subject(:lifecycle_data) { CNBLifecycleDataModel.make([]) }
 
+    describe 'buildpack_lifecycle_buildpacks association' do
+      it 'orders by id via the default_order_by_id extension' do
+        lifecycle_data.reload
+
+        sqls = []
+        logger = Logger.new(StringIO.new)
+        logger.define_singleton_method(:info) { |msg| sqls << msg }
+        CNBLifecycleDataModel.db.loggers << logger
+
+        lifecycle_data.buildpack_lifecycle_buildpacks
+
+        CNBLifecycleDataModel.db.loggers.delete(logger)
+        sql = sqls.find { |s| s.include?('buildpack_lifecycle_buildpacks') }
+        expect(sql).to match(/ORDER BY .id./)
+      end
+    end
+
     describe '#stack' do
       it 'persists the stack' do
         lifecycle_data.stack = 'cflinuxfs4'