Bug/schema naming

[beporter]

Proposed fix for #131.

Adds an option for specifying the Schema class name on the command line.

Updates _getSchema() and _updateSchema() to use a new _getSchemaClassName() method, which merges existing behavior for plugins with a new default for base Cake applications.

Most importantly, Config/Schema/schema.php files will now be generated using AppSchema as the default class name instead of APP_DIR . 'Schema'.

Thanks to @burzum for the initial attempt at resolving this.

[beporter]

The unit tests in this area don't cover every case. (They didn't even before this PR.) This could use some more field testing.

[beporter]

I have confirmed locally that Config/Schema/schema.php files will be properly recognized in the following cases:

• Not using the new --schema-class argument, schema.php using class WwwSchema (existing behavior of matching APP_DIR value).
• Not using --schema-class, schema.php using class AppSchema (new behavior).
• Using --schema-class different, schema.php using class DifferentSchema (new behavior).
• Writing a new schema.php file for the core Cake app uses class AppSchema by default (new behavior).
• Writing a new schema.php file when using --schema-class different works properly, but only provided the file already specifies class DifferentSchema. (The limitation here is that you can't read a different name from what you want to write back into the schema.php file since the command line arg applies to both _getSchema() and _updateSchema(), but that makes decent sense to me anyway.)
• I have not tested plugins in any way.

[beporter]

Sigh. Do not merge.

There's an issue here. The SchemaShell has two possible arguments: --name and --file. If you only specify --name (as this PR does), then the --file param is automatically set to match. This means you''ll end up with a Config/Schema/app.php file instead of `Config/Schema/schema.php as expected.

Specifying both --file schema.php and --name app during schema generate doesn't seem to work.

EDIT: This is the reason why: https://github.com/cakephp/cakephp/blob/92957913d93404d8554345d4e41cb3c723a0a842/lib/Cake/Console/Command/SchemaShell.php#L74,L76

You can't tell the SchemaShell that you explicitly want the default file name in spite of the --name arg.

This relates to cakephp/cakephp#4174.

[burzum]

So there is no way to resolve this properly until this gets fixed in the core. Can you please show this issue to Mark Story to get it fixed?

[beporter]

Correct. I'll do a PR to fix the SchemaShell this week.

On Aug 10, 2014, at 5:57 PM, Florian Krämer [email protected] wrote:

So there is no way to resolve this properly until this gets fixed in the core.

—
Reply to this email directly or view it on GitHub.

[burzum]

Great! 👍

[beporter]

I submitted a PR for the SchemaShell fixes that are required for the MigrationShell to work properly. We'll have to wait on this PR until that fix has made its way into a Cake core release.

⚠️ This PR will cause more problems than it fixes until the SchemaShell uses its command line args correctly.

[burzum]

Yes, I've just seen that. Good job on that!

The question for me is now how to deal with that change in the migrations plugin, we need to provide some BC compatibility in the case this change will break something.

[beporter]

My PR checks for both class AppSchema and class APP_DIR . Schema, so the Migrations plugin should continue to behave in the proper way for existing Schema's generated to use a class name that is based on APP_DIR.

All new Schema files will be written with class AppSchema by default after this PR, so either the code can remain in place indefinitely, or in a year or so the fall-back to APP_DIR can be removed once it was safe to assume most projects would be updated.

[beporter]

As I said before, more testing couldn't hurt though since the unit tests themselves were a little short in this area to begin with (it's kinda hard to test, in fairness.)

[beporter]

cakephp/cakephp#4230 has been merged, but we still need to wait for a public release before my changes here will be effective. The SchemaShell fix should be included in 2.5.4 based on 4230's Milestone setting.

[burzum]

Thanks for your work on this @beporter! I set this to hold and will merge it when there is a new release of CakePHP. I'm just not sure yet if we should wait for 2.6 or already do it for 2.5.4.

[deizel]

Thanks!

[beporter]

😄