Commit 9afed290 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Prevent infinite loops in ObjectHierarchy

parent 3b591ffe
...@@ -30,6 +30,8 @@ class RemoveCyclicHierarchiesInEpics < ActiveRecord::Migration[5.0] ...@@ -30,6 +30,8 @@ class RemoveCyclicHierarchiesInEpics < ActiveRecord::Migration[5.0]
# We only need to update the first epic of the loop to break the cycle # We only need to update the first epic of the loop to break the cycle
epic_ids_to_update = epics_grouped_by_loop.map { |path, epics| epics.first['id'] } epic_ids_to_update = epics_grouped_by_loop.map { |path, epics| epics.first['id'] }
# rubocop:disable Lint/UnneededCopDisableDirective
# rubocop:disable Migration/UpdateColumnInBatches
update_column_in_batches(:epics, :parent_id, nil) do |table, query| update_column_in_batches(:epics, :parent_id, nil) do |table, query|
query.where( query.where(
table[:id].in(epic_ids_to_update) table[:id].in(epic_ids_to_update)
......
...@@ -11,7 +11,15 @@ describe RemoveCyclicHierarchiesInEpics, :migration, :postgresql do ...@@ -11,7 +11,15 @@ describe RemoveCyclicHierarchiesInEpics, :migration, :postgresql do
def create_epic_with_defaults!(attributes = {}) def create_epic_with_defaults!(attributes = {})
@last_iid += 1 @last_iid += 1
epics.create!({iid: @last_iid, group_id: group.id, author_id: user.id, title: "Epic", title_html: "Epic"}.merge(attributes)) epics.create!(
attributes.reverse_merge(
iid: @last_iid,
group_id: group.id,
author_id: user.id,
title: "Epic",
title_html: "Epic"
)
)
end end
let!(:epic_self_loop) { create_epic_with_defaults! } let!(:epic_self_loop) { create_epic_with_defaults! }
...@@ -53,5 +61,3 @@ describe RemoveCyclicHierarchiesInEpics, :migration, :postgresql do ...@@ -53,5 +61,3 @@ describe RemoveCyclicHierarchiesInEpics, :migration, :postgresql do
expect(epic_not_in_loop.reload.parent_id).not_to be_nil expect(epic_not_in_loop.reload.parent_id).not_to be_nil
end end
end end
...@@ -144,7 +144,7 @@ module Gitlab ...@@ -144,7 +144,7 @@ module Gitlab
cte = SQL::RecursiveCTE.new(:base_and_ancestors) cte = SQL::RecursiveCTE.new(:base_and_ancestors)
base_query = ancestors_base.except(:order) base_query = ancestors_base.except(:order)
base_query = base_query.select("1 as #{DEPTH_COLUMN}", objects_table[Arel.star]) if hierarchy_order base_query = base_query.select("1 as #{DEPTH_COLUMN}", "ARRAY[id] AS tree_path", "false AS tree_cycle", objects_table[Arel.star]) if hierarchy_order
cte << base_query cte << base_query
...@@ -154,7 +154,17 @@ module Gitlab ...@@ -154,7 +154,17 @@ module Gitlab
.where(objects_table[:id].eq(cte.table[:parent_id])) .where(objects_table[:id].eq(cte.table[:parent_id]))
.except(:order) .except(:order)
parent_query = parent_query.select(cte.table[DEPTH_COLUMN] + 1, objects_table[Arel.star]) if hierarchy_order if hierarchy_order
quoted_objects_table_name = model.connection.quote_table_name(objects_table.name)
parent_query = parent_query.select(
cte.table[DEPTH_COLUMN] + 1,
"tree_path || #{quoted_objects_table_name}.id",
"#{quoted_objects_table_name}.id = ANY(tree_path)",
objects_table[Arel.star]
).where(cte.table[:tree_cycle].eq(false))
end
parent_query = parent_query.where(cte.table[:parent_id].not_eq(stop_id)) if stop_id parent_query = parent_query.where(cte.table[:parent_id].not_eq(stop_id)) if stop_id
cte << parent_query cte << parent_query
...@@ -167,7 +177,7 @@ module Gitlab ...@@ -167,7 +177,7 @@ module Gitlab
cte = SQL::RecursiveCTE.new(:base_and_descendants) cte = SQL::RecursiveCTE.new(:base_and_descendants)
base_query = descendants_base.except(:order) base_query = descendants_base.except(:order)
base_query = base_query.select("1 as #{DEPTH_COLUMN}", objects_table[Arel.star]) if with_depth base_query = base_query.select("1 AS #{DEPTH_COLUMN}", "ARRAY[id] AS tree_path", "false AS tree_cycle", objects_table[Arel.star]) if with_depth
cte << base_query cte << base_query
...@@ -177,7 +187,16 @@ module Gitlab ...@@ -177,7 +187,16 @@ module Gitlab
.where(objects_table[:parent_id].eq(cte.table[:id])) .where(objects_table[:parent_id].eq(cte.table[:id]))
.except(:order) .except(:order)
descendants_query = descendants_query.select(cte.table[DEPTH_COLUMN] + 1, objects_table[Arel.star]) if with_depth if with_depth
quoted_objects_table_name = model.connection.quote_table_name(objects_table.name)
descendants_query = descendants_query.select(
cte.table[DEPTH_COLUMN] + 1,
"tree_path || #{quoted_objects_table_name}.id",
"#{quoted_objects_table_name}.id = ANY(tree_path)",
objects_table[Arel.star]
).where(cte.table[:tree_cycle].eq(false))
end
cte << descendants_query cte << descendants_query
cte cte
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment