From 9c288c20210ee86482b4e497f344aa961d64507b Mon Sep 17 00:00:00 2001 From: Alex Kriuchykhin Date: Thu, 3 Oct 2024 14:19:55 +0200 Subject: [PATCH] Improve speed of inventory tables rendering on the backend [SCI-11134] (#7923) --- Gemfile | 1 + Gemfile.lock | 23 +++++--- app/controllers/repository_rows_controller.rb | 6 +- app/helpers/application_helper.rb | 7 +-- app/helpers/input_sanitize_helper.rb | 5 +- app/helpers/repository_datatable_helper.rb | 55 ++++++++----------- app/models/repository_text_value.rb | 2 + .../repository_text_value_serializer.rb | 2 +- app/services/repository_datatable_service.rb | 7 ++- .../repository_snapshot_datatable_service.rb | 3 +- app/services/smart_annotations/tag_to_html.rb | 9 +-- app/services/smart_annotations/tag_to_text.rb | 5 +- ...nnotation_flag_to_repository_text_value.rb | 14 +++++ db/schema.rb | 4 +- 14 files changed, 78 insertions(+), 65 deletions(-) create mode 100644 db/migrate/20241002122340_add_smart_annotation_flag_to_repository_text_value.rb diff --git a/Gemfile b/Gemfile index d8f544316..e28a8437a 100644 --- a/Gemfile +++ b/Gemfile @@ -62,6 +62,7 @@ gem 'logging', '~> 2.0.0' gem 'nested_form_fields' gem 'nokogiri', '~> 1.16.5' # HTML/XML parser gem 'noticed' +gem 'oj' gem 'rails_autolink', '~> 1.1', '>= 1.1.6' gem 'rgl' # Graph framework for project diagram calculations gem 'roo', '~> 2.10.0' # Spreadsheet parser diff --git a/Gemfile.lock b/Gemfile.lock index 83e1acc9f..82d0e20e7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -97,9 +97,9 @@ GEM erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - active_model_serializers (0.10.13) - actionpack (>= 4.1, < 7.1) - activemodel (>= 4.1, < 7.1) + active_model_serializers (0.10.14) + actionpack (>= 4.1) + activemodel (>= 4.1) case_transform (>= 0.2) jsonapi-renderer (>= 0.1.1.beta1, < 0.3) activejob (7.0.8.4) @@ -195,6 +195,7 @@ GEM erubi (>= 1.0.0) rack (>= 0.9.0) rouge (>= 1.0.0) + bigdecimal (3.1.8) bindata (2.5.0) binding_of_caller (1.0.0) debug_inspector (>= 0.0.1) @@ -202,7 +203,7 @@ GEM msgpack (~> 1.2) brakeman (6.1.2) racc - builder (3.2.4) + builder (3.3.0) bullet (7.0.7) activesupport (>= 3.0.0) uniform_notifier (~> 1.11) @@ -316,7 +317,7 @@ GEM railties (>= 5) down (5.4.1) addressable (~> 2.8) - erubi (1.12.0) + erubi (1.13.0) et-orbi (1.2.11) tzinfo execjs (2.8.1) @@ -364,7 +365,7 @@ GEM httparty (0.21.0) mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) - i18n (1.14.5) + i18n (1.14.6) concurrent-ruby (~> 1.0) i18n-js (3.9.2) i18n (>= 0.6.6) @@ -372,7 +373,7 @@ GEM mini_magick (>= 4.9.5, < 5) ruby-vips (>= 2.0.17, < 3) iniparse (1.5.0) - jbuilder (2.11.5) + jbuilder (2.13.0) actionview (>= 5.0.0) activesupport (>= 5.0.0) jmespath (1.6.2) @@ -435,8 +436,7 @@ GEM mime-types-data (3.2023.0218.1) mini_magick (4.12.0) mini_mime (1.1.5) - mini_portile2 (2.8.7) - minitest (5.23.1) + minitest (5.25.1) msgpack (1.7.1) multi_json (1.15.0) multi_test (1.1.0) @@ -475,6 +475,9 @@ GEM rack (>= 1.2, < 4) snaky_hash (~> 2.0) version_gem (~> 1.1) + oj (3.16.6) + bigdecimal (>= 3.0) + ostruct (>= 0.2) omniauth (2.1.2) hashie (>= 3.4.6) rack (>= 2.2.3) @@ -509,6 +512,7 @@ GEM validate_url webfinger (~> 2.0) orm_adapter (0.5.0) + ostruct (0.6.0) overcommit (0.60.0) childprocess (>= 0.6.3, < 5) iniparse (~> 1.4) @@ -835,6 +839,7 @@ DEPENDENCIES newrelic_rpm nokogiri (~> 1.16.5) noticed + oj omniauth (~> 2.1) omniauth-azure-activedirectory-v2 omniauth-linkedin-oauth2 diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index 6a82de2a9..cc71c26fa 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -29,11 +29,7 @@ class RepositoryRowsController < ApplicationController @all_rows_count = datatable_service.all_count @columns_mappings = datatable_service.mappings @repository_rows = datatable_service.repository_rows - .preload(:repository_columns, - :created_by, - :archived_by, - :last_modified_by, - repository_cells: { value: @repository.cell_preload_includes }) + .preload(:repository_columns, repository_cells: { value: @repository.cell_preload_includes }) .page(page) .per(per_page) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 513a90ee7..5802404e7 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -141,10 +141,9 @@ module ApplicationHelper # Check if text have smart annotations of users # and outputs a popover with user information def smart_annotation_filter_users(text, team, base64_encoded_imgs: false) - sa_user = /\[\@(.*?)~([0-9a-zA-Z]+)\]/ - text.gsub(sa_user) do |el| - match = el.match(sa_user) - user = User.find_by_id(match[2].base62_decode) + text.gsub(SmartAnnotations::TagToHtml::USER_REGEX) do |el| + match = el.match(SmartAnnotations::TagToHtml::USER_REGEX) + user = User.find_by(id: match[2].base62_decode) next unless user popover_for_user_name(user, team, false, false, base64_encoded_imgs) diff --git a/app/helpers/input_sanitize_helper.rb b/app/helpers/input_sanitize_helper.rb index e30a23666..73c9eaca2 100644 --- a/app/helpers/input_sanitize_helper.rb +++ b/app/helpers/input_sanitize_helper.rb @@ -46,9 +46,8 @@ module InputSanitizeHelper sanitizer_config = Constants::INPUT_SANITIZE_CONFIG.deep_dup text = sanitize_input(text, tags, sanitizer_config: sanitizer_config) - if text =~ SmartAnnotations::TagToHtml::USER_REGEX || text =~ SmartAnnotations::TagToHtml::REGEX - text = smart_annotation_parser(text, team, base64_encoded_imgs, preview_repository) - end + text = smart_annotation_parser(text, team, base64_encoded_imgs, preview_repository) if text.match?(SmartAnnotations::TagToHtml::ALL_REGEX) + auto_link( text, html: { target: '_blank' }, diff --git a/app/helpers/repository_datatable_helper.rb b/app/helpers/repository_datatable_helper.rb index aa78df6aa..7744db911 100644 --- a/app/helpers/repository_datatable_helper.rb +++ b/app/helpers/repository_datatable_helper.rb @@ -15,29 +15,20 @@ module RepositoryDatatableHelper !repository.is_a?(SoftLockedRepository) stock_consumption_permitted = has_stock_management && options[:include_stock_consumption] && options[:my_module] && stock_consumption_permitted?(repository, options[:my_module]) + default_columns_method_name = "#{repository.class.name.underscore}_default_columns" repository_rows.map do |record| - row = public_send("#{repository.class.name.underscore}_default_columns", record) - row.merge!( - DT_RowId: record.id, - DT_RowAttr: { 'data-state': row_style(record), 'data-e2e': "e2e-TR-invInventory-bodyRow-#{record.id}" }, - recordInfoUrl: Rails.application.routes.url_helpers.repository_repository_row_path(repository, record), - rowRemindersUrl: - Rails.application.routes.url_helpers - .active_reminder_repository_cells_repository_repository_row_url( - repository, - record - ), - relationshipsUrl: - Rails.application.routes.url_helpers - .relationships_repository_repository_row_url(record.repository_id, record.id), - relationships_enabled: repository_row_connections_enabled, - code: record.code - ) - - if reminders_enabled - row['hasActiveReminders'] = record.has_active_stock_reminders || record.has_active_datetime_reminders - end + row = public_send(default_columns_method_name, record) + row['code'] = record.code + row['DT_RowId'] = record.id + row['DT_RowAttr'] = { 'data-state': row_style(record), 'data-e2e': "e2e-TR-invInventory-bodyRow-#{record.id}" } + row['recordInfoUrl'] = Rails.application.routes.url_helpers.repository_repository_row_path(repository.id, record.id) + row['rowRemindersUrl'] = Rails.application.routes.url_helpers + .active_reminder_repository_cells_repository_repository_row_url(repository.id, record.id) + row['relationshipsUrl'] = Rails.application.routes.url_helpers + .relationships_repository_repository_row_url(record.repository_id, record.id) + row['relationships_enabled'] = repository_row_connections_enabled + row['hasActiveReminders'] = record.has_active_stock_reminders || record.has_active_datetime_reminders if reminders_enabled unless options[:view_mode] || repository.is_a?(SoftLockedRepository) row['recordUpdateUrl'] = @@ -51,8 +42,7 @@ module RepositoryDatatableHelper custom_cells = record.repository_cells.filter { |cell| cell.value_type != 'RepositoryStockValue' } custom_cells.each do |cell| - row[columns_mappings[cell.repository_column.id]] = - serialize_repository_cell_value(cell, team, repository, reminders_enabled: reminders_enabled) + row[columns_mappings[cell.repository_column_id]] = serialize_repository_cell_value(cell, team, repository, reminders_enabled: reminders_enabled) end if has_stock_management @@ -193,15 +183,14 @@ module RepositoryDatatableHelper '1': record.code, '2': escape_input(record.name), '3': I18n.l(record.created_at, format: :full), - '4': escape_input(record.created_by.full_name), + '4': escape_input(record.created_by_full_name), 'recordInfoUrl': Rails.application.routes.url_helpers .repository_repository_row_path(repository_snapshot, record) } # Add custom columns record.repository_cells.each do |cell| - row[columns_mappings[cell.repository_column.id]] = - serialize_repository_cell_value(cell, team, repository_snapshot) + row[columns_mappings[cell.repository_column_id]] = serialize_repository_cell_value(cell, team, repository_snapshot) end if has_stock_management @@ -239,11 +228,11 @@ module RepositoryDatatableHelper '3': escape_input(record.name), '4': "#{record.parent_connections_count || 0} / #{record.child_connections_count || 0}", '5': I18n.l(record.created_at, format: :full), - '6': escape_input(record.created_by.full_name), + '6': escape_input(record.created_by_full_name), '7': (record.updated_at ? I18n.l(record.updated_at, format: :full) : ''), - '8': escape_input(record.last_modified_by.full_name), + '8': escape_input(record.last_modified_by_full_name), '9': (record.archived_on ? I18n.l(record.archived_on, format: :full) : ''), - '10': escape_input(record.archived_by&.full_name) + '10': escape_input(record.archived_by_full_name) } end @@ -254,9 +243,9 @@ module RepositoryDatatableHelper '3': escape_input(record.name), '4': "#{record.parent_connections_count || 0} / #{record.child_connections_count || 0}", '5': I18n.l(record.created_at, format: :full), - '6': escape_input(record.created_by.full_name), + '6': escape_input(record.created_by_full_name), '7': (record.archived_on ? I18n.l(record.archived_on, format: :full) : ''), - '8': escape_input(record.archived_by&.full_name) + '8': escape_input(record.archived_by_full_name) } end @@ -266,9 +255,9 @@ module RepositoryDatatableHelper '2': record.code, '3': escape_input(record.name), '4': I18n.l(record.created_at, format: :full), - '5': escape_input(record.created_by.full_name), + '5': escape_input(record.created_by_full_name), '6': (record.archived_on ? I18n.l(record.archived_on, format: :full) : ''), - '7': escape_input(record.archived_by&.full_name), + '7': escape_input(record.archived_by_full_name), '8': escape_input(record.external_id) } end diff --git a/app/models/repository_text_value.rb b/app/models/repository_text_value.rb index 529f89c79..be59b0a11 100644 --- a/app/models/repository_text_value.rb +++ b/app/models/repository_text_value.rb @@ -10,6 +10,8 @@ class RepositoryTextValue < ApplicationRecord has_one :repository_cell, as: :value, dependent: :destroy, touch: true accepts_nested_attributes_for :repository_cell + before_save -> { self.has_smart_annotation = data.match?(SmartAnnotations::TagToHtml::ALL_REGEX) }, if: -> { data_changed? } + validates :repository_cell, presence: true validates :data, presence: true, length: { maximum: Constants::TEXT_MAX_LENGTH } diff --git a/app/serializers/repository_datatable/repository_text_value_serializer.rb b/app/serializers/repository_datatable/repository_text_value_serializer.rb index f1d174f5a..d851ad09b 100644 --- a/app/serializers/repository_datatable/repository_text_value_serializer.rb +++ b/app/serializers/repository_datatable/repository_text_value_serializer.rb @@ -9,7 +9,7 @@ module RepositoryDatatable def value @user = scope[:user] { - view: custom_auto_link(value_object.data, simple_format: true, team: scope[:team]), + view: value_object.has_smart_annotation? ? custom_auto_link(value_object.data, simple_format: true, team: scope[:team]) : escape_input(value_object.data), edit: value_object.data } end diff --git a/app/services/repository_datatable_service.rb b/app/services/repository_datatable_service.rb index 44b0d58f6..f7062703c 100644 --- a/app/services/repository_datatable_service.rb +++ b/app/services/repository_datatable_service.rb @@ -128,8 +128,13 @@ class RepositoryDatatableService repository_rows = repository_rows.where(id: advanced_search(repository_rows)) if @params[:advanced_search].present? - repository_rows.left_outer_joins(:created_by, :archived_by, :last_modified_by) + repository_rows.joins('LEFT OUTER JOIN "users" "created_by" ON "created_by"."id" = "repository_rows"."created_by_id"') + .joins('LEFT OUTER JOIN "users" "last_modified_by" ON "last_modified_by"."id" = "repository_rows"."last_modified_by_id"') + .joins('LEFT OUTER JOIN "users" "archived_by" ON "archived_by"."id" = "repository_rows"."archived_by_id"') .select('repository_rows.*') + .select('MAX("created_by"."full_name") AS created_by_full_name') + .select('MAX("last_modified_by"."full_name") AS last_modified_by_full_name') + .select('MAX("archived_by"."full_name") AS archived_by_full_name') .select('COUNT("repository_rows"."id") OVER() AS filtered_count') .group('repository_rows.id') end diff --git a/app/services/repository_snapshot_datatable_service.rb b/app/services/repository_snapshot_datatable_service.rb index c0e8528ea..32f6c0511 100644 --- a/app/services/repository_snapshot_datatable_service.rb +++ b/app/services/repository_snapshot_datatable_service.rb @@ -44,8 +44,9 @@ class RepositorySnapshotDatatableService < RepositoryDatatableService repository_rows = results end - repository_rows.left_outer_joins(:created_by) + repository_rows.joins('LEFT OUTER JOIN "users" "created_by" ON "created_by"."id" = "repository_rows"."created_by_id"') .select('repository_rows.*') + .select('MAX("created_by"."full_name") AS created_by_full_name') .select('COUNT("repository_rows"."id") OVER() AS filtered_count') .group('repository_rows.id') end diff --git a/app/services/smart_annotations/tag_to_html.rb b/app/services/smart_annotations/tag_to_html.rb index c543c1169..317770e1d 100644 --- a/app/services/smart_annotations/tag_to_html.rb +++ b/app/services/smart_annotations/tag_to_html.rb @@ -2,8 +2,9 @@ module SmartAnnotations class TagToHtml - REGEX = /\[\#(.*?)~(prj|exp|tsk|rep_item)~([0-9a-zA-Z]+)\]/.freeze - USER_REGEX = /\[@(.*?)~([0-9a-zA-Z]+)\]/.freeze + ALL_REGEX = /\[(@(.*?)|\#(.*?)~(prj|exp|tsk|rep_item))~([0-9a-zA-Z]+)\]/ + ITEMS_REGEX = /\[\#(.*?)~(prj|exp|tsk|rep_item)~([0-9a-zA-Z]+)\]/ + USER_REGEX = /\[@(.*?)~([0-9a-zA-Z]+)\]/ attr_reader :html def initialize(user, team, text, preview_repository = false) @@ -18,7 +19,7 @@ module SmartAnnotations rep_item: RepositoryRow }.freeze def parse(user, team, text, preview_repository = false) - @html = text.gsub(REGEX) do |el| + @html = text.gsub(ITEMS_REGEX) do |el| value = extract_values(el) type = value[:object_type] begin @@ -49,7 +50,7 @@ module SmartAnnotations end def extract_values(element) - match = element.match(REGEX) + match = element.match(ITEMS_REGEX) { name: match[1], object_type: match[2], diff --git a/app/services/smart_annotations/tag_to_text.rb b/app/services/smart_annotations/tag_to_text.rb index 70819ea54..51ab125b7 100644 --- a/app/services/smart_annotations/tag_to_text.rb +++ b/app/services/smart_annotations/tag_to_text.rb @@ -2,6 +2,9 @@ module SmartAnnotations class TagToText + USER_REGEX = /\[@(.*?)~([0-9a-zA-Z]+)\]/ + ITEMS_REGEX = /\[\#(.*?)~(prj|exp|tsk|rep_item)~([0-9a-zA-Z]+)\]/ + attr_reader :text def initialize(user, team, text, is_shared_object: false) @@ -11,8 +14,6 @@ module SmartAnnotations private - USER_REGEX = /\[\@(.*?)~([0-9a-zA-Z]+)\]/ - ITEMS_REGEX = /\[\#(.*?)~(prj|exp|tsk|rep_item)~([0-9a-zA-Z]+)\]/ OBJECT_MAPPINGS = { prj: Project, exp: Experiment, tsk: MyModule, diff --git a/db/migrate/20241002122340_add_smart_annotation_flag_to_repository_text_value.rb b/db/migrate/20241002122340_add_smart_annotation_flag_to_repository_text_value.rb new file mode 100644 index 000000000..5ae3fc994 --- /dev/null +++ b/db/migrate/20241002122340_add_smart_annotation_flag_to_repository_text_value.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddSmartAnnotationFlagToRepositoryTextValue < ActiveRecord::Migration[7.0] + def up + add_column :repository_text_values, :has_smart_annotation, :boolean, null: false, default: false + + execute('UPDATE repository_text_values SET has_smart_annotation = TRUE ' \ + 'WHERE data ~ \'\[(@(.*?)|\#(.*?)~(prj|exp|tsk|rep_item))~([0-9a-zA-Z]+)\]\'') + end + + def down + remove_column :repository_text_values, :has_smart_annotation + end +end diff --git a/db/schema.rb b/db/schema.rb index 6dfb6998e..6bbdd86ef 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_07_05_122903) do +ActiveRecord::Schema[7.0].define(version: 2024_10_02_122340) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gist" enable_extension "pg_trgm" @@ -936,6 +936,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_07_05_122903) do t.datetime "updated_at", precision: nil t.bigint "created_by_id", null: false t.bigint "last_modified_by_id", null: false + t.boolean "has_smart_annotation", default: false, null: false t.index "trim_html_tags((data)::text) gin_trgm_ops", name: "index_repository_text_values_on_data", using: :gin end @@ -1550,7 +1551,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_07_05_122903) do add_foreign_key "tags", "projects" add_foreign_key "tags", "users", column: "created_by_id" add_foreign_key "tags", "users", column: "last_modified_by_id" - add_foreign_key "team_shared_objects", "repositories", column: "shared_object_id" add_foreign_key "team_shared_objects", "teams" add_foreign_key "teams", "users", column: "created_by_id" add_foreign_key "teams", "users", column: "last_modified_by_id"