Originally, the test only asserted that the broadcasters inherited the correct channel names. Now, the test also asserts that the actual closure and the channel options are inherited too.
Remove the with() -- the test ran four times with the same TestingBroadcaster, so the only tested broadcaster was actually the testing one. Running the test just once is enough, the same things are still covered.
Channel auth closures (registered using Broadcast::channel() e.g. in routes/channels.php) are only ever read from the default broadcaster -- both Broadcast::channel() and Broadcast::auth() go through the default broadcaster. TenancyBroadcastManager passed the closures to every broadcaster it resolved, which is pointless -- even in central context, Laravel doesn't do anything like that).
Instead, bind a fresh BroadcastManager and pass the central channel closures to its default broadcaster while extending Broadcaster. So now, tenant context behaves exactly like central context, just with broadcasters resolved using the tenant credentials.
Also update comments.
TenancyBroadcastManager now overrides resolve() (only to pass the central channels to the newly resolved broadcasters) instead of get(), so broadcasters stay cached for the duration of the tenant's context.
Remove TenancyBroadcastManager::$tenantBroadcasters -- all broadcasters now inherit the central channels, so even custom drivers work without any additional configuration/registration.
Channels registered via Broadcast::channel() in tenant context are no longer lost on subsequent broadcaster retrievals (e.g. during /broadcasting/auth). Added a dedicated test for this,
This PR adds the LogChannelBootstrapper to provide tenant-specific
logging configuration. The bootstrapper automatically configures storage
path channels to use tenant-specific directories (NOTE: for this to work
correctly, the bootstrapper has to run AFTER
FilesystemTenancyBootstrapper, otherwise, the logs still won't be
separated, unless you use overrides) and supports custom channel
overrides for custom logging scenarios -- mapping tenant properties to
the channel config, or using custom closures an array with the logging
config, e.g. for making the slack channel (that's not handled by the
bootstrapper by default) tenant-specific.
The bootstrapper first modifies the channel config, then forgets the
channel from LogManager so that on the next logging attempt, the channel
is re-resolved with the modified config. Otherwise, the channel would
just use the initial config **if the channel was resolved before**. If
the channel wasn't resolved before, it'll always be resolved with the
updated (tenant) config, unless the configuration fails. In that case,
the config will be reverted (the central config will be restored) and
the error will be logged using the original channel.
When using a channel stack, the `stack` channel itself also has to be
forgotten, since the LogManager could retain e.g. the original `stack`
channel's webhook URL, while the underlying `slack` channel would use
the updated one, and while logging, the app would actually use the
initial webhook URL instead of the updated one (encountered this issue
while testing).
Note that **all** channels in `$storagePathChannels` and
`$channelOverrides` are affected.
Also, adding `'attachment' => 'false'` to the slack channel's config
makes the slack channel work with Discord webhooks (just a cool thing we
figured out whlle testing the bootstrapper).
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
### Test for DatabaseTenancyBootstrapper throwing an exception when
`DB_URL` is set
The test meant to cover this set `central.url` before creating a tenant.
That made the `CreateDatabase` job fail with a QueryException (it tried
to use the URL host as a database name). The bootstrapper's check was
never reached, and the exception that the test intended to cover wasn't
ever thrown in the end. The test passed for the wrong reason.
Fixed by creating the tenant first, setting the url only after that, and
asserting that `tenancy()->initialize()` throws the bootstrapper's
specific exception.
### Reference env variable that Laravel uses now (`DB_URL`) instead of
`DATABASE_URL`
Changed all `DATABASE_URL` references in Tenancy to `DB_URL`. [In
Laravel
11](96508d43ec (diff-e4382b565f69d02de16eef89c8d5ca2b60a679ffca90ab8b7d85fbd78f30075bR35)),
the `DATABASE_URL` env variable got renamed to `DB_URL` in
`config/database.php`. The env var got renamed quite a while ago, so
referencing `DATABASE_URL` in Tenancy's comments and tests was a bit
confusing.
### Minor cleanup
The `harden prevents tenants from using the database of another tenant`
test now reads the central connection name from
`config('tenancy.database.central_connection')` instead of hardcoding
`'central'` for consistency with the other tests.
The original BroadcastingTest file had a very similar test that was removed during a refactor. Added it back and improved it (used the testing broadcaster instead of NullBroadcaster, improved the test's name comments)
### Parameter validation
In `statement()` calls of `TenantDatabaseManager`s, use parameter
binding when possible. When that's not possible, validate the parameters
using `validateParameter()` or `validatePassword()`.
Passwords use a less strict allowlist than other parameters (e.g. DB
names), since passwords tend to use more special characters but we can
afford to be more restrictive in the generic `validateParameter()`.
In `SQLiteDatabaseManager`, names of file-based databases are validated
(in `createDatabase`, `deleteDatabase` and `databaseExists`) using a
similar allowlist to the (non-password) parameters in other DB managers,
with an additional character: `.` (this addition is necessary since
file-based SQLite databases end with `.sqlite`).
### DatabaseTenancyBootstrapper - harden() and the lost test file
While checking for more places that could use validation, I realized
that it's possible to update tenant's db_name to the central DB or the
DB of another tenant. Added the `DatabaseTenancyBootstrapper::$harden`
property -- setting it to true prevents tenants from connecting to the
wrong databases (`RuntimeException` is thrown after connecting to the
wrong database).
Also, the DatabaseTenancyBootstrapper test file was ignored while
running tests because it lacked the `Test` suffix. Added the suffix and
fixed the broken `DATABASE_URL` test in the file.
### SQLiteDatabaseManager - respect static $path property in
makeConnectionConfig()
SQLiteDatabaseManager had a bug in `makeConnectionConfig`: the method
didn't respect the static `$path` property, it used `database_path()`
instead. Added a regression test for that. Also recognizing in-memory
SQLite databases (using `isInMemory()`) is more strict now so that
simply having a db_name with `_tenancy_inmemory_` somewhere in the name
doesn't make a file-based database considered in-memory.
### MySQLDatabaseManager - charset and collation defaulting
Creating databases with `null` charsets and collations resulted in a
`QueryException`, since null isn't a valid charset/collation. To solve
that, in the `CREATE DATABASE` statement in MySQLDatabaseManager, only
add charset/collation to the statement if they are not null.
MySQL defaults to the server's charset and collation, so it's safe to
not pass any charset/collation in the `CREATE DATABASE` statement and
let MySQL choose. Also, if e.g. collation is non-null and charset is
null, MySQL will use a charset compatible with the used collation, and
this works both ways.
---------
Co-authored-by: Samuel Stancl <samuel@archte.ch>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This PR adds the `toRoute()` method override to `TenancyUrlGenerator`.
`toRoute()` now attempts to find a tenant equivalent of the passed route
(= a route with the same name as the passed one, but with the tenant
prefix) and generates URL for the tenant route. This behavior can be
bypassed using the bypass parameter, like with the `route()` method
override `TenancyUrlGenerator` had until now.
The primary reason for adding this is that Livewire v4 no longer uses
the `route()` helper (which automatically prefixes the passed route name
because of the override in `TenancyUrlGenerator`) in
`Livewire::getUpdateUri()`. Now, it uses `toRoute()`
(544aa3dfb8 (diff-e7609f8b0a60bde5a85067803d4e2f08f235c7cee9225a51ea67a85ff9a1d694R52)),
which didn't automatically swap the route for its 'tenant.'-prefixed
equivalent in tenant context (until now). So for the Livewire
integration to work with path identification, we need to override
`toRoute()` as described.
The `temporarySignedRoute()` override got removed because
`temporarySignedRoute()` calls `route()` under the hood, there's no need
to specifically override `temporarySignedRoute()`.
> Note: Browsing old convos, it seems like the `temporarySignedRoute()`
override was needed to make Livewire file uploads work with path
identification, but it's not needed anymore. TenancyUrlGenerator had
some changes since then, and now, I can't see the _exact_ reason why we
needed the override (`temporarySignedRoute()` uses `route()` under the
hood, so the only thing that should really matter is overriding
`route()`/`toRoute()`). It was likely a leftover from some older
implementation.
The `route()` override got simplified. Since `route()` uses `toRoute()`
under the hood, the `route()` override only has to have the prefixing
logic. The rest is delegated to `toRoute()`.
> Note: Even though we override `toRoute()` now which `route()` uses for
generating the URLs, we still need to override `route()` for its
`$this->routes->getByName($name)` call to receive the prefixed name. For
example, if `route()` wasn't overridden, and we only had one route:
`tenant.foo` (no central `foo` route), and we'd call `route('foo')`,
we'd get an exception saying that route "foo" wasn't found, even if
automatic route name prefixing was enabled and `toRoute()` was
overridden. With the `route()` override, `route('foo')` acts as if we
passed 'tenant.foo' instead of 'foo'.
Comments in TenancyUrlGenerator and UrlGeneratorBootstrapper got updated
to be more accurate. All _intentionally_ affected methods are listed in
TenancyUrlGenerator's docblock.
---------
Co-authored-by: Samuel Stancl <samuel@archte.ch>
The `CreateTenantStorage` and `DeleteTenantStorage` listeners were used
alongside JobPipelines. When the `TenantCreated` JobPipeline had
`shouldBeQueued(true)` and the `Listeners\CreateTenantStorage` was
uncommented, the listener would throw an exception
(`Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException
Database tenantX.sqlite does not exist.`) because at the time of
executing the listener, the tenant DB wasn't created yet.
The same issue could likely also occur in the `DeleteTenantStorage`
listener as it uses `tenancy()->run()` to resolve the tenant's storage
path which wouldn't work if the tenant's database (or other resources)
was already deleted, making initialization impossible.
This PR changes `DeleteTenantStorage` into a job and puts it (commented)
into the job pipeline, so that it can be queued with the rest of the
jobs. It also removes `CreateTenantStorage` because it should be
redundant with the FilesystemTenancyBootstrapper creating the same paths
automatically when storage path is suffixed.
The old classes are kept but deprecated for backwards compatibility.
We've also added some edge case hardening to `DeleteTenantStorage` to
make sure it never deletes the central storage path directory, which
previously could in theory occur due to a misconfiguration if a user
enabled this job/listener but disabled storage path suffixing.
Co-authored-by: Samuel Štancl <samuel@archte.ch>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Tests now use datasets with all drivers that are in `TenancyBroadcastManager::$tenantBroadcasters` by default plus the custom driver. Also add assertions for updating the tenant properties/config in tenant context.
Tests from BroadcastingTest moved to the appropriate bootstrapper test files. The new tenant credentials test has assertions equal to both the original property -> config mapping test and the config -> credentials test.
Using `URL::temporarySignedRoute()` in tenant context with
`UrlGeneratorBootstrapper` enabled doesn't work the same as `route()`.
The bypass parameter doesn't actually bypass the route name prefixing.
`route()` is called in the `parent::temporarySignedRoute()` call, and
because the bypass parameter is removed before calling
`parent::temporarySignedRoute()`, the underlying `route()` call doesn't
get the bypass parameter and it ends up attempting to generate URL for a
route with the name prefixed with 'tenant.'.
This PR adds the bypass parameter back after `prepareRouteInputs()`, so
that `parent::temporarySignedRoute()` receives it, and the underlying
`route()` call respects it. Also added basic tests for the
`URL::temporarySignedRoute()` behavior (the new bypass parameter test
works as a regression test).
This is because the CreateTenantStorage listener only runs when
a tenant is created, but in multi-server setups the directory may
need to be created each time a tenant is *used*, not just created.
Also changed the listeners to use TenantEvent instead of specific
events, to make it possible to use them with other events, such as
TenancyBootstrapped.
Also update permission bits in a few mkdir() calls to better scope
data to the current OS user.
Also fix a typo in CacheTenancyBootstrapper (exception message).
Before, when using UrlGeneratorBootstrapper, and your app had a
`https://` url, in tenant context, the url would have the `http://`
scheme.
Now, the bootstrapper makes sure that the TenancyUrlGenerator inherits
the original UrlGenerator's scheme. So if your app has e.g. url
"https://some-url.test", `route('home')` in tenant context will return
"http**s**://some-url.test/home" (originally, you'd get
"http://some-url.test/home" - the original scheme - https - wouldn't be
respected in the tenant context).
This PR addresses the issue reported on Discord
(https://discord.com/channels/976506366502006874/976506736120823909/1399012794514411621).
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
* Initial implementation (lukinovec)
* Make sure DatabaseCacheBootstrapper runs after DatabaseTenancyBootstrapper, misc wip changes
* Fix withTenantDatabases()
* Add failing test (GlobalCacheTest)
* Configure globalCache's DB stores to use central connection instead of default connection every time it's reinstantiated
* Make GlobalCache facade not cached. Even though it wasn't causing issues
in our existing tests, it likely was flaky, and making it not $cached
makes it now consistent with global_cache() - always getting a new
CacheManager from the globalCache container binding
* Add database connection assertions in GlobalCacheTest
* Run all cached resolver/global cache tests with DatabaseCacheBootstrapper
* Reset adjustCacheManagerUsing in revert() and TestCase
* Reset static $stores property
* Finalize GlobalCache-related changes
* tests: remove pointless cache TTLs
* Refactor DatabaseCacheBootstrapper
* Refactor tests
Co-authored-by: lukinovec <lukinovec@gmail.com>
* Test encrypted cookie identification
* Add Fortify bootstrapper custom query param passing test
* Correct Fortify route bootstrapper test (todo refactor, convoluted)
* Clarify Fortify bootstrapper test
* Fix encrypted cookie identification test
* Move encrypted cookie assertion to "cookie identification works"
* Cover configured tenant model columns in cached resolver tests
* Refactor testing resolver with default vs custom tenant model name config
* Delete resolved todo
* Make code more concise
* Keep initial formatting (minimize diff noise)
* Make dataset/helper method parameter clearer
* Clarify fortify test
* Clarify assertions, improve comments
* Delete excessive comments, make existing comments consistent and clearer
* Make cached resolver test file clearer, update outdated comments
* Use the tenant model column term consistently
* FIx inconsistencies
* Provide more info in comment
* make comment more clear
* static property reset
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
* UrlGenerator: set defaults based on config; request data: move config to config file+resolver
* Claude code adjustments
* improve request data tests, simplify complex test in UrlGeneratorBootstrapperTest
* url generator test: test changing tenant parameter name
* request data identification: add tenant_model_column configuration
* defaultParameterNames -> passQueryParameter
* move comment
* minor refactor in PathIdentificationTest, expand CLAUDE.md to include early identification section
* Fix COLOR_FLAG
* improve test name
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* TenancyUrlGenerator: add a check for queryParameterName being null
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Fix code style (php-cs-fixer)
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add Laravel 12 support, drop Laravel 11 support
* Fix RLS tree generation (specify schema name in generateTrees())
* ci fixes, use stable virtualcolumn version
---------
Co-authored-by: lukinovec <lukinovec@gmail.com>
* cleanup, resolve todos, add immediate todos
* Improve path_identification_middleware docblock
* rename leave() method in tests
* wip fix hardcoded values making assumptions about the parameters used in routing
* defaultParameterNames
* fix CreatesDatabaseUsers return values
* $tenant -> tenant()
* resolve more todos
* make comment block a complete block
* Correct useTenantRoutesInFortify(), delete unused import
* test fixes
* remove todos
* remove JobPipeline todo
* simplify comment example
* remove todo
* fix VERSION_PREFIX in queue.yml
---------
Co-authored-by: lukinovec <lukinovec@gmail.com>
* Make RootUrlBootstrapper run ONLY in CLI by default (add $rootUrlOverrideInTests), work with resolved UrlGenerator
* Make resolving 'url' return a pre-created generator instance instead of creating it on every app('url') call
* Take care of doubling tenant keys in TenancyUrlGenerator, add regression test for using UrlGenerator and RootUrl bootstrappers together
* Fix code style (php-cs-fixer)
* refactor RootUrlBootstrapper
* add docblock
* clarify docblock
* simplify test: use concrete values instead of overly dynamic code
* Fix bootstrapper order in test, add url('/') assertion
* Use $this->app instead of app()
* Improve TenancyUrlGenerator and RootUrlBootstrapperTest clarity
* Revert attempt to maintain compatibility between the two bootstrappers
* Delete bootstrapper combining test
* Fix code style (php-cs-fixer)
---------
Co-authored-by: lukinovec <lukinovec@gmail.com>
Co-authored-by: PHP CS Fixer <phpcsfixer@example.com>
* Separate route-level domain identification test from path/request to improve readability WIP
* Get rid of confusing datasets in route-level identifcation tests
* Clean up updated tests
* Simplify early id tests
* Reduce dataset duplication
* Improve test readability, fix false positive, polish details
* Separate early ID test from defaulting test (WIP)
* Finish improving and correcting the early identification/default route mode tests
* Make flag/default mode usage more clear by improving the docblock in DealsWithRouteContexts
* Fix PHPUnit deprecation warnings
* code review
* code review
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
* Only retrieve domains if the relationship and the domains table exist (DomianTenantResolver)
* Resolve todo, add other todos
* Use constructor promotion in DeleteDomains
* Fix imports + domain deletion test
* Confirm that turning on resolver caching doesn't break the tests
* Fix Tenant model imports
* Fix code style (php-cs-fixer)
* remove runtime schema check
* temp: enable resolver cache
* make 'autoincrement ids are supported' pass
* disable resolver cache
---------
Co-authored-by: PHP CS Fixer <phpcsfixer@example.com>
Co-authored-by: Samuel Štancl <samuel.stancl@gmail.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
