Merge pull request #4506 from tgxworld/fix_video_and_audio_uploads

FIX: Videos and audio files were not associated to the post.

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
 

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

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
+      <a href="#{attachment_upload.url}">Link</a>
+      <img src="#{image_upload.url}">
+
+      <video width="100%" height="100%" controls>
+        <source src="http://myforum.com#{video_upload.url}">
+        <a href="http://myforum.com#{video_upload.url}">http://myforum.com#{video_upload.url}</a>
+      </video>
+
+      <audio controls>
+        <source src="http://myforum.com#{audio_upload.url}">
+        <a href="http://myforum.com#{audio_upload.url}">http://myforum.com#{audio_upload.url}</a>
+      </audio>
+      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

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