Skip to content

improvement: passing tenant in the transaction reason - #2917

Merged
zachdaniel merged 2 commits into
ash-project:mainfrom
C-Sinclair:improvement/tenant-in-transaction-reason
Sep 9, 2026
Merged

improvement: passing tenant in the transaction reason#2917
zachdaniel merged 2 commits into
ash-project:mainfrom
C-Sinclair:improvement/tenant-in-transaction-reason

Conversation

@C-Sinclair

@C-Sinclair C-Sinclair commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

A data layer that picks its connection per tenant needs the tenant in c:Ash.DataLayer.transaction/4. The reason Ash builds names the resource, the action and the actor, but not the tenant.

  • Ash.DataLayer.transaction_reason/0 gains optional(:tenant) => term() on every variant that already declares optional(:data_layer_context). The key is optional, so a data layer that ignores it is unaffected.
  • Ash.Changeset puts changeset.tenant on the reason in Ash.Changeset.with_hooks/3. That one site covers create, update and destroy.
  • Ash.Actions.Create.Bulk, Ash.Actions.Update.Bulk and Ash.Actions.Destroy.Bulk pass opts[:tenant] at both their outer and inner transactions. Ash.Actions.Read passes query.tenant.
  • Ash.Actions.Update.UpdateMany gains :tenant and :data_layer_context. It built the only reason that had neither.

The tenant sits beside :data_layer_context rather than inside it, so a data layer reads a named key instead of a convention:

def transaction(_resource, fun, _timeout, %{tenant: tenant}) do
  Repo.put_dynamic_repo(repo_for_tenant(tenant))
  Repo.transaction(fun)
end

Where this came from

ash_sqlite#224 adds strategy :context to AshSqlite, giving each tenant its own SQLite file. It carries AshSqlite.Changes.CarryTenant and AshSqlite.Transformers.CarryTenant only to put the tenant where transaction/4 can read it. @zachdaniel asked why that was necessary and suggested Ash set it in the transaction metadata. Both modules are deleted there once this lands.

With the transformer disabled, create, update, destroy, bulk_create and bulk_update all reach transaction/4 with data_layer_context: %{} and no tenant. Reads were the only path already carrying it, on query.

Not in this PR

No test asserts the new key. Of the built-in data layers only Ash.DataLayer.Mnesia implements transaction/4, and it does not read the reason, so a test needs a data layer that records what it is given. Ash.DataLayer.Ets answers false to :transact and implements neither rollback/2 nor in_transaction?/1, so wrapping it means writing a transaction implementation too. I can add such a module under test/support if you want the coverage here rather than in the data layer that consumes it.

A data layer that chooses its connection per tenant needs the tenant in
`c:Ash.DataLayer.transaction/4`. Ash calls that callback above the data layer, and
the reason it builds names the resource, the action and the actor, but not the
tenant. A database-per-tenant data layer therefore has nothing to select a
connection with, and has to smuggle the tenant into the changeset context with a
change of its own before the transaction opens.

`Ash.Changeset` already forwards `changeset.context[:data_layer]` to the callback
as `:data_layer_context`, so the delivery path exists. What is missing is that
nothing puts the tenant into it. This adds `:tenant` to the reason instead, beside
the context rather than inside it, so it is named rather than found:

  * `Ash.Changeset` sets `changeset.tenant`, which covers create, update and
    destroy.
  * `Ash.Actions.Create.Bulk`, `Ash.Actions.Update.Bulk` and
    `Ash.Actions.Destroy.Bulk` set `opts[:tenant]`, at both their outer and inner
    transactions.
  * `Ash.Actions.Update.UpdateMany` gains `:tenant` and `:data_layer_context`. It
    built the only reason with neither.
  * `Ash.Actions.Read` sets `query.tenant`. A read already carried its query, so
    the tenant was reachable there, and naming it keeps every reason uniform.

`Ash.DataLayer.transaction_reason/0` declares `optional(:tenant) => term()` on each
variant that already declares `optional(:data_layer_context)`. The key is optional,
so a data layer that ignores it is unaffected.

This also gives `Ash.transaction/2` somewhere to put a tenant, should it take one.
C-Sinclair added a commit to C-Sinclair/ash_sqlite that referenced this pull request Sep 8, 2026
Depends on ash-project/ash#2917, which adds `:tenant` to the transaction reason.
Until that merges, the nine tests that reach `transaction/4` through an Ash action
fail with "carried no tenant". Every other test passes.

`reason_tenant/1` becomes `reason[:tenant]`. It previously looked in three places,
because no path named the tenant and each forwarded something different: the
single-record paths a data layer context, the bulk paths a whole changeset
context, and a read its query.

`AshSqlite.Changes.CarryTenant` and `AshSqlite.Transformers.CarryTenant` are
deleted. They existed only to put the tenant somewhere `transaction/4` could reach
it, which is what @zachdaniel objected to, and Ash naming it removes the reason for
them to exist.

`without_binder/3` goes too. It raised when a `strategy :context` resource had no
binder, which `AshSqlite.Verifiers.VerifyTenantBinder` already fails the compile
for. Its branch stays as `fun.()`, because a resource with multitenancy this data
layer does not resolve to a connection, such as `strategy :attribute`, reaches
that clause with a tenant and nothing to bind.
@C-Sinclair C-Sinclair changed the title improvement: name the tenant in the transaction reason improvement: passing tenant in the transaction reason Sep 8, 2026
@C-Sinclair
C-Sinclair marked this pull request as ready for review September 8, 2026 14:00
type: :bulk_update,
metadata: %{resource: resource, action: action.name},
tenant: opts[:tenant],
data_layer_context: opts[:data_layer_context] || %{}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were also missing data_layer_context for Update Many, so I've added it here as well

@zachdaniel

Copy link
Copy Markdown
Contributor

I think we will also need to add the tenant option to Ash.transact and Ash.transaction

  @transaction_opts_schema [
    timeout: [
      type: :timeout,
      doc: """
      The time in milliseconds (as an integer) to wait for the transaction to finish or `:infinity` to wait indefinitely.

      If not specified then default behaviour is adapter specific - for `Ecto`-based data layers it will be `15_000`.
      """
    ],
    return_notifications?: [
      type: :boolean,
      default: false,
      doc: """
      Use this if you want to manually handle sending notifications.

      If true the returned tuple will contain notifications list as the last element.

      To send notifications use `Ash.Notifier.notify(notifications)`. It sends any notifications that can be sent, and returns the rest.
      """
    ]
  ]

and thread it through.

…easons

`Ash.transact/3` and `Ash.transaction/3` build the `:custom` transaction reason
and took no tenant, so a data layer that chooses its connection per tenant got
nothing from a hand-written transaction. `@transaction_opts_schema` gains
`:tenant`, typed `{:protocol, Ash.ToTenant}` like every other tenant option, and
both functions put it on the reason.

`Ash.Actions.Action.run/4` builds the `:generic` reason for a generic action with
`transaction? true`. It named the actor and the data layer context but not the
tenant, so it now sets `input.tenant`.

`Ash.DataLayer.transaction_reason/0` declares `optional(:tenant) => term()` on the
`:custom` variant and on the catch-all variant. The six variants that declare
`optional(:data_layer_context)` already had it.
@zachdaniel
zachdaniel merged commit f1602c4 into ash-project:main Sep 9, 2026
48 of 51 checks passed
@zachdaniel

Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

C-Sinclair added a commit to C-Sinclair/ash_sqlite that referenced this pull request Sep 9, 2026
ash-project/ash#2917 merged as f1602c4 and puts `:tenant` on the transaction
reason, which `AshSqlite.DataLayer.transaction/4` reads. It is not in a release
yet: v3.33.1 was cut the day before it merged.

The nine tests that reach `transaction/4` through an Ash action now pass, and
the whole suite is green at 203.

`ash_version/1` still takes ASH_VERSION, so `local` and a version number keep
working. Restore `ash_version("~> 3.34")` once a release carries the change,
as 4400623 did after 627d7c5.
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