Add default ORDER BY id to Sequel model queries

Adds a Sequel extension that hooks into fetch methods (all, each, first)
to add ORDER BY id just before execution, ensuring deterministic query
results for consistent API responses and reliable test behavior.

Skips ordering when incompatible (GROUP BY, compounds, DISTINCT ON,
from_self) or unnecessary (explicit ORDER BY, no id column).

Author: philippthun

app/models/runtime/buildpack_lifecycle_data_model.rb

@@ -28,8 +28,7 @@ class BuildpackLifecycleDataModel < Sequel::Model(:buildpack_lifecycle_data)
     one_to_many :buildpack_lifecycle_buildpacks,
                 class: '::VCAP::CloudController::BuildpackLifecycleBuildpackModel',
                 key: :buildpack_lifecycle_data_guid,
-                primary_key: :guid,
-                order: :id
+                primary_key: :guid
     plugin :nested_attributes
     nested_attributes :buildpack_lifecycle_buildpacks, destroy: true
     add_association_dependencies buildpack_lifecycle_buildpacks: :destroy

app/models/runtime/cnb_lifecycle_data_model.rb

@@ -27,8 +27,7 @@ class CNBLifecycleDataModel < Sequel::Model(:cnb_lifecycle_data)
     one_to_many :buildpack_lifecycle_buildpacks,
                 class: '::VCAP::CloudController::BuildpackLifecycleBuildpackModel',
                 key: :cnb_lifecycle_data_guid,
-                primary_key: :guid,
-                order: :id
+                primary_key: :guid
     plugin :nested_attributes
     nested_attributes :buildpack_lifecycle_buildpacks, destroy: true
     add_association_dependencies buildpack_lifecycle_buildpacks: :destroy

lib/cloud_controller/db.rb

@@ -5,6 +5,7 @@
 require 'cloud_controller/execution_context'
 require 'sequel/extensions/query_length_logging'
 require 'sequel/extensions/request_query_metrics'
+require 'sequel/extensions/default_order_by_id'
 
 module VCAP::CloudController
   class DB
@@ -45,6 +46,7 @@ def self.connect(opts, logger)
       add_connection_expiration_extension(db, opts)
       add_connection_validator_extension(db, opts)
       db.extension(:requires_unique_column_names_in_subquery)
+      db.extension(:default_order_by_id)
       add_connection_metrics_extension(db)
       db
     end

lib/sequel/extensions/default_order_by_id.rb

@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+# 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
+# execution. This ensures ordering is only added to the final query, not to
+# subqueries or compound query parts.
+#
+# Skips default ordering when:
+# - Query already has explicit ORDER BY
+# - Query is incompatible (GROUP BY, compounds, DISTINCT ON, from_self)
+# - Query is schema introspection (LIMIT 0)
+# - Model doesn't have id as primary key
+# - id is not in the select list
+#
+# For JOIN queries with SELECT *, uses qualified column (table.id) to avoid
+# ambiguity.
+#
+# Ensures deterministic query results for consistent API responses and
+# reliable test behavior.
+#
+# Usage:
+#   DB.extension(:default_order_by_id)
+#
+module Sequel
+  module DefaultOrderById
+    module DatasetMethods
+      def all(*, &)
+        id_col = id_column_for_order
+        return super unless id_col
+
+        order(id_col).all(*, &)
+      end
+
+      def each(*, &)
+        id_col = id_column_for_order
+        return super unless id_col
+
+        order(id_col).each(*, &)
+      end
+
+      def first(*, &)
+        id_col = id_column_for_order
+        return super unless id_col
+
+        order(id_col).first(*, &)
+      end
+
+      private
+
+      def id_column_for_order
+        return if already_ordered? || incompatible_with_order? || not_a_data_query? || !model_has_id_primary_key?
+
+        find_id_column
+      end
+
+      def already_ordered?
+        opts[:order]
+      end
+
+      def incompatible_with_order?
+        opts[:group] ||       # Aggregated results don't have individual ids
+          opts[:compounds] || # Compound queries (e.g. UNION) have own ordering
+          distinct_on? ||     # DISTINCT ON requires matching ORDER BY
+          from_self?          # Outer query handles ordering
+      end
+
+      def distinct_on?
+        opts[:distinct].is_a?(Array) && opts[:distinct].any?
+      end
+
+      def from_self?
+        opts[:from].is_a?(Array) && opts[:from].any? { |f| f.is_a?(Sequel::SQL::AliasedExpression) }
+      end
+
+      def not_a_data_query?
+        opts[:limit] == 0 # Schema introspection query
+      end
+
+      def model_has_id_primary_key?
+        return false unless respond_to?(:model) && model
+
+        model.primary_key == :id
+      end
+
+      def find_id_column
+        select_cols = opts[:select]
+
+        if select_cols.nil? || select_cols.empty?
+          # SELECT * includes id
+          if opts[:join]
+            # Qualify to avoid ambiguity with joined tables
+            return Sequel.qualify(model.table_name, :id)
+          end
+
+          return :id
+        end
+
+        select_cols.each do |col|
+          # SELECT table.* includes id
+          return :id if col.is_a?(Sequel::SQL::ColumnAll) && col.table == model.table_name
+
+          id_col = extract_id_column(col)
+          return id_col if id_col
+        end
+
+        nil
+      end
+
+      def extract_id_column(col)
+        case col
+        when Symbol
+          col if col == :id || col.to_s.end_with?('__id')
+        when Sequel::SQL::Identifier
+          col if col.value == :id
+        when Sequel::SQL::QualifiedIdentifier
+          col if col.column == :id
+        when Sequel::SQL::AliasedExpression
+          # ORDER BY uses the alias, not the original expression
+          col.alias if col.alias == :id
+        end
+      end
+    end
+  end
+
+  Dataset.register_extension(:default_order_by_id, DefaultOrderById::DatasetMethods)
+end

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

@@ -0,0 +1,121 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'sequel/extensions/default_order_by_id'
+
+RSpec.describe 'Sequel::DefaultOrderById' do
+  let(:model_class) { VCAP::CloudController::Organization }
+  let(:db) { model_class.db }
+
+  def capture_sql(&)
+    sqls = []
+    db.loggers << (logger = Class.new do
+      define_method(:info) { |msg| sqls << msg if msg.include?('SELECT') }
+      define_method(:debug) { |_| }
+      define_method(:error) { |_| }
+    end.new)
+    yield
+    db.loggers.delete(logger)
+    sqls.last
+  end
+
+  describe 'default ordering' do
+    it 'adds ORDER BY id to model queries' do
+      sql = capture_sql { model_class.dataset.first }
+      expect(sql).to match(/ORDER BY .id./)
+    end
+  end
+
+  describe 'already_ordered?' do
+    it 'preserves explicit ORDER BY' do
+      sql = capture_sql { model_class.dataset.order(:name).first }
+      expect(sql).to match(/ORDER BY .name./)
+      expect(sql).not_to match(/ORDER BY .id./)
+    end
+  end
+
+  describe 'incompatible_with_order?' do
+    it 'skips for GROUP BY' do
+      ds = model_class.dataset.group(:status)
+      expect(ds.sql).not_to match(/ORDER BY/)
+    end
+
+    it 'skips for compound queries (UNION)' do
+      ds1 = model_class.dataset.where(name: 'a')
+      ds2 = model_class.dataset.where(name: 'b')
+      sql = capture_sql { ds1.union(ds2, all: true, from_self: false).all }
+      expect(sql).not_to match(/\) ORDER BY/)
+    end
+
+    it 'skips for DISTINCT ON' do
+      sql = capture_sql { model_class.dataset.distinct(:guid).all }
+      expect(sql).not_to match(/ORDER BY/)
+    end
+
+    it 'skips for from_self (subquery)' do
+      sql = capture_sql { model_class.dataset.where(name: 'a').from_self.all }
+      expect(sql).to match(/AS .t1.$/)
+    end
+  end
+
+  describe 'not_a_data_query?' do
+    it 'skips for schema introspection (columns!)' do
+      sql = capture_sql { model_class.dataset.columns! }
+      expect(sql).not_to match(/ORDER BY/)
+    end
+  end
+
+  describe 'model_has_id_primary_key?' do
+    it 'skips for models with non-id primary key' do
+      guid_pk_model = Class.new(Sequel::Model(db[:organizations])) do
+        set_primary_key :guid
+      end
+      sql = capture_sql { guid_pk_model.dataset.first }
+      expect(sql).not_to match(/ORDER BY/)
+    end
+  end
+
+  describe 'find_id_column' do
+    context 'with SELECT *' do
+      it 'uses unqualified :id' do
+        sql = capture_sql { model_class.dataset.first }
+        expect(sql).to match(/ORDER BY .id./)
+        expect(sql).not_to match(/ORDER BY .organizations.\..id./)
+      end
+
+      it 'uses qualified column for JOIN to avoid ambiguity' do
+        sql = capture_sql { model_class.dataset.join(:spaces, organization_id: :id).first }
+        expect(sql).to match(/ORDER BY .organizations.\..id./)
+      end
+    end
+
+    context 'with SELECT table.*' do
+      it 'uses unqualified :id' do
+        sql = capture_sql { model_class.dataset.select(Sequel::SQL::ColumnAll.new(:organizations)).join(:spaces, organization_id: :id).first }
+        expect(sql).to match(/ORDER BY .id./)
+        expect(sql).not_to match(/ORDER BY .organizations.\..id./)
+      end
+    end
+
+    context 'with qualified id in select list' do
+      it 'uses the qualified column' do
+        sql = capture_sql { model_class.dataset.select(:organizations__id, :organizations__name).first }
+        expect(sql).to match(/ORDER BY .organizations.\..id./)
+      end
+    end
+
+    context 'with aliased id in select list' do
+      it 'uses the alias' do
+        sql = capture_sql { model_class.dataset.select(Sequel.as(:organizations__id, :id), :name).first }
+        expect(sql).to match(/ORDER BY .id./)
+      end
+    end
+
+    context 'without id in select list' do
+      it 'skips ordering' do
+        sql = capture_sql { model_class.dataset.select(:guid, :name).all }
+        expect(sql).not_to match(/ORDER BY/)
+      end
+    end
+  end
+end