From 9d926dc956dac57166cd503b5e215a99e96d066e Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Fri, 5 Jul 2019 16:56:05 +0200 Subject: [PATCH] Refactor TinyMce assets, user avatars, zip files [SCI-3539] --- .../javascripts/users/registrations/edit.js | 5 +- app/controllers/protocols_controller.rb | 2 +- app/controllers/tiny_mce_assets_controller.rb | 16 +-- .../users/registrations_controller.rb | 102 +++------------- app/controllers/zip_exports_controller.rb | 8 +- app/helpers/application_helper.rb | 6 +- app/helpers/reports_helper.rb | 14 +-- app/models/asset.rb | 55 --------- app/models/concerns/tiny_mce_images.rb | 49 ++++---- app/models/team.rb | 18 +-- app/models/temp_file.rb | 3 +- app/models/tiny_mce_asset.rb | 113 ++++++++---------- app/models/user.rb | 49 ++++++-- app/models/zip_export.rb | 78 ++++-------- .../v1/repository_asset_value_serializer.rb | 4 +- .../api/v1/result_asset_serializer.rb | 4 +- .../generate_workflow_image_service.rb | 1 + app/views/users/registrations/edit.html.erb | 1 - config/initializers/constants.rb | 6 +- .../initializers/filter_parameter_logging.rb | 2 +- config/routes.rb | 1 - ...0190613134100_convert_to_active_storage.rb | 4 +- docker-compose.yml | 2 + spec/services/templates_service_spec.rb | 1 - 24 files changed, 196 insertions(+), 348 deletions(-) diff --git a/app/assets/javascripts/users/registrations/edit.js b/app/assets/javascripts/users/registrations/edit.js index 2f59aeca6..55d363221 100644 --- a/app/assets/javascripts/users/registrations/edit.js +++ b/app/assets/javascripts/users/registrations/edit.js @@ -82,8 +82,8 @@ croppieContainer.croppie({ viewport: { type: 'circle' } }); $('.new-avatar-preview-container').off('update.croppie').on('update.croppie', function() { croppieContainer.croppie('result', { type: 'base64', format: 'jpeg', circle: false }) - .then(function(html) { - $('#user_avatar').val(html); + .then(function(image) { + $('#user_avatar').val(image); }); }); }; @@ -98,5 +98,6 @@ // Local file uploading animateSpinner(); } + $fileInput[0].value = ''; }); }()); diff --git a/app/controllers/protocols_controller.rb b/app/controllers/protocols_controller.rb index f70175b69..f810c7411 100644 --- a/app/controllers/protocols_controller.rb +++ b/app/controllers/protocols_controller.rb @@ -791,7 +791,7 @@ class ProtocolsController < ApplicationController asset_file_name = asset_guid.to_s + File.extname(asset.file_file_name).to_s ostream.put_next_entry("#{step_dir}/#{asset_file_name}") - input_file = asset.open + input_file = asset.file.download ostream.print(input_file.read) input_file.close end diff --git a/app/controllers/tiny_mce_assets_controller.rb b/app/controllers/tiny_mce_assets_controller.rb index bbc921bfa..2e64a4248 100644 --- a/app/controllers/tiny_mce_assets_controller.rb +++ b/app/controllers/tiny_mce_assets_controller.rb @@ -1,16 +1,19 @@ # frozen_string_literal: true class TinyMceAssetsController < ApplicationController - def create image = params.fetch(:file) { render_404 } - tiny_img = TinyMceAsset.new(image: image, - team_id: current_team.id, - saved: false) - if tiny_img.save + tiny_img = TinyMceAsset.new(team_id: current_team.id, saved: false) + + tiny_img.transaction do + tiny_img.save! + tiny_img.image.attach(io: image, filename: image.original_filename) + end + + if tiny_img.persisted? render json: { image: { - url: view_context.image_url(tiny_img.url(:large)), + url: url_for(tiny_img.image), token: Base62.encode(tiny_img.id) } }, content_type: 'text/html' @@ -20,5 +23,4 @@ class TinyMceAssetsController < ApplicationController }, status: :unprocessable_entity end end - end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 58ac7bb8f..d36d90945 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -8,40 +8,8 @@ class Users::RegistrationsController < Devise::RegistrationsController def avatar user = User.find_by_id(params[:id]) || current_user - style = params[:style] || "icon_small" - # TODO: Maybe avatar should be an Asset, so it's methods could be used, - # e.g. presigned_url in this case - redirect_to user.avatar.url(style.to_sym), status: 307 - end - - # Validates asset and then generates S3 upload posts, because - # otherwise untracked files could be uploaded to S3 - def signature - respond_to do |format| - format.json { - - # Changed avatar values are only used for pre-generating S3 key - # and user object is not persisted with this values. - current_user.empty_avatar avatar_params[:file].original_filename, - avatar_params[:file].size - - validation_asset = Asset.new(avatar_params) - if current_user.valid? && validation_asset.valid? - render json: { - posts: generate_upload_posts - } - else - if validation_asset.errors[:file].any? - # Add file content error - current_user.errors[:avatar] << validation_asset.errors[:file].first - end - render json: { - status: 'error', - errors: current_user.errors - }, status: :bad_request - end - } - end + style = params[:style] || :icon_small + redirect_to user.avatar_url(style) end def update_resource(resource, params) @@ -50,12 +18,10 @@ class Users::RegistrationsController < Devise::RegistrationsController if params.include? :change_password # Special handling if changing password params.delete(:change_password) - if ( - resource.valid_password?(params[:current_password]) and - params.include? :password and - params.include? :password_confirmation and - params[:password].blank? - ) then + if resource.valid_password?(params[:current_password]) && + params.include?(:password) && + params.include?(:password_confirmation) && + params[:password].blank? # If new password is blank and we're in process of changing # password, add error to the resource and return false resource.errors.add(:password, :blank) @@ -65,16 +31,26 @@ class Users::RegistrationsController < Devise::RegistrationsController end elsif params.include? :change_avatar params.delete(:change_avatar) - unless params.include? :avatar + if !params.include?(:avatar) resource.errors.add(:avatar, :blank) false else + temp_file = Tempfile.new('avatar', Rails.root.join('tmp')) + begin + temp_file.binmode + temp_file.write(Base64.decode64(params[:avatar].sub(%r{^data:image\/jpeg\;base64,}, ''))) + temp_file.rewind + resource.avatar.attach(io: temp_file, filename: 'avatar.jpg') + ensure + temp_file.close + temp_file.unlink + end + params.delete(:avatar) resource.update_without_password(params) end - elsif params.include? :email or params.include? :password + elsif params.include?(:email) || params.include?(:password) # For changing email or password, validate current_password resource.update_with_password(params) - else # For changing some attributes, no current_password validation # is required @@ -235,7 +211,6 @@ class Users::RegistrationsController < Devise::RegistrationsController :full_name, :initials, :avatar, - :avatar_file_name, :email, :password, :password_confirmation, @@ -251,45 +226,6 @@ class Users::RegistrationsController < Devise::RegistrationsController ) end - # Generates posts for uploading files (many sizes of same file) - # to S3 server - def generate_upload_posts - posts = [] - file_size = current_user.avatar_file_size - content_type = current_user.avatar_content_type - s3_post = S3_BUCKET.presigned_post( - key: current_user.avatar.path[1..-1], - success_action_status: '201', - acl: 'private', - storage_class: "STANDARD", - content_length_range: file_size..file_size, - content_type: content_type - ) - posts.push({ - url: s3_post.url, - fields: s3_post.fields - }) - - current_user.avatar.options[:styles].each do |style, option| - s3_post = S3_BUCKET.presigned_post( - key: current_user.avatar.path(style)[1..-1], - success_action_status: '201', - acl: 'public-read', - storage_class: "REDUCED_REDUNDANCY", - content_length_range: 1..Rails.configuration.x.file_max_size_mb.megabytes, - content_type: content_type - ) - posts.push({ - url: s3_post.url, - fields: s3_post.fields, - style_option: option, - mime_type: content_type - }) - end - - posts - end - private def layout diff --git a/app/controllers/zip_exports_controller.rb b/app/controllers/zip_exports_controller.rb index e30f1300a..5c8d5e669 100644 --- a/app/controllers/zip_exports_controller.rb +++ b/app/controllers/zip_exports_controller.rb @@ -5,12 +5,10 @@ class ZipExportsController < ApplicationController before_action :check_download_permissions, except: :file_expired def download - if @zip_export.stored_on_s3? - redirect_to @zip_export.presigned_url(download: true), status: 307 + if !@zip_export.zip_file.attached? + render_404 else - send_file @zip_export.zip_file.path, - filename: URI.unescape(@zip_export.zip_file_file_name), - type: 'application/zip' + redirect_to rails_blob_path(@zip_export.zip_file, disposition: 'attachment') end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 56ef2b58c..d7b26e533 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -197,11 +197,7 @@ module ApplicationHelper def user_avatar_absolute_url(user, style) begin unless missing_avatar(user, style) - image = if user.avatar.options[:storage].to_sym == :s3 - URI.parse(user.avatar.url(style)).open.to_a.join - else - File.open(user.avatar.path(style)).to_a.join - end + image = File.open(user.avatar_variant(style)) encoded_data = Base64.strict_encode64(image) avatar_base64 = "data:#{user.avatar_content_type};base64,#{encoded_data}" return avatar_base64 diff --git a/app/helpers/reports_helper.rb b/app/helpers/reports_helper.rb index f6fce8d87..c43ef03fe 100644 --- a/app/helpers/reports_helper.rb +++ b/app/helpers/reports_helper.rb @@ -98,19 +98,7 @@ module ReportsHelper # "Hack" to omit file preview URL because of WKHTML issues def report_image_asset_url(asset, type = :asset, klass = nil) - prefix = '' - if ENV['PAPERCLIP_STORAGE'].present? && - ENV['MAIL_SERVER_URL'].present? && - ENV['PAPERCLIP_STORAGE'] == 'filesystem' - prefix = ENV['MAIL_SERVER_URL'] - end - if !prefix.empty? && - !prefix.include?('http://') && - !prefix.include?('https://') - prefix = "http://#{prefix}" - end - size = type == :tiny_mce_asset ? :large : :medium - url = prefix + asset.url(size, timeout: Constants::URL_LONG_EXPIRE_TIME) + url = type == :tiny_mce_asset ? asset.preview : asset.large_preview image_tag(url, class: klass) end diff --git a/app/models/asset.rb b/app/models/asset.rb index 44c0f0be8..29bce0908 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -83,7 +83,6 @@ class Asset < ApplicationRecord after_validation :filter_paperclip_errors # Needed because Paperclip validatates on creation after_initialize :filter_paperclip_errors, if: :new_record? - before_destroy :paperclip_delete, prepend: true after_save { result&.touch; step&.touch } attr_accessor :file_content, :file_info, :preview_cached, :in_template @@ -330,23 +329,6 @@ class Asset < ApplicationRecord end end - # Workaround for making Paperclip work with asset deletion (might be because - # of how the asset models are implemented) - def paperclip_delete - report_elements.destroy_all - asset_text_datum.destroy if asset_text_datum.present? - # Nullify needed to force paperclip file deletion - file = nil - save && reload - end - - def destroy - super() - # Needed, otherwise the object isn't deleted, because of how the asset - # models are implemented - delete - end - # If team is provided, its space_taken # is updated as well def update_estimated_size(team = nil) @@ -369,43 +351,6 @@ class Asset < ApplicationRecord end end - def url(style = :original, timeout: Constants::URL_SHORT_EXPIRE_TIME) - if file.is_stored_on_s3? && !file.processing? - presigned_url(style, timeout: timeout) - else - file.url(style) - end - end - - # When using S3 file upload, we can limit file accessibility with url signing - def presigned_url(style = :original, - download: false, - timeout: Constants::URL_SHORT_EXPIRE_TIME) - if file.is_stored_on_s3? - if download - download_arg = 'attachment; filename=' + URI.escape(file_file_name) - else - download_arg = nil - end - - signer = Aws::S3::Presigner.new(client: S3_BUCKET.client) - signer.presigned_url(:get_object, - bucket: S3_BUCKET.name, - key: file.path(style)[1..-1], - expires_in: timeout, - # this response header forces object download - response_content_disposition: download_arg) - end - end - - def open - if file.is_stored_on_s3? - Kernel.open(presigned_url, "rb") - else - File.open(file.path, "rb") - end - end - # Preserving attachments (on client-side) between failed validations # (only usable for small/few files!). # Needs to be called before save method and view needs to have diff --git a/app/models/concerns/tiny_mce_images.rb b/app/models/concerns/tiny_mce_images.rb index a7965d4e8..84f5ab59b 100644 --- a/app/models/concerns/tiny_mce_images.rb +++ b/app/models/concerns/tiny_mce_images.rb @@ -18,23 +18,17 @@ module TinyMceImages description = TinyMceAsset.update_old_tinymce(description, self) tiny_mce_assets.each do |tm_asset| - tmp_f = Tempfile.open(tm_asset.image_file_name, Rails.root.join('tmp')) - begin - tm_asset.image.copy_to_local_file(:large, tmp_f.path) - encoded_tm_asset = Base64.strict_encode64(tmp_f.read) - new_tm_asset_src = "data:image/jpg;base64,#{encoded_tm_asset}" - html_description = Nokogiri::HTML(description) - tm_asset_to_update = html_description.css( - "img[data-mce-token=\"#{Base62.encode(tm_asset.id)}\"]" - )[0] - next unless tm_asset_to_update + tm_asset_key = tm_asset.image.preview.key + encoded_tm_asset = Base64.strict_encode64(tm_asset.image.service.download(tm_asset_key)) + new_tm_asset_src = "data:image/jpg;base64,#{encoded_tm_asset}" + html_description = Nokogiri::HTML(description) + tm_asset_to_update = html_description.css( + "img[data-mce-token=\"#{Base62.encode(tm_asset.id)}\"]" + )[0] + next unless tm_asset_to_update - tm_asset_to_update.attributes['src'].value = new_tm_asset_src - description = html_description.css('body').inner_html.to_s - ensure - tmp_f.close - tmp_f.unlink - end + tm_asset_to_update.attributes['src'].value = new_tm_asset_src + description = html_description.css('body').inner_html.to_s end description end @@ -72,12 +66,15 @@ module TinyMceImages cloned_img_ids = [] tiny_mce_assets.each do |tiny_img| tiny_img_clone = TinyMceAsset.new( - image: tiny_img.image, estimated_size: tiny_img.estimated_size, object: target, team: team ) - tiny_img_clone.save! + + tiny_img_clone.transaction do + tiny_img_clone.save! + tiny_img_clone.image.attach(io: tiny_img.image.open, filename: tiny_img.image.filename.to_s) + end target.tiny_mce_assets << tiny_img_clone cloned_img_ids << [tiny_img.id, tiny_img_clone.id] @@ -86,7 +83,7 @@ module TinyMceImages end def copy_unknown_tiny_mce_images - asset_team_id = Team.find_by_object(self) + asset_team_id = Team.find_by_object(self).id return unless asset_team_id object_field = Extends::RICH_TEXT_FIELD_MAPPINGS[self.class.name] @@ -97,19 +94,27 @@ module TinyMceImages if image['data-mce-token'] asset = TinyMceAsset.find_by_id(Base62.decode(image['data-mce-token'])) - next if asset && asset.object == self + next if asset && asset.object == self && asset.team_id != asset_team_id new_image = asset.image + new_image_filename = new_image.image.filename.to_s else - new_image = URI.parse(image['src']) + # We need implement size and type checks here + new_image = URI.parse(image['src']).open + new_image_filename = asset.class.generate_unique_secure_token + '.jpg' + # new_image_filename = File.basename(new_image.base_uri.path) end new_asset = TinyMceAsset.create( - image: new_image, object: self, team_id: asset_team_id ) + new_asset.transaction do + new_asset.save! + new_asset.image.attach(io: new_image, filename: new_image_filename) + end + image['src'] = '' image['class'] = 'img-responsive' image['data-mce-token'] = Base62.encode(new_asset.id) diff --git a/app/models/team.rb b/app/models/team.rb index 054ba05de..dbc88f7da 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -327,14 +327,16 @@ class Team < ApplicationRecord end def self.find_by_object(obj) - case obj.class.name - when 'Protocol' - obj.team_id - when 'MyModule', 'Step' - obj.protocol.team_id - when 'ResultText' - obj.result.my_module.protocol.team_id - end + find( + case obj.class.name + when 'Protocol' + obj.team_id + when 'MyModule', 'Step' + obj.protocol.team_id + when 'ResultText' + obj.result.my_module.protocol.team_id + end + ) end private diff --git a/app/models/temp_file.rb b/app/models/temp_file.rb index 134dfa830..8ed198c35 100644 --- a/app/models/temp_file.rb +++ b/app/models/temp_file.rb @@ -3,8 +3,7 @@ class TempFile < ApplicationRecord validates :session_id, presence: true - has_attached_file :file - do_not_validate_attachment_file_type :file + has_one_attached :file class << self def destroy_obsolete(temp_file_id) diff --git a/app/models/tiny_mce_asset.rb b/app/models/tiny_mce_asset.rb index 8e4c0b287..701f7e825 100644 --- a/app/models/tiny_mce_asset.rb +++ b/app/models/tiny_mce_asset.rb @@ -4,7 +4,7 @@ class TinyMceAsset < ApplicationRecord extend ProtocolsExporter attr_accessor :reference before_create :set_reference, optional: true - after_create :update_estimated_size, :self_destruct + after_create :calculate_estimated_size, :self_destruct after_destroy :release_team_space belongs_to :team, inverse_of: :tiny_mce_assets, optional: true @@ -17,20 +17,23 @@ class TinyMceAsset < ApplicationRecord belongs_to :object, polymorphic: true, optional: true, inverse_of: :tiny_mce_assets - has_attached_file :image, - styles: { large: [Constants::LARGE_PIC_FORMAT, :jpg] }, - convert_options: { large: '-quality 100 -strip' } - validates_attachment_content_type :image, - content_type: %r{^image/#{ Regexp.union( - Constants::WHITELISTED_IMAGE_TYPES - ) }} - validates_attachment :image, - presence: true, - size: { - less_than: Rails.configuration.x\ - .file_max_size_mb.megabytes - } + has_one_attached :image + + # has_attached_file :image, + # styles: { large: [Constants::LARGE_PIC_FORMAT, :jpg] }, + # convert_options: { large: '-quality 100 -strip' } + + # validates_attachment_content_type :image, + # content_type: %r{^image/#{ Regexp.union( + # Constants::WHITELISTED_IMAGE_TYPES + # ) }} + # validates_attachment :image, + # presence: true, + # size: { + # less_than: Rails.configuration.x\ + # .file_max_size_mb.megabytes + # } validates :estimated_size, presence: true def self.update_images(object, images) @@ -58,48 +61,17 @@ class TinyMceAsset < ApplicationRecord tm_assets = description.css('img[data-mce-token]') tm_assets.each do |tm_asset| asset_id = tm_asset.attr('data-mce-token') - new_asset_url = find_by_id(Base62.decode(asset_id)) - if new_asset_url - tm_asset.attributes['src'].value = new_asset_url.url + new_asset = obj.tiny_mce_assets.find_by_id(Base62.decode(asset_id)) + if new_asset + tm_asset.attributes['src'].value = Rails.application.routes.url_helpers.url_for(new_asset.image) tm_asset['class'] = 'img-responsive' end end description.css('body').inner_html.to_s end - def presigned_url(style = :large, - download: false, - timeout: Constants::URL_LONG_EXPIRE_TIME) - if stored_on_s3? - download_arg = ('attachment; filename=' + CGI.escape(image_file_name) if download) - - signer = Aws::S3::Presigner.new(client: S3_BUCKET.client) - signer.presigned_url(:get_object, - bucket: S3_BUCKET.name, - key: image.path(style)[1..-1], - expires_in: timeout, - response_content_disposition: download_arg) - end - end - - def stored_on_s3? - image.options[:storage].to_sym == :s3 - end - - def url(style = :large, timeout: Constants::URL_LONG_EXPIRE_TIME) - if image.is_stored_on_s3? - presigned_url(style, timeout: timeout) - else - image.url(style) - end - end - - def open - if image.is_stored_on_s3? - Kernel.open(presigned_url, 'rb') - else - File.open(image.path, 'rb') - end + def preview + image.variant(resize: Constants::LARGE_PIC_FORMAT) end def self.delete_unsaved_image(id) @@ -107,6 +79,21 @@ class TinyMceAsset < ApplicationRecord asset.destroy if asset && !asset.saved end + def self.update_estimated_size(id) + asset = find_by_id(id) + return unless asset&.image&.attached? + + size = asset.image.blob.byte_size + return if size.blank? + + e_size = size * Constants::ASSET_ESTIMATED_SIZE_FACTOR + asset.update(estimated_size: e_size) + Rails.logger.info "Asset #{id}: Estimated size successfully calculated" + # update team space taken + asset.team.take_space(e_size) + asset.team.save + end + def self.update_old_tinymce(description, obj = nil) return description unless description @@ -133,11 +120,9 @@ class TinyMceAsset < ApplicationRecord if exists? order(:id).each do |tiny_mce_asset| asset_guid = get_guid(tiny_mce_asset.id) - asset_file_name = - "rte-#{asset_guid.to_s + - File.extname(tiny_mce_asset.image_file_name).to_s}" + asset_file_name = "rte-#{asset_guid.to_s + File.extname(tiny_mce_asset.image.filename.to_s)}" ostream.put_next_entry("#{dir}/#{asset_file_name}") - input_file = tiny_mce_asset.open + input_file = tiny_mce_asset.image.download ostream.print(input_file.read) input_file.close end @@ -161,12 +146,17 @@ class TinyMceAsset < ApplicationRecord return false unless team_id tiny_img_clone = TinyMceAsset.new( - image: image, estimated_size: estimated_size, object: obj, team_id: team_id ) - tiny_img_clone.save! + + tiny_img_clone.transaction do + tiny_img_clone.save! + tiny_img_clone.image.attach(io: image.download, filename: image.filename.to_s) + end + + return false unless tiny_img_clone.persisted? obj.tiny_mce_assets << tiny_img_clone # Prepare array of image to update @@ -187,15 +177,8 @@ class TinyMceAsset < ApplicationRecord TinyMceAsset.delay(queue: :assets, run_at: 1.days.from_now).delete_unsaved_image(id) end - def update_estimated_size - return if image_file_size.blank? - - es = image_file_size * Constants::ASSET_ESTIMATED_SIZE_FACTOR - update(estimated_size: es) - Rails.logger.info "Asset #{id}: Estimated size successfully calculated" - # update team space taken - team.take_space(es) - team.save + def calculate_estimated_size + TinyMceAsset.delay(queue: :assets, run_at: 5.minutes.from_now).update_estimated_size(id) end def release_team_space diff --git a/app/models/user.rb b/app/models/user.rb index 8228f67e0..d7a6a215a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,14 +15,17 @@ class User < ApplicationRecord :timeoutable, :omniauthable, omniauth_providers: Extends::OMNIAUTH_PROVIDERS, stretches: Constants::PASSWORD_STRETCH_FACTOR - has_attached_file :avatar, - styles: { - medium: Constants::MEDIUM_PIC_FORMAT, - thumb: Constants::THUMB_PIC_FORMAT, - icon: Constants::ICON_PIC_FORMAT, - icon_small: Constants::ICON_SMALL_PIC_FORMAT - }, - default_url: Constants::DEFAULT_AVATAR_URL + + has_one_attached :avatar + + # has_attached_file :avatar, + # styles: { + # medium: Constants::MEDIUM_PIC_FORMAT, + # thumb: Constants::THUMB_PIC_FORMAT, + # icon: Constants::ICON_PIC_FORMAT, + # icon_small: Constants::ICON_SMALL_PIC_FORMAT + # }, + # default_url: Constants::DEFAULT_AVATAR_URL auto_strip_attributes :full_name, :initials, nullify: false validates :full_name, @@ -35,10 +38,10 @@ class User < ApplicationRecord presence: true, length: { maximum: Constants::EMAIL_MAX_LENGTH } - validates_attachment :avatar, - :content_type => { :content_type => ["image/jpeg", "image/png"] }, - size: { less_than: Constants::AVATAR_MAX_SIZE_MB.megabyte, - message: I18n.t('client_api.user.avatar_too_big') } + # validates_attachment :avatar, + # :content_type => { :content_type => ["image/jpeg", "image/png"] }, + # size: { less_than: Constants::AVATAR_MAX_SIZE_MB.megabyte, + # message: I18n.t('client_api.user.avatar_too_big') } validate :time_zone_check store_accessor :settings, :time_zone, :notifications_settings @@ -229,7 +232,7 @@ class User < ApplicationRecord # If other errors besides parameter "avatar" exist, # they will propagate to "avatar" also, so remove them # and put all other (more specific ones) in it - after_validation :filter_paperclip_errors + # after_validation :filter_paperclip_errors before_destroy :destroy_notifications @@ -249,6 +252,26 @@ class User < ApplicationRecord @avatar_remote_url = url_value end + def avatar_variant(style) + format = case style.to_sym + when :medium + Constants::MEDIUM_PIC_FORMAT + when :thumb + Constants::THUMB_PIC_FORMAT + when :icon + Constants::ICON_PIC_FORMAT + when :icon_small + Constants::ICON_SMALL_PIC_FORMAT + else + Constants::ICON_SMALL_PIC_FORMAT + end + avatar.variant(resize: format) + end + + def avatar_url(style) + Rails.application.routes.url_helpers.url_for(avatar_variant(style)) + end + def date_format settings[:date_format] || Constants::DEFAULT_DATE_FORMAT end diff --git a/app/models/zip_export.rb b/app/models/zip_export.rb index c44590437..27ede12ef 100644 --- a/app/models/zip_export.rb +++ b/app/models/zip_export.rb @@ -22,71 +22,45 @@ class ZipExport < ApplicationRecord belongs_to :user, optional: true # Override path only for S3 - if ENV['PAPERCLIP_STORAGE'] == 's3' - s3_path = - if ENV['S3_SUBFOLDER'] - "/#{ENV['S3_SUBFOLDER']}/zip_exports/:attachment/"\ - ":id_partition/:hash/:style/:filename" - else - '/zip_exports/:attachment/:id_partition/:hash/:style/:filename' - end + # if ENV['PAPERCLIP_STORAGE'] == 's3' + # s3_path = + # if ENV['S3_SUBFOLDER'] + # "/#{ENV['S3_SUBFOLDER']}/zip_exports/:attachment/"\ + # ":id_partition/:hash/:style/:filename" + # else + # '/zip_exports/:attachment/:id_partition/:hash/:style/:filename' + # end + # + # has_attached_file :zip_file, path: s3_path + # else + # has_attached_file :zip_file + # end - has_attached_file :zip_file, path: s3_path - else - has_attached_file :zip_file - end + has_one_attached :zip_file - validates_attachment :zip_file, - content_type: { content_type: 'application/zip' } + # validates_attachment :zip_file, + # content_type: { content_type: 'application/zip' } after_create :self_destruct - # When using S3 file upload, we can limit file accessibility with url signing - def presigned_url(style = :original, - download: false, - timeout: Constants::URL_SHORT_EXPIRE_TIME) - if stored_on_s3? - if download - download_arg = 'attachment; filename=' + URI.escape(zip_file_file_name) - else - download_arg = nil - end - - signer = Aws::S3::Presigner.new(client: S3_BUCKET.client) - signer.presigned_url(:get_object, - bucket: S3_BUCKET.name, - key: zip_file.path(style)[1..-1], - expires_in: timeout, - response_content_disposition: download_arg) - end - end - - def stored_on_s3? - zip_file.options[:storage].to_sym == :s3 - end - def self.delete_expired_export(id) export = find_by_id(id) - export.destroy if export + export&.destroy end def generate_exportable_zip(user, data, type, options = {}) - I18n.backend.date_format = - user.settings[:date_format] || Constants::DEFAULT_DATE_FORMAT - zip_input_dir = FileUtils.mkdir_p( - File.join(Rails.root, "tmp/temp_zip_#{Time.now.to_i}") - ).first - zip_dir = FileUtils.mkdir_p(File.join(Rails.root, 'tmp/zip-ready')).first - zip_file = File.new( - File.join(zip_dir, "export_#{Time.now.strftime('%F %H-%M-%S_UTC')}.zip"), - 'w+' - ) + I18n.backend.date_format = user.settings[:date_format] || Constants::DEFAULT_DATE_FORMAT + zip_input_dir = FileUtils.mkdir_p(File.join(Rails.root, "tmp/temp_zip_#{Time.now.to_i}")).first + tmp_zip_dir = FileUtils.mkdir_p(File.join(Rails.root, 'tmp/zip-ready')).first + tmp_zip_name = "export_#{Time.now.strftime('%F %H-%M-%S_UTC')}.zip" + tmp_zip_file = File.new(File.join(tmp_zip_dir, tmp_zip_name), 'w+') + fill_content(zip_input_dir, data, type, options) - zip!(zip_input_dir, zip_file) - self.zip_file = File.open(zip_file) + zip!(zip_input_dir, tmp_zip_file) + zip_file.attach(io: File.open(tmp_zip_file), filename: tmp_zip_name) generate_notification(user) if save ensure - FileUtils.rm_rf([zip_input_dir, zip_file], secure: true) + FileUtils.rm_rf([zip_input_dir, tmp_zip_file], secure: true) end handle_asynchronously :generate_exportable_zip diff --git a/app/serializers/api/v1/repository_asset_value_serializer.rb b/app/serializers/api/v1/repository_asset_value_serializer.rb index c6099a3fd..ad7eb2458 100644 --- a/app/serializers/api/v1/repository_asset_value_serializer.rb +++ b/app/serializers/api/v1/repository_asset_value_serializer.rb @@ -20,10 +20,8 @@ module Api def url if !object.asset&.file&.exists? nil - elsif object.asset&.file&.is_stored_on_s3? - object.asset.presigned_url(download: true) else - object.asset.file.url + rails_blob_path(object.asset.file, disposition: 'attachment') end end end diff --git a/app/serializers/api/v1/result_asset_serializer.rb b/app/serializers/api/v1/result_asset_serializer.rb index 0e31473a0..8e24e9a3b 100644 --- a/app/serializers/api/v1/result_asset_serializer.rb +++ b/app/serializers/api/v1/result_asset_serializer.rb @@ -21,10 +21,8 @@ module Api def url if !object.asset&.file&.exists? nil - elsif object.asset&.file&.is_stored_on_s3? - object.asset.presigned_url(download: true) else - object.asset.file.url + rails_blob_path(object.asset.file, disposition: 'attachment') end end end diff --git a/app/services/experiments/generate_workflow_image_service.rb b/app/services/experiments/generate_workflow_image_service.rb index 8d0027796..39a69324f 100644 --- a/app/services/experiments/generate_workflow_image_service.rb +++ b/app/services/experiments/generate_workflow_image_service.rb @@ -78,6 +78,7 @@ module Experiments @graph.output(png: file.path) @exp.workflowimg.attach(io: file, filename: File.basename(file.path)) file.close + file.unlink @exp.touch(:workflowimg_updated_at) end end diff --git a/app/views/users/registrations/edit.html.erb b/app/views/users/registrations/edit.html.erb index ef24e8ce8..9d9cc5464 100644 --- a/app/views/users/registrations/edit.html.erb +++ b/app/views/users/registrations/edit.html.erb @@ -239,4 +239,3 @@ <%= stylesheet_pack_tag 'custom/croppie_styles' %> <%= javascript_include_tag("canvas-to-blob.min") %> <%= javascript_include_tag "users/registrations/edit" %> - diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index afdf18be1..1bd3a771b 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -90,9 +90,9 @@ class Constants # Picture size formats LARGE_PIC_FORMAT = '800x600>'.freeze MEDIUM_PIC_FORMAT = '300x300>'.freeze - THUMB_PIC_FORMAT = '100x100#'.freeze - ICON_PIC_FORMAT = '40x40#'.freeze - ICON_SMALL_PIC_FORMAT = '30x30#'.freeze + THUMB_PIC_FORMAT = '100x100'.freeze + ICON_PIC_FORMAT = '40x40'.freeze + ICON_SMALL_PIC_FORMAT = '30x30'.freeze # Hands-on-table number of starting columns and rows HANDSONTABLE_INIT_COLS_CNT = 5 diff --git a/config/initializers/filter_parameter_logging.rb b/config/initializers/filter_parameter_logging.rb index ec91e2ccf..156ba453d 100644 --- a/config/initializers/filter_parameter_logging.rb +++ b/config/initializers/filter_parameter_logging.rb @@ -1,4 +1,4 @@ # Be sure to restart your server when you modify this file. # Configure sensitive parameters which will be filtered from the log file. -Rails.application.config.filter_parameters += %i(password image) +Rails.application.config.filter_parameters += [:password, :image, /^avatar$/] diff --git a/config/routes.rb b/config/routes.rb index c9109fe76..a9a665a6a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -594,7 +594,6 @@ Rails.application.routes.draw do devise_scope :user do get 'avatar/:id/:style' => 'users/registrations#avatar', as: 'avatar' - post 'avatar_signature' => 'users/registrations#signature' get 'users/auth_token_sign_in' => 'users/sessions#auth_token_create' get 'users/sign_up_provider' => 'users/registrations#new_with_provider' post 'users/complete_sign_up_provider' => diff --git a/db/migrate/20190613134100_convert_to_active_storage.rb b/db/migrate/20190613134100_convert_to_active_storage.rb index 76593aa1b..0d651d7d4 100644 --- a/db/migrate/20190613134100_convert_to_active_storage.rb +++ b/db/migrate/20190613134100_convert_to_active_storage.rb @@ -99,8 +99,8 @@ class ConvertToActiveStorage < ActiveRecord::Migration[5.2] if ENV['PAPERCLIP_STORAGE'] == 's3' interpolate(':class/:attachment/:id_partition/:hash/original/:filename', instance, attachment) else - key = SecureRandom.uuid - File.join('storage', key.first(2), key.first(4).last(2)) + instance.class.generate_unique_secure_token + # File.join('storage', key.first(2), key.first(4).last(2)) end end diff --git a/docker-compose.yml b/docker-compose.yml index bfbdcf1a6..a89a1f446 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,7 @@ services: - .:/usr/src/app - scinote_development_bundler:/usr/local/bundle/ - scinote_development_files:/usr/src/app/public/system + - scinote_development_storage:/usr/src/app/storage webpack: build: @@ -54,3 +55,4 @@ volumes: scinote_development_postgres: scinote_development_bundler: scinote_development_files: + scinote_development_storage: diff --git a/spec/services/templates_service_spec.rb b/spec/services/templates_service_spec.rb index 8274761c1..d7799b889 100644 --- a/spec/services/templates_service_spec.rb +++ b/spec/services/templates_service_spec.rb @@ -88,7 +88,6 @@ describe TemplatesService do .to eq(tmpl_step.step_comments.size) end end - Asset.all.each(&:paperclip_delete) FileUtils.remove_dir(tmplts_dir) end end