Jump to content

Endless recursion in Page::resetTrackChanges


Go to solution Solved by adrian,

Recommended Posts

Not sure if this is by design - or if I'm perhaps missing the obvious. I'm just getting my feet wet with pw (2.5.3) and have created a partial sitemap template (nothing fancy, just recursing through a partial tree). When I added a page field to my sitemap template, selected a page and tried to save, I got a memory limit exceeded error. The problem appeared only when the page picked in the page field was the same as the one being edited.

    public function resetTrackChanges($trackChanges = true) {
        parent::resetTrackChanges($trackChanges);
        foreach($this->data as $key => $value) {
            
            if(is_object($value) && $value instanceof Wire) $value->resetTrackChanges($trackChanges);
        }
        return $this;
    }

Looking at this method, I guess it's easy to see that this will run in an endless loop if one of the properties of $page is once again $page. As a quick fix, this could be cured by comparing $this and $value in the conditional, but there'd still be a possibility of building circular patterns that have the same problem. Not sure how to go about that.

Edit: Of course, InputfieldPage prevents me from even assigning the same page here at all (my fault). Still, the check in Inputfieldpage::isValidPage appears to happen too late.

Link to post
Share on other sites
  • Solution

Just a quick comment - not sure if this is exactly the same problem or not:

https://github.com/ryancramerdesign/ProcessWire/issues/663

I think there still needs to be some work done for a proper fix. If you think it is the same issue, would you mind commenting there, or if not, maybe post a new issue.

Link to post
Share on other sites

I did my first fork and pull request with git - which turned out to be far less mysterious than anticipated - and Ryan already accepted my fix. Thanks!
 

Edited by horst
@BitPoet: Hope you don't mind, I added the link to the github repo.
  • Like 5
Link to post
Share on other sites
  • 3 years later...

Hey guys,

I have battling with this for way too many hours now and wondering if someone might have any clue what's going on, cause my brain is no longer working :)

I have a very customized user template with several page reference fields that allow selection of other users. The selectors for these page fields prevent selecting the current user so it's not the same as what @BitPoet reported and fixed, but it's certainly related.

Of the the page reference fields I have, it's only one that causes the trouble, but I can't see any difference between it and the others.

Here's the export of the tester (which is fine) and the subscriber (which results in the recursion).

Spoiler

{
    "tester": {
        "id": 127,
        "type": "FieldtypePage",
        "flags": 32768,
        "name": "tester",
        "label": "Tester",
        "derefAsPage": 2,
        "inputfield": "InputfieldSelect",
        "parent_id": "",
        "labelFieldName": ".",
        "labelFieldFormat": "{first_name} {last_name}",
        "collapsed": 0,
        "showIf": "user_type=1146",
        "editLinks": 1,
        "newPageParent": "",
        "operator": "%=",
        "searchFields": "name, first_name, last_name",
        "findPagesSelector": "template=user, user_type=1147, check_access=0, include=all",
        "optionColumns": 0,
        "allowUnpub": "",
        "defaultValue": "",
        "template_id": "",
        "template_ids": "",
        "findPagesSelect": "",
        "addable": "",
        "themeOffset": "",
        "themeBorder": "",
        "themeColor": "",
        "columnWidth": 100,
        "required": "",
        "requiredIf": "",
        "newPageLink": ""
    },
    "subscriber": {
        "id": 196,
        "type": "FieldtypePage",
        "flags": 32768,
        "name": "subscriber",
        "label": "Subscriber",
        "derefAsPage": 2,
        "inputfield": "InputfieldSelect",
        "parent_id": "",
        "findPagesSelector": "template=user, user_type=1146, check_access=0, include=all",
        "labelFieldName": ".",
        "labelFieldFormat": "{first_name} {last_name}",
        "collapsed": 0,
        "showIf": "user_type=1147|1148",
        "newPageParent": "",
        "allowUnpub": "",
        "defaultValue": "",
        "template_id": "",
        "template_ids": "",
        "findPagesSelect": "",
        "addable": "",
        "themeOffset": "",
        "themeBorder": "",
        "themeColor": "",
        "columnWidth": 100,
        "required": "",
        "requiredIf": "",
        "editLinks": "",
        "newPageLink": ""
    }
}

 

 

At the moment the only way I seem to be able to prevent recursion is to add a:

if($key == 'subscriber') continue;

to the foreach loop in the resetTrackChanges() method. 

There has to be something I am missing, but right now I have no idea what :)

Suggestions greatly appreciated!

Link to post
Share on other sites

@adrian, is there a chance that somehow the value is in fact the user being edited? You could dump the ID of $value to check.

Just a guess, but I'm wondering if the strict comparison here...

$value !== $this

...doesn't catch a user page because it is a object of classname User rather than just a plain Page.

  • Like 1
Link to post
Share on other sites
6 hours ago, Robin S said:

is there a chance that somehow the value is in fact the user being edited?

No chance as far as I can tell.

I have debugged the values of $this and $value and the only thing I can ascertain is that the recursion only happens with there is a selected value for the page field (this is probably to be expected), but they never match.

This is the result from being on the user with ID 1122 which has the selected subscribed user as 1100.

            if($key == 'subscriber' && $value !== '') {
                bd($this .':'. $value);
                continue;
            }

1122:1100
1100:
1122:1100
1122:1100
1100:
1122:1100
1122:1100

If I don't "continue" in that conditional, then I get the following - note the extra occurrences of the 1100 page which as I mentioned is the page the is selected from the page I'm currently on. Note that on page 1100 the subscriber field is hidden and nothing is selected.

1122:1100
1100:
1122:1100
1100:
1122:1100
1100:
1122:1100
1100:
1122:1100
1100:
1122:1100
1100:
1122:1100
1100:
1122:1100

etc etc etc

 

Thanks if you have anymore ideas!

 

Link to post
Share on other sites

You might find out more if you print out the names ($key) of all properties of type page and their value's (page) ids. There should be some kind of repeating pattern then that leads from subscriber to tester back to subscriber, but their might be (quite) some hops in between.

  • Like 2
Link to post
Share on other sites

@BitPoet - You're a legend!

I had dumped everything I could think of, including your suggestion, but this comment:

Quote

pattern then that leads from subscriber to tester back to subscriber

triggered something in my tired brain :)

It's because one of the other page fields points back to the initial user. I didn't think about the recursion happening from another field. The question of course now is how to prevent it happening. I feel like this is a PW bug because I think it's valid to tag a user from one as a "subscriber" and yet still tag the original as a "tester" for that user.

I feel like that foreach loop in resetTrackChanges() needs to keep track of page ids that have already been processed.

What do you think about this for a solution to propose to Ryan:

    public function resetTrackChanges($trackChanges = true) {
        parent::resetTrackChanges($trackChanges);
        $processedPages = new PageArray();
        foreach($this->data as $key => $value) {
            $processedPages->add($value);
            if(is_object($value) && $value instanceof Wire && $value !== $this && !$processedPages->has($value)) $value->resetTrackChanges($trackChanges);
        }
        return $this;
    }

PS I probably need to check that $value is an instance of Page before trying to add it :)

  • Like 1
Link to post
Share on other sites

It's a little more complicated than that - need to make sure that $processedPages is defined outside the method so it' not reset on each recursive call. I know Ryan will have his approach to this so I'm not going to bother refining right now, but if anyone is interested, here is the new issue: https://github.com/processwire/processwire-issues/issues/572

  • Like 1
Link to post
Share on other sites

That would only catch flat recursion though. The only way to catch deeper nesting I can think of would be to pass the starting object along. Something like this (untested):

    public function resetTrackChanges($trackChanges = true, $startingObj = null) {
        parent::resetTrackChanges($trackChanges);
        if(!$startingObj) $startingObj = $this;
        foreach($this->data as $key => $value) {
            if(is_object($value) && $value instanceof Wire && $value !== $startingObj)
                $value->resetTrackChanges($trackChanges, $startingObj);
        }
        return $this;
    }

Of course, that method signature and logic might have to be implemented by all Wire derived classes that implement their own resetTrackChanges method.

On second thought, that doesn't help either. You're spot on with keeping track of seen pages outside of recursion. On the other hand, multiple resetTrackChanges calls might happen in the same program run, so there needs to be a mechanism that cleans up the tracking list.

  • Like 1
Link to post
Share on other sites
3 minutes ago, BitPoet said:

Something like this

Thanks - I have referenced your idea in the Github issue - will see what Ryan says.

Cheers for the help!

 

Link to post
Share on other sites

No need for thanks. I hit my head on that problem a few times myself, so it's pure self-preservation in the long run ;) My approach above, as I figured out just a little too late, won't work out anyway, since it might not be original object that starts the recursion.

Link to post
Share on other sites
6 hours ago, Robin S said:

@adrian, until a fix is found a workaround is to set one (or both) of the Page Reference fields to dereference as a PageArray.
Related issue: https://github.com/processwire/processwire-issues/issues/152

Thanks for pointing this out - I'd forgotten about that issue. Unfortunately I have a lot of code and a lot of Page Reference fields that rely on single page / null so I am hesitant to change at this point in the project. I am testing your other solution though and so far so good

 

Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
  • Recently Browsing   0 members

    No registered users viewing this page.

  • Similar Content

    • By prestoav
      Hi all,
      I could have sworn I used to be able to use the site generic 'title' field as a sub field of a repeater field. However I've tried to do this on two 3.0.165 sites recently and, while it will add the title field in the repeater field setup, it wont save the repeater title sub field's content when the repeater is used in a page template and edited.
      It;'s not a big issue but I wondered if this was a known restriction?
    • By rjgamer
      Hi,
      is there a hook after the current (active) page got created? Or which method got called in the Page class after the Constructor of the current page got initialized?
      Thanks.
       
    • By rjgamer
      Hi guys,
      the field "redirect_last" of type DateTime got not updated. The update on the field "redirect_counter" works and got saved.
      Does anybody know what I did wrong in my code?
      if ($input->urlSegment(1) === 'redirect') { $page->of(false); $page->redirect_last = time(); $page->redirect_counter += 1; if ($page->save('redirect_counter')) { $session->redirect($page->website_url, 302); } } Thanks.
    • By theoretic
      Hi there! And thanks for Processwire!
      I have an interesting task which i cannot fulfill as i want. Maybe someone could help me please?
      Let's imagine a simple page structure of this kind:
      Category 1
      + Item 1.1
      + Item 1.2
      Category 2
      + Item 2.1
      + Item 2.2
      My task is to attach some items to more than one category, at least to show some items on different frontend category pages. With PW, it's a piece of cake. I've just created a field called Items (of type Page Reference) and attrached it to Category template. Since i have lots of items inside each category i preferred to use Page Autocomplete input for my Items field. The pages available for autocomplete are restricted by a very simple selector:
      template=item
      It works like a charm. But later i decided to make this autocomplete even smarter and to exclude current category children items from it. I tried to update my selector this way...
      template=item,parent!=(page)
      ...and oops, this broke my selector. My autocomplete founds nothing. Sorry, i had to replace the square braces by () because of this forum limitations, i swear i'm using square brackets in real-life selector!
      What am i doing wrong? And is there any way to include current page info in autocomplete-related selectors? Thanks in advance!
       
    • By louisstephens
      So I ran into a very strange issue today. I have a template with a pagetable and I went to add an item to it, when I went to select an image (for an image field) the page instantly threw up an error
      "ProcessPageSearchLive: No search specified"
      The page's content also switched to the image attached. This all worked perfectly last week (local mamp box). Has anyone experienced this before, and how did you solve it?
       

×
×
  • Create New...