From: Radhika Chippada Date: Wed, 8 Apr 2015 23:48:13 +0000 (-0400) Subject: 5365: add @distinct handling to workbench select queries and use this to preload... X-Git-Tag: 1.1.0~1692^2~7 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/9daf42fbdb868939653c6e3ca8a4fffd1cf94e31 5365: add @distinct handling to workbench select queries and use this to preload portable_data_hashes as well. --- diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb index 1b59c574b7..4f89f262b7 100644 --- a/apps/workbench/app/controllers/application_controller.rb +++ b/apps/workbench/app/controllers/application_controller.rb @@ -1068,6 +1068,39 @@ class ApplicationController < ActionController::Base @all_log_collections_for end + # Helper method to get one collection for the given portable_data_hash + # This is used to determine if a pdh is readable by the current_user + helper_method :collection_for_pdh + def collection_for_pdh pdh + raise ArgumentError, 'No input argument' unless pdh + preload_for_pdhs([pdh]) + @all_pdhs_for[pdh] ||= [] + end + + # Helper method to preload one collection each for the given pdhs + # This is used to determine if a pdh is readable by the current_user + helper_method :preload_for_pdhs + def preload_for_pdhs pdhs + @all_pdhs_for ||= {} + + raise ArgumentError, 'Argument is not an array' unless pdhs.is_a? Array + return @all_pdhs_for if pdhs.empty? + + # if already preloaded for all of these pdhs, return + if not pdhs.select { |x| @all_pdhs_for[x].nil? }.any? + return @all_pdhs_for + end + + pdhs.each do |x| + @all_pdhs_for[x] = [] + end + # TODO: make sure we get every page of results from API server + Collection.select(%w(portable_data_hash)).where(portable_data_hash: pdhs).distinct().each do |collection| + @all_pdhs_for[collection.portable_data_hash] << collection + end + @all_pdhs_for + end + # helper method to get object of a given dataclass and uuid helper_method :object_for_dataclass def object_for_dataclass dataclass, uuid diff --git a/apps/workbench/app/helpers/application_helper.rb b/apps/workbench/app/helpers/application_helper.rb index 449d82390c..b145fddcc9 100644 --- a/apps/workbench/app/helpers/application_helper.rb +++ b/apps/workbench/app/helpers/application_helper.rb @@ -177,15 +177,15 @@ module ApplicationHelper end end - def link_to_arvados_object_if_readable(attrvalue, link_text, link_text_if_not_readable, use_friendly_name=false, resource_class=nil) - resource_class = resource_class_for_uuid(attrvalue) if !resource_class + def link_to_arvados_object_if_readable(attrvalue, link_text, link_text_if_not_readable, use_friendly_name=false) + resource_class = resource_class_for_uuid(attrvalue) if !resource_class return link_text_if_not_readable end - if resource_class.andand.to_s == 'Collection' + if resource_class.to_s == 'Collection' if CollectionsHelper.match(attrvalue) - readable = Collection.find? attrvalue # portable_data_hash + readable = collection_for_pdh(attrvalue).any? else readable = collections_for_object(attrvalue).any? end diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb index f19d47435a..2fca11ecf1 100644 --- a/apps/workbench/app/models/arvados_base.rb +++ b/apps/workbench/app/models/arvados_base.rb @@ -135,6 +135,10 @@ class ArvadosBase < ActiveRecord::Base ArvadosResourceList.new(self).select(*args) end + def self.distinct(*args) + ArvadosResourceList.new(self).distinct(*args) + end + def self.eager(*args) ArvadosResourceList.new(self).eager(*args) end diff --git a/apps/workbench/app/models/arvados_resource_list.rb b/apps/workbench/app/models/arvados_resource_list.rb index d989715080..9f66d39227 100644 --- a/apps/workbench/app/models/arvados_resource_list.rb +++ b/apps/workbench/app/models/arvados_resource_list.rb @@ -16,6 +16,11 @@ class ArvadosResourceList self end + def distinct(bool=true) + @distinct = bool + self + end + def limit(max_results) if not max_results.nil? and not max_results.is_a? Integer raise ArgumentError("argument to limit() must be an Integer or nil") @@ -178,6 +183,7 @@ class ArvadosResourceList api_params[:select] = @select if @select api_params[:order] = @orderby_spec if @orderby_spec api_params[:filters] = @filters if @filters + api_params[:distinct] = @distinct if @distinct item_count = 0 diff --git a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb index 09d6ab70be..d0959f2652 100644 --- a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb +++ b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb @@ -148,12 +148,7 @@ <% if k == :uuid %> - <% resource_class = resource_class_for_uuid(current_component[k]) %> - <% if resource_class.andand.to_s == 'Job' %> - <%= link_to_arvados_object_if_readable(current_component[k], current_component[k], current_component[k], false, resource_class) %> - <% else %> - <%= link_to_if_arvados_object current_component[k], link_text: current_component[k] %> - <% end %> + <%= link_to_arvados_object_if_readable(current_component[k], current_component[k], current_component[k], false) %> <% elsif k.to_s.end_with? 'uuid' %> <%= link_to_if_arvados_object current_component[k], friendly_name: true %> <% elsif k.to_s.end_with? '_at' %> diff --git a/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb index d9814a0a70..dcbdb1d2c9 100644 --- a/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb +++ b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb @@ -92,6 +92,7 @@ job_output_pdhs = job_outputs.select {|x| !(m = CollectionsHelper.match(x)).nil?}.compact job_output_uuids = job_outputs - job_output_pdhs preload_collections_for_objects job_output_uuids if job_output_uuids.any? + preload_for_pdhs job_output_pdhs if job_output_pdhs.any? %> <% pipeline_jobs.each_with_index do |pj, i| %> diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml index c1ec91723d..bb06dac2db 100644 --- a/services/api/test/fixtures/pipeline_instances.yml +++ b/services/api/test/fixtures/pipeline_instances.yml @@ -272,9 +272,7 @@ pipeline_in_publicly_accessible_project: script: foo script_version: master script_parameters: - input: - dataclass: Collection - title: foo instance input + input: zzzzz-4zz18-4en62shvi99lxd4 log: zzzzz-4zz18-4en62shvi99lxd4 output: b519d9cb706a29fc7ea24dbea2f05851+93 state: Complete @@ -301,9 +299,7 @@ pipeline_in_publicly_accessible_project_but_other_objects_elsewhere: script: foo script_version: master script_parameters: - input: - dataclass: Collection - title: foo instance input + input: zzzzz-4zz18-bv31uwvy3neko21 log: zzzzz-4zz18-bv31uwvy3neko21 output: zzzzz-4zz18-bv31uwvy3neko21 state: Complete