Optimize listing of file attachments on steps and results, improve handling of WOPI tokens [SCI-11412] (#8149)

app/controllers/results_controller.rb

@@ -77,9 +77,10 @@ class ResultsController < ApplicationController
   end
 
   def assets
-    render json: @result.assets,
+    render json: @result.assets.preload(:preview_image_attachment, file_attachment: :blob, result: { my_module: { experiment: :project, user_assignments: %i(user user_role) } }),
            each_serializer: AssetSerializer,
-           user: current_user
+           user: current_user,
+           managable_result: can_manage_result?(@result)
   end
 
   def upload_attachment

app/controllers/steps_controller.rb

@@ -28,9 +28,11 @@ class StepsController < ApplicationController
   end
 
   def attachments
-    render json: @step.assets,
+    render json: @step.assets.preload(:preview_image_attachment, file_attachment: :blob,
+                                      step: { protocol: { my_module: { experiment: :project, user_assignments: %i(user user_role) } } }),
            each_serializer: AssetSerializer,
-           user: current_user
+           user: current_user,
+           managable_step: can_manage_step?(@step)
   end
 
   def upload_attachment

app/controllers/wopi_controller.rb

@@ -293,7 +293,7 @@ class WopiController < ActionController::Base
         @can_write = can_manage_step?(@assoc)
         @close_url = protocols_url(only_path: false, host: ENV['WOPI_USER_HOST'])
 
-        @breadcrump_brand_name = 'Projects'
+        @breadcrumb_brand_name = @protocol.name
         @breadcrumb_brand_url = root_url(only_path: false, host: ENV['WOPI_USER_HOST'])
         @breadcrumb_folder_name = 'Protocol managament'
       end

app/models/asset.rb

@@ -357,8 +357,8 @@ class Asset < ApplicationRecord
   end
 
   def unlock_expired
-    with_lock do
-      if !lock_ttl.nil? && lock_ttl < Time.now.to_i
+    if !lock_ttl.nil? && lock_ttl < Time.now.to_i
+      with_lock do
         self.lock = nil
         self.lock_ttl = nil
         save!
@@ -373,7 +373,7 @@ class Asset < ApplicationRecord
   end
 
   def editable_image?
-    !locked? && (%r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES_EDITABLE)}} =~ file.content_type).present?
+    (%r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES_EDITABLE)}} =~ file.content_type).present?
   end
 
   def generate_base64(style)

app/models/token.rb

@@ -4,5 +4,5 @@ class Token < ApplicationRecord
   validates :token, presence: true
   validates :ttl, presence: true
 
-  belongs_to :user, foreign_key: 'user_id', class_name: 'User', inverse_of: :tokens
+  belongs_to :user, inverse_of: :tokens
 end

app/models/user.rb

@@ -460,23 +460,17 @@ class User < ApplicationRecord
     Rails.logger.warn "WOPI: searching by token #{token}"
     User.joins(:tokens)
         .where(tokens: { token: token })
-        .find_by('tokens.ttl = 0 OR tokens.ttl > ?', Time.now.to_i)
+        .find_by('tokens.ttl > ?', Time.now.utc.to_i)
   end
 
   def get_wopi_token
     # WOPI does not have a good way to request a new token,
     # so a new token should be provided each time this is called,
     # while keeping any old tokens as long as they have not yet expired
-    tokens = Token.where(user_id: id).distinct
-
-    tokens.each do |token|
-      token.delete if token.ttl < Time.now.to_i
-    end
-
-    token_string =  "#{Devise.friendly_token(20)}-#{id}"
-    # WOPI uses millisecond TTLs
-    ttl = (Time.now + 1.day).to_i
-    wopi_token = Token.create(token: token_string, ttl: ttl, user_id: id)
+    token_string = "#{Devise.friendly_token(20)}-#{id}"
+    # WOPI uses millisecond TTLs and 10 hours TTL is recommended by the vendor
+    ttl = Time.now.utc.to_i + Constants::WOPI_TOKEN_VALIDITY
+    wopi_token = tokens.create!(token: token_string, ttl: ttl)
     Rails.logger.warn("WOPI: generating new token #{wopi_token.token}")
     wopi_token
   end

app/serializers/asset_serializer.rb

@@ -86,7 +86,7 @@ class AssetSerializer < ActiveModel::Serializer
   end
 
   def pdf_previewable
-    object.pdf_previewable? if object.file.attached?
+    object.pdf_previewable?
   end
 
   def pdf
@@ -100,7 +100,7 @@ class AssetSerializer < ActiveModel::Serializer
   end
 
   def image_editable
-    object.editable_image?
+    @image_editable ||= object.editable_image?
   end
 
   def checksum
@@ -142,7 +142,7 @@ class AssetSerializer < ActiveModel::Serializer
       versions: (asset_versions_path(object) if attached)
     }
     user = scope[:user] || @instance_options[:user]
-    if can_manage_asset?(user, object)
+    if managable?
       urls.merge!(
         toggle_view_mode: toggle_view_mode_path(object),
         edit_asset: edit_asset_path(object),
@@ -157,9 +157,9 @@ class AssetSerializer < ActiveModel::Serializer
     end
 
     urls[:restore_version] = asset_restore_version_path(object) if can_restore_asset?(user, object)
-    urls[:open_vector_editor_edit] = edit_gene_sequence_asset_path(object.id) if can_manage_asset?(user, object)
+    urls[:open_vector_editor_edit] = edit_gene_sequence_asset_path(object.id) if managable?
 
-    if can_manage_asset?(user, object) && can_open_asset_locally?(user, object)
+    if managable? && can_open_asset_locally?(user, object)
       urls[:open_locally] = asset_sync_show_path(object)
       urls[:open_locally_api] = Constants::ASSET_SYNC_URL
       urls[:asset_show] = asset_show_path(object)
@@ -176,4 +176,13 @@ class AssetSerializer < ActiveModel::Serializer
     user = scope[:user] || @instance_options[:user]
     can_open_asset_locally?(user, object)
   end
+
+  private
+
+  def managable?
+    return true if @instance_options[:managable_step] || @instance_options[:managable_result]
+
+    user = scope[:user] || @instance_options[:user]
+    @managable ||= can_manage_asset?(user, object)
+  end
 end

app/utilities/active_storage_file_util.rb

@@ -3,6 +3,8 @@
 module ActiveStorageFileUtil
   # Method expects instance of ActiveStorage::Blob as argument
   def previewable_document?(blob)
+    return false if blob.blank?
+
     previewable = Constants::PREVIEWABLE_FILE_TYPES.include?(blob.content_type)
 
     file_extension = blob.filename.extension

config/initializers/constants.rb

@@ -309,6 +309,8 @@ class Constants
     docx docm odt xlsx xlsm xlsb ods pptx ppsx odp
   ).freeze
 
+  WOPI_TOKEN_VALIDITY = 10.hours.to_i
+
   TEXT_EXTRACT_FILE_TYPES = [
     'application/msword',
     'application/vnd.openxmlformats-officedocument.wordprocessingml.template',

config/initializers/scheduler.rb

@@ -34,6 +34,13 @@ if ENV['WORKER'].present?
     RepositoryItemDateReminderJob.perform_now
   end
 
+  if ENV['WOPI_ENABLED'] == 'true'
+    # Clean up expired WOPI tokens
+    schedule_task(scheduler, '1d') do
+      Token.where(ttl: ...Time.now.utc.to_i).delete_all
+    end
+  end
+
   schedule_task(scheduler, '1d') do
     NotificationCleanupJob.perform_now
   end

spec/controllers/wopi_controller_spec.rb

@@ -24,7 +24,7 @@ describe WopiController, type: :controller do
   let(:step_asset) { create :step_asset, step: step, asset: asset }
   let(:step_asset_in_repository) { create :step_asset, step: step_in_repository, asset: asset }
   let(:result_asset) { create :result_asset, result: result, asset: asset }
-  let(:token) { Token.create(token: 'token', ttl: 0, user_id: user.id) }
+  let(:token) { Token.create(token: 'token', ttl: Time.now.utc.to_i + Constants::WOPI_TOKEN_VALIDITY, user_id: user.id) }
 
   describe 'POST unlock' do
     before do