Skip to content

Commit e3f4ca5

Browse files
committed
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.
1 parent ce02452 commit e3f4ca5

4 files changed

Lines changed: 72 additions & 1 deletion

File tree

lib/sequel/extensions/default_order_by_id.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
# Sequel extension that adds default ORDER BY id to model queries.
44
#
5-
# Hooks into fetch methods (all, each, first) to add ORDER BY id just before
5+
# Hooks into fetch methods (all, each, first) and placeholder_literalizer_loader
6+
# (used for optimized association loading) to add ORDER BY id just before
67
# execution. This ensures ordering is only added to the final query, not to
78
# subqueries or compound query parts.
89
#
@@ -47,6 +48,13 @@ def first(*, &)
4748
ds.first(*, &)
4849
end
4950

51+
def placeholder_literalizer_loader(&block)
52+
super do |pl, ds|
53+
result_ds = block.call(pl, ds)
54+
result_ds.send(:default_order_by_id) || result_ds
55+
end
56+
end
57+
5058
private
5159

5260
def default_order_by_id

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,32 @@ def capture_sql(&)
153153
end
154154
end
155155
end
156+
157+
describe 'association loading (placeholder_literalizer_loader)' do
158+
let(:child_class) { Class.new(Sequel::Model(db[:test_default_order_join])) }
159+
160+
it 'adds ORDER BY id to association queries' do
161+
child = child_class
162+
parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
163+
define_singleton_method(:name) { 'TestParent' } # vcap_relations plugin derives reciprocal from self.name
164+
one_to_many :children, class: child, key: :main_id
165+
end
166+
parent = parent_class.new
167+
parent.values[:id] = 1
168+
sql = capture_sql { parent.children }
169+
expect(sql).to match(/ORDER BY .id./)
170+
end
171+
172+
it 'does not override explicit association order' do
173+
child = child_class
174+
parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
175+
define_singleton_method(:name) { 'TestParent' }
176+
one_to_many :children, class: child, key: :main_id, order: Sequel.desc(:id)
177+
end
178+
parent = parent_class.new
179+
parent.values[:id] = 1
180+
sql = capture_sql { parent.children }
181+
expect(sql).to match(/ORDER BY .id. DESC/)
182+
end
183+
end
156184
end

spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,24 @@ module VCAP::CloudController
1111
let(:attr_salt) { :encrypted_buildpack_url_salt }
1212
end
1313

14+
describe 'buildpack_lifecycle_buildpacks association' do
15+
it 'orders by id via the default_order_by_id extension' do
16+
lifecycle_data.save
17+
lifecycle_data.reload
18+
19+
sqls = []
20+
logger = Logger.new(StringIO.new)
21+
logger.define_singleton_method(:info) { |msg| sqls << msg }
22+
BuildpackLifecycleDataModel.db.loggers << logger
23+
24+
lifecycle_data.buildpack_lifecycle_buildpacks
25+
26+
BuildpackLifecycleDataModel.db.loggers.delete(logger)
27+
sql = sqls.find { |s| s.include?('buildpack_lifecycle_buildpacks') }
28+
expect(sql).to match(/ORDER BY .id./)
29+
end
30+
end
31+
1432
describe '#stack' do
1533
it 'persists the stack' do
1634
lifecycle_data.stack = 'cflinuxfs4'

spec/unit/models/runtime/cnb_lifecycle_data_model_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,23 @@ module VCAP::CloudController
44
RSpec.describe CNBLifecycleDataModel do
55
subject(:lifecycle_data) { CNBLifecycleDataModel.make([]) }
66

7+
describe 'buildpack_lifecycle_buildpacks association' do
8+
it 'orders by id via the default_order_by_id extension' do
9+
lifecycle_data.reload
10+
11+
sqls = []
12+
logger = Logger.new(StringIO.new)
13+
logger.define_singleton_method(:info) { |msg| sqls << msg }
14+
CNBLifecycleDataModel.db.loggers << logger
15+
16+
lifecycle_data.buildpack_lifecycle_buildpacks
17+
18+
CNBLifecycleDataModel.db.loggers.delete(logger)
19+
sql = sqls.find { |s| s.include?('buildpack_lifecycle_buildpacks') }
20+
expect(sql).to match(/ORDER BY .id./)
21+
end
22+
end
23+
724
describe '#stack' do
825
it 'persists the stack' do
926
lifecycle_data.stack = 'cflinuxfs4'

0 commit comments

Comments
 (0)