Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for using Sushi models with the exists validation rule #88

Merged
merged 4 commits into from
Apr 21, 2021
Merged

Add support for using Sushi models with the exists validation rule #88

merged 4 commits into from
Apr 21, 2021

Conversation

bakerkretzmar
Copy link
Contributor

@bakerkretzmar bakerkretzmar commented Apr 20, 2021

This PR adds Sushi's generated database connection to the app config database.connections array, which makes it possible to use Sushi models with the exists database rule. Accomplishes the same thing as #80 but using Laravel's existing validation infrastructure.

@calebporzio
Copy link
Owner

Nice! This is great!

@calebporzio calebporzio merged commit da11d58 into calebporzio:master Apr 21, 2021
@bakerkretzmar bakerkretzmar deleted the support-exists-validation branch April 21, 2021 13:39
@mallardduck
Copy link
Contributor

@calebporzio - this looks super useful, any chance you can cut a tagged release for this?

calebporzio pushed a commit that referenced this pull request Aug 11, 2021
After introducing sushi to our project, our monitoring tool reported the following
error (stacktrace shortened for brevity):

```
PDOException: SQLSTATE[HY000]: General error: 1 table "schmelzkurve_scoring_table_results" already exists
#88 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(485): PDOStatement::execute
#87 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(485): Illuminate\Database\Connection::Illuminate\Database\{closure}
#86 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(685): Illuminate\Database\Connection::runQueryCallback
#85 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(652): Illuminate\Database\Connection::run
#84 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(486): Illuminate\Database\Connection::statement
#83 /vendor/laravel/framework/src/Illuminate/Database/Schema/Blueprint.php(109): Illuminate\Database\Schema\Blueprint::build
#82 /vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php(365): Illuminate\Database\Schema\Builder::build
#81 /vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php(228): Illuminate\Database\Schema\Builder::create
#80 /vendor/calebporzio/sushi/src/Sushi.php(142): App\Models\SchmelzkurveScoringTableResult::createTable
#79 /vendor/calebporzio/sushi/src/Sushi.php(89): App\Models\SchmelzkurveScoringTableResult::migrate
#78 /vendor/calebporzio/sushi/src/Sushi.php(45): App\Models\SchmelzkurveScoringTableResult::Sushi\{closure}
#77 /vendor/calebporzio/sushi/src/Sushi.php(66): App\Models\SchmelzkurveScoringTableResult::bootSushi
```

This error can happen in rare circumstances due to a race condition.
Concurrent requests may both see the necessary preconditions for
the table creation, but only one can actually succeed.

My initial attempt at resolving this was to look for a method that
creates the table only if it does not exist (`create table if not exists`),
but Laravel exposes no such method. Thus, I think the only viable solution
is to risk it and catch the resulting error.
@calebporzio
Copy link
Owner

Hi @mallardduck & @bakerkretzmar - I'm going to tag this release right now, can one of you document this addition in the README file?

@mallardduck
Copy link
Contributor

@calebporzio - sure thing. i'll whip up a PR real quick

@calebporzio
Copy link
Owner

Thanks!

@calebporzio
Copy link
Owner

Just tagged 2.3.0

@danharrin
Copy link
Contributor

There's actually a major issue with this - it only works if you have one Sushi model 😆 Spent a few hours bashing my head on the table about it this morning. Will submit another PR.

@danharrin
Copy link
Contributor

Pushed a fix for the multiple model issue in #96.

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.

4 participants