From 4ca42b9618ef138b25aa839642cd02beca28a1bb Mon Sep 17 00:00:00 2001 From: Simon Claessens Date: Tue, 26 Jan 2021 15:06:19 +0100 Subject: [PATCH] manage version when rename view --- lib/scenic/adapters/postgres.rb | 30 ++++++++++- lib/scenic/command_recorder.rb | 11 ++-- .../command_recorder/statement_arguments.rb | 29 ++++++---- lib/scenic/definition.rb | 2 + lib/scenic/errors.rb | 10 ++++ lib/scenic/statements.rb | 51 ++++++++++++------ spec/scenic/adapters/postgres_spec.rb | 53 +++++++++++++++++++ .../statement_arguments_spec.rb | 4 +- spec/scenic/command_recorder_spec.rb | 33 ++++++++---- spec/scenic/statements_spec.rb | 14 +++-- 10 files changed, 187 insertions(+), 50 deletions(-) create mode 100644 lib/scenic/errors.rb diff --git a/lib/scenic/adapters/postgres.rb b/lib/scenic/adapters/postgres.rb index e45ab2b0..a0acbebc 100644 --- a/lib/scenic/adapters/postgres.rb +++ b/lib/scenic/adapters/postgres.rb @@ -265,10 +265,24 @@ def refresh_materialized_view(name, concurrently: false, cascade: false) end end + # Compare the SQL definition of the view stored in the database + # with the definition used to create the migrations. + # + # @param definition [Scenic::Definition] view definition to compare + # with the database. + # + # @return [Boolean] + def view_with_similar_definition?(definition) + decompiled_view_sql(definition.name) == decompiled_sql(definition.to_sql) + end + private attr_reader :connectable - delegate :execute, :quote_table_name, :rename_index, to: :connection + delegate( + :execute, :quote_table_name, :rename_index, :select_value, :transaction, + to: :connection + ) def connection Connection.new(connectable.connection) @@ -294,6 +308,20 @@ def refresh_dependencies_for(name, concurrently: false) concurrently: concurrently, ) end + + def decompiled_sql(sql_definition) + temporary_view_name = "temp_view_for_decompilation" + view_name = quote_view_name(temporary_view_name) + transaction do + execute "CREATE TEMPORARY VIEW #{view_name} AS #{sql_definition};" + decompiled_view_sql(temporary_view_name) + end + end + + def decompiled_view_sql(name) + view_name = quote_table_name(name) + select_value "SELECT pg_get_viewdef(to_regclass(#{view_name}))" + end end end end diff --git a/lib/scenic/command_recorder.rb b/lib/scenic/command_recorder.rb index ce41573d..980bc151 100644 --- a/lib/scenic/command_recorder.rb +++ b/lib/scenic/command_recorder.rb @@ -28,13 +28,10 @@ def invert_replace_view(args) end def invert_rename_view(args) - options = args.extract_options! - old_name, new_name = args - - args = [new_name, old_name] - args << options unless options.empty? - - [:rename_view, args] + perform_scenic_inversion( + :rename_view, + StatementArguments.new(args).invert_names.to_a, + ) end private diff --git a/lib/scenic/command_recorder/statement_arguments.rb b/lib/scenic/command_recorder/statement_arguments.rb index 5b128267..7465ffe5 100644 --- a/lib/scenic/command_recorder/statement_arguments.rb +++ b/lib/scenic/command_recorder/statement_arguments.rb @@ -3,11 +3,12 @@ module CommandRecorder # @api private class StatementArguments def initialize(args) - @args = args.freeze + @args = args.clone + @options = @args.extract_options! end def view - @args[0] + args[0] end def version @@ -19,27 +20,37 @@ def revert_to_version end def invert_version - StatementArguments.new([view, options_for_revert]) + StatementArguments.new([*args, options_for_revert]) end def remove_version - StatementArguments.new([view, options_without_version]) + StatementArguments.new([*args, options_without_version]) + end + + def invert_names + StatementArguments.new([*args.reverse, options]) end def to_a - @args.to_a.dup.delete_if(&:empty?) + args.to_a.dup.delete_if(&:empty?).tap do |array| + array << options if options.present? + end end private - def options - @options ||= @args[1] || {} - end + attr_reader :args, :options def options_for_revert options.clone.tap do |revert_options| - revert_options[:version] = revert_to_version + revert_options.delete(:version) revert_options.delete(:revert_to_version) + if revert_to_version.present? + revert_options[:version] = revert_to_version + end + if version.present? + revert_options[:revert_to_version] = version + end end end diff --git a/lib/scenic/definition.rb b/lib/scenic/definition.rb index dac137cd..1c200771 100644 --- a/lib/scenic/definition.rb +++ b/lib/scenic/definition.rb @@ -1,6 +1,8 @@ module Scenic # @api private class Definition + attr_reader :name + def initialize(name, version) @name = name @version = version.to_i diff --git a/lib/scenic/errors.rb b/lib/scenic/errors.rb new file mode 100644 index 00000000..31fefc43 --- /dev/null +++ b/lib/scenic/errors.rb @@ -0,0 +1,10 @@ +module Scenic + # Raised when a view definition in the database is different + # from the definition used to create migrations. + class StoredDefinitionError < StandardError + def initialize + path = Scenic.configuration.definitions_path + super("View definition in the database different from in the #{path}") + end + end +end diff --git a/lib/scenic/statements.rb b/lib/scenic/statements.rb index 3d9c1177..717ebac1 100644 --- a/lib/scenic/statements.rb +++ b/lib/scenic/statements.rb @@ -34,16 +34,16 @@ def create_view(name, version: nil, sql_definition: nil, materialized: false) version = 1 end - sql_definition ||= definition(name, version) + sql_definition ||= definition(name, version).to_sql if materialized - Scenic.database.create_materialized_view( + database.create_materialized_view( name, sql_definition, no_data: no_data(materialized), ) else - Scenic.database.create_view(name, sql_definition) + database.create_view(name, sql_definition) end end @@ -62,9 +62,9 @@ def create_view(name, version: nil, sql_definition: nil, materialized: false) # def drop_view(name, revert_to_version: nil, materialized: false) if materialized - Scenic.database.drop_materialized_view(name) + database.drop_materialized_view(name) else - Scenic.database.drop_view(name) + database.drop_view(name) end end @@ -103,16 +103,16 @@ def update_view(name, version: nil, sql_definition: nil, revert_to_version: nil, ) end - sql_definition ||= definition(name, version) + sql_definition ||= definition(name, version).to_sql if materialized - Scenic.database.update_materialized_view( + database.update_materialized_view( name, sql_definition, no_data: no_data(materialized), ) else - Scenic.database.update_view(name, sql_definition) + database.update_view(name, sql_definition) end end @@ -141,15 +141,18 @@ def replace_view(name, version: nil, revert_to_version: nil, materialized: false raise ArgumentError, "Cannot replace materialized views" end - sql_definition = definition(name, version) + sql_definition = definition(name, version).to_sql - Scenic.database.replace_view(name, sql_definition) + database.replace_view(name, sql_definition) end # Rename a database view by name. # # @param from_name [String, Symbol] The previous name of the database view. - # @param from_name [String, Symbol] The next name of the database view. + # @param to_name [String, Symbol] The next name of the database view. + # @param version [Fixnum] The version number of the view `to_name`. + # @param revert_to_version [Fixnum] The version number + # of the view `from_name` to rollback to on `rake db rollback` # @param materialized [Boolean, Hash] True if updating a materialized view. # Set to { rename_indexes: true } to rename materialized view indexes # by substituing in their name the previous view name @@ -159,22 +162,40 @@ def replace_view(name, version: nil, revert_to_version: nil, materialized: false # @example Rename a view # drop_view(:engggggement_reports, :engagement_reports) # - def rename_view(from_name, to_name, materialized: false) + def rename_view( + from_name, to_name, + version: nil, revert_to_version: nil, materialized: false + ) + if version.blank? + raise ArgumentError, "version is required" + end + + if revert_to_version.blank? + raise ArgumentError, "revert_to_version is required" + end + if materialized - Scenic.database.rename_materialized_view( + database.rename_materialized_view( from_name, to_name, rename_indexes: rename_indexes(materialized), ) else - Scenic.database.rename_view(from_name, to_name) + database.rename_view(from_name, to_name) end + + similar_definition = database.view_with_similar_definition?( + definition(to_name, version) + ) + raise StoredDefinitionError unless similar_definition end private + delegate :database, to: :Scenic + def definition(name, version) - Scenic::Definition.new(name, version).to_sql + Scenic::Definition.new(name, version) end def no_data(materialized) diff --git a/spec/scenic/adapters/postgres_spec.rb b/spec/scenic/adapters/postgres_spec.rb index dab7ff2d..4d0aab2f 100644 --- a/spec/scenic/adapters/postgres_spec.rb +++ b/spec/scenic/adapters/postgres_spec.rb @@ -261,6 +261,59 @@ module Adapters end end end + + describe "#view_with_similar_definition?" do + context "when the view has a similar definition" do + it "returns true" do + adapter = Postgres.new + ActiveRecord::Base.connection.execute <<~SQL + CREATE VIEW greetings AS SELECT text 'hi' AS greeting + SQL + + sql_defintion = <<~SQL + SELECT text 'hi' + AS greeting + SQL + definition = instance_double( + "Scenic::Definition", + name: "greeting", to_sql: sql_defintion + ) + + expect( + adapter.view_with_similar_definition?(definition) + ).to be_true + end + end + context "when the view doesn't exists on the database" do + it "returns false" do + adapter = Postgres.new + + definition = instance_double( + "Scenic::Definition", + name: "greeting", to_sql: "SELECT text 'hi' AS greeting" + ) + + expect( + adapter.view_with_similar_definition?(definition) + ).to be_false + end + end + context "when the view has a different definition" do + it "returns false" do + adapter = Postgres.new + ActiveRecord::Base.connection.execute <<~SQL + CREATE VIEW greetings AS SELECT text 'hi' AS hello + SQL + + definition = instance_double( + "Scenic::Definition", + name: "greeting", to_sql: "SELECT text 'hi' AS greeting" + ) + + expect(adapter.view_with_similar_definition?(definition)).to be_false + end + end + end end end end diff --git a/spec/scenic/command_recorder/statement_arguments_spec.rb b/spec/scenic/command_recorder/statement_arguments_spec.rb index ae21299b..4b46c5b3 100644 --- a/spec/scenic/command_recorder/statement_arguments_spec.rb +++ b/spec/scenic/command_recorder/statement_arguments_spec.rb @@ -28,13 +28,13 @@ module Scenic::CommandRecorder end describe "#invert_version" do - it "returns object with version set to revert_to_version" do + it "returns object with version interverted with revert_to_version" do raw_args = [:meatballs, { version: 42, revert_to_version: 15 }] inverted_args = StatementArguments.new(raw_args).invert_version expect(inverted_args.version).to eq 15 - expect(inverted_args.revert_to_version).to be nil + expect(inverted_args.revert_to_version).to be 42 end end end diff --git a/spec/scenic/command_recorder_spec.rb b/spec/scenic/command_recorder_spec.rb index dbf1fbb3..e3efa664 100644 --- a/spec/scenic/command_recorder_spec.rb +++ b/spec/scenic/command_recorder_spec.rb @@ -64,7 +64,7 @@ it "reverts to update_view with the specified revert_to_version" do args = [:users, { version: 2, revert_to_version: 1 }] - revert_args = [:users, { version: 1 }] + revert_args = [:users, { version: 1, revert_to_version: 2 }] recorder.revert { recorder.update_view(*args) } @@ -90,7 +90,7 @@ it "reverts to replace_view with the specified revert_to_version" do args = [:users, { version: 2, revert_to_version: 1 }] - revert_args = [:users, { version: 1 }] + revert_args = [:users, { version: 1, revert_to_version: 2 }] recorder.revert { recorder.replace_view(*args) } @@ -107,23 +107,34 @@ describe "#rename_view" do it "records the created view" do - recorder.rename_view :from, :to + recorder.rename_view :from, :to, version: 1, revert_to_version: 2 - expect(recorder.commands).to eq [[:rename_view, [:from, :to], nil]] + expect(recorder.commands).to eq [[ + :rename_view, [:from, :to, version: 1, revert_to_version: 2], nil + ]] end it "reverts to drop_view when not passed a version" do - recorder.revert { recorder.rename_view :from, :to } + recorder.revert do + recorder.rename_view :from, :to, version: 1, revert_to_version: 2 + end - expect(recorder.commands).to eq [[:rename_view, [:to, :from]]] + expect(recorder.commands).to eq [[ + :rename_view, [:to, :from, version: 2, revert_to_version: 1] + ]] end it "reverts materialized views appropriately" do - recorder.revert { recorder.rename_view :from, :to, materialized: true } - - expect(recorder.commands).to eq [ - [:rename_view, [:to, :from, materialized: true]], - ] + recorder.revert do + recorder.rename_view( + :from, :to, version: 1, revert_to_version: 2, materialized: true + ) + end + + expect(recorder.commands).to eq [[ + :rename_view, + [:to, :from, version: 2, revert_to_version: 1, materialized: true], + ]] end end diff --git a/spec/scenic/statements_spec.rb b/spec/scenic/statements_spec.rb index fd22ae2c..f5105d9c 100644 --- a/spec/scenic/statements_spec.rb +++ b/spec/scenic/statements_spec.rb @@ -194,14 +194,17 @@ module Scenic describe "rename_view" do it "rename the view in the database" do - connection.rename_view(:from, :to) + connection.rename_view(:from, :to, version: 1, revert_to_version: 1) expect(Scenic.database).to have_received(:rename_view) .with(:from, :to) end it "rename the materialized view in the database" do - connection.rename_view(:from, :to, materialized: true) + connection.rename_view( + :from, :to, + version: 1, revert_to_version: 1, materialized: true + ) expect(Scenic.database).to have_received(:rename_materialized_view) .with(:from, :to, rename_indexes: false) @@ -209,9 +212,10 @@ module Scenic it "rename the materialized view in the database and rename indexes" do connection.rename_view( - :from, - :to, - materialized: { rename_indexes: true }, + :from, :to, + version: 1, + revert_to_version: 1, + materialized: { rename_indexes: true } ) expect(Scenic.database).to have_received(:rename_materialized_view)