diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index d3c0a5502..26ebac43e 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -512,23 +512,13 @@ def annotate_one_file(file_name, info_block, position, options = {}) return false unless File.exist?(file_name) old_content = File.read(file_name) return false if old_content =~ /#{SKIP_ANNOTATION_PREFIX}.*\n/ - - # Ignore the Schema version line because it changes with each migration - header_pattern = /(^# Table name:.*?\n(#.*[\r]?\n)*[\r]?)/ - old_header = old_content.match(header_pattern).to_s - new_header = info_block.match(header_pattern).to_s - - column_pattern = /^#[\t ]+[\w\*`]+[\t ]+.+$/ - old_columns = old_header && old_header.scan(column_pattern).sort - new_columns = new_header && new_header.scan(column_pattern).sort - - return false if old_columns == new_columns && !options[:force] + return false if columns_unchanged?(old_content, info_block, options) && !options[:force] abort "annotate error. #{file_name} needs to be updated, but annotate was run with `--frozen`." if options[:frozen] # Replace inline the old schema info with the new schema info - wrapper_open = options[:wrapper_open] ? "# #{options[:wrapper_open]}\n" : "" - wrapper_close = options[:wrapper_close] ? "# #{options[:wrapper_close]}\n" : "" + wrapper_open = options[:wrapper_open] ? "#{wrapper_line(options[:wrapper_open])}\n" : "" + wrapper_close = options[:wrapper_close] ? "#{wrapper_line(options[:wrapper_close])}\n" : "" wrapped_info_block = "#{wrapper_open}#{info_block}#{wrapper_close}" old_annotation = old_content.match(annotate_pattern(options)).to_s @@ -939,6 +929,34 @@ def mb_chars_ljust(string, length) def non_ascii_length(string) string.to_s.chars.reject(&:ascii_only?).length end + + def wrapper_line(wrapper) + "# #{wrapper}" + end + + def columns_unchanged?(old_content, info_block, options) + # Ignore the Schema version line because it changes with each migration + header_pattern = /(^# Table name:.*?\n(#.*[\r]?\n)*[\r]?)/ + old_header = old_content.match(header_pattern).to_s + new_header = info_block.match(header_pattern).to_s + + column_pattern = /^#[\t ]+[\w\*`]+[\t ]+.+$/ + old_columns = [] + new_columns = [] + + if old_header + old_columns = old_header.scan(column_pattern).sort + old_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open] + old_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close] + end + if new_header + new_columns = new_header.scan(column_pattern).sort + new_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open] + new_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close] + end + + old_columns == new_columns + end end class BadModelFileError < LoadError diff --git a/spec/lib/annotate/annotate_models_spec.rb b/spec/lib/annotate/annotate_models_spec.rb index 1c906b2d5..badcedcbd 100644 --- a/spec/lib/annotate/annotate_models_spec.rb +++ b/spec/lib/annotate/annotate_models_spec.rb @@ -1711,13 +1711,14 @@ def write_model(file_name, file_content) def annotate_one_file(options = {}) Annotate.set_defaults(options) options = Annotate.setup_options(options) - AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options) + result = AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options) # Wipe settings so the next call will pick up new values... Annotate.instance_variable_set('@has_set_defaults', false) Annotate::POSITION_OPTIONS.each { |key| ENV[key.to_s] = '' } Annotate::FLAG_OPTIONS.each { |key| ENV[key.to_s] = '' } Annotate::PATH_OPTIONS.each { |key| ENV[key.to_s] = '' } + result end def magic_comments_list_each @@ -1882,6 +1883,21 @@ class User < ActiveRecord::Base end end + describe 'wrapper value matches column pattern' do + let(:wrapper) { 'Schema generated with `annotate`' } + + it 'does not consider this as a new column and does not try to re-annotate' do + res = annotate_one_file wrapper_open: wrapper, wrapper_close: wrapper + expect(res).to eq(true) + + expect(File.read(@model_file_name)) + .to eq("# #{wrapper}\n#{@schema_info}# #{wrapper}\n\n#{@file_content}") + + res = annotate_one_file wrapper_open: wrapper, wrapper_close: wrapper + expect(res).to eq(false) # Model file unchanged + end + end + describe "if a file can't be annotated" do before do allow(AnnotateModels).to receive(:get_loaded_model_by_path).with('user').and_return(nil)