X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/b69d0344335cafe4f4b6d4af229616b5251d21ea..b5c15775caf865d8fed5d4839d4a082f6518bba4:/services/api/app/controllers/application_controller.rb

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 2644a06579..b1e2a4008f 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -29,6 +29,9 @@ class ApplicationController < ActionController::Base
   include DbCurrentTime
 
   respond_to :json
+
+  # Although CSRF protection is already enabled by default, this is
+  # still needed to reposition CSRF checks later in callback order.
   protect_from_forgery
 
   ERROR_ACTIONS = [:render_error, :render_not_found]
@@ -43,13 +46,15 @@ class ApplicationController < ActionController::Base
 
   before_action :catch_redirect_hint
   before_action :load_required_parameters
-  before_action(:find_object_by_uuid,
-                except: [:index, :create] + ERROR_ACTIONS)
-  before_action(:set_nullable_attrs_to_null, only: [:update, :create])
   before_action :load_limit_offset_order_params, only: [:index, :contents]
+  before_action :load_select_param
+  before_action(:find_object_by_uuid,
+                except: [:index, :create, :update] + ERROR_ACTIONS)
+  before_action :find_object_for_update, only: [:update]
   before_action :load_where_param, only: [:index, :contents]
   before_action :load_filters_param, only: [:index, :contents]
   before_action :find_objects_for_index, :only => :index
+  before_action(:set_nullable_attrs_to_null, only: [:update, :create])
   before_action :reload_object_before_update, :only => :update
   before_action(:render_404_if_no_object,
                 except: [:index, :create] + ERROR_ACTIONS)
@@ -99,7 +104,7 @@ class ApplicationController < ActionController::Base
   end
 
   def show
-    send_json @object.as_api_response(nil, select: @select)
+    send_json @object.as_api_response(nil, select: select_for_klass(@select, model_class))
   end
 
   def create
@@ -118,7 +123,7 @@ class ApplicationController < ActionController::Base
     attrs_to_update = resource_attrs.reject { |k,v|
       [:kind, :etag, :href].index k
     }
-    @object.update_attributes! attrs_to_update
+    @object.update! attrs_to_update
     show
   end
 
@@ -182,7 +187,7 @@ class ApplicationController < ActionController::Base
       if params[pname].is_a?(Boolean)
         return params[pname]
       else
-        logger.warn "Warning: received non-boolean parameter '#{pname}' on #{self.class.inspect}."
+        logger.warn "Warning: received non-boolean value #{params[pname].inspect} for boolean parameter #{pname} on #{self.class.inspect}, treating as false."
       end
     end
     false
@@ -196,7 +201,7 @@ class ApplicationController < ActionController::Base
     end
     err[:errors] ||= args
     err[:errors].map! do |err|
-      err += " (" + Thread.current[:request_id] + ")"
+      err += " (#{request.request_id})"
     end
     err[:error_token] = [Time.now.utc.to_i, "%08x" % rand(16 ** 8)].join("+")
     status = err.delete(:status) || 422
@@ -226,6 +231,24 @@ class ApplicationController < ActionController::Base
     @objects = model_class.apply_filters(@objects, @filters)
   end
 
+  def select_for_klass sel, model_class, raise_unknown=true
+    return nil if sel.nil?
+    # Filter the select fields to only the ones that apply to the
+    # given class.
+    sel.map do |column|
+      sp = column.split(".")
+      if sp.length == 2 && sp[0] == model_class.table_name && model_class.selectable_attributes.include?(sp[1])
+        sp[1]
+      elsif model_class.selectable_attributes.include? column
+        column
+      elsif raise_unknown
+        raise ArgumentError.new("Invalid attribute '#{column}' of #{model_class.name} in select parameter")
+      else
+        nil
+      end
+    end.compact
+  end
+
   def apply_where_limit_order_params model_class=nil
     model_class ||= self.model_class
     apply_filters model_class
@@ -289,7 +312,7 @@ class ApplicationController < ActionController::Base
         # Map attribute names in @select to real column names, resolve
         # those to fully-qualified SQL column names, and pass the
         # resulting string to the select method.
-        columns_list = model_class.columns_for_attributes(@select).
+        columns_list = model_class.columns_for_attributes(select_for_klass @select, model_class).
           map { |s| "#{ar_table_name}.#{ActiveRecord::Base.connection.quote_column_name s}" }
         @objects = @objects.select(columns_list.join(", "))
       end
@@ -304,7 +327,7 @@ class ApplicationController < ActionController::Base
     @objects = @objects.order(@orders.join ", ") if @orders.any?
     @objects = @objects.limit(@limit)
     @objects = @objects.offset(@offset)
-    @objects = @objects.distinct(@distinct) if not @distinct.nil?
+    @objects = @objects.distinct() if @distinct
   end
 
   # limit_database_read ensures @objects (which must be an
@@ -315,7 +338,7 @@ class ApplicationController < ActionController::Base
     return if @limit == 0 || @limit == 1
     model_class ||= self.model_class
     limit_columns = model_class.limit_index_columns_read
-    limit_columns &= model_class.columns_for_attributes(@select) if @select
+    limit_columns &= model_class.columns_for_attributes(select_for_klass @select, model_class) if @select
     return if limit_columns.empty?
     model_class.transaction do
       limit_query = @objects.
@@ -397,7 +420,7 @@ class ApplicationController < ActionController::Base
     if not current_user
       respond_to do |format|
         format.json { send_error("Not logged in", status: 401) }
-        format.html { redirect_to '/auth/joshid' }
+        format.html { redirect_to '/login' }
       end
       false
     end
@@ -419,22 +442,14 @@ class ApplicationController < ActionController::Base
   end
 
   def set_current_request_id
-    req_id = request.headers['X-Request-Id']
-    if !req_id || req_id.length < 1 || req_id.length > 1024
-      # Client-supplied ID is either missing or too long to be
-      # considered friendly.
-      req_id = "req-" + Random::DEFAULT.rand(2**128).to_s(36)[0..19]
-    end
-    response.headers['X-Request-Id'] = Thread.current[:request_id] = req_id
-    Rails.logger.tagged(req_id) do
+    Rails.logger.tagged(request.request_id) do
       yield
     end
-    Thread.current[:request_id] = nil
   end
 
   def append_info_to_payload(payload)
     super
-    payload[:request_id] = response.headers['X-Request-Id']
+    payload[:request_id] = request.request_id
     payload[:client_ipaddr] = @remote_ip
     payload[:client_auth] = current_api_client_authorization.andand.uuid || nil
   end
@@ -471,7 +486,11 @@ class ApplicationController < ActionController::Base
     controller_name
   end
 
-  def find_object_by_uuid
+  def find_object_for_update
+    find_object_by_uuid(with_lock: true)
+  end
+
+  def find_object_by_uuid(with_lock: false)
     if params[:id] and params[:id].match(/\D/)
       params[:uuid] = params.delete :id
     end
@@ -481,8 +500,23 @@ class ApplicationController < ActionController::Base
     @orders = []
     @filters = []
     @objects = nil
+
+    # This is a little hacky but sometimes the fields the user wants
+    # to selecting on are unrelated to the object being loaded here,
+    # for example groups#contents, so filter the fields that will be
+    # used in find_objects_for_index and then reset afterwards.  In
+    # some cases, code that modifies the @select list needs to set
+    # @preserve_select.
+    @preserve_select = @select
+    @select = select_for_klass(@select, self.model_class, false)
+
     find_objects_for_index
-    @object = @objects.first
+    if with_lock && Rails.configuration.API.LockBeforeUpdate
+      @object = @objects.lock.first
+    else
+      @object = @objects.first
+    end
+    @select = @preserve_select
   end
 
   def nullable_attributes
@@ -578,7 +612,7 @@ class ApplicationController < ActionController::Base
       if @objects.respond_to? :except
         list[:items_available] = @objects.
           except(:limit).except(:offset).
-          distinct.count(:id)
+          count(@distinct ? :id : '*')
       end
     when 'none'
     else
@@ -611,7 +645,7 @@ class ApplicationController < ActionController::Base
         # Make sure params[key] is either true or false -- not a
         # string, not nil, etc.
         if not params.include?(key)
-          params[key] = info[:default]
+          params[key] = info[:default] || false
         elsif [false, 'false', '0', 0].include? params[key]
           params[key] = false
         elsif [true, 'true', '1', 1].include? params[key]
@@ -626,6 +660,11 @@ class ApplicationController < ActionController::Base
 
   def self._create_requires_parameters
     {
+      select: {
+        type: 'array',
+        description: "Attributes of the new object to return in the response.",
+        required: false,
+      },
       ensure_unique_name: {
         type: "boolean",
         description: "Adjust name to ensure uniqueness instead of returning an error on (owner_uuid, name) collision.",
@@ -643,7 +682,23 @@ class ApplicationController < ActionController::Base
   end
 
   def self._update_requires_parameters
-    {}
+    {
+      select: {
+        type: 'array',
+        description: "Attributes of the updated object to return in the response.",
+        required: false,
+      },
+    }
+  end
+
+  def self._show_requires_parameters
+    {
+      select: {
+        type: 'array',
+        description: "Attributes of the object to return in the response.",
+        required: false,
+      },
+    }
   end
 
   def self._index_requires_parameters
@@ -651,8 +706,12 @@ class ApplicationController < ActionController::Base
       filters: { type: 'array', required: false },
       where: { type: 'object', required: false },
       order: { type: 'array', required: false },
-      select: { type: 'array', required: false },
-      distinct: { type: 'boolean', required: false },
+      select: {
+        type: 'array',
+        description: "Attributes of each object to return in the response.",
+        required: false,
+      },
+      distinct: { type: 'boolean', required: false, default: false },
       limit: { type: 'integer', required: false, default: DEFAULT_LIMIT },
       offset: { type: 'integer', required: false, default: 0 },
       count: { type: 'string', required: false, default: 'exact' },
@@ -670,11 +729,6 @@ class ApplicationController < ActionController::Base
     }
   end
 
-  def client_accepts_plain_text_stream
-    (request.headers['Accept'].split(' ') &
-     ['text/plain', '*/*']).count > 0
-  end
-
   def render *opts
     if opts.first
       response = opts.first[:json]