Fixed Can't export two event listeners of the same event and callback to development output

[Jeremy P]

Affected version

2.0.0 RC1

In earlier versions of XF2, it was no longer possible to set two event listeners to the same callback because of a unique index in MySQL. Fortunately, this was addressed here: https://xenforo.com/community/threa...eners-for-the-same-event-and-callback.135261/

Unfortunately, two listeners registered to the same callback won't be properly exported to the development output, presumably because the file naming scheme is dependent only on the callback name itself (and there can only ever be one file). This winds up being pretty annoying, as in practice I've still had to resort to creating multiple callback methods to work around this.

 

[Chris D]

Yeah I'm not actually sure it was the right decision to remove that index for this exact reason.

Could you explain what the use case actually is for having two event listeners to the same event and same callback? What's the particular issue with just having separate callback methods? Doesn't seem particularly laborious IMO.

 

[Jeremy P]

Could you explain what the use case actually is for having two event listeners to the same event and same callback? What's the particular issue with just having separate callback methods? Doesn't seem particularly laborious IMO.

Any events that take hints where you might want to run the same callback with multiple different hints. As it is, you can either run a callback for every single occurrence of an event, or for only one occurrence of an event. There's no real middle ground, which rather limits the usefulness of the hint system.

The most common use case in XF2 is probably with template events. I need to run exactly the same code to load exactly the same parameters into a couple of templates. As it is now, I have a bunch of boilerplate to work around this. It's not the end of the world, but it seems kind of silly to have to do this.

public static function templaterTemplatePreRender(
    \XF\Template\Templater $templater,
    &$type,
    &$template,
    array &$params
) {
    $params['foo'] = $bar;
}

public static function templaterTemplatePreRenderTemplate1(
   \XF\Template\Templater $templater,
    &$type,
    &$template,
    array &$params
) {
    static::templaterTemplatePreRender(
        $templater,
        $type,
        $template,
        $params
    );
}

public static function templaterTemplatePreRenderTemplate2(
   \XF\Template\Templater $templater,
    &$type,
    &$template,
    array &$params
) {
    static::templaterTemplatePreRender(
        $templater,
        $type,
        $template,
        $params
    );
}

 

[Chris D]

So, potentially, we could introduce the hint here to make it unique.

<event>_<class>_<method>_<hint>.json

It does mean there's still a restriction on using the same event, class, method without a hint but I suspect that will be ok.

 

[Jeremy P]

So, potentially, we could introduce the hint here to make it unique.

<event>_<class>_<method>_<hint>.json

It does mean there's still a restriction on using the same event, class, method without a hint but I suspect that will be ok.

Yes that would be excellent I can't really imagine any good use case for duplicate callbacks without hints.

 

[Chris D]

Problem is that it's potentially a large change this late in the game. But, even so, it's still better we make it now than later. Not promising anything, leave it with us

 

[Chris D]

We've brought back the unique index, but this time thrown hint into the mix.

We were getting somewhat concerned about the potential length of the file name that could be produced (given that the event ID, callback, and hint have a combined max length of 480 characters!) so we've made a larger change to the filename format. Although not ideal, there isn't really an ideal way.

We create an MD5 hash of <calback_class>-<callback_method>-<hint> and name the file <event_id>_<hash>.json producing file names such as app_pub_start_end_6ed181aff35952c5d0748969237f41b8.json.

The RC2 upgrade script will take care of deleting the old files and renaming for you.

 

[Jake B.]

The RC2 upgrade script will take care of deleting the old files and renaming for you.

How will this be handled for add-ons that aren't installed when you upgrade?

 

[Chris D]

Not really a lot we can do about that. My suggestion would be to make sure they are installed when you upgrade, or you have a built release that you can install from the _data files. Otherwise you will have to re-create the event listeners from scratch.

If you do install from a released version of the add-on then you can use the command xf-dev:export-code-event-listeners --addon <addon_id> but this won't tidy up the old files if they exist.

 

[Jake B.]

We have the _data directory gitignored, so I'll have to make sure everyone knows not to upgrade to RC2 unless they have their forks of the repository up to date with all add-ons installed (or not to upgrade it so I can just take care of it myself). Seems like a bit of a dangerous change to make, would there be any way to import all listeners from files whether they're using the new format or not, and then have it determine if the file name is correct and rename it at that point?

Since all of the details required to generate the filename (regardless of which format it uses) are available in the json file it seems like this would be sufficient to do before the listener is actually inserted

 

[Chris D]

It’s a one off so I would expect it to be sufficiently manageable with the steps I described already.

 

[Chris D]

We've made some changes here.

On import, we don't care what the file name is, so if you have the old format filename, these will still get imported.

On export, we will write the new file in the new filename format, and at that time delete the old format filename if we detect it.

We don't bother doing anything on upgrade anymore, so don't expect your filenames to change automatically, but if you do an export it should fix them to the new format.

We're likely not actually bothering with the unique index, either, and just relying on the entity to detect and prevent duplicates.

 

[Jake B.]

We've made some changes here.

On import, we don't care what the file name is, so if you have the old format filename, these will still get imported.

On export, we will write the new file in the new filename format, and at that time delete the old format filename if we detect it.

Great, seems much cleaner and less prone to issues down the road when someone realizes their code event listeners were gone