From 7db33cc512ba3b6f5e52c71de2bde43cbc161dde Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 18 Oct 2016 15:58:45 +0800 Subject: [PATCH] FIX: Videos and audio files were not associated to the post. --- app/models/upload.rb | 6 +++ lib/cooked_post_processor.rb | 12 +----- spec/components/cooked_post_processor_spec.rb | 31 +++++++++++++-- spec/models/upload_spec.rb | 39 +++++++++++++++++-- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index b14c1d97d87..80e30c80cb2 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -230,6 +230,12 @@ class Upload < ActiveRecord::Base url = url.sub(Discourse.asset_host, "") if Discourse.asset_host.present? # when using s3, we need to replace with the absolute base url url = url.sub(SiteSetting.s3_cdn_url, Discourse.store.absolute_base_url) if SiteSetting.s3_cdn_url.present? + + # always try to get the path + if (uri = URI(url)).scheme + url = uri.path + end + Upload.find_by(url: url) end diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 8c63bc61bcf..5b1fa3a4749 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -60,16 +60,8 @@ class CookedPostProcessor def keep_reverse_index_up_to_date upload_ids = Set.new - @doc.css("a[href]").each do |a| - href = a["href"].to_s - if upload = Upload.get_from_url(href) - upload_ids << upload.id - end - end - - @doc.css("img[src]").each do |img| - src = img["src"].to_s - if upload = Upload.get_from_url(src) + @doc.css("a/@href", "img/@src").each do |media| + if upload = Upload.get_from_url(media.value) upload_ids << upload.id end end diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index cdf5431bf79..cd7eddf1b72 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -22,7 +22,7 @@ describe CookedPostProcessor do context "cooking options" do context "regular user" do - let(:post) { Fabricate(:post) } + let(:post) { Fabricate(:post) } it "doesn't omit nofollow" do cpp = CookedPostProcessor.new(post) @@ -41,14 +41,37 @@ describe CookedPostProcessor do end context ".keep_reverse_index_up_to_date" do + let(:video_upload) { Fabricate(:upload, url: '/uploads/default/1/1234567890123456.mp4' ) } + let(:image_upload) { Fabricate(:upload, url: '/uploads/default/1/1234567890123456.jpg' ) } + let(:audio_upload) { Fabricate(:upload, url: '/uploads/default/1/1234567890123456.ogg') } + let(:attachment_upload) { Fabricate(:upload, url: '/uploads/default/1/1234567890123456.csv') } - let(:post) { build(:post_with_uploads, id: 123) } + let(:raw) do + <<~RAW + Link + + + + + + RAW + end + + let(:post) { Fabricate(:post, raw: raw) } let(:cpp) { CookedPostProcessor.new(post) } it "finds all the uploads in the post" do - Upload.expects(:get_from_url).with("/uploads/default/2/2345678901234567.jpg") - Upload.expects(:get_from_url).with("/uploads/default/1/1234567890123456.jpg") cpp.keep_reverse_index_up_to_date + + expect(PostUpload.where(post: post).map(&:upload_id).sort).to eq( + [video_upload.id, image_upload.id, audio_upload.id, attachment_upload.id].sort + ) end it "cleans the reverse index up for the current post" do diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 64a487686ff..5dd981e6a02 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -124,21 +124,54 @@ describe Upload do context ".get_from_url" do let(:url) { "/uploads/default/original/3X/1/0/10f73034616a796dfd70177dc54b6def44c4ba6f.png" } - let!(:upload) { Fabricate(:upload, url: url) } + let(:upload) { Fabricate(:upload, url: url) } it "works when the file has been uploaded" do - expect(Upload.get_from_url(url)).to eq(upload) + expect(Upload.get_from_url(upload.url)).to eq(upload) end it "works when using a cdn" do begin original_asset_host = Rails.configuration.action_controller.asset_host Rails.configuration.action_controller.asset_host = 'http://my.cdn.com' - expect(Upload.get_from_url(URI.join("http://my.cdn.com", url).to_s)).to eq(upload) + + expect(Upload.get_from_url( + URI.join("http://my.cdn.com", upload.url).to_s + )).to eq(upload) ensure Rails.configuration.action_controller.asset_host = original_asset_host end end + + it "should return the right upload when using the full URL" do + expect(Upload.get_from_url( + URI.join("http://discourse.some.com:3000/", upload.url).to_s + )).to eq(upload) + end + + describe "s3 store" do + let(:path) { "/original/3X/1/0/10f73034616a796dfd70177dc54b6def44c4ba6f.png" } + let(:url) { "//#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com#{path}" } + + before do + SiteSetting.enable_s3_uploads = true + SiteSetting.s3_upload_bucket = "s3-upload-bucket" + SiteSetting.s3_access_key_id = "some key" + SiteSetting.s3_secret_access_key = "some secret key" + end + + after do + SiteSetting.enable_s3_uploads = false + end + + it "should return the right upload when using a CDN for s3" do + upload + s3_cdn_url = 'https://mycdn.slowly.net' + SiteSetting.s3_cdn_url = s3_cdn_url + + expect(Upload.get_from_url(URI.join(s3_cdn_url, path).to_s)).to eq(upload) + end + end end describe '.generate_digest' do