Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app/models/runtime/buildpack_lifecycle_data_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions app/models/runtime/cnb_lifecycle_data_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/cloud_controller/db.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/cloud_controller/diego/constants.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'diego/lrp_constants'

module VCAP::CloudController
module Diego
STAGING_DOMAIN = 'cf-app-staging'.freeze
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/diego/reporters/instances_reporter.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require 'utils/workpool'
require 'cloud_controller/diego/constants'
require 'cloud_controller/diego/reporters/reporter_mixins'
require 'diego/lrp_constants'

module VCAP::CloudController
module Diego
Expand Down
134 changes: 134 additions & 0 deletions lib/sequel/extensions/default_order_by_id.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# 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)
return col if id_expression?(col)

return col.alias if col.is_a?(Sequel::SQL::AliasedExpression) && id_expression?(col.expression)

nil
end

def id_expression?(expr)
case expr
when Symbol
expr == :id || expr.to_s.end_with?('__id')
when Sequel::SQL::Identifier
expr.value == :id
when Sequel::SQL::QualifiedIdentifier
expr.column == :id
else
false
end
end
end
end

Dataset.register_extension(:default_order_by_id, DefaultOrderById::DatasetMethods)
end
5 changes: 5 additions & 0 deletions spec/lightweight_db_spec_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require 'lightweight_spec_helper'
require 'sequel'
require 'support/bootstrap/db_connection_string'

DB = Sequel.connect(DbConnectionString.new.to_s)
24 changes: 2 additions & 22 deletions spec/support/bootstrap/db_config.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
require 'cloud_controller/db'
require 'cloud_controller/database_parts_parser'
require_relative 'db_connection_string'

class DbConfig
def initialize(connection_string: ENV.fetch('DB_CONNECTION_STRING', nil), db_type: ENV.fetch('DB', nil))
@connection_string = connection_string || default_connection_string(db_type || 'postgres')
@connection_string = DbConnectionString.new(connection_string: connection_string, db_type: db_type).to_s
initialize_environment_for_cc_spawning
end

Expand Down Expand Up @@ -49,25 +50,4 @@ def self.reset_environment
def initialize_environment_for_cc_spawning
ENV['DB_CONNECTION_STRING'] = connection_string
end

def default_connection_string(db_type)
"#{default_connection_prefix(db_type)}/#{default_name}"
end

def default_connection_prefix(db_type)
default_connection_prefixes = {
'mysql' => ENV['MYSQL_CONNECTION_PREFIX'] || 'mysql2://root:password@localhost:3306',
'postgres' => ENV['POSTGRES_CONNECTION_PREFIX'] || 'postgres://postgres@localhost:5432'
}

default_connection_prefixes[db_type]
end

def default_name
if ENV['TEST_ENV_NUMBER'].presence
"cc_test_#{ENV.fetch('TEST_ENV_NUMBER')}"
else
'cc_test_1'
end
end
end
30 changes: 30 additions & 0 deletions spec/support/bootstrap/db_connection_string.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
class DbConnectionString
def initialize(connection_string: ENV.fetch('DB_CONNECTION_STRING', nil), db_type: ENV.fetch('DB', nil))
@connection_string = connection_string || default_connection_string(db_type || 'postgres')
end

def to_s
@connection_string
end

private

def default_connection_string(db_type)
"#{default_connection_prefix(db_type)}/#{default_name}"
end

def default_connection_prefix(db_type)
{
'mysql' => ENV['MYSQL_CONNECTION_PREFIX'] || 'mysql2://root:password@localhost:3306',
'postgres' => ENV['POSTGRES_CONNECTION_PREFIX'] || 'postgres://postgres@localhost:5432'
}.fetch(db_type)
end

def default_name
if ENV['TEST_ENV_NUMBER'].to_s.empty?
'cc_test_1'
else
"cc_test_#{ENV.fetch('TEST_ENV_NUMBER')}"
end
end
end
Loading
Loading