Merge branch 'extend_markdown_upload' into 'master'

Generalizes image upload in drag and drop in markdown to all files

From https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/265 by Hannes Rosenögger.

- [x] Rebase on master when !1553 is merged in

See merge request !1530

CHANGELOG

@@ -10,6 +10,7 @@ v 7.9.0 (unreleased)
   - Save web edit in new branch
   - Fix ordering of imported but unchanged projects (Marco Wessel)
   - Mobile UI improvements: make aside content expandable
+  - Generalize image upload in drag and drop in markdown to all files (Hannes Rosenögger)
 
 v 7.8.1
   - Fix run of custom post receive hooks

Gemfile

@@ -203,6 +203,7 @@ group :development do
   gem "letter_opener"
   gem 'quiet_assets', '~> 1.0.1'
   gem 'rack-mini-profiler', require: false
+  gem "byebug"
 
   # Better errors handler
   gem 'better_errors'

Gemfile.lock

@@ -65,6 +65,9 @@ GEM
       sass (>= 3.2.19)
     browser (0.7.2)
     builder (3.2.2)
+    byebug (3.2.0)
+      columnize (~> 0.8)
+      debugger-linecache (~> 1.2)
     cal-heatmap-rails (0.0.1)
     capybara (2.2.1)
       mime-types (>= 1.16)
@@ -92,6 +95,7 @@ GEM
     coffee-script-source (1.6.3)
     colored (1.2)
     colorize (0.5.8)
+    columnize (0.9.0)
     connection_pool (2.1.0)
     coveralls (0.7.0)
       multi_json (~> 1.3)
@@ -107,6 +111,7 @@ GEM
     daemons (1.1.9)
     database_cleaner (1.3.0)
     debug_inspector (0.0.2)
+    debugger-linecache (1.2.0)
     default_value_for (3.0.0)
       activerecord (>= 3.2.0, < 5.0)
     descendants_tracker (0.0.3)
@@ -647,6 +652,7 @@ DEPENDENCIES
   binding_of_caller
   bootstrap-sass (~> 3.0)
   browser
+  byebug
   cal-heatmap-rails (~> 0.0.1)
   capybara (~> 2.2.1)
   carrierwave

app/assets/javascripts/dropzone_input.js.coffee

@@ -6,10 +6,10 @@ class @DropzoneInput
     divHover = "<div class=\"div-dropzone-hover\"></div>"
     divSpinner = "<div class=\"div-dropzone-spinner\"></div>"
     divAlert = "<div class=\"" + alertClass + "\"></div>"
-    iconPicture = "<i class=\"fa fa-picture-o div-dropzone-icon\"></i>"
+    iconPaperclip = "<i class=\"fa fa-paperclip div-dropzone-icon\"></i>"
     iconSpinner = "<i class=\"fa fa-spinner fa-spin div-dropzone-icon\"></i>"
     btnAlert = "<button type=\"button\"" + alertAttr + ">&times;</button>"
-    project_image_path_upload = window.project_image_path_upload or null
+    project_uploads_path = window.project_uploads_path or null
 
     form_textarea = $(form).find("textarea.markdown-area")
     form_textarea.wrap "<div class=\"div-dropzone\"></div>"
@@ -19,7 +19,7 @@ class @DropzoneInput
     form_dropzone = $(form).find('.div-dropzone')
     form_dropzone.parent().addClass "div-dropzone-wrapper"
     form_dropzone.append divHover
-    $(".div-dropzone-hover").append iconPicture
+    $(".div-dropzone-hover").append iconPaperclip
     form_dropzone.append divSpinner
     $(".div-dropzone-spinner").append iconSpinner
     $(".div-dropzone-spinner").css
@@ -72,13 +72,12 @@ class @DropzoneInput
       form.find(".md-preview-holder").hide()
 
     dropzone = form_dropzone.dropzone(
-      url: project_image_path_upload
+      url: project_uploads_path
       dictDefaultMessage: ""
       clickable: true
-      paramName: "markdown_img"
+      paramName: "file"
       maxFilesize: 10
       uploadMultiple: false
-      acceptedFiles: "image/jpg,image/jpeg,image/gif,image/png"
       headers:
         "X-CSRF-Token": $("meta[name=\"csrf-token\"]").attr("content")
 
@@ -132,8 +131,10 @@ class @DropzoneInput
 
     child = $(dropzone[0]).children("textarea")
 
-    formatLink = (str) ->
-      "![" + str.alt + "](" + str.url + ")"
+    formatLink = (link) ->
+      text = "[#{link.alt}](#{link.url})"
+      text = "!#{text}" if link.is_image
+      text
 
     handlePaste = (event) ->
       pasteEvent = event.originalEvent
@@ -177,9 +178,9 @@ class @DropzoneInput
 
     uploadFile = (item, filename) ->
       formData = new FormData()
-      formData.append "markdown_img", item, filename
+      formData.append "file", item, filename
       $.ajax
-        url: project_image_path_upload
+        url: project_uploads_path
         type: "POST"
         data: formData
         dataType: "json"
@@ -233,5 +234,7 @@ class @DropzoneInput
       $(@).closest('.gfm-form').find('.div-dropzone').click()
       return
 
-  formatLink: (str) ->
-    "![" + str.alt + "](" + str.url + ")"
+  formatLink: (link) ->
+    text = "[#{link.alt}](#{link.url})"
+    text = "!#{text}" if link.is_image
+    text
\ No newline at end of file

app/assets/javascripts/notes.js.coffee

@@ -39,9 +39,6 @@ class @Notes
     # reset main target form after submit
     $(document).on "ajax:complete", ".js-main-target-form", @resetMainTargetForm
 
-    # attachment button
-    $(document).on "click", ".js-choose-note-attachment-button", @chooseNoteAttachment
-
     # update the file name when an attachment is selected
     $(document).on "change", ".js-note-attachment-input", @updateFormAttachment
 
@@ -73,7 +70,6 @@ class @Notes
     $(document).off "click", ".js-note-delete"
     $(document).off "click", ".js-note-attachment-delete"
     $(document).off "ajax:complete", ".js-main-target-form"
-    $(document).off "click", ".js-choose-note-attachment-button"
     $(document).off "click", ".js-discussion-reply-button"
     $(document).off "click", ".js-add-diff-note-button"
     $(document).off "visibilitychange"
@@ -173,15 +169,6 @@ class @Notes
 
     form.find(".js-note-text").data("autosave").reset()
 
-  ###
-  Called when clicking the "Choose File" button.
-
-  Opens the file selection dialog.
-  ###
-  chooseNoteAttachment: ->
-    form = $(this).closest("form")
-    form.find(".js-note-attachment-input").click()
-
   ###
   Shows the main form and does some setup on it.
 

app/assets/stylesheets/sections/notes.scss

@@ -66,6 +66,22 @@ ul.notes {
         overflow: auto;
         word-wrap: break-word;
         @include md-typography;
+
+        a[href*="/uploads/"] {
+          &:before {
+            margin-right: 4px;
+            
+            font: normal normal normal 14px/1 FontAwesome;
+            font-size: inherit;
+            text-rendering: auto;
+            -webkit-font-smoothing: antialiased;
+            content: "\f0c6";
+          }
+
+          &:hover:before {
+            text-decoration: none;
+          }
+        }
       }
     }
     .note-header {

app/controllers/files_controller.rb

-class FilesController < ApplicationController
-  skip_before_filter :authenticate_user!, :reject_blocked
-
-  def download
-    note = Note.find(params[:id])
-    uploader = note.attachment
-
-    if uploader.file_storage?
-      if can?(current_user, :read_project, note.project)
-        disposition = uploader.image? ? 'inline' : 'attachment'
-        send_file uploader.file.path, disposition: disposition
-      else
-        not_found!
-      end
-    else
-      redirect_to uploader.url
-    end
-  end
-end

app/controllers/projects/uploads_controller.rb

 class Projects::UploadsController < Projects::ApplicationController
-  layout "project"
+  layout 'project'
 
   before_filter :project
 
+  def create
+    link_to_file = ::Projects::UploadService.new(project, params[:file]).
+      execute
+
+    respond_to do |format|
+      if link_to_file
+        format.json do
+          render json: { link: link_to_file }
+        end
+      else
+        format.json do
+          render json: 'Invalid file.', status: :unprocessable_entity
+        end
+      end
+    end
+  end
+
   def show
-    path = File.join(project.path_with_namespace, params[:secret])
-    uploader = FileUploader.new('uploads', path)
+    uploader = FileUploader.new(project, params[:secret])
+
+    return redirect_to uploader.url unless uploader.file_storage?
 
     uploader.retrieve_from_store!(params[:filename])
 
-    if uploader.file.exists?
-      # Right now, these are always images, so we can safely render them inline.
-      send_file uploader.file.path, disposition: 'inline'
-    else
-      not_found!
-    end
+    return not_found! unless uploader.file.exists?
+
+    disposition = uploader.image? ? 'inline' : 'attachment'
+    send_file uploader.file.path, disposition: disposition
   end
 end

app/controllers/projects_controller.rb

@@ -146,18 +146,6 @@ class ProjectsController < ApplicationController
     end
   end
 
-  def upload_image
-    link_to_image = ::Projects::ImageService.new(repository, params, root_url).execute
-
-    respond_to do |format|
-      if link_to_image
-        format.json { render json: { link: link_to_image } }
-      else
-        format.json { render json: 'Invalid file.', status: :unprocessable_entity }
-      end
-    end
-  end
-
   def toggle_star
     current_user.toggle_star(@project)
     @project.reload
@@ -170,15 +158,6 @@ class ProjectsController < ApplicationController
 
   private
 
-  def upload_path
-    base_dir = FileUploader.generate_dir
-    File.join(repository.path_with_namespace, base_dir)
-  end
-
-  def accepted_images
-    %w(png jpg jpeg gif)
-  end
-
   def set_title
     @title = 'New Project'
   end

app/controllers/uploads_controller.rb

 class UploadsController < ApplicationController
-  skip_before_filter :authenticate_user!, :reject_blocked
+  skip_before_filter :authenticate_user!, :reject_blocked!
   before_filter :authorize_access
 
   def show
     model = params[:model].camelize.constantize.find(params[:id])
     uploader = model.send(params[:mounted_as])
 
-    if uploader.file_storage?
-      if !model.respond_to?(:project) || can?(current_user, :read_project, model.project)
+    return not_found! if model.respond_to?(:project) && !can?(current_user, :read_project, model.project)
+
+    return redirect_to uploader.url unless uploader.file_storage?
+
+    return not_found! unless uploader.file.exists?
+
     disposition = uploader.image? ? 'inline' : 'attachment'
     send_file uploader.file.path, disposition: disposition
-      else
-        not_found!
-      end
-    else
-      redirect_to uploader.url
-    end
   end
 
   def authorize_access
     unless params[:mounted_as] == 'avatar'
-      authenticate_user! && reject_blocked
+      authenticate_user! && reject_blocked!
     end
   end
 end

app/models/group.rb

@@ -23,7 +23,7 @@ class Group < Namespace
   validate :avatar_type, if: ->(user) { user.avatar_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
-  mount_uploader :avatar, AttachmentUploader
+  mount_uploader :avatar, AvatarUploader
 
   after_create :post_create_hook
   after_destroy :post_destroy_hook

app/models/project.rb

@@ -144,7 +144,7 @@ class Project < ActiveRecord::Base
     if: ->(project) { project.avatar && project.avatar_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
-  mount_uploader :avatar, AttachmentUploader
+  mount_uploader :avatar, AvatarUploader
 
   # Scopes
   scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) }

app/models/user.rb

@@ -179,7 +179,7 @@ class User < ActiveRecord::Base
     end
   end
 
-  mount_uploader :avatar, AttachmentUploader
+  mount_uploader :avatar, AvatarUploader
 
   # Scopes
   scope :admins, -> { where(admin:  true) }

app/services/projects/image_service.rb

-module Projects
-  class ImageService < BaseService
-    include Rails.application.routes.url_helpers
-    def initialize(repository, params, root_url)
-      @repository, @params, @root_url = repository, params.dup, root_url
-    end
-
-    def execute
-      uploader = FileUploader.new('uploads', upload_path, accepted_images)
-      image = @params['markdown_img']
-
-      if image && correct_mime_type?(image)
-        alt = image.original_filename
-        uploader.store!(image)
-        link = {
-                 'alt' => File.basename(alt, '.*'),
-                 'url' => File.join(@root_url, uploader.url)
-               }
-      else
-        link = nil
-      end
-    end
-
-    protected
-
-    def upload_path
-      base_dir = FileUploader.generate_dir
-      File.join(@repository.path_with_namespace, base_dir)
-    end
-
-    def accepted_images
-      %w(png jpg jpeg gif)
-    end
-
-    def correct_mime_type?(image)
-      accepted_images.map{ |format| image.content_type.include? format }.any?
-    end
-  end
-end

app/services/projects/upload_service.rb

+module Projects
+  class UploadService < BaseService
+    def initialize(project, file)
+      @project, @file = project, file
+    end
+
+    def execute
+      return nil unless @file
+
+      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?
+      }
+    end
+  end
+end

app/uploaders/attachment_uploader.rb

@@ -3,8 +3,6 @@
 class AttachmentUploader < CarrierWave::Uploader::Base
   storage :file
 
-  after :store, :reset_events_cache
-
   def store_dir
     "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
   end
@@ -22,15 +20,7 @@ class AttachmentUploader < CarrierWave::Uploader::Base
     false
   end
 
-  def secure_url
-    Gitlab.config.gitlab.relative_url_root + "/files/#{model.class.to_s.underscore}/#{model.id}/#{file.filename}"
-  end
-
   def file_storage?
     self.class.storage == CarrierWave::Storage::File
   end
-
-  def reset_events_cache(file)
-    model.reset_events_cache if model.is_a?(User)
-  end
 end

app/uploaders/avatar_uploader.rb

+# encoding: utf-8
+
+class AvatarUploader < CarrierWave::Uploader::Base
+  storage :file
+
+  after :store, :reset_events_cache
+
+  def store_dir
+    "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
+  end
+
+  def image?
+    img_ext = %w(png jpg jpeg gif bmp tiff)
+    if file.respond_to?(:extension)
+      img_ext.include?(file.extension.downcase)
+    else
+      # Not all CarrierWave storages respond to :extension
+      ext = file.path.split('.').last.downcase
+      img_ext.include?(ext)
+    end
+  rescue
+    false
+  end
+
+  def file_storage?
+    self.class.storage == CarrierWave::Storage::File
+  end
+
+  def reset_events_cache(file)
+    model.reset_events_cache if model.is_a?(User)
+  end
+end

app/uploaders/file_uploader.rb

@@ -2,40 +2,47 @@
 class FileUploader < CarrierWave::Uploader::Base
   storage :file
 
-  def initialize(base_dir, path = '', allowed_extensions = nil)
-    @base_dir = base_dir
-    @path = path
-    @allowed_extensions = allowed_extensions
+  attr_accessor :project, :secret
+
+  def initialize(project, secret = self.class.generate_secret)
+    @project = project
+    @secret = secret
   end
 
   def base_dir
-    @base_dir
+    "uploads"
   end
 
   def store_dir
-    File.join(@base_dir, @path)
+    File.join(base_dir, @project.path_with_namespace, @secret)
   end
 
   def cache_dir
-    File.join(@base_dir, 'tmp', @path)
+    File.join(base_dir, 'tmp', @project.path_with_namespace, @secret)
   end
 
-  def extension_white_list
-    @allowed_extensions
+  def self.generate_secret
+    SecureRandom.hex
   end
 
-  def store!(file)
-    @filename = self.class.generate_filename(file)
-    super
+  def secure_url
+    File.join(Gitlab.config.gitlab.url, @project.path_with_namespace, "uploads", @secret, file.filename)
   end
 
-  def self.generate_filename(file)
-    original_filename = File.basename(file.original_filename, '.*')
-    extension = File.extname(file.original_filename)
-    new_filename = Digest::MD5.hexdigest(original_filename) + extension
+  def file_storage?
+    self.class.storage == CarrierWave::Storage::File
   end
 
-  def self.generate_dir
-    SecureRandom.hex(5)
+  def image?
+    img_ext = %w(png jpg jpeg gif bmp tiff)
+    if file.respond_to?(:extension)
+      img_ext.include?(file.extension.downcase)
+    else
+      # Not all CarrierWave storages respond to :extension
+      ext = file.path.split('.').last.downcase
+      img_ext.include?(ext)
+    end
+  rescue
+    false
   end
 end

app/views/events/event/_note.html.haml

@@ -18,9 +18,9 @@
     - note = event.target
     - if note.attachment.url
       - if note.attachment.image?
-        = link_to note.attachment.secure_url, target: '_blank' do
-          = image_tag note.attachment.secure_url, class: 'note-image-attach'
+        = link_to note.attachment.url, target: '_blank' do
+          = image_tag note.attachment.url, class: 'note-image-attach'
       - else
-        = link_to note.attachment.secure_url, target: "_blank", class: 'note-file-attach' do
+        = link_to note.attachment.url, target: "_blank", class: 'note-file-attach' do
           %i.fa.fa-paperclip
           = note.attachment_identifier

app/views/projects/_issuable_form.html.haml

@@ -23,7 +23,7 @@
           Parsed with
           #{link_to 'GitLab Flavored Markdown', help_page_path('markdown', 'markdown'), target: '_blank'}.
         .pull-right
-          Attach images (JPG, PNG, GIF) by dragging & dropping
+          Attach files by dragging & dropping
           or #{link_to 'selecting them', '#', class: 'markdown-selector' }.
 
       .clearfix

app/views/projects/issues/_form.html.haml

@@ -11,4 +11,4 @@
     e.preventDefault();
   });
 
-  window.project_image_path_upload = "#{upload_image_namespace_project_path @project.namespace, @project}";
+  window.project_uploads_path = "#{namespace_project_uploads_path @project.namespace, @project}";

app/views/projects/merge_requests/_form.html.haml

@@ -9,4 +9,4 @@
     e.preventDefault();
   });
 
-  window.project_image_path_upload = "#{upload_image_namespace_project_path @project.namespace, @project}";
+  window.project_uploads_path = "#{namespace_project_uploads_path @project.namespace, @project}";

app/views/projects/merge_requests/_new_submit.html.haml

@@ -27,7 +27,7 @@
               Parsed with
               #{link_to 'Gitlab Flavored Markdown', help_page_path('markdown', 'markdown'), target: '_blank'}.
             .pull-right
-              Attach images (JPG, PNG, GIF) by dragging & dropping
+              Attach files by dragging & dropping
               or #{link_to 'selecting them', '#', class: 'markdown-selector'}.
 
           .clearfix
@@ -113,10 +113,11 @@
     e.preventDefault();
   });
 
-  window.project_image_path_upload = "#{upload_image_namespace_project_path @project.namespace, @project}";
+  window.project_uploads_path = "#{namespace_project_uploads_path @project.namespace, @project}";
 
 :javascript
   var merge_request
   merge_request = new MergeRequest({
     action: 'commits'
   });
+

app/views/projects/milestones/_form.html.haml

@@ -25,7 +25,7 @@
             = render 'projects/zen', f: f, attr: :description, classes: 'description form-control'
             .hint
               .pull-left Milestones are parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"), target: '_blank'}.
-              .pull-left Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
+              .pull-left Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
           .clearfix
           .error-alert
     .col-md-6
@@ -51,4 +51,4 @@
     onSelect: function(dateText, inst) { $("#milestone_due_date").val(dateText) }
   }).datepicker("setDate", $.datepicker.parseDate('yy-mm-dd', $('#milestone_due_date').val()));
 
-  window.project_image_path_upload = "#{upload_image_namespace_project_path @project.namespace, @project}";
+  window.project_uploads_path = "#{namespace_project_uploads_path @project.namespace, @project}";

app/views/projects/notes/_edit_form.html.haml

@@ -6,17 +6,9 @@
 
       .comment-hints.clearfix
         .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"),{ target: '_blank', tabindex: -1 }}
-        .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }.
+        .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }.
 
     .note-form-actions
       .buttons
         = f.submit 'Save Comment', class: "btn btn-primary btn-save btn-grouped js-comment-button"
         = link_to  'Cancel', "#", class: "btn btn-cancel note-edit-cancel"
\ No newline at end of file
-
-      .note-form-option.hidden-xs
-        %a.choose-btn.btn.js-choose-note-attachment-button
-          %i.fa.fa-paperclip
-          %span Choose File ...
-         
-        %span.file_name.js-attachment-filename
-        = f.file_field :attachment, class: "js-note-attachment-input hidden"

app/views/projects/notes/_form.html.haml

@@ -11,7 +11,7 @@
 
     .comment-hints.clearfix
       .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"),{ target: '_blank', tabindex: -1 }}
-      .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }.
+      .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }.
 
 
   .note-form-actions
@@ -20,13 +20,5 @@
       = yield(:note_actions)
       %a.btn.grouped.js-close-discussion-note-form Cancel
 
-    .note-form-option.hidden-xs
-      %a.choose-btn.btn.js-choose-note-attachment-button
-        %i.fa.fa-paperclip
-        %span Choose File ...
-       
-      %span.file_name.js-attachment-filename
-      = f.file_field :attachment, class: "js-note-attachment-input hidden"
-
 :javascript
-  window.project_image_path_upload = "#{upload_image_namespace_project_path @project.namespace, @project}";
+  window.project_uploads_path = "#{namespace_project_uploads_path @project.namespace, @project}";

app/views/projects/notes/_note.html.haml

@@ -57,10 +57,10 @@
       - if note.attachment.url
         .note-attachment
           - if note.attachment.image?
-            = link_to note.attachment.secure_url, target: '_blank' do
-              = image_tag note.attachment.secure_url, class: 'note-image-attach'
+            = link_to note.attachment.url, target: '_blank' do
+              = image_tag note.attachment.url, class: 'note-image-attach'
           .attachment
-            = link_to note.attachment.secure_url, target: "_blank" do
+            = link_to note.attachment.url, target: "_blank" do
               %i.fa.fa-paperclip
               = note.attachment_identifier
               = link_to delete_attachment_namespace_project_note_path(@project.namespace, @project, note),

app/views/projects/wikis/_form.html.haml

@@ -26,7 +26,7 @@
         = render 'projects/zen', f: f, attr: :content, classes: 'description form-control'
         .col-sm-12.hint
           .pull-left Wiki content is parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"), target: '_blank'}
-          .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
+          .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
 
       .clearfix
       .error-alert
@@ -43,5 +43,4 @@
       = link_to "Cancel", namespace_project_wiki_path(@project.namespace, @project, :home), class: "btn btn-cancel"
 
 :javascript
-  window.project_image_path_upload = "#{upload_image_namespace_project_path @project.namespace, @project}";
-
+  window.project_uploads_path = "#{namespace_project_uploads_path @project.namespace, @project}";

config/routes.rb

@@ -92,9 +92,9 @@ Gitlab::Application.routes.draw do
         constraints:  { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /.+/ }
 
     # Project markdown uploads
-    get ":namespace_id/:id/:secret/:filename",
+    get ":namespace_id/:project_id/:secret/:filename",
       to:           "projects/uploads#show",
-      constraints:  { namespace_id: /[a-zA-Z.0-9_\-]+/, id: /[a-zA-Z.0-9_\-]+/, filename: /.+/ }
+      constraints:  { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: /.+/ }
   end
 
   #
@@ -116,11 +116,6 @@ Gitlab::Application.routes.draw do
   get 'public' => 'explore/projects#index'
   get 'public/projects' => 'explore/projects#index'
 
-  #
-  # Attachments serving
-  #
-  get 'files/:type/:id/:filename' => 'files#download', constraints: { id: /\d+/, type: /[a-z]+/, filename:  /.+/ }
-
   #
   # Admin Area
   #
@@ -260,7 +255,6 @@ Gitlab::Application.routes.draw do
         put :transfer
         post :archive
         post :unarchive
-        post :upload_image
         post :toggle_star
         post :markdown_preview
         get :autocomplete_sources
@@ -455,6 +449,12 @@ Gitlab::Application.routes.draw do
             delete :delete_attachment
           end
         end
+
+        resources :uploads, only: [:create] do
+          collection do
+            get ":secret/:filename", action: :show, as: :show, constraints: { filename: /.+/ }
+          end
+        end
       end
 
     end

features/steps/groups.rb

@@ -110,7 +110,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
   end
 
   step 'I should see new group "Owned" avatar' do
-    Group.find_by(name: "Owned").avatar.should be_instance_of AttachmentUploader
+    Group.find_by(name: "Owned").avatar.should be_instance_of AvatarUploader
     Group.find_by(name: "Owned").avatar.url.should == "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/gitlab_logo.png"
   end
 

features/steps/profile/profile.rb

@@ -29,7 +29,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
   end
 
   step 'I should see new avatar' do
-    @user.avatar.should be_instance_of AttachmentUploader
+    @user.avatar.should be_instance_of AvatarUploader
     @user.avatar.url.should == "/uploads/user/avatar/#{ @user.id }/gitlab_logo.png"
   end
 

features/steps/project/project.rb

@@ -35,7 +35,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
   end
 
   step 'I should see new project avatar' do
-    @project.avatar.should be_instance_of AttachmentUploader
+    @project.avatar.should be_instance_of AvatarUploader
     url = @project.avatar.url
     url.should == "/uploads/project/avatar/#{ @project.id }/gitlab_logo.png"
   end

spec/controllers/projects/uploads_controller_spec.rb

+require('spec_helper')
+
+describe Projects::UploadsController do
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+  let(:jpg)     { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
+  let(:txt)     { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
+
+  describe "POST #create" do
+    before do
+      sign_in(user)
+      project.team << [user, :developer]
+    end
+
+    context "without params['file']" do
+      it "returns an error" do
+        post :create, 
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param, 
+          format: :json
+        expect(response.status).to eq(422)
+      end
+    end
+
+    context 'with valid image' do
+      before do
+        post :create,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          file: jpg,
+          format: :json
+      end
+
+      it 'returns a content with original filename, new link, and correct type.' do
+        expect(response.body).to match '\"alt\":\"rails_sample\"'
+        expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads"
+        expect(response.body).to match '\"is_image\":true'
+      end
+    end
+
+    context 'with valid non-image file' do
+      before do
+        post :create, 
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param, 
+          file: txt, 
+          format: :json
+      end
+
+      it 'returns a content with original filename, new link, and correct type.' do
+        expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
+        expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads"
+        expect(response.body).to match '\"is_image\":false'
+      end
+    end
+  end
+end

spec/controllers/projects_controller_spec.rb

@@ -7,44 +7,6 @@ describe ProjectsController do
   let(:jpg)     { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
   let(:txt)     { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
 
-  describe "POST #upload_image" do
-    before do
-      sign_in(user)
-      project.team << [user, :developer]
-    end
-
-    context "without params['markdown_img']" do
-      it "returns an error" do
-        post(:upload_image, namespace_id: project.namespace.to_param,
-             id: project.to_param, format: :json)
-        expect(response.status).to eq(422)
-      end
-    end
-
-    context "with invalid file" do
-      before do
-        post(:upload_image, namespace_id: project.namespace.to_param,
-             id: project.to_param, markdown_img: txt, format: :json)
-      end
-
-      it "returns an error" do
-        expect(response.status).to eq(422)
-      end
-    end
-
-    context "with valid file" do
-      before do
-        post(:upload_image, namespace_id: project.namespace.to_param,
-             id: project.to_param, markdown_img: jpg, format: :json)
-      end
-
-      it "returns a content with original filename and new link." do
-        expect(response.body).to match "\"alt\":\"rails_sample\""
-        expect(response.body).to match "\"url\":\"http://test.host/uploads/#{project.path_with_namespace}"
-      end
-    end
-  end
-
   describe "POST #toggle_star" do
     it "toggles star if user is signed in" do
       sign_in(user)

spec/services/projects/image_service_spec.rb

-require 'spec_helper'
-
-describe Projects::ImageService do
-  describe 'Image service' do
-    before do
-      @user = create :user
-      @project = create :project, creator_id: @user.id, namespace: @user.namespace
-    end
-
-    context 'for valid gif file' do
-      before do
-        gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
-        @link_to_image = upload_image(@project.repository, { 'markdown_img' => gif }, "http://test.example/")
-      end
-
-      it { expect(@link_to_image).to have_key("alt") }
-      it { expect(@link_to_image).to have_key("url") }
-      it { expect(@link_to_image).to have_value("banana_sample") }
-      it { expect(@link_to_image["url"]).to match("http://test.example/uploads/#{@project.path_with_namespace}") }
-      it { expect(@link_to_image["url"]).to match("banana_sample.gif") }
-    end
-
-     context 'for valid png file' do
-      before do
-        png = fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png')
-        @link_to_image = upload_image(@project.repository, { 'markdown_img' => png }, "http://test.example/")
-      end
-
-      it { expect(@link_to_image).to have_key("alt") }
-      it { expect(@link_to_image).to have_key("url") }
-      it { expect(@link_to_image).to have_value("dk") }
-      it { expect(@link_to_image["url"]).to match("http://test.example/uploads/#{@project.path_with_namespace}") }
-      it { expect(@link_to_image["url"]).to match("dk.png") }
-    end
-
-     context 'for valid jpg file' do
-      before do
-        jpg = fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg')
-        @link_to_image = upload_image(@project.repository, { 'markdown_img' => jpg }, "http://test.example/")
-      end
-
-      it { expect(@link_to_image).to have_key("alt") }
-      it { expect(@link_to_image).to have_key("url") }
-      it { expect(@link_to_image).to have_value("rails_sample") }
-      it { expect(@link_to_image["url"]).to match("http://test.example/uploads/#{@project.path_with_namespace}") }
-      it { expect(@link_to_image["url"]).to match("rails_sample.jpg") }
-    end
-
-    context 'for txt file' do
-      before do
-        txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain')
-        @link_to_image = upload_image(@project.repository, { 'markdown_img' => txt }, "http://test.example/")
-      end
-
-      it { expect(@link_to_image).to be_nil }
-    end
-  end
-
-  def upload_image(repository, params, root_url)
-    Projects::ImageService.new(repository, params, root_url).execute
-  end
-end

spec/services/projects/upload_service_spec.rb

+require 'spec_helper'
+
+describe Projects::UploadService do
+  describe 'File service' do
+    before do
+      @user = create :user
+      @project = create :project, creator_id: @user.id, namespace: @user.namespace
+    end
+
+    context 'for valid gif file' do
+      before do
+        gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
+        @link_to_file = upload_file(@project.repository, gif)
+      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).to have_value('banana_sample') }
+      it { expect(@link_to_file['is_image']).to equal(true) }
+      it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
+      it { expect(@link_to_file['url']).to match('banana_sample.gif') }
+    end
+
+     context 'for valid png file' do
+      before do
+        png = fixture_file_upload(Rails.root + 'spec/fixtures/dk.png',
+          'image/png')
+        @link_to_file = upload_file(@project.repository, png)
+      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_value('dk') }
+      it { expect(@link_to_file).to have_key('is_image') }
+      it { expect(@link_to_file['is_image']).to equal(true) }
+      it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
+      it { expect(@link_to_file['url']).to match('dk.png') }
+    end
+
+     context 'for valid jpg file' do
+      before do
+        jpg = fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg')
+        @link_to_file = upload_file(@project.repository, jpg)
+      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).to have_value('rails_sample') }
+      it { expect(@link_to_file['is_image']).to equal(true) }
+      it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
+      it { expect(@link_to_file['url']).to match('rails_sample.jpg') }
+    end
+
+    context 'for txt file' do
+      before do
+        txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain')
+        @link_to_file = upload_file(@project.repository, txt)
+      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).to have_value('doc_sample.txt') }
+      it { expect(@link_to_file['is_image']).to equal(false) }
+      it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
+      it { expect(@link_to_file['url']).to match('doc_sample.txt') }
+    end
+  end
+
+  def upload_file(repository, file)
+    Projects::UploadService.new(repository, file).execute
+  end
+end