From ec1f3afa73b16546fcd29ef30e7b2bae6d8e6a0f Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 28 Apr 2014 09:44:36 +0200 Subject: [PATCH] Improved storage backend. --- app/controllers/ticket_articles_controller.rb | 4 +- app/controllers/users_controller.rb | 2 +- app/models/application_model.rb | 2 +- app/models/channel/email_build.rb | 2 +- app/models/package.rb | 9 +- app/models/store.rb | 199 +++++++++++++++++- app/models/ticket/search_index.rb | 2 +- db/migrate/20120101000030_create_storage.rb | 6 +- .../20130925000001_create_activity_stream.rb | 1 - db/migrate/20140427000001_update_storage.rb | 9 + test/unit/store_test.rb | 77 +++++-- 11 files changed, 276 insertions(+), 37 deletions(-) create mode 100644 db/migrate/20140427000001_update_storage.rb diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb index c804eff09..c799dd508 100644 --- a/app/controllers/ticket_articles_controller.rb +++ b/app/controllers/ticket_articles_controller.rb @@ -125,7 +125,7 @@ class TicketArticlesController < ApplicationController # find file file = Store.find(params[:id]) send_data( - file.store_file.data, + file.content, :filename => file.filename, :type => file.preferences['Content-Type'] || file.preferences['Mime-Type'], :disposition => 'inline' @@ -148,7 +148,7 @@ class TicketArticlesController < ApplicationController if list file = Store.find(list.first) send_data( - file.store_file.data, + file.content, :filename => file.filename, :type => 'message/rfc822', :disposition => 'inline' diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 6d4f4c207..b57ba7125 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -598,7 +598,7 @@ curl http://localhost/api/v1/users/image/8d6cca1c6bdc226cf2ba131e264ca2c7 -v -u if list && list[0] file = Store.find( list[0] ) send_data( - file.store_file.data, + file.content, :filename => file.filename, :type => file.preferences['Content-Type'] || file.preferences['Mime-Type'], :disposition => 'inline' diff --git a/app/models/application_model.rb b/app/models/application_model.rb index 1976cce15..81f035f8d 100644 --- a/app/models/application_model.rb +++ b/app/models/application_model.rb @@ -810,7 +810,7 @@ store attachments for this object article_store.push Store.add( :object => self.class.to_s, :o_id => self.id, - :data => attachment.store_file.data, + :data => attachment.content, :filename => attachment.filename, :preferences => attachment.preferences, :created_by_id => self.created_by_id, diff --git a/app/models/channel/email_build.rb b/app/models/channel/email_build.rb index d3c77754c..b9870d6f7 100644 --- a/app/models/channel/email_build.rb +++ b/app/models/channel/email_build.rb @@ -41,7 +41,7 @@ class Channel::EmailBuild mail.attachments[attachment.filename] = { :content_type => attachment.preferences['Content-Type'], :mime_type => attachment.preferences['Mime-Type'], - :content => attachment.store_file.data + :content => attachment.content } end end diff --git a/app/models/package.rb b/app/models/package.rb index 11347c178..e0c4d2dc4 100644 --- a/app/models/package.rb +++ b/app/models/package.rb @@ -352,11 +352,10 @@ class Package < ApplicationModel if !list || !list.first raise "No such file in storage list #{name} #{version}" end - store_file = list.first.store_file - if !store_file + if !list.first.content raise "No such file in storage #{name} #{version}" end - store_file.data + list.first.content end def self._read_file(file, fullpath = false) @@ -374,7 +373,7 @@ class Package < ApplicationModel rescue => e raise 'ERROR: ' + e.inspect end - return contents + contents end def self._write_file(file, permission, data) @@ -411,7 +410,7 @@ class Package < ApplicationModel rescue => e raise 'ERROR: ' + e.inspect end - return true + true end def self._delete_file(file, permission, data) diff --git a/app/models/store.rb b/app/models/store.rb index d887d1bad..a99ce1535 100644 --- a/app/models/store.rb +++ b/app/models/store.rb @@ -8,6 +8,26 @@ class Store < ApplicationModel belongs_to :store_file, :class_name => 'Store::File' validates :filename, :presence => true +=begin + +add an attachment to storage + + result = Store.add( + :object => 'Ticket::Article', + :o_id => 4711, + :data => binary_string, + :preferences => { + :content_type => 'image/png', + :content_id => 234, + } + ) + +returns + + result = true + +=end + def self.add(data) data = data.stringify_keys @@ -31,7 +51,7 @@ class Store < ApplicationModel if file == nil file = Store::File.create( :data => data['data'], - :md5 => md5 + :md5 => md5, ) end @@ -44,9 +64,34 @@ class Store < ApplicationModel # store meta data store = Store.create(data) - return store + true end +=begin + +get attachment of object + + list = Store.list( + :object => 'Ticket::Article', + :o_id => 4711, + ) + +returns + + result = [store1, store2] + + store1 = { + :size => 94123, + :filename => 'image.png', + :preferences => { + :content_type => 'image/png', + :content_id => 234, + } + } + store1.content # binary_string + +=end + def self.list(data) # search store_object_id = Store::Object.lookup( :name => data[:object] ) @@ -55,6 +100,21 @@ class Store < ApplicationModel return stores end +=begin + +remove an attachment to storage + + result = Store.remove( + :object => 'Ticket::Article', + :o_id => 4711, + ) + +returns + + result = true + +=end + def self.remove(data) # search store_object_id = Store::Object.lookup( :name => data[:object] ) @@ -62,22 +122,141 @@ class Store < ApplicationModel where( :o_id => data[:o_id] ). order('created_at ASC, id ASC') stores.each do |store| + + # check backend for references + files = Store.where( :store_file_id => store.store_file_id ) + if files.count == 1 && files.first.id == store.id + Store::File.find( store.store_file_id ).destroy + end + store.destroy end return true end - class Object < ApplicationModel - validates :name, :presence => true + # get attachment + def content + file = Store::File.where( :id => self.store_file_id ).first + return if !file + if file.file_system + return file.read_from_fs + end + file.data + end +end + +class Store::Object < ApplicationModel + validates :name, :presence => true +end + +class Store::File < ApplicationModel + before_validation :add_md5 + before_create :check_location + + # generate file location + def get_locaton + + # generate directory + base = Rails.root.to_s + "/storage/fs/" + path = self.md5.scan(/./).join('/') + location = "#{ base }/#{path}" + + # create directory if not exists + if !File.exist?( location ) + FileUtils.mkdir_p( location ) + end + location += "/file" end - class File < ApplicationModel - before_validation :add_md5 + # read file from fs + def read_from_fs + puts "read from fs #{self.get_locaton}" + return if !File.exist?( self.get_locaton ) + data = File.open( self.get_locaton, 'rb' ) + content = data.read - private - def add_md5 - self.md5 = Digest::MD5.hexdigest( self.data ) + # check md5 + md5 = Digest::MD5.hexdigest( content ) + if md5 != self.md5 + raise "ERROR: Corrupt file in fs #{self.get_locaton}, md5 should be #{self.md5} but is #{md5}" + end + content + end + + # write file to fs + def write_to_fs + + # install file + permission = '600' + if !File.exist?( self.get_locaton ) + puts "NOTICE: storge write '#{self.get_locaton}' (#{permission})" + file = File.new( self.get_locaton, 'wb' ) + file.write( self.data ) + file.close + end + File.chmod( permission.to_i(8), self.get_locaton ) + + # check md5 + md5 = Digest::MD5.hexdigest( self.read_from_fs ) + if md5 != self.md5 + raise "ERROR: Corrupt file in fs #{self.get_locaton}, md5 should be #{self.md5} but is #{md5}" + end + + true + end + + # write file to db + def write_to_db + + # read and check md5 + content = self.read_from_fs + + # store in database + self.data = content + self.save + + # check md5 against db content + md5 = Digest::MD5.hexdigest( self.data ) + if md5 != self.md5 + raise "ERROR: Corrupt file in db #{self.get_locaton}, md5 should be #{self.md5} but is #{md5}" + end + + true + end + + def self.move_to_fs + Store::File.where( :file_system => false ).each {|item| + item.write_to_fs + item.update_attribute( :file_system, true ) + item.update_attribute( :data, nil ) + } + end + + def self.move_to_db + Store::File.where( :file_system => true ).each {|item| + item.write_to_db + item.update_attribute( :file_system, false ) + if File.exist?( item.get_locaton ) + puts "NOTICE: storge remove '#{item.get_locaton}'" + File.delete( item.get_locaton ) + end + } + end + + private + + def check_location + + # write initial to fs if needed + if self.file_system && self.data + self.write_to_fs + self.data = nil end end -end + def add_md5 + if self.data && !self.md5 + self.md5 = Digest::MD5.hexdigest( self.data ) + end + end +end \ No newline at end of file diff --git a/app/models/ticket/search_index.rb b/app/models/ticket/search_index.rb index 19de07e08..f97044414 100644 --- a/app/models/ticket/search_index.rb +++ b/app/models/ticket/search_index.rb @@ -67,7 +67,7 @@ returns end data = { "_name" => attachment.filename, - "content" => Base64.encode64( attachment.store_file.data ) + "content" => Base64.encode64( attachment.content ) } article_attributes['attachments'].push data } diff --git a/db/migrate/20120101000030_create_storage.rb b/db/migrate/20120101000030_create_storage.rb index e59a9a8bc..e12166a74 100644 --- a/db/migrate/20120101000030_create_storage.rb +++ b/db/migrate/20120101000030_create_storage.rb @@ -11,7 +11,7 @@ class CreateStorage < ActiveRecord::Migration t.timestamps end add_index :stores, [:store_object_id, :o_id] - + create_table :store_objects do |t| t.column :name, :string, :limit => 250, :null => false t.column :note, :string, :limit => 250, :null => true @@ -20,8 +20,8 @@ class CreateStorage < ActiveRecord::Migration add_index :store_objects, [:name], :unique => true create_table :store_files do |t| - t.column :data, :binary, :limit => 100.megabytes - t.column :md5, :string, :limit => 60, :null => false + t.column :data, :binary, :limit => 200.megabytes, :null => true + t.column :md5, :string, :limit => 60, :null => false t.timestamps end add_index :store_files, [:md5], :unique => true diff --git a/db/migrate/20130925000001_create_activity_stream.rb b/db/migrate/20130925000001_create_activity_stream.rb index 027a3cbdb..34bb3a99a 100644 --- a/db/migrate/20130925000001_create_activity_stream.rb +++ b/db/migrate/20130925000001_create_activity_stream.rb @@ -1,6 +1,5 @@ class CreateActivityStream < ActiveRecord::Migration def up - create_table :activity_streams do |t| t.references :activity_stream_type, :null => false t.references :activity_stream_object, :null => false diff --git a/db/migrate/20140427000001_update_storage.rb b/db/migrate/20140427000001_update_storage.rb new file mode 100644 index 000000000..ccf58b030 --- /dev/null +++ b/db/migrate/20140427000001_update_storage.rb @@ -0,0 +1,9 @@ +class UpdateStorage < ActiveRecord::Migration + def up + change_column :store_files, :data, :binary, :limit => 200.megabytes, :null => true + add_column :store_files, :file_system, :boolean, :null => false, :default => false + end + + def down + end +end diff --git a/test/unit/store_test.rb b/test/unit/store_test.rb index 51cd1af2d..ed497ca9e 100644 --- a/test/unit/store_test.rb +++ b/test/unit/store_test.rb @@ -1,32 +1,33 @@ # encoding: utf-8 require 'test_helper' - + class StoreTest < ActiveSupport::TestCase test 'store attachment' do files = [ { :data => 'hello world', :filename => 'test.txt', + :o_id => 1, }, { :data => 'hello world äöüß', :filename => 'testäöüß.txt', + :o_id => 2, }, { :data => IO.read('test/fixtures/test1.pdf'), :filename => 'test.pdf', - }, + :o_id => 3, + }, ] - files.each { |file| - md5 = Digest::MD5.hexdigest( file[:data] ) # add attachments store = Store.add( :object => 'Test', - :o_id => 1, + :o_id => file[:o_id], :data => file[:data], :filename => file[:filename], :preferences => {}, @@ -37,24 +38,76 @@ class StoreTest < ActiveSupport::TestCase # get list of attachments attachments = Store.list( :object => 'Test', - :o_id => 1 + :o_id => file[:o_id], ) assert attachments - + # md5 check - md5_new = Digest::MD5.hexdigest( attachments[0].store_file.data ) - assert_equal( md5, md5_new ) + md5_new = Digest::MD5.hexdigest( attachments[0].content ) + assert_equal( md5, md5_new, "check file #{ file[:filename] }") # filename check assert_equal( file[:filename], attachments[0].filename ) - # delete attachments + } + + Store::File.move_to_fs + + files.each { |file| + md5 = Digest::MD5.hexdigest( file[:data] ) + + # get list of attachments + attachments = Store.list( + :object => 'Test', + :o_id => file[:o_id], + ) + assert attachments + + # md5 check + md5_new = Digest::MD5.hexdigest( attachments[0].content ) + assert_equal( md5, md5_new, "check file #{ file[:filename] }") + + # filename check + assert_equal( file[:filename], attachments[0].filename ) + } + + Store::File.move_to_db + + files.each { |file| + md5 = Digest::MD5.hexdigest( file[:data] ) + + # get list of attachments + attachments = Store.list( + :object => 'Test', + :o_id => file[:o_id], + ) + assert attachments + + # md5 check + md5_new = Digest::MD5.hexdigest( attachments[0].content ) + assert_equal( md5, md5_new, "check file #{ file[:filename] }") + + # filename check + assert_equal( file[:filename], attachments[0].filename ) + } + + # delete attachments + files.each { |file| success = Store.remove( :object => 'Test', - :o_id => 1 + :o_id => file[:o_id], ) assert success - } + } + + # check attachments again + files.each { |file| + attachments = Store.list( + :object => 'Test', + :o_id => file[:o_id], + ) + assert !attachments[0] + } end end