diff --git a/lib/scenic/schema_dumper.rb b/lib/scenic/schema_dumper.rb index 68aab1b2..9887913b 100644 --- a/lib/scenic/schema_dumper.rb +++ b/lib/scenic/schema_dumper.rb @@ -3,6 +3,34 @@ module Scenic # @api private module SchemaDumper + # A hash to do topological sort + class TSortableHash < Hash + include TSort + + alias tsort_each_node each_key + def tsort_each_child(node, &block) + fetch(node).each(&block) + end + end + + # Query for the dependencies between views + DEPENDENT_SQL = <<~SQL.freeze + SELECT distinct dependent_ns.nspname AS dependent_schema + , dependent_view.relname AS dependent_view + , source_ns.nspname AS source_schema + , source_table.relname AS source_table + FROM pg_depend + JOIN pg_rewrite ON pg_depend.objid = pg_rewrite.oid + JOIN pg_class as dependent_view ON pg_rewrite.ev_class = dependent_view.oid + JOIN pg_class as source_table ON pg_depend.refobjid = source_table.oid + JOIN pg_namespace dependent_ns ON dependent_ns.oid = dependent_view.relnamespace + JOIN pg_namespace source_ns ON source_ns.oid = source_table.relnamespace + WHERE dependent_ns.nspname = ANY (current_schemas(false)) AND source_ns.nspname = ANY (current_schemas(false)) + AND source_table.relname != dependent_view.relname + AND source_table.relkind IN ('m', 'v') AND dependent_view.relkind IN ('m', 'v') + ORDER BY dependent_view.relname; + SQL + def tables(stream) super views(stream) @@ -22,9 +50,40 @@ def views(stream) private def dumpable_views_in_database - @dumpable_views_in_database ||= Scenic.database.views.reject do |view| + if @ordered_dumpable_views_in_database + return @ordered_dumpable_views_in_database + end + + existing_views = Scenic.database.views.reject do |view| ignored?(view.name) end + + @ordered_dumpable_views_in_database = + tsorted_views(existing_views.map(&:name)).map do |view_name| + existing_views.find { |ev| ev.name == view_name } + end.compact + end + + # When dumping the views, their order must be topologically + # sorted to take into account dependencies + def tsorted_views(views_names) + views_hash = TSortableHash.new + + ::Scenic.database.execute(DEPENDENT_SQL).each do |relation| + source_v = relation["source_table"] + dependent = relation["dependent_view"] + views_hash[dependent] ||= [] + views_hash[source_v] ||= [] + views_hash[dependent] << source_v + views_names.delete(source_v) + views_names.delete(dependent) + end + + # after dependencies, there might be some views left + # that don't have any dependencies + views_names.sort.each { |v| views_hash[v] = [] } + + views_hash.tsort end unless ActiveRecord::SchemaDumper.private_instance_methods(false).include?(:ignored?) diff --git a/spec/scenic/schema_dumper_spec.rb b/spec/scenic/schema_dumper_spec.rb index 87e683b7..fc66b68a 100644 --- a/spec/scenic/schema_dumper_spec.rb +++ b/spec/scenic/schema_dumper_spec.rb @@ -7,19 +7,21 @@ class SearchInAHaystack < ActiveRecord::Base end describe Scenic::SchemaDumper, :db do - it "dumps a create_view for a view in the database" do - view_definition = "SELECT 'needle'::text AS haystack" - Search.connection.create_view :searches, sql_definition: view_definition + let(:conn) { Search.connection } + let(:output) do stream = StringIO.new + ActiveRecord::SchemaDumper.dump(conn, stream) + stream.string + end - ActiveRecord::SchemaDumper.dump(Search.connection, stream) - - output = stream.string + it "dumps a create_view for a view in the database" do + view_definition = "SELECT 'needle'::text AS haystack" + conn.create_view :searches, sql_definition: view_definition expect(output).to include 'create_view "searches", sql_definition: <<-SQL' expect(output).to include view_definition - Search.connection.drop_view :searches + conn.drop_view :searches silence_stream(STDOUT) { eval(output) } @@ -28,12 +30,8 @@ class SearchInAHaystack < ActiveRecord::Base it "dumps a create_view for a materialized view in the database" do view_definition = "SELECT 'needle'::text AS haystack" - Search.connection.create_view :searches, materialized: true, sql_definition: view_definition - stream = StringIO.new - - ActiveRecord::SchemaDumper.dump(Search.connection, stream) - - output = stream.string + conn.create_view :searches, + materialized: true, sql_definition: view_definition expect(output).to include 'create_view "searches", materialized: true, sql_definition: <<-SQL' expect(output).to include view_definition @@ -42,27 +40,18 @@ class SearchInAHaystack < ActiveRecord::Base context "with views in non public schemas" do it "dumps a create_view including namespace for a view in the database" do view_definition = "SELECT 'needle'::text AS haystack" - Search.connection.execute "CREATE SCHEMA scenic; SET search_path TO scenic, public" - Search.connection.create_view :"scenic.searches", sql_definition: view_definition - stream = StringIO.new - - ActiveRecord::SchemaDumper.dump(Search.connection, stream) + conn.execute "CREATE SCHEMA scenic; SET search_path TO scenic, public" + conn.create_view :"scenic.searches", sql_definition: view_definition - output = stream.string expect(output).to include 'create_view "scenic.searches",' - Search.connection.drop_view :'scenic.searches' + conn.drop_view :'scenic.searches' end end it "ignores tables internal to Rails" do view_definition = "SELECT 'needle'::text AS haystack" - Search.connection.create_view :searches, sql_definition: view_definition - stream = StringIO.new - - ActiveRecord::SchemaDumper.dump(Search.connection, stream) - - output = stream.string + conn.create_view :searches, sql_definition: view_definition expect(output).to include 'create_view "searches"' expect(output).not_to include "ar_internal_metadata" @@ -72,16 +61,13 @@ class SearchInAHaystack < ActiveRecord::Base context "with views using unexpected characters in name" do it "dumps a create_view for a view in the database" do view_definition = "SELECT 'needle'::text AS haystack" - Search.connection.create_view '"search in a haystack"', sql_definition: view_definition - stream = StringIO.new - - ActiveRecord::SchemaDumper.dump(Search.connection, stream) + conn.create_view '"search in a haystack"', + sql_definition: view_definition - output = stream.string expect(output).to include 'create_view "\"search in a haystack\"",' expect(output).to include view_definition - Search.connection.drop_view :'"search in a haystack"' + conn.drop_view :'"search in a haystack"' silence_stream(STDOUT) { eval(output) } @@ -92,24 +78,134 @@ class SearchInAHaystack < ActiveRecord::Base context "with views using unexpected characters, name including namespace" do it "dumps a create_view for a view in the database" do view_definition = "SELECT 'needle'::text AS haystack" - Search.connection.execute( + conn.execute( "CREATE SCHEMA scenic; SET search_path TO scenic, public", ) - Search.connection.create_view 'scenic."search in a haystack"', + conn.create_view 'scenic."search in a haystack"', sql_definition: view_definition - stream = StringIO.new - - ActiveRecord::SchemaDumper.dump(Search.connection, stream) - output = stream.string expect(output).to include 'create_view "scenic.\"search in a haystack\"",' expect(output).to include view_definition - Search.connection.drop_view :'scenic."search in a haystack"' + conn.drop_view :'scenic."search in a haystack"' silence_stream(STDOUT) { eval(output) } expect(SearchInAHaystack.take.haystack).to eq "needle" end end + + context "with viewes ordered by name" do + let(:views) do + output.lines.grep(/create_view/).map do |view_line| + view_line.match('create_view "(?.*)"')[:name] + end + end + + context "without dependencies" do + it "sorts views without dependencies" do + conn.create_view "cucumber_needles", + sql_definition: "SELECT 'kukumbas'::text as needle" + conn.create_view "vip_needles", + sql_definition: "SELECT 'vip'::text as needle" + conn.create_view "none_needles", + sql_definition: "SELECT 'none_needles'::text as needle" + + # Same here, no dependencies among existing views, all views are sorted + sorted_views = %w[cucumber_needles vip_needles none_needles].sort + + expect(views).to eq(sorted_views) + end + end + + context "with dependencies" do + it "sorts according to dependencies" do + conn.create_table(:tasks) { |t| t.integer :performer_id } + conn.create_table(:notes) do |t| + t.text :title + t.integer :author_id + end + conn.create_table(:users) { |t| t.text :nickname } + conn.create_table(:roles) do |t| + t.text :name + t.integer :user_id + end + + conn.create_view "recent_tasks", + sql_definition: <<-SQL + SELECT id, performer_id from tasks where id > 42 + SQL + conn.create_view "old_roles", + sql_definition: <<-SQL + SELECT id, name from roles where id > 56 + SQL + conn.create_view "nirvana_notes", + sql_definition: <<-SQL + SELECT notes.id, notes.title, notes.author_id, old_roles.name FROM notes + JOIN old_roles ON notes.author_id = old_roles.id + SQL + conn.create_view "angry_zombies", + sql_definition: <<-SQL + SELECT users.nickname, recent_tasks.id as task_id, nirvana_notes.title as talk FROM users + JOIN roles ON roles.user_id = users.id + JOIN nirvana_notes ON nirvana_notes.author_id = roles.id + JOIN recent_tasks ON recent_tasks.performer_id = roles.id + SQL + conn.create_view "doctor_zombies", + sql_definition: <<-SQL + SELECT id FROM old_roles WHERE name LIKE '%Dr%' + SQL + conn.create_view "xenomorphs", + sql_definition: <<-SQL + SELECT id, name FROM roles WHERE name = 'xeno' + SQL + conn.create_view "important_messages", + sql_definition: <<-SQL + SELECT id, title FROM notes WHERE title LIKE '%NSFW%' + SQL + + # Converted with https://github.com/ggerganov/dot-to-ascii + # digraph { + # rankdir = "BT"; + # xenomorphs; + # important_messages; + # doctor_zombies -> old_roles; + # nirvana_notes -> old_roles; + # angry_zombies -> nirvana_notes; + # angry_zombies -> recent_tasks; + # } + # + # +--------------------+ + # | doctor_zombies | + # +--------------------+ + # | + # | + # v + # +--------------------+ + # | old_roles | + # +--------------------+ + # ^ + # | + # | + # +--------------------+ + # | nirvana_notes | + # +--------------------+ + # ^ + # | + # | + # +--------------------+ +--------------+ + # | angry_zombies | --> | recent_tasks | + # +--------------------+ +--------------+ + # +--------------------+ + # | important_messages | + # +--------------------+ + # +--------------------+ + # | xenomorphs | + # +--------------------+ + expect(views).to eq(%w[old_roles nirvana_notes recent_tasks + angry_zombies doctor_zombies + important_messages xenomorphs]) + end + end + end end