X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/de54cdcea8dca015e3b08bb23f7221faa4814ef0..c2aceca339ec3a6f3d853865cebd0efe348ff518:/services/api/lib/load_param.rb

diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 69d74f4349..7119eb2348 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -1,3 +1,7 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
 # Mixin module for reading out query parameters from request params.
 #
 # Expects:
@@ -6,18 +10,18 @@
 #   @where, @filters, @limit, @offset, @orders
 module LoadParam
 
-  # Default limit on number of rows to return in a single query.
+  # Default number of rows to return in a single query.
   DEFAULT_LIMIT = 100
 
   # Load params[:where] into @where
   def load_where_param
     if params[:where].nil? or params[:where] == ""
       @where = {}
-    elsif params[:where].is_a? Hash
+    elsif [Hash, ActionController::Parameters].include? params[:where].class
       @where = params[:where]
     elsif params[:where].is_a? String
       begin
-        @where = Oj.load(params[:where])
+        @where = SafeJSON.load(params[:where])
         raise unless @where.is_a? Hash
       rescue
         raise ArgumentError.new("Could not parse \"where\" param as an object")
@@ -33,27 +37,26 @@ module LoadParam
       @filters += params[:filters]
     elsif params[:filters].is_a? String and !params[:filters].empty?
       begin
-        f = Oj.load params[:filters]
-        raise unless f.is_a? Array
-        @filters += f
+        f = SafeJSON.load(params[:filters])
+        if not f.nil?
+          raise unless f.is_a? Array
+          @filters += f
+        end
       rescue
         raise ArgumentError.new("Could not parse \"filters\" param as an array")
       end
     end
   end
 
-  def default_orders
-    ["#{table_name}.modified_at desc"]
-  end
-
   # Load params[:limit], params[:offset] and params[:order]
   # into @limit, @offset, @orders
-  def load_limit_offset_order_params
+  def load_limit_offset_order_params(fill_table_names: true)
     if params[:limit]
       unless params[:limit].to_s.match(/^\d+$/)
         raise ArgumentError.new("Invalid value for limit parameter")
       end
-      @limit = params[:limit].to_i
+      @limit = [params[:limit].to_i,
+                Rails.configuration.API.MaxItemsPerResponse].min
     else
       @limit = DEFAULT_LIMIT
     end
@@ -68,19 +71,78 @@ module LoadParam
     end
 
     @orders = []
-    if params[:order]
-      params[:order].split(',').each do |order|
+    if (params[:order].is_a?(Array) && !params[:order].empty?) || !params[:order].blank?
+      od = []
+      (case params[:order]
+       when String
+         if params[:order].starts_with? '['
+           od = SafeJSON.load(params[:order])
+           raise unless od.is_a? Array
+           od
+         else
+           params[:order].split(',')
+         end
+       when Array
+         params[:order]
+       else
+         []
+       end).each do |order|
+        order = order.to_s
         attr, direction = order.strip.split " "
         direction ||= 'asc'
-        if attr.match /^[a-z][_a-z0-9]+$/ and
-            model_class.columns.collect(&:name).index(attr) and
-            ['asc','desc'].index direction.downcase
-          @orders << "#{table_name}.#{attr} #{direction.downcase}"
+        # The attr can have its table unspecified if it happens to be for the current "model_class" (the first case)
+        # or it can be fully specified with the database tablename (the second case) (e.g. "collections.name").
+        # NB that the security check for the second case table_name will not work if the model
+        # has used set_table_name to use an alternate table name from the Rails standard.
+        # I could not find a perfect way to handle this well, but ActiveRecord::Base.send(:descendants)
+        # would be a place to start if this ever becomes necessary.
+        if (attr.match(/^[a-z][_a-z0-9]+$/) &&
+            model_class.columns.collect(&:name).index(attr) &&
+            ['asc','desc'].index(direction.downcase))
+          if fill_table_names
+            @orders << "#{table_name}.#{attr} #{direction.downcase}"
+          else
+            @orders << "#{attr} #{direction.downcase}"
+          end
+        elsif attr.match(/^([a-z][_a-z0-9]+)\.([a-z][_a-z0-9]+)$/) and
+            ['asc','desc'].index(direction.downcase) and
+            ActiveRecord::Base.connection.tables.include?($1) and
+            $1.classify.constantize.columns.collect(&:name).index($2)
+          # $1 in the above checks references the first match from the regular expression, which is expected to be the database table name
+          # $2 is of course the actual database column name
+          @orders << "#{attr} #{direction.downcase}"
         end
       end
     end
-    if @orders.empty?
-      @orders = default_orders
+
+    # If the client-specified orders don't amount to a full ordering
+    # (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.
+    #
+    # Clean up the resulting list of orders such that no column
+    # uselessly appears twice (Postgres might not optimize this out
+    # for us) and no columns uselessly appear after a unique column
+    # (Postgres does not optimize this out for us; as of 9.2, "order
+    # by id, modified_at desc, uuid" is slow but "order by id" is
+    # fast).
+    orders_given_and_default = @orders + model_class.default_orders
+    order_cols_used = {}
+    @orders = []
+    orders_given_and_default.each do |order|
+      otablecol = order.split(' ')[0]
+
+      next if order_cols_used[otablecol]
+      order_cols_used[otablecol] = true
+
+      @orders << order
+
+      otable, ocol = otablecol.split('.')
+      if otable == table_name and model_class.unique_columns.include?(ocol)
+        # we already have a full ordering; subsequent entries would be
+        # superfluous
+        break
+      end
     end
 
     case params[:select]
@@ -88,8 +150,8 @@ module LoadParam
       @select = params[:select]
     when String
       begin
-        @select = Oj.load params[:select]
-        raise unless @select.is_a? Array
+        @select = SafeJSON.load(params[:select])
+        raise unless @select.is_a? Array or @select.nil? or !@select
       rescue
         raise ArgumentError.new("Could not parse \"select\" param as an array")
       end
@@ -99,16 +161,14 @@ module LoadParam
       # Any ordering columns must be selected when doing select,
       # otherwise it is an SQL error, so filter out invaliding orderings.
       @orders.select! { |o|
+        col, _ = o.split
         # match select column against order array entry
-        @select.select { |s| /^#{table_name}.#{s}( (asc|desc))?$/.match o }.any?
+        @select.select { |s| col == "#{table_name}.#{s}" }.any?
       }
     end
 
-    if params[:distinct].is_a? String
-      @distinct = params[:distinct]
-    end
-
+    @distinct = true if (params[:distinct] == true || params[:distinct] == "true")
+    @distinct = false if (params[:distinct] == false || params[:distinct] == "false")
   end
 
-
 end