isInMemory should check if the name looks ilke an in-memory database name and return bool (it shouldn't throw validation errors).
Also, make the validation methods non-static.
In makeConnectionConfig, changing the $this->getPath($databaseName) line back to `$baseConfig['database'] = database_path($databaseName);` will make the added test fail.
"database tenancy bootstrapper throws an exception if DATABASE_URL is set" was failing with the null $databaseUrl because the tenant DB was never created. This test was ignored during test runs because the test file lacked the 'Test' suffix.
Since It's possible to update tenant's db_name to the central DB or the DB of another tenant. Setting $harden to true prevents tenants from connecting to the wrong databases.
Also, make the validateParameter method ignore null parameters, e.g. for cases when tenants are created using Tenant::make() without tenancy_db_username set -- $databaseConfig->getUsername() allows null, same should go for the validate method whose only concern is checking strings for invalid characters.
Also, use parameterAllowlist() instead of the static property (so that we can e.g. override it later in SQLiteDatabaseManager, since overriding the static property doesn't work).
DB manager methods validate the parameters they use in SQL statements using validateParameter() (excluding parameters passed via bindings in SELECT statements).
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>
At the moment, `where()` cannot be used correctly while using
`withoutPending()`. For example, if we have a single non-pending tenant
in our DB (with ID 'foo'), queries like
`Tenant::withoutPending()->where('id', 'nonexistent')->first()`will
incorrectly return the non-pending tenant ('foo').
This is because `withoutPending()` does
`$builder->whereNull('data->pending_since')->orWhereNull('data')`. These
two aren't grouped, so `withoutPending()->where('id', 'nonexistent')`
basically translates to "WHERE data->pending_since IS NULL **OR (data IS
NULL AND id = 'nonexistent')**". So the query will include all tenants
whose `pending_since` is null (= all non-pending tenants).
Grouping `->whereNull('data->pending_since')->orWhereNull('data')` in a
closure passed to a separate `where()` fixes this issue.
phpstan started failing with '... implements generic interface
Illuminate\Database\Eloquent\Scope but does not specify its types:
TModel'. We solve this by adding an implements docblock to the scopes
implementing that interface. They're fairly generic - we just use the
Model type itself in the code - so we use Model for the type parameter.
This prevents race conditions that may occur if there are two concurrent
processes trying to create the storage path for the tenant. The
storagePath() method runs during bootstrap() which can easily happen
in two places at once. The race condition specifically occurs in between
the is_dir() check and the mkdir() call, the latter producing an
exception if the dir already exist. We simply ignore any error coming
out of mkdir() and then check for success separately.
We could omit that success check since failure is unlikely and would
only occur due to a server misconfiguration that would manifest itself
in other ways as well, but this way the simple TOC/TOU race condition
is prevented while other errors are still reported.
We apply the same change to the mkdir() in scopeSessions() as the logic
is similar.
Resolves#1452
Some bootstrappers read attributes of the tenant during bootstrap() but
don't respond to changes made to the tenant afterwards.
Therefore, when making changes to the tenant that'd affect the behavior
of a bootstrapper, it's necessary to reinitialize tenancy (if it matters
that changes are reflected immediately). This adds a convenience helper
for that purpose.
