X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/b2979b0db0b6b5ee7ad3001ffcc8f76ff4d96f05..b60e52e031eeba96ef1a301660de2f8cec4518b6:/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 9a54abe4d0..3465a78320 100644 --- a/services/api/app/controllers/application_controller.rb +++ b/services/api/app/controllers/application_controller.rb @@ -20,12 +20,11 @@ class ApplicationController < ActionController::Base include LoadParam include RecordFilters - ERROR_ACTIONS = [:render_error, :render_not_found] - - respond_to :json protect_from_forgery + ERROR_ACTIONS = [:render_error, :render_not_found] + before_filter :respond_with_json_by_default before_filter :remote_ip before_filter :load_read_auths @@ -46,6 +45,17 @@ class ApplicationController < ActionController::Base attr_accessor :resource_attrs + begin + rescue_from(Exception, + ArvadosModel::PermissionDeniedError, + :with => :render_error) + rescue_from(ActiveRecord::RecordNotFound, + ActionController::RoutingError, + ActionController::UnknownController, + AbstractController::ActionNotFound, + :with => :render_not_found) + end + def index @objects.uniq!(&:id) if @select.nil? or @select.include? "id" if params[:eager] and params[:eager] != '0' and params[:eager] != 0 and params[:eager] != '' @@ -55,12 +65,52 @@ class ApplicationController < ActionController::Base end def show - render json: @object.as_api_response + render json: @object.as_api_response(nil, select: @select) end def create @object = model_class.new resource_attrs - @object.save! + retry_save = true + + if @object.respond_to? :name and params[:ensure_unique_name] + # Record the original name. See below. + name_stem = @object.name + counter = 1 + end + + while retry_save + begin + retry_save = false + @object.save! + rescue ActiveRecord::RecordNotUnique => rn + # Dig into the error to determine if it is specifically calling out a + # (owner_uuid, name) uniqueness violation. In this specific case, and + # the client requested a unique name with ensure_unique_name==true, + # update the name field and try to save again. Loop as necessary to + # discover a unique name. It is necessary to handle name choosing at + # this level (as opposed to the client) to ensure that record creation + # never fails due to a race condition. + if rn.original_exception.is_a? PG::UniqueViolation + # Unfortunately ActiveRecord doesn't abstract out any of the + # necessary information to figure out if this the error is actually + # the specific case where we want to apply the ensure_unique_name + # behavior, so the following code is specialized to Postgres. + err = rn.original_exception + detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL) + if /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail + logger.error "params[:ensure_unique_name] is #{params[:ensure_unique_name]}" + if params[:ensure_unique_name] + counter += 1 + @object.name = "#{name_stem} (#{counter})" + retry_save = true + end + end + end + if not retry_save + raise + end + end + end show end @@ -85,21 +135,6 @@ class ApplicationController < ActionController::Base end end - begin - rescue_from Exception, - :with => :render_error - rescue_from ActiveRecord::RecordNotFound, - :with => :render_not_found - rescue_from ActionController::RoutingError, - :with => :render_not_found - rescue_from ActionController::UnknownController, - :with => :render_not_found - rescue_from AbstractController::ActionNotFound, - :with => :render_not_found - rescue_from ArvadosModel::PermissionDeniedError, - :with => :render_error - end - def render_404_if_no_object render_not_found "Object not found" if !@object end @@ -109,35 +144,55 @@ class ApplicationController < ActionController::Base if e.respond_to? :backtrace and e.backtrace logger.error e.backtrace.collect { |x| x + "\n" }.join('') end - if @object and @object.errors and @object.errors.full_messages and not @object.errors.full_messages.empty? + if (@object.respond_to? :errors and + @object.errors.andand.full_messages.andand.any?) errors = @object.errors.full_messages + logger.error errors.inspect else errors = [e.inspect] end status = e.respond_to?(:http_status) ? e.http_status : 422 - render json: { errors: errors }, status: status + send_error(*errors, status: status) end def render_not_found(e=ActionController::RoutingError.new("Path not found")) logger.error e.inspect - render json: { errors: ["Path not found"] }, status: 404 + send_error("Path not found", status: 404) end protected + def send_error(*args) + if args.last.is_a? Hash + err = args.pop + else + err = {} + end + err[:errors] ||= args + err[:error_token] = [Time.now.utc.to_i, "%08x" % rand(16 ** 8)].join("+") + status = err.delete(:status) || 422 + logger.error "Error #{err[:error_token]}: #{status}" + render json: err, status: status + end + def find_objects_for_index @objects ||= model_class.readable_by(*@read_users) apply_where_limit_order_params end - def apply_where_limit_order_params - ar_table_name = @objects.table_name - - ft = record_filters @filters, ar_table_name + def apply_filters model_class=nil + model_class ||= self.model_class + ft = record_filters @filters, model_class if ft[:cond_out].any? - @objects = @objects.where(ft[:cond_out].join(' AND '), *ft[:param_out]) + @objects = @objects.where('(' + ft[:cond_out].join(') AND (') + ')', + *ft[:param_out]) end + end + + def apply_where_limit_order_params *args + apply_filters *args + ar_table_name = @objects.table_name if @where.is_a? Hash and @where.any? conditions = ['1=1'] @where.each do |attr,value| @@ -191,7 +246,26 @@ class ApplicationController < ActionController::Base end end - @objects = @objects.select(@select.map { |s| "#{table_name}.#{ActiveRecord::Base.connection.quote_column_name s.to_s}" }.join ", ") if @select + if @select + unless action_name.in? %w(create update destroy) + # 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. + api_column_map = model_class.attributes_required_columns + columns_list = @select. + flat_map { |attr| api_column_map[attr] }. + uniq. + map { |s| "#{table_name}.#{ActiveRecord::Base.connection.quote_column_name s}" } + @objects = @objects.select(columns_list.join(", ")) + end + + # This information helps clients understand what they're seeing + # (Workbench always expects it), but they can't select it explicitly + # because it's not an SQL column. Always add it. + # (This is harmless, given that clients can deduce what they're + # looking at by the returned UUID anyway.) + @select |= ["kind"] + end @objects = @objects.order(@orders.join ", ") if @orders.any? @objects = @objects.limit(@limit) @objects = @objects.offset(@offset) @@ -244,12 +318,8 @@ class ApplicationController < ActionController::Base def require_login if not current_user respond_to do |format| - format.json { - render :json => { errors: ['Not logged in'] }.to_json, status: 401 - } - format.html { - redirect_to '/auth/joshid' - } + format.json { send_error("Not logged in", status: 401) } + format.html { redirect_to '/auth/joshid' } end false end @@ -257,14 +327,14 @@ class ApplicationController < ActionController::Base def admin_required unless current_user and current_user.is_admin - render :json => { errors: ['Forbidden'] }.to_json, status: 403 + send_error("Forbidden", status: 403) end end def require_auth_scope if @read_auths.empty? if require_login != false - render :json => { errors: ['Forbidden'] }.to_json, status: 403 + send_error("Forbidden", status: 403) end false end