Add MySQL support and multi-database CI testing

[rameerez]

Summary

This PR fixes GitHub issue #9 and adds comprehensive multi-database testing to ensure the gem works reliably with SQLite, PostgreSQL, and MySQL.

What triggered this

A user reported that running migrations on MySQL failed with:

Mysql2::Error: BLOB, TEXT, GEOMETRY or JSON column 'metadata' can't have a default value

This is because MySQL 8+ doesn't allow default values on JSON columns, but our migration used default: {}.

What we did

1. Fixed MySQL JSON column compatibility

Added a json_column_default helper in migrations that:

• Returns nil for MySQL (where JSON defaults aren't allowed)
• Returns {} for SQLite and PostgreSQL (where they work fine)

The models now handle nil metadata gracefully by defaulting to {} in their accessors.

2. Fixed metadata caching bug (19 test failures)

While implementing the MySQL fix, we discovered a caching bug where reload() didn't clear the cached metadata, causing stale data issues. Fixed by:

• Adding reload(*) override to clear @indifferent_metadata cache
• Adding before_save :sync_metadata_cache to persist in-place modifications

3. Added multi-database CI testing

Following the Pay gem's best practices, we added:

• SQLite job: Tests all Ruby versions (3.3, 3.4) and Pay versions (8.3-11.0)
• PostgreSQL job: Tests against PostgreSQL 16
• MySQL job: Tests against MySQL 8

Key CI decisions:

• Uses db:migrate:reset instead of db:test:prepare to avoid loading schema.rb which has SQLite-specific defaults
• Proper Docker service configuration with health checks
• DATABASE_URL automatically overrides database.yml (standard Rails behavior)

Pitfalls we avoided

1. schema.rb incompatibility: The auto-generated schema.rb has default: {} which fails on MySQL. We use db:migrate:reset in CI to run migrations instead of loading schema.

2. Metadata cache staleness: The @indifferent_metadata cache wasn't cleared on reload(), causing tests to see stale data after database updates.

3. In-place modification not persisting: Changes like metadata["key"] = "value" weren't being saved because they didn't trigger dirty tracking. Fixed with before_save callback.

Files changed

File	Purpose
lib/generators/.../create_usage_credits_tables.rb.erb	Added json_column_default helper
lib/usage_credits/models/wallet.rb	Metadata nil handling, cache fixes
lib/usage_credits/models/transaction.rb	Metadata nil handling, cache fixes
lib/usage_credits/models/fulfillment.rb	Metadata nil handling, cache fixes
.github/workflows/test.yml	Multi-database CI (SQLite, PostgreSQL, MySQL)
test/dummy/config/database.yml	Simplified to standard Rails pattern
test/dummy/db/migrate/...	Uses json_column_default helper
Gemfile	Added mysql2 gem for testing

Test results

All 700 tests pass across:

• Pay 8.3, 9.0, 10.0, 11.0
• Ruby 3.3, 3.4

Test plan

• All existing tests pass locally (700 tests, 0 failures)
• Tests pass on all Pay versions (8.3, 9.0, 10.0, 11.0)
• CI passes for SQLite job
• CI passes for PostgreSQL job
• CI passes for MySQL job (the main fix verification)

Closes

Closes #9

[claude]

PR Review: MySQL/PostgreSQL Compatibility and Multi-Database CI Testing

I've thoroughly reviewed this PR and I'm impressed with the comprehensive approach to fixing the MySQL compatibility issue and adding multi-database testing. Overall, this is high-quality work that addresses a critical bug while improving the gem's robustness.

---

✅ Strengths

1. Excellent Problem Analysis

The PR description clearly documents the root cause (MySQL 8+ JSON column default value restriction) and the cascading issues discovered during the fix. The metadata caching bug discovery shows thorough testing.

2. Smart Migration Helper

The json_column_default helper is a clean, database-agnostic solution:

def json_column_default
  return nil if connection.adapter_name.downcase.include?('mysql')
  {}
end

This follows the Rails convention of handling database differences in migrations rather than application code.

3. Comprehensive Metadata Caching Fix

The three-pronged approach to metadata handling is solid:

• metadata getter handles nil gracefully (super || {})
• reload(*) clears the cache to prevent stale data
• before_save :sync_metadata_cache persists in-place modifications

This pattern is consistently applied across Wallet, Transaction, and Fulfillment models.

4. Robust CI Setup

Following the Pay gem's best practices with separate jobs for SQLite, PostgreSQL, and MySQL is the right approach. The use of db:migrate:reset instead of db:test:prepare is crucial and well-documented.

5. Database-Specific JSON Query Handling

The PayChargeExtension fixes for JSON queries show good understanding of database differences:

• PostgreSQL: Uses @> operator for JSON containment
• MySQL: Uses CAST(? AS JSON) for proper JSON comparison
• SQLite: Uses 1 instead of true for boolean values

---

🔍 Observations & Suggestions

1. Test Coverage for Metadata Caching ⚠️

While the metadata caching fix is critical (fixed 19 test failures), I don't see explicit tests for the caching behavior itself. Consider adding tests like:

test "metadata cache is cleared on reload" do
  wallet = create(:wallet)
  wallet.metadata["key"] = "value"
  wallet.save\!
  
  # Simulate another process updating metadata
  wallet.class.find(wallet.id).update\!(metadata: { "key" => "new_value" })
  
  # Without clearing cache, this would return stale data
  assert_equal "new_value", wallet.reload.metadata["key"]
end

test "in-place metadata modifications are persisted" do
  wallet = create(:wallet)
  wallet.metadata["key"] = "value"
  wallet.save\!
  
  assert_equal "value", wallet.reload.metadata["key"]
end

These tests would document the fix and prevent regressions.

2. Schema Version Compatibility ℹ️

The schema version change from 8.0 to 7.2 is necessary for Rails 7.2 compatibility, but this means the schema file should probably be regenerated from Rails 7.2 rather than Rails 8 to ensure consistency. This is a minor point.

3. Database Adapter Detection Pattern 💭

The connection.adapter_name.downcase.include?('mysql') pattern is used in multiple places. Consider extracting this into a helper method for consistency:

def mysql_adapter?
  connection.adapter_name.downcase.include?('mysql')
end

def postgres_adapter?
  connection.adapter_name.downcase.include?('postgres')
end

This would make the code more maintainable if adapter detection logic needs to change.

4. Edge Case: Metadata = nil Before First Access ✅

The sync_metadata_cache callback correctly handles the case where metadata is never accessed before save:

elsif read_attribute(:metadata).nil?
  write_attribute(:metadata, {})

This prevents MySQL from rejecting inserts with NULL values. Good defensive programming!

5. Ruby 4.0 Testing ❓

I notice Ruby 4.0 testing was added back in commit 7dac4e5. Ruby 4.0 doesn't exist yet (current stable is 3.4). Was this meant to be Ruby 3.4 or is this testing against a pre-release version?

---

🐛 Potential Issues

1. Race Condition in Metadata Caching ⚠️

The current implementation has a theoretical race condition:

def metadata
  @indifferent_metadata ||= ActiveSupport::HashWithIndifferentAccess.new(super || {})
end

If two threads call metadata simultaneously before the cache is set, they could get different hash instances. In practice, this is unlikely to cause issues since Rails models aren't thread-safe by default, but it's worth noting.

A safer approach might be:

def metadata
  @indifferent_metadata ||= begin
    ActiveSupport::HashWithIndifferentAccess.new(read_attribute(:metadata) || {})
  end
end

Using read_attribute instead of super makes the caching more explicit.

2. Metadata Setter Doesn't Mark as Dirty ℹ️

The metadata setter clears the cache but doesn't explicitly mark the attribute as changed:

def metadata=(hash)
  @indifferent_metadata = nil  # Clear cache
  super(hash.is_a?(Hash) ? hash.to_h : {})
end

This is fine because super handles dirty tracking, but it's subtle. Consider adding a comment explaining this.

---

🔒 Security Considerations

✅ No security concerns identified. The changes are purely infrastructure/compatibility improvements with no user-facing attack surface.

---

🎯 Performance Considerations

Minor Concern: Metadata Caching Overhead

The before_save callback adds a small overhead to every save operation:

def sync_metadata_cache
  if @indifferent_metadata
    write_attribute(:metadata, @indifferent_metadata.to_h)
  elsif read_attribute(:metadata).nil?
    write_attribute(:metadata, {})
  end
end

This is negligible in practice, but for high-throughput scenarios with many saves, this could be optimized to only run when metadata is actually used:

before_save :sync_metadata_cache, if: -> { defined?(@indifferent_metadata) }

The current implementation is fine for this gem's use case.

---

📋 Test Plan Assessment

✅ Comprehensive test coverage:

• All 700 tests pass across Pay versions 8.3, 9.0, 10.0, 11.0
• Multi-database CI (SQLite, PostgreSQL, MySQL)
• Ruby 3.3, 3.4 compatibility

Suggestion: Consider adding a test that actually runs migrations on MySQL to verify the json_column_default fix works as intended.

---

📚 Documentation

✅ The PR description is excellent - it clearly documents:

• The problem and root cause
• The solution and approach
• Pitfalls avoided
• Files changed with purposes

Minor suggestion: Consider adding a note in the README or migration template comments about MySQL JSON column limitations for future maintainers.

---

✨ Summary

This is high-quality work that:

1. ✅ Fixes a critical MySQL compatibility bug
2. ✅ Improves test coverage with multi-database CI
3. ✅ Fixes a metadata caching bug discovered during testing
4. ✅ Follows Rails conventions and best practices
5. ✅ Has comprehensive documentation

Recommended Actions Before Merge:

1. Add explicit tests for metadata caching behavior (high priority)
2. Clarify Ruby 4.0 vs 3.4 in CI matrix (medium priority)
3. Consider extracting database adapter detection helpers (low priority, nice-to-have)

Overall Assessment: ✅ Approved with minor suggestions

Great work fixing a nasty MySQL compatibility issue and improving the gem's robustness! The multi-database testing will catch these issues early in the future.

---

Test plan status:

• All existing tests pass (700 tests, 0 failures)
• Tests pass on all Pay versions (8.3, 9.0, 10.0, 11.0)
• CI passes for SQLite job
• CI passes for PostgreSQL job
• CI passes for MySQL job

Would benefit from:

• Explicit metadata caching tests
• Migration test on MySQL in CI