[ARVADOS] updated: 287a99b59415dfad11034057fd608b38d1355512

[Git user]

Summary of changes:
 services/api/app/controllers/application_controller.rb | 15 ---------------
 services/api/app/models/arvados_model.rb               | 15 ++++++++++++++-
 services/api/test/integration/collections_api_test.rb  |  2 +-
 3 files changed, 15 insertions(+), 17 deletions(-)

       via  287a99b59415dfad11034057fd608b38d1355512 (commit)
      from  6c564c9ce38a31df9f14e1988f4065c4854516d8 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 287a99b59415dfad11034057fd608b38d1355512
Author: Lucas Di Pentima <lucas at curoverse.com>
Date:   Tue Aug 16 18:22:04 2016 -0300

    9333: Treating the "attribute is not a string" error case the same as the other invalid attributes. Moved the checks to ArvadosModel. Updated tests.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 89bda3c..3a88818 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -277,21 +277,6 @@ 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.
-        if @select.empty?
-          raise ArgumentError.new("Attribute selection list cannot be empty")
-        end
-        api_column_map = model_class.attributes_required_columns
-        invalid_attrs = []
-        @select.each do |s|
-          next if ["href", "kind", "etag"].include? s
-          if not s.is_a? String
-            raise ArgumentError.new("Attribute '#{s}' should be a string")
-          end
-          invalid_attrs.append(s) if not api_column_map.include? s
-        end
-        if not invalid_attrs.empty?
-          raise ArgumentError.new("Invalid attribute(s): '#{invalid_attrs.join(', ')}'")
-        end
         columns_list = model_class.columns_for_attributes(@select).
           map { |s| "#{ar_table_name}.#{ActiveRecord::Base.connection.quote_column_name s}" }
         @objects = @objects.select(columns_list.join(", "))
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 6cd40a4..0f45f35 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -105,9 +105,22 @@ class ArvadosModel < ActiveRecord::Base
   end
 
   def self.columns_for_attributes(select_attributes)
+    if select_attributes.empty?
+      raise ArgumentError.new("Attribute selection list cannot be empty")
+    end
+    api_column_map = attributes_required_columns
+    invalid_attrs = []
+    select_attributes.each do |s|
+      next if ["href", "kind", "etag"].include? s
+      if not s.is_a? String or not api_column_map.include? s
+        invalid_attrs << s
+      end
+    end
+    if not invalid_attrs.empty?
+      raise ArgumentError.new("Invalid attribute(s): '#{invalid_attrs.inspect}'")
+    end
     # Given an array of attribute names to select, return an array of column
     # names that must be fetched from the database to satisfy the request.
-    api_column_map = attributes_required_columns
     select_attributes.flat_map { |attr| api_column_map[attr] }.uniq
   end
 
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index e67f1b1..0bedc07 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -82,7 +82,7 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
           :select => [['bogus']].to_json
         }, auth(:active)
     assert_response 422
-    assert_match /Attribute.*should be a string/, json_response['errors'].join(' ')
+    assert_match /Invalid attribute.*bogus/, json_response['errors'].join(' ')
   end
 
   test "controller 404 response is json" do

-----------------------------------------------------------------------


hooks/post-receive
--