Skip to content

Commit

Permalink
Add #full_attachment_path and make sure it doesn't get deleted even w…
Browse files Browse the repository at this point in the history
…hen empty.

This is because on production apps, it's very common for the attachment_path to be a symlink to a shared dir (like public/files => shared/public/files), but attachment_fu was removing the link whenever it was deleting the last attachment file, or even updating it. So the symlink was being deleted and new attachments where being saved directly in a newly created (by attachment_fu) public/files directory in the release path instead of the shared path.
  • Loading branch information
oboxodo authored and technoweenie committed Mar 19, 2010
1 parent f963029 commit a86a624
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
12 changes: 9 additions & 3 deletions lib/attachment_fu.rb
Expand Up @@ -207,7 +207,12 @@ def public_path(thumbnail = nil)

def full_path(thumbnail = nil)
return nil if !has_attachment?
File.expand_path(File.join(AttachmentFu.root_path, attachment_path, *partitioned_path(thumbnailed_filename(thumbnail))))
File.expand_path(File.join(full_attachment_path, *partitioned_path(thumbnailed_filename(thumbnail))))
end

def full_attachment_path
return nil if !has_attachment?
File.expand_path(File.join(AttachmentFu.root_path, attachment_path))
end

# Sets the path to the attachment about to be saved. Could be a string path to a file,
Expand Down Expand Up @@ -337,17 +342,18 @@ def check_task_progress(stack)
# the empty asset paths.
def delete_attachment
fp = full_path
fap = full_attachment_path
return if fp.blank?
FileUtils.rm fp if File.exist?(fp)
dir_name = File.dirname(fp)
default = %w(. ..)
while dir_name != AttachmentFu.root_path
while dir_name != fap
dir_exists = File.exists?(dir_name)
if !dir_exists || (Dir.entries(dir_name) - default).empty?
FileUtils.rm_rf(dir_name) if dir_exists
dir_name.sub! /\/\w+$/, ''
else
dir_name = AttachmentFu.root_path
dir_name = fap
end
end
end
Expand Down
18 changes: 15 additions & 3 deletions spec/attachment_fu_spec.rb
Expand Up @@ -20,6 +20,10 @@ class QueuedAsset < ActiveRecord::Base
@asset.full_path.should be_nil
end

it "has nil #full_attachment_path" do
@asset.full_attachment_path.should be_nil
end

it "has nil #partitioned_path" do
@asset.partitioned_path.should == nil
end
Expand Down Expand Up @@ -83,6 +87,10 @@ class QueuedAsset < ActiveRecord::Base
@asset.full_path.should == nil
end

it "has no full_attachment_path" do
@asset.full_attachment_path.should == nil
end

it "has no public_path" do
@asset.public_path.should == nil
end
Expand All @@ -106,6 +114,10 @@ class QueuedAsset < ActiveRecord::Base
@asset.full_path.should == File.expand_path(File.join(AttachmentFu.root_path, "public/afu_spec_assets/#{@asset.partitioned_path * '/'}/guinea_pig.rb"))
end

it "stores asset in full_attachment_path" do
@asset.full_path.should == File.expand_path(File.join(@asset.full_attachment_path, "#{@asset.partitioned_path * '/'}/guinea_pig.rb"))
end

it "creates full_path from record id and attachment_path" do
@asset.full_path.should == File.expand_path(File.join(AttachmentFu.root_path, "public/afu_spec_assets/#{@asset.partitioned_path * '/'}/#{@asset.filename}"))
end
Expand Down Expand Up @@ -199,7 +211,7 @@ class QueuedAsset < ActiveRecord::Base
File.exist?(@asset.full_path).should == false
end

(1..4).each do |i|
(1..2).each do |i|
it "deletes empty path ##{i}" do
@asset.destroy
dir_to_check = @dir.split("/")[0..-i] * "/"
Expand All @@ -214,9 +226,9 @@ class QueuedAsset < ActiveRecord::Base
end
end

it "keeps AttachmentFu.root_path" do
it "keeps #full_attachment_path" do
@asset.destroy
dir_to_check = @dir.split("/")[0..-5] * "/"
dir_to_check = @dir.split("/")[0..-3] * "/"
fail "#{dir_to_check.inspect} is deleted" unless File.directory?(dir_to_check)
end
end
Expand Down

0 comments on commit a86a624

Please sign in to comment.