3412: Make non-database fields selectable from API server.

This makes it possible to pass method-back field names, like
"data_size" and "files" from Collections, to the select parameter.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 4a1a7d46dba7f1e7e2a765119c398ca96ae74685..d3b5c6b14725d9549deb1423ac443757366b5abe 100644 (file)
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -203,7 +203,17 @@ 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
+      # 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
     @objects = @objects.order(@orders.join ", ") if @orders.any?
     @objects = @objects.limit(@limit)
     @objects = @objects.offset(@offset)

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 1247e365b1fd5f65e86993a75b412eb6c2743ea9..539f69d6cabd5afac1e0d3e4b19a8337bb231897 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -68,6 +68,28 @@ class ArvadosModel < ActiveRecord::Base
     self.columns.select { |col| col.name == attr.to_s }.first
   end
 
+  def self.attributes_required_columns
+    # This method returns a hash.  Each key is the name of an API attribute,
+    # and it's mapped to a list of database columns that must be fetched
+    # to generate that attribute.
+    # This implementation generates a simple map of attributes to
+    # matching column names.  Subclasses can override this method
+    # to specify that method-backed API attributes need to fetch
+    # specific columns from the database.
+    all_columns = columns.map(&:name)
+    api_column_map = Hash.new { |hash, key| hash[key] = [] }
+    methods.grep(/^api_accessible_\w+$/).each do |method_name|
+      next if method_name == :api_accessible_attributes
+      send(method_name).each_pair do |api_attr_name, col_name|
+        col_name = col_name.to_s
+        if all_columns.include?(col_name)
+          api_column_map[api_attr_name.to_s] |= [col_name]
+        end
+      end
+    end
+    api_column_map
+  end
+
   # Return nil if current user is not allowed to see the list of
   # writers. Otherwise, return a list of user_ and group_uuids with
   # write permission. (If not returning nil, current_user is always in

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 50dd42cd1cce1cccaa4c5fa0145e37eea22d583f..ee3b29c5bdc5244523d72aa9a5963e600ebc6ac2 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -12,6 +12,12 @@ class Collection < ArvadosModel
     t.add :manifest_text
   end
 
+  def self.attributes_required_columns
+    super.merge({ "data_size" => ["manifest_text"],
+                  "files" => ["manifest_text"],
+                })
+  end
+
   def redundancy_status
     if redundancy_confirmed_as.nil?
       'unconfirmed'

diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index e4bbd5cd25d0506af16b21b79d02487627fc852f..2996f1b79251efc9367f0ab5a3246d974d6f89f6 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -24,6 +24,29 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
     assert_not_nil assigns(:objects)
   end
 
+  test "can get non-database fields via index select" do
+    authorize_with :active
+    get(:index, filters: [["uuid", "=", collections(:foo_file).uuid]],
+        select: %w(uuid owner_uuid files))
+    assert_response :success
+    assert_equal(1, json_response["items"].andand.size,
+                 "wrong number of items returned for index")
+    assert_equal([[".", "foo", 3]], json_response["items"].first["files"],
+                 "wrong file list in index result")
+  end
+
+  test "can select only non-database fields for index" do
+    authorize_with :active
+    get(:index, select: %w(data_size files))
+    assert_response :success
+    assert(json_response["items"].andand.any?, "no items found in index")
+    json_response["items"].each do |coll|
+      assert_equal(coll["data_size"],
+                   coll["files"].inject(0) { |size, fspec| size + fspec.last },
+                   "mismatch between data size and file list")
+    end
+  end
+
   [0,1,2].each do |limit|
     test "get index with limit=#{limit}" do
       authorize_with :active