Fix race condition in slug generation

CHANGELOG.md

@@ -1,3 +1,25 @@
+## [0.2.1] - 2026-02-19
+
+### Fixed
+- **Race condition in slug generation**: When two processes create records with the same base slug simultaneously, the second process now retries with a new random suffix instead of crashing with `RecordNotUnique`
+
+### Added
+- `around_create :retry_create_on_slug_unique_violation` for NOT NULL slug columns (pre-INSERT collision handling)
+- `with_slug_retry` helper with PostgreSQL-safe savepoints (`requires_new: true`)
+- `slug_column_not_null?` optimization to skip savepoint overhead for nullable slug columns
+- `slug_unique_violation?` detection supporting SQLite, PostgreSQL, and MySQL error formats
+
+### Changed
+- `set_slug` now uses shared retry handling with savepoints on `RecordNotUnique` for slug collisions (after_create path)
+- Retry exhaustion raises `RecordNotUnique` instead of falling back to timestamp suffix (fail-fast behavior)
+- `update_slug_if_nil` explicitly uses non-bang `save` to avoid exceptions on read operations
+
+### Known Limitations
+- **`around_create` retries rely on Rails internals**: The NOT NULL create retry path re-invokes the `around_create` callback chain. This behavior is covered by tests, but it is not explicitly documented as a public Rails callback contract.
+- **`before_create` callbacks will re-execute on retry**: If a retry is needed, `before_create` callbacks run again. Design callbacks to be idempotent or use guards.
+- **`RecordInvalid` is not retried**: Only DB-level `RecordNotUnique` triggers retry. A validation-layer uniqueness race that raises `RecordInvalid` will bubble up.
+- **Custom index names**: If your slug unique index has a non-standard name that doesn't contain `slug` or `_on_slug`, violations will bubble up instead of retrying.
+
 ## [0.2.0] - 2026-01-16
 
 - Added a full Minitest test suite

README.md

@@ -80,9 +80,53 @@ Product.first.slug
 
 If your model has a `slug` attribute in the database, `slugifiable` will automatically generate a slug for that model upon instance creation, and save it to the DB.
 
-> [!IMPORTANT]
-> Your `slug` attribute **SHOULD NOT** have `null: false` in the migration / database. If it does, `slugifiable` will not be able to save the slug to the database, and will raise an error like `ERROR:  null value in column "slug" of relation "posts" violates not-null constraint (PG::NotNullViolation)`
-> This is because records are created without a slug, and the slug is generated later.
+### Nullable vs NOT NULL Slug Columns
+
+**Nullable columns (default, simpler):**
+```ruby
+# migration
+add_column :products, :slug, :string
+add_index :products, :slug, unique: true
+
+# model
+class Product < ApplicationRecord
+  include Slugifiable::Model
+  generate_slug_based_on :name
+end
+```
+
+**NOT NULL columns (requires `before_validation` setup):**
+```ruby
+# migration
+add_column :products, :slug, :string, null: false
+add_index :products, :slug, unique: true
+
+# model
+class Product < ApplicationRecord
+  include Slugifiable::Model
+  generate_slug_based_on :name
+
+  before_validation :ensure_slug_present, on: :create
+
+  private
+
+  def ensure_slug_present
+    self.slug = compute_slug if slug.blank?
+  end
+end
+```
+
+> [!NOTE]
+> When using NOT NULL slug columns, `slugifiable` handles race conditions automatically. If two processes try to create records with the same slug simultaneously, the second one will retry with a new random suffix.
+
+> [!CAUTION]
+> For NOT NULL slug columns, retries re-run the `around_create` callback chain. If a slug collision happens, your `before_create` callbacks will run again. Keep `before_create` callbacks idempotent (or move side effects to `after_create`/background jobs).
+
+> [!NOTE]
+> Retry handling is DB-constraint-driven (`RecordNotUnique`), not validation-driven. A validation-layer race that raises `RecordInvalid` will bubble up.
+
+> [!NOTE]
+> Slug collision detection matches errors containing `slug`/`_on_slug`. If your index uses a custom name that does not include those patterns, retries will not trigger and the exception will bubble up.
 
 If you're generating slugs based off the model `id`, you can also set a desired length:
 ```ruby

lib/slugifiable/model.rb

@@ -33,11 +33,12 @@ module Model
     # 10^18 fits safely in a 64-bit integer
     MAX_NUMBER_LENGTH = 18
 
-    # Maximum number of attempts to generate a unique slug
-    # before falling back to timestamp-based suffix
+    # Maximum number of attempts to generate a unique slug before raising.
+    # Also used by generate_unique_slug for EXISTS? check loop (with timestamp fallback).
     MAX_SLUG_GENERATION_ATTEMPTS = 10
 
     included do
+      around_create :retry_create_on_slug_unique_violation
       after_create :set_slug
       after_find :update_slug_if_nil
       validates :slug, uniqueness: true
@@ -138,6 +139,53 @@ def compute_slug_based_on_attribute(attribute_name)
 
     private
 
+    # S1: around_create retry for NOT NULL slug columns (pre-INSERT collision)
+    # S2: Uses savepoint (requires_new: true) for PostgreSQL compatibility
+    # S4: Skips overhead when slug column is nullable (no INSERT-time collision possible)
+    def retry_create_on_slug_unique_violation
+      return yield unless slug_persisted? && slug_column_not_null?
+
+      with_slug_retry(for_insert: true) do |attempts|
+        # `around_create` retries can re-enter create callbacks. Set a fresh slug
+        # before retrying so pre-insert slug strategies don't reuse a collided value.
+        self.slug = compute_slug if attempts.positive?
+        yield
+      end
+    end
+
+    # S3: Shared retry helper with savepoints for both paths
+    # PostgreSQL requires savepoints: without them, retry fails with
+    # "current transaction is aborted, commands ignored until end"
+    def with_slug_retry(for_insert: false)
+      attempts = 0
+      begin
+        self.class.transaction(requires_new: true) { yield(attempts) }
+      rescue ActiveRecord::RecordNotUnique => e
+        raise unless slug_unique_violation?(e)
+        raise if for_insert && persisted? # Already inserted; don't retry whole create
+
+        attempts += 1
+        raise if attempts >= MAX_SLUG_GENERATION_ATTEMPTS # S6: Raise on exhaustion
+
+        retry
+      end
+    end
+
+    # S4: Optimization guard - skip savepoint wrapper when slug is nullable
+    def slug_column_not_null?
+      return false unless self.class.respond_to?(:columns_hash)
+      column = self.class.columns_hash["slug"]
+      column && !column.null
+    end
+
+    # S5: Detect slug unique violations across PostgreSQL/MySQL/SQLite
+    # Pattern matches: "slug" as word boundary OR "_on_slug" (index naming convention)
+    # Safe false-negative for custom index names (error bubbles up instead of silent retry)
+    def slug_unique_violation?(error)
+      message = error.message.to_s.downcase
+      message.match?(/\bslug\b|_on_slug\b/)
+    end
+
     def normalize_length(length, default, max)
       length = length.to_i
       return default if length <= 0
@@ -170,7 +218,7 @@ def generate_unique_slug(base_slug)
 
       # If we couldn't find a unique slug after MAX_SLUG_GENERATION_ATTEMPTS,
       # append timestamp + random to ensure uniqueness
-      if attempts == MAX_SLUG_GENERATION_ATTEMPTS
+      if attempts >= MAX_SLUG_GENERATION_ATTEMPTS
         slug_candidate = "#{base_slug}-#{Time.current.to_i}-#{SecureRandom.random_number(1000)}"
       end
 
@@ -218,15 +266,25 @@ def has_slug_method?
       self.class.method_defined?(:slug) || self.class.private_method_defined?(:slug)
     end
 
+    # S1: after_create retry for nullable slug columns (post-INSERT collision)
+    # S2: Uses savepoint via with_slug_retry for PostgreSQL compatibility
     def set_slug
       return unless slug_persisted?
+      return unless slug.blank?
 
-      self.slug = compute_slug if id_changed? || slug.blank?
-      self.save
+      with_slug_retry do |_attempts|
+        self.slug = compute_slug
+        save!
+      end
     end
 
+    # S7: Non-bang save for after_find repair path to avoid read-time exceptions
+    # This path handles legacy records that may have nil slugs
     def update_slug_if_nil
-      set_slug if slug_persisted? && self.slug.nil?
+      return unless slug_persisted? && slug.nil?
+
+      self.slug = compute_slug
+      save # Non-bang: read operations should not raise
     end
 
   end