Revert "Revert NOT NULL constraints on file_store columns"cleanup-fill-file-store-background-migrations-with-not-null-constraint

This reverts commit 3894be2f1b512c89bfd7df433809519be0577efc.

6 files changed, 33 insertions, 13 deletions

diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index 3b952391b7e..52255696ecd 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -10,11 +10,10 @@ module Ci
     mount_uploader :file, JobArtifactUploader
 
     before_save :set_size, if: :file_changed?
+    before_save :set_file_store, if: :file_changed?
     after_save :update_project_statistics_after_save, if: :size_changed?
     after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
 
-    after_save :update_file_store, if: :file_changed?
-
     scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
 
     delegate :exists?, :open, to: :file
@@ -25,10 +24,8 @@ module Ci
       trace: 3
     }
 
-    def update_file_store
-      # The file.object_store is set during `uploader.store!`
-      # which happens after object is inserted/updated
-      self.update_column(:file_store, file.object_store)
+    def set_file_store
+      self.file_store = file.object_store
     end
 
     def self.artifacts_size_for(project)
diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb
index 84487031ee5..e0a0803a596 100644
--- a/app/models/lfs_object.rb
+++ b/app/models/lfs_object.rb
@@ -7,16 +7,16 @@ class LfsObject < ActiveRecord::Base
 
   scope :with_files_stored_locally, -> { where(file_store: [nil, LfsObjectUploader::Store::LOCAL]) }
 
+  before_save :set_file_store, if: :file_changed?
+
   validates :oid, presence: true, uniqueness: true
 
   mount_uploader :file, LfsObjectUploader
 
   after_save :update_file_store, if: :file_changed?
 
-  def update_file_store
-    # The file.object_store is set during `uploader.store!`
-    # which happens after object is inserted/updated
-    self.update_column(:file_store, file.object_store)
+  def set_file_store
+    self.file_store = file.object_store
   end
 
   def project_allowed_access?(project)
diff --git a/db/post_migrate/20180701172935_add_not_null_constraint_to_file_store.rb b/db/post_migrate/20180701172935_add_not_null_constraint_to_file_store.rb
new file mode 100644
index 00000000000..18130a84fae
--- /dev/null
+++ b/db/post_migrate/20180701172935_add_not_null_constraint_to_file_store.rb
@@ -0,0 +1,19 @@
+class AddNotNullConstraintToFileStore < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    change_column_null :ci_job_artifacts, :file_store, false
+    change_column_null :lfs_objects, :file_store, false
+    change_column_null :uploads, :store, false
+  end
+
+  def down
+    change_column_null :ci_job_artifacts, :file_store, true
+    change_column_null :lfs_objects, :file_store, true
+    change_column_null :uploads, :store, true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 55f4a04751d..3c161dc3b7b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -376,7 +376,7 @@ ActiveRecord::Schema.define(version: 20180702174635) do
     t.integer "project_id", null: false
     t.integer "job_id", null: false
     t.integer "file_type", null: false
-    t.integer "file_store"
+    t.integer "file_store", null: false
     t.integer "size", limit: 8
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -1084,9 +1084,10 @@ ActiveRecord::Schema.define(version: 20180702174635) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string "file"
-    t.integer "file_store"
+    t.integer "file_store", null: false
   end
 
+  add_index "lfs_objects", ["file_store"], name: "index_lfs_objects_on_file_store", using: :btree
   add_index "lfs_objects", ["oid"], name: "index_lfs_objects_on_oid", unique: true, using: :btree
 
   create_table "lfs_objects_projects", force: :cascade do |t|
@@ -1981,11 +1982,12 @@ ActiveRecord::Schema.define(version: 20180702174635) do
     t.datetime "created_at", null: false
     t.string "mount_point"
     t.string "secret"
-    t.integer "store"
+    t.integer "store", 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", ["store"], name: "index_uploads_on_store", using: :btree
   add_index "uploads", ["uploader", "path"], name: "index_uploads_on_uploader_and_path", using: :btree
 
   create_table "user_agent_details", force: :cascade do |t|
diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb
index 3d3287d8168..02f61c330aa 100644
--- a/spec/factories/ci/job_artifacts.rb
+++ b/spec/factories/ci/job_artifacts.rb
@@ -4,6 +4,7 @@ FactoryBot.define do
   factory :ci_job_artifact, class: Ci::JobArtifact do
     job factory: :ci_build
     file_type :archive
+    file_store ObjectStorage::Store::LOCAL
 
     trait :remote_store do
       file_store JobArtifactUploader::Store::REMOTE
diff --git a/spec/factories/lfs_objects.rb b/spec/factories/lfs_objects.rb
index c909471bb55..971d39bc1fd 100644
--- a/spec/factories/lfs_objects.rb
+++ b/spec/factories/lfs_objects.rb
@@ -4,6 +4,7 @@ FactoryBot.define do
   factory :lfs_object do
     sequence(:oid) { |n| "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a%05x" % n }
     size 499013
+    file_store ObjectStorage::Store::LOCAL
   end
 
   trait :with_file do