From 7af3e9eb9acbf23127dec19d8c2be31b5003075b Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Thu, 7 May 2026 12:58:50 +0200
Subject: [PATCH] Fix default_order_by_id for association loading

Add placeholder_literalizer_loader hook so the extension applies
ORDER BY id to Sequel association queries that use the optimized
loader path.

Add tests to buildpack and CNB lifecycle data model specs verifying
the association is ordered by id via the extension.
---
 lib/sequel/extensions/default_order_by_id.rb  | 10 ++++++-
 .../extensions/default_order_by_id_spec.rb    | 28 +++++++++++++++++++
 .../buildpack_lifecycle_data_model_spec.rb    | 18 ++++++++++++
 .../runtime/cnb_lifecycle_data_model_spec.rb  | 17 +++++++++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/lib/sequel/extensions/default_order_by_id.rb b/lib/sequel/extensions/default_order_by_id.rb
index 5ec36b3fd86..70b0f7bbaeb 100644
--- a/lib/sequel/extensions/default_order_by_id.rb
+++ b/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
diff --git a/spec/unit/lib/sequel/extensions/default_order_by_id_spec.rb b/spec/unit/lib/sequel/extensions/default_order_by_id_spec.rb
index efcb6db28b0..90c15feeb1a 100644
--- a/spec/unit/lib/sequel/extensions/default_order_by_id_spec.rb
+++ b/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
diff --git a/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb b/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb
index d9af5203d63..57cf3463c48 100644
--- a/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb
+++ b/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'
diff --git a/spec/unit/models/runtime/cnb_lifecycle_data_model_spec.rb b/spec/unit/models/runtime/cnb_lifecycle_data_model_spec.rb
index 0ed5c960c26..d5d2cb534f3 100644
--- a/spec/unit/models/runtime/cnb_lifecycle_data_model_spec.rb
+++ b/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'