about summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/api/v1/streaming_controller.rb14
-rw-r--r--app/models/account.rb7
-rw-r--r--app/models/concerns/remotable.rb2
-rw-r--r--config/initializers/paperclip.rb19
-rw-r--r--spec/models/account_spec.rb4
-rw-r--r--spec/models/concerns/remotable_spec.rb13
6 files changed, 41 insertions, 18 deletions
diff --git a/app/controllers/api/v1/streaming_controller.rb b/app/controllers/api/v1/streaming_controller.rb
index 66b812e76..ebb17608c 100644
--- a/app/controllers/api/v1/streaming_controller.rb
+++ b/app/controllers/api/v1/streaming_controller.rb
@@ -5,11 +5,17 @@ class Api::V1::StreamingController < Api::BaseController
 
   def index
     if Rails.configuration.x.streaming_api_base_url != request.host
-      uri = URI.parse(request.url)
-      uri.host = URI.parse(Rails.configuration.x.streaming_api_base_url).host
-      redirect_to uri.to_s, status: 301
+      redirect_to streaming_api_url, status: 301
     else
-      raise ActiveRecord::RecordNotFound
+      not_found
     end
   end
+
+  private
+
+  def streaming_api_url
+    Addressable::URI.parse(request.url).tap do |uri|
+      uri.host = Addressable::URI.parse(Rails.configuration.x.streaming_api_base_url).host
+    end.to_s
+  end
 end
diff --git a/app/models/account.rb b/app/models/account.rb
index 01d45e36c..2f43f337f 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -310,10 +310,9 @@ class Account < ApplicationRecord
   def save_with_optional_media!
     save!
   rescue ActiveRecord::RecordInvalid
-    self.avatar              = nil
-    self.header              = nil
-    self[:avatar_remote_url] = ''
-    self[:header_remote_url] = ''
+    self.avatar = nil
+    self.header = nil
+
     save!
   end
 
diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb
index 082302619..b7a476c87 100644
--- a/app/models/concerns/remotable.rb
+++ b/app/models/concerns/remotable.rb
@@ -18,7 +18,7 @@ module Remotable
           return
         end
 
-        return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.blank? || self[attribute_name] == url
+        return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.blank? || (self[attribute_name] == url && send("#{attachment_name}_file_name").present?)
 
         begin
           Request.new(:get, url).perform do |response|
diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb
index a0253f4bc..d3602e655 100644
--- a/config/initializers/paperclip.rb
+++ b/config/initializers/paperclip.rb
@@ -1,10 +1,11 @@
 # frozen_string_literal: true
 
-Paperclip.options[:read_timeout] = 60
-
 Paperclip.interpolates :filename do |attachment, style|
-  return attachment.original_filename if style == :original
-  [basename(attachment, style), extension(attachment, style)].delete_if(&:blank?).join('.')
+  if style == :original
+    attachment.original_filename
+  else
+    [basename(attachment, style), extension(attachment, style)].delete_if(&:blank?).join('.')
+  end
 end
 
 Paperclip::Attachment.default_options.merge!(
@@ -24,17 +25,21 @@ if ENV['S3_ENABLED'] == 'true'
     storage: :s3,
     s3_protocol: s3_protocol,
     s3_host_name: s3_hostname,
+
     s3_headers: {
       'X-Amz-Multipart-Threshold' => ENV.fetch('S3_MULTIPART_THRESHOLD') { 15.megabytes }.to_i,
       'Cache-Control' => 'public, max-age=315576000, immutable',
     },
+
     s3_permissions: ENV.fetch('S3_PERMISSION') { 'public-read' },
     s3_region: s3_region,
+
     s3_credentials: {
       bucket: ENV['S3_BUCKET'],
       access_key_id: ENV['AWS_ACCESS_KEY_ID'],
       secret_access_key: ENV['AWS_SECRET_ACCESS_KEY'],
     },
+
     s3_options: {
       signature_version: ENV.fetch('S3_SIGNATURE_VERSION') { 'v4' },
       http_open_timeout: 5,
@@ -49,6 +54,7 @@ if ENV['S3_ENABLED'] == 'true'
       endpoint: ENV['S3_ENDPOINT'],
       force_path_style: true
     )
+
     Paperclip::Attachment.default_options[:url] = ':s3_path_url'
   end
 
@@ -74,6 +80,7 @@ elsif ENV['SWIFT_ENABLED'] == 'true'
       openstack_region: ENV['SWIFT_REGION'],
       openstack_cache_ttl: ENV.fetch('SWIFT_CACHE_TTL') { 60 },
     },
+
     fog_directory: ENV['SWIFT_CONTAINER'],
     fog_host: ENV['SWIFT_OBJECT_URL'],
     fog_public: true
@@ -82,7 +89,7 @@ else
   Paperclip::Attachment.default_options.merge!(
     storage: :filesystem,
     use_timestamp: true,
-    path: (ENV['PAPERCLIP_ROOT_PATH'] || ':rails_root/public/system') + '/:class/:attachment/:id_partition/:style/:filename',
-    url: (ENV['PAPERCLIP_ROOT_URL'] || '/system') + '/:class/:attachment/:id_partition/:style/:filename',
+    path: ENV.fetch('PAPERCLIP_ROOT_PATH', ':rails_root/public/system') + '/:class/:attachment/:id_partition/:style/:filename',
+    url: ENV.fetch('PAPERCLIP_ROOT_URL', '/system') + '/:class/:attachment/:id_partition/:style/:filename',
   )
 end
diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
index 3eec464bd..b2f6234cb 100644
--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@@ -126,8 +126,8 @@ RSpec.describe Account, type: :model do
       end
 
       it 'sets default avatar, header, avatar_remote_url, and header_remote_url' do
-        expect(account.avatar_remote_url).to eq ''
-        expect(account.header_remote_url).to eq ''
+        expect(account.avatar_remote_url).to eq 'https://remote.test/invalid_avatar'
+        expect(account.header_remote_url).to eq expectation.header_remote_url
         expect(account.avatar_file_name).to  eq nil
         expect(account.header_file_name).to  eq nil
       end
diff --git a/spec/models/concerns/remotable_spec.rb b/spec/models/concerns/remotable_spec.rb
index a4289cc45..99a60cbf6 100644
--- a/spec/models/concerns/remotable_spec.rb
+++ b/spec/models/concerns/remotable_spec.rb
@@ -18,6 +18,8 @@ RSpec.describe Remotable do
 
     def hoge=(arg); end
 
+    def hoge_file_name; end
+
     def hoge_file_name=(arg); end
 
     def has_attribute?(arg); end
@@ -109,12 +111,21 @@ RSpec.describe Remotable do
       end
 
       context 'foo[attribute_name] == url' do
-        it 'makes no request' do
+        it 'makes no request if file is saved' do
           allow(foo).to receive(:[]).with(attribute_name).and_return(url)
+          allow(foo).to receive(:hoge_file_name).and_return('foo.jpg')
 
           foo.hoge_remote_url = url
           expect(request).not_to have_been_requested
         end
+
+        it 'makes request if file is not saved' do
+          allow(foo).to receive(:[]).with(attribute_name).and_return(url)
+          allow(foo).to receive(:hoge_file_name).and_return(nil)
+
+          foo.hoge_remote_url = url
+          expect(request).to have_been_requested
+        end
       end
 
       context "scheme is https, parsed_url.host isn't empty, and foo[attribute_name] != url" do