X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/29634bb07a1f3c9be44e34b24e71badc4b42a860..421e478d139906b48a2ff31a32a385639eae8e01:/services/api/app/controllers/arvados/v1/groups_controller.rb

diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 6498b7a512..36839a1da0 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -7,6 +7,7 @@ require "trashable"
 class Arvados::V1::GroupsController < ApplicationController
   include TrashableController
 
+  before_action :load_include_param, only: [:shared, :contents]
   skip_before_action :find_object_by_uuid, only: :shared
   skip_before_action :render_404_if_no_object, only: :shared
 
@@ -32,19 +33,18 @@ class Arvados::V1::GroupsController < ApplicationController
     params = _index_requires_parameters.
       merge({
               uuid: {
-                type: 'string', required: false, default: nil,
+                type: 'string', required: false, default: '',
               },
               recursive: {
                 type: 'boolean', required: false, default: false, description: 'Include contents from child groups recursively.',
               },
               include: {
-                type: 'string', required: false, description: 'Include objects referred to by listed field in "included" (only owner_uuid).',
+                type: 'array', required: false, description: 'Include objects referred to by listed fields in "included" response field. Subsets of ["owner_uuid", "container_uuid"] are supported.',
               },
               include_old_versions: {
                 type: 'boolean', required: false, default: false, description: 'Include past collection versions.',
               }
             })
-    params.delete(:select)
     params
   end
 
@@ -91,7 +91,7 @@ class Arvados::V1::GroupsController < ApplicationController
       attrs_to_update = resource_attrs.reject { |k, v|
         [:kind, :etag, :href].index k
       }.merge({async_permissions_update: true})
-      @object.update_attributes!(attrs_to_update)
+      @object.update!(attrs_to_update)
       @object.save!
       render_accepted
     else
@@ -120,6 +120,7 @@ class Arvados::V1::GroupsController < ApplicationController
   end
 
   def contents
+    @orig_select = @select
     load_searchable_objects
     list = {
       :kind => "arvados#objectList",
@@ -127,11 +128,13 @@ class Arvados::V1::GroupsController < ApplicationController
       :self_link => "",
       :offset => @offset,
       :limit => @limit,
-      :items_available => @items_available,
       :items => @objects.as_api_response(nil)
     }
+    if params[:count] != 'none'
+      list[:items_available] = @items_available
+    end
     if @extra_included
-      list[:included] = @extra_included.as_api_response(nil, {select: @select})
+      list[:included] = @extra_included.as_api_response(nil, {select: @orig_select})
     end
     send_json(list)
   end
@@ -146,7 +149,6 @@ class Arvados::V1::GroupsController < ApplicationController
     # This also returns (in the "included" field) the objects that own
     # those projects (users or non-project groups).
     #
-    #
     # The intended use of this endpoint is to support clients which
     # wish to browse those projects which are visible to the user but
     # are not part of the "home" project.
@@ -158,14 +160,22 @@ class Arvados::V1::GroupsController < ApplicationController
 
     apply_where_limit_order_params
 
-    if params["include"] == "owner_uuid"
+    if @include.include?("owner_uuid")
       owners = @objects.map(&:owner_uuid).to_set
-      @extra_included = []
+      @extra_included ||= []
       [Group, User].each do |klass|
         @extra_included += klass.readable_by(*@read_users).where(uuid: owners.to_a).to_a
       end
     end
 
+    if @include.include?("container_uuid")
+      @extra_included ||= []
+      container_uuids = @objects.map { |o|
+        o.respond_to?(:container_uuid) ? o.container_uuid : nil
+      }.compact.to_set.to_a
+      @extra_included += Container.where(uuid: container_uuids).to_a
+    end
+
     index
   end
 
@@ -177,6 +187,19 @@ class Arvados::V1::GroupsController < ApplicationController
 
   protected
 
+  def load_include_param
+    @include = params[:include]
+    if @include.nil? || @include == ""
+      @include = Set[]
+    elsif @include.is_a?(String) && @include.start_with?('[')
+      @include = SafeJSON.load(@include).to_set
+    elsif @include.is_a?(String)
+      @include = Set[@include]
+    else
+      return send_error("'include' parameter must be a string or array", status: 422)
+    end
+  end
+
   def load_searchable_objects
     all_objects = []
     @items_available = 0
@@ -186,6 +209,13 @@ class Arvados::V1::GroupsController < ApplicationController
     # apply to each table being searched, not "groups".
     load_limit_offset_order_params(fill_table_names: false)
 
+    if params['count'] == 'none' and @offset != 0 and (params['last_object_class'].nil? or params['last_object_class'].empty?)
+      # can't use offset without getting counts, so
+      # fall back to count=exact behavior.
+      params['count'] = 'exact'
+      set_count_none = true
+    end
+
     # Trick apply_where_limit_order_params into applying suitable
     # per-table values. *_all are the real ones we'll apply to the
     # aggregate set.
@@ -199,10 +229,7 @@ class Arvados::V1::GroupsController < ApplicationController
 
     request_filters = @filters
 
-    klasses = [Group,
-     Job, PipelineInstance, PipelineTemplate, ContainerRequest, Workflow,
-     Collection,
-     Human, Specimen, Trait]
+    klasses = [Group, ContainerRequest, Workflow, Collection]
 
     table_names = Hash[klasses.collect { |k| [k, k.table_name] }]
 
@@ -240,13 +267,31 @@ class Arvados::V1::GroupsController < ApplicationController
       end
     end
 
+    # Check that any fields in @select are valid for at least one class
+    if @select
+      all_attributes = []
+      klasses.each do |klass|
+        all_attributes.concat klass.selectable_attributes
+      end
+      if klasses.include?(ContainerRequest) && @include.include?("container_uuid")
+        all_attributes.concat Container.selectable_attributes
+      end
+      @select.each do |check|
+        if !all_attributes.include? check
+          raise ArgumentError.new "Invalid attribute '#{check}' in select"
+        end
+      end
+    end
+    any_selections = @select
+
     included_by_uuid = {}
 
     seen_last_class = false
-    klasses.each do |klass|
-      @offset = 0 if seen_last_class  # reset offset for the new next type being processed
+    error_by_class = {}
+    any_success = false
 
-      # if current klass is same as params['last_object_class'], mark that fact
+    klasses.each do |klass|
+      # check if current klass is same as params['last_object_class']
       seen_last_class = true if((params['count'].andand.==('none')) and
                                 (params['last_object_class'].nil? or
                                  params['last_object_class'].empty? or
@@ -255,7 +300,9 @@ class Arvados::V1::GroupsController < ApplicationController
       # if klasses are specified, skip all other klass types
       next if wanted_klasses.any? and !wanted_klasses.include?(klass.to_s)
 
-      # don't reprocess klass types that were already seen
+      # if specified, and count=none, then only look at the klass in
+      # last_object_class.
+      # for whatever reason, this parameter exists separately from 'wanted_klasses'
       next if params['count'] == 'none' and !seen_last_class
 
       # don't process rest of object types if we already have needed number of objects
@@ -268,14 +315,21 @@ class Arvados::V1::GroupsController < ApplicationController
         request_orders.andand.find { |r| r =~ /^#{klass.table_name}\./i || r !~ /\./ } ||
         klass.default_orders.join(", ")
 
-      @select = nil
+      @select = select_for_klass any_selections, klass, false
+
       where_conds = filter_by_owner
-      if klass == Collection
+      if klass == Collection && @select.nil?
         @select = klass.selectable_attributes - ["manifest_text", "unsigned_manifest_text"]
       elsif klass == Group
         where_conds = where_conds.merge(group_class: ["project","filter"])
       end
 
+      # Make signed manifest_text not selectable because controller
+      # currently doesn't know to sign it.
+      if @select
+        @select = @select - ["manifest_text"]
+      end
+
       @filters = request_filters.map do |col, op, val|
         if !col.index('.')
           [col, op, val]
@@ -295,13 +349,33 @@ class Arvados::V1::GroupsController < ApplicationController
         @objects = exclude_home @objects, klass
       end
 
+      # Adjust the limit based on number of objects fetched so far
       klass_limit = limit_all - all_objects.count
       @limit = klass_limit
-      apply_where_limit_order_params klass
+
+      begin
+        apply_where_limit_order_params klass
+      rescue ArgumentError => e
+        if e.inspect =~ /Invalid attribute '.+' for operator '.+' in filter/ or
+          e.inspect =~ /Invalid attribute '.+' for subproperty filter/
+          error_by_class[klass.name] = e
+          next
+        end
+        raise
+      else
+        any_success = true
+      end
+
+      # This actually fetches the objects
       klass_object_list = object_list(model_class: klass)
+
+      # If count=none, :items_available will be nil, and offset is
+      # required to be 0.
       klass_items_available = klass_object_list[:items_available] || 0
       @items_available += klass_items_available
       @offset = [@offset - klass_items_available, 0].max
+
+      # Add objects to the list of objects to be returned.
       all_objects += klass_object_list[:items]
 
       if klass_object_list[:limit] < klass_limit
@@ -311,7 +385,7 @@ class Arvados::V1::GroupsController < ApplicationController
         limit_all = all_objects.count
       end
 
-      if params["include"] == "owner_uuid"
+      if @include.include?("owner_uuid")
         owners = klass_object_list[:items].map {|i| i[:owner_uuid]}.to_set
         [Group, User].each do |ownerklass|
           ownerklass.readable_by(*@read_users).where(uuid: owners.to_a).each do |ow|
@@ -319,19 +393,36 @@ class Arvados::V1::GroupsController < ApplicationController
           end
         end
       end
+
+      if @include.include?("container_uuid") && klass == ContainerRequest
+        containers = klass_object_list[:items].collect { |cr| cr[:container_uuid] }.to_set
+        Container.where(uuid: containers.to_a).each do |c|
+          included_by_uuid[c.uuid] = c
+        end
+      end
     end
 
-    if params["include"]
+    # Only error out when every searchable object type errored out
+    if !any_success && error_by_class.size > 0
+      error_msg = error_by_class.collect do |klass, err|
+        "#{err} on object type #{klass}"
+      end.join("\n")
+      raise ArgumentError.new(error_msg)
+    end
+
+    if !@include.empty?
       @extra_included = included_by_uuid.values
     end
 
+    if set_count_none
+      params['count'] = 'none'
+    end
+
     @objects = all_objects
     @limit = limit_all
     @offset = offset_all
   end
 
-  protected
-
   def exclude_home objectlist, klass
     # select records that are readable by current user AND
     #   the owner_uuid is a user (but not the current user) OR
@@ -350,5 +441,4 @@ class Arvados::V1::GroupsController < ApplicationController
                      "EXISTS(SELECT 1 FROM groups as gp where gp.uuid=#{klass.table_name}.owner_uuid and gp.group_class != 'project')",
                      user_uuid: current_user.uuid)
   end
-
 end