Merge branch '21666-provision-test-improvement'

[arvados.git] / 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 c1dc7a49612162aa9fcc79211c4358e77005da5b..be73d39dd1b43ba2032d22afdf8837cd17e750bb 100644 (file)
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -46,7 +46,6 @@ class Arvados::V1::GroupsController < ApplicationController
                 type: 'boolean', required: false, default: false, description: 'Include past collection versions.',
               }
             })
-    params.delete(:select)
     params
   end
 
@@ -93,7 +92,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
@@ -103,7 +102,7 @@ class Arvados::V1::GroupsController < ApplicationController
 
   def destroy
     if !TRASHABLE_CLASSES.include?(@object.group_class)
-      return @object.destroy
+      @object.destroy
       show
     else
       super # Calls destroy from TrashableController module
@@ -219,10 +218,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] }]
 
@@ -260,9 +256,26 @@ 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
+      @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
+    error_by_class = {}
+    any_success = false
+
     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
@@ -288,14 +301,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]
@@ -318,7 +338,19 @@ class Arvados::V1::GroupsController < ApplicationController
       # 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)
@@ -349,6 +381,14 @@ class Arvados::V1::GroupsController < ApplicationController
       end
     end
 
+    # 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 params["include"]
       @extra_included = included_by_uuid.values
     end