diff --git a/README.md b/README.md index eb9dd60..526cfd3 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,9 @@ AB ... ``` -String sequences follow the same pattern as Excel columns. +String sequences follow Ruby's [`String#next`](https://ruby-doc.org/3.4/String.html#method-i-next) logic. The column type is inferred from the database schema. + +> **Deprecation**: Explicitly passing an initial value whose type differs from the database column type is deprecated. For example, `auto_increment :ref, initial: 1` on a `string` column will emit a warning. When `initial` is not set, the default is automatically inferred from the column type (`"1"` for string columns, `1` for integer columns). ### Scoped Sequences @@ -249,7 +251,7 @@ auto_increment :number, | Option | Description | Default | | ------------- | ------------------------------------------------------------------ | --------- | | `column` | Column to increment. Can be integer or string. | `:code` | -| `initial` | Starting value. Integer or string. | `1` | +| `initial` | Starting value. Must match the database column type (`Integer` for integer columns, `String` for string columns). When omitted, inferred from the column type. | `1` or `"1"` (inferred) | | `scope` | Restricts the sequence to matching column values. | `nil` | | `model_scope` | Applies Active Record scopes before calculating the maximum value. | `nil` | | `force` | Overwrites an already assigned value. | `false` | diff --git a/lib/auto_increment/active_record.rb b/lib/auto_increment/active_record.rb index 32e3fdf..78aa0fd 100644 --- a/lib/auto_increment/active_record.rb +++ b/lib/auto_increment/active_record.rb @@ -9,10 +9,38 @@ module ActiveRecord # +AutoIncrement::ActiveRecord::ClassMethods+ module ClassMethods def auto_increment(column = nil, **options) + column ||= options.fetch(:column, :code) + + auto_increment_deprecate_type_mismatch(column, options[:initial]) if options.key?(:initial) + send("before_#{options.fetch(:before, :create)}") do |record| Incrementor.new(record, column, **options).run end end + + private + + def auto_increment_deprecate_type_mismatch(column, initial) + col = begin + columns_hash[column.to_s] + rescue ActiveRecord::ConnectionNotEstablished, ActiveRecord::NoDatabaseError + # Ignore connection/schema errors during class loading + return + end + return unless col + + col_type = col.type + return if col_type == :integer && initial.is_a?(Integer) + return if col_type.in?(%i[string text]) && initial.is_a?(String) + + model_name = name.presence || "anonymous" + + warn( + "[DEPRECATION] The initial value type (#{initial.class}) does not match " \ + "the column type (#{col_type}) for column '#{column}' on #{model_name}. " \ + "This behavior is deprecated and will raise an error in the future." + ) + end end end end diff --git a/lib/auto_increment/incrementor.rb b/lib/auto_increment/incrementor.rb index 69ca149..a87bb8a 100644 --- a/lib/auto_increment/incrementor.rb +++ b/lib/auto_increment/incrementor.rb @@ -7,7 +7,7 @@ class Incrementor def initialize(record, column = nil, **options) @record = record @column = column || options.fetch(:column, :code) - @initial = options.fetch(:initial, 1) + @initial = resolve_initial(options) @force = options.fetch(:force, false) @scope = Array.wrap(options[:scope]).compact @model_scope = Array.wrap(options[:model_scope]).compact @@ -54,9 +54,10 @@ def build_model_scope(query) def maximum query = maximum_query - if string? - query.select("#{@column} max") - .order(Arel.sql("LENGTH(#{@column}) DESC, #{@column} DESC")) + if column_string? + quoted_column = @record.class.connection.quote_column_name(@column) + query.select("#{quoted_column} max") + .order(Arel.sql("LENGTH(#{quoted_column}) DESC, #{quoted_column} DESC")) .first.try :max else query.maximum @column @@ -73,8 +74,15 @@ def increment max.blank? ? @initial : max.next end - def string? - @initial.instance_of?(String) + def resolve_initial(options) + return options[:initial] if options.key?(:initial) + + column_string? ? "1" : 1 + end + + def column_string? + col = @record.class.columns_hash[@column.to_s] + col&.type&.in?(%i[string text]) end end end diff --git a/spec/lib/active_record_spec.rb b/spec/lib/active_record_spec.rb index 8273bff..1059e6c 100644 --- a/spec/lib/active_record_spec.rb +++ b/spec/lib/active_record_spec.rb @@ -3,6 +3,7 @@ require "spec_helper" require "models/account" require "models/user" +require "models/post" describe AutoIncrement do before :all do @@ -73,4 +74,60 @@ describe "uses model scopes" do it { expect(@user3_account2.letter_code).to eq("C") } end + + describe "string column with integer initial" do + it "increments correctly past the 9-to-10 boundary" do + 15.times do |i| + post = Post.create! + expect(post.ref.to_i).to eq(i + 1) + end + end + end + + describe "deprecation warning" do + it "warns when initial is a string on an integer column" do + expect { + Class.new(ActiveRecord::Base) do + self.table_name = "accounts" + auto_increment :code, initial: "A" + end + }.to output(/\[DEPRECATION\] The initial value type \(String\) does not match the column type \(integer\) for column 'code'.*raise an error in the future/).to_stderr + end + + it "warns when initial is an integer on a string column" do + expect { + Class.new(ActiveRecord::Base) do + self.table_name = "posts" + auto_increment :ref, initial: 1 + end + }.to output(/\[DEPRECATION\] The initial value type \(Integer\) does not match the column type \(string\) for column 'ref'.*raise an error in the future/).to_stderr + end + + it "does not warn when types match (integer column, integer initial)" do + expect { + Class.new(ActiveRecord::Base) do + self.table_name = "accounts" + auto_increment :code, initial: 100 + end + }.not_to output(/\[DEPRECATION\]/).to_stderr + end + + it "does not warn when types match (string column, string initial)" do + expect { + Class.new(ActiveRecord::Base) do + self.table_name = "posts" + auto_increment :ref, initial: "X" + end + }.not_to output(/\[DEPRECATION\]/).to_stderr + end + + it "does not warn when initial is omitted on a string column (auto-detects)" do + expect { + Class.new(ActiveRecord::Base) do + self.table_name = "posts" + auto_increment :ref + end + }.not_to output(/\[DEPRECATION\]/).to_stderr + end + end end diff --git a/spec/lib/incrementor_spec.rb b/spec/lib/incrementor_spec.rb index 64be7f4..b89ac49 100644 --- a/spec/lib/incrementor_spec.rb +++ b/spec/lib/incrementor_spec.rb @@ -3,6 +3,7 @@ require "spec_helper" require "models/account" require "models/user" +require "models/post" describe AutoIncrement::Incrementor do def create_account(code:, name: "seed") @@ -17,8 +18,8 @@ def create_user(code:, name: "seed") end describe "#run" do - describe "integer" do - it "sets initial value to 1 when no records exist" do + describe "integer column" do + it "auto-detects initial value 1 when no initial is given" do account = Account.new AutoIncrement::Incrementor.new(account).run expect(account.code).to eq 1 @@ -39,13 +40,19 @@ def create_user(code:, name: "seed") end end - describe "string" do - it "uses the initial value when no records exist" do + describe "string column" do + it "sets initial value when no records exist" do user = User.new AutoIncrement::Incrementor.new(user, column: :letter_code, initial: "A").run expect(user.letter_code).to eq "A" end + it "auto-detects initial value '1' when no initial is given" do + post = Post.new + AutoIncrement::Incrementor.new(post, column: :ref).run + expect(post.ref).to eq "1" + end + { "A" => "B", "Z" => "AA", @@ -60,16 +67,24 @@ def create_user(code:, name: "seed") expect(user.letter_code).to eq next_value end end + + it "uses length-aware ordering inferred from the column schema" do + %w[1 2 3 4 5 6 7 8 9 10].each { |v| create_user(code: v) } + + user = User.new + AutoIncrement::Incrementor.new(user, column: :letter_code, initial: 1).run + expect(user.letter_code).to eq "11" + end end - context "when column value is already set" do - it "does not change the column if force is false" do + describe "force" do + it "does not overwrite an existing value when force is false" do account = Account.new(code: 5) expect { AutoIncrement::Incrementor.new(account).run } .not_to change { account.code } end - it "changes the column if force is true" do + it "overwrites an existing value when force is true" do create_account(code: 10) account = Account.new(code: 5) AutoIncrement::Incrementor.new(account, force: true).run @@ -83,7 +98,7 @@ def create_user(code:, name: "seed") end end - context "scoped increment" do + describe "scope" do it "only considers records within the same scope" do create_account(code: 10, name: "other") @@ -101,38 +116,36 @@ def create_user(code:, name: "seed") AutoIncrement::Incrementor.new(user, column: :letter_code, initial: "A", model_scope: :with_mark).run expect(user.letter_code).to eq "D" end - end - end - describe "model_scope option" do - it "applies model scopes when building the query" do - create_user(code: "C", name: "Mark") - create_user(code: "A", name: "Other") + it "applies model scopes when building the query" do + create_user(code: "C", name: "Mark") + create_user(code: "A", name: "Other") - user = User.new(name: "Mark") - AutoIncrement::Incrementor.new(user, column: :letter_code, initial: "A", model_scope: :with_mark).run - expect(user.letter_code).to eq "D" - end + user = User.new(name: "Mark") + AutoIncrement::Incrementor.new(user, column: :letter_code, initial: "A", model_scope: :with_mark).run + expect(user.letter_code).to eq "D" + end - it "only considers records matching the model scope for integer columns" do - create_account(code: 10, name: "Mark") - create_account(code: 5, name: "Other") - account = Account.new(name: "Mark") - AutoIncrement::Incrementor.new(account, column: :code, initial: 1, model_scope: :only_mark).run - expect(account.code).to eq 11 + it "only considers records matching the model scope" do + create_account(code: 10, name: "Mark") + create_account(code: 5, name: "Other") + account = Account.new(name: "Mark") + AutoIncrement::Incrementor.new(account, column: :code, initial: 1, model_scope: :only_mark).run + expect(account.code).to eq 11 + end end - end - describe "locking the query" do - it "increments correctly with lock enabled" do - create_account(code: 10) - account = Account.new - incrementor = AutoIncrement::Incrementor.new(account, lock: true) + describe "lock" do + it "increments correctly with lock enabled" do + create_account(code: 10) + account = Account.new + incrementor = AutoIncrement::Incrementor.new(account, lock: true) - expect(incrementor.send(:maximum_query).lock_value).to eq true + expect(incrementor.send(:maximum_query).lock_value).to eq true - incrementor.run - expect(account.code).to eq 11 + incrementor.run + expect(account.code).to eq 11 + end end end end diff --git a/spec/models/post.rb b/spec/models/post.rb new file mode 100644 index 0000000..e4b6067 --- /dev/null +++ b/spec/models/post.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +# Spec +Post+ — string column with default integer initial +class Post < ActiveRecord::Base + auto_increment :ref +end diff --git a/spec/support/active_record.rb b/spec/support/active_record.rb index afe67d5..a42ec64 100644 --- a/spec/support/active_record.rb +++ b/spec/support/active_record.rb @@ -12,3 +12,8 @@ t.integer :account_id t.string :letter_code end + +# +ActiveRecord+ migration for Posts (string column, integer initial) +ActiveRecord::Migration.create_table :posts do |t| + t.string :ref +end