Split migration in add_column and post-deployment46806-typed-ci-variables-no-DB-default

Instead of add_column_with_default add variable_type column with no DB
default in regular migration and populate with post-deployment
migration.

6 files changed, 66 insertions, 11 deletions

diff --git a/app/models/concerns/has_variable.rb b/app/models/concerns/has_variable.rb
index b2ef9ce2022..a3ebc921d59 100644
--- a/app/models/concerns/has_variable.rb
+++ b/app/models/concerns/has_variable.rb
@@ -18,6 +18,13 @@ module HasVariable
       file: 2
     }
 
+    before_validation(on: :create) do
+      self.variable_type = 'env_var' unless variable_type?
+    end
+
+    validates :variable_type,
+      presence: true
+
     validates :key,
       presence: true,
       length: { maximum: 255 },
@@ -35,6 +42,10 @@ module HasVariable
     def key=(new_key)
       super(new_key.to_s.strip)
     end
+
+    def variable_type
+      super.presence || 'env_var'
+    end
   end
 
   def to_runner_variable
diff --git a/db/migrate/20190415030217_add_variable_type_to_ci_variables.rb b/db/migrate/20190415030217_add_variable_type_to_ci_variables.rb
index 8c504fc51a3..3c430a2a33d 100644
--- a/db/migrate/20190415030217_add_variable_type_to_ci_variables.rb
+++ b/db/migrate/20190415030217_add_variable_type_to_ci_variables.rb
@@ -1,15 +1,11 @@
 # frozen_string_literal: true
 
 class AddVariableTypeToCiVariables < ActiveRecord::Migration[5.0]
-  include Gitlab::Database::MigrationHelpers
-  disable_ddl_transaction!
-
   DOWNTIME = false
-  ENV_VAR_VARIABLE_TYPE = 1
 
   def up
     %i(ci_group_variables ci_pipeline_schedule_variables ci_pipeline_variables ci_variables).each do |table_name|
-      add_column_with_default(table_name, :variable_type, :smallint, default: ENV_VAR_VARIABLE_TYPE)
+      add_column(table_name, :variable_type, :smallint, null: true)
     end
   end
 
diff --git a/db/post_migrate/20190416111354_populate_ci_variables_type.rb b/db/post_migrate/20190416111354_populate_ci_variables_type.rb
new file mode 100644
index 00000000000..9341dec9319
--- /dev/null
+++ b/db/post_migrate/20190416111354_populate_ci_variables_type.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class PopulateCiVariablesType < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  ENV_VAR_VARIABLE_TYPE = 1
+
+  disable_ddl_transaction!
+
+  def up
+    %i(ci_group_variables ci_pipeline_schedule_variables ci_pipeline_variables ci_variables).each do |table_name|
+      update_column_in_batches(table_name, :variable_type, ENV_VAR_VARIABLE_TYPE) do |table, query|
+        query.where(table[:variable_type].eq(nil))
+      end
+    end
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a673b159725..e3099bd6ab1 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20190415030217) do
+ActiveRecord::Schema.define(version: 20190416111354) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -417,7 +417,7 @@ ActiveRecord::Schema.define(version: 20190415030217) do
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
     t.boolean "masked", default: false, null: false
-    t.integer "variable_type", limit: 2, default: 1, null: false
+    t.integer "variable_type", limit: 2
     t.index ["group_id", "key"], name: "index_ci_group_variables_on_group_id_and_key", unique: true, using: :btree
   end
 
@@ -457,7 +457,7 @@ ActiveRecord::Schema.define(version: 20190415030217) do
     t.integer "pipeline_schedule_id", null: false
     t.datetime_with_timezone "created_at"
     t.datetime_with_timezone "updated_at"
-    t.integer "variable_type", limit: 2, default: 1, null: false
+    t.integer "variable_type", limit: 2
     t.index ["pipeline_schedule_id", "key"], name: "index_ci_pipeline_schedule_variables_on_schedule_id_and_key", unique: true, using: :btree
   end
 
@@ -484,7 +484,7 @@ ActiveRecord::Schema.define(version: 20190415030217) do
     t.string "encrypted_value_salt"
     t.string "encrypted_value_iv"
     t.integer "pipeline_id", null: false
-    t.integer "variable_type", limit: 2, default: 1, null: false
+    t.integer "variable_type", limit: 2
     t.index ["pipeline_id", "key"], name: "index_ci_pipeline_variables_on_pipeline_id_and_key", unique: true, using: :btree
   end
 
@@ -619,7 +619,7 @@ ActiveRecord::Schema.define(version: 20190415030217) do
     t.boolean "protected", default: false, null: false
     t.string "environment_scope", default: "*", null: false
     t.boolean "masked", default: false, null: false
-    t.integer "variable_type", limit: 2, default: 1, null: false
+    t.integer "variable_type", limit: 2
     t.index ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true, using: :btree
   end
 
diff --git a/spec/migrations/populate_ci_variables_type_spec.rb b/spec/migrations/populate_ci_variables_type_spec.rb
new file mode 100644
index 00000000000..910e402c344
--- /dev/null
+++ b/spec/migrations/populate_ci_variables_type_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20190416111354_populate_ci_variables_type.rb')
+
+describe PopulateCiVariablesType, :migration do
+  let(:projects) { table(:projects) }
+  let(:ci_variables) { table(:ci_variables) }
+
+  it 'populates variable type for legacy CI variables' do
+    project = projects.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce', namespace_id: 1)
+    # Skipping ci_group_variables, ci_pipeline_schedule_variables, and ci_pipeline_variables,
+    # same update statement is used for all tables.
+    ci_variables.create(project_id: project.id, key: 'LEGACY_VARIABLE', value: 'Content goes here')
+    ci_variables.create(project_id: project.id, key: 'FILE', value: 'Content goes here', variable_type: Ci::Variable.variable_types['file'])
+
+    migrate!
+
+    file_variable = ci_variables.find_by(key: 'FILE')
+    expect(file_variable.variable_type).to eq(Ci::Variable.variable_types['file'])
+
+    legacy_variable = ci_variables.find_by(key: 'LEGACY_VARIABLE')
+    expect(legacy_variable.variable_type).to eq(Ci::Variable.variable_types['env_var'])
+  end
+end
diff --git a/spec/support/shared_examples/models/ci_variable_shared_examples.rb b/spec/support/shared_examples/models/ci_variable_shared_examples.rb
index 57d2a69c939..cfab1727cf8 100644
--- a/spec/support/shared_examples/models/ci_variable_shared_examples.rb
+++ b/spec/support/shared_examples/models/ci_variable_shared_examples.rb
@@ -9,7 +9,11 @@ shared_examples_for 'CI variable' do
     end
 
     it "defaults variable type to env_var" do
-      expect(subject.variable_type).to eq("env_var")
+      variable = create(described_class.table_name.singularize)
+      file_variable = create(described_class.table_name.singularize, variable_type: 'file')
+
+      expect(variable.reload.variable_type_before_type_cast).to eq(1)
+      expect(file_variable.reload.variable_type_before_type_cast).to eq(2)
     end
 
     it "supports variable type file" do