Added hasManyThrough relation and store relations in variable

[vedmant]

Hi, I've added hasManyThrough relation and make it store relations in variable for not to making new query and object each time when relation is called.

Also I've removed get() from many relations, because it's impossible to add some additional conditions, for example I want to get user's post that are published, so I can call $user->posts->where('published', 1)->get(); However this change can break somebody's code if he used many relation in current way.

Sorry, but I didn't add documentation, I may add it later if will have time.

[avbdr]

Thank you for a pull request. You patch will be caching results limited to where() conditions of the first get(). As well your patch seems would break multisave feature. Can you work out this moments please and write a documentation for a hasManyThrough as Im not sure what usecase does it have?

[vedmant]

About caching not exactly, I've removed get() from has many and has many through, so it will return model object with prepared where and join (for through) conditions, so it doesn't return result itself. This is how it's done in other frameworks, for example Kohana or Laravel. About has one, it can be cached as it's only one object possible. Also it can be added something like refresh function for reinitializing whole model class in case DB related records was altered. Making query each time when has one relation have to be accessed is not good idea, Kohana and Laravel cache this type of results.

I'll add documentation little later. About usecase: for example you have tables users, roles, and user_role, in this case user can have many roles through user_role tables and backwards role has many users, in other words many to many relation, if to get user roles it will be following:

'roles' => ['hasManyThrough', 'Model_Role', 'user_role', 'user_id', 'role_id'],

Two last parameters can be auto generated, but in camel case? Usually DB schemes use snake case... By the way the same with timestamps, it's better to be able to set timestamps columns. Also I would suggest auto serializable columns or columns casting, so when loaded value is casted to specific type (or decode from json) so on... And one more, it lacks of belongTo relation, when local key points to far table, I didn't add as I didn't need it for now in my project.

About multisave feature, how it may break it, from what I see it just calls save() on attached to data array model, relations weren't saved into data array.

[avbdr]

I have applied your patch, runned tests/dbObjectTests.php, fixed 2 errors and gave up on 3rd.
restoration of a cached object is not possible because as all dbObject instances are storing reference to the same MysqliDb instance. so first get() will will reset the conditions.
if we will be copying MysqlidDb for each new dbObject, that will increase memory use
and as well will bring new issues which should be resolved. So the idea of caching is maybe good,
but still implementation is not fully finished.

As for the multisave:

        foreach ($this->data as $key => &$value) {
            if ($value instanceof dbObject && $value->isNew == true) {
                $id = $value->save();

it iterates over an array of data and if its finding dbObjects

[avbdr]

if (!is_array ($user->products->get()) || (count ($user->products->get()) != 3)) {

is it failing. :) first get() will clearout mysqlidb::_where array

[vedmant]

Yeah I see, didn't notice that it uses one instance of MysqliDb. Maybe it's good idea to make new instance of MysqliDb for each dbObject, I don't think it will increase RAM usage a lot as MysqliDb doesn't have too many dynamic properties, class instance itself doesn't take any extra RAM. It will need static mysqli property, and method to get it, then create new MysqliDb with the same mysqli.

About multisave still didn't get how it interacts with relations...

If I have time I'll work on it, I like this library, I think Laravel eloquent is too heavy for really small projects.

[vedmant]

Forget about multiple instances, but it worked fine with one instance, which is weird. There is now some bug with mysqliDbTests.php, can you check it?

[avbdr]

this is the case why you are not hitting the bug we were talking about. You are get()ing products only once now.

-if (!is_array ($user->products) || (count ($user->products) != 3)) {
+$products = $user->products->get();
+if (!is_array ($products) || (count ($products) != 3)) {

as well your change userId to user also avoiding 1 issue.
Id say we need to add your 'user' case as a new testcase
as hasOne have 2nd parameter optional and if its not set relation name itself used as a key.

Mysqlidb issue is caused by copy() seems after serialize() or clone its tricky to modify $_mysqli variable. I will take a look on this later.

[avbdr]

as well I have fixed a typo in preLoad :) thanks

[avbdr]

Ok. I have fixed a bug and expanded tests to cover your 'user' usecase.
With a new fix all tests should be passing as is with the only change for user->products to user->products->get()

[vedmant]

Great! I can merge changes later.

[avbdr]

please try not to commit unrelated things :)

[avbdr]

im still thinking about the use of this feature. you are not going to cache results but just a set of properties + 1 new $modelName () call per cache. Are you sure that they are so expensive to recreate without caching?

[vedmant]

It's only helpful for hasOne, and belongsTo (which is also have to be added). For hasMany and hasManyThrough it doesn't make difference, but it's simpler to keep all relations one way.

[avbdr]

but library already have hasOne caching:

        if (isset ($this->data[$name]) && $this->data[$name] instanceof dbObject)
            return $this->data[$name];
...
return $this->data[$name] = $obj->byId($this->data[$key]);

Maybe lets forget about all this mods and will finish implementation of hasMany, hasManyThough and belongsTo?

[vedmant]

I see, I didn't notice it, just used this lib for few days. Yes I agree, it will be enough just to edit many relationships.