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.

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 (ORDER BY before UNION is a syntax error)
- Query has DISTINCT ON (requires matching ORDER BY)
- Query is a subquery (outer query handles ordering)
- 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. For SELECT table.* (e.g., many_to_many associations),
it uses unqualified id since only one table's columns are in the result.

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,116 @@
+# frozen_string_literal: true
+
+# This Sequel extension adds a default ORDER BY id to model queries.
+#
+# 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 (ORDER BY before UNION is a syntax error)
+# - Query has DISTINCT ON (requires matching ORDER BY)
+# - Query is a subquery (outer query handles ordering)
+# - 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. For SELECT table.* (e.g., many_to_many associations), it
+# uses unqualified id since only one table's columns are in the result.
+#
+# 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 select_sql
+        id_column = find_id_column
+        if id_column
+          order(id_column).select_sql
+        else
+          super
+        end
+      end
+
+      private
+
+      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] || # ORDER BY before UNION is a syntax error
+          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,78 @@
+# 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 }
+
+  describe 'adding default order' do
+    it 'adds ORDER BY id to model queries' do
+      expect(model_class.dataset.sql).to match(/ORDER BY .id.$/)
+    end
+
+    it 'preserves explicit order' do
+      expect(model_class.dataset.order(:name).sql).to match(/ORDER BY .name.$/)
+    end
+  end
+
+  describe 'skipping default order' do
+    it 'skips for queries with GROUP BY' do
+      expect(model_class.dataset.group(:status).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 = ds1.union(ds2, all: true, from_self: false).sql
+      # ORDER BY before UNION is a syntax error; subsequent datasets are parenthesized so it's harmless there
+      expect(sql).to start_with('SELECT * FROM "organizations" WHERE ("name" = \'a\') UNION')
+    end
+
+    it 'skips for DISTINCT ON queries' do
+      sql = model_class.dataset.distinct(:guid).sql
+      # 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 = model_class.dataset.where(name: 'a').from_self.sql
+      # Outer query should not have ORDER BY - subquery handles it
+      expect(sql).to end_with('AS "t1"')
+    end
+
+    it 'skips for queries where id is not in select list' do
+      expect(model_class.dataset.select(:guid, :name).sql).not_to match(/ORDER BY/)
+    end
+  end
+
+  describe 'handling JOIN queries' do
+    it 'uses qualified column for SELECT * to avoid ambiguity' do
+      sql = model_class.dataset.join(:spaces, organization_id: :id).sql
+      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 = model_class.dataset.select(Sequel::SQL::ColumnAll.new(:organizations)).join(:spaces, organization_id: :id).sql
+      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 = model_class.dataset.select(:organizations__id, :organizations__name).sql
+      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 = model_class.dataset.select(Sequel.as(:organizations__id, :id), :name).sql
+      expect(sql).to match(/ORDER BY .id.$/)
+    end
+  end
+end