Bug? Upgrade method not being called in modules

[JayGee]

I'm not sure if this is potentially a core bug or an error/misunderstanding on my part. I'm working on a module where I'm pretty sure the upgrade method never gets called. I've tested with a simple message output and changing the version numbers up and down but I can never get any output from it. I've tried with and without using the ___hookable prefix on both my own method and the ones I'm calling.

public function upgrade($fromVersion,$toVersion) {
        $this->___install();
        $this->wire()->message('Upgrade method called');
}

I note here just today somebody else is flagging the same issue.

Does anybody have any insight into this or what I might be doing wrong?

 

[bernhard]

Did you do a modules refresh? Module version changes are only applied after refreshing the modules cache.

[JayGee]

Yes, I've been using the module refresh to trigger the change but it doesn't seem to fire and it does register the change in version number and displays the default upgrade message. But no custom functionality within the upgrade method seems to get called.

I've raised an issue now on GitHub just now actually: https://github.com/processwire/processwire-issues/issues/1634

As an aside - I've tried setting the version both up and down... I presume as the version numbers are strings not numbers any version change should trigger the upgrade method?

[BitPoet]

It should work if you define the upgrade method as hookable (___upgrade).

[JayGee]

53 minutes ago, BitPoet said:

It should work if you define the upgrade method as hookable (___upgrade).

Hi @BitPoet - I'm pretty sure I did try both with and without defining it as hookable, but will do another test to double check and report back.

[JayGee]

8 hours ago, Guy Incognito said:

Hi @BitPoet - I'm pretty sure I did try both with and without defining it as hookable, but will do another test to double check and report back.

I've checked now. Making it hookable doesn't seem to make any difference. The method just doesn't get called and the test message is never displayed.

I'm not misunderstanding the functionality am I? The method should be called whenever the version number bumps right?

[Robin S]

@Guy Incognito, as a general debugging tip: you should use a debugging tool like TracyDebugger rather than relying on message() because if some code somewhere does an exit() or die() then you might not see any visible message. For example, Session::redirect() calls exit(0) and maybe other core methods too. TracyDebugger has a Dumps Recorder panel that can capture any dumps that otherwise might get missed.

And when you're troubleshooting and trying to narrow down an issue you don't want any non-essential code executing. In your example you have:

public function upgrade($fromVersion,$toVersion) {
        $this->___install();
        $this->wire()->message('Upgrade method called');
}

But it would be better not to call ___install() before your debugging message because something might be happening in ___install() that prevents your message.

So probably this would be a good way to check if the upgrade method is being called:

public function ___upgrade($fromVersion, $toVersion) {
	bd($fromVersion, "fromVersion");
	bd($toVersion, "toVersion");
}

As for the problem itself, I tested here and can report:

1. The method name must be ___upgrade() and not upgrade(). The docs for Module make it sound like it is optional whether the method is made hookable with the underscore prefix but that seems to be incorrect because I can't see the method firing if the underscores are missing. So either that's a bug or the docs need to be updated.

2. The method is called when the module is next loaded, not necessarily when you do a Modules > Refresh. The core message gives a hint about this:

If your module has autoload set to true then it will be loaded immediately after the refresh (and on every other page load). But if it's not autoload and you want to see the ___upgrade() method called then you can force the module to load like this:

$modules->get('YourModuleName');

This isn't a bug as whatever actions you are taking in ___upgrade() will get applied in time for the next usage.

[JayGee]

Hi @Robin S thanks for your help on this. I have a few personal modules I want to update and then later submit on a couple of public modules we've adapted in-house, so wanted to be sure I got my head round what is going on with how upgrades are meant to work, so everybody's assistance and time here is much appreciated. ? 

 

10 hours ago, Robin S said:

as a general debugging tip.... you don't want any non-essential code executing. In your example you have:

Yes I typically would carry out a more in-depth debugging but in this case, as it seems you have confirmed, I suspected that I may have been misusing the method rather than other bugs being at-play so was trying to keep things simple to start with. But definitely agree with your advised approach.

 

10 hours ago, Robin S said:

1. The method name must be ___upgrade() and not upgrade(). The docs for Module make it sound like it is optional whether the method is made hookable with the underscore prefix but that seems to be incorrect because I can't see the method firing if the underscores are missing. So either that's a bug or the docs need to be updated.

Agree this isn't clear - I've always thought the hooks were optional.

 

10 hours ago, Robin S said:

If your module has autoload set to true then it will be loaded immediately after the refresh (and on every other page load). But if it's not autoload and you want to see the ___upgrade() method called then you can force the module to load like this:

Ok this is interested as the module I was experimenting with is definitely set to autoload and this hasn't been my experience... so there's likely something else going on I need to troubleshoot. But now at least I know where not to be looking!!! ?

 

10 hours ago, Robin S said:

This isn't a bug as whatever actions you are taking in ___upgrade() will get applied in time for the next usage.

I'll update and close the issue I raised accordingly.

[szabesz]

10 hours ago, Robin S said:

If your module has autoload set to true then it will be loaded immediately after the refresh (and on every other page load). But if it's not autoload and you want to see the ___upgrade() method called then you can force the module to load like this:

Just a quote from Ryan from the past (I forgot to jot down the source of the original, sorry...)

"In order to run an upgrade/download process, the module must be loaded and its upgrade() method called. With the exception of "autoload" modules, ProcessWire loads modules on demand, as requested from the API. This is an intended behavior. When you do a "Modules > Refresh", it should tell you about version changes it found and also indicate that it will apply those when the module is next loaded.

Module changes are applied when the module is next loaded, so it's not a matter of scheme (http, etc.) but rather just whether the module is needed and thus loaded for the request. Many modules are only used for rendering http requests (such as Process) modules, so it may have that appearance that http is required for some.

It's best that updates are only applied when a module is actually going to be used, since it is the module's own upgrade() method that is called to perform the upgrade. And loading a module in a context where it wouldn't typically be used might be problematic or cause errors. But if you have a need for to do this, you could try doing a foreach $modules and load each module individually $modules->getModule($name)... updates will be applied as each is loaded. Again though I think it may be potentially problematic to do that since this is not the way it's designed to apply upgrades."

[JayGee]

Thanks @szabesz that adds good context as to why it works like it does and it makes perfect sense.

[Robin S]

@Guy Incognito, could you please reopen the GitHub issue because I think there is a bug that needs attention? I've added a comment to the issue.

[JayGee]

4 minutes ago, Robin S said:

@Guy Incognito, could you please reopen the GitHub issue because I think there is a bug that needs attention? I've added a comment to the issue.

Done.

[JayGee]

@Robin S I just wanted to post back and confirm that now I know not to look for

___upgrade() method firing on module refresh alone I can see everything working exactly as you described.

 Thanks all. ?

 

On 10/26/2022 at 2:26 AM, Robin S said:

An aside - I wonder if the wording here would be better as 'applied when each module is next loaded' just to clarify that the refresh itself doesn't guarantee a module loading... although it may just be me that made this mistake!