Allow a prefix for model class names

[markchalloner]

This patch will check to see if there is a class_prefix in ORMWrapper::$_config and if there is prepend it to the class_name.

This allows creating model classes less likey to clash with existing classes but keep the factory method simple:

// Configure a class prefix (defaults to empty)
ORMWrapper::configure('class_prefix', 'Model');

// Create entity
class ModelUser extends Model {}

// Create user
$user = Model::factory('User')->create();

Thanks

[treffynnon]

This was going to be added in the next release so very handy to have your patch! Have you had any thoughts on how namespaces might be handled in a similar way? Paris was updated to handle namespaces in the latest release so ideally any patch for this functionality would handle both.

[markchalloner]

It would make sense to me to allow a seperate configuration for namespaces to avoid repetition as much as possible (though either should be possible):

ORMWrapper::configure('namespace', 'Full\Namespace\To\Models');

$user = Model::factory('User')->create();
$book = Model::factory('Book')->create();

Rather than:

$user = Model::factory('Full\Namespace\To\Models\User')->create();
$book = Model::factory('Full\Namespace\To\Models\Book')->create();

For my patch to work with namespaces in theory should be fairly easy:

• explode on \
• cat the prefix to the final component
• implode

... but I'm not sure mixing and matching is a great idea.

I'll have a think and see what I can come up with and how the two work together.

[treffynnon]

Thank you for following up on this.

I agree and think they should be configured separately, but in the same way as each other like in your example code.

Looking forward to seeing what you come up with!

[markchalloner]

Ok, try that on for size.

I added a static function to normalise the class name passed into the factory before it get passed anywhere else. It tries to combine prefixes and namespaces (semi)-intelligently:

• Checks if the prefix is already specified in the factory:

ORMWrapper::configure('class_prefix', 'Prefix');
Model::factory('PrefixClass'); // Calls PrefixClass not PrefixPrefixClass

• Adds the Prefix to the base class if a namespace is specified:

ORMWrapper::configure('class_prefix', 'Prefix');
Model::factory('Namespace\\Class'); // Calls Namespace\PrefixClass

• Trims additional backslashes from namespaces before submitting the classname internally (to avoid additional underscores being generated in the table name) to cater for Model::factory('\\Namespace\\Class')

Model::factory('\\Namespace\\Class'); // Uses table namespace_class not _namespace_class

• Allows overriding of the configured namespace when calling factory:

ORMWrapper::configure('namespace', 'Namespace');
Model::factory('Namespace2\\Class'); // Calls Namespace2\Class not Namespace\Class or Namespace\Namespace2\Class

Cheers

[markchalloner]

Forgot to say, slight update to idiorm to allow for late static binding in the configure function, means that idiorm/paris becomes dependent on 5.3: http://php.net/manual/en/language.oop5.late-static-bindings.php

[treffynnon]

Thank you for the patch Mark and the thorough notes you have included. Hopefully I will have a chance to review it really soon - probably next week. A bit stacked with work at the moment, but wanted to assure you that your help is much appreciated.

[tag]

It's a lot of great work, but I'm not sure I like that the namespace alters the table name used in the db side. For example, Full\Namespace\To\Models\User maps to the db table full_namespace_to_models_user.

Is that desired behavior? (It's not what I want for me, but may be what the community is seeking.)

As an aside, if the "late static binding" approach is adopted, it should be applied consistently throughout Paris, and there can be other clean-up in ORMWrapper, for example removing the ``::for_table()static method, which is essentially identical to Idiorm's version inORM`. (The late static approach is currently used only once in Idiorm, which has caused problems for some, e.g., ivannovak/idiorm@351caf2.) [EDIT: Fixed link]

Conversely, the need for late static binding can be avoided (and < 5.3 compatibility preserved) with some tweaks .... without testing it, I'm guessing it would require this patch to use ORMWrapper::_get_static_property() instead of attempting to access the ::_config values directly.

[markchalloner]

Agree with @tag here that by default the prefix and namespace shouldn't alter the table name, however this should be configurable. Going to leave the final decision of whether the namespacing/prefixing should be on or off by default, but my vote is for off.

Regarding late static binding, I don't think _get_static_property() would work: it's a static method on the Model class and while it could be used instead of direct access to the config property, wouldn't solve the underlying issue that without static:: instead of self::, configure sets properties on ORM not ORMWrapper.

3 alternatives I can see:

• Create a full configure method on ORMWrapper. That's not very DRY though
• Set Paris configuration using ORM::configure and access using ORM::$_config. That doesn't make sense though
• Use static:: instead of self:: in ORM::configure as in previous commit. PHP < 5.3 does error but then has been unsupported for almost 2 years (http://www.php.net/eol.php)...

Thanks

[treffynnon]

I have been looking at this on and off today. I think it is really quite complex and I don't like the interaction between class prefixes and namespaces. I feel the concept should be really simple:

    Model::auto_prefix_models_with = '\\App\\Namespace\\';
    $model = Model::factory('ModelName'); // calls \App\Namespace\ModelName
    $model = Model::factory('AnotherModelName'); // calls \App\Namespace\AnotherModelName

    Model::auto_prefix_models_with = 'MyModelPrefix_';
    $model = Model::factory('SomeModel'); // calls MyModelPrefix_SomeModel
    $model = Model::factory('ADifferentModel'); // calls MyModelPrefix_ADifferentModel

Is there some reason this simpler approach cannot work? I am attempting to cut down the number of confused issue tickets that will roll in from users who cannot work out why a prefix and namespace are both being applied etc.

As for the whole namespace appearing in the file name issue. I feel that this is best fixed in the model itself by specifying a $_table static class property. What are your thoughts on this? Seems like the most logical place to specify the table name to me - I prefer this to relying on the file name to be honest.

I don't know, but it just feels this feature is too clever and more difficult to understand than it needs to be. The code I have suggested above would seem to cover the 80% in my opinion, but perhaps I haven't given it enough thought like yourself. Please do feedback if you think I am being too close minded.

Does anyone else have an opinion on this? What would you find useful @tag ?

[markchalloner]

Agree with simplifying the prefixing. Taking a step back it is just prepending a string whether ist a class name prefix or a namespace.

I don't think adding the namespace to the table name by default makes sense, I would have though the (namespacing) majority would use:

Class: \App\Namespace\User
Table: user

i.e not app_namespace_user.

I'm happy if auto_namespacing tables is left out for simplicity, $_table can be used instead.

[treffynnon]

Thank you for your hard work and your pull request. I have gone in the direction discussed however with commit 0ca5d55

[Chumper]

Sorry to open this one again.

What should i do if i just want to prefix the tables?

for example: all my models are at a sepcific namespace: e.g. User: \database\models\User.
However, i want to give all my tables the prefix "test", in this case i just override the static public variable $_table.
So far so good, but here comes the problem:

If the prefix is dynamiclly configurable in my config file i cant set the static property with like: $_table = Config::get('prefix')."user" because php does not allow me to declare a static variable dynamiclly.

In this case my only solution is write directly after the class: User::$_table = Config::get('prefix')."user" with is kind of bad.

How do i solve this problem?

[treffynnon]

You can't change a static variable dynamically like you say so you need to add some code to Paris itself (or extend Paris) to automatically prepend the prefix when the table name is requested by code.

[Chumper]

Could this be a additional feature inside Paris?

Like

Model::database_prefix = 'test';

if yes, i will try to take a look into it.

[treffynnon]

It seems like a feature very few people would use and nobody else has
requested. I would suggest that you extend Paris for your local
implementation as it doesn't feel like something that should be added to
the library to me.