Merge branch 'rs-carrierwave-db' into 'master'

Record file uploads in the database

See merge request !8893

29 files changed, 528 insertions, 19 deletions

diff --git a/Gemfile b/Gemfile
index ca0fc9810a1..0869eba116b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -79,7 +79,7 @@ gem 'kaminari', '~> 0.17.0'
 gem 'hamlit', '~> 2.6.1'
 
 # Files attachments
-gem 'carrierwave', '~> 0.10.0'
+gem 'carrierwave', '~> 0.11.0'
 
 # Drag and Drop UI
 gem 'dropzonejs-rails', '~> 0.7.1'
diff --git a/Gemfile.lock b/Gemfile.lock
index 933f7199979..f59bde27bc4 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -103,11 +103,12 @@ GEM
     capybara-screenshot (1.0.11)
       capybara (>= 1.0, < 3)
       launchy
-    carrierwave (0.10.0)
+    carrierwave (0.11.2)
       activemodel (>= 3.2.0)
       activesupport (>= 3.2.0)
       json (>= 1.7)
       mime-types (>= 1.16)
+      mimemagic (>= 0.3.0)
     cause (0.1)
     charlock_holmes (0.7.3)
     chronic (0.10.2)
@@ -850,7 +851,7 @@ DEPENDENCIES
   bundler-audit (~> 0.5.0)
   capybara (~> 2.6.2)
   capybara-screenshot (~> 1.0.0)
-  carrierwave (~> 0.10.0)
+  carrierwave (~> 0.11.0)
   charlock_holmes (~> 0.7.3)
   chronic (~> 0.10.2)
   chronic_duration (~> 0.10.6)
diff --git a/app/models/appearance.rb b/app/models/appearance.rb
index e4106e1c2e9..c79326e8427 100644
--- a/app/models/appearance.rb
+++ b/app/models/appearance.rb
@@ -10,4 +10,5 @@ class Appearance < ActiveRecord::Base
 
   mount_uploader :logo,         AttachmentUploader
   mount_uploader :header_logo,  AttachmentUploader
+  has_many :uploads, as: :model, dependent: :destroy
 end
diff --git a/app/models/group.rb b/app/models/group.rb
index e1ac7b63c6b..bd0ecae3da4 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -28,6 +28,7 @@ class Group < Namespace
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   mount_uploader :avatar, AvatarUploader
+  has_many :uploads, as: :model, dependent: :destroy
 
   after_create :post_create_hook
   after_destroy :post_destroy_hook
diff --git a/app/models/project.rb b/app/models/project.rb
index 1ac4a178a9b..7d211784c3c 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -212,6 +212,7 @@ class Project < ActiveRecord::Base
   before_save :ensure_runners_token
 
   mount_uploader :avatar, AvatarUploader
+  has_many :uploads, as: :model, dependent: :destroy
 
   # Scopes
   default_scope { where(pending_delete: false) }
diff --git a/app/models/upload.rb b/app/models/upload.rb
new file mode 100644
index 00000000000..13987931b05
--- /dev/null
+++ b/app/models/upload.rb
@@ -0,0 +1,63 @@
+class Upload < ActiveRecord::Base
+  # Upper limit for foreground checksum processing
+  CHECKSUM_THRESHOLD = 100.megabytes
+
+  belongs_to :model, polymorphic: true
+
+  validates :size, presence: true
+  validates :path, presence: true
+  validates :model, presence: true
+  validates :uploader, presence: true
+
+  before_save  :calculate_checksum, if:     :foreground_checksum?
+  after_commit :schedule_checksum,  unless: :foreground_checksum?
+
+  def self.remove_path(path)
+    where(path: path).destroy_all
+  end
+
+  def self.record(uploader)
+    remove_path(uploader.relative_path)
+
+    create(
+      size: uploader.file.size,
+      path: uploader.relative_path,
+      model: uploader.model,
+      uploader: uploader.class.to_s
+    )
+  end
+
+  def absolute_path
+    return path unless relative_path?
+
+    uploader_class.absolute_path(self)
+  end
+
+  def calculate_checksum
+    return unless exist?
+
+    self.checksum = Digest::SHA256.file(absolute_path).hexdigest
+  end
+
+  def exist?
+    File.exist?(absolute_path)
+  end
+
+  private
+
+  def foreground_checksum?
+    size <= CHECKSUM_THRESHOLD
+  end
+
+  def schedule_checksum
+    UploadChecksumWorker.perform_async(id)
+  end
+
+  def relative_path?
+    !path.start_with?('/')
+  end
+
+  def uploader_class
+    Object.const_get(uploader)
+  end
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index dfba51d3b00..bd57904a2cd 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -191,6 +191,7 @@ class User < ActiveRecord::Base
   end
 
   mount_uploader :avatar, AvatarUploader
+  has_many :uploads, as: :model, dependent: :destroy
 
   # Scopes
   scope :admins, -> { where(admin: true) }
diff --git a/app/uploaders/attachment_uploader.rb b/app/uploaders/attachment_uploader.rb
index 6aa1f5a8c50..109eb2fea0b 100644
--- a/app/uploaders/attachment_uploader.rb
+++ b/app/uploaders/attachment_uploader.rb
@@ -1,4 +1,5 @@
 class AttachmentUploader < GitlabUploader
+  include RecordsUploads
   include UploaderHelper
 
   storage :file
diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb
index b4c393c6f2c..66d3bcb998a 100644
--- a/app/uploaders/avatar_uploader.rb
+++ b/app/uploaders/avatar_uploader.rb
@@ -1,4 +1,5 @@
 class AvatarUploader < GitlabUploader
+  include RecordsUploads
   include UploaderHelper
 
   storage :file
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 0d2edaeff3b..d6ccf0dc92c 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -1,9 +1,31 @@
 class FileUploader < GitlabUploader
+  include RecordsUploads
   include UploaderHelper
+
   MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)}
 
   storage :file
 
+  def self.absolute_path(upload_record)
+    File.join(
+      self.dynamic_path_segment(upload_record.model),
+      upload_record.path
+    )
+  end
+
+  # Returns the part of `store_dir` that can change based on the model's current
+  # path
+  #
+  # This is used to build Upload paths dynamically based on the model's current
+  # namespace and path, allowing us to ignore renames or transfers.
+  #
+  # model - Object that responds to `path_with_namespace`
+  #
+  # Returns a String without a trailing slash
+  def self.dynamic_path_segment(model)
+    File.join(CarrierWave.root, base_dir, model.path_with_namespace)
+  end
+
   attr_accessor :project
   attr_reader :secret
 
@@ -13,13 +35,21 @@ class FileUploader < GitlabUploader
   end
 
   def store_dir
-    File.join(base_dir, @project.path_with_namespace, @secret)
+    File.join(dynamic_path_segment, @secret)
   end
 
   def cache_dir
     File.join(base_dir, 'tmp', @project.path_with_namespace, @secret)
   end
 
+  def model
+    project
+  end
+
+  def relative_path
+    self.file.path.sub("#{dynamic_path_segment}/", '')
+  end
+
   def to_markdown
     to_h[:markdown]
   end
@@ -40,6 +70,10 @@ class FileUploader < GitlabUploader
 
   private
 
+  def dynamic_path_segment
+    self.class.dynamic_path_segment(model)
+  end
+
   def generate_secret
     SecureRandom.hex
   end
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index bd7de4ed562..d662ba6820c 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -1,4 +1,8 @@
 class GitlabUploader < CarrierWave::Uploader::Base
+  def self.absolute_path(upload_record)
+    File.join(CarrierWave.root, upload_record.path)
+  end
+
   def self.base_dir
     'uploads'
   end
@@ -18,4 +22,15 @@ class GitlabUploader < CarrierWave::Uploader::Base
   def move_to_store
     true
   end
+
+  # Designed to be overridden by child uploaders that have a dynamic path
+  # segment -- that is, a path that changes based on mutable attributes of its
+  # associated model
+  #
+  # For example, `FileUploader` builds the storage path based on the associated
+  # project model's `path_with_namespace` value, which can change when the
+  # project or its containing namespace is moved or renamed.
+  def relative_path
+    self.file.path.sub("#{root}/", '')
+  end
 end
diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb
new file mode 100644
index 00000000000..4c127f29250
--- /dev/null
+++ b/app/uploaders/records_uploads.rb
@@ -0,0 +1,34 @@
+module RecordsUploads
+  extend ActiveSupport::Concern
+
+  included do
+    after :store,   :record_upload
+    before :remove, :destroy_upload
+  end
+
+  private
+
+  # After storing an attachment, create a corresponding Upload record
+  #
+  # NOTE: We're ignoring the argument passed to this callback because we want
+  # the `SanitizedFile` object from `CarrierWave::Uploader::Base#file`, not the
+  # `Tempfile` object the callback gets.
+  #
+  # Called `after :store`
+  def record_upload(_tempfile)
+    return unless file_storage?
+    return unless file.exists?
+
+    Upload.record(self)
+  end
+
+  # Before removing an attachment, destroy any Upload records at the same path
+  #
+  # Called `before :remove`
+  def destroy_upload(*args)
+    return unless file_storage?
+    return unless file
+
+    Upload.remove_path(relative_path)
+  end
+end
diff --git a/app/workers/upload_checksum_worker.rb b/app/workers/upload_checksum_worker.rb
new file mode 100644
index 00000000000..78931f1258f
--- /dev/null
+++ b/app/workers/upload_checksum_worker.rb
@@ -0,0 +1,12 @@
+class UploadChecksumWorker
+  include Sidekiq::Worker
+  include DedicatedSidekiqQueue
+
+  def perform(upload_id)
+    upload = Upload.find(upload_id)
+    upload.calculate_checksum
+    upload.save!
+  rescue ActiveRecord::RecordNotFound
+    Rails.logger.error("UploadChecksumWorker: couldn't find upload #{upload_id}, skipping")
+  end
+end
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 97620cc9c7f..824f99e687e 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -29,6 +29,7 @@
   - [email_receiver, 2]
   - [emails_on_push, 2]
   - [mailers, 2]
+  - [upload_checksum, 1]
   - [use_key, 1]
   - [repository_fork, 1]
   - [repository_import, 1]
diff --git a/db/migrate/20170130221926_create_uploads.rb b/db/migrate/20170130221926_create_uploads.rb
new file mode 100644
index 00000000000..6f06c5dd840
--- /dev/null
+++ b/db/migrate/20170130221926_create_uploads.rb
@@ -0,0 +1,20 @@
+class CreateUploads < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :uploads do |t|
+      t.integer :size, limit: 8, null: false
+      t.string :path, null: false
+      t.string :checksum, limit: 64
+      t.references :model, polymorphic: true
+      t.string :uploader, null: false
+      t.datetime :created_at, null: false
+    end
+
+    add_index :uploads, :path
+    add_index :uploads, :checksum
+    add_index :uploads, [:model_id, :model_type]
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 94fd64bc561..624cf9432d0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1224,6 +1224,20 @@ ActiveRecord::Schema.define(version: 20170306170512) do
   add_index "u2f_registrations", ["key_handle"], name: "index_u2f_registrations_on_key_handle", using: :btree
   add_index "u2f_registrations", ["user_id"], name: "index_u2f_registrations_on_user_id", using: :btree
 
+  create_table "uploads", force: :cascade do |t|
+    t.integer "size", limit: 8, null: false
+    t.string "path", null: false
+    t.string "checksum", limit: 64
+    t.integer "model_id"
+    t.string "model_type"
+    t.string "uploader", null: false
+    t.datetime "created_at", null: false
+  end
+
+  add_index "uploads", ["checksum"], name: "index_uploads_on_checksum", using: :btree
+  add_index "uploads", ["model_id", "model_type"], name: "index_uploads_on_model_id_and_model_type", using: :btree
+  add_index "uploads", ["path"], name: "index_uploads_on_path", using: :btree
+
   create_table "user_agent_details", force: :cascade do |t|
     t.string "user_agent", null: false
     t.string "ip_address", null: false
diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index 699c6f77cec..cd6961a7bd5 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -35,6 +35,19 @@ describe Projects::UploadsController do
         expect(response.body).to match '\"alt\":\"rails_sample\"'
         expect(response.body).to match "\"url\":\"/uploads"
       end
+
+      # NOTE: This is as close as we're getting to an Integration test for this
+      # behavior. We're avoiding a proper Feature test because those should be
+      # testing things entirely user-facing, which the Upload model is very much
+      # not.
+      it 'creates a corresponding Upload record' do
+        upload = Upload.last
+
+        aggregate_failures do
+          expect(upload).to exist
+          expect(upload.model).to eq project
+        end
+      end
     end
 
     context 'with valid non-image file' do
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index f20b6be51e1..1a1280e5198 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -199,6 +199,7 @@ project:
 - project_authorizations
 - route
 - statistics
+- uploads
 award_emoji:
 - awardable
 - user
diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb
index 0b72a2f979b..1060bf3cbf4 100644
--- a/spec/models/appearance_spec.rb
+++ b/spec/models/appearance_spec.rb
@@ -7,4 +7,6 @@ RSpec.describe Appearance, type: :model do
 
   it { is_expected.to validate_presence_of(:title) }
   it { is_expected.to validate_presence_of(:description) }
+
+  it { is_expected.to have_many(:uploads).dependent(:destroy) }
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 8cfc2085458..5d87938235a 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -13,6 +13,7 @@ describe Group, models: true do
     it { is_expected.to have_many(:shared_projects).through(:project_group_links) }
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
     it { is_expected.to have_many(:labels).class_name('GroupLabel') }
+    it { is_expected.to have_many(:uploads).dependent(:destroy) }
     it { is_expected.to have_one(:chat_team) }
 
     describe '#members & #requesters' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index ee4f4092062..84bdcbe8e59 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -71,6 +71,7 @@ describe Project, models: true do
     it { is_expected.to have_many(:project_group_links).dependent(:destroy) }
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
     it { is_expected.to have_many(:forks).through(:forked_project_links) }
+    it { is_expected.to have_many(:uploads).dependent(:destroy) }
 
     context 'after initialized' do
       it "has a project_feature" do
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
new file mode 100644
index 00000000000..4c832c87d6a
--- /dev/null
+++ b/spec/models/upload_spec.rb
@@ -0,0 +1,151 @@
+require 'rails_helper'
+
+describe Upload, type: :model do
+  describe 'assocations' do
+    it { is_expected.to belong_to(:model) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:size) }
+    it { is_expected.to validate_presence_of(:path) }
+    it { is_expected.to validate_presence_of(:model) }
+    it { is_expected.to validate_presence_of(:uploader) }
+  end
+
+  describe 'callbacks' do
+    context 'for a file above the checksum threshold' do
+      it 'schedules checksum calculation' do
+        stub_const('UploadChecksumWorker', spy)
+
+        upload = described_class.create(
+          path: __FILE__,
+          size: described_class::CHECKSUM_THRESHOLD + 1.kilobyte,
+          model: build_stubbed(:user),
+          uploader: double('ExampleUploader')
+        )
+
+        expect(UploadChecksumWorker)
+          .to have_received(:perform_async).with(upload.id)
+      end
+    end
+
+    context 'for a file at or below the checksum threshold' do
+      it 'calculates checksum immediately before save' do
+        upload = described_class.new(
+          path: __FILE__,
+          size: described_class::CHECKSUM_THRESHOLD,
+          model: build_stubbed(:user),
+          uploader: double('ExampleUploader')
+        )
+
+        expect { upload.save }
+          .to change { upload.checksum }.from(nil)
+          .to(a_string_matching(/\A\h{64}\z/))
+      end
+    end
+  end
+
+  describe '.remove_path' do
+    it 'removes all records at the given path' do
+      described_class.create!(
+        size: File.size(__FILE__),
+        path: __FILE__,
+        model: build_stubbed(:user),
+        uploader: 'AvatarUploader'
+      )
+
+      expect { described_class.remove_path(__FILE__) }.
+        to change { described_class.count }.from(1).to(0)
+    end
+  end
+
+  describe '.record' do
+    let(:fake_uploader) do
+      double(
+        file: double(size: 12_345),
+        relative_path: 'foo/bar.jpg',
+        model: build_stubbed(:user),
+        class: 'AvatarUploader'
+      )
+    end
+
+    it 'removes existing paths before creation' do
+      expect(described_class).to receive(:remove_path)
+        .with(fake_uploader.relative_path)
+
+      described_class.record(fake_uploader)
+    end
+
+    it 'creates a new record and assigns size, path, model, and uploader' do
+      upload = described_class.record(fake_uploader)
+
+      aggregate_failures do
+        expect(upload).to be_persisted
+        expect(upload.size).to eq fake_uploader.file.size
+        expect(upload.path).to eq fake_uploader.relative_path
+        expect(upload.model_id).to eq fake_uploader.model.id
+        expect(upload.model_type).to eq fake_uploader.model.class.to_s
+        expect(upload.uploader).to eq fake_uploader.class
+      end
+    end
+  end
+
+  describe '#absolute_path' do
+    it 'returns the path directly when already absolute' do
+      path = '/path/to/namespace/project/secret/file.jpg'
+      upload = described_class.new(path: path)
+
+      expect(upload).not_to receive(:uploader_class)
+
+      expect(upload.absolute_path).to eq path
+    end
+
+    it "delegates to the uploader's absolute_path method" do
+      uploader = spy('FakeUploader')
+      upload = described_class.new(path: 'secret/file.jpg')
+      expect(upload).to receive(:uploader_class).and_return(uploader)
+
+      upload.absolute_path
+
+      expect(uploader).to have_received(:absolute_path).with(upload)
+    end
+  end
+
+  describe '#calculate_checksum' do
+    it 'calculates the SHA256 sum' do
+      upload = described_class.new(
+        path: __FILE__,
+        size: described_class::CHECKSUM_THRESHOLD - 1.megabyte
+      )
+      expected = Digest::SHA256.file(__FILE__).hexdigest
+
+      expect { upload.calculate_checksum }
+        .to change { upload.checksum }.from(nil).to(expected)
+    end
+
+    it 'returns nil for a non-existant file' do
+      upload = described_class.new(
+        path: __FILE__,
+        size: described_class::CHECKSUM_THRESHOLD - 1.megabyte
+      )
+
+      expect(upload).to receive(:exist?).and_return(false)
+
+      expect(upload.calculate_checksum).to be_nil
+    end
+  end
+
+  describe '#exist?' do
+    it 'returns true when the file exists' do
+      upload = described_class.new(path: __FILE__)
+
+      expect(upload).to exist
+    end
+
+    it 'returns false when the file does not exist' do
+      upload = described_class.new(path: "#{__FILE__}-nope")
+
+      expect(upload).not_to exist
+    end
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index b99cde64675..adb5b538922 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -36,6 +36,7 @@ describe User, models: true do
     it { is_expected.to have_many(:builds).dependent(:nullify) }
     it { is_expected.to have_many(:pipelines).dependent(:nullify) }
     it { is_expected.to have_many(:chat_names).dependent(:destroy) }
+    it { is_expected.to have_many(:uploads).dependent(:destroy) }
 
     describe '#group_members' do
       it 'does not include group memberships for which user is a requester' do
diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb
index c42eeba4b9c..150c8ccaef7 100644
--- a/spec/services/projects/upload_service_spec.rb
+++ b/spec/services/projects/upload_service_spec.rb
@@ -10,7 +10,7 @@ describe Projects::UploadService, services: true do
     context 'for valid gif file' do
       before do
         gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
-        @link_to_file = upload_file(@project.repository, gif)
+        @link_to_file = upload_file(@project, gif)
       end
 
       it { expect(@link_to_file).to have_key(:alt) }
@@ -23,7 +23,7 @@ describe Projects::UploadService, services: true do
       before do
         png = fixture_file_upload(Rails.root + 'spec/fixtures/dk.png',
           'image/png')
-        @link_to_file = upload_file(@project.repository, png)
+        @link_to_file = upload_file(@project, png)
       end
 
       it { expect(@link_to_file).to have_key(:alt) }
@@ -35,7 +35,7 @@ describe Projects::UploadService, services: true do
     context 'for valid jpg file' do
       before do
         jpg = fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg')
-        @link_to_file = upload_file(@project.repository, jpg)
+        @link_to_file = upload_file(@project, jpg)
       end
 
       it { expect(@link_to_file).to have_key(:alt) }
@@ -47,7 +47,7 @@ describe Projects::UploadService, services: true do
     context 'for txt file' do
       before do
         txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain')
-        @link_to_file = upload_file(@project.repository, txt)
+        @link_to_file = upload_file(@project, txt)
       end
 
       it { expect(@link_to_file).to have_key(:alt) }
@@ -60,14 +60,14 @@ describe Projects::UploadService, services: true do
       before do
         txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain')
         allow(txt).to receive(:size) { 1000.megabytes.to_i }
-        @link_to_file = upload_file(@project.repository, txt)
+        @link_to_file = upload_file(@project, txt)
       end
 
       it { expect(@link_to_file).to eq(nil) }
     end
   end
 
-  def upload_file(repository, file)
-    Projects::UploadService.new(repository, file).execute
+  def upload_file(project, file)
+    Projects::UploadService.new(project, file).execute
   end
 end
diff --git a/spec/support/carrierwave.rb b/spec/support/carrierwave.rb
index 72af2c70324..b4b016e408f 100644
--- a/spec/support/carrierwave.rb
+++ b/spec/support/carrierwave.rb
@@ -1,7 +1,7 @@
-CarrierWave.root = 'tmp/tests/uploads'
+CarrierWave.root = File.expand_path('tmp/tests/public', Rails.root)
 
 RSpec.configure do |config|
   config.after(:each) do
-    FileUtils.rm_rf('tmp/tests/uploads')
+    FileUtils.rm_rf(CarrierWave.root)
   end
 end
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index b0f5be55c33..d9113ef4095 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -1,7 +1,19 @@
 require 'spec_helper'
 
 describe FileUploader do
-  let(:uploader) { described_class.new(build_stubbed(:project)) }
+  let(:uploader) { described_class.new(build_stubbed(:empty_project)) }
+
+  describe '.absolute_path' do
+    it 'returns the correct absolute path by building it dynamically' do
+      project = build_stubbed(:project)
+      upload = double(model: project, path: 'secret/foo.jpg')
+
+      dynamic_segment = project.path_with_namespace
+
+      expect(described_class.absolute_path(upload))
+        .to end_with("#{dynamic_segment}/secret/foo.jpg")
+    end
+  end
 
   describe 'initialize' do
     it 'generates a secret if none is provided' do
@@ -32,4 +44,13 @@ describe FileUploader do
       expect(uploader.move_to_store).to eq(true)
     end
   end
+
+  describe '#relative_path' do
+    it 'removes the leading dynamic path segment' do
+      fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')
+      uploader.store!(fixture_file_upload(fixture))
+
+      expect(uploader.relative_path).to match(/\A\h{32}\/rails_sample.jpg\z/)
+    end
+  end
 end
diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb
new file mode 100644
index 00000000000..5c26e334a6e
--- /dev/null
+++ b/spec/uploaders/records_uploads_spec.rb
@@ -0,0 +1,97 @@
+require 'rails_helper'
+
+describe RecordsUploads do
+  let(:uploader) do
+    class RecordsUploadsExampleUploader < GitlabUploader
+      include RecordsUploads
+
+      storage :file
+
+      def model
+        FactoryGirl.build_stubbed(:user)
+      end
+    end
+
+    RecordsUploadsExampleUploader.new
+  end
+
+  def upload_fixture(filename)
+    fixture_file_upload(Rails.root.join('spec', 'fixtures', filename))
+  end
+
+  describe 'callbacks' do
+    it 'calls `record_upload` after `store`' do
+      expect(uploader).to receive(:record_upload).once
+
+      uploader.store!(upload_fixture('doc_sample.txt'))
+    end
+
+    it 'calls `destroy_upload` after `remove`' do
+      expect(uploader).to receive(:destroy_upload).once
+
+      uploader.store!(upload_fixture('doc_sample.txt'))
+
+      uploader.remove!
+    end
+  end
+
+  describe '#record_upload callback' do
+    it 'returns early when not using file storage' do
+      allow(uploader).to receive(:file_storage?).and_return(false)
+      expect(Upload).not_to receive(:record)
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+    end
+
+    it "returns early when the file doesn't exist" do
+      allow(uploader).to receive(:file).and_return(double(exists?: false))
+      expect(Upload).not_to receive(:record)
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+    end
+
+    it 'creates an Upload record after store' do
+      expect(Upload).to receive(:record)
+        .with(uploader)
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+    end
+
+    it 'it destroys Upload records at the same path before recording' do
+      existing = Upload.create!(
+        path: File.join('uploads', 'rails_sample.jpg'),
+        size: 512.kilobytes,
+        model: build_stubbed(:user),
+        uploader: uploader.class.to_s
+      )
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+
+      expect { existing.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(Upload.count).to eq 1
+    end
+  end
+
+  describe '#destroy_upload callback' do
+    it 'returns early when not using file storage' do
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+
+      allow(uploader).to receive(:file_storage?).and_return(false)
+      expect(Upload).not_to receive(:remove_path)
+
+      uploader.remove!
+    end
+
+    it 'returns early when file is nil' do
+      expect(Upload).not_to receive(:remove_path)
+
+      uploader.remove!
+    end
+
+    it 'it destroys Upload records at the same path after removal' do
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+
+      expect { uploader.remove! }.to change { Upload.count }.from(1).to(0)
+    end
+  end
+end
diff --git a/spec/uploaders/uploader_helper_spec.rb b/spec/uploaders/uploader_helper_spec.rb
index e9efd13b9aa..c47f09adb6d 100644
--- a/spec/uploaders/uploader_helper_spec.rb
+++ b/spec/uploaders/uploader_helper_spec.rb
@@ -1,10 +1,14 @@
 require 'rails_helper'
 
 describe UploaderHelper do
-  class ExampleUploader < CarrierWave::Uploader::Base
-    include UploaderHelper
+  let(:uploader) do
+    example_uploader = Class.new(CarrierWave::Uploader::Base) do
+      include UploaderHelper
 
-    storage :file
+      storage :file
+    end
+
+    example_uploader.new
   end
 
   def upload_fixture(filename)
@@ -12,8 +16,6 @@ describe UploaderHelper do
   end
 
   describe '#image_or_video?' do
-    let(:uploader) { ExampleUploader.new }
-
     it 'returns true for an image file' do
       uploader.store!(upload_fixture('dk.png'))
 
diff --git a/spec/workers/upload_checksum_worker_spec.rb b/spec/workers/upload_checksum_worker_spec.rb
new file mode 100644
index 00000000000..911360da66c
--- /dev/null
+++ b/spec/workers/upload_checksum_worker_spec.rb
@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+describe UploadChecksumWorker do
+  describe '#perform' do
+    it 'rescues ActiveRecord::RecordNotFound' do
+      expect { described_class.new.perform(999_999) }.not_to raise_error
+    end
+
+    it 'calls calculate_checksum_without_delay and save!' do
+      upload = spy
+      expect(Upload).to receive(:find).with(999_999).and_return(upload)
+
+      described_class.new.perform(999_999)
+
+      expect(upload).to have_received(:calculate_checksum)
+      expect(upload).to have_received(:save!)
+    end
+  end
+end