Skip to content

Commit

Permalink
Fix rubocop offenses
Browse files Browse the repository at this point in the history
  • Loading branch information
serg-kovalev committed Feb 8, 2023
1 parent 32b0327 commit e145024
Show file tree
Hide file tree
Showing 14 changed files with 86 additions and 64 deletions.
8 changes: 4 additions & 4 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
AllCops:
TargetRubyVersion: 2.3.0
TargetRubyVersion: 2.7.0
Exclude:
- "tmp/**/*"
- "bin/*"
Expand All @@ -11,7 +11,7 @@ Bundler/OrderedGems:
Gemspec/OrderedDependencies:
Enabled: false

Layout/AlignParameters:
Layout/ParameterAlignment:
Enabled: true
EnforcedStyle: with_fixed_indentation
Layout/ConditionPosition:
Expand All @@ -20,7 +20,7 @@ Layout/DotPosition:
EnforcedStyle: leading
Layout/ExtraSpacing:
Enabled: true
Layout/IndentAssignment:
Layout/AssignmentIndentation:
Enabled: False
Layout/MultilineOperationIndentation:
Enabled: true
Expand All @@ -33,7 +33,7 @@ Lint/AmbiguousOperator:
Enabled: true
Lint/AmbiguousRegexpLiteral:
Enabled: true
Lint/DuplicatedKey:
Lint/DuplicateHashKey:
Enabled: true

Metrics/ClassLength:
Expand Down
25 changes: 13 additions & 12 deletions lib/generators/scenic/materializable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,21 @@ module Materializable

included do
class_option :materialized,
type: :boolean,
required: false,
desc: "Makes the view materialized",
default: false
type: :boolean,
required: false,
desc: "Makes the view materialized",
default: false
class_option :no_data,
type: :boolean,
required: false,
desc: "Adds WITH NO DATA when materialized view creates/updates",
default: false
type: :boolean,
required: false,
desc: "Adds WITH NO DATA when materialized view " \
"creates/updates",
default: false
class_option :replace,
type: :boolean,
required: false,
desc: "Uses replace_view instead of update_view",
default: false
type: :boolean,
required: false,
desc: "Uses replace_view instead of update_view",
default: false
end

private
Expand Down
30 changes: 12 additions & 18 deletions lib/generators/scenic/model/model_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,17 @@ class ModelGenerator < Rails::Generators::NamedBase

def invoke_rails_model_generator
invoke "model",
[file_path.singularize],
options.merge(
fixture_replacement: false,
migration: false,
)
[file_path.singularize],
options.merge(
fixture_replacement: false,
migration: false,
)
end

def inject_model_methods
if materialized? && generating?
inject_into_class "app/models/#{file_path.singularize}.rb", class_name do
inject_into_class "app/models/#{file_path.singularize}.rb",
class_name do
evaluate_template("model.erb")
end
end
Expand All @@ -38,19 +39,12 @@ def evaluate_template(source)
context = instance_eval("binding", __FILE__, __LINE__)

if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+
erb = ERB.new(
::File.binread(source),
trim_mode: "-",
eoutvar: "@output_buffer",
)
else
erb = ERB.new(
::File.binread(source),
nil,
"-",
"@output_buffer",
)
end
erb = ERB.new(
::File.binread(source),
trim_mode: "-",
eoutvar: "@output_buffer",
)

erb.result(context)
end
Expand Down
9 changes: 6 additions & 3 deletions lib/scenic/adapters/postgres.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ def views
#
# @return [void]
def create_view(name, sql_definition, if_not_exists: false)
if if_not_exists
return if views.any? { |view| view.name == name }
if if_not_exists && views.any? { |view| view.name == name }
return
end

execute "CREATE VIEW #{quote_table_name(name)} AS #{sql_definition};"
end

Expand Down Expand Up @@ -142,7 +143,8 @@ def drop_view(name, if_exists: false)
# in use does not support materialized views.
#
# @return [void]
def create_materialized_view(name, sql_definition, no_data: false, if_not_exists: false)
def create_materialized_view(name, sql_definition, no_data: false,
if_not_exists: false)
raise_unless_materialized_views_supported

definition_if_not_exists = if_not_exists ? "IF NOT EXISTS " : ""
Expand Down Expand Up @@ -239,6 +241,7 @@ def refresh_materialized_view(name, concurrently: false, cascade: false)
private

attr_reader :connectable

delegate :execute, :quote_table_name, to: :connection

def connection
Expand Down
2 changes: 1 addition & 1 deletion lib/scenic/adapters/postgres/index_reapplication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def with_savepoint(name)
yield
connection.execute("RELEASE SAVEPOINT #{name}")
true
rescue
rescue StandardError
connection.execute("ROLLBACK TO SAVEPOINT #{name}")
false
end
Expand Down
1 change: 1 addition & 0 deletions lib/scenic/adapters/postgres/indexes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def on(name)
private

attr_reader :connection

delegate :quote_table_name, to: :connection

def indexes_on(name)
Expand Down
7 changes: 5 additions & 2 deletions lib/scenic/schema_dumper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@ def dumpable_views_in_database
end
end

unless ActiveRecord::SchemaDumper.private_instance_methods(false).include?(:ignored?)
unless ActiveRecord::SchemaDumper.private_instance_methods(false)
.include?(:ignored?)
# This method will be present in Rails 4.2.0 and can be removed then.
def ignored?(table_name)
["schema_migrations", ignore_tables].flatten.any? do |ignored|
case ignored
when String then remove_prefix_and_suffix(table_name) == ignored
when Regexp then remove_prefix_and_suffix(table_name) =~ ignored
else
raise StandardError, "ActiveRecord::SchemaDumper.ignore_tables accepts an array of String and / or Regexp values."
raise StandardError,
"ActiveRecord::SchemaDumper.ignore_tables accepts an array " \
"of String and / or Regexp values."
end
end
end
Expand Down
15 changes: 10 additions & 5 deletions lib/scenic/statements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ module Statements
# SELECT * FROM users WHERE users.active = 't'
# SQL
#
def create_view(name, version: nil, sql_definition: nil, materialized: false, if_not_exists: false)
def create_view(name, version: nil, sql_definition: nil,
materialized: false, if_not_exists: false)
if version.present? && sql_definition.present?
raise(
ArgumentError,
Expand All @@ -46,7 +47,8 @@ def create_view(name, version: nil, sql_definition: nil, materialized: false, if
if_not_exists: if_not_exists,
)
else
Scenic.database.create_view(name, sql_definition, if_not_exists: if_not_exists)
Scenic.database.create_view(name, sql_definition,
if_not_exists: if_not_exists)
end
end

Expand All @@ -65,7 +67,8 @@ def create_view(name, version: nil, sql_definition: nil, materialized: false, if
# @example Drop a view, rolling back to version 3 on rollback
# drop_view(:users_who_recently_logged_in, revert_to_version: 3)
#
def drop_view(name, revert_to_version: nil, materialized: false, if_exists: false)
def drop_view(name, revert_to_version: nil, materialized: false,
if_exists: false)
if materialized
Scenic.database.drop_materialized_view(name, if_exists: if_exists)
else
Expand Down Expand Up @@ -93,7 +96,8 @@ def drop_view(name, revert_to_version: nil, materialized: false, if_exists: fals
# @example
# update_view :engagement_reports, version: 3, revert_to_version: 2
#
def update_view(name, version: nil, sql_definition: nil, revert_to_version: nil, materialized: false)
def update_view(name, version: nil, sql_definition: nil,
revert_to_version: nil, materialized: false)
if version.blank? && sql_definition.blank?
raise(
ArgumentError,
Expand Down Expand Up @@ -137,7 +141,8 @@ def update_view(name, version: nil, sql_definition: nil, revert_to_version: nil,
# @example
# replace_view :engagement_reports, version: 3, revert_to_version: 2
#
def replace_view(name, version: nil, revert_to_version: nil, materialized: false)
def replace_view(name, version: nil, revert_to_version: nil,
materialized: false)
if version.blank?
raise ArgumentError, "version is required"
end
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/revert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def migration_class
end

def run_migration(migration, directions)
silence_stream(STDOUT) do
silence_stream($stdout) do
Array.wrap(directions).each do |direction|
migration.migrate(direction)
end
Expand Down
17 changes: 13 additions & 4 deletions spec/scenic/adapters/postgres_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ module Adapters
it "successfully creates a view with :if_not_exists if view does not exist" do
adapter = Postgres.new

adapter.create_view("greetings", "SELECT text 'hi' AS greeting", if_not_exists: true)
adapter.create_view("greetings", "SELECT text 'hi' AS greeting",
if_not_exists: true)

expect(adapter.views.map(&:name)).to include("greetings")
end
Expand All @@ -24,7 +25,10 @@ module Adapters
adapter = Postgres.new

adapter.create_view("greetings", "SELECT text 'hi' AS greeting")
expect { adapter.create_view("greetings", "SELECT text 'hi' AS greeting", if_not_exists: true) }
expect {
adapter.create_view("greetings", "SELECT text 'hi' AS greeting",
if_not_exists: true)
}
.not_to raise_error
end
end
Expand Down Expand Up @@ -106,7 +110,9 @@ module Adapters
it "does not raise error with :if_exists if view does not exist" do
adapter = Postgres.new

expect { adapter.drop_view("greetings", if_exists: true) }.not_to raise_error
expect {
adapter.drop_view("greetings", if_exists: true)
}.not_to raise_error
end
end

Expand Down Expand Up @@ -138,7 +144,10 @@ module Adapters
it "does not raise error with :if_exists if view does not exist" do
adapter = Postgres.new

expect { adapter.drop_materialized_view("greetings", if_exists: true) }.not_to raise_error
expect {
adapter.drop_materialized_view("greetings",
if_exists: true)
}.not_to raise_error
end

it "raises an exception if the version of PostgreSQL is too old" do
Expand Down
2 changes: 1 addition & 1 deletion spec/scenic/command_recorder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
recorder.revert { recorder.create_view :greetings, materialized: true }

expect(recorder.commands).to eq [
[:drop_view, [:greetings, materialized: true]],
[:drop_view, [:greetings, { materialized: true }]],
]
end
end
Expand Down
22 changes: 13 additions & 9 deletions spec/scenic/schema_dumper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class SearchInAHaystack < ActiveRecord::Base

Search.connection.drop_view :searches

silence_stream(STDOUT) { eval(output) }
silence_stream($stdout) { eval(output) }

expect(Search.first.haystack).to eq "needle"
end
Expand All @@ -37,14 +37,15 @@ class SearchInAHaystack < ActiveRecord::Base
expect(output).to include "~ '\\\\d+'::text"

Search.connection.drop_view :searches
silence_stream(STDOUT) { eval(output) }
silence_stream($stdout) { eval(output) }

expect(Search.first.haystack).to eq "needle"
end

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
Search.connection.create_view :searches, materialized: true,
sql_definition: view_definition
stream = StringIO.new

ActiveRecord::SchemaDumper.dump(Search.connection, stream)
Expand All @@ -59,7 +60,8 @@ class SearchInAHaystack < ActiveRecord::Base
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
Search.connection.create_view :"scenic.searches",
sql_definition: view_definition
stream = StringIO.new

ActiveRecord::SchemaDumper.dump(Search.connection, stream)
Expand All @@ -74,7 +76,8 @@ class SearchInAHaystack < ActiveRecord::Base
it "handles active record table name prefixes and suffixes" do
with_affixed_tables(prefix: "a_", suffix: "_z") do
view_definition = "SELECT 'needle'::text AS haystack"
Search.connection.create_view :a_searches_z, sql_definition: view_definition
Search.connection.create_view :a_searches_z,
sql_definition: view_definition
stream = StringIO.new

ActiveRecord::SchemaDumper.dump(Search.connection, stream)
Expand Down Expand Up @@ -102,7 +105,8 @@ 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
Search.connection.create_view '"search in a haystack"',
sql_definition: view_definition
stream = StringIO.new

ActiveRecord::SchemaDumper.dump(Search.connection, stream)
Expand All @@ -113,7 +117,7 @@ class SearchInAHaystack < ActiveRecord::Base

Search.connection.drop_view :'"search in a haystack"'

silence_stream(STDOUT) { eval(output) }
silence_stream($stdout) { eval(output) }

expect(SearchInAHaystack.take.haystack).to eq "needle"
end
Expand All @@ -126,7 +130,7 @@ class SearchInAHaystack < ActiveRecord::Base
"CREATE SCHEMA scenic; SET search_path TO scenic, public",
)
Search.connection.create_view 'scenic."search in a haystack"',
sql_definition: view_definition
sql_definition: view_definition
stream = StringIO.new

ActiveRecord::SchemaDumper.dump(Search.connection, stream)
Expand All @@ -137,7 +141,7 @@ class SearchInAHaystack < ActiveRecord::Base

Search.connection.drop_view :'scenic."search in a haystack"'

silence_stream(STDOUT) { eval(output) }
silence_stream($stdout) { eval(output) }

expect(SearchInAHaystack.take.haystack).to eq "needle"
end
Expand Down
Loading

0 comments on commit e145024

Please sign in to comment.