From b413bcfcaa854152a16c5eb82b4ef8ba9fea8007 Mon Sep 17 00:00:00 2001 From: Lovro Bikic Date: Fri, 23 Apr 2021 04:38:10 +0200 Subject: [PATCH 1/5] Add support for annotating check constraints Signed-off-by: Lovro Bikic --- README.md | 1 + lib/annotate/annotate_models.rb | 29 ++++ lib/annotate/constants.rb | 3 +- lib/annotate/parser.rb | 6 + .../templates/auto_annotate_models.rake | 1 + lib/tasks/annotate_models.rake | 1 + spec/lib/annotate/annotate_models_spec.rb | 127 +++++++++++++++++- spec/lib/annotate/parser_spec.rb | 11 ++ 8 files changed, 173 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index ea489f587..b5c7ff790 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,7 @@ you can do so with a simple environment variable, instead of editing the -a, --active-admin Annotate active_admin models -v, --version Show the current version of this gem -m, --show-migration Include the migration version number in the annotation + -c, --show-check-constraints List the table's check constraints in the annotation -k, --show-foreign-keys List the table's foreign key constraints in the annotation --ck, --complete-foreign-keys Complete foreign key names in the annotation diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index fc503839b..b66e15444 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -178,6 +178,10 @@ def get_schema_info(klass, header, options = {}) info << get_foreign_key_info(klass, options) end + if options[:show_check_constraints] && klass.table_exists? + info << get_check_constraint_info(klass, options) + end + info << get_schema_footer_text(klass, options) end @@ -352,6 +356,31 @@ def get_foreign_key_info(klass, options = {}) fk_info end + def get_check_constraint_info(klass, options = {}) + cc_info = if options[:format_markdown] + "#\n# ### Check Constraints\n#\n" + else + "#\n# Check Constraints\n#\n" + end + + return '' unless klass.connection.respond_to?(:supports_check_constraints?) && + klass.connection.supports_check_constraints? && klass.connection.respond_to?(:check_constraints) + + check_constraints = klass.connection.check_constraints(klass.table_name) + return '' if check_constraints.empty? + + max_size = check_constraints.map { |check_constraint| check_constraint.name.size }.max + 1 + check_constraints.sort_by(&:name).each do |check_constraint| + cc_info << if options[:format_markdown] + sprintf("# * `%s`: `(%s)`\n", check_constraint.name, check_constraint.expression) + else + sprintf("# %-#{max_size}.#{max_size}s (%s)\n", check_constraint.name, check_constraint.expression) + end + end + + cc_info + end + # Add a schema block to a file. If the file already contains # a schema info block (a comment starting with "== Schema Information"), # check if it matches the block that is already there. If so, leave it be. diff --git a/lib/annotate/constants.rb b/lib/annotate/constants.rb index cd2148f39..57a26151f 100644 --- a/lib/annotate/constants.rb +++ b/lib/annotate/constants.rb @@ -18,7 +18,8 @@ module Constants :trace, :timestamp, :exclude_serializers, :classified_sort, :show_foreign_keys, :show_complete_foreign_keys, :exclude_scaffolds, :exclude_controllers, :exclude_helpers, - :exclude_sti_subclasses, :ignore_unknown_models, :with_comment + :exclude_sti_subclasses, :ignore_unknown_models, :with_comment, + :show_check_constraints ].freeze OTHER_OPTIONS = [ diff --git a/lib/annotate/parser.rb b/lib/annotate/parser.rb index cb27b8b5d..2c7dcfc04 100644 --- a/lib/annotate/parser.rb +++ b/lib/annotate/parser.rb @@ -173,6 +173,12 @@ def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength env['include_version'] = 'yes' end + option_parser.on('-c', + '--show-check-constraints', + "List the table's check constraints in the annotation") do + env['show_check_constraints'] = 'yes' + end + option_parser.on('-k', '--show-foreign-keys', "List the table's foreign key constraints in the annotation") do diff --git a/lib/generators/annotate/templates/auto_annotate_models.rake b/lib/generators/annotate/templates/auto_annotate_models.rake index 1f355249b..78b75eb51 100644 --- a/lib/generators/annotate/templates/auto_annotate_models.rake +++ b/lib/generators/annotate/templates/auto_annotate_models.rake @@ -17,6 +17,7 @@ if Rails.env.development? 'position_in_fixture' => 'before', 'position_in_factory' => 'before', 'position_in_serializer' => 'before', + 'show_check_constraints' => 'false', 'show_foreign_keys' => 'true', 'show_complete_foreign_keys' => 'false', 'show_indexes' => 'true', diff --git a/lib/tasks/annotate_models.rake b/lib/tasks/annotate_models.rake index 76d8cfe63..448fa80b5 100644 --- a/lib/tasks/annotate_models.rake +++ b/lib/tasks/annotate_models.rake @@ -18,6 +18,7 @@ task annotate_models: :environment do options[:position_in_factory] = Annotate::Helpers.fallback(ENV['position_in_factory'], ENV['position']) options[:position_in_test] = Annotate::Helpers.fallback(ENV['position_in_test'], ENV['position']) options[:position_in_serializer] = Annotate::Helpers.fallback(ENV['position_in_serializer'], ENV['position']) + options[:show_check_constraints] = Annotate::Helpers.true?(ENV['show_check_constraints']) options[:show_foreign_keys] = Annotate::Helpers.true?(ENV['show_foreign_keys']) options[:show_complete_foreign_keys] = Annotate::Helpers.true?(ENV['show_complete_foreign_keys']) options[:show_indexes] = Annotate::Helpers.true?(ENV['show_indexes']) diff --git a/spec/lib/annotate/annotate_models_spec.rb b/spec/lib/annotate/annotate_models_spec.rb index b5f167d67..c31beadfa 100644 --- a/spec/lib/annotate/annotate_models_spec.rb +++ b/spec/lib/annotate/annotate_models_spec.rb @@ -41,16 +41,24 @@ def mock_foreign_key(name, from_column, to_table, to_column = 'id', constraints on_update: constraints[:on_update]) end - def mock_connection(indexes = [], foreign_keys = []) + def mock_check_constraint(name, expression) + double('CheckConstraintDefinition', + name: name, + expression: expression) + end + + def mock_connection(indexes = [], foreign_keys = [], check_constraints = []) double('Conn', indexes: indexes, foreign_keys: foreign_keys, - supports_foreign_keys?: true) + check_constraints: check_constraints, + supports_foreign_keys?: true, + supports_check_constraints?: true) end - def mock_class(table_name, primary_key, columns, indexes = [], foreign_keys = []) + def mock_class(table_name, primary_key, columns, indexes = [], foreign_keys = [], check_constraints = []) options = { - connection: mock_connection(indexes, foreign_keys), + connection: mock_connection(indexes, foreign_keys, check_constraints), table_exists?: true, table_name: table_name, primary_key: primary_key, @@ -221,7 +229,7 @@ def mock_column(name, type, options = {}) end let :klass do - mock_class(:users, primary_key, columns, indexes, foreign_keys) + mock_class(:users, primary_key, columns, indexes, foreign_keys, check_constraints) end let :indexes do @@ -232,6 +240,10 @@ def mock_column(name, type, options = {}) [] end + let :check_constraints do + [] + end + context 'when option is not present' do let :options do {} @@ -756,6 +768,73 @@ def mock_column(name, type, options = {}) end end + context 'when check constraints exist' do + let :columns do + [ + mock_column(:id, :integer), + mock_column(:age, :integer) + ] + end + + context 'when option "show_check_constraints" is true' do + let :options do + { show_check_constraints: true } + end + + context 'when check constraints are defined' do + let :check_constraints do + [ + mock_check_constraint('alive', 'age < 150'), + mock_check_constraint('must_be_adult', 'age >= 18') + ] + end + + let :expected_result do + <<~EOS + # Schema Info + # + # Table name: users + # + # id :integer not null, primary key + # age :integer not null + # + # Check Constraints + # + # alive (age < 150) + # must_be_adult (age >= 18) + # + EOS + end + + it 'returns schema info with check constraint information' do + is_expected.to eq expected_result + end + end + + context 'when check constraint is not defined' do + let :check_constraints do + [] + end + + let :expected_result do + <<~EOS + # Schema Info + # + # Table name: users + # + # id :integer not null, primary key + # age :integer not null + # + EOS + end + + it 'returns schema info without check constraint information' do + is_expected.to eq expected_result + end + end + end + end + context 'when foreign keys exist' do let :columns do [ @@ -1492,6 +1571,44 @@ def mock_column(name, type, options = {}) end end + context 'when option "show_check_constraints" is true' do + let :options do + { format_markdown: true, show_check_constraints: true } + end + + context 'when check constraints are defined' do + let :check_constraints do + [ + mock_check_constraint('min_name_length', 'LENGTH(name) > 2') + ] + end + + let :expected_result do + <<~EOS + # == Schema Information + # + # Table name: `users` + # + # ### Columns + # + # Name | Type | Attributes + # ----------- | ------------------ | --------------------------- + # **`id`** | `integer` | `not null, primary key` + # **`name`** | `string(50)` | `not null` + # + # ### Check Constraints + # + # * `min_name_length`: `(LENGTH(name) > 2)` + # + EOS + end + + it 'returns schema info with check constraint information in Markdown format' do + is_expected.to eq expected_result + end + end + end + context 'when option "show_foreign_keys" is true' do let :options do { format_markdown: true, show_foreign_keys: true } diff --git a/spec/lib/annotate/parser_spec.rb b/spec/lib/annotate/parser_spec.rb index 176e453e3..16a18e25f 100644 --- a/spec/lib/annotate/parser_spec.rb +++ b/spec/lib/annotate/parser_spec.rb @@ -260,6 +260,17 @@ module Annotate # rubocop:disable Metrics/ModuleLength end end + %w[-c --show-check-constraints].each do |option| + describe option do + let(:env_key) { 'show_check_constraints' } + let(:set_value) { 'yes' } + it 'sets the ENV variable' do + expect(ENV).to receive(:[]=).with(env_key, set_value) + Parser.parse([option]) + end + end + end + %w[-k --show-foreign-keys].each do |option| describe option do let(:env_key) { 'show_foreign_keys' } From 6501d87999c37be4b59987efb944d6a949e5bafd Mon Sep 17 00:00:00 2001 From: Lovro Bikic Date: Fri, 23 Apr 2021 05:01:09 +0200 Subject: [PATCH 2/5] Disable parameter lists cop Signed-off-by: Lovro Bikic --- spec/lib/annotate/annotate_models_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/lib/annotate/annotate_models_spec.rb b/spec/lib/annotate/annotate_models_spec.rb index c31beadfa..c91f2cd3d 100644 --- a/spec/lib/annotate/annotate_models_spec.rb +++ b/spec/lib/annotate/annotate_models_spec.rb @@ -56,6 +56,7 @@ def mock_connection(indexes = [], foreign_keys = [], check_constraints = []) supports_check_constraints?: true) end + # rubocop:disable Metrics/ParameterLists def mock_class(table_name, primary_key, columns, indexes = [], foreign_keys = [], check_constraints = []) options = { connection: mock_connection(indexes, foreign_keys, check_constraints), @@ -70,6 +71,7 @@ def mock_class(table_name, primary_key, columns, indexes = [], foreign_keys = [] double('An ActiveRecord class', options) end + # rubocop:enable Metrics/ParameterLists def mock_column(name, type, options = {}) default_options = { From 7cc5481becfa88c024ecf7332e6dc1663138ed52 Mon Sep 17 00:00:00 2001 From: Lovro Bikic Date: Wed, 4 May 2022 16:53:21 +0200 Subject: [PATCH 3/5] Squish check constraint expressions Signed-off-by: Lovro Bikic --- lib/annotate/annotate_models.rb | 4 ++-- spec/lib/annotate/annotate_models_spec.rb | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index b66e15444..744848087 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -372,9 +372,9 @@ def get_check_constraint_info(klass, options = {}) max_size = check_constraints.map { |check_constraint| check_constraint.name.size }.max + 1 check_constraints.sort_by(&:name).each do |check_constraint| cc_info << if options[:format_markdown] - sprintf("# * `%s`: `(%s)`\n", check_constraint.name, check_constraint.expression) + sprintf("# * `%s`: `(%s)`\n", check_constraint.name, check_constraint.expression.squish) else - sprintf("# %-#{max_size}.#{max_size}s (%s)\n", check_constraint.name, check_constraint.expression) + sprintf("# %-#{max_size}.#{max_size}s (%s)\n", check_constraint.name, check_constraint.expression.squish) end end diff --git a/spec/lib/annotate/annotate_models_spec.rb b/spec/lib/annotate/annotate_models_spec.rb index c91f2cd3d..db00406dc 100644 --- a/spec/lib/annotate/annotate_models_spec.rb +++ b/spec/lib/annotate/annotate_models_spec.rb @@ -787,7 +787,13 @@ def mock_column(name, type, options = {}) let :check_constraints do [ mock_check_constraint('alive', 'age < 150'), - mock_check_constraint('must_be_adult', 'age >= 18') + mock_check_constraint('must_be_adult', 'age >= 18'), + mock_check_constraint('multiline_test', <<~SQL) + CASE + WHEN (age >= 18) THEN (age <= 21) + ELSE true + END + SQL ] end @@ -802,8 +808,9 @@ def mock_column(name, type, options = {}) # # Check Constraints # - # alive (age < 150) - # must_be_adult (age >= 18) + # alive (age < 150) + # multiline_test (CASE WHEN (age >= 18) THEN (age <= 21) ELSE true END) + # must_be_adult (age >= 18) # EOS end @@ -1581,7 +1588,13 @@ def mock_column(name, type, options = {}) context 'when check constraints are defined' do let :check_constraints do [ - mock_check_constraint('min_name_length', 'LENGTH(name) > 2') + mock_check_constraint('min_name_length', 'LENGTH(name) > 2'), + mock_check_constraint('multiline_test', <<~SQL) + CASE + WHEN (age >= 18) THEN (age <= 21) + ELSE true + END + SQL ] end @@ -1601,6 +1614,7 @@ def mock_column(name, type, options = {}) # ### Check Constraints # # * `min_name_length`: `(LENGTH(name) > 2)` + # * `multiline_test`: `(CASE WHEN (age >= 18) THEN (age <= 21) ELSE true END)` # EOS end From 97a523259723da70d579f485cbea2717e0f45adb Mon Sep 17 00:00:00 2001 From: Lovro Bikic Date: Thu, 5 May 2022 16:06:46 +0200 Subject: [PATCH 4/5] Handle missing expression Signed-off-by: Lovro Bikic --- lib/annotate/annotate_models.rb | 8 ++++++-- spec/lib/annotate/annotate_models_spec.rb | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 744848087..78da462e0 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -371,10 +371,14 @@ def get_check_constraint_info(klass, options = {}) max_size = check_constraints.map { |check_constraint| check_constraint.name.size }.max + 1 check_constraints.sort_by(&:name).each do |check_constraint| + expression = check_constraint.expression ? "(#{check_constraint.expression.squish})" : nil + cc_info << if options[:format_markdown] - sprintf("# * `%s`: `(%s)`\n", check_constraint.name, check_constraint.expression.squish) + cc_info_markdown = sprintf("# * `%s`", check_constraint.name) + cc_info_markdown << sprintf(": `%s`", expression) if expression + cc_info_markdown << "\n" else - sprintf("# %-#{max_size}.#{max_size}s (%s)\n", check_constraint.name, check_constraint.expression.squish) + sprintf("# %-#{max_size}.#{max_size}s %s", check_constraint.name, expression).rstrip + "\n" end end diff --git a/spec/lib/annotate/annotate_models_spec.rb b/spec/lib/annotate/annotate_models_spec.rb index db00406dc..ca7098c82 100644 --- a/spec/lib/annotate/annotate_models_spec.rb +++ b/spec/lib/annotate/annotate_models_spec.rb @@ -788,6 +788,7 @@ def mock_column(name, type, options = {}) [ mock_check_constraint('alive', 'age < 150'), mock_check_constraint('must_be_adult', 'age >= 18'), + mock_check_constraint('missing_expression', nil), mock_check_constraint('multiline_test', <<~SQL) CASE WHEN (age >= 18) THEN (age <= 21) @@ -808,9 +809,10 @@ def mock_column(name, type, options = {}) # # Check Constraints # - # alive (age < 150) - # multiline_test (CASE WHEN (age >= 18) THEN (age <= 21) ELSE true END) - # must_be_adult (age >= 18) + # alive (age < 150) + # missing_expression + # multiline_test (CASE WHEN (age >= 18) THEN (age <= 21) ELSE true END) + # must_be_adult (age >= 18) # EOS end @@ -1589,6 +1591,7 @@ def mock_column(name, type, options = {}) let :check_constraints do [ mock_check_constraint('min_name_length', 'LENGTH(name) > 2'), + mock_check_constraint('missing_expression', nil), mock_check_constraint('multiline_test', <<~SQL) CASE WHEN (age >= 18) THEN (age <= 21) @@ -1614,6 +1617,7 @@ def mock_column(name, type, options = {}) # ### Check Constraints # # * `min_name_length`: `(LENGTH(name) > 2)` + # * `missing_expression` # * `multiline_test`: `(CASE WHEN (age >= 18) THEN (age <= 21) ELSE true END)` # EOS From 1406c8e2b1ae0ae2ff6b10a1ae5251856bee03e2 Mon Sep 17 00:00:00 2001 From: Lovro Bikic Date: Wed, 29 Mar 2023 10:23:15 +0200 Subject: [PATCH 5/5] Fix rubocop offenses Signed-off-by: Lovro Bikic --- lib/annotate/annotate_models.rb | 2 +- lib/annotate/parser.rb | 2 +- spec/lib/annotate/annotate_models_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 78da462e0..be2a30cb4 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -131,7 +131,7 @@ def retrieve_indexes_from_table(klass) # to create a comment block containing a line for # each column. The line contains the column name, # the type (and length), and any optional attributes - def get_schema_info(klass, header, options = {}) + def get_schema_info(klass, header, options = {}) # rubocop:disable Metrics/MethodLength info = "# #{header}\n" info << get_schema_header_text(klass, options) diff --git a/lib/annotate/parser.rb b/lib/annotate/parser.rb index 2c7dcfc04..3f5ebdb00 100644 --- a/lib/annotate/parser.rb +++ b/lib/annotate/parser.rb @@ -48,7 +48,7 @@ def commit end end - def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength + def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize has_set_position = {} option_parser.banner = 'Usage: annotate [options] [model_file]*' diff --git a/spec/lib/annotate/annotate_models_spec.rb b/spec/lib/annotate/annotate_models_spec.rb index ca7098c82..59cd8eb1c 100644 --- a/spec/lib/annotate/annotate_models_spec.rb +++ b/spec/lib/annotate/annotate_models_spec.rb @@ -405,7 +405,7 @@ def mock_column(name, type, options = {}) end end - context 'with Globalize gem' do + context 'with Globalize gem' do # rubocop:disable RSpec/MultipleMemoizedHelpers let :translation_klass do double('Folder::Post::Translation', to_s: 'Folder::Post::Translation',