Skip to content

Conversation

groyoh
Copy link
Contributor

@groyoh groyoh commented Jul 4, 2025

Context

events table cleanup currently does not work properly if the transaction strategy is used. This is because DatabaseCleaner will only apply the strategy on the default database connection. Since events is using a separate connection, the connection will not be rolled back.

Take the following spec:

require "rails_helper"

RSpec.describe DatabaseCleaner do
  it "has zero events" do
    expect(Event.count).to eq(0)

    create(:event)

    expect(Event.count).to eq(1)
  end

  it "also has zero events" do
    # This should work if each example is wrapped in a transaction
    expect(Event.count).to eq(0)
  end
end

This will fail with the following error:

> lago exec api bundle exec rspec --color --format documentation spec/database_cleaner_spec.rb

DatabaseCleaner
  has zero events
  also has zero events (FAILED - 1)

Failures:

  1) DatabaseCleaner also has zero events
     Failure/Error: expect(Event.count).to eq(0)

       expected: 0
            got: 1

       (compared using ==)
     # ./spec/database_cleaner_spec.rb:16:in 'block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:160:in 'block (3 levels) in <top (required)>'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/strategy.rb:30:in 'DatabaseCleaner::Strategy#cleaning'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:34:in 'block (2 levels) in DatabaseCleaner::Cleaners#cleaning'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:35:in 'DatabaseCleaner::Cleaners#cleaning'
     # ./spec/spec_helper.rb:159:in 'block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in 'block (2 levels) in <main>'

Finished in 0.58037 seconds (files took 2.43 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/database_cleaner_spec.rb:14 # DatabaseCleaner also has zero events

The only reason it works with deletion strategy is because events table is within the same database as the default (primary) connection and gets deleted/truncated when DatabaseCleaner cleans the default database.

Description

How to fix it ?

The fix is to define a specific strategy for events database:

DatabaseCleaner[:active_record, db: EventsRecord].strategy = :transaction

Note that we have to use EventsRecord instead of events as db. The reason is that DatabaseCleaner tries to retrieve the first active record class whose connection is matching the database name. This will be ApplicationRecord since its connection (primary/default) uses the same database name as the events connection. Then DatabaseCleaner will use the connection from that class to handle the cleanup. In this case, the cleanup (transaction) will happen on the default database instead of the events. See https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/912acbe1facddfe1b14f21107f6ddc9df290de53/lib/database_cleaner/active_record/base.rb#L82.

Failing tests

Fixing the configuration caused a few tests to fail as they were relying on the creation of events. Those events used to be created without a transaction so the tests worked fine. Now we need to set transaction: false for those tests in order for the events to be saved to the database as the default strategy for events is now transaction.

Performance impact

In order to compare the impact of the deletion on the spec performance, I ran the following command on main and this branch:

lago exec api bundle exec rspec spec/scenarios/fees/recurring_fees_spec.rb spec/scenarios/invoices/advance_charges_spec.rb spec
/scenarios/invoices/invoices_spec.rb spec/services/billable_metrics/breakdown/unique_count_service_spec.rb spec/services/events/stores/postgres_store_spec.rb sp
ec/services/fees/charge_service_spec.rb spec/services/invoices/calculate_fees_service_spec.rb spec/services/invoices/preview_service_spec.rb spec/scenarios/charge_models/prorated_graduated_spec.rb

The results I got:

  • On main: 49.94 seconds
  • On this branch: 55.92 seconds

If the performance impact is/become unacceptable, an alternative could be to use the null strategy or use a top-level after(:context) { DatabaseCleaner[:active_record, db: EventsRecord].clean_with(:deletion) } for those tests.

@groyoh groyoh marked this pull request as ready for review July 4, 2025 08:59
`events` database cleanup currently does not work properly if the `transaction` is used. This is because `DatabaseCleaner` will only apply the strategy on the default database connection. Since `events` is using a separate connection, the connection will not be rolled back.

Take the following spec:

```ruby
require "rails_helper"

RSpec.describe DatabaseCleaner do
  it "has zero events" do
    expect(Event.count).to eq(0)

    create(:event)

    expect(Event.count).to eq(1)
  end

  it "also has zero events" do
    # This should work if each example is wrapped in a transaction
    expect(Event.count).to eq(0)
  end
end
```

This will fail with the following error:

```shell
⋊> lago exec api bundle exec rspec --color --format documentation spec/database_cleaner_spec.rb

DatabaseCleaner
  has zero events
  also has zero events (FAILED - 1)

Failures:

  1) DatabaseCleaner also has zero events
     Failure/Error: expect(Event.count).to eq(0)

       expected: 0
            got: 1

       (compared using ==)
     # ./spec/database_cleaner_spec.rb:16:in 'block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:160:in 'block (3 levels) in <top (required)>'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/strategy.rb:30:in 'DatabaseCleaner::Strategy#cleaning'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:34:in 'block (2 levels) in DatabaseCleaner::Cleaners#cleaning'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:35:in 'DatabaseCleaner::Cleaners#cleaning'
     # ./spec/spec_helper.rb:159:in 'block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in 'block (2 levels) in <main>'

Finished in 0.58037 seconds (files took 2.43 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/database_cleaner_spec.rb:14 # DatabaseCleaner also has zero events
```

The only reason why it works with `deletion` strategy is because `events` table are within the same database as the `default` connection and gets deleted/truncated when `DatabaseCleaner` cleans the `default` database.

The fix is to use define a specific strategy for `events` database:

```ruby
DatabaseCleaner[:active_record, db: EventsRecord].strategy = :transaction
```

Note that we have to use `EventsRecord` instead of `events` as `db`. The reason is that `DatabaseCleaner` tries to retrieve the first active record class whose connection is matching the database name. This will be `ApplicationRecord` since it uses the same database configuration as `events`. Then `DatabaseCleaner` will use the connection from that class to handle the cleanup. In this case, the cleanup will happen on the `default` database instead of the `events`. See https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/912acbe1facddfe1b14f21107f6ddc9df290de53/lib/database_cleaner/active_record/base.rb#L82.
@groyoh groyoh merged commit 95004ed into main Jul 7, 2025
14 checks passed
@groyoh groyoh deleted the fix/db-cleaner branch July 7, 2025 10:13
diegocharles pushed a commit that referenced this pull request Jul 11, 2025
## Context

`events` table cleanup currently does not work properly if the `transaction` strategy is used. This is because `DatabaseCleaner` will only apply the strategy on the default database connection. Since `events` is using a separate connection, the connection will not be rolled back.

Take the following spec:

```ruby
require "rails_helper"

RSpec.describe DatabaseCleaner do
  it "has zero events" do
    expect(Event.count).to eq(0)

    create(:event)

    expect(Event.count).to eq(1)
  end

  it "also has zero events" do
    # This should work if each example is wrapped in a transaction
    expect(Event.count).to eq(0)
  end
end
```

This will fail with the following error:

```shell
⋊> lago exec api bundle exec rspec --color --format documentation spec/database_cleaner_spec.rb

DatabaseCleaner
  has zero events
  also has zero events (FAILED - 1)

Failures:

  1) DatabaseCleaner also has zero events
     Failure/Error: expect(Event.count).to eq(0)

       expected: 0
            got: 1

       (compared using ==)
     # ./spec/database_cleaner_spec.rb:16:in 'block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:160:in 'block (3 levels) in <top (required)>'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/strategy.rb:30:in 'DatabaseCleaner::Strategy#cleaning'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:34:in 'block (2 levels) in DatabaseCleaner::Cleaners#cleaning'
     # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:35:in 'DatabaseCleaner::Cleaners#cleaning'
     # ./spec/spec_helper.rb:159:in 'block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in 'block (2 levels) in <main>'

Finished in 0.58037 seconds (files took 2.43 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/database_cleaner_spec.rb:14 # DatabaseCleaner also has zero events
```

The only reason it works with `deletion` strategy is because `events` table is within the same database as the `default` (`primary`) connection and gets deleted/truncated when `DatabaseCleaner` cleans the `default` database.

## Description

**How to fix it ?**

The fix is to define a specific strategy for `events` database:

```ruby
DatabaseCleaner[:active_record, db: EventsRecord].strategy = :transaction
```

Note that we have to use `EventsRecord` instead of `events` as `db`. The reason is that `DatabaseCleaner` tries to retrieve the first active record class whose connection is matching the database name. This will be `ApplicationRecord` since its connection (`primary`/`default`) uses the same database name as the `events` connection. Then `DatabaseCleaner` will use the connection from that class to handle the cleanup. In this case, the cleanup (`transaction`) will happen on the `default` database instead of the `events`. See https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/912acbe1facddfe1b14f21107f6ddc9df290de53/lib/database_cleaner/active_record/base.rb#L82.

**Failing tests**

Fixing the configuration caused a few tests to fail as they were relying on the creation of `events`. Those events used to be created without a transaction so the tests worked fine. Now we need to set `transaction: false` for those tests in order for the events to be saved to the database as the default strategy for `events` is now `transaction`.

**Performance impact**

In order to compare the impact of the deletion on the spec performance, I ran the following command on `main` and this branch:

```shell
lago exec api bundle exec rspec spec/scenarios/fees/recurring_fees_spec.rb spec/scenarios/invoices/advance_charges_spec.rb spec
/scenarios/invoices/invoices_spec.rb spec/services/billable_metrics/breakdown/unique_count_service_spec.rb spec/services/events/stores/postgres_store_spec.rb sp
ec/services/fees/charge_service_spec.rb spec/services/invoices/calculate_fees_service_spec.rb spec/services/invoices/preview_service_spec.rb spec/scenarios/charge_models/prorated_graduated_spec.rb
```

The results I got:

* On `main`: 49.94 seconds
* On this branch: 55.92 seconds

If the performance impact is/become unacceptable, an alternative could be to use the `null` strategy or use a top-level `after(:context) { DatabaseCleaner[:active_record, db: EventsRecord].clean_with(:deletion) }` for those tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants