Add default order by id to one_to_many and many_to_many associations

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(*)
+        ds = with_default_order_if_needed
+        return super if ds.equal?(self)
+
+        ds.all(*)
+      end
+
+      def each(*)
+        ds = with_default_order_if_needed
+        return super if ds.equal?(self)
+
+        ds.each(*)
+      end
+
+      def first(*)
+        ds = with_default_order_if_needed
+        return super if ds.equal?(self)
+
+        ds.first(*)
+      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