uri column not populated when using split mode+non-sync queue connection

[andreasbohman]

When using file for collection config and eloquent for tree+entries (I've seen it referenced as "split mode" in other issues), we've come across a bit of a weird bug. In combination with using something else than sync for queue connection (I've tested with both redis and database), the uri column does not get populated on the inital save when creating an entry. We first noticed it recently after upgrading from 1.x, and kept getting 404s for new entries.

To reproduce I did this:

1. Configure Redis locally
2. Fresh install of Statamic 4.20.0
3. In .env-file, set QUEUE_CONNECTION=redis
4. Install eloquent-driver 2.6.1, following the instructions for incremental IDs
5. in config/statamic/eloquent-driver.php, set collections.driver to file
6. Configure a collection as orderable (full yaml below)
7. Create a new entry, mark as published and Save & Publish
8. Try to access the entry on the site, it returns a 404.

Collection config pagez.yaml:

title: Pagez
template: default
layout: layout
revisions: false
route: '{parent_uri}/{slug}'
sort_dir: asc
date_behavior:
  past: public
  future: private
preview_targets:
  -
    label: Entry
    url: '{permalink}'
    refresh: true
structure:
  root: true

Sidenote: I did have to create the tree manually in the database in order to avoid the bug described in #198, but that seems unrelated to this bug.

Also, it does not seem to occur when using revisions.

We're having trouble pinpointing where this issue might be occuring, so any help on this is greatly appreciated. Thanks in advance, and thanks for all the great work being done with this package.

[ryanmitchell]

Interesting.

So uri is blank on initial save, but filled on subsequent saves? The uri is filled by https://github.com/statamic/cms/blob/4.x/src/Entries/UpdateStructuredEntryUris.php

I think we need to implement updateEntryUris() on the eloquent Collection repository, similar to how we have updateEntryOrder.

Given that direction, is that something you could work on as a pull request?

[andreasbohman]

@ryanmitchell Thanks for replying. Correct, uri does indeed fill on any subsequent save.

Well, it's already there, right? That's kind of what's confusing me, since I don't really see how that method could stop working just because another queue connection is used.

As this is a problem our client's licensed Pro site, naturally they'd very much appreciate any support we can get resolving this as it seems to be an issue more or less related to core Statamic functionality. Once I have some spare time though, I'll try to help out and do a little more digging.

[ryanmitchell]

So it is, my mistake.

That makes me wonder if the event is firing before the entry->save() happens, so the entry isnt found for its uri to be updated.

[ryanmitchell]

Actually I think it might be related to the same issue as #198. I have a PR on core (https://github.com/statamic/cms/pull/8658) that will enable us to resolve this by binding our version of collection tree to the stache collections repository.

[andreasbohman]

Interesting! I tested your core PR branch, and there was no difference, however the bindings seem to work properly at least. However, the more I think about it, I kind of wonder how it ever worked in the first place? I mean, updateEntryUris is a member of Collection. And if I'm in split mode, I'm using Statamic\Entries\Collection, i.e the Stache original. That will, in turn, call Statamic\Stache\Stores::updateEntryUris, correct? I'm nowhere near the Eloquent entries in that case.

Wouldn't I need my own CollectionRepository class that essentially only overrides updateEntryUris, but does everything else as-is when I'm in split mode? Kind of how the original version of this package worked back in the day? Still, I realize I may not understand all of the ins and outs of this - clearly, it works somehow when using sync queues.

[ryanmitchell]

The core PR is only half of what is required. I need to also make a PR here when it merges to bind the eloquent tree classes. I think it will work then.

[ryanmitchell]

I've opened the eloquent PR here - https://github.com/statamic/eloquent-driver/pull/203 - let me know if this helps with your issue.

[andreasbohman]

Alright, so I've done some more testing with core 4.21.0 and the (now merged) PR #203 . Using the same test case, the same issue persists; I have to save the entry an extra time in order for the uri column to populate.

Continuing on my "theory" from my earlier comment above, I created my own CollectionRepository, which extends the original stache one and only overrides the updateEntryUris method (stolen from earlier versions of eloquent-driver):

public function updateEntryUris($collection, $ids = null)
    {
        $query = $collection->queryEntries();

        if ($ids) {
            $query->whereIn('id', $ids);
        }

        $query->get()->each(function ($entry) {
            app('statamic.eloquent.entries.model')::find($entry->id())->update(['uri' => $entry->uri()]);
        });
    }

Now, the issue is gone! Again, as far as I can tell, something along those lines is needed in split mode, since the CollectionRepository included in eloquent-driver is entirely bypassed in that case. Ultimately, I suppose it's an issue of separation of concerns - eloquent entries are currently dependent on a member of CollectionRepository to work properly.

Question is how to solve it in an unobtrusive way? I mean, you could override the CollectionRepository silently within eloquent-driver if split mode is detected, but that seems a bit aggressive. Maybe introduce a third driver option?

[ryanmitchell]

Thanks for the time on this @andreasbohman.

The strange thing is that $collection->queryEntries() queries through the Entry facade: https://github.com/statamic/cms/blob/1c287bc5dd89169b19cc550bf2fd94f8aab25969/src/Entries/Collection.php#L263-L266

So it should be getting Eloquent entries in your case, not Stache entries.

Are you able to set up a quick sample repo showing the issue so I can try and dig into what I've missed when allowing splitting repositories?

[andreasbohman]

Right, $collection->queryEntries() is working correctly, no problem there. But look here: https://github.com/statamic/cms/blob/1c287bc5dd89169b19cc550bf2fd94f8aab25969/src/Entries/UpdateStructuredEntryUris.php#L33

The collection instance will still be the stache one, since we're using file driver for collections in my case. Since updateEntryUris is a member of CollectionRepository, the following method is the one that will be called in the end, no? https://github.com/statamic/cms/blob/1c287bc5dd89169b19cc550bf2fd94f8aab25969/src/Stache/Stores/CollectionsStore.php#L82

I've set this up so that you can try for yourself: https://github.com/andreasbohman/eloquent-uri-issue-demo

Note that you have to have to use Redis or Mysql for the queue connection. You can toggle my fix on and off by setting config collection.driver to hybrid.

[ryanmitchell]

Yep you've followed the rabbit trail all the way to the bottom. Good work. Sorry for adding confusion.

@jasonvarga any thoughts on this? updateEntryUris() may belong better on the CollectionTree more than the Collection now? The same issue is going to exist on updateEntryOrder()

[andreasbohman]

Alright so, it seems that I'm responsible for the majority of the confusion here :) It turns out that the original, direct issue at hand where uri would not populate when using non-sync queue was actually due to a local configuration error on our end where a queue worker would not run as expected. Very clumsy of me not to double check this before opening this issue, apologies for that.

uri does indeed seem to update with the UpdateCollectionEntryOrder job. As that job simply runs save() on each entry, evidently the uri gets set somewhere in there, I haven't asserted exactly where though. Possibly, this could be worked on and optimized. I don't know if this was intentional from the start, or if it's an unintentional side effect, but I suppose it may be worth having the discussion anyway about updateEntryUris and possibly separating it from Collection.

Again, sorry for the confusion. You may want to close this issue and move the discussion about updateEntryUris/possible optimization somewhere else.

[ryanmitchell]

Ah great, glad you got it solved!