To trigger a plugin event, please don't request your own dispatcher. Use JFactory::getApplication()->triggerEvent()

Also, I think we don't really need own events for each case, just pass the action as an argument to the generic event.

I see what you're saying

Not sure what to do about the app inst errors, as the test is operating outside of the application. So I switched back to creating a dispatcher

Tests which are interacting with anything in JFactory usually have to set
the singleton instance it needs to the public vars. This is also usually a
bad sign that things are getting couples together somewhat heavily.

Even with calling JEventDispatcher you'd still need to update the tests to
reset its singleton instance after the test otherwise you're causing side
effects and altering global state in the test case leaving a pre-configured
instance in place.

On Monday, May 30, 2016, Michael Richey notifications@github.com wrote:

Not sure what to do about the app inst errors, as the test is operating
outside of the application. So I switched back to creating a dispatcher

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#10681 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAWfoSs6IHS3w7WyWb65-xBNNzgAaSRlks5qG1GRgaJpZM4Ip-Xf
.

I'm not sure where to go from here...

Can you point me to an example of resetting a singleton instance in the test?

So I switched back to creating a dispatcher

Please use the Factory one and fix the tests instead ????

Do we need the triggerEvent to be private? And why use such a function in the first place, when a trigger can be fired with mere two lines of code.

Why a common event and not separate events per action? Each method signature can be matched to relevant action (not necessarily exact however).

Having a common event for all triggers would eventually force a plugin developer to use a if/else or switch/case within the handler always. And subsequently each handling action would be carried out by other protected methods in those plugins.

IMO, it could be something similar to:

$dispatcher = JEventDispatcher::getInstance();
$dispatcher->trigger('onFilesystemFileCopy', array($src, $dest, $path));

and the plugin method signature as:

public function onFilesystemFileCopy($source, $destination, $path = null);

Do we need the triggerEvent to be private? And why use such a function in the first place, when a trigger can be fired with mere two lines of code.

Personally I would get rid of the triggerEvent method as well and just use the two liner.

Why a common event and not separate events per action? Each method signature can be matched to relevant action (not necessarily exact however).

Imho, a single event is sufficient when the action is passed as an argument. I think more events don't make plugins simpler, in contrary. I don't think there will be many plugins which act differently on each action. Most will act similar or the same on multiple events, or only act on one action anyway.

Individual event names is how I started to build it, which is where the triggerEvent method came from - I'd pass an action and it would build the event name (a copy action became the onFilesystemCopy event within triggerEvent).

I can see both sides though... On one hand, if I'm only working with one event, it would be convenient to define just the one method. On the other hand, a switch gives flexibility to stack event cases under one switch and only define cases which apply. Each has merit and I'm not for or against either.

My need for it is fairly simple. I want to trigger file mirroring actions between servers.

I made triggerEvent private because there really isn't a reason for it to be called outside of a filesystem event.....but now that you mention it - yes, there is a reason - firing a change in the event that reprocessing is required...

Although my first preference would be to use separate triggers, I don't have any strong reason to discourage a single event.

In that case I'd like to suggest passing method as the first parameter for a consistent parameter position.
Passing __METHOD__ is also not future proof, as we may some day move around and rename the class to something like \Joomla\Filesystem\File, Who knows!

Something like this should be good (the handler function assumed to be supporting variable number of arguments based on context, else we may have associative array instead):

$dispatcher->trigger('onFilesystemEvent', array('filesystem.file.copy', $src, $dest, $path));
// ... and ...
$dispatcher->trigger('onFilesystemEvent', array('filesystem.folder.copy', $src, $dest));

Yes, please use context instead of __METHOD__ and remove function triggerEvent()! It makes code more transparent and @izharaazmi is right concerning argument "not future proof".

@stutteringp0et Please put your comments or make changes as you like so that we can move this ahead.

I still think coupling the filesystem code to the database is risky and unnecessary. With this PR a filesystem call will result in a database query being executed. There could be unforeseen consequences. IMHO, the use-cases you mention would be better handled by the operating system.

I see your point, but the operating system options are not as fast-acting as this could be, and not all hosts allow installation of things like pyrsyncd or unison for mirroring operations. For things like image resizing or virus scan you're stuck with crontab directory scans - not exactly the real-time solution I'm hoping for.

It's sad to hear so much negative against the idea.

For image resizing and virus scanning, I think you'd be better off adding an ImageUploaded or FileUploaded event to the media manager where you have "semantic" knowledge of the intent behind the event.

If you're trying to mirror filesystems across hosts that don't allow installation of server binaries then I think you're going to end up in a world of pain. Distributed systems are hard. If a client asks you to do that, my advice would be to run away.

I'd be a lot more positive if I thought this was a good idea.

I suppose I'll go the inotify route for my own systems and everyone else can just fend for themselves.