Collection and Query relationship

This is just a general question. A lot of inspiration comes from Laravel and might be different here.

So I looked at the Collection class and tried to extend it from the Illuninate\Support\Collection class. Both of them tries to do the same so it should work after a few adjustments and breaking method renames...

The main motivation for this is that currently it's not possible to map through collections (or the Builder class for that matter) without a foreach and temporary variables. (array_map doesn't work)

// Something like this would be the goal

// Get the ids
$ids = rila_posts()->pluck('id');

// Get all the categories
$ids = rila_posts()->flatMap(function($item) {
    return $item->get_categories();
});

But after extending it, I realized it won't work like that, because $this->check(); needs to be called before any collection method, which is not possible without redefining all the methods. (Which would be needed anyway for this, but that's not the point now.)

In Laravel the process looks like this:

// At this point we have a query object
$adults = App\User::where('age', '>', 18);

// And now it's transformed to a collection
$adults = App\User::where('age', '>', 18)->get();

So as soon you do something with the items, it queries the data. This is what you tried to do, but wouldn't this require to start with a Query object and inside the __call() and all the Iterator, etc methods decide to actually query the items and return a collection. So what I'm trying to ask is: why does the collection has to know anything about querying the database? Is there any reason for this?

// Any reason not to do this?
function rila_posts( $args = '' ) {
    return new Rila\Query\Posts( $args );
}

$posts = rila_posts(); // Query object
$posts = rila_posts()->type('news')->where('author', 1); // Still a Query object

// And now it's a collection
$posts = rila_posts()->type('news')->where('author', 1)->map(function($item) {
    return $item->get_something();
});

Why does the collection has to know anything about querying the database? Is there any reason for this?

Yes, there is. And that is the fact that while writing the collections, I did not have a lot of experience with Laravel (and I still don't). Nothing more.

Separating the query and the actual collection makes all the sense in the world, now that you enlightened me about it. Currently I'm quite occupied and I don't have time to implement this, but I would definitely like to keep the discussion open.

There are a few questions that need to be answered before we proceed, namely:

Can we keep this completely backwards compatible or we will have to release it in a new major version (ex. 1.0)?
Can we use the existing functionality from the classes that extend Rila\Collection and "simply" convert them to queries?
We already have a class named Query, which shares some functionality (trait Rila\Query_Args) with the posts collection. How do you think we should proceed with it?
If we are going to mimic the Query Builder of Laravel, should we try to keep our minds open to custom database tables? Considering how easy it is to link Rila\Item with a custom table, probably we should try to add some flexibility in the query builder, in order to actually allow proper OOP JOINs and whatnot.

Actually, the bigger question (than point 3) is why are you calling the object Query? If am not mistaken, you mean the Query Builder of Laravel. There the class is in the Database\Query namespace, if we don't need things like Database\Query\Expression and Database\Query\JoinClause in Rila, I guess we could simply call it Rila\Query_Builder.

I will try and answer the questions from my perspective.

Starting with number 2, I don't see anything that looks hard. I started with https://github.com/RadoslavGeorgiev/rila-framework/blob/master/classes/class-collection.php and removed all of the generic collection methods first. Those include:

offset* methods, which belong to ArrayAccess
All methods, belonging to Iterator
The sort method and the comparator

Next, there are a few methods, which we can easily move to the new collections, like

__toString()
implode()

After that I ditched $initialized and the check method, as instead of them we will actually have the transition from a query(builder) to a collection.

At this point things were quite clean, as you can see at https://gist.github.com/RadoslavGeorgiev/0247993ca18f47e70dde9f0086f1e5f4#file-stage-1-php

Starting from the bottom, the next mission is to change the filter method. Ideally, we would use the where method from the collection, however this would kill backwards compatability. I left the filter method in place but changed it so that it would filter the collection and directly return it. Here I am already assuming that the method, which instantiates a collection will be called get.

Next, I reworked the constructor, as you can see in Stage 3. I also switched the class to abstract. Anyway, my thinking:

If there is no request, we're starting with a basic (or maybe even empty) query. Let the child class prepare some arguments in its initialize method, if there is one. For example, post query builders will set the post type to post and things like that.
If we strictly have IDs, we can save them in the $ids variable and let the initialize method of sub-classes prepare the query based on them (ex. posts_per_page => -1, post__in => $this->ids)
If we already have objects (meaning loaded items), I am saving them in the items property, but now this seems wrong. I'll continue with that in a bit.
If nothing has already been found, I assume that the request contains normal arguments.

So the third point is an issue here. If we have already loaded all items, this means that there is no point of a having a query - we should directly create and return a collection, but that cannot be done in the constructor. So I decided it's time to create a static method that could return either a query or a collection.

I changed $item_type to ITEM_TYPE, created the static create method and made the constructor private. And I realized that ITEM_TYPE does not work as a constant, because there is no way to define that it's required. Therefore I switched to a static method, which returns the required item type. I did not make it abstract, because of what I read in this article. Basically, even though it's supposed to be okay to have an abstract static method, some people will get warnings and I want to avoid that for now.

...

Long story short, a couple of "small" things and a few hours later:

I finished the basics of the Query_Builder class.
Recreated the Collection method. We need to check which methods need to be re-implemented for backwards compatibility.
Implemented IteratorAggregate in order to allow using the usage of Query_Builder in a foreach loop.
Added a count method (for the Countable interface) that blindly creates a new collection and returns its count.

Here is the intermediate result: https://gist.github.com/RadoslavGeorgiev/2b5f9c2288be18f51f5f82029f8a7cc2

Checklist:

Check whether Query_Builder::all() can behave as the old Collection::all() for every builder type.
Check if any of the methods, called through __call will have problems because of the fact that a collection is instantiated every time it's called.
Monitor if all of the instances of $this->get() are (not) generating too many database calls.
Remember why the old collections were implementing ArrayAccess and somehow check if it is still needed.
Basically, consider some sort of collection (or at least data) caching until the request gets changed.
Check DW projects to see if anybody is referencing the Rila\Collection\* class directly. If not, we can just rewrite the default mapping for posts, the rila_posts() function and we'd be done. Of course, we'll need to rewrite the other collection types too, but that's easy.

Keep in mind that not a single line from the gist has been actually executed and/or tested.

Before I post this, quick answers of my questions so far:

Can we keep this completely backwards compatible or we will have to release it in a new major version (ex. 1.0): I think we can.
Can we use the existing functionality from the classes that extend Rila\Collection and "simply" convert them to queries? Pretty much
We already have a class named Query, which shares some functionality (trait Rila\Query_Args) with the posts collection. How do you think we should proceed with it? It's actually not affected at all so far. Maybe it would be cool if it also receives a ->get() method, which returns a collection, but other than that, we don't need to touch it at all.
If we are going to mimic the Query Builder of Laravel, should we try to keep our minds open to custom database tables? Considering how easy it is to link Rila\Item with a custom table, probably we should try to add some flexibility in the query builder, in order to actually allow proper OOP JOINs and whatnot. This one I am too tired to consider right now...

Is this a satisfying answer? :D

Woah, I did not expect such a reply. Thanks for the deep consideration. I read a lot in the last days, and now I try to share my ride.

As I already mentioned above, I think it's a breaking change. Even if we manage to get the same API somehow, it's still a big structural change, which won't be compatible to the state before or it would simply be confusing. Also I think there is a fight between modern PHP and "the WP way" and solving it (imho) would at least worth a consideration. I'm talking about PSR2, PSR4 and organizing files into folders according to namespace, etc. It might not be related to this topic or important at all, but these kind of changes would only make sense when there's a breaking change.

What we're talking here is more like Eloquent, than the Query\Builder. (https://github.com/laravel/framework/tree/5.5/src/Illuminate/Database/Eloquent) This is the name of the whole model and query building of Laravel. Starts with the Model class and uses the Builder and the Collection, which uses the Query/Builder and the Support\Collection. So it would look something like this:

Laravel	WP
Eloquent\Model	Rila\Item
Eloquent\Builder	Rila\Query_Builder
Query\Builer	WP_Query
Eloquent\Collection	Rila\Collection
Support\Collection	Support\Collection

So what we're trying to rebuild here is Eloquent, but instead of using the Laravel query builder, we would use WP_Query. But why exactly? Does using WP_Query improve plugin compatibility? The only thing matters is using filters correctly, otherwise there's no connection between Rila and plugins, right?

So my next thought was: why would we want to rebuild Eloquent? It has thousands of contributors and it's proven. What's best is that the philosophy is quite close, not accidentally of course, so in my head it doesn't look too forced. So I was thinking how to use WP database with Laravel, somebody must have nice solutions for this, and yeah, searching for 'Eloquent Wordpress' gives all the answers:

So some of these look good for either:

forking them
building on top of them
or just simply borrowing ideas.

But that 3rd one with 1500 stars looks rock solid. It has all the models, collections, queries, pagination, acf. Everything unit tested. Even the translation is here as alias: https://github.com/corcel/corcel/blob/master/src/Model/Post.php#L99

<?php
// Fresh WP install
// composer require jgrossi/corcel
// Theme's index.php

include __DIR__ . '/../../../vendor/autoload.php';

$params = [
    'database'  => DB_NAME,
    'username'  => DB_USER,
    'password'  => DB_PASSWORD,
];
Corcel\Database::connect($params);

$page = Corcel\Model\Post::type('page')->first();

$page->post_title = 'new title';
$page->save();

print_r( $page->title );
exit;

So this would be one option, building on top of it. It has no attribute mapping, no Site object, no templating, no create post type / add fields support, etc. They seem very strict about using it in a Laravel install, so the use case is different, but the base seems very stable and could be used by Rila.

namespace Rila\Model;

use Corcel\Model\Post;

class Post_Type extends Post
{
    // Logic for register_post_type, register_fields, mapping, etc.
}

Does that make any sense?

Okay, I'm going to combine the answers of everything here, but let's try not to mix topics.

Switching to PSR<*>

In the beginning I wanted to follow WP in terms of file names, but as the framework will never go to WP.org, I wouldn't mind switching to proper namespace-based file/folder structure. Considering that everything is in namespaces already, we can just switch, without it being a breaking change.

The only reason against switching is that we would practically kill the old history in GitHub, but I don't think anybody cares about it yet. If you'd like to continue the topic, please open a new ticket.

Using a WP-focused Eloquent fork

So what we're trying to rebuild here is Eloquent, but instead of using the Laravel query builder, we would use WP_Query. But why exactly? Does using WP_Query improve plugin compatibility? The only thing matters is using filters correctly, otherwise there's no connection between Rila and plugins, right?

So my next thought was: why would we want to rebuild Eloquent?

Caching. Once WordPress retreives a post from the database, it won't be retrieved again. If we use a custom query (builder) to load the post and its meta into the model and call $post->url (which internally uses get_permalink), we will be loading the same data twice. Not only that, but at some point, when the framework is not read-only anymore, we would have to manually purge caches in both directions, so basically we'd be shooting ourselves in the foot.
Since I don't think that we would be rebuilding Eloquent really , why would we rebuild WordPress with something external?
__WordPress is already loading a WP_Query__ in order to determine what is to be displayed. So no matter what we do, we wouldn't be improving things, but actually adding additional queries and whtnot.
Plugin compatibility is also something very important to consider. Even installing something as simple as https://wordpress.org/plugins/taxonomy-terms-order/ would require extra code, which would repel a lot of developers, interested in the framework.

Overall, the point of the framework is to provide a solid (or at least better) structure for projects. Although when using it, a lot of things are different, it's still WordPress under the hood in pretty much as many ways as possible.

Let's talk about this in person when we meet, because here the discussion would be endless, but overall I'm quite against the switch. Completely avoiding WordPress for the front-end and using a custom ORM is not an awful idea, but for me it's not an idea that fits together with the purpose of Rila at all.

Backwards compatibility

Even if we manage to get the same API somehow, it's still a big structural change

Well, isn't this the point of having APIs (because even something as simple as rila_posts() is an API)?

If we were to use something similar to the code from my previous reply, we would only have an issue if somebody is extending a collection class manually, but I doubt anybody besides me is doing that.

As I mentioned, the only difference API-wise would be that ->where() would not modify the collection, but return a new copy. And for me, this would be actually an improvement, because I have issues with the fact that calling post.children().where({ }) in different contexts throws an exception.

Summary

Let's go step by step. We can start by using the collections from Laravel in a nice and lean way, while maintaining backwards compatibility.

Once we have done that, we may take a look at the next piece, which would probably be the support for custom tables, ORM and etc. Before I get there though, I want to finish Ultimate Fields and see if and how I'd proceed with the custom tables there. As I've told you, my dream is to have a nice custom-database-table integration between UF and Rila and that would require a lot of precise and strategic moves.

So, anything particular that seems wrong in my implementation so far?

RadoslavGeorgiev / rila-framework