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

Overriding the default table naming behavior doesn't work #101

Closed
ghost opened this issue Jul 31, 2014 · 15 comments
Closed

Overriding the default table naming behavior doesn't work #101

ghost opened this issue Jul 31, 2014 · 15 comments

Comments

@ghost
Copy link

ghost commented Jul 31, 2014


To disregard namespace information when calculating the table name:

class User extends Model {
public static $_table_use_short_name = true;
}

To override the default naming behaviour and directly specify a table name:

class User extends Model {
public static $_table = 'my_user_table';
}


I have found it in the documentation. But it doesn't work for me.
php version: 5.5.6

@treffynnon
Copy link
Collaborator

It was missed in the last release and shouldn't have gone into the docs. A pull request to add it was made earlier today: #100

@michaelward82
Copy link
Contributor

This functionality should work in the current release - according to the unit tests, it does work.

@michaelward82
Copy link
Contributor

For clarity, PR #100 is for the global config option for the same functionality, as was discussed in PR #90.

@michaelward82
Copy link
Contributor

@redjee could you confirm in your composer.lock file that you are using version 1.5.1 of Idiorm and version 1.5.3 of Paris?

You should see some lines similar to the following

"name": "j4mie/paris",
"version": "v1.5.3"

@treffynnon
Copy link
Collaborator

I thought he was referring to the global. My bad!

@ghost
Copy link
Author

ghost commented Aug 1, 2014

Yes, I'm using the latest version of Idiorm (1.5.1) and Paris (1.5.3).
But these static class variables doesn't work.

@michaelward82
Copy link
Contributor

@redjee Could you run the following test app for me and tell me the results.

You will need to have PHP's sqlite3 PDO driver installed to run this (or you can setup and configure a real DB with other and user tables. Comment out require 'mock-pdo.php'; should you do so.).

From the CLI, do

git clone https://github.com/michaelward82/paris-test-01-08-2014.git
cd paris-test-01-08-2014
php ./composer.phar install
php index.php

I expect, and get, the following output:

SELECT * FROM `user`
SELECT * FROM `testing_other`

If you examine the source you will notice that the classes are declared in a testing namespace.
User class has property public static $_table_use_short_name = true; which omits testing as part of the table name and looks simply for user.

The Other class does not have the property and prepends the table name with testing, looking for testing_other

Let us know your results.

@ghost
Copy link
Author

ghost commented Aug 4, 2014

This test passed successfully.
But I have noticed that it gives an error

Class 'Other' not found in...

when you write

Model::factory('Other')->find_many()

instead of

Other::find_many();

@ghost
Copy link
Author

ghost commented Aug 4, 2014

And one more note:
These two static class variables are ignored when using
Model::$auto_prefix_models = '\prefix\';

@ghost
Copy link
Author

ghost commented Aug 4, 2014

Also static class variables are ignored in function

$this->has_many(className);

@michaelward82
Copy link
Contributor

Thankyou @redjee - I will look in to the issues.

@michaelward82
Copy link
Contributor

Hi there @redjee, thanks for taking the time to run the test app. I've investigated your issues as much as possible, and here is what I have found.

Examples

I have updated the test app. Either clone it again or do a git pull.

Expected output this time is:

SELECT * FROM `short_table_name`
SELECT * FROM `short_table_name`
SELECT * FROM `short_table_name`
SELECT * FROM `name_space_long_table_name`
SELECT * FROM `name_space_long_table_name`
SELECT * FROM `name_space_long_table_name`
SELECT * FROM `short_table_name`
SELECT * FROM `name_space_long_table_name`

Have a browse through the examples in index.php and see how they relate to the issues I discuss below.

First issue

Model::factory('Other')->find_many();

We're using a namespace - you must use model prefixes or add the namespace to the parameter. Instead try

Model::factory('\\testing\\Other')->find_many();

or

Model::$auto_prefix_models = '\\testing\\';
Model::factory('Other')->find_many();

Second issue

Calling a Model using the static methods, e.g. Model::find_many(); is invalid when used with model prefixes.

When we pass a class name to the factory method, this parameter will be combined with the model prefix to determine which class to use. In the sample code, if we add

Model::$auto_prefix_models = '\\testing\\';
Model::factory('User')->find_many();

Then we would attempt to call a class of \testing\User. This exists and all is OK.

Using the static method:

Model::$auto_prefix_models = '\\testing\\';
User::find_many();

in this situation PHP must already know the namespace for User else it cannot call the class. The class is aware of its own name (with namespace), and this is used when we add the prefix. However, the class name already has it's namespace information, and adding the prefix will cause the class name to be invalid. The above method will attempt to use the class name \testing\testing\User.

I will see if we can prevent this situation through code - at the very least I will note the limitation in the docs.

Use of the $auto_prefix_models is unnecessary and invalid when using the static (non-factory) method.

Third issue

$this->has_many(className);

I haven't done any testing on this, but could it be related to the namespace issue as in issue one?

tl;dr

  • When using namespaces with the factory method you must either use a model prefix to specify the namespace as a prefix, or pass the namespace in the parameter.
  • When using static method calling (not the factory method) the use of model prefixes are invalid.

@michaelward82
Copy link
Contributor

@redjee Have you had any time to try the updated tests as mentioned above?

@ghost
Copy link
Author

ghost commented Aug 13, 2014

@michaelward82 Your tests passed successfully. Thank you for advanced explanation, now it became clearly for me, problem was in wrong use of namespace feature. I think we can close this issue.

@michaelward82
Copy link
Contributor

Excellent.

@ghost ghost closed this as completed Aug 13, 2014
This issue was closed.
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

No branches or pull requests

2 participants