From e42fc3117b6d3a18df3b7c87e6f6e6273275a24c Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Mon, 26 Mar 2018 13:19:09 +0200 Subject: [PATCH] Implement search in file repository cells and improve performance [SCI-2111] --- Gemfile.lock | 5 +- app/controllers/search_controller.rb | 98 ++++++++++++------- app/models/asset.rb | 51 +++------- app/views/search/index.html.erb | 42 +++++--- app/views/search/results/_assets.html.erb | 25 +++-- .../results/partials/_asset_text.html.erb | 32 +++++- .../partials/_repository_row_text.html.erb | 7 ++ .../partials/_repository_text.html.erb | 7 ++ config/initializers/extends.rb | 6 +- config/locales/en.yml | 3 +- 10 files changed, 174 insertions(+), 102 deletions(-) create mode 100644 app/views/search/results/partials/_repository_row_text.html.erb create mode 100644 app/views/search/results/partials/_repository_text.html.erb diff --git a/Gemfile.lock b/Gemfile.lock index ecfaef70f..339037097 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,10 +1,11 @@ GIT remote: https://github.com/biosistemika/canaid - revision: f2000c19b75e66ea929a44cb0575262b7f5fc13e + revision: 943ae9b9801819fd2513f6ab9e1143ad8de523ce branch: master specs: - canaid (1.0.1) + canaid (1.0.2) devise (>= 3.4.1) + docile (>= 1.1.0) rails (>= 4) GIT diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 19f912ba4..f3b87e4e0 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -4,6 +4,8 @@ class SearchController < ApplicationController def index redirect_to new_search_path unless @search_query + @search_id = params[:search_id] ? params[:search_id] : generate_search_id + count_search_results search_projects if @search_category == :projects @@ -86,6 +88,10 @@ class SearchController < ApplicationController protected + def generate_search_id + SecureRandom.urlsafe_base64(32) + end + def search_by_name(model) model.search(current_user, true, @@ -109,52 +115,61 @@ class SearchController < ApplicationController end def count_by_repository - count_total = 0 - search_results = Repository.search(current_user, - @search_query, - Constants::SEARCH_NO_LIMIT, - nil, - match_case: @search_case, - whole_word: @search_whole_word, - whole_phrase: @search_whole_phrase) - @repository_search_count = {} - current_user.teams.includes(:repositories).each do |team| - team_results = {} - team_results[:count] = 0 - team_results[:repositories] = {} - team.repositories.each do |repository| - repository_results = {} - repository_results[:id] = repository.id - repository_results[:count] = 0 - search_results.each do |result| - if repository.id == result.id - count_total += result.counter - repository_results[:count] += result.counter + @repository_search_count = + Rails.cache.fetch("#{@search_id}/repository_search_count", + expires_in: 5.minutes) do + search_count = {} + search_results = Repository.search(current_user, + @search_query, + Constants::SEARCH_NO_LIMIT, + nil, + match_case: @search_case, + whole_word: @search_whole_word, + whole_phrase: @search_whole_phrase) + + current_user.teams.includes(:repositories).each do |team| + team_results = {} + team_results[:count] = 0 + team_results[:repositories] = {} + team.repositories.each do |repository| + repository_results = {} + repository_results[:id] = repository.id + repository_results[:count] = 0 + search_results.each do |result| + if repository.id == result.id + repository_results[:count] += result.counter + end + end + team_results[:repositories][repository.name] = repository_results + team_results[:count] += repository_results[:count] end + search_count[team.name] = team_results end - team_results[:repositories][repository.name] = repository_results - team_results[:count] += repository_results[:count] + search_count end - @repository_search_count[team.name] = team_results + + count_total = 0 + @repository_search_count.each_value do |team_results| + count_total += team_results[:count] end count_total end def count_search_results - @project_search_count = count_by_name Project - @experiment_search_count = count_by_name Experiment - @module_search_count = count_by_name MyModule - @result_search_count = count_by_name Result - @tag_search_count = count_by_name Tag - @report_search_count = count_by_name Report - @protocol_search_count = count_by_name Protocol - @step_search_count = count_by_name Step - @checklist_search_count = count_by_name Checklist - @sample_search_count = count_by_name Sample + @project_search_count = fetch_cached_count Project + @experiment_search_count = fetch_cached_count Experiment + @module_search_count = fetch_cached_count MyModule + @result_search_count = fetch_cached_count Result + @tag_search_count = fetch_cached_count Tag + @report_search_count = fetch_cached_count Report + @protocol_search_count = fetch_cached_count Protocol + @step_search_count = fetch_cached_count Step + @checklist_search_count = fetch_cached_count Checklist + @sample_search_count = fetch_cached_count Sample @repository_search_count_total = count_by_repository - @asset_search_count = count_by_name Asset - @table_search_count = count_by_name Table - @comment_search_count = count_by_name Comment + @asset_search_count = fetch_cached_count Asset + @table_search_count = fetch_cached_count Table + @comment_search_count = fetch_cached_count Comment @search_results_count = @project_search_count @search_results_count += @experiment_search_count @@ -172,6 +187,15 @@ class SearchController < ApplicationController @search_results_count += @comment_search_count end + def fetch_cached_count(type) + exp = 5.minutes + Rails.cache.fetch( + "#{@search_id}/#{type.name.underscore}_search_count", expires_in: exp + ) do + count_by_name type + end + end + def search_projects @project_results = [] @project_results = search_by_name(Project) if @project_search_count > 0 diff --git a/app/models/asset.rb b/app/models/asset.rb index 318cc104a..055db8873 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -91,37 +91,19 @@ class Asset < ApplicationRecord def self.search( user, - include_archived, + _include_archived, query = nil, page = 1, _current_team = nil, options = {} ) - step_ids = - Step - .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .joins(:step_assets) - .distinct - .pluck('step_assets.id') - result_ids = - Result - .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .joins(:result_asset) - .distinct - .pluck('result_assets.id') - - ids = + new_query = Asset - .select(:id) .distinct - .joins('LEFT OUTER JOIN step_assets ON step_assets.asset_id = assets.id') - .joins('LEFT OUTER JOIN result_assets ON ' \ - 'result_assets.asset_id = assets.id') - .joins('LEFT JOIN asset_text_data ON ' \ - 'assets.id = asset_text_data.asset_id') - .where('(step_assets.id IN (?) OR result_assets.id IN (?))', - step_ids, result_ids) + .select('assets.*') + .left_outer_joins(:asset_text_datum) + .where(team: user.teams) a_query = s_query = '' @@ -144,7 +126,7 @@ class Asset < ApplicationRecord a_query = '\\y(' + a_query + ')\\y' s_query = s_query.tr('\'', '"') - ids = ids.where( + new_query = new_query.where( "(trim_html_tags(assets.file_file_name) #{like} ? " \ "OR asset_text_data.data_vector @@ to_tsquery(?))", a_query, @@ -165,7 +147,7 @@ class Asset < ApplicationRecord .map { |t| t + ':*' } .join('|') .tr('\'', '"') - ids = ids.where( + new_query = new_query.where( "(trim_html_tags(assets.file_file_name) #{like} ANY (array[?]) " \ "OR asset_text_data.data_vector @@ to_tsquery(?))", a_query, @@ -175,19 +157,14 @@ class Asset < ApplicationRecord # Show all results if needed if page != Constants::SEARCH_NO_LIMIT - ids = ids - .limit(Constants::SEARCH_LIMIT) - .offset((page - 1) * Constants::SEARCH_LIMIT) + new_query.select("ts_headline(data, to_tsquery('" + + sanitize_sql_for_conditions(s_query) + + "'), 'StartSel=, StopSel=') headline") + .limit(Constants::SEARCH_LIMIT) + .offset((page - 1) * Constants::SEARCH_LIMIT) + else + new_query end - - Asset - .joins('LEFT JOIN asset_text_data ON ' \ - ' assets.id = asset_text_data.asset_id') - .select('assets.*') - .select("ts_headline(data, to_tsquery('" + - sanitize_sql_for_conditions(s_query) + - "'), 'StartSel=, StopSel=') headline") - .where('assets.id IN (?)', ids) end def is_image? diff --git a/app/views/search/index.html.erb b/app/views/search/index.html.erb index b00a61fcb..b42d20441 100644 --- a/app/views/search/index.html.erb +++ b/app/views/search/index.html.erb @@ -51,7 +51,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @project_search_count %> <%= t'Projects' %> @@ -64,7 +65,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @experiment_search_count %> <%= fa_icon 'flask' %> <%= t'Experiments' %> @@ -77,7 +79,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @module_search_count %> <%= t'Modules' %> @@ -90,7 +93,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @result_search_count %> <%= t'Results' %> @@ -103,7 +107,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @tag_search_count %> <%= t'Tags' %> @@ -116,7 +121,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @report_search_count %> <%= t'Reports' %> @@ -129,7 +135,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @protocol_search_count %> <%= t'Protocols' %> @@ -142,7 +149,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @step_search_count %> <%= t'Steps' %> @@ -155,7 +163,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @checklist_search_count %> <%= t'Checklists' %> @@ -168,7 +177,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @sample_search_count %> <%= t'Samples' %> @@ -182,7 +192,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @asset_search_count %> <%= t'Assets' %> @@ -195,7 +206,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @table_search_count %> <%= t'Tables' %> @@ -208,7 +220,8 @@ > + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= @comment_search_count %> <%= t'Comments' %> @@ -229,7 +242,8 @@ + match_case: @search_case, utf8: '✓', + search_id: @search_id}.to_query %>"> <%= values[:count] %> <%= repository %> diff --git a/app/views/search/results/_assets.html.erb b/app/views/search/results/_assets.html.erb index f33c3b0fe..255a8b769 100644 --- a/app/views/search/results/_assets.html.erb +++ b/app/views/search/results/_assets.html.erb @@ -12,13 +12,6 @@ <%= render partial: "search/results/partials/asset_text.html.erb", locals: { asset: asset, query: search_query } %> - - <% if asset.headline.present? && !asset.headline.empty? && asset.headline.include?("") %> -
-

<%= highlight(sanitize_input(asset.headline), search_query.strip.split(/\s+/)) %>

-
- <% end %> -

<%=t "search.index.created_at" %> @@ -93,6 +86,24 @@ <%= render partial: "search/results/partials/team_text.html.erb", locals: { team: asset.result.my_module.experiment.project.team } %> + <% elsif asset.repository_asset_value %> + + <%=t "search.index.repository_row" %> + <%= render partial: "search/results/partials/repository_row_text.html.erb", + locals: { repository_row: asset.repository_asset_value.repository_cell.repository_row } %> + +
+ + <%=t "search.index.repository" %> + <%= render partial: "search/results/partials/repository_text.html.erb", + locals: { repository: asset.repository_asset_value.repository_cell.repository_row.repository } %> + +
+ + <%=t "search.index.team" %> + <%= render partial: "search/results/partials/team_text.html.erb", + locals: { team: asset.team } %> + <% end %>

diff --git a/app/views/search/results/partials/_asset_text.html.erb b/app/views/search/results/partials/_asset_text.html.erb index 1e81800a3..f5a483393 100644 --- a/app/views/search/results/partials/_asset_text.html.erb +++ b/app/views/search/results/partials/_asset_text.html.erb @@ -1,15 +1,43 @@ <% query ||= nil %> +<% asset_read_allowed = false %> <% text = query.present? ? highlight(asset.file_file_name, query.strip.split(/\s+/)) : asset.file_file_name %> <% if asset.step %> <% protocol = asset.step.protocol %> <% if can_read_protocol_in_module?(protocol) || - can_read_protocol_in_repository?(protocol) || - (asset.result && can_read_experiment?(protocol.my_module.experiment)) %> + can_read_protocol_in_repository?(protocol) %> + <% asset_read_allowed = true %> <%= text %> + <% else %> + <%= text %> + <% end %> +<% elsif asset.result %> + <% if can_read_experiment?(asset.result.my_module.experiment) %> + <% asset_read_allowed = true %> + + <%= text %> + + <% else %> + <%= text %> + <% end %> +<% elsif asset.repository_asset_value %> + <% if can_read_team?(asset.repository_asset_value.repository_cell.repository_row.repository.team) %> + <% asset_read_allowed = true %> + + <%= text %> + + <% else %> + <%= text %> <% end %> <% else %> <%= text %> <% end %> + + +<% if asset_read_allowed && asset.headline.present? && asset.headline.include?("") %> +
+

<%= highlight(sanitize_input(asset.headline), query.strip.split(/\s+/)) %>

+
+<% end %> diff --git a/app/views/search/results/partials/_repository_row_text.html.erb b/app/views/search/results/partials/_repository_row_text.html.erb new file mode 100644 index 000000000..ea6c23340 --- /dev/null +++ b/app/views/search/results/partials/_repository_row_text.html.erb @@ -0,0 +1,7 @@ +<% if can_read_team?(repository_row.repository.team) %> + <%= route_to_other_team repository_path(id: repository_row.repository.id), + repository_row.repository.team, + repository_row.name %> +<% else %> + <%= repository_row.name %> +<% end %> diff --git a/app/views/search/results/partials/_repository_text.html.erb b/app/views/search/results/partials/_repository_text.html.erb new file mode 100644 index 000000000..fbc7f2d5d --- /dev/null +++ b/app/views/search/results/partials/_repository_text.html.erb @@ -0,0 +1,7 @@ +<% if can_read_team?(repository.team) %> + <%= route_to_other_team repository_path(id: repository.id), + repository.team, + repository.name %> +<% else %> + <%= repository.name %> +<% end %> diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index bd0a90886..a70e4b38c 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -52,11 +52,13 @@ class Extends # Extra attributes used for search in repositories, text columns # are only supported REPOSITORY_EXTRA_SEARCH_ATTR = ['repository_text_values.data', - 'repository_list_items.data'] + 'repository_list_items.data', + 'assets.file_file_name'] # Array of includes used in search query for repository rows REPOSITORY_SEARCH_INCLUDES = [:repository_text_value, - repository_list_value: :repository_list_item] + repository_list_value: :repository_list_item, + repository_asset_value: :asset] # List of implemented core API versions API_VERSIONS = ['20170715'] diff --git a/config/locales/en.yml b/config/locales/en.yml index b675d7f22..3e30a9f39 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -152,6 +152,8 @@ en: description: "Description: " no_description: "No description" team: "Team: " + repository: "Repository: " + repository_row: "Repository item: " project: "Project: " experiment: "Experiment: " protocol: "Protocol: " @@ -211,7 +213,6 @@ en: one: "1 day" other: "%{count} days" module_one_day_due_html: "Task %{module} is due in less than 1 day." - new_comment: "New comment" user_role: "Role: " user_full_name: "User: " edit_user: "Edit role"