From 66da04e142bb80ff25d65900292bf99c7e4252f2 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curoverse.com>
Date: Sat, 23 Jan 2016 00:23:49 -0500
Subject: [PATCH] 8289: Do not add fallback orders if client already specified
 an unambiguous order.

---
 services/api/app/models/arvados_model.rb      |  4 +++
 services/api/lib/load_param.rb                |  7 ++++-
 .../test/functional/arvados/v1/query_test.rb  | 26 +++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 services/api/test/functional/arvados/v1/query_test.rb

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 35dd1a94c9..6cd40a4458 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -115,6 +115,10 @@ class ArvadosModel < ActiveRecord::Base
     ["#{table_name}.modified_at desc", "#{table_name}.uuid"]
   end
 
+  def self.unique_columns
+    ["id", "uuid"]
+  end
+
   # If current user can manage the object, return an array of uuids of
   # users and groups that have permission to write the object. The
   # first two elements are always [self.owner_uuid, current user's
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 718aaeaf69..c6c8ddfac4 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -111,7 +111,12 @@ module LoadParam
     # (e.g., [] or ['owner_uuid desc']), fall back on the default
     # orders to ensure repeating the same request (possibly with
     # different limit/offset) will return records in the same order.
-    @orders += model_class.default_orders
+    unless @orders.any? do |order|
+        otable, ocol = order.split(' ')[0].split('.')
+        otable == table_name and model_class.unique_columns.include?(ocol)
+      end
+      @orders += model_class.default_orders
+    end
 
     case params[:select]
     when Array
diff --git a/services/api/test/functional/arvados/v1/query_test.rb b/services/api/test/functional/arvados/v1/query_test.rb
new file mode 100644
index 0000000000..56db54c3a4
--- /dev/null
+++ b/services/api/test/functional/arvados/v1/query_test.rb
@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class Arvados::V1::QueryTest < ActionController::TestCase
+  test 'no fallback orders when order is unambiguous' do
+    @controller = Arvados::V1::LogsController.new
+    authorize_with :active
+    get :index, {
+      order: ['id asc'],
+      controller: 'logs',
+    }
+    assert_response :success
+    assert_equal ['logs.id asc'], assigns(:objects).order_values
+  end
+
+  test 'fallback orders when order is ambiguous' do
+    @controller = Arvados::V1::LogsController.new
+    authorize_with :active
+    get :index, {
+      order: ['event_type asc'],
+      controller: 'logs',
+    }
+    assert_response :success
+    assert_equal('logs.event_type asc, logs.modified_at desc, logs.uuid',
+                 assigns(:objects).order_values.join(', '))
+  end
+end
-- 
2.39.5