Fixed Enqueueing jobs BC break

[Mr Lucky]

OK if I disable @Xon User Essentials

ErrorException: Fatal Error: Declaration of SV\UserEssentials\XF\Job\Manager::enqueueUnique($uniqueId, $jobClass, array $params = [], $manual = true) must be compatible with XF\Job\Manager::enqueueUnique($uniqueId, $jobClass, array $params = [], $manual = true, int $priority = 100) src/addons/SV/UserEssentials/XF/Job/Manager.php:12
Generated by: xxx Apr 13, 2024 at 11:07 AM
Stack trace
#0 [internal function]: XF::handleFatalError()
#1 {main}
Request state
array(4) {
  ["url"] => string(10) "/admin.php"
  ["referrer"] => string(41) "https://xf2.cafesaxophone.com/install.php"
  ["_GET"] => array(0) {
  }
  ["_POST"] => array(0) {
  }
}

 

[Chris D]

I'll speak with the team. We may decide not to break backwards compatibility here so @Xon might not have to make any changes to the add-on.

 

[Xon]

The function User Essentials extends enqueueUnique, is so the add-on can inject some additional parameters into a job, as I can't extend it in any other way:

public function enqueueUnique($uniqueId, $jobClass, array $params = [], $manual = true)
{
    if (Globals::$injectIntoJob)
    {
        $params = \array_merge($params, Globals::$injectIntoJob);
    }

    return parent::enqueueUnique($uniqueId, $jobClass, $params, $manual);
}

I haven't had time to look into the latest beta, but it feels like the soup of enqueue/enqueueAutoBlocking/enqueueUnique/enqueueLater/etc is getting somewhat unwieldly.

Updating the addon shouldn't be hard, just as long as I don't need to chase a moving target!

 

[Chris D]

@Xon I'm not sure we want to handle this by adding various new methods to avoid changing the signature of the current ones.

My current thought is we could add a new prepareEnqueueParams method. This would receive all of the arguments passed in to _enqueue and by default simply return the $params.

Seems like a sensible extension point and would allow through conditionals against all of the other arguments to target adding specific params to specific types of jobs.

Would that work for you?

I appreciate this doesn't automatically fix the issue of the method signature changing in the first place without the add-on being updated but it's probably a more robust solution going forwards. We can probably back port this to 2.2.16 as well, which may further help the migration. We don't have a target date for 2.2.16 right now but it should be some time before 2.3 stable. We'd most likely add the $priority argument to this new method even for 2.2.16 to further avoid inevitable issues for everyone if we were to change THAT signature.

 

[Xon]

That sounds good!

 

[Chris D]

The plans are evolving slightly but either way we'll get it sorted.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.3.0 Beta 5).

Change log:

Introduce a JobParams object with an appropriate `prepareJobParams` method called within `_enqueue` to allow various job parameters to be extended.

There may be a delay before changes are rolled out to the XenForo Community.

 

[Chris D]

So I've now essentially broken backwards compatibility even more! With this approach though the various methods can and potentially will change in the future, but hopefully we've now solved the need to extend them at all.

    public function enqueueUnique(string $uniqueId, string $jobClass, array $params = [], bool $manual = true, int $priority = 100): ?int
    {
        $jobParams = (new JobParams())
            ->setUniqueId($uniqueId)
            ->setJobClass($jobClass)
            ->setParams($params)
            ->setManual($manual)
            ->setPriority($priority);

        return $this->_enqueue($jobParams);
    }

We decided to encapsulate all of the job parameters into a separate object which is then passed into _enqueue without all of the other parameters. With that in mind, it's probably safe to extend _enqueue itself but for this particular use case, you may prefer to extend:

    protected function prepareJobParams(JobParams $jobParams): JobParams
    {
        return $jobParams;
    }

I think this tackles this particular issue in a more elegant way going forwards though doesn't remove the immediate pain of things breaking in the first place.

Feedback/thoughts welcome but it's tricky to find a balance here

 

[Xon]

@Chris D the only improvement I can think of is passing JobParams to \XF::extendClass().

 

[kick]

Feedback/thoughts welcome but it's tricky to find a balance here

Great! Will this information be published in HYS or somewhere else?