MySQL adapter #246

andrewmarkle · 2025-11-07T14:57:23Z

Add MySQL Support

This PR adds full MySQL support enabling the gem to work with both mysql2 and trilogy adapters alongside the existing SQLite support.

This is a bit of a big PR. If you prefer I would happily split this up into smaller chunks:

Get test infrastructure ready to support multiple database adapters
Add devcontainers that can run mysql
Add mysql adapter / tests

What's New

MySQL Database Adapter

Implemented ActiveRecord::Tenanted::DatabaseAdapters::MySQL with full feature parity to the SQLite adapter
Registered both mysql2 and trilogy adapters in the adapter registry

Devcontainer support

To make installing a mysql db and running the tests easy. No need to mess about with homebrew.

Testing Infrastructure

Added comprehensive MySQL test scenarios covering:
- Primary database with tenanted models
- Primary named database configuration
- Secondary database configuration
- All migration and schema testing scenarios
Updated integration test script (bin/test-integration) to:
- Support MySQL-specific environment variables (MYSQL_UNIQUE_PREFIX, MYSQL_HOST)
- Generate unique database names per test run to avoid conflicts
- Handle database dropping in MySQL
Added MySQL 8.0 service to CI workflow for both unit and integration tests

Dependencies

Added mysql2 gem to development/test dependencies in Gemfile

Technical Details

The MySQL adapter implements the same interface as the SQLite adapter but with MySQL-specific behavior:

Tenant Discovery: Uses SQL pattern matching (SHOW DATABASES LIKE 'pattern') instead of file globbing
Database Names: Validates against MySQL's naming restrictions (max 64 characters, alphanumeric + underscore/$/hyphen only)
Connection Management: Uses temporary connections without database (which is very similar to how Rails does this)

Testing

All existing tests should pass with both SQLite and MySQL adapters. The integration test suite now runs scenarios against both database types, ensuring feature parity.

To test locally I've been using the devcontainer.

devcontainer up --workspace-folder .
devcontainer exec --workspace-folder . bundle

# then you should be able to run the tests
devcontainer exec --workspace-folder . bin/test-unit
devcontainer exec --workspace-folder . bin/test-integration

Known Limitations

There are some features still missing to fully support MySQL. The main one is supporting the use-case for using a DATABASE_URL to set everything up. In theory this should just work but I haven't tested yet and want to fully test this in the gem. I'll open a PR for this later!

Breaking Changes

None. This is purely additive functionality.

it from the tasks)

This ensures the adapter actually broadcasts over the WebSocket connection within the same process, which is what system tests need.

flavorjones · 2025-11-09T20:02:52Z

I rebased onto current main. Reviewing now!

flavorjones

Oh my gosh, this PR is great, thank you for the huge contribution!

I have two primary reservations:

the devcontainer setup seems very particular for your needs (v8.0 and not v8.4 or v9.0, and mounting /var/lib/mysql) and I'd like to move that to a separate PR or discussion if that's OK.
parallel testing doesn't work for me -- does it work for you?

On (2), in particular, I tend to run the unit test suite highly parallelized (~20 workers). But even running with NCPU=2 for two workers raises exceptions:

ActiveRecord::Tenanted::Tenant::#cache_key::scenario::mysql/primary_named_db::secondary_record::created in tenanted context#test_includes_the_tenant_name:
ActiveRecord::StatementInvalid: Mysql2::Error: Table 'schema_migrations' already exists

Just curious if this is something that doesn't happen to you, in which case I'll dig in.

I've made quite a few comments in here, but many of them are "to-dos" and don't necessarily need to be fixed in this PR.

I'm going to spend some time in the morning appending small changes to this branch in response to some of my comments. Maybe we can collaborate a bit on getting this merged?

Thank you again!!! ❤️❤️🙏

flavorjones · 2025-11-09T20:26:09Z

.devcontainer/Dockerfile

@@ -0,0 +1,15 @@
+FROM ghcr.io/rails/devcontainer/images/ruby:3.3.5


Is devcontainer introduced only to run the mysql server? This container is pretty big and has other utilities and libraries being installed that I don't think the test suite needs.

Why not just use the mysql docker image? I can imagine having a bin/setup script that runs something like:

docker run -d \ --restart unless-stopped \ -p "127.0.0.1:3306:3306" \ --name=mysql84 \ -e MYSQL_ROOT_PASSWORD= -e MYSQL_ALLOW_EMPTY_PASSWORD=true \ mysql:8.4

My goal was to just make setting up mysql easy and be able to run all the tests with this. To do that you also need chrome and maybe these other things...? (I'll have to verify!)

That said though we could definitely do a simpler thing though. I'll take a look at this!

flavorjones · 2025-11-09T20:27:55Z

.github/workflows/ci.yml

    runs-on: ubuntu-latest
+    services:
+      mysql:
+        image: mysql:8.0


Is it worth setting up a matrix to test against 8 and 9? And maybe even patch releases like 8.4?

We could! I'm not that worried that we're doing anything too novel that would break b/w versions of mysql though. Probably testing against the latest version is good enough 🤷‍♂️. But I'm very open to doing that if you think it's a good idea.

We're really only doing "SHOW DATABASES LIKE '#{database_path}' and DROP DATABASE IF EXISTS which will work across all versions. The rest of the stuff we're just calling the built-in Rails methods which are already well tested. Just not too worried about it I guess!

flavorjones · 2025-11-09T20:46:59Z

bin/test-integration

  puts "Creating integration app for #{COLOR_FG_BLUE}#{scenario}#{COLOR_RESET} at #{app_path}"

+  # Generate a unique database prefix for this scenario to avoid conflicts
+  db_prefix = "test_#{scenario[:database]}_#{scenario[:models]}_#{Process.pid}".gsub(/[^a-zA-Z0-9_]/, "_")


I'm curious what conflicts you're avoiding with this that DatabaseConfiguration#test_worker_id= isn't sufficient to prevent. Can you explain a bit more?

I was having a bunch of contention issues before this when running integration tests in parallel.

In SQLite each test run uses a unique temporary directory which means that multiple test runs never touched the same database file.

But with MySQL it's different b/c we have the same shared server. What this means is that all parallel test runs originally used the same database name which caused collisions and contention. Different tests were trying to drop, create, or migrate using the same schema.

There are different ways we could set this up but I just went with an env variable that gets evaluated at runtime (<%= ENV.fetch('MYSQL_DB_PREFIX', 'primary') %>) from the database.yml. This happens when we db:drop, db:prepare, etc. and just ensures that each test database is unique from any others.

I actually didn't realize you could run the unit tests in parallel and I didn't implement something similar for them. But the problem you're seeing running the unit tests is the same problem that this was supposed to address.

flavorjones · 2025-11-09T21:06:36Z

bin/test-integration

-      # create a fake tenant database to validate that it is deleted in the test suite
-      dev_db = Dir.glob("storage/development/*/development-tenant/main.sqlite3").first
-      test_db = dev_db.gsub("/development/", "/test/").gsub("development-tenant", "delete-me")
-      FileUtils.mkdir_p(File.dirname(test_db))
-      FileUtils.touch(test_db)
-


Why is this being removed? This test database needs to be here to make this test meaningful:

https://github.com/basecamp/activerecord-tenanted/blob/main/test/integration/test/testcase_test.rb#L12-L15

And I think the mysql implementation also needs to delete existing tenant databases at the start of the test suite, though you may need to change this setup to something that will work for both databases, maybe:

run_cmd(scenario, "bin/rails db:migrate", env: { ARTENANT: "delete-me" })

I may have misunderstood what this does. I just added in a db:drop at the start of the test run which I thought was a bit simpler / handles all database types. But the test you mentioned would be redundant then: https://github.com/basecamp/activerecord-tenanted/blob/main/test/integration/test/testcase_test.rb#L12-L15.

We can keep this in if you want! But we still need to db:drop with mysql just to ensure that we're starting from a clean slate every time.

flavorjones · 2025-11-09T21:09:42Z

bin/test-integration


-      # set up the database and schema files
-      run_cmd(scenario, "bin/rails db:prepare")
+     adapter_config = {"RAILS_ENV" => "test","ARTENANT_SCHEMA_DUMP" => "1"}


On main, the integration test suite runs rails db:prepare in the development environment, and then the tests get run in the test environment (implicitly setting up the databases).

Can you say a bit more about this change?

I added this in b/c I kept getting this error when trying to run any database tasks.

bin/rails db:drop (1.89s) bin/rails aborted! ActiveRecord::EnvironmentMismatchError: You are attempting to modify a database that was last run in `test` environment. (ActiveRecord::EnvironmentMismatchError) You are running in `development` environment. If you are sure you want to continue, first set the environment using: bin/rails db:environment:set RAILS_ENV=development Tasks: TOP => db:drop => db:check_protected_environments (See full trace by running task with --trace) ERROR: Command failed

But I could easily be doing something wrong!

flavorjones · 2025-11-09T22:07:26Z

lib/active_record/tenanted/database_tasks.rb

+          rescue ActiveRecord::NoDatabaseError
+            database_already_initialized = false
+          end


It doesn't feel like we should be rescuing this here. If you know this is a condition that will often happen, where table_exists? should return false, I'd prefer if we either fixed the method upstream and/or patched the method.

Looking at table_exists? I'm not sure if this error is raised by ConnectionPool#with_connection or ConnectionAdapter#data_source_exists?, do you know?

flavorjones · 2025-11-09T22:21:06Z

lib/active_record/tenanted/database_tasks.rb

          # dump the schema and schema cache
          if Rails.env.development? || ENV["ARTENANT_SCHEMA_DUMP"].present?
-            if migrated
+            if migrated || !database_already_initialized


I don't think we need to add the check for !database_already_initialized, unless I'm misunderstanding the intent. The tests all pass without it.

flavorjones · 2025-11-09T22:22:24Z

test/integration/test/turbo_broadcast_test.rb

+    rescue Capybara::ElementNotFound
+      sleep 0.1


Why is this method rescuing and sleeping? Seems like it should raise an exception if the assert_selector failed.

flavorjones · 2025-11-09T22:31:03Z

test/unit/database_configurations_test.rb

+      end
+    end
+
+    describe "new_tenant_config" do


Oof, this made me realize how little test coverage the gem has for this method. 😬

flavorjones · 2025-11-09T22:47:50Z

test/unit/database_configurations_test.rb

+        test "only returns tenants from tenanted databases, not shared databases" do
+          assert_empty(base_config.tenants)
+
+          TenantedApplicationRecord.create_tenant("foo")
+
+          assert_equal([ "foo" ], base_config.tenants)
+
+          all_databases = ActiveRecord::Base.configurations.configs_for(env_name: base_config.env_name)
+
+          untenanted_config = all_databases.reject { |c| c.configuration_hash[:tenanted] }
+          untenanted_config.each do |shared_config|
+            assert_not_includes(base_config.tenants, shared_config.database)
+          end
+        end


🤔 This test implicitly depends on the naming in the mysql scenarios' database.yml files, so it's not testing anything under sqlite.

Instead, I'd like to rewrite this test to explicitly modify ActiveRecord::Base.configurations so there is an additional untenanted config that is named like a tenant.

andrewmarkle · 2025-11-10T12:38:08Z

Thanks Mike! Feel free to make any changes as you see fit. I won’t have time to go through the feedback today but I will circle back this week. 👌

flavorjones · 2025-11-10T22:59:27Z

@andrewmarkle I'm dealing with a fire drill at work, so I may not have much time this week. I'm very keen to land this so it's at the top of my "OSS to-do list". 🙌

andrewmarkle marked this pull request as draft November 7, 2025 15:08

andrewmarkle changed the title ~~Mysql adapter~~ MySQL adapter Nov 7, 2025

andrewmarkle marked this pull request as ready for review November 7, 2025 16:58

andrewmarkle added 27 commits November 9, 2025 12:58

Move tests into adapter folders

98169fd

Add mysql and a devcontainer to easily run it

ea0e14d

Add mysql adapter with tests

8466298

Simplify devcontainer setup

d975bd4

Clean up the mysql adapter

bbd4e53

Refactor

4445329

Update integration script

1b87d51

Add adapter folder and get sqlite integration tests to pass

1c6eda2

Add chrome to devcontainer so that we can run integration tests in it

94e6d86

A couple fixes. Switch to mysql

132e9c6

Fix database tasks

9da4474

Fix tests for sqlite

ef0819d

Make fake database creation work for all adapters

5929062

shorten the db name so it's shorter than the mysql max (64)

e980a15

Working(ish) integration tests with mysql

6c9121b

Specify RAILS_ENV

1b147f0

Refactor and try to make test_workerize work

ce9f288

Isolate the dbs for integration tests

7b28b4b

Make sure dbs start from a clean state everytime

f69a5c1

Enable integration tests for mysql and sqlite

bda859a

Add mysql schema_cache and make sure the unit tests use it

7152457

Refactor cleaning up databases in unit tests

d47aa8c

Rename methods

8418c24

Simplify SystemTest setup

31fe774

Revert un-needed change

e004cd9

More cleanup

b22fa30

Simplify

6f46023

andrewmarkle added 20 commits November 9, 2025 15:02

Add test to ensure shared dbs don't get passed to adapter

798c40d

Add tests to ensure we get a valid mysql database name

84fd830

Add tests for when a host option is provided and more cleanup

bc76b0e

Move comment

1027b04

Refactor test-integration setup for mysql

8ce81ff

Simplify migration logic

bb2b311

Improve the test coverage of database_tasks

6431651

Refactor and clean up

554f378

Run both types of adapters in the integration tests

c9c421a

Reject the connection if the tenant is an empty string

c47579c

Configure CI to add mysql

18ca09c

Align testing adapters more closely and add more test scenarios to mysql

cb16d87

Fix test and ensure temp connection is more obviously set (and exclude

2e8b21c

it from the tasks)

Ensure unit test only runs for sqlite

3c055d5

Fix mismatch in system tests for action cable setup

eabf8c8

This ensures the adapter actually broadcasts over the WebSocket connection within the same process, which is what system tests need.

Try explicitly busting the cache

ed0539c

Updo the cache busting and add some debug info

76c2acf

Try this

41e1f23

Try to fix the debug methods

745df5d

Cleanup fix for test

d238e6b

flavorjones force-pushed the mysql-adapter-with-polish branch from 11fde4d to d238e6b Compare November 9, 2025 20:02

flavorjones requested changes Nov 9, 2025

View reviewed changes

andrewmarkle marked this pull request as draft November 14, 2025 15:51

Refactor temp_connection name

d8d75c9

		@@ -0,0 +1,15 @@
		FROM ghcr.io/rails/devcontainer/images/ruby:3.3.5

MySQL adapter #246

Are you sure you want to change the base?

MySQL adapter #246

Uh oh!

Conversation

andrewmarkle commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add MySQL Support

What's New

Technical Details

Testing

Known Limitations

Breaking Changes

Uh oh!

flavorjones commented Nov 9, 2025

Uh oh!

flavorjones left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewmarkle Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewmarkle commented Nov 10, 2025

Uh oh!

flavorjones commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andrewmarkle commented Nov 7, 2025 •

edited

Loading

andrewmarkle Nov 14, 2025 •

edited

Loading