Merge branch 'api-project-upload' into 'master'

Add API project upload endpoint

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/4317

See merge request !2329

11 files changed, 94 insertions, 51 deletions

diff --git a/app/assets/javascripts/dropzone_input.js.coffee b/app/assets/javascripts/dropzone_input.js.coffee
index 30a35a04339..c714c0fa939 100644
--- a/app/assets/javascripts/dropzone_input.js.coffee
+++ b/app/assets/javascripts/dropzone_input.js.coffee
@@ -66,7 +66,7 @@ class @DropzoneInput
 
       success: (header, response) ->
         child = $(dropzone[0]).children("textarea")
-        $(child).val $(child).val() + formatLink(response.link) + "\n"
+        $(child).val $(child).val() + response.link.markdown + "\n"
         return
 
       error: (temp, errorMessage) ->
@@ -99,11 +99,6 @@ class @DropzoneInput
 
     child = $(dropzone[0]).children("textarea")
 
-    formatLink = (link) ->
-      text = "[#{link.alt}](#{link.url})"
-      text = "!#{text}" if link.is_image
-      text
-
     handlePaste = (event) ->
       pasteEvent = event.originalEvent
       if pasteEvent.clipboardData and pasteEvent.clipboardData.items
@@ -162,7 +157,7 @@ class @DropzoneInput
           closeAlertMessage()
 
         success: (e, textStatus, response) ->
-          insertToTextArea(filename, formatLink(response.responseJSON.link))
+          insertToTextArea(filename, response.responseJSON.link.markdown)
 
         error: (response) ->
           showError(response.responseJSON.message)
@@ -202,8 +197,3 @@ class @DropzoneInput
       e.preventDefault()
       $(@).closest('.gfm-form').find('.div-dropzone').click()
       return
-
-  formatLink: (link) ->
-    text = "[#{link.alt}](#{link.url})"
-    text = "!#{text}" if link.is_image
-    text
diff --git a/app/services/projects/download_service.rb b/app/services/projects/download_service.rb
index 99f22293d0d..6386f57fb0d 100644
--- a/app/services/projects/download_service.rb
+++ b/app/services/projects/download_service.rb
@@ -16,13 +16,7 @@ module Projects
       uploader.download!(@url)
       uploader.store!
 
-      filename = uploader.image? ? uploader.file.basename : uploader.file.filename
-
-      {
-        'alt'       => filename,
-        'url'       => uploader.secure_url,
-        'is_image'  => uploader.image?
-      }
+      uploader.to_h
     end
 
     private
diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb
index 279550d6f4a..012e82a7704 100644
--- a/app/services/projects/upload_service.rb
+++ b/app/services/projects/upload_service.rb
@@ -10,13 +10,7 @@ module Projects
       uploader = FileUploader.new(@project)
       uploader.store!(@file)
 
-      filename = uploader.image? ? uploader.file.basename : uploader.file.filename
-
-      {
-        alt:      filename,
-        url:      uploader.secure_url,
-        is_image: uploader.image?
-      }
+      uploader.to_h
     end
 
     private
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index ac920119a85..86d24469e05 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -30,4 +30,19 @@ class FileUploader < CarrierWave::Uploader::Base
   def secure_url
     File.join("/uploads", @secret, file.filename)
   end
+
+  def to_h
+    filename = image? ? self.file.basename : self.file.filename
+    escaped_filename = filename.gsub("]", "\\]")
+
+    markdown = "[#{escaped_filename}](#{self.secure_url})"
+    markdown.prepend("!") if image?
+
+    {
+      alt:      filename,
+      url:      self.secure_url,
+      is_image: image?,
+      markdown: markdown
+    }
+  end
 end
diff --git a/doc/api/projects.md b/doc/api/projects.md
index 0ca81ffd49e..37d74216c1b 100644
--- a/doc/api/projects.md
+++ b/doc/api/projects.md
@@ -482,6 +482,34 @@ Parameters:
 
 - `id` (required) - The ID of a project
 
+## Uploads
+
+### Upload a file
+
+Uploads a file to the specified project to be used in an issue or merge request description, or a comment.
+
+```
+POST /projects/:id/uploads
+```
+
+Parameters:
+
+- `id` (required) - The ID of the project
+- `file` (required) - The file to be uploaded
+
+```json
+{
+  "alt": "dk",
+  "url": "/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png",
+  "is_image": true,
+  "markdown": "![dk](/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png)"
+}
+```
+
+**Note**: The returned `url` is relative to the project path.
+In Markdown contexts, the link is automatically expanded when the format in `markdown` is used.
+
+
 ## Team members
 
 ### List project team members
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 0781236cf6d..8b1390e3289 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -269,7 +269,7 @@ module API
       # Remove a forked_from relationship
       #
       # Parameters:
-      # id: (required) - The ID of the project being marked as a fork
+      #   id: (required) - The ID of the project being marked as a fork
       # Example Request:
       #  DELETE /projects/:id/fork
       delete ":id/fork" do
@@ -278,6 +278,16 @@ module API
           user_project.forked_project_link.destroy
         end
       end
+
+      # Upload a file
+      #
+      # Parameters:
+      #   id: (required) - The ID of the project
+      #   file: (required) - The file to be uploaded
+      post ":id/uploads" do
+        ::Projects::UploadService.new(user_project, params[:file]).execute
+      end
+
       # search for projects current_user has access to
       #
       # Parameters:
diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index 2b252b32887..2ca21af5bc8 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -74,7 +74,7 @@ module Gitlab
 
       def sent_notification
         return nil unless reply_key
-        
+
         SentNotification.for(reply_key)
       end
 
@@ -82,10 +82,7 @@ module Gitlab
         attachments = Email::AttachmentUploader.new(message).execute(sent_notification.project)
 
         attachments.each do |link|
-          text = "[#{link[:alt]}](#{link[:url]})"
-          text.prepend("!") if link[:is_image]
-
-          reply << "\n\n#{text}"
+          reply << "\n\n#{link[:markdown]}"
         end
 
         reply
diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb
index 403ebeec474..db580b5e578 100644
--- a/lib/gitlab/fogbugz_import/importer.rb
+++ b/lib/gitlab/fogbugz_import/importer.rb
@@ -232,9 +232,7 @@ module Gitlab
 
         return nil if res.nil?
 
-        text = "[#{res['alt']}](#{res['url']})"
-        text = "!#{text}" if res['is_image']
-        text
+        res[:markdown]
       end
 
       def build_attachment_url(rel_url)
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index b535413bbd4..abe179cd4af 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -42,7 +42,7 @@ describe Gitlab::Email::Receiver, lib: true do
   context "when the email was auto generated" do
     let!(:reply_key) { '636ca428858779856c226bb145ef4fad' }
     let!(:email_raw) { fixture_file("emails/auto_reply.eml") }
-    
+
     it "raises an AutoGeneratedEmailError" do
       expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::AutoGeneratedEmailError)
     end
@@ -90,7 +90,7 @@ describe Gitlab::Email::Receiver, lib: true do
 
   context "when the reply is blank" do
     let!(:email_raw) { fixture_file("emails/no_content_reply.eml") }
-    
+
     it "raises an EmptyEmailError" do
       expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError)
     end
@@ -107,13 +107,16 @@ describe Gitlab::Email::Receiver, lib: true do
   end
 
   context "when everything is fine" do
+    let(:markdown) { "![image](uploads/image.png)" }
+
     before do
       allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return(
         [
           {
             url: "uploads/image.png",
             is_image: true,
-            alt: "image"
+            alt: "image",
+            markdown: markdown
           }
         ]
       )
@@ -132,7 +135,7 @@ describe Gitlab::Email::Receiver, lib: true do
 
       note = noteable.notes.last
 
-      expect(note.note).to include("![image](uploads/image.png)")
+      expect(note.note).to include(markdown)
     end
   end
 end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index ab2530859ea..6f4c336b66c 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -353,6 +353,20 @@ describe API::API, api: true  do
     end
   end
 
+  describe "POST /projects/:id/uploads" do
+    before { project }
+
+    it "uploads the file and returns its info" do
+      post api("/projects/#{project.id}/uploads", user), file: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")
+
+      expect(response.status).to be(201)
+      expect(json_response['alt']).to eq("dk")
+      expect(json_response['url']).to start_with("/uploads/")
+      expect(json_response['url']).to end_with("/dk.png")
+      expect(json_response['is_image']).to eq(true)
+    end
+  end
+
   describe 'GET /projects/:id' do
     before { project }
     before { project_member }
diff --git a/spec/services/projects/download_service_spec.rb b/spec/services/projects/download_service_spec.rb
index 5ceed5af9a5..f252e2c5902 100644
--- a/spec/services/projects/download_service_spec.rb
+++ b/spec/services/projects/download_service_spec.rb
@@ -33,12 +33,12 @@ describe Projects::DownloadService, services: true do
           @link_to_file = download_file(@project, url)
         end
 
-        it { expect(@link_to_file).to have_key('alt') }
-        it { expect(@link_to_file).to have_key('url') }
-        it { expect(@link_to_file).to have_key('is_image') }
-        it { expect(@link_to_file['is_image']).to be true }
-        it { expect(@link_to_file['url']).to match('rails_sample.jpg') }
-        it { expect(@link_to_file['alt']).to eq('rails_sample') }
+        it { expect(@link_to_file).to have_key(:alt) }
+        it { expect(@link_to_file).to have_key(:url) }
+        it { expect(@link_to_file).to have_key(:is_image) }
+        it { expect(@link_to_file[:is_image]).to be true }
+        it { expect(@link_to_file[:url]).to match('rails_sample.jpg') }
+        it { expect(@link_to_file[:alt]).to eq('rails_sample') }
       end
 
       context 'a txt file' do
@@ -47,12 +47,12 @@ describe Projects::DownloadService, services: true do
           @link_to_file = download_file(@project, url)
         end
 
-        it { expect(@link_to_file).to have_key('alt') }
-        it { expect(@link_to_file).to have_key('url') }
-        it { expect(@link_to_file).to have_key('is_image') }
-        it { expect(@link_to_file['is_image']).to be false }
-        it { expect(@link_to_file['url']).to match('doc_sample.txt') }
-        it { expect(@link_to_file['alt']).to eq('doc_sample.txt') }
+        it { expect(@link_to_file).to have_key(:alt) }
+        it { expect(@link_to_file).to have_key(:url) }
+        it { expect(@link_to_file).to have_key(:is_image) }
+        it { expect(@link_to_file[:is_image]).to be false }
+        it { expect(@link_to_file[:url]).to match('doc_sample.txt') }
+        it { expect(@link_to_file[:alt]).to eq('doc_sample.txt') }
       end
     end
   end