diff options
| -rw-r--r-- | app/controllers/attachments_controller.rb | 30 | ||||
| -rw-r--r-- | app/controllers/issues_controller.rb | 4 | ||||
| -rw-r--r-- | app/models/attachment.rb | 21 | ||||
| -rw-r--r-- | app/views/attachments/upload.api.rsb | 3 | ||||
| -rw-r--r-- | config/routes.rb | 2 | ||||
| -rw-r--r-- | lib/redmine.rb | 6 | ||||
| -rw-r--r-- | test/integration/api_test/attachments_test.rb | 35 | ||||
| -rw-r--r-- | test/integration/api_test/issues_test.rb | 68 | ||||
| -rw-r--r-- | test/integration/routing/attachments_test.rb | 8 | ||||
| -rw-r--r-- | vendor/plugins/acts_as_attachable/lib/acts_as_attachable.rb | 17 |
10 files changed, 175 insertions, 19 deletions
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index 53ff69ba8..e6fa8a8a8 100644 --- a/app/controllers/attachments_controller.rb +++ b/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 :file_readable, :read_authorize, :except => :destroy + before_filter :find_project, :except => :upload + 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 diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 7adad0008..e8ff416b7 100644 --- a/app/controllers/issues_controller.rb +++ b/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) diff --git a/app/models/attachment.rb b/app/models/attachment.rb index d87a9df4c..84e945c44 100644 --- a/app/models/attachment.rb +++ b/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) - self.disk_filename = Attachment.disk_filename(filename) - self.content_type = @temp_file.content_type.to_s.chomp - if content_type.blank? + if @temp_file.respond_to?(:original_filename) + self.filename = @temp_file.original_filename + end + 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 diff --git a/app/views/attachments/upload.api.rsb b/app/views/attachments/upload.api.rsb new file mode 100644 index 000000000..edd0b0af4 --- /dev/null +++ b/app/views/attachments/upload.api.rsb @@ -0,0 +1,3 @@ +api.upload do + api.token @attachment.token +end diff --git a/config/routes.rb b/config/routes.rb index 552ecb911..21718e096 100644 --- a/config/routes.rb +++ b/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} diff --git a/lib/redmine.rb b/lib/redmine.rb index df4284bdb..d88a69d51 100644 --- a/lib/redmine.rb +++ b/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 :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new]} + 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], :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 diff --git a/test/integration/api_test/attachments_test.rb b/test/integration/api_test/attachments_test.rb index 1f8d67cf3..16fc531a3 100644 --- a/test/integration/api_test/attachments_test.rb +++ b/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 diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb index ae07deec5..2d9df063d 100644 --- a/test/integration/api_test/issues_test.rb +++ b/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 diff --git a/test/integration/routing/attachments_test.rb b/test/integration/routing/attachments_test.rb index 852e4ef16..55ffe4eb9 100644 --- a/test/integration/routing/attachments_test.rb +++ b/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 diff --git a/vendor/plugins/acts_as_attachable/lib/acts_as_attachable.rb b/vendor/plugins/acts_as_attachable/lib/acts_as_attachable.rb index 346e0ab8f..afae0751d 100644 --- a/vendor/plugins/acts_as_attachable/lib/acts_as_attachable.rb +++ b/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) - attachments.each_value do |attachment| + if attachments.is_a?(Hash) + 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 - a = Attachment.create(:file => file, - :description => attachment['description'].to_s.strip, - :author => author) + next unless file.size > 0 + a = Attachment.create(:file => file, :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 |