Merge branch 's3_refactor'

spec/components/file_store/s3_store_spec.rb

@@ -16,6 +16,7 @@ describe FileStore::S3Store do
     SiteSetting.s3_upload_bucket = "s3-upload-bucket"
     SiteSetting.s3_access_key_id = "s3-access-key-id"
     SiteSetting.s3_secret_access_key = "s3-secret-access-key"
+    SiteSetting.enable_s3_uploads = true
   end
 
   shared_context 's3 helpers' do

spec/components/final_destination_spec.rb

@@ -292,6 +292,7 @@ describe FinalDestination do
     end
 
     it "returns true for the S3 CDN url" do
+      SiteSetting.enable_s3_uploads = true
       SiteSetting.s3_cdn_url = "https://s3.example.com"
       expect(fd("https://s3.example.com/some/thing").is_dest_valid?).to eq(true)
     end

spec/components/pretty_text_spec.rb

@@ -623,29 +623,56 @@ describe PrettyText do
     expect(PrettyText.cook(raw)).to eq(html.strip)
   end
 
-  it 'can substitute s3 cdn correctly' do
-    SiteSetting.enable_s3_uploads = true
-    SiteSetting.s3_access_key_id = "XXX"
-    SiteSetting.s3_secret_access_key = "XXX"
-    SiteSetting.s3_upload_bucket = "test"
-    SiteSetting.s3_cdn_url = "https://awesome.cdn"
+  describe 's3_cdn' do
 
-    # add extra img tag to ensure it does not blow up
-    raw = <<~HTML
-      <img>
-      <img src='https:#{Discourse.store.absolute_base_url}/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg'>
-      <img src='http:#{Discourse.store.absolute_base_url}/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg'>
-      <img src='#{Discourse.store.absolute_base_url}/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg'>
-    HTML
+    def test_s3_cdn
+      # add extra img tag to ensure it does not blow up
+      raw = <<~HTML
+        <img>
+        <img src='https:#{Discourse.store.absolute_base_url}/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg'>
+        <img src='http:#{Discourse.store.absolute_base_url}/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg'>
+        <img src='#{Discourse.store.absolute_base_url}/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg'>
+      HTML
 
-    html = <<~HTML
-      <p><img><br>
-      <img src="https://awesome.cdn/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg"><br>
-      <img src="https://awesome.cdn/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg"><br>
-      <img src="https://awesome.cdn/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg"></p>
-    HTML
+      html = <<~HTML
+        <p><img><br>
+        <img src="https://awesome.cdn/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg"><br>
+        <img src="https://awesome.cdn/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg"><br>
+        <img src="https://awesome.cdn/original/9/9/99c9384b8b6d87f8509f8395571bc7512ca3cad1.jpg"></p>
+      HTML
 
-    expect(PrettyText.cook(raw)).to eq(html.strip)
+      expect(PrettyText.cook(raw)).to eq(html.strip)
+    end
+
+    before do
+      GlobalSetting.reset_s3_cache!
+    end
+
+    after do
+      GlobalSetting.reset_s3_cache!
+    end
+
+    it 'can substitute s3 cdn when added via global setting' do
+
+      global_setting :s3_access_key_id, 'XXX'
+      global_setting :s3_secret_access_key, 'XXX'
+      global_setting :s3_bucket, 'XXX'
+      global_setting :s3_region, 'XXX'
+      global_setting :s3_cdn_url, 'https://awesome.cdn'
+
+      test_s3_cdn
+    end
+
+    it 'can substitute s3 cdn correctly' do
+      SiteSetting.s3_access_key_id = "XXX"
+      SiteSetting.s3_secret_access_key = "XXX"
+      SiteSetting.s3_upload_bucket = "test"
+      SiteSetting.s3_cdn_url = "https://awesome.cdn"
+
+      SiteSetting.enable_s3_uploads = true
+
+      test_s3_cdn
+    end
   end
 
   describe "emoji" do

spec/components/stylesheet/importer_spec.rb

@@ -20,8 +20,13 @@ describe Stylesheet::Importer do
   end
 
   it "applies S3 CDN to background category images" do
+    SiteSetting.s3_use_iam_profile = true
+    SiteSetting.s3_upload_bucket = 'test'
+    SiteSetting.s3_region = 'ap-southeast-2'
     SiteSetting.s3_cdn_url = "https://s3.cdn"
 
+    SiteSetting.enable_s3_uploads = true
+
     background = Fabricate(:upload_s3)
     category = Fabricate(:category, uploaded_background: background)
 

spec/helpers/application_helper_spec.rb

@@ -2,6 +2,50 @@ require 'rails_helper'
 
 describe ApplicationHelper do
 
+  describe "preload_script" do
+    it "provides brotli links to brotli cdn" do
+      set_cdn_url "https://awesome.com"
+      set_env "COMPRESS_BROTLI", "1"
+
+      helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'
+      link = helper.preload_script('application')
+
+      expect(link).to eq("<link rel='preload' href='https://awesome.com/brotli_asset/application.js' as='script'/>\n<script src='https://awesome.com/brotli_asset/application.js'></script>")
+    end
+
+    context "with s3 CDN" do
+      before do
+        global_setting :s3_bucket, 'test_bucket'
+        global_setting :s3_region, 'ap-australia'
+        global_setting :s3_access_key_id, '123'
+        global_setting :s3_secret_access_key, '123'
+        global_setting :s3_cdn_url, 'https://s3cdn.com'
+        set_env "COMPRESS_BROTLI", "1"
+      end
+
+      it "returns magic brotli mangling for brotli requests" do
+
+        helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'
+        link = helper.preload_script('application')
+
+        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.br.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.br.js'></script>")
+      end
+
+      it "gives s3 cdn if asset host is not set" do
+        link = helper.preload_script('application')
+
+        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.js'></script>")
+      end
+
+      it "gives s3 cdn even if asset host is set" do
+        set_cdn_url "https://awesome.com"
+        link = helper.preload_script('application')
+
+        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.js'></script>")
+      end
+    end
+  end
+
   describe "escape_unicode" do
     it "encodes tags" do
       expect(helper.escape_unicode("<tag>")).to eq("\u003ctag>")

spec/models/global_setting_spec.rb

@@ -9,6 +9,21 @@ end
 
 describe GlobalSetting do
 
+  describe '.use_s3_assets?' do
+    it 'returns false by default' do
+      expect(GlobalSetting.use_s3?).to eq(false)
+    end
+
+    it 'returns true once set' do
+      global_setting :s3_bucket, 'test_bucket'
+      global_setting :s3_region, 'ap-australia'
+      global_setting :s3_access_key_id, '123'
+      global_setting :s3_secret_access_key, '123'
+
+      expect(GlobalSetting.use_s3?).to eq(true)
+    end
+  end
+
   describe '.safe_secret_key_base' do
     it 'sets redis token if it is somehow flushed after 30 seconds' do
 

spec/models/topic_link_click_spec.rb

@@ -145,6 +145,9 @@ describe TopicLinkClick do
 
           it "works with s3 urls" do
             SiteSetting.s3_cdn_url = "https://discourse-s3-cdn.global.ssl.fastly.net"
+            SiteSetting.s3_access_key_id = 'X'
+            SiteSetting.s3_secret_access_key = 'X'
+            SiteSetting.enable_s3_uploads = true
 
             post = Fabricate(:post, topic: @topic, raw: "[test](//test.localhost/uploads/default/my-test-link)")
             TopicLink.extract_from(post)

spec/rails_helper.rb

@@ -124,6 +124,11 @@ RSpec.configure do |config|
     Sidekiq::Worker.clear_all
 
     I18n.locale = :en
+
+    if $test_cleanup_callbacks
+      $test_cleanup_callbacks.reverse_each(&:call)
+      $test_cleanup_callbacks = nil
+    end
   end
 
   class TestCurrentUserProvider < Auth::DefaultCurrentUserProvider
@@ -146,6 +151,45 @@ class TrackTimeStub
   end
 end
 
+def before_next_spec(&callback)
+  ($test_cleanup_callbacks ||= []) << callback
+end
+
+def global_setting(name, value)
+  GlobalSetting.reset_s3_cache!
+
+  GlobalSetting.stubs(name).returns(value)
+
+  before_next_spec do
+    GlobalSetting.reset_s3_cache!
+  end
+end
+
+def set_env(var, value)
+  old = ENV.fetch var, :missing
+
+  ENV[var] = value
+
+  before_next_spec do
+    if old == :missing
+      ENV.delete var
+    else
+      ENV[var] = old
+    end
+  end
+end
+
+def set_cdn_url(cdn_url)
+  global_setting :cdn_url, cdn_url
+  Rails.configuration.action_controller.asset_host = cdn_url
+  ActionController::Base.asset_host = cdn_url
+
+  before_next_spec do
+    Rails.configuration.action_controller.asset_host = nil
+    ActionController::Base.asset_host = nil
+  end
+end
+
 def freeze_time(now = Time.now)
   datetime = DateTime.parse(now.to_s)
   time = Time.parse(now.to_s)