Use a single sql statement for ADO query

Since cluster_projects table does not have a lot of records,
currently it has 11,638, it seems better to use a single sql statement
to update all the records

2 files changed, 32 insertions, 55 deletions

diff --git a/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb b/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb
index 2d3e9acaa62..392e64eeade 100644
--- a/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb
+++ b/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb
@@ -6,12 +6,7 @@ class MigrateAutoDevOpsDomainToClusterDomain < ActiveRecord::Migration[5.0]
   DOWNTIME = false
 
   def up
-    domains_info = connection.exec_query(project_auto_devops_query).rows
-    domains_info.each_slice(1_000) do |batch|
-      update_clusters_query = build_clusters_query(Hash[*batch.flatten])
-
-      connection.exec_query(update_clusters_query)
-    end
+    execute(update_clusters_domain_query)
   end
 
   def down
@@ -20,59 +15,35 @@ class MigrateAutoDevOpsDomainToClusterDomain < ActiveRecord::Migration[5.0]
 
   private
 
-  def project_auto_devops_table
-    @project_auto_devops_table ||= ProjectAutoDevops.arel_table
-  end
-
-  def cluster_projects_table
-    @cluster_projects_table ||= Clusters::Project.arel_table
+  def update_clusters_domain_query
+    if Gitlab::Database.mysql?
+      mysql_query
+    else
+      postgresql_query
+    end
   end
 
-  # Fetches ProjectAutoDevops records with:
-  # - A domain set
-  # - With a Clusters::Project related to Project
-  #
-  # Returns an array of arrays like:
-  # => [
-  #     [177, "104.198.38.135.nip.io"],
-  #     [178, "35.232.213.111.nip.io"],
-  #     ...
-  #    ]
-  # Where the first element is the Cluster ID and
-  # the second element is the domain.
-  def project_auto_devops_query
-    project_auto_devops_table.join(cluster_projects_table, Arel::Nodes::OuterJoin)
-      .on(project_auto_devops_table[:project_id].eq(cluster_projects_table[:project_id]))
-      .where(project_auto_devops_table[:domain].not_eq(nil).and(project_auto_devops_table[:domain].not_eq('')))
-      .project(cluster_projects_table[:cluster_id], project_auto_devops_table[:domain])
-      .to_sql
+  def mysql_query
+    <<~HEREDOC
+    UPDATE clusters, project_auto_devops, cluster_projects
+    SET
+      clusters.domain = project_auto_devops.domain
+    WHERE
+      cluster_projects.cluster_id = clusters.id
+      AND project_auto_devops.project_id = cluster_projects.project_id
+      AND project_auto_devops.domain != ''
+    HEREDOC
   end
 
-  # Returns an SQL UPDATE query using a CASE statement
-  # to update multiple cluster rows with different values.
-  #
-  # Example:
-  # UPDATE clusters
-  # SET domain = (CASE
-  #   WHEN id = 177 then '104.198.38.135.nip.io'
-  #   WHEN id = 178 then '35.232.213.111.nip.io'
-  #   WHEN id = 179 then '35.232.168.149.nip.io'
-  #   WHEN id = 180 then '35.224.116.88.nip.io'
-  # END)
-  # WHERE id IN (177,178,179,180);
-  def build_clusters_query(cluster_domains_info)
+  def postgresql_query
     <<~HEREDOC
     UPDATE clusters
-      SET domain = (CASE
-        #{cluster_when_statements(cluster_domains_info)}
-      END)
-    WHERE id IN (#{cluster_domains_info.keys.join(",")});
+    SET domain = project_auto_devops.domain
+    FROM cluster_projects, project_auto_devops
+    WHERE
+      cluster_projects.cluster_id = clusters.id
+      AND project_auto_devops.project_id = cluster_projects.project_id
+      AND project_auto_devops.domain != ''
     HEREDOC
   end
-
-  def cluster_when_statements(cluster_domains_info)
-    cluster_domains_info.map do |cluster_id, domain|
-      "WHEN id = #{cluster_id} then '#{domain}'"
-    end.join("\n")
-  end
 end
diff --git a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb
index 09013ee4bd0..c7fd27588a5 100644
--- a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb
+++ b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb
@@ -46,12 +46,14 @@ describe MigrateAutoDevOpsDomainToClusterDomain, :migration do
     let(:domain) { 'example-domain.com' }
 
     before do
-      setup_cluster_projects_with_domain(quantity: 20, domain: nil)
+      setup_cluster_projects_with_domain(quantity: 25, domain: nil)
     end
 
     it 'should only update specific cluster projects' do
       migrate!
 
+      expect(clusters_with_domain.count).to eq(20)
+
       project_auto_devops_with_domain.each do |project_auto_devops|
         cluster_project = Clusters::Project.find_by(project_id: project_auto_devops.project_id)
         cluster = Clusters::Cluster.find(cluster_project.cluster_id)
@@ -59,6 +61,8 @@ describe MigrateAutoDevOpsDomainToClusterDomain, :migration do
         expect(cluster.domain).to be_present
       end
 
+      expect(clusters_without_domain.count).to eq(25)
+
       project_auto_devops_without_domain.each do |project_auto_devops|
         cluster_project = Clusters::Project.find_by(project_id: project_auto_devops.project_id)
         cluster = Clusters::Cluster.find(cluster_project.cluster_id)
@@ -74,10 +78,12 @@ describe MigrateAutoDevOpsDomainToClusterDomain, :migration do
     cluster_projects = cluster_projects_table.last(quantity)
 
     cluster_projects.each do |cluster_project|
+      specific_domain = "#{cluster_project.id}-#{domain}" if domain
+
       project_auto_devops_table.create(
         project_id: cluster_project.project_id,
         enabled: true,
-        domain: domain
+        domain: specific_domain
       )
     end
   end