diff --git a/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee b/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee index b43483c31..45ed32c8a 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee @@ -38,8 +38,7 @@ class App.UiElement.richtext # delete attachment from storage App.Ajax.request( type: 'DELETE' - url: "#{App.Config.get('api_path')}/ticket_attachment_upload" - data: JSON.stringify(id: id), + url: "#{App.Config.get('api_path')}/upload_caches/#{@form_id}/items/#{id}" processData: false ) @@ -57,59 +56,58 @@ class App.UiElement.richtext @attachmentsHolder = item.find('.attachments') @cancelContainer = item.find('.js-cancel') - u = => html5Upload.initialize( - uploadUrl: App.Config.get('api_path') + '/ticket_attachment_upload' - dropContainer: item.closest('form').get(0) - cancelContainer: @cancelContainer - inputField: item.find('input').get(0) - maxSimultaneousUploads: 1, - key: 'File' - data: - form_id: item.closest('form').find('[name=form_id]').val() - onFileAdded: (file) => + upload_initialize_callback = => + form_id = item.closest('form').find('[name=form_id]').val() + html5Upload.initialize( + uploadUrl: "#{App.Config.get('api_path')}/upload_caches/#{form_id}" + dropContainer: item.closest('form').get(0) + cancelContainer: @cancelContainer + inputField: item.find('input').get(0) + maxSimultaneousUploads: 1, + key: 'File' + onFileAdded: (file) => - file.on( - onStart: => - @attachmentPlaceholder.addClass('hide') - @attachmentUpload.removeClass('hide') - @cancelContainer.removeClass('hide') - item.find('[contenteditable]').trigger('fileUploadStart') - App.Log.debug 'UiElement.richtext', 'upload start' + file.on( + onStart: => + @attachmentPlaceholder.addClass('hide') + @attachmentUpload.removeClass('hide') + @cancelContainer.removeClass('hide') + item.find('[contenteditable]').trigger('fileUploadStart') + App.Log.debug 'UiElement.richtext', 'upload start' - onAborted: => - @attachmentPlaceholder.removeClass('hide') - @attachmentUpload.addClass('hide') - item.find('input').val('') - item.find('[contenteditable]').trigger('fileUploadStop', ['aborted']) + onAborted: => + @attachmentPlaceholder.removeClass('hide') + @attachmentUpload.addClass('hide') + item.find('input').val('') + item.find('[contenteditable]').trigger('fileUploadStop', ['aborted']) - # Called after received response from the server - onCompleted: (response) => - response = JSON.parse(response) + # Called after received response from the server + onCompleted: (response) => + response = JSON.parse(response) - @attachmentPlaceholder.removeClass('hide') - @attachmentUpload.addClass('hide') + @attachmentPlaceholder.removeClass('hide') + @attachmentUpload.addClass('hide') - # reset progress bar - @progressBar.width(parseInt(0) + '%') - @progressText.text('') + # reset progress bar + @progressBar.width(parseInt(0) + '%') + @progressText.text('') - renderFile(response.data) - item.find('input').val('') - item.find('[contenteditable]').trigger('fileUploadStop', ['completed']) - App.Log.debug 'UiElement.richtext', 'upload complete', response.data + renderFile(response.data) + item.find('input').val('') + item.find('[contenteditable]').trigger('fileUploadStop', ['completed']) + App.Log.debug 'UiElement.richtext', 'upload complete', response.data - # Called during upload progress, first parameter - # is decimal value from 0 to 100. - onProgress: (progress, fileSize, uploadedBytes) => - @progressBar.width(parseInt(progress) + '%') - @progressText.text(parseInt(progress)) - # hide cancel on 90% - if parseInt(progress) >= 90 - @cancelContainer.addClass('hide') - App.Log.debug 'UiElement.richtext', 'uploadProgress ', parseInt(progress) - - ) - ) - App.Delay.set(u, 100, undefined, 'form_upload') + # Called during upload progress, first parameter + # is decimal value from 0 to 100. + onProgress: (progress, fileSize, uploadedBytes) => + @progressBar.width(parseInt(progress) + '%') + @progressText.text(parseInt(progress)) + # hide cancel on 90% + if parseInt(progress) >= 90 + @cancelContainer.addClass('hide') + App.Log.debug 'UiElement.richtext', 'uploadProgress ', parseInt(progress) + ) + ) + App.Delay.set(upload_initialize_callback, 100, undefined, 'form_upload') item diff --git a/app/assets/javascripts/app/controllers/_ui_element/textarea.coffee b/app/assets/javascripts/app/controllers/_ui_element/textarea.coffee index 494936c73..3987bd43f 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/textarea.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/textarea.coffee @@ -22,11 +22,10 @@ class App.UiElement.textarea # only add upload item if html element exists if $('#' + fileUploaderId )[0] + form_id = item.closest('form').find('[name=form_id]').val() $('#' + fileUploaderId ).fineUploader( request: - endpoint: App.Config.get('api_path') + '/ticket_attachment_upload' - params: - form_id: item.closest('form').find('[name=form_id]').val() + endpoint: "#{App.Config.get('api_path')}/upload_caches/#{form_id}" text: uploadButton: App.Utils.icon('paperclip') template: '
' + diff --git a/app/assets/javascripts/app/controllers/ticket_zoom.coffee b/app/assets/javascripts/app/controllers/ticket_zoom.coffee index 89f174b58..c5a9abd88 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom.coffee @@ -958,8 +958,7 @@ class App.TicketZoom extends App.Controller # reset/delete uploaded attachments App.Ajax.request( type: 'DELETE' - url: App.Config.get('api_path') + '/ticket_attachment_upload' - data: JSON.stringify(form_id: @form_id) + url: "#{App.Config.get('api_path')}/upload_caches/#{@form_id}" processData: false ) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee index 50805f372..d4e66b40b 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee @@ -172,13 +172,11 @@ class App.TicketZoomArticleNew extends App.Controller }) html5Upload.initialize( - uploadUrl: App.Config.get('api_path') + '/ticket_attachment_upload' + uploadUrl: "#{App.Config.get('api_path')}/upload_caches/#{@form_id}" dropContainer: @$('.article-add').get(0) cancelContainer: @cancelContainer inputField: @$('.article-attachment input').get(0) key: 'File' - data: - form_id: @form_id maxSimultaneousUploads: 1 onFileAdded: (file) => @@ -599,9 +597,8 @@ class App.TicketZoomArticleNew extends App.Controller # delete attachment from storage App.Ajax.request( - type: 'DELETE' - url: App.Config.get('api_path') + '/ticket_attachment_upload' - data: JSON.stringify(id: id) + type: 'DELETE' + url: "#{App.Config.get('api_path')}/upload_caches/#{@form_id}/items/#{id}" processData: false ) diff --git a/app/controllers/concerns/creates_ticket_articles.rb b/app/controllers/concerns/creates_ticket_articles.rb index 58fcfd304..1c8987aac 100644 --- a/app/controllers/concerns/creates_ticket_articles.rb +++ b/app/controllers/concerns/creates_ticket_articles.rb @@ -55,10 +55,7 @@ module CreatesTicketArticles # find attachments in upload cache if form_id - article.attachments = Store.list( - object: 'UploadCache', - o_id: form_id, - ) + article.attachments = UploadCache.new(form_id).attachments end # set subtype of present @@ -130,11 +127,8 @@ module CreatesTicketArticles .first { |taskbar| taskbar.persisted_form_id == form_id } &.update!(state: {}) - # remove attachments from upload cache - Store.remove( - object: 'UploadCache', - o_id: form_id, - ) + # remove temporary attachment cache + UploadCache.new(form_id).destroy article end diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb index 138a23fdd..afb502f54 100644 --- a/app/controllers/ticket_articles_controller.rb +++ b/app/controllers/ticket_articles_controller.rb @@ -154,66 +154,6 @@ class TicketArticlesController < ApplicationController raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!' end - # DELETE /ticket_attachment_upload - def ticket_attachment_upload_delete - - if params[:id].present? - Store.remove_item(params[:id]) - render json: { - success: true, - } - return - end - - if params[:form_id].present? - Store.remove( - object: 'UploadCache', - o_id: params[:form_id], - ) - render json: { - success: true, - } - return - end - - render json: { message: 'No such id or form_id!' }, status: :unprocessable_entity - end - - # POST /ticket_attachment_upload - def ticket_attachment_upload_add - - # store file - file = params[:File] - content_type = file.content_type - if !content_type || content_type == 'application/octet-stream' - content_type = if MIME::Types.type_for(file.original_filename).first - MIME::Types.type_for(file.original_filename).first.content_type - else - 'application/octet-stream' - end - end - headers_store = { - 'Content-Type' => content_type - } - store = Store.add( - object: 'UploadCache', - o_id: params[:form_id], - data: file.read, - filename: file.original_filename, - preferences: headers_store - ) - - # return result - render json: { - success: true, - data: { - id: store.id, - filename: file.original_filename, - size: store.size, - } - } - end - # POST /ticket_attachment_upload_clone_by_article def ticket_attachment_upload_clone_by_article article = Ticket::Article.find(params[:article_id]) diff --git a/app/controllers/upload_caches_controller.rb b/app/controllers/upload_caches_controller.rb new file mode 100644 index 000000000..088d592d2 --- /dev/null +++ b/app/controllers/upload_caches_controller.rb @@ -0,0 +1,58 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +class UploadCachesController < ApplicationController + prepend_before_action :authentication_check + + # POST /upload_caches/1 + def update + file = params[:File] + content_type = file.content_type + if !content_type || content_type == 'application/octet-stream' + mime_type = MIME::Types.type_for(file.original_filename).first + content_type = mime_type&.content_type || 'application/octet-stream' + end + headers_store = { + 'Content-Type' => content_type + } + + store = cache.add( + filename: file.original_filename, + data: file.read, + preferences: headers_store + ) + + # return result + render json: { + success: true, + data: { + id: store.id, # TODO: rename? + filename: file.original_filename, + size: store.size, + } + } + end + + # DELETE /upload_caches/1 + def destroy + cache.destroy + + render json: { + success: true, + } + end + + # DELETE /upload_caches/1/items/1 + def remove_item + cache.remove_item(params[:store_id]) + + render json: { + success: true, + } + end + + private + + def cache + UploadCache.new(params[:id]) + end +end diff --git a/app/models/taskbar.rb b/app/models/taskbar.rb index 992093d55..2207a67e7 100644 --- a/app/models/taskbar.rb +++ b/app/models/taskbar.rb @@ -54,7 +54,7 @@ class Taskbar < ApplicationModel def attachments return [] if persisted_form_id.blank? - Store.list(object: 'UploadCache', o_id: persisted_form_id) + UploadCache.new(persisted_form_id).attachments end def add_attachments_to_attributes(attributes) diff --git a/config/routes/ticket.rb b/config/routes/ticket.rb index fef82d720..cebc84c52 100644 --- a/config/routes/ticket.rb +++ b/config/routes/ticket.rb @@ -42,8 +42,6 @@ Zammad::Application.routes.draw do match api_path + '/ticket_articles/:id', to: 'ticket_articles#update', via: :put match api_path + '/ticket_articles/:id', to: 'ticket_articles#destroy', via: :delete match api_path + '/ticket_attachment/:ticket_id/:article_id/:id', to: 'ticket_articles#attachment', via: :get - match api_path + '/ticket_attachment_upload', to: 'ticket_articles#ticket_attachment_upload_add', via: :post - match api_path + '/ticket_attachment_upload', to: 'ticket_articles#ticket_attachment_upload_delete', via: :delete match api_path + '/ticket_attachment_upload_clone_by_article/:article_id', to: 'ticket_articles#ticket_attachment_upload_clone_by_article', via: :post match api_path + '/ticket_article_plain/:id', to: 'ticket_articles#article_plain', via: :get diff --git a/config/routes/upload_cache.rb b/config/routes/upload_cache.rb new file mode 100644 index 000000000..0a3d26090 --- /dev/null +++ b/config/routes/upload_cache.rb @@ -0,0 +1,9 @@ +Zammad::Application.routes.draw do + api_path = Rails.configuration.api_path + + # upload cache + match api_path + '/upload_caches/:id', to: 'upload_caches#update', via: :post + match api_path + '/upload_caches/:id', to: 'upload_caches#destroy', via: :delete + match api_path + '/upload_caches/:id/items/:store_id', to: 'upload_caches#remove_item', via: :delete + +end diff --git a/lib/upload_cache.rb b/lib/upload_cache.rb new file mode 100644 index 000000000..853f24c79 --- /dev/null +++ b/lib/upload_cache.rb @@ -0,0 +1,98 @@ +# A wrapper class around Store that handles temporary attachment uploads +# and provides an interface for those. +class UploadCache + + attr_reader :id + + # Initializes a UploadCache for a given form_id. + # + # @example + # cache = UploadCache.new(form_id) + # + # @return [UploadCache] + def initialize(id) + # conversion to Integer is required for proper Store#o_id comparsion + @id = id.to_i + end + + # Adds a Store item with the given attributes for the form_id. + # + # @see Store#add + # + # @example + # cache = UploadCache.new(form_id) + # store = cache.add( + # filename: file.original_filename, + # data: file.read, + # preferences: { + # 'Content-Type' => 'application/octet-stream' + # } + # ) + # + # @return [Store] the created Store item + def add(args) + Store.add( + args.merge( + object: store_object, + o_id: id, + ) + ) + end + + # Provides all Store items associated to the form_id. + # + # @see Store#list + # + # @example + # attachments = UploadCache.new(form_id).attachments + # + # @return [Array] an enumerator of Store items + def attachments + Store.list( + object: store_object, + o_id: id, + ) + end + + # Removes all Store items associated to the form_id. + # + # @see Store#remove + # + # @example + # UploadCache.new(form_id).destroy + # + def destroy + Store.remove( + object: store_object, + o_id: id, + ) + end + + # Removes all Store items associated to the form_id. + # + # @see Store#remove + # + # @example + # UploadCache.new(form_id).remove_item(store_id) + # + # @raise [Exceptions::UnprocessableEntity] in cases where a Store item should get deleted that is not an UploadCache item + # + def remove_item(store_id = nil) + store = Store.find(store_id) + if store.o_id != id || store.store_object_id != store_object_id + raise Exceptions::UnprocessableEntity, "Attempt to delete Store item #{store_id} that is not bound to UploadCache object" + end + + Store.remove_item(store_id) + end + + private + + def store_object + self.class.name + end + + def store_object_id + Store::Object.lookup(name: store_object).id + end +end diff --git a/spec/fixtures/upload/hello_world.txt b/spec/fixtures/upload/hello_world.txt new file mode 100644 index 000000000..c57eff55e --- /dev/null +++ b/spec/fixtures/upload/hello_world.txt @@ -0,0 +1 @@ +Hello World! \ No newline at end of file diff --git a/spec/lib/upload_cache_spec.rb b/spec/lib/upload_cache_spec.rb new file mode 100644 index 000000000..eeb424c17 --- /dev/null +++ b/spec/lib/upload_cache_spec.rb @@ -0,0 +1,114 @@ +require 'rails_helper' + +RSpec.describe UploadCache do + + let(:subject) { described_class.new(1337) } + + # required for adding items to the Store + before { UserInfo.current_user_id = 1 } + + describe '#initialize' do + + it 'converts given (form_)id to an Integer' do + expect(described_class.new('1337').id).to eq(1337) + end + end + + describe '#add' do + + it 'adds a Store item' do + expect do + subject.add( + data: 'content_file3_normally_should_be_an_image', + filename: 'some_file3.jpg', + preferences: { + 'Content-Type' => 'image/jpeg', + 'Mime-Type' => 'image/jpeg', + 'Content-Disposition' => 'attached', + }, + ) + end.to change { Store.count }.by(1) + end + end + + describe '#attachments' do + + before do + subject.add( + data: 'hello world', + filename: 'some.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + end + + it 'returns all Store items' do + attachments = subject.attachments + + expect(attachments.count).to be(1) + expect(attachments).to include(Store.last) + end + end + + describe '#destroy' do + + before do + subject.add( + data: 'hello world', + filename: 'some.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + + subject.add( + data: 'hello other world', + filename: 'another_some.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + end + + it 'removes all added Store items' do + expect { subject.destroy }.to change { Store.count }.by(-2) + end + end + + describe '#remove_item' do + + before do + subject.add( + data: 'hello world', + filename: 'some.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + end + + it 'removes the Store item matching the given ID' do + expect { subject.remove_item(Store.last.id) }.to change { Store.count }.by(-1) + end + + it 'prevents removage of non UploadCache Store items' do + + item = Store.add( + object: 'Ticket', + o_id: 1, + data: "Can't touch this", + filename: 'keep.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + + expect { subject.remove_item(item.id) }.to raise_error(Exceptions::UnprocessableEntity) + end + + it 'fails for non existing UploadCache Store items' do + expect { subject.remove_item(1337) }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/spec/requests/upload_cache_spec.rb b/spec/requests/upload_cache_spec.rb new file mode 100644 index 000000000..277ea9492 --- /dev/null +++ b/spec/requests/upload_cache_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' + +RSpec.describe 'UploadCache', type: :request do + + let(:user) { create(:customer_user) } + let(:form_id) { 1337 } + let(:upload_cache) { UploadCache.new(form_id) } + + # required for adding items to the Store + before { UserInfo.current_user_id = 1 } + + before { authenticated_as(user) } + + describe '/upload_caches/:id' do + + context 'for POST requests' do + + it 'adds items to UploadCache' do + params = { + File: fixture_file_upload('upload/hello_world.txt', 'text/plain') + } + post "/api/v1/upload_caches/#{form_id}", params: params + + expect(response).to have_http_status(200) + end + + it 'detects Content-Type for binary uploads' do + params = { + File: fixture_file_upload('upload/hello_world.txt', 'application/octet-stream') + } + post "/api/v1/upload_caches/#{form_id}", params: params + + expect(Store.last.preferences['Content-Type']).to eq('text/plain') + end + end + + context 'for DELETE requests' do + + before do + 2.times do |iteration| + upload_cache.add( + data: "Can't touch this #{iteration}", + filename: 'keep.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + end + end + + it 'removes all form_id UploadCache items' do + expect do + delete "/api/v1/upload_caches/#{form_id}", as: :json + end.to change { upload_cache.attachments }.to([]) + end + end + end + + describe '/upload_caches/:id/items/:store_id' do + + context 'for DELETE requests' do + + before do + upload_cache.add( + data: "Can't touch this", + filename: 'keep.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + end + + it 'removes a UploadCache item by given store id' do + + store_id = upload_cache.attachments.first.id + delete "/api/v1/upload_caches/#{form_id}/items/#{store_id}", as: :json + + expect(Store.exists?(store_id)).to be(false) + end + end + end +end