Adds support for adding attachments to issues through the REST API (#8171).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8928 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/attachments_controller.rb

@@ -16,11 +16,12 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class AttachmentsController < ApplicationController
-  before_filter :find_project
+  before_filter :find_project, :except => :upload
-  before_filter :file_readable, :read_authorize, :except => :destroy
+  before_filter :file_readable, :read_authorize, :only => [:show, :download]
   before_filter :delete_authorize, :only => :destroy
+  before_filter :authorize_global, :only => :upload
 
-  accept_api_auth :show, :download
+  accept_api_auth :show, :download, :upload
 
   def show
     respond_to do |format|
@@ -58,6 +59,29 @@ class AttachmentsController < ApplicationController
 
   end
 
+  def upload
+    # Make sure that API users get used to set this content type
+    # as it won't trigger Rails' automatic parsing of the request body for parameters
+    unless request.content_type == 'application/octet-stream'
+      render :nothing => true, :status => 406
+      return
+    end
+
+    @attachment = Attachment.new(:file => request.body)
+    @attachment.author = User.current
+    @attachment.filename = "test" #ActiveSupport::SecureRandom.hex(16)
+
+    if @attachment.save
+      respond_to do |format|
+        format.api { render :action => 'upload', :status => :created }
+      end
+    else
+      respond_to do |format|
+        format.api { render_validation_errors(@attachment) }
+      end
+    end
+  end
+
   verify :method => :delete, :only => :destroy
   def destroy
     # Make sure association callbacks are called

app/controllers/issues_controller.rb

@@ -149,7 +149,7 @@ class IssuesController < ApplicationController
 
   def create
     call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue })
-    @issue.save_attachments(params[:attachments])
+    @issue.save_attachments(params[:attachments] || (params[:issue] && params[:issue][:uploads]))
     if @issue.save
       call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})
       respond_to do |format|
@@ -181,7 +181,7 @@ class IssuesController < ApplicationController
 
   def update
     return unless update_issue_from_params
-    @issue.save_attachments(params[:attachments])
+    @issue.save_attachments(params[:attachments] || (params[:issue] && params[:issue][:uploads]))
     saved = false
     begin
       saved = @issue.save_issue_with_child_records(params, @time_entry)

app/models/attachment.rb

@@ -76,21 +76,32 @@ class Attachment < ActiveRecord::Base
     unless incoming_file.nil?
       @temp_file = incoming_file
       if @temp_file.size > 0
-        self.filename = sanitize_filename(@temp_file.original_filename)
+        if @temp_file.respond_to?(:original_filename)
-        self.disk_filename = Attachment.disk_filename(filename)
+          self.filename = @temp_file.original_filename
-        self.content_type = @temp_file.content_type.to_s.chomp
+        end
-        if content_type.blank?
+        if @temp_file.respond_to?(:content_type)
+          self.content_type = @temp_file.content_type.to_s.chomp
+        end
+        if content_type.blank? && filename.present?
           self.content_type = Redmine::MimeType.of(filename)
         end
         self.filesize = @temp_file.size
       end
     end
   end
-	
+  
   def file
     nil
   end
 
+  def filename=(arg)
+    write_attribute :filename, sanitize_filename(arg.to_s)
+    if new_record? && disk_filename.blank?
+      self.disk_filename = Attachment.disk_filename(filename)
+    end
+    filename
+  end
+
   # Copies the temporary file to its final location
   # and computes its MD5 hash
   def files_to_final_location

app/views/attachments/upload.api.rsb

@@ -0,0 +1,3 @@
+api.upload do
+  api.token @attachment.token
+end

config/routes.rb

@@ -409,6 +409,8 @@ ActionController::Routing::Routes.draw do |map|
                 :conditions => {:method => :get}
   end
 
+  map.connect 'uploads.:format', :controller => 'attachments', :action => 'upload', :conditions => {:method => :post}
+
   map.connect 'robots.txt', :controller => 'welcome',
               :action => 'robots', :conditions => {:method => :get}
 

lib/redmine.rb

@@ -67,13 +67,13 @@ Redmine::AccessControl.map do |map|
                                   :journals => [:index, :diff],
                                   :queries => :index,
                                   :reports => [:issue_report, :issue_report_details]}
-    map.permission :add_issues, {:issues => [:new, :create, :update_form]}
+    map.permission :add_issues, {:issues => [:new, :create, :update_form], :attachments => :upload}
-    map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new]}
+    map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new], :attachments => :upload}
     map.permission :manage_issue_relations, {:issue_relations => [:index, :show, :create, :destroy]}
     map.permission :manage_subtasks, {}
     map.permission :set_issues_private, {}
     map.permission :set_own_issues_private, {}, :require => :loggedin
-    map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new]}
+    map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new], :attachments => :upload}
     map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :move_issues, {:issues => [:bulk_edit, :bulk_update]}, :require => :loggedin

test/integration/api_test/attachments_test.rb

@@ -82,4 +82,39 @@ class ApiTest::AttachmentsTest < ActionController::IntegrationTest
       end
     end
   end
+
+  context "POST /uploads" do
+    should "return the token" do
+      set_tmp_attachments_directory
+      assert_difference 'Attachment.count' do
+        post '/uploads.xml', 'File content', {'Content-Type' => 'application/octet-stream'}.merge(credentials('jsmith'))
+        assert_response :created
+        assert_equal 'application/xml', response.content_type
+
+        xml = Hash.from_xml(response.body)
+        assert_kind_of Hash, xml['upload']
+        token = xml['upload']['token']
+        assert_not_nil token
+
+        attachment = Attachment.first(:order => 'id DESC')
+        assert_equal token, attachment.token
+        assert_nil attachment.container
+        assert_equal 2, attachment.author_id
+        assert_equal 'File content'.size, attachment.filesize
+        assert attachment.content_type.blank?
+        assert attachment.filename.present?
+        assert_match /\d+_[0-9a-z]+/, attachment.diskfile
+        assert File.exist?(attachment.diskfile)
+        assert_equal 'File content', File.read(attachment.diskfile)
+      end
+    end
+
+    should "not accept other content types" do
+      set_tmp_attachments_directory
+      assert_no_difference 'Attachment.count' do
+        post '/uploads.xml', 'PNG DATA', {'Content-Type' => 'image/png'}.merge(credentials('jsmith'))
+        assert_response 406
+      end
+    end
+  end
 end

test/integration/api_test/issues_test.rb

@@ -707,4 +707,72 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
       assert_nil Issue.find_by_id(6)
     end
   end
+
+  def test_create_issue_with_uploaded_file
+    set_tmp_attachments_directory
+
+    # upload the file
+    assert_difference 'Attachment.count' do
+      post '/uploads.xml', 'test_create_with_upload', {'Content-Type' => 'application/octet-stream'}.merge(credentials('jsmith'))
+      assert_response :created
+    end
+    xml = Hash.from_xml(response.body)
+    token = xml['upload']['token']
+    attachment = Attachment.first(:order => 'id DESC')
+
+    # create the issue with the upload's token
+    assert_difference 'Issue.count' do
+      post '/issues.xml',
+        {:issue => {:project_id => 1, :subject => 'Uploaded file', :uploads => [{:token => token, :filename => 'test.txt', :content_type => 'text/plain'}]}},
+        credentials('jsmith')
+      assert_response :created
+    end
+    issue = Issue.first(:order => 'id DESC')
+    assert_equal 1, issue.attachments.count
+    assert_equal attachment, issue.attachments.first
+
+    attachment.reload
+    assert_equal 'test.txt', attachment.filename
+    assert_equal 'text/plain', attachment.content_type
+    assert_equal 'test_create_with_upload'.size, attachment.filesize
+    assert_equal 2, attachment.author_id
+
+    # get the issue with its attachments
+    get "/issues/#{issue.id}.xml", :include => 'attachments'
+    assert_response :success
+    xml = Hash.from_xml(response.body)
+    attachments = xml['issue']['attachments']
+    assert_kind_of Array, attachments
+    assert_equal 1, attachments.size
+    url = attachments.first['content_url']
+    assert_not_nil url
+
+    # download the attachment
+    get url
+    assert_response :success
+  end
+
+  def test_update_issue_with_uploaded_file
+    set_tmp_attachments_directory
+
+    # upload the file
+    assert_difference 'Attachment.count' do
+      post '/uploads.xml', 'test_upload_with_upload', {'Content-Type' => 'application/octet-stream'}.merge(credentials('jsmith'))
+      assert_response :created
+    end
+    xml = Hash.from_xml(response.body)
+    token = xml['upload']['token']
+    attachment = Attachment.first(:order => 'id DESC')
+
+    # update the issue with the upload's token
+    assert_difference 'Journal.count' do
+      put '/issues/1.xml',
+        {:issue => {:notes => 'Attachment added', :uploads => [{:token => token, :filename => 'test.txt', :content_type => 'text/plain'}]}},
+        credentials('jsmith')
+      assert_response :ok
+    end
+
+    issue = Issue.find(1)
+    assert_include attachment, issue.attachments
+  end
 end

test/integration/routing/attachments_test.rb

@@ -49,5 +49,13 @@ class RoutingAttachmentsTest < ActionController::IntegrationTest
            { :method => 'delete', :path => "/attachments/1" },
            { :controller => 'attachments', :action => 'destroy', :id => '1' }
          )
+    assert_routing(
+           { :method => 'post', :path => '/uploads.xml' },
+           { :controller => 'attachments', :action => 'upload', :format => 'xml' }
+    )
+    assert_routing(
+           { :method => 'post', :path => '/uploads.json' },
+           { :controller => 'attachments', :action => 'upload', :format => 'json' }
+    )
   end
 end

vendor/plugins/acts_as_attachable/lib/acts_as_attachable.rb

@@ -61,18 +61,23 @@ module Redmine
         end
 
         def save_attachments(attachments, author=User.current)
-          if attachments && attachments.is_a?(Hash)
+          if attachments.is_a?(Hash)
-            attachments.each_value do |attachment|
+            attachments = attachments.values
+          end
+          if attachments.is_a?(Array)
+            attachments.each do |attachment|
               a = nil
               if file = attachment['file']
-                next unless file && file.size > 0
+                next unless file.size > 0
-                a = Attachment.create(:file => file,
+                a = Attachment.create(:file => file, :author => author)
-                                      :description => attachment['description'].to_s.strip,
-                                      :author => author)
               elsif token = attachment['token']
                 a = Attachment.find_by_token(token)
+                next unless a
+                a.filename = attachment['filename'] unless attachment['filename'].blank?
+                a.content_type = attachment['content_type']
               end
               next unless a
+              a.description = attachment['description'].to_s.strip
               if a.new_record?
                 unsaved_attachments << a
               else