laravel / cashier-stripe

Laravel Cashier provides an expressive, fluent interface to Stripe's subscription billing services.
https://laravel.com/docs/billing
MIT License
2.37k stars 671 forks source link

Error in migration when using postgres. #747

Closed oscarteg closed 5 years ago

oscarteg commented 5 years ago

Description:

Then migration 2019_05_03_000001_create_customer_columns gives an error when using the postgres with Laravel. Running php artisan migrate on a fresh installation gives the following error.

Illuminate\Database\QueryException  : SQLSTATE[42704]: Undefined object: 7 ERROR:  collation "utf8mb4_bin" for encoding "UTF8" does not exist at character 57 (SQL: alter table "users" add column "stripe_id" varchar(255) collate "utf8mb4_bin" null, add column "card_brand" varchar(255) null, add column "card_last_four" varchar(4) null, add column "trial_ends_at" timestamp(0) without time zone null)

Steps To Reproduce:

  1. Fresh installation of laravel
  2. Change .env to use postgres as driver
  3. Add cashier as dependency
  4. php artisan migrate
driesvints commented 5 years ago

Hmm, doesn't this collation exists on Postgres? I'm unfamiliar with Postgres so I don't know for sure. Stripe actually recommends to use this for MySQL but now I'm not sure if it's universal for every DB type: https://stripe.com/docs/upgrades#what-changes-does-stripe-consider-to-be-backwards-compatible

hanspagel commented 5 years ago

Can confirm, had this issue too. Had to delete the collation from the migration.

driesvints commented 5 years ago

We'll need to find a universal way for this since it's recommended by Stripe to add this.

pedrommone commented 5 years ago

Just had the same issue.

As https://stripe.com/docs/upgrades#what-changes-does-stripe-consider-to-be-backwards-compatible says: If for example you’re using MySQL, you should store IDs in a VARCHAR(255) COLLATE utf8_bin column (the COLLATE configuration ensures case-sensitivity in lookups).

IMHO, there are two ways to fix that:

  1. Ignore since its specific for MySQL;
  2. Reproduce the same behavior for PgSQL.

Searching a little but, I've found that using LIKE for queries, as you can see here, can reproduce the case-sensitive behavior.

pedrommone commented 5 years ago

Maybe the Laravel Schema should implement an $table->string('stripe_id')->sensitive() flag.

driesvints commented 5 years ago

I think that would be pretty hard to do. I'd be pretty opinionated to enforce this with utf8mb4_bin in MySQL.

rwdim commented 5 years ago

Postgres uses utf8mb4 by default, but it's called "utf8". This should use the database config values: 'mysql' => [ ... 'charset' => 'utf8mb4', and 'pgsql' => [ 'charset' => 'utf8', values.

driesvints commented 5 years ago

@rwdim if we'd change it to utf8_bin would that work?

rwdim commented 5 years ago

The solution for me was to add Cashier::ignoreMigrations() to AppServiceProvider::register, and change


``` to ```$table->string('stripe_id')->nullable()->index();``` in
my migrations directory.

R

On Mon, Aug 26, 2019, 7:58 AM Dries Vints <notifications@github.com> wrote:

> @rwdim <https://github.com/rwdim> if we'd change it to utf8_bin would
> that work?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/laravel/cashier/issues/747?email_source=notifications&email_token=AADFWGLWQCHPLYSXKVLGGN3QGPHOHA5CNFSM4IL5CQJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5EJHMI#issuecomment-524850097>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AADFWGO73DCVXSG2RKLOZC3QGPHOHANCNFSM4IL5CQJQ>
> .
>
driesvints commented 5 years ago

@rwdim okay.

Can anyone try out on postgres by changing the migrations to use utf8_bin instead to see if that works?

rwdim commented 5 years ago

No, it does not work., neither does collation('utf8').

Undefined object: 7 ERROR: collation "utf8_bin" for encoding "UTF8" does

not exist

Based on what I read, utf8mb4_bin is a case-insensitive solution in mysql that mirrors the default utf8 case-insensitive behavior in postgresql. Therefore, selecting utf8 as the character set for the database is all you need to do in PG, and omit the collation.

Based on the PG docs, collation is used to implement logic specific to locales (en_us vs de_de vs ja_jp) ( https://www.postgresql.org/docs/11/collation.html https://www.postgresql.org/docs/10/collation.html ). R

rwdim commented 5 years ago

How about adding a collation field to the cashier config settings, and if it's set use that?

Otherwise don't do any collation.

On Mon, Aug 26, 2019, 8:32 AM Randy Dryburgh me@rwd.im wrote:

No, it does not work., neither does collation('utf8').

Undefined object: 7 ERROR: collation "utf8_bin" for encoding "UTF8"

does not exist

Based on what I read, utf8mb4_bin is a case-insensitive solution in mysql that mirrors the default utf8 case-insensitive behavior in postgresql. Therefore, selecting utf8 as the character set for the database is all you need to do in PG, and omit the collation.

Based on the PG docs, collation is used to implement logic specific to locales (en_us vs de_de vs ja_jp) ( https://www.postgresql.org/docs/11/collation.html https://www.postgresql.org/docs/10/collation.html ). R

driesvints commented 5 years ago

I think maybe removing the default collation and noting something in the docs could be a good idea.

@staudenmeir do you think if we could add this to the schema builder to provide a solution for both postgres and mysql (and maybe the other two drivers as well)?

rwdim commented 5 years ago

One thing... not sure why utf8mb4_bin was used, since the key that comes back appears to me to be ascii salt+uuid(16) . I know Stripe suggests that it may be something else in the future, but given that they don't say what, default collation is as good a guess as a utf8 scope-limited variant.

Just my 0.000002BC :)

On Mon, Aug 26, 2019 at 9:37 AM Dries Vints notifications@github.com wrote:

I think maybe removing the default collation and noting something in the docs could be a good idea.

@staudenmeir https://github.com/staudenmeir do you think if we could add this to the schema builder to provide a solution for both postgres and mysql (and maybe the other two drivers as well)?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/laravel/cashier/issues/747?email_source=notifications&email_token=AADFWGNOCPYPOS5TAVDKPSTQGPTCNA5CNFSM4IL5CQJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5ESBLI#issuecomment-524886189, or mute the thread https://github.com/notifications/unsubscribe-auth/AADFWGLC7C5FGFOZ3OHXULLQGPTCNANCNFSM4IL5CQJQ .

staudenmeir commented 5 years ago

This was "caused" by https://github.com/laravel/framework/pull/29213. Before, Laravel ignored the collation on PostgreSQL.

I don't see how we could fix this in the schema builder.

Not the most elegant solution: We could detect the database driver in the migration and only add the collation on MySQL.

driesvints commented 5 years ago

I think maybe it's best that we just remove the collation for now and add a note in the docs. I'll send in a PR.