From 178705ee28b8ccf03b55cce531d2c362f0620e83 Mon Sep 17 00:00:00 2001 From: Takuma Ishikawa Date: Wed, 28 Feb 2024 14:52:45 +0900 Subject: [PATCH 1/3] bundle update rubocop --- Gemfile.lock | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 19c566d..5d6f0c7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -174,6 +174,7 @@ GEM activerecord kaminari-core (= 1.2.1) kaminari-core (1.2.1) + language_server-protocol (3.17.0.3) launchy (2.5.0) addressable (~> 2.7) loofah (2.10.0) @@ -218,9 +219,10 @@ GEM actionpack (>= 4.2) omniauth (~> 2.0) os (1.1.1) - parallel (1.20.1) - parser (3.0.2.0) + parallel (1.24.0) + parser (3.3.0.5) ast (~> 2.4.1) + racc pg (1.2.3) presto-client (0.6.5) faraday (~> 0.12) @@ -277,13 +279,13 @@ GEM method_source rake (>= 0.13) thor (~> 1.0) - rainbow (3.0.0) + rainbow (3.1.1) rake (13.0.6) ransack (2.4.2) activerecord (>= 5.2.4) activesupport (>= 5.2.4) i18n - regexp_parser (2.1.1) + regexp_parser (2.9.0) representable (3.1.1) declarative (< 0.1.0) trailblazer-option (>= 0.1.1, < 0.2.0) @@ -291,7 +293,7 @@ GEM retriable (3.1.2) revision_plate (0.1.2) rack - rexml (3.2.5) + rexml (3.2.6) ridgepole (0.9.5) activerecord (>= 5.1, < 6.2) diffy @@ -314,19 +316,21 @@ GEM rspec-mocks (~> 3.10) rspec-support (~> 3.10) rspec-support (3.10.2) - rubocop (1.18.3) + rubocop (1.60.2) + json (~> 2.3) + language_server-protocol (>= 3.17.0) parallel (~> 1.10) - parser (>= 3.0.0.0) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) - rexml - rubocop-ast (>= 1.7.0, < 2.0) + rexml (>= 3.2.5, < 4.0) + rubocop-ast (>= 1.30.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.8.0) - parser (>= 3.0.1.1) + unicode-display_width (>= 2.4.0, < 3.0) + rubocop-ast (1.30.0) + parser (>= 3.2.1.0) ruby-prof (1.4.3) - ruby-progressbar (1.11.0) + ruby-progressbar (1.13.0) ruby_parser (3.16.0) sexp_processor (~> 4.15, >= 4.15.1) sexp_processor (4.15.3) @@ -357,7 +361,7 @@ GEM tzinfo (2.0.4) concurrent-ruby (~> 1.0) uber (0.1.0) - unicode-display_width (2.0.0) + unicode-display_width (2.5.0) webrick (1.7.0) websocket-driver (0.7.5) websocket-extensions (>= 0.1.0) @@ -416,4 +420,4 @@ DEPENDENCIES simplecov BUNDLED WITH - 2.2.3 + 2.4.22 From 0d21f1878e8f215626c23569f99120baca3c57b3 Mon Sep 17 00:00:00 2001 From: Takuma Ishikawa Date: Wed, 28 Feb 2024 15:12:38 +0900 Subject: [PATCH 2/3] rubocop: Inherit rules from cookpad/styleguide and auto-correct all Also enable some extentions according to rubocop suggestions --- .gitignore | 1 + .rubocop.yml | 19 ++++++++++++------ Gemfile | 4 ++++ Gemfile.lock | 17 ++++++++++++++++ app/batches/import_data_source_definitions.rb | 8 ++++---- app/batches/import_schema_definitions.rb | 6 +++--- app/batches/import_schema_raw_datasets.rb | 2 +- app/batches/import_table_definitions.rb | 6 +++--- app/batches/import_table_raw_datasets.rb | 4 ++-- app/batches/synchronize_data_sources.rb | 20 +++++++++---------- app/controllers/data_sources_controller.rb | 4 ++-- app/controllers/masked_data_controller.rb | 2 +- app/models/data_source.rb | 2 +- .../data_source_adapters/bigquery_adapter.rb | 17 ++++++++-------- app/models/search_result.rb | 2 +- app/models/table_memo_raw_dataset.rb | 2 +- .../markdown_previews/create.json.jbuilder | 1 - config/environments/development.rb | 2 +- config/environments/test.rb | 2 +- .../html/pipeline/autolink_keyword_filter.rb | 4 ++-- spec/batches/synchronize_data_sources_spec.rb | 4 ++-- spec/factories/column_memo_factory.rb | 2 +- spec/factories/column_memo_log_factory.rb | 2 +- spec/factories/database_memo_factory.rb | 2 +- spec/factories/database_memo_log_factory.rb | 2 +- spec/factories/keyword_log_factory.rb | 2 +- spec/factories/schema_memo_factory.rb | 2 +- spec/factories/schema_memo_log_factory.rb | 2 +- spec/factories/table_memo_factory.rb | 2 +- spec/factories/table_memo_log_factory.rb | 2 +- spec/factories/user_factory.rb | 2 +- spec/requests/masked_data_spec.rb | 2 +- spec/requests/table_memos_spec.rb | 2 +- 33 files changed, 91 insertions(+), 62 deletions(-) diff --git a/.gitignore b/.gitignore index 58d6d1b..dbc2035 100644 --- a/.gitignore +++ b/.gitignore @@ -16,5 +16,6 @@ public/packs .env.docker coverage +.rubocop-https?--* .DS_Store diff --git a/.rubocop.yml b/.rubocop.yml index 1570aad..4fe7c2a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,15 +1,22 @@ +inherit_from: + - https://raw.githubusercontent.com/cookpad/styleguide/master/.rubocop.yml + +inherit_mode: + merge: + - Exclude + +require: + - rubocop-capybara + - rubocop-factory_bot + - rubocop-rails + - rubocop-rspec + AllCops: Exclude: - 'vendor/**/*' - 'tmp/**/*' - 'db/**/*' -Style: - Enabled: false - -Layout/EmptyLineAfterGuardClause: - Enabled: false - Layout/LineLength: Enabled: false diff --git a/Gemfile b/Gemfile index bb396c9..6b1afbb 100644 --- a/Gemfile +++ b/Gemfile @@ -57,6 +57,10 @@ group :development do gem 'ridgepole' gem 'ruby-prof' gem 'rubocop', require: false + gem 'rubocop-capybara', require: false + gem 'rubocop-factory_bot', require: false + gem 'rubocop-rails', require: false + gem 'rubocop-rspec', require: false gem 'brakeman', require: false gem 'foreman' end diff --git a/Gemfile.lock b/Gemfile.lock index 5d6f0c7..81d7f92 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -329,6 +329,19 @@ GEM unicode-display_width (>= 2.4.0, < 3.0) rubocop-ast (1.30.0) parser (>= 3.2.1.0) + rubocop-capybara (2.20.0) + rubocop (~> 1.41) + rubocop-factory_bot (2.25.1) + rubocop (~> 1.41) + rubocop-rails (2.23.1) + activesupport (>= 4.2.0) + rack (>= 1.1) + rubocop (>= 1.33.0, < 2.0) + rubocop-ast (>= 1.30.0, < 2.0) + rubocop-rspec (2.26.1) + rubocop (~> 1.40) + rubocop-capybara (~> 2.17) + rubocop-factory_bot (~> 2.22) ruby-prof (1.4.3) ruby-progressbar (1.13.0) ruby_parser (3.16.0) @@ -414,6 +427,10 @@ DEPENDENCIES rouge rspec-rails rubocop + rubocop-capybara + rubocop-factory_bot + rubocop-rails + rubocop-rspec ruby-prof silencer simpacker diff --git a/app/batches/import_data_source_definitions.rb b/app/batches/import_data_source_definitions.rb index 2341687..6d4821a 100644 --- a/app/batches/import_data_source_definitions.rb +++ b/app/batches/import_data_source_definitions.rb @@ -9,14 +9,14 @@ def self.run(data_source_name) db_memo = DatabaseMemo.find_or_create_by!(name: data_source.name) schema_memos = db_memo.schema_memos linked_schema_names = schema_memos.where(linked: true).pluck(:name) - schema_memos.each {|memo| memo.linked = false } + schema_memos.each { |memo| memo.linked = false } all_table_memos = schema_memos.map(&:table_memos).map(&:to_a).flatten - all_table_memos.each {|memo| memo.linked = false } + all_table_memos.each { |memo| memo.linked = false } data_source_tables.group_by(&:schema_name).each do |schema_name, source_tables| next unless linked_schema_names.include?(schema_name) - schema_memo = schema_memos.find {|memo| memo.name == schema_name } + schema_memo = schema_memos.find { |memo| memo.name == schema_name } next if schema_memo.nil? schema_memo.linked = true begin @@ -26,7 +26,7 @@ def self.run(data_source_name) end end - schema_memos.each {|memo| memo.save! if memo.has_changes_to_save? } + schema_memos.each { |memo| memo.save! if memo.has_changes_to_save? } db_memos.save! if db_memo.has_changes_to_save? end diff --git a/app/batches/import_schema_definitions.rb b/app/batches/import_schema_definitions.rb index 8e0cc19..a059ec8 100644 --- a/app/batches/import_schema_definitions.rb +++ b/app/batches/import_schema_definitions.rb @@ -4,11 +4,11 @@ def self.run(data_source_name, schema_name) Rails.logger.info "[Start] Import definition of #{schema_name} schema in #{data_source_name}" data_source = DataSource.find_by(name: data_source_name) - source_tables = data_source.data_source_tables.select {|table| table.schema_name == schema_name } + source_tables = data_source.data_source_tables.select { |table| table.schema_name == schema_name } schema_memo = data_source.database_memo.schema_memos.find_by!(name: schema_name, linked: true) table_memos = schema_memo.table_memos - table_memos.each {|memo| memo.linked = false } + table_memos.each { |memo| memo.linked = false } if source_tables.empty? schema_memo.linked = false @@ -16,7 +16,7 @@ def self.run(data_source_name, schema_name) self.import_table_memos!(source_tables, table_memos) end - table_memos.each {|memo| memo.save! if memo.has_changes_to_save? } + table_memos.each { |memo| memo.save! if memo.has_changes_to_save? } schema_memo.save! if schema_memo.has_changes_to_save? Rails.logger.info "[Update] #{schema_name} schema" if schema_memo.saved_changes? diff --git a/app/batches/import_schema_raw_datasets.rb b/app/batches/import_schema_raw_datasets.rb index 9d637dc..e0198c3 100644 --- a/app/batches/import_schema_raw_datasets.rb +++ b/app/batches/import_schema_raw_datasets.rb @@ -4,7 +4,7 @@ def self.run(data_source_name, schema_name) Rails.logger.info "[Start] Import dataset of #{schema_name} schema in #{data_source_name}" data_source = DataSource.find_by(name: data_source_name) - source_tables = data_source.data_source_tables.select {|dst| dst.schema_name == schema_name } + source_tables = data_source.data_source_tables.select { |dst| dst.schema_name == schema_name } schema_memo = data_source.database_memo.schema_memos.find_by!(name: schema_name, linked: true) diff --git a/app/batches/import_table_definitions.rb b/app/batches/import_table_definitions.rb index 5e02a08..f718337 100644 --- a/app/batches/import_table_definitions.rb +++ b/app/batches/import_table_definitions.rb @@ -4,7 +4,7 @@ def self.run(data_source_name, schema_name, table_name) Rails.logger.info "[Start] Import definition of #{schema_name}.#{table_name} table in #{data_source_name}" data_source = DataSource.find_by(name: data_source_name) - source_table = data_source.data_source_tables.find {|dst| dst.full_table_name == "#{schema_name}.#{table_name}" } + source_table = data_source.data_source_tables.find { |dst| dst.full_table_name == "#{schema_name}.#{table_name}" } schema_memo = data_source.database_memo.schema_memos.find_by!(name: schema_name, linked: true) table_memo = schema_memo.table_memos.find_or_create_by!(name: table_name) @@ -31,10 +31,10 @@ def self.import_column_memos!(source_table, table_memo) columns = source_table.columns column_names = columns.map(&:name) - column_memos.reject {|memo| column_names.include?(memo.name) }.each {|memo| memo.update!(linked: false) } + column_memos.reject { |memo| column_names.include?(memo.name) }.each { |memo| memo.update!(linked: false) } columns.each_with_index do |column, position| - column_memo = column_memos.find {|memo| memo.name == column.name } || table_memo.column_memos.build(name: column.name) + column_memo = column_memos.find { |memo| memo.name == column.name } || table_memo.column_memos.build(name: column.name) column_memo.linked = true column_memo.assign_attributes(sql_type: column.sql_type, default: column.default, nullable: column.null, position: position) column_memo.save! if column_memo.has_changes_to_save? diff --git a/app/batches/import_table_raw_datasets.rb b/app/batches/import_table_raw_datasets.rb index 78ab3a8..3dc28f0 100644 --- a/app/batches/import_table_raw_datasets.rb +++ b/app/batches/import_table_raw_datasets.rb @@ -5,7 +5,7 @@ def self.run(data_source_name, schema_name, table_name) Rails.logger.info "[Start] Import dataset of #{schema_name}.#{table_name} table in #{data_source_name}" data_source = DataSource.find_by(name: data_source_name) - source_table = data_source.data_source_tables.find {|dst| dst.full_table_name == "#{schema_name}.#{table_name}" } + source_table = data_source.data_source_tables.find { |dst| dst.full_table_name == "#{schema_name}.#{table_name}" } schema_memo = data_source.database_memo.schema_memos.find_by!(name: schema_name, linked: true) table_memo = schema_memo.table_memos.find_or_create_by!(name: table_name) @@ -42,7 +42,7 @@ def self.import_table_memo_raw_dataset_rows!(table_memo, source_table, columns) end table_memo.raw_dataset.rows.delete_all source_table.fetch_rows(DEFAULT_FETCH_ROWS_LIMIT).each do |row| - table_memo.raw_dataset.rows.create!(row: row.map.with_index{|value, i| dataset_columns[i].format_value(value) }) + table_memo.raw_dataset.rows.create!(row: row.map.with_index { |value, i| dataset_columns[i].format_value(value) }) end end end diff --git a/app/batches/synchronize_data_sources.rb b/app/batches/synchronize_data_sources.rb index 32a55b3..8cc27ef 100644 --- a/app/batches/synchronize_data_sources.rb +++ b/app/batches/synchronize_data_sources.rb @@ -10,17 +10,17 @@ def self.run def self.import_data_source!(data_source) db_memo = DatabaseMemo.find_or_create_by!(name: data_source.name) schema_memos = db_memo.schema_memos.includes(table_memos: [:column_memos, :raw_dataset]).to_a - schema_memos.each {|memo| memo.linked = false } + schema_memos.each { |memo| memo.linked = false } all_table_memos = schema_memos.map(&:table_memos).map(&:to_a).flatten - all_table_memos.each {|memo| memo.linked = false } + all_table_memos.each { |memo| memo.linked = false } data_source_tables = data_source.data_source_tables data_source_tables.group_by(&:schema_name).each do |schema_name, source_tables| - schema_memo = schema_memos.find {|memo| memo.name == schema_name } || db_memo.schema_memos.create!(name: schema_name ) + schema_memo = schema_memos.find { |memo| memo.name == schema_name } || db_memo.schema_memos.create!(name: schema_name ) schema_memo.linked = true - table_memos = all_table_memos.select {|memo| memo.schema_memo_id == schema_memo.id } + table_memos = all_table_memos.select { |memo| memo.schema_memo_id == schema_memo.id } source_tables.each do |source_table| begin @@ -30,13 +30,13 @@ def self.import_data_source!(data_source) end end end - schema_memos.each {|memo| memo.save! if memo.changed? } - all_table_memos.each {|memo| memo.save! if memo.changed? } + schema_memos.each { |memo| memo.save! if memo.changed? } + all_table_memos.each { |memo| memo.save! if memo.changed? } end def self.import_table_memo!(schema_memo, table_memos, source_table) table_name = source_table.table_name - table_memo = table_memos.find {|memo| memo.name == table_name } || schema_memo.table_memos.create!(name: table_name ) + table_memo = table_memos.find { |memo| memo.name == table_name } || schema_memo.table_memos.create!(name: table_name ) table_memo.linked = true column_memos = table_memo.column_memos.to_a @@ -45,10 +45,10 @@ def self.import_table_memo!(schema_memo, table_memos, source_table) import_view_query!(table_memo, source_table) column_names = columns.map(&:name) - column_memos.reject {|memo| column_names.include?(memo.name) }.each {|memo| memo.update!(linked: false) } + column_memos.reject { |memo| column_names.include?(memo.name) }.each { |memo| memo.update!(linked: false) } columns.each_with_index do |column, position| - column_memo = column_memos.find {|memo| memo.name == column.name } || table_memo.column_memos.build(name: column.name) + column_memo = column_memos.find { |memo| memo.name == column.name } || table_memo.column_memos.build(name: column.name) column_memo.linked = true column_memo.assign_attributes(sql_type: column.sql_type, default: column.default, nullable: column.null, position: position) column_memo.save! if column_memo.changed? @@ -79,7 +79,7 @@ def self.import_table_memo_raw_dataset_rows!(table_memo, source_table, columns) end table_memo.raw_dataset.rows.delete_all source_table.fetch_rows(DEFAULT_FETCH_ROWS_LIMIT).each do |row| - table_memo.raw_dataset.rows.create!(row: row.map.with_index{|value, i| dataset_columns[i].format_value(value) }) + table_memo.raw_dataset.rows.create!(row: row.map.with_index { |value, i| dataset_columns[i].format_value(value) }) end end private_class_method :import_table_memo_raw_dataset_rows! diff --git a/app/controllers/data_sources_controller.rb b/app/controllers/data_sources_controller.rb index d15c99e..902a76b 100644 --- a/app/controllers/data_sources_controller.rb +++ b/app/controllers/data_sources_controller.rb @@ -37,8 +37,8 @@ def edit(id) @imported_schema_memos = @data_source.database_memo.schema_memos @subscribe_schema_names = @imported_schema_memos.where(linked: true).map(&:name) @only_dmemo_schema_names = @imported_schema_memos.pluck(:name) - @data_source_schema_names - @only_dmemo_schemas = @only_dmemo_schema_names.map{|s| [s, 'unknown']} - @all_schemas = (@data_source_schemas + @only_dmemo_schemas).sort_by{|s| s[0]} # s[0] is schema name + @only_dmemo_schemas = @only_dmemo_schema_names.map { |s| [s, 'unknown'] } + @all_schemas = (@data_source_schemas + @only_dmemo_schemas).sort_by { |s| s[0] } # s[0] is schema name rescue NotImplementedError => e # when not implement fetch_schema_names for adapter # data_sources/:id/edit page does not view Schema Candidates block diff --git a/app/controllers/masked_data_controller.rb b/app/controllers/masked_data_controller.rb index a4717bb..c639d72 100644 --- a/app/controllers/masked_data_controller.rb +++ b/app/controllers/masked_data_controller.rb @@ -13,7 +13,7 @@ def show def new @masked_datum = MaskedDatum.new - @database_name_options = (["*"] + DatabaseMemo.pluck(:name)).map {|x| [x, x]} + @database_name_options = (["*"] + DatabaseMemo.pluck(:name)).map { |x| [x, x] } end def create(masked_datum) diff --git a/app/models/data_source.rb b/app/models/data_source.rb index 42e842b..e058a00 100644 --- a/app/models/data_source.rb +++ b/app/models/data_source.rb @@ -17,7 +17,7 @@ def source_table_names def data_source_table(schema_name, table_name, table_names) return if ignore?(schema_name, table_name) - schema_name, _ = table_names.find {|schema, table| schema == schema_name && table == table_name } + schema_name, _ = table_names.find { |schema, table| schema == schema_name && table == table_name } return nil unless schema_name DataSourceTable.new(self, schema_name, table_name) end diff --git a/app/models/data_source_adapters/bigquery_adapter.rb b/app/models/data_source_adapters/bigquery_adapter.rb index ae71b50..2313e56 100644 --- a/app/models/data_source_adapters/bigquery_adapter.rb +++ b/app/models/data_source_adapters/bigquery_adapter.rb @@ -42,14 +42,15 @@ def bq_dataset return @bq_dataset if @bq_dataset config = @data_source.bigquery_config - client = config.credentials ? - Google::Cloud::Bigquery.new( - project_id: config.project_id, - credentials: JSON.parse(config.credentials), - ) : - Google::Cloud::Bigquery.new( - project_id: config.project_id, - ) + client = + if config.credentials + Google::Cloud::Bigquery.new( + project_id: config.project_id, + credentials: JSON.parse(config.credentials), + ) + else + Google::Cloud::Bigquery.new(project_id: config.project_id) + end @bq_dataset = client.dataset(config.dataset) raise DataSource::ConnectionBad.new("dataset \"#{config.dataset}\" not found") if @bq_dataset.nil? diff --git a/app/models/search_result.rb b/app/models/search_result.rb index da1e8e8..fd4c643 100644 --- a/app/models/search_result.rb +++ b/app/models/search_result.rb @@ -10,7 +10,7 @@ def search!(table_page:, column_page:, per_page:) .order(description: :desc, updated_at: :desc) .page(table_page).per(per_page) self.columns = ColumnMemo.where("column_memos.name LIKE ? OR column_memos.description LIKE ?", pattern, pattern) - .eager_load(table_memo: {schema_memo: :database_memo}) + .eager_load(table_memo: { schema_memo: :database_memo }) .order(description: :desc, updated_at: :desc) .page(column_page).per(per_page) end diff --git a/app/models/table_memo_raw_dataset.rb b/app/models/table_memo_raw_dataset.rb index d85644b..5f949e9 100644 --- a/app/models/table_memo_raw_dataset.rb +++ b/app/models/table_memo_raw_dataset.rb @@ -5,6 +5,6 @@ class TableMemoRawDataset < ApplicationRecord has_many :rows, class_name: "TableMemoRawDatasetRow", dependent: :destroy def same_columns?(source_columns) - source_columns.all? {|column| columns.where(name: column.name, sql_type: column.sql_type).exists? } + source_columns.all? { |column| columns.where(name: column.name, sql_type: column.sql_type).exists? } end end diff --git a/app/views/markdown_previews/create.json.jbuilder b/app/views/markdown_previews/create.json.jbuilder index 04d7b71..e3709e1 100644 --- a/app/views/markdown_previews/create.json.jbuilder +++ b/app/views/markdown_previews/create.json.jbuilder @@ -1,2 +1 @@ json.(@preview, :html) - diff --git a/config/environments/development.rb b/config/environments/development.rb index a8fae7f..c3ee70a 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -19,7 +19,7 @@ config.cache_store = :memory_store config.public_file_server.headers = { - 'Cache-Control' => "public, max-age=#{2.days.to_i}" + 'Cache-Control' => "public, max-age=#{2.days.to_i}", } else config.action_controller.perform_caching = false diff --git a/config/environments/test.rb b/config/environments/test.rb index 81ec7b4..4df6544 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -15,7 +15,7 @@ # Configure public file server for tests with Cache-Control for performance. config.public_file_server.enabled = true config.public_file_server.headers = { - 'Cache-Control' => "public, max-age=#{1.hour.to_i}" + 'Cache-Control' => "public, max-age=#{1.hour.to_i}", } # Show full error reports and disable caching. diff --git a/lib/autoload/html/pipeline/autolink_keyword_filter.rb b/lib/autoload/html/pipeline/autolink_keyword_filter.rb index 50ea70e..27b7d61 100644 --- a/lib/autoload/html/pipeline/autolink_keyword_filter.rb +++ b/lib/autoload/html/pipeline/autolink_keyword_filter.rb @@ -9,7 +9,7 @@ def call doc.search('.//text()').each do |node| next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS) content = node.to_html - html = content.gsub(patterns) {|matched| + html = content.gsub(patterns) { |matched| %{#{matched}} } next if html == content @@ -19,7 +19,7 @@ def call end def patterns - @patterns ||= Regexp.union(context[:autolink_keywords].keys.sort_by {|k| -k.size }) + @patterns ||= Regexp.union(context[:autolink_keywords].keys.sort_by { |k| -k.size }) end end end diff --git a/spec/batches/synchronize_data_sources_spec.rb b/spec/batches/synchronize_data_sources_spec.rb index 7b78c5f..564ddd4 100644 --- a/spec/batches/synchronize_data_sources_spec.rb +++ b/spec/batches/synchronize_data_sources_spec.rb @@ -36,7 +36,7 @@ end it "fails data_sources sync" do - expect{ batch.run }.to raise_error(DataSource::ConnectionBad) + expect { batch.run }.to raise_error(DataSource::ConnectionBad) expect(data_source.database_memo).to be_present expect(SchemaMemo.count).to eq(0) @@ -72,7 +72,7 @@ context "when columns doesn't changed" do before do stub_const("SynchronizeDataSources::DEFAULT_FETCH_ROWS_LIMIT", 4) - 4.times{|i| FactoryBot.create(:keyword, name: "sushi #{i}") } + 4.times { |i| FactoryBot.create(:keyword, name: "sushi #{i}") } batch.import_data_source!(data_source) end diff --git a/spec/factories/column_memo_factory.rb b/spec/factories/column_memo_factory.rb index 95b292e..9c412b5 100644 --- a/spec/factories/column_memo_factory.rb +++ b/spec/factories/column_memo_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :column_memo do table_memo - sequence(:name) {|n| "column#{n}" } + sequence(:name) { |n| "column#{n}" } description { "# column memo" } end end diff --git a/spec/factories/column_memo_log_factory.rb b/spec/factories/column_memo_log_factory.rb index d4bdd26..92166c3 100644 --- a/spec/factories/column_memo_log_factory.rb +++ b/spec/factories/column_memo_log_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :column_memo_log do column_memo - sequence(:revision) {|n| n } + sequence(:revision) { |n| n } user description { "# column memo" } description_diff { "+# column memo" } diff --git a/spec/factories/database_memo_factory.rb b/spec/factories/database_memo_factory.rb index 71d79ed..9f7a901 100644 --- a/spec/factories/database_memo_factory.rb +++ b/spec/factories/database_memo_factory.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :database_memo do - sequence(:name) {|n| "database#{n}" } + sequence(:name) { |n| "database#{n}" } description { "# database memo" } end end diff --git a/spec/factories/database_memo_log_factory.rb b/spec/factories/database_memo_log_factory.rb index 105e497..9b9c975 100644 --- a/spec/factories/database_memo_log_factory.rb +++ b/spec/factories/database_memo_log_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :database_memo_log do database_memo - sequence(:revision) {|n| n } + sequence(:revision) { |n| n } user description { "# database memo" } description_diff { "+# database memo" } diff --git a/spec/factories/keyword_log_factory.rb b/spec/factories/keyword_log_factory.rb index b4c08af..e3fbaf7 100644 --- a/spec/factories/keyword_log_factory.rb +++ b/spec/factories/keyword_log_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :keyword_log do keyword - sequence(:revision) {|n| n } + sequence(:revision) { |n| n } user description { "# keyword memo" } description_diff { "+# keyrword memo" } diff --git a/spec/factories/schema_memo_factory.rb b/spec/factories/schema_memo_factory.rb index 28fcbff..0467ecb 100644 --- a/spec/factories/schema_memo_factory.rb +++ b/spec/factories/schema_memo_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :schema_memo do database_memo - sequence(:name) {|n| "schema#{n}" } + sequence(:name) { |n| "schema#{n}" } description { "# schema memo" } end end diff --git a/spec/factories/schema_memo_log_factory.rb b/spec/factories/schema_memo_log_factory.rb index 9471932..416d199 100644 --- a/spec/factories/schema_memo_log_factory.rb +++ b/spec/factories/schema_memo_log_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :schema_memo_log do schema_memo - sequence(:revision) {|n| n } + sequence(:revision) { |n| n } user description { "# schema memo" } description_diff { "+# schema memo" } diff --git a/spec/factories/table_memo_factory.rb b/spec/factories/table_memo_factory.rb index 7766a3d..ffb4fe0 100644 --- a/spec/factories/table_memo_factory.rb +++ b/spec/factories/table_memo_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :table_memo do schema_memo - sequence(:name) {|n| "table#{n}" } + sequence(:name) { |n| "table#{n}" } description { "# table memo" } end end diff --git a/spec/factories/table_memo_log_factory.rb b/spec/factories/table_memo_log_factory.rb index 2ffbf17..471a591 100644 --- a/spec/factories/table_memo_log_factory.rb +++ b/spec/factories/table_memo_log_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :table_memo_log do table_memo - sequence(:revision) {|n| n } + sequence(:revision) { |n| n } user description { "# table memo" } description_diff { "+# table memo" } diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index da5302b..b06e2f2 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :user do provider { "google_oauth2" } - sequence(:uid) {|n| (n * 100).to_s } + sequence(:uid) { |n| (n * 100).to_s } name { "name_#{uid}" } email { "#{name}@gmail.com" } image_url { "https://lh3.googleusercontent.com/-l5MDH3jtWXc/AAAAAAAAAAI/AAAAAAAAAAA/2wjfVaIkYNY/photo.jpg" } diff --git a/spec/requests/masked_data_spec.rb b/spec/requests/masked_data_spec.rb index c7fb4a9..6a49b2c 100644 --- a/spec/requests/masked_data_spec.rb +++ b/spec/requests/masked_data_spec.rb @@ -19,7 +19,7 @@ it "shows" do get new_masked_datum_path expect(response).to render_template("masked_data/new") - expect(assigns(:database_name_options)).to eq([["*", "*"], ["foo", "foo"]]) + expect(assigns(:database_name_options)).to eq([%w[* *], %w[foo foo]]) end end end diff --git a/spec/requests/table_memos_spec.rb b/spec/requests/table_memos_spec.rb index db2b368..1ad4bd6 100644 --- a/spec/requests/table_memos_spec.rb +++ b/spec/requests/table_memos_spec.rb @@ -21,7 +21,7 @@ get database_schema_table_path(database_memo.name, schema_memo.name, table_memo.name) expect(response).to render_template("table_memos/show") expect(table_memo).to eq(assigns(:table_memo)) - %w(id name description adapter host port db_name user).each {|attr| expect(page).to have_content(data_source[attr]) } + %w(id name description adapter host port db_name user).each { |attr| expect(page).to have_content(data_source[attr]) } expect(page).not_to have_content(/\d+:\d+:\d+ UTC/) end From e50dc2b487e3982ea17857de7e72363765a011f7 Mon Sep 17 00:00:00 2001 From: Takuma Ishikawa Date: Wed, 28 Feb 2024 15:13:27 +0900 Subject: [PATCH 3/3] Refactor class methods of MaskedDatum - Avoid using class variables https://github.com/cookpad/styleguide/blob/master/ruby.en.md#variables - But class_attribute with the default value which is calculated using an instance method is a bit difficult - Remove cache and query directly, in order to remove both class variables and class_attributes - The calculation cost will be roughly the same --- app/controllers/table_memos_controller.rb | 6 +++- app/models/masked_datum.rb | 38 +++++++++----------- app/views/table_memos/_raw_dataset.html.haml | 4 +-- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/app/controllers/table_memos_controller.rb b/app/controllers/table_memos_controller.rb index dc23e8f..91a820b 100644 --- a/app/controllers/table_memos_controller.rb +++ b/app/controllers/table_memos_controller.rb @@ -12,7 +12,11 @@ def show(database_name, schema_name, name) take! @raw_dataset = @table_memo.raw_dataset if @raw_dataset - @raw_dataset_columns = @raw_dataset.columns.order(:position) + @masked_columns = MaskedDatum.masked_columns(database_name, name) + @raw_dataset_columns = @raw_dataset.columns.order(:position).map { |column| { + data: column, + masked: @masked_columns.include?(::MaskedDatum::ANY_NAME) || @masked_columns.include?(column.name), + }} @raw_dataset_rows = @raw_dataset.rows.pluck(:row) end @view_meta_data = @table_memo.view_meta_data diff --git a/app/models/masked_datum.rb b/app/models/masked_datum.rb index 3364ef3..3d445ee 100644 --- a/app/models/masked_datum.rb +++ b/app/models/masked_datum.rb @@ -1,38 +1,32 @@ class MaskedDatum < ApplicationRecord - validates :database_name, :table_name, :column_name, presence: true - - after_save :update_data! + ANY_NAME = "*".freeze - def self.data - @@data ||= update_data! - end + validates :database_name, :table_name, :column_name, presence: true - def self.update_data! - @@data = all.map(&:pack) - end + scope :masking_database, ->(database_name) { + where(database_name: database_name, table_name: ANY_NAME, column_name: ANY_NAME) + } + scope :masking_table, ->(database_name, table_name) { + masking_database(database_name) + .or(where(database_name: database_name, table_name: table_name, column_name: ANY_NAME)) + .or(where(database_name: ANY_NAME, table_name: table_name, column_name: ANY_NAME)) + } def self.masked_database?(database_name) - data.include?("#{database_name}/*/*") + self.masking_database(database_name).exists? end def self.masked_table?(database_name, table_name) - return true if masked_database?(database_name) - data.include?("#{database_name}/#{table_name}/*") + self.masking_table(database_name, table_name).exists? end - def self.masked_column?(database_name, table_name, column_name) - return true if masked_database?(database_name) - return true if masked_table?(database_name, table_name) - data.include?("#{database_name}/#{table_name}/#{column_name}") || data.include?("#{database_name}/*/#{column_name}") || data.include?("*/*/#{column_name}") + def self.masked_columns(database_name, table_name) + return [ANY_NAME] if self.masked_table?(database_name, table_name) + + Set.new(self.where(database_name: database_name, table_name: table_name).pluck(:column_name)) end def pack "#{database_name}/#{table_name}/#{column_name}" end - - private - - def update_data! - self.class.update_data! - end end diff --git a/app/views/table_memos/_raw_dataset.html.haml b/app/views/table_memos/_raw_dataset.html.haml index e446c6a..ca3f5a2 100644 --- a/app/views/table_memos/_raw_dataset.html.haml +++ b/app/views/table_memos/_raw_dataset.html.haml @@ -6,7 +6,7 @@ %table.table.table-hover.table-bordered.table-striped.text-sm{ role: "grid" } %tr - @raw_dataset_columns.each do |column| - %th #{column.name} (#{column.sql_type}) + %th #{column.fetch(:data).name} (#{column.fetch(:data).sql_type}) - if @table_memo.masked? %tr %td.text-center{ colspan: @raw_dataset_columns.size } @@ -18,7 +18,7 @@ %tr - Array.wrap(row).zip(@raw_dataset_columns).each_with_index do |(value, column), i| %td - - if MaskedDatum.masked_column?(database_name, table_name, column.name) + - if column.fetch(:masked) = t("masked_text") - else = value