Add second order_by parameter, as created_at might not be unique

Resources included by decorators are ordered by 'created_at', which only
has a resolution in seconds. This may lead to arbitrary ordering. To
ensure a consistent ordering, 'guid', which has already been utilized by
the SequelPaginator for secondary ordering, has been added to the
decorators as well.

Additional database statements aim to return the oldest or newest
entries from a selection. For those, 'id' has been added as the second
'order_by' parameter.

The specs testing the results of included resources have been improved
to ensure a specific ordering by setting a definitive creation date. In
some instances, the matcher has been changed from 'match_array' to 'eq',
enabling the tests to verify the order of elements.

app/decorators/field_service_instance_broker_decorator.rb

@@ -22,7 +22,7 @@ def decorate(hash, service_instances)
                 join(:service_instances, service_plan_id: :service_plans__id).
                 where(service_instances__id: managed_service_instances.map(&:id)).
                 distinct.
-                order_by(:service_brokers__created_at).
+                order_by(:service_brokers__created_at, :service_brokers__guid).
                 select(:service_brokers__name, :service_brokers__guid, :service_brokers__created_at).
                 all
 

app/decorators/field_service_instance_offering_decorator.rb

@@ -21,7 +21,7 @@ def decorate(hash, service_instances)
                   join(:service_instances, service_plan_id: :service_plans__id).
                   where(service_instances__id: managed_service_instances.map(&:id)).
                   distinct.
-                  order_by(:services__created_at).
+                  order_by(:services__created_at, :services__guid).
                   select(:services__label, :services__guid, :services__description, :services__tags, :services__extra, :services__service_broker_id, :services__created_at).
                   all
 

app/decorators/field_service_instance_organization_decorator.rb

@@ -17,7 +17,7 @@ def decorate(hash, resources)
       spaces = resources.map { |r| r.try(:space) || r }.uniq
       orgs = spaces.map(&:organization).uniq
 
-      hash[:included][:organizations] = orgs.sort_by(&:created_at).map do |org|
+      hash[:included][:organizations] = orgs.sort_by { |o| [o.created_at, o.guid] }.map do |org|
         org_view = {}
         org_view[:name] = org.name if @fields.include?('name')
         org_view[:guid] = org.guid if @fields.include?('guid')

app/decorators/field_service_instance_plan_decorator.rb

@@ -19,7 +19,7 @@ def decorate(hash, service_instances)
       hash[:included] ||= {}
       plans = managed_service_instances.map(&:service_plan).uniq
 
-      hash[:included][:service_plans] = plans.sort_by(&:created_at).map do |plan|
+      hash[:included][:service_plans] = plans.sort_by { |p| [p.created_at, p.guid] }.map do |plan|
         plan_view = {}
         plan_view[:guid] = plan.guid if @fields.include?('guid')
         plan_view[:name] = plan.name if @fields.include?('name')

app/decorators/field_service_instance_space_decorator.rb

@@ -17,7 +17,7 @@ def decorate(hash, resources)
 
       spaces = resources.map { |r| r.try(:space) || r }.uniq
 
-      hash[:included][:spaces] = spaces.sort_by(&:created_at).map do |space|
+      hash[:included][:spaces] = spaces.sort_by { |s| [s.created_at, s.guid] }.map do |space|
         temp = {}
         temp[:guid] = space.guid if @fields.include?('guid')
         temp[:name] = space.name if @fields.include?('name')

app/decorators/field_service_offering_service_broker_decorator.rb

@@ -15,7 +15,7 @@ def initialize(fields)
     def decorate(hash, service_offerings)
       hash[:included] ||= {}
       service_brokers = service_offerings.map(&:service_broker).uniq
-      hash[:included][:service_brokers] = service_brokers.sort_by(&:created_at).map do |broker|
+      hash[:included][:service_brokers] = service_brokers.sort_by { |sb| [sb.created_at, sb.guid] }.map do |broker|
         broker_view = {}
         broker_view[:name] = broker.name if @fields.include?('name')
         broker_view[:guid] = broker.guid if @fields.include?('guid')

app/decorators/field_service_plan_service_broker_decorator.rb

@@ -16,7 +16,7 @@ def decorate(hash, service_plans)
       hash[:included] ||= {}
       service_offerings = service_plans.map(&:service).uniq
       service_brokers = service_offerings.map(&:service_broker).uniq
-      hash[:included][:service_brokers] = service_brokers.sort_by(&:created_at).map do |broker|
+      hash[:included][:service_brokers] = service_brokers.sort_by { |sb| [sb.created_at, sb.guid] }.map do |broker|
         broker_view = {}
         broker_view[:name] = broker.name if @fields.include?('name')
         broker_view[:guid] = broker.guid if @fields.include?('guid')

app/decorators/include_binding_app_decorator.rb

@@ -19,7 +19,7 @@ def apps(bindings)
         app_guids = bindings.pluck(:app_guid).compact.uniq
         return if app_guids.empty?
 
-        AppModel.where(guid: app_guids).order(:created_at).
+        AppModel.where(guid: app_guids).order(:created_at, :guid).
           eager(Presenters::V3::AppPresenter.associated_resources).all
       end
     end

app/decorators/include_binding_route_decorator.rb

@@ -20,7 +20,7 @@ def match?(include_params)
 
         def routes(route_bindings)
           route_ids = route_bindings.map(&:route_id)
-          routes = Route.where(id: route_ids).order_by(:created_at).
+          routes = Route.where(id: route_ids).order_by(:created_at, :guid).
                    eager(Presenters::V3::RoutePresenter.associated_resources).all
           routes.map { |route| Presenters::V3::RoutePresenter.new(route).to_hash }
         end

app/decorators/include_binding_service_instance_decorator.rb

@@ -18,7 +18,7 @@ def decorate(hash, bindings)
       private
 
       def service_instances(bindings)
-        ServiceInstance.where(guid: bindings.map(&:service_instance_guid).uniq).order(:created_at).
+        ServiceInstance.where(guid: bindings.map(&:service_instance_guid).uniq).order(:created_at, :guid).
           eager(Presenters::V3::ServiceInstancePresenter.associated_resources).all
       end
     end

app/decorators/include_organization_decorator.rb

@@ -8,7 +8,7 @@ def match?(include)
       def decorate(hash, resources)
         hash[:included] ||= {}
         organization_ids = resources.map { |r| r.space.organization_id }.uniq
-        organizations = Organization.where(id: organization_ids).order(:created_at).
+        organizations = Organization.where(id: organization_ids).order(:created_at, :guid).
                         eager(Presenters::V3::OrganizationPresenter.associated_resources).all
 
         hash[:included][:organizations] = organizations.map { |organization| Presenters::V3::OrganizationPresenter.new(organization).to_hash }

app/decorators/include_role_organization_decorator.rb

@@ -11,7 +11,7 @@ def decorate(hash, roles)
         hash[:included] ||= {}
         organization_ids = roles.reject(&:for_space?).map(&:organization_id).uniq
         unless organization_ids.empty?
-          organizations = Organization.where(id: organization_ids).order(:created_at).
+          organizations = Organization.where(id: organization_ids).order(:created_at, :guid).
                           eager(Presenters::V3::OrganizationPresenter.associated_resources).all
         end
 

app/decorators/include_role_space_decorator.rb

@@ -9,7 +9,7 @@ def decorate(hash, roles)
         hash[:included] ||= {}
         space_ids = roles.select(&:for_space?).map(&:space_id).uniq
         unless space_ids.empty?
-          spaces = Space.where(id: space_ids).order(:created_at).
+          spaces = Space.where(id: space_ids).order(:created_at, :guid).
                    eager(Presenters::V3::SpacePresenter.associated_resources).all
         end
 

app/decorators/include_role_user_decorator.rb

@@ -8,7 +8,7 @@ def match?(include_params)
       def decorate(hash, roles)
         hash[:included] ||= {}
         user_guids = roles.map(&:user_guid).uniq
-        users = User.where(guid: user_guids).order(:created_at).
+        users = User.where(guid: user_guids).order(:created_at, :guid).
                 eager(Presenters::V3::UserPresenter.associated_resources).all
         uaa_users = User.uaa_users_info(user_guids)
 

app/decorators/include_service_plan_service_offering_decorator.rb

@@ -7,7 +7,7 @@ def match?(include)
 
       def decorate(hash, service_plans)
         hash[:included] ||= {}
-        service_offerings = Service.where(id: service_plans.map(&:service_id).uniq).order(:created_at).
+        service_offerings = Service.where(id: service_plans.map(&:service_id).uniq).order(:created_at, :guid).
                             eager(Presenters::V3::ServiceOfferingPresenter.associated_resources).all
 
         hash[:included][:service_offerings] = service_offerings.map do |service_offering|

app/decorators/include_service_plan_space_organization_decorator.rb

@@ -9,9 +9,9 @@ def decorate(hash, service_plans)
         hash[:included] ||= {}
         space_ids = service_plans.map { |p| p.service.service_broker.space_id }.compact.uniq
         unless space_ids.empty?
-          spaces = Space.where(id: space_ids).order(:created_at).
+          spaces = Space.where(id: space_ids).order(:created_at, :guid).
                    eager(Presenters::V3::SpacePresenter.associated_resources).all
-          orgs = Organization.where(id: spaces.map(&:organization_id).uniq).order(:created_at).
+          orgs = Organization.where(id: spaces.map(&:organization_id).uniq).order(:created_at, :guid).
                  eager(Presenters::V3::OrganizationPresenter.associated_resources).all
         end
 

app/decorators/include_space_decorator.rb

@@ -8,7 +8,7 @@ def match?(include)
       def decorate(hash, resources)
         hash[:included] ||= {}
         space_guids = resources.map(&:space_guid).uniq
-        spaces_query = Space.where(guid: space_guids).order(:created_at).
+        spaces_query = Space.where(guid: space_guids).order(:created_at, :guid).
                        eager(Presenters::V3::SpacePresenter.associated_resources)
         spaces_query = with_readable_space_guids(spaces_query)
 

app/decorators/include_space_organization_decorator.rb

@@ -8,7 +8,7 @@ def match?(include)
       def decorate(hash, spaces)
         hash[:included] ||= {}
         organization_ids = spaces.map(&:organization_id).uniq
-        organizations = Organization.where(id: organization_ids).order(:created_at).
+        organizations = Organization.where(id: organization_ids).order(:created_at, :guid).
                         eager(Presenters::V3::OrganizationPresenter.associated_resources).all
 
         hash[:included][:organizations] = organizations.map { |organization| Presenters::V3::OrganizationPresenter.new(organization).to_hash }

app/jobs/runtime/prune_completed_builds.rb

@@ -20,7 +20,7 @@ def perform
             builds_dataset = BuildModel.where(app_guid:)
 
             builds_to_keep = builds_dataset.
-                             order(Sequel.desc(:created_at)).
+                             order(Sequel.desc(:created_at), Sequel.desc(:id)).
                              limit(max_retained_builds_per_app).
                              select(:id)
 

app/jobs/runtime/prune_completed_deployments.rb

@@ -20,7 +20,7 @@ def perform
             deployments_dataset = DeploymentModel.where(app_guid:)
 
             deployments_to_keep = deployments_dataset.
-                                  order(Sequel.desc(:created_at)).
+                                  order(Sequel.desc(:created_at), Sequel.desc(:id)).
                                   limit(max_retained_deployments_per_app).
                                   select(:id)
 

app/jobs/runtime/prune_excess_app_revisions.rb

@@ -16,7 +16,7 @@ def perform
             revision_dataset = RevisionModel.where(app_guid:)
             next if revision_dataset.count <= max_retained_revisions_per_app
 
-            revisions_to_keep = revision_dataset.order(Sequel.desc(:created_at)).
+            revisions_to_keep = revision_dataset.order(Sequel.desc(:created_at), Sequel.desc(:id)).
                                 limit(max_retained_revisions_per_app).
                                 select(:id)
 

app/presenters/v3/route_presenter.rb

@@ -1,5 +1,5 @@
 require 'presenters/v3/base_presenter'
-require 'presenters/v3/route_destination_presenter'
+require 'presenters/v3/route_destinations_presenter'
 require 'presenters/mixins/metadata_presentation_helpers'
 require 'presenters/helpers/censorship'
 

lib/cloud_controller/bits_expiration.rb

@@ -58,7 +58,7 @@ def enqueue_package_delete_job(package_guid)
     end
 
     def filter_non_expirable(dataset, storage_count)
-      data_to_keep = dataset.order_by(Sequel.desc(:created_at)).limit(storage_count).select(:id)
+      data_to_keep = dataset.order_by(Sequel.desc(:created_at), Sequel.desc(:id)).limit(storage_count).select(:id)
       dataset.exclude(id: data_to_keep)
     end
   end

lib/cloud_controller/deployment_updater/updater.rb

@@ -97,7 +97,7 @@ def deploying_web_process
       end
 
       def oldest_web_process_with_instances
-        @oldest_web_process_with_instances ||= app.web_processes.select { |process| process.instances > 0 }.min_by(&:created_at)
+        @oldest_web_process_with_instances ||= app.web_processes.select { |process| process.instances > 0 }.min_by { |p| [p.created_at, p.id] }
       end
 
       def interim_web_process

spec/request/roles_spec.rb

@@ -4,8 +4,8 @@
 RSpec.describe 'Roles Request' do
   let(:user) { VCAP::CloudController::User.make(guid: 'user_guid') }
   let(:admin_header) { admin_headers_for(user) }
-  let(:org) { VCAP::CloudController::Organization.make(guid: 'big-org') }
-  let(:space) { VCAP::CloudController::Space.make(guid: 'big-space', organization: org) }
+  let(:org) { VCAP::CloudController::Organization.make(guid: 'big-org', created_at: Time.now.utc - 1.second) }
+  let(:space) { VCAP::CloudController::Space.make(guid: 'big-space', organization: org, created_at: Time.now.utc - 1.second) }
   let(:user_with_role) { VCAP::CloudController::User.make(guid: 'user_with_role') }
   let(:user_guid) { user.guid }
   let(:space_guid) { space.guid }
@@ -822,7 +822,7 @@
 
   describe 'GET /v3/roles' do
     let(:api_call) { ->(user_headers) { get '/v3/roles', nil, user_headers } }
-    let(:other_user) { VCAP::CloudController::User.make(guid: 'other-user-guid') }
+    let(:other_user) { VCAP::CloudController::User.make(guid: 'other-user-guid', created_at: Time.now.utc - 1.second) }
 
     let!(:space_auditor) do
       VCAP::CloudController::SpaceAuditor.make(

spec/request/service_credential_bindings_spec.rb

@@ -55,13 +55,17 @@
     end
 
     context 'given a mixture of bindings' do
-      let(:now) { Time.now }
-      let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space:) }
+      let(:now) { Time.now.utc }
+      let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, created_at: now - 1.second) }
       let(:other_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: other_space) }
       let!(:key_binding) { VCAP::CloudController::ServiceKey.make(service_instance: instance, created_at: now - 4.seconds) }
       let!(:other_key_binding) { VCAP::CloudController::ServiceKey.make(service_instance: other_instance, created_at: now - 3.seconds) }
       let!(:app_binding) do
-        VCAP::CloudController::ServiceBinding.make(service_instance: instance, created_at: now - 2.seconds).tap do |binding|
+        VCAP::CloudController::ServiceBinding.make(
+          app: VCAP::CloudController::AppModel.make(space: space, created_at: now - 1.second),
+          service_instance: instance,
+          created_at: now - 2.seconds
+        ).tap do |binding|
           operate_on(binding)
         end
       end

spec/request/service_instances_spec.rb

@@ -3,9 +3,9 @@
 
 RSpec.describe 'V3 service instances' do
   let(:user) { VCAP::CloudController::User.make }
-  let(:org) { VCAP::CloudController::Organization.make }
+  let(:org) { VCAP::CloudController::Organization.make(created_at: Time.now.utc - 1.second) }
   let!(:org_annotation) { VCAP::CloudController::OrganizationAnnotationModel.make(key_prefix: 'pre.fix', key_name: 'foo', value: 'bar', resource_guid: org.guid) }
-  let(:space) { VCAP::CloudController::Space.make(organization: org) }
+  let(:space) { VCAP::CloudController::Space.make(organization: org, created_at: Time.now.utc - 1.second) }
   let!(:space_annotation) { VCAP::CloudController::SpaceAnnotationModel.make(key_prefix: 'pre.fix', key_name: 'baz', value: 'wow', space: space) }
   let(:another_space) { VCAP::CloudController::Space.make }
 
@@ -189,8 +189,30 @@
     end
 
     context 'given a mixture of managed, user-provided and shared service instances' do
-      let!(:msi_1) { VCAP::CloudController::ManagedServiceInstance.make(space:) }
-      let!(:msi_2) { VCAP::CloudController::ManagedServiceInstance.make(space: another_space) }
+      let!(:msi_1) do
+        VCAP::CloudController::ManagedServiceInstance.make(
+          service_plan: VCAP::CloudController::ServicePlan.make(
+            service: VCAP::CloudController::Service.make(
+              service_broker: VCAP::CloudController::ServiceBroker.make(created_at: Time.now.utc - 2.seconds),
+              created_at: Time.now.utc - 2.seconds
+            ),
+            created_at: Time.now.utc - 2.seconds
+          ),
+          space: space
+        )
+      end
+      let!(:msi_2) do
+        VCAP::CloudController::ManagedServiceInstance.make(
+          service_plan: VCAP::CloudController::ServicePlan.make(
+            service: VCAP::CloudController::Service.make(
+              service_broker: VCAP::CloudController::ServiceBroker.make(created_at: Time.now.utc - 1.second),
+              created_at: Time.now.utc - 1.second
+            ),
+            created_at: Time.now.utc - 1.second
+          ),
+          space: another_space
+        )
+      end
       let!(:upsi_1) { VCAP::CloudController::UserProvidedServiceInstance.make(space:) }
       let!(:upsi_2) { VCAP::CloudController::UserProvidedServiceInstance.make(space: another_space) }
       let!(:ssi) { VCAP::CloudController::ManagedServiceInstance.make(space: another_space) }

spec/request/service_offerings_spec.rb

@@ -601,7 +601,11 @@
     end
 
     describe 'fields' do
-      let!(:service_1) { VCAP::CloudController::Service.make }
+      let!(:service_1) do
+        VCAP::CloudController::Service.make(
+          service_broker: VCAP::CloudController::ServiceBroker.make(created_at: Time.now.utc - 1.second)
+        )
+      end
       let!(:service_2) { VCAP::CloudController::Service.make }
 
       it 'can include service broker name and guid' do

spec/request/service_plans_spec.rb

@@ -558,7 +558,12 @@
     end
 
     describe 'includes' do
-      let(:space_1) { VCAP::CloudController::Space.make }
+      let(:space_1) do
+        VCAP::CloudController::Space.make(
+          organization: VCAP::CloudController::Organization.make(created_at: Time.now.utc - 1.second),
+          created_at: Time.now.utc - 1.second
+        )
+      end
       let(:space_2) { VCAP::CloudController::Space.make }
 
       context 'when including `space.organization`' do
@@ -608,7 +613,13 @@
     end
 
     describe 'fields' do
-      let!(:plan_1) { VCAP::CloudController::ServicePlan.make }
+      let!(:plan_1) do
+        VCAP::CloudController::ServicePlan.make(
+          service: VCAP::CloudController::Service.make(
+            service_broker: VCAP::CloudController::ServiceBroker.make(created_at: Time.now.utc - 1.second)
+          )
+        )
+      end
       let!(:plan_2) { VCAP::CloudController::ServicePlan.make }
 
       it 'can include service broker name and guid' do

spec/request/service_route_bindings_spec.rb

@@ -215,7 +215,7 @@
 
     describe 'include' do
       context 'when including `service_instance`' do
-        let(:instance) { VCAP::CloudController::UserProvidedServiceInstance.make(:routing) }
+        let(:instance) { VCAP::CloudController::UserProvidedServiceInstance.make(:routing, created_at: Time.now.utc - 1.second) }
         let(:other_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(:routing) }
 
         before do
@@ -244,7 +244,7 @@
       end
 
       it 'can include `route`' do
-        route = VCAP::CloudController::Route.make
+        route = VCAP::CloudController::Route.make(created_at: Time.now.utc - 1.second)
         other_route = VCAP::CloudController::Route.make
 
         si = VCAP::CloudController::ManagedServiceInstance.make(:routing, space: route.space)

spec/unit/actions/revision_create_spec.rb

@@ -112,7 +112,7 @@ module VCAP::CloudController
             )
           end.to change { RevisionModel.where(app:).count }.by(1)
 
-          expect(RevisionModel.order_by(:created_at).map(&:version)).to eq([1, 2])
+          expect(RevisionModel.order_by(:created_at, :id).map(&:version)).to eq([1, 2])
         end
 
         it 'rolls over to version 1 when we pass version 9999' do
@@ -130,7 +130,7 @@ module VCAP::CloudController
             commands_by_process_type: { 'web' => 'run my app' },
             user_audit_info: user_audit_info
           )
-          expect(RevisionModel.order_by(:created_at).map(&:version)).to eq([2, 9998, 9999, 1])
+          expect(RevisionModel.order_by(:created_at, :id).map(&:version)).to eq([2, 9998, 9999, 1])
         end
 
         it 'replaces any existing revisions after rolling over' do
@@ -148,7 +148,7 @@ module VCAP::CloudController
             commands_by_process_type: { 'web' => 'run my app' },
             user_audit_info: user_audit_info
           )
-          expect(RevisionModel.order_by(:created_at).map(&:version)).to eq([9998, 9999, 1, 2])
+          expect(RevisionModel.order_by(:created_at, :id).map(&:version)).to eq([9998, 9999, 1, 2])
         end
       end
     end