Add default ORDER BY id to Sequel model queries

Adds a Sequel extension that automatically appends ORDER BY id to
model queries, ensuring deterministic results for consistent API
responses and reliable test behavior in parallel runs.

The extension hooks into data fetching methods (all, each, first) to
add ORDER BY id just before execution. This ensures the order is only
added to the final query, not to subqueries or UNION parts.

Skips adding the default order when:
- Query already has an explicit ORDER BY clause
- Query has GROUP BY (aggregated results don't have individual ids)
- Query is schema introspection (LIMIT 0)
- Query has UNION/INTERSECT/EXCEPT (compounds)
- Query has DISTINCT ON (requires matching ORDER BY)
- Query is a subquery (from_self)
- Model doesn't have id as primary key
- id is not in the select list

For JOIN queries with SELECT *, it uses a qualified column (table.id)
to avoid ambiguity.

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,138 @@
+# frozen_string_literal: true
+
+# This Sequel extension adds a default ORDER BY id to model queries.
+#
+# It hooks into data fetching methods (all, each, first) to add ORDER BY id
+# just before execution. This ensures the order is only added to the final
+# query, not to subqueries or UNION parts.
+#
+# It skips adding the default order when:
+# - Query already has an explicit ORDER BY clause
+# - Query has GROUP BY (aggregated results don't have individual ids)
+# - Query is schema introspection (LIMIT 0)
+# - Query has UNION/INTERSECT/EXCEPT (compounds)
+# - Query has DISTINCT ON (requires matching ORDER BY)
+# - Query is a subquery (from_self)
+# - Model doesn't have id as primary key
+# - id is not in the select list
+#
+# For JOIN queries with SELECT *, it uses a qualified column (table.id)
+# to avoid ambiguity.
+#
+# This ensures deterministic query results, which is important for:
+# - Consistent API responses
+# - Reliable test behavior (especially in parallel test runs)
+#
+# Usage:
+#   DB.extension(:default_order_by_id)
+#
+module Sequel
+  module DefaultOrderById
+    module DatasetMethods
+      def all(*args, &block)
+        ds = with_default_order_if_needed
+        return super if ds.equal?(self)
+
+        ds.all(*args, &block)
+      end
+
+      def each(*args, &block)
+        ds = with_default_order_if_needed
+        return super if ds.equal?(self)
+
+        ds.each(*args, &block)
+      end
+
+      def first(*args, &block)
+        ds = with_default_order_if_needed
+        return super if ds.equal?(self)
+
+        ds.first(*args, &block)
+      end
+
+      private
+
+      def with_default_order_if_needed
+        id_column = find_id_column
+        return self unless id_column
+
+        order(id_column)
+      end
+
+      def find_id_column
+        return nil if should_skip_default_order?
+
+        find_id_in_select_list
+      end
+
+      def should_skip_default_order?
+        opts[:order] || # Already has explicit order
+          opts[:group] || # Aggregated results don't have individual ids
+          opts[:limit] == 0 || # Schema introspection, not a real query
+          opts[:compounds] || # UNION/INTERSECT/EXCEPT
+          distinct_on? || # DISTINCT ON requires matching ORDER BY
+          subquery? || # Outer query handles ordering
+          !model_has_id_primary_key? # No id column to order by
+      end
+
+      def distinct_on?
+        opts[:distinct].is_a?(Array) && opts[:distinct].any?
+      end
+
+      def subquery?
+        opts[:from].is_a?(Array) && opts[:from].any? { |f| f.is_a?(Sequel::SQL::AliasedExpression) }
+      end
+
+      def model_has_id_primary_key?
+        return false unless respond_to?(:model) && model
+
+        model.primary_key == :id
+      rescue StandardError
+        false
+      end
+
+      def find_id_in_select_list
+        select_cols = opts[:select]
+
+        # SELECT * includes all columns including id
+        if select_cols.nil? || select_cols.empty?
+          return qualified_id_column if opts[:join]
+
+          return :id
+        end
+
+        # Find id column in select list
+        select_cols.each do |col|
+          # ColumnAll (e.g., SELECT table.*) includes all columns including id
+          # Use unqualified :id since only this table's columns are selected
+          return :id if col.is_a?(Sequel::SQL::ColumnAll) && col.table == model.table_name
+
+          # Check for :id, :table__id, or aliased id
+          id_col = extract_id_column(col)
+          return id_col if id_col
+        end
+
+        nil
+      end
+
+      def qualified_id_column
+        Sequel.qualify(model.table_name, :id)
+      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
+          col.alias if col.alias == :id # Use alias symbol (not the full expression with AS)
+        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,98 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'sequel/extensions/default_order_by_id'
+
+RSpec.describe 'Sequel::DefaultOrderById' do
+  # Use an existing model for testing
+  let(:model_class) { VCAP::CloudController::Organization }
+  let(:db) { model_class.db }
+
+  # Capture executed SQL
+  def capture_sql(&block)
+    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)
+    block.call
+    db.loggers.delete(logger)
+    sqls.last
+  end
+
+  describe 'adding default order' 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
+
+    it 'preserves explicit order' 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 'skipping default order' do
+    it 'skips for queries with GROUP BY' do
+      # Just check that SQL is generated correctly, don't execute since it's invalid SQL
+      ds = model_class.dataset.group(:status)
+      expect(ds.sql).not_to match(/ORDER BY/)
+    end
+
+    it 'skips for UNION queries' 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 }
+      # No ORDER BY at the outer level
+      expect(sql).not_to match(/\) ORDER BY/)
+    end
+
+    it 'skips for DISTINCT ON queries' do
+      sql = capture_sql { model_class.dataset.distinct(:guid).all }
+      # DISTINCT ON requires ORDER BY to match the distinct columns
+      expect(sql).not_to match(/ORDER BY/)
+    end
+
+    it 'skips for subqueries (from_self)' do
+      sql = capture_sql { model_class.dataset.where(name: 'a').from_self.all }
+      # Outer query should not have ORDER BY after the subquery alias
+      expect(sql).to match(/AS .t1.$/)
+    end
+
+    it 'skips for queries where id is not in select list' do
+      sql = capture_sql { model_class.dataset.select(:guid, :name).all }
+      expect(sql).not_to match(/ORDER BY/)
+    end
+  end
+
+  describe 'handling JOIN queries' do
+    it 'uses qualified column for SELECT * 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
+
+    it 'uses unqualified column for SELECT table.* (many_to_many pattern)' do
+      # many_to_many associations use SELECT table.* which creates a ColumnAll
+      # Unqualified id is safe here since only one table's columns are selected
+      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
+
+  describe 'handling qualified id columns' do
+    it 'uses qualified column when present in select' do
+      sql = capture_sql { model_class.dataset.select(:organizations__id, :organizations__name).first }
+      expect(sql).to match(/ORDER BY .organizations.\..id./)
+    end
+  end
+
+  describe 'handling aliased id columns' do
+    it 'orders by alias when id is aliased' do
+      sql = capture_sql { model_class.dataset.select(Sequel.as(:organizations__id, :id), :name).first }
+      expect(sql).to match(/ORDER BY .id./)
+    end
+  end
+end