Skip to content

Commit

Permalink
Fix possible incorrect circular dependency error happening when verif…
Browse files Browse the repository at this point in the history
…ying the acyclic property of the migrations graph
  • Loading branch information
ellmetha committed Mar 14, 2023
1 parent 8a681ae commit 0778be7
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
19 changes: 19 additions & 0 deletions spec/marten/db/management/migrations/graph_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,25 @@ describe Marten::DB::Management::Migrations::Graph do
graph.ensure_acyclic_property.should be_nil
end

it "does not raise if there are no circular dependencies in the graph when migrations depend on multiple apps" do
app_1_migration_1 = Marten::DB::Management::Migrations::GraphSpec::TestMigration1.new
app_2_migration_1 = Marten::DB::Management::Migrations::GraphSpec::TestMigration3.new
app_2_migration_2 = Marten::DB::Management::Migrations::GraphSpec::TestMigration4.new
app_2_migration_3 = Marten::DB::Management::Migrations::GraphSpec::TestMigration5.new

graph = Marten::DB::Management::Migrations::Graph.new
graph.add_node(app_1_migration_1)
graph.add_node(app_2_migration_1)
graph.add_node(app_2_migration_2)
graph.add_node(app_2_migration_3)

graph.add_dependency(app_2_migration_2, app_2_migration_1.id)
graph.add_dependency(app_2_migration_3, app_2_migration_2.id)
graph.add_dependency(app_2_migration_3, app_1_migration_1.id)

graph.ensure_acyclic_property.should be_nil
end

it "raises if there is a circular dependency in the graph" do
migration_1 = Marten::DB::Management::Migrations::GraphSpec::TestMigration1.new
migration_2 = Marten::DB::Management::Migrations::GraphSpec::TestMigration2.new
Expand Down
29 changes: 25 additions & 4 deletions src/marten/db/management/migrations/graph.cr
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,32 @@ module Marten

# Verifies that the graph does not contain cycles.
def ensure_acyclic_property : Nil
seen = [] of String
remaining_nodes = @nodes.keys

# Perform a DFS traversal of the directed graph and verifies that no cycles are present.
@nodes.keys.each do |node_id|
acyclic_dfs_traversal(node_id, seen, forwards: false) unless seen.includes?(node_id)
while !remaining_nodes.empty?
node = remaining_nodes.pop
stack = [node]

while !stack.empty?
stack_updated = false

@nodes[stack.last].children.each do |child|
child_node_id = child.migration.id

if stack.includes?(child_node_id)
raise Errors::CircularDependency.new("Circular dependency identified up to '#{child_node_id}'")
end

if remaining_nodes.includes?(child_node_id)
stack << child_node_id
stack_updated = true
remaining_nodes.delete(child_node_id)
break
end
end

stack.pop if !stack_updated
end
end
end

Expand Down

0 comments on commit 0778be7

Please sign in to comment.