FIX: automatic backup uploads to S3 when using a region

app/controllers/user_avatars_controller.rb

@@ -64,12 +64,14 @@ class UserAvatarsController < ApplicationController
       if Discourse.store.external? || File.exists?(original)
         optimized = get_optimized_image(upload, size)
 
-        if Discourse.store.external?
+        if optimized
-          expires_in 1.day, public: true
+          if Discourse.store.external?
-          return redirect_to optimized.url
+            expires_in 1.day, public: true
-        end
+            return redirect_to optimized.url
+          end
 
-        image = Discourse.store.path_for(optimized)
+          image = Discourse.store.path_for(optimized)
+        end
       end
     end
 

app/models/backup.rb

@@ -1,3 +1,5 @@
+require "s3_helper"
+
 class Backup
   include UrlHelper
   include ActiveModel::SerializerSupport
@@ -38,14 +40,26 @@ class Backup
     remove_from_s3 if SiteSetting.enable_s3_backups?
   end
 
+  def s3_bucket
+    return @s3_bucket if @s3_bucket
+    raise Discourse::SiteSettingMissing.new("s3_backup_bucket") if SiteSetting.s3_backup_bucket.blank?
+    @s3_bucket = SiteSetting.s3_backup_bucket.downcase
+  end
+
+  def s3
+    return @s3_helper if @s3_helper
+    @s3_helper = S3Helper.new(s3_bucket)
+  end
+
   def upload_to_s3
-    return unless fog_directory
+    return unless s3
-    fog_directory.files.create(key: @filename, public: false, body: File.read(@path))
+    file = File.read(@path)
+    s3.upload(file, @filename)
   end
 
   def remove_from_s3
-    return unless fog
+    return unless s3
-    fog.delete_object(SiteSetting.s3_backup_bucket, @filename)
+    s3.remove(@filename)
   end
 
   def self.base_directory
@@ -67,40 +81,8 @@ class Backup
   def self.remove_old
     return if Rails.env.development?
     all_backups = Backup.all
-    return unless all_backups.size > SiteSetting.maximum_backups
+    return if all_backups.size <= SiteSetting.maximum_backups
     all_backups[SiteSetting.maximum_backups..-1].each(&:remove)
   end
 
-  private
-
-    def s3_options
-      options = {
-        provider: 'AWS',
-        region: SiteSetting.s3_region.blank? ? "us-east-1" : SiteSetting.s3_region,
-      }
-      if (SiteSetting.s3_use_iam_profile.present?)
-        options.merge!(:use_iam_profile => true)
-      else
-        options.merge!(:aws_access_key_id => SiteSetting.s3_access_key_id,
-         :aws_secret_access_key => SiteSetting.s3_secret_access_key)
-      end
-      options
-    end
-
-    def fog
-      return @fog if @fog
-      return unless ((SiteSetting.s3_access_key_id.present? &&
-                    SiteSetting.s3_secret_access_key.present?) ||
-                    SiteSetting.s3_use_iam_profile.present?) &&
-                    SiteSetting.s3_backup_bucket.present?
-      require 'fog'
-      @fog = Fog::Storage.new(s3_options)
-    end
-
-    def fog_directory
-      return @fog_directory if @fog_directory
-      return unless fog
-      @fog_directory ||= fog.directories.get(SiteSetting.s3_backup_bucket)
-    end
-
 end

app/models/optimized_image.rb

@@ -19,40 +19,45 @@ class OptimizedImage < ActiveRecord::Base
     unless thumbnail
       external_copy = Discourse.store.download(upload) if Discourse.store.external?
       original_path = if Discourse.store.external?
-        external_copy.path
+        external_copy.try(:path)
       else
         Discourse.store.path_for(upload)
       end
 
-      # create a temp file with the same extension as the original
+      if original_path.blank?
-      extension = File.extname(original_path)
+        Rails.logger.error("Could not find file in the store located at url: #{upload.url}")
-      temp_file = Tempfile.new(["discourse-thumbnail", extension])
-      temp_path = temp_file.path
-      original_path += "[0]" unless opts[:allow_animation]
-
-      if resize(original_path, temp_path, width, height)
-        thumbnail = OptimizedImage.create!(
-          upload_id: upload.id,
-          sha1: Digest::SHA1.file(temp_path).hexdigest,
-          extension: File.extname(temp_path),
-          width: width,
-          height: height,
-          url: "",
-        )
-        # store the optimized image and update its url
-        url = Discourse.store.store_optimized_image(temp_file, thumbnail)
-        if url.present?
-          thumbnail.url = url
-          thumbnail.save
-        else
-          Rails.logger.error("Failed to store avatar #{size} for #{upload.url} from #{source}")
-        end
       else
-        Rails.logger.error("Failed to create optimized image #{width}x#{height} for #{upload.url}")
+        # create a temp file with the same extension as the original
+        extension = File.extname(original_path)
+        temp_file = Tempfile.new(["discourse-thumbnail", extension])
+        temp_path = temp_file.path
+        original_path += "[0]" unless opts[:allow_animation]
+
+        if resize(original_path, temp_path, width, height)
+          thumbnail = OptimizedImage.create!(
+            upload_id: upload.id,
+            sha1: Digest::SHA1.file(temp_path).hexdigest,
+            extension: File.extname(temp_path),
+            width: width,
+            height: height,
+            url: "",
+          )
+          # store the optimized image and update its url
+          url = Discourse.store.store_optimized_image(temp_file, thumbnail)
+          if url.present?
+            thumbnail.url = url
+            thumbnail.save
+          else
+            Rails.logger.error("Failed to store avatar #{size} for #{upload.url} from #{source}")
+          end
+        else
+          Rails.logger.error("Failed to create optimized image #{width}x#{height} for #{upload.url}")
+        end
+
+        # close && remove temp file
+        temp_file.close!
       end
 
-      # close && remove temp file
-      temp_file.close!
       # make sure we remove the cached copy from external stores
       external_copy.close! if Discourse.store.external?
     end

config/locales/server.en.yml

@@ -805,7 +805,7 @@ en:
     allow_restore: "Allow restore, which can replace ALL site data! Leave false unless you plan to restore a backup"
     maximum_backups: "The maximum amount of backups to keep on disk. Older backups are automatically deleted"
     backup_daily: "Automatically create a site backup once a day."
-    enable_s3_backups: "Upload backups to S3 when complete. IMPORTANT: Requires valid S3 credentials entered in Files settings."
+    enable_s3_backups: "Upload backups to S3 when complete. IMPORTANT: requires valid S3 credentials entered in Files settings."
     s3_backup_bucket: "The remote bucket to hold backups. WARNING: Make sure it is a private bucket."
 
     active_user_rate_limit_secs: "How frequently we update the 'last_seen_at' field, in seconds"
@@ -832,7 +832,7 @@ en:
     clean_orphan_uploads_grace_period_hours: "Grace period (in hours) before an orphan upload is removed."
     purge_deleted_uploads_grace_period_days: "Grace period (in days) before a deleted upload is erased."
     purge_inactive_users_grace_period_days: "Grace period (in days) before an inactive user is deleted."
-    enable_s3_uploads: "Place uploads on Amazon S3 storage."
+    enable_s3_uploads: "Place uploads on Amazon S3 storage. IMPORTANT: requires valid S3 credentials (both access key id & secret access key)."
     s3_use_iam_profile: 'Use AWS EC2 IAM role to retrieve keys. NOTE: enabling will override "s3 access key id" and "s3 secret access key" settings.'
     s3_upload_bucket: "The Amazon S3 bucket name that files will be uploaded into. WARNING: must be lowercase, no periods."
     s3_access_key_id: "The Amazon S3 access key id that will be used to upload images."

lib/file_store/s3_store.rb

@@ -1,10 +1,14 @@
 require 'file_store/base_store'
+require_dependency "s3_helper"
 require_dependency "file_helper"
 
 module FileStore
 
   class S3Store < BaseStore
-    @fog_loaded ||= require 'fog'
+
+    def initialize(s3_helper = nil)
+      @s3_helper = s3_helper || S3Helper.new(s3_bucket, tombstone_prefix)
+    end
 
     def store_upload(file, upload, content_type = nil)
       path = get_path_for_upload(file, upload)
@@ -46,10 +50,10 @@ module FileStore
     end
 
     def download(upload)
+      return unless has_been_uploaded?(upload.url)
       url = SiteSetting.scheme + ":" + upload.url
       max_file_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes
-
+      FileHelper.download(url, max_file_size, "discourse-s3", true)
-      FileHelper.download(url, max_file_size, "discourse-s3")
     end
 
     def avatar_template(avatar)
@@ -58,143 +62,56 @@ module FileStore
     end
 
     def purge_tombstone(grace_period)
-      update_tombstone_lifecycle(grace_period)
+      @s3_helper.update_tombstone_lifecycle(grace_period)
     end
 
     private
 
-    def get_path_for_upload(file, upload)
+      def get_path_for_upload(file, upload)
-      "#{upload.id}#{upload.sha1}#{upload.extension}"
+        "#{upload.id}#{upload.sha1}#{upload.extension}"
-    end
-
-    def get_path_for_optimized_image(file, optimized_image)
-      "#{optimized_image.id}#{optimized_image.sha1}_#{optimized_image.width}x#{optimized_image.height}#{optimized_image.extension}"
-    end
-
-    def get_path_for_avatar(file, avatar, size)
-      relative_avatar_template(avatar).gsub("{size}", size.to_s)
-    end
-
-    def relative_avatar_template(avatar)
-      "avatars/#{avatar.sha1}/{size}#{avatar.extension}"
-    end
-
-    def store_file(file, path, filename = nil, content_type = nil)
-      # if this fails, it will throw an exception
-      upload(file, path, filename, content_type)
-      # url
-      "#{absolute_base_url}/#{path}"
-    end
-
-    def remove_file(url)
-      return unless has_been_uploaded?(url)
-      filename = File.basename(url)
-      remove(filename)
-    end
-
-    def s3_bucket
-      SiteSetting.s3_upload_bucket.downcase
-    end
-
-    def check_missing_site_settings
-      raise Discourse::SiteSettingMissing.new("s3_upload_bucket")     if SiteSetting.s3_upload_bucket.blank?
-      unless SiteSetting.s3_use_iam_profile.present?
-        raise Discourse::SiteSettingMissing.new("s3_access_key_id")     if SiteSetting.s3_access_key_id.blank?
-        raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank?
-      end
-    end
-
-    def s3_options
-      options = {
-        provider: 'AWS',
-        scheme: SiteSetting.scheme,
-        # cf. https://github.com/fog/fog/issues/2381
-        path_style: dns_compatible?(s3_bucket, SiteSetting.use_https?),
-      }
-      options[:region] = SiteSetting.s3_region unless SiteSetting.s3_region.empty?
-      if (SiteSetting.s3_use_iam_profile.present?)
-        options.merge!(:use_iam_profile => true)
-      else
-        options.merge!(:aws_access_key_id => SiteSetting.s3_access_key_id,
-         :aws_secret_access_key => SiteSetting.s3_secret_access_key)
-      end
-      options
-    end
-
-    def fog_with_options
-      check_missing_site_settings
-      Fog::Storage.new(s3_options)
-    end
-
-    def get_or_create_directory(bucket)
-      fog = fog_with_options
-      directory = fog.directories.get(bucket)
-      directory = fog.directories.create(key: bucket) unless directory
-      directory
-    end
-
-    def upload(file, unique_filename, filename=nil, content_type=nil)
-      args = {
-        key: unique_filename,
-        public: true,
-        body: file
-      }
-
-      if filename && !FileHelper.is_image?(filename)
-        args[:content_disposition] = "attachment; filename=\"#{filename}\""
       end
 
-      args[:content_type] = content_type if content_type
+      def get_path_for_optimized_image(file, optimized_image)
-
+        "#{optimized_image.id}#{optimized_image.sha1}_#{optimized_image.width}x#{optimized_image.height}#{optimized_image.extension}"
-      get_or_create_directory(s3_bucket).files.create(args)
-    end
-
-    def remove(unique_filename)
-      fog = fog_with_options
-      # copy the file in tombstone
-      fog.copy_object(unique_filename, s3_bucket, tombstone_prefix + unique_filename, s3_bucket)
-      # delete the file
-      fog.delete_object(s3_bucket, unique_filename)
-    rescue Excon::Errors::NotFound
-      # If the file cannot be found, don't raise an error.
-      # I am not certain if this is the right thing to do but we can't deploy
-      # right now. Please review this @ZogStriP
-    end
-
-    def update_tombstone_lifecycle(grace_period)
-      # cf. http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html
-      fog_with_options.put_bucket_lifecycle(s3_bucket, lifecycle(grace_period))
-    end
-
-    def lifecycle(grace_period)
-      {
-        "Rules" => [{
-          "Prefix" => tombstone_prefix,
-          "Enabled" => true,
-          "Expiration" => { "Days" => grace_period }
-        }]
-      }
-    end
-
-    def tombstone_prefix
-      "tombstone/"
-    end
-
-    # cf. https://github.com/aws/aws-sdk-core-ruby/blob/master/lib/aws/plugins/s3_bucket_dns.rb#L56-L78
-    def dns_compatible?(bucket_name, ssl)
-      if valid_subdomain?(bucket_name)
-        bucket_name.match(/\./) && ssl ? false : true
-      else
-        false
       end
-    end
 
-    def valid_subdomain?(bucket_name)
+      def get_path_for_avatar(file, avatar, size)
-      bucket_name.size < 64 &&
+        relative_avatar_template(avatar).gsub("{size}", size.to_s)
-      bucket_name =~ /^[a-z0-9][a-z0-9.-]+[a-z0-9]$/ &&
+      end
-      bucket_name !~ /(\d+\.){3}\d+/ &&
+
-      bucket_name !~ /[.-]{2}/
+      def relative_avatar_template(avatar)
-    end
+        "avatars/#{avatar.sha1}/{size}#{avatar.extension}"
+      end
+
+      def store_file(file, path, filename=nil, content_type=nil)
+        # stored uploaded are public by default
+        options = { public: true }
+        # add a "content disposition" header for "attachments"
+        options[:content_disposition] = "attachment; filename=\"#{filename}\"" if filename && !FileHelper.is_image?(filename)
+        # add a "content type" header when provided (ie. for "attachments")
+        options[:content_type] = content_type if content_type
+        # if this fails, it will throw an exception
+        @s3_helper.upload(file, path, options)
+        # return the upload url
+        "#{absolute_base_url}/#{path}"
+      end
+
+      def remove_file(url)
+        return unless has_been_uploaded?(url)
+        filename = File.basename(url)
+        # copy the removed file to tombstone
+        @s3_helper.remove(filename, true)
+      end
+
+      def s3_bucket
+        return @s3_bucket if @s3_bucket
+        raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank?
+        @s3_bucket = SiteSetting.s3_upload_bucket.downcase
+      end
+
+      def tombstone_prefix
+        "tombstone/"
+      end
 
   end
 

lib/s3_helper.rb

@@ -0,0 +1,102 @@
+require "fog"
+
+class S3Helper
+
+  def initialize(s3_bucket, tombstone_prefix=nil, fog=nil)
+    raise Discourse::InvalidParameters.new("s3_bucket") if s3_bucket.blank?
+
+    @s3_bucket = s3_bucket
+    @tombstone_prefix = tombstone_prefix
+
+    check_missing_site_settings
+
+    @fog = fog || Fog::Storage.new(s3_options)
+  end
+
+  def upload(file, unique_filename, options={})
+    args = {
+      body: file,
+      key: unique_filename,
+      public: false,
+    }
+
+    args.merge!(options)
+
+    directory = get_or_create_directory(@s3_bucket)
+    directory.files.create(args)
+  end
+
+  def remove(unique_filename, copy_to_tombstone=false)
+    # copy the file in tombstone
+    if copy_to_tombstone && @tombstone_prefix.present?
+      @fog.copy_object(unique_filename, @s3_bucket, @tombstone_prefix + unique_filename, @s3_bucket)
+    end
+    # delete the file
+    @fog.delete_object(@s3_bucket, unique_filename)
+  rescue Excon::Errors::NotFound
+    # if the file cannot be found, don't raise an error
+  end
+
+  def update_tombstone_lifecycle(grace_period)
+    return if @tombstone_prefix.blank?
+    # cf. http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html
+    @fog.put_bucket_lifecycle(@s3_bucket, lifecycle(grace_period))
+  end
+
+  private
+
+    def check_missing_site_settings
+      unless SiteSetting.s3_use_iam_profile
+        raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank?
+        raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank?
+      end
+    end
+
+    def s3_options
+      options = { provider: 'AWS', scheme: SiteSetting.scheme }
+
+      # cf. https://github.com/fog/fog/issues/2381
+      options[:path_style] = dns_compatible?(@s3_bucket, SiteSetting.use_https?)
+
+      options[:region] = SiteSetting.s3_region unless SiteSetting.s3_region.blank?
+
+      if SiteSetting.s3_use_iam_profile
+        options.merge!(use_iam_profile: true)
+      else
+        options.merge!(aws_access_key_id: SiteSetting.s3_access_key_id,
+                       aws_secret_access_key: SiteSetting.s3_secret_access_key)
+      end
+
+      options
+    end
+
+    def get_or_create_directory(bucket)
+      directory = @fog.directories.get(bucket)
+      directory = @fog.directories.create(key: bucket) unless directory
+      directory
+    end
+
+    def lifecycle(grace_period)
+      {
+        "Rules" => [{
+          "Prefix" => @tombstone_prefix,
+          "Enabled" => true,
+          "Expiration" => { "Days" => grace_period }
+        }]
+      }
+    end
+
+    # cf. https://github.com/aws/aws-sdk-core-ruby/blob/master/aws-sdk-core/lib/aws-sdk-core/plugins/s3_bucket_dns.rb#L65-L80
+    def dns_compatible?(bucket_name, ssl)
+      return false unless valid_subdomain?(bucket_name)
+      bucket_name.match(/\./) && ssl ? false : true
+    end
+
+    def valid_subdomain?(bucket_name)
+      bucket_name.size < 64 &&
+      bucket_name =~ /^[a-z0-9][a-z0-9.-]+[a-z0-9]$/ &&
+      bucket_name !~ /(\d+\.){3}\d+/ &&
+      bucket_name !~ /[.-]{2}/
+    end
+
+end

spec/components/discourse_spec.rb

@@ -76,7 +76,10 @@ describe Discourse do
     end
 
     it "returns S3Store when S3 is enabled" do
-      SiteSetting.expects(:enable_s3_uploads?).returns(true)
+      SiteSetting.stubs(:enable_s3_uploads?).returns(true)
+      SiteSetting.stubs(:s3_upload_bucket).returns("s3_bucket")
+      SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id")
+      SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key")
       Discourse.store.should be_a(FileStore::S3Store)
     end
 
@@ -132,7 +135,7 @@ describe Discourse do
       Sidekiq.error_handlers.clear
       Sidekiq.error_handlers << logger
     end
-    
+
     it "should not fail when called" do
       exception = StandardError.new
 

spec/components/file_store/s3_store_spec.rb

@@ -1,10 +1,10 @@
 require 'spec_helper'
-require 'fog'
 require 'file_store/s3_store'
 
 describe FileStore::S3Store do
 
-  let(:store) { FileStore::S3Store.new }
+  let(:s3_helper) { stub }
+  let(:store) { FileStore::S3Store.new(s3_helper) }
 
   let(:upload) { build(:upload) }
   let(:uploaded_file) { file_from_fixtures("logo.png") }
@@ -19,18 +19,14 @@ describe FileStore::S3Store do
     SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket")
     SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id")
     SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key")
-    Fog.mock!
-    Fog::Mock.reset
-    Fog::Mock.delay = 0
   end
 
-  after(:each) { Fog.unmock! }
-
   describe ".store_upload" do
 
     it "returns an absolute schemaless url" do
       upload.stubs(:id).returns(42)
       upload.stubs(:extension).returns(".png")
+      s3_helper.expects(:upload)
       store.store_upload(uploaded_file, upload).should == "//s3_upload_bucket.s3.amazonaws.com/42e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98.png"
     end
 
@@ -40,6 +36,7 @@ describe FileStore::S3Store do
 
     it "returns an absolute schemaless url" do
       optimized_image.stubs(:id).returns(42)
+      s3_helper.expects(:upload)
       store.store_optimized_image(optimized_image_file, optimized_image).should == "//s3_upload_bucket.s3.amazonaws.com/4286f7e437faa5a7fce15d1ddcb9eaeaea377667b8_100x200.png"
     end
 
@@ -49,6 +46,7 @@ describe FileStore::S3Store do
 
     it "returns an absolute schemaless url" do
       avatar.stubs(:id).returns(42)
+      s3_helper.expects(:upload)
       store.store_avatar(avatar_file, avatar, 100).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/100.png"
     end
 
@@ -98,6 +96,23 @@ describe FileStore::S3Store do
     store.internal?.should == false
   end
 
+  describe ".download" do
+
+    it "does nothing if the file hasn't been uploaded to that store" do
+      upload.stubs(:url).returns("/path/to/image.png")
+      FileHelper.expects(:download).never
+      store.download(upload)
+    end
+
+    it "works" do
+      upload.stubs(:url).returns("//s3_upload_bucket.s3.amazonaws.com/1337.png")
+      max_file_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes
+      FileHelper.expects(:download).with("http://s3_upload_bucket.s3.amazonaws.com/1337.png", max_file_size, "discourse-s3", true)
+      store.download(upload)
+    end
+
+  end
+
   describe ".avatar_template" do
 
     it "is present" do
@@ -106,4 +121,13 @@ describe FileStore::S3Store do
 
   end
 
+  describe ".purge_tombstone" do
+
+    it "updates tombstone lifecycle" do
+      s3_helper.expects(:update_tombstone_lifecycle)
+      store.purge_tombstone(1.day)
+    end
+
+  end
+
 end

spec/components/s3_helper_spec.rb

@@ -0,0 +1,98 @@
+require "s3_helper"
+require "spec_helper"
+
+describe "S3Helper" do
+
+  before(:each) do
+    SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id")
+    SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key")
+    Fog.mock!
+    Fog::Mock.reset
+    Fog::Mock.delay = 0
+  end
+
+  after(:each) do
+    Fog.unmock!
+  end
+
+
+  let(:s3_bucket) { "s3_bucket_name" }
+  let(:tombstone_prefix) { nil }
+  let(:fog) { stub }
+  let(:s3) { S3Helper.new(s3_bucket, tombstone_prefix, fog) }
+
+  let(:filename) { "logo.png" }
+  let(:file) { file_from_fixtures(filename) }
+
+  it "ensures the bucket name isn't blank" do
+    -> { S3Helper.new("") }.should raise_error(Discourse::InvalidParameters)
+  end
+
+  describe ".upload" do
+
+    let(:fog) { nil }
+
+    it "works" do
+      result = s3.upload(file, filename)
+      expect(result).to be_a Fog::Storage::AWS::File
+    end
+
+  end
+
+  describe ".remove" do
+
+    context "without tombstone prefix" do
+
+      it "only deletes the object even when asked to copy it to the tombstone" do
+        fog.expects(:copy_object).never
+        fog.expects(:delete_object).with(s3_bucket, filename)
+        s3.remove(filename, true)
+      end
+
+    end
+
+    context "with tombstone prefix" do
+
+      let(:tombstone_prefix) { "tombstone/" }
+
+      it "only deletes the object by default" do
+        fog.expects(:copy_object).never
+        fog.expects(:delete_object).with(s3_bucket, filename)
+        s3.remove(filename)
+      end
+
+      it "copies the object to the tombstone and deletes it when asked for" do
+        fog.expects(:copy_object)
+        fog.expects(:delete_object).with(s3_bucket, filename)
+        s3.remove(filename, true)
+      end
+
+    end
+
+  end
+
+  describe ".update_tombstone_lifecycle" do
+
+    context "without tombstone prefix" do
+
+      it "doesn't call put_bucket_lifecycle" do
+        fog.expects(:put_bucket_lifecycle).never
+        s3.update_tombstone_lifecycle(3.days)
+      end
+
+    end
+
+    context "with tombstone prefix" do
+
+      let(:tombstone_prefix) { "tombstone/" }
+
+      it "calls put_bucket_lifecycle" do
+        fog.expects(:put_bucket_lifecycle)
+        s3.update_tombstone_lifecycle(3.days)
+      end
+
+    end
+
+  end
+
+end