Support for Plugins

[claviska]

This issue is for plugin planning and development.

TODO

• Events
  • add listeners
  • remove listeners
  • dispatch events
    • post.*
    • tag.*
    • user.*
    • navigation.*
    • setting.*
    • route.*
    • ui.*
    • admin.*
    • system.*
      • ready
      • terminate
• Database table
  • Fields TBD
  • Update default.database.sql
  • Add upgrade instructions to INSTALL.md
• Plugin class
  • Method to scan content/plugins (similar to Theme::getAll())
  • Method to retrieve enabled plugins
  • Methods to enable/disable plugins
  • Method to delete plugins
• [] Documentation

[claviska]

I'm going to move plugins into beta because the current version seems pretty stable and we could benefit from not worrying so much about semver just yet. (As they say, move quickly and break things! 😖)

In 7a7c2ca, I added a few basic methods to handle events. I envision them working a lot like jQuery's on and off methods.

To listen for events

// Without a namespace
Postleaf::on('post.save', function($event) {
    // Do stuff
});

// With a namespace
Postleaf::on('post.save/yourNamespace', function($event) {
    // Do stuff
});

Namespacing is useful so you can stop listening to a specific event without breaking all of its other listeners. By convention, they're separated by a slash.

To stop listening for events

// Stop all listeners for a specific event
Postleaf::off('post.save');

// Stop all listeners for a specific event with a specific namespace
Postleaf::off('post.save/yourNamespace')

// Stop all listeners for all events with a specific namespace
Postleaf::off('/yourNamespace');

Dispatching Events

The app will dispatch events like this:

// Dispatch the post.save event
Postleaf::dispatchEvent('post.save', $event_data);

// Dispatch the user.delete event
Postleaf::dispatchEvent('user.delete', $event_data);

I'm thinking that event data should be passed to this method by reference so callbacks can make necessary changes that feed back to the app.

More info

• Event names can contain dots . to indicate their relation to resources or specific sections of the app.
• Namespaces are delimited by a slash /.
• Events are dispatched in the order they're created.
• Events can be stacked, even with the same namespace (i.e. creating multiple listeners with the same event/namespace won't overwrite existing listeners).
• Subsequent events can be canceled by returning false from a callback. I'm not sure this should work this way — maybe we need to control the event data and add a flag (think of how event.stopPropagation() works in JS). Open to suggestions.

I think this is a good start, but I'd like some feedback before proceeding. The next step is to add event hooks to various parts of the app and expose some additional functionality (such as the ability to add routes).

[karsasmus]

This looks great.
These 3 additions (on, off, dispatch) are so simple yet so powerful. Thanks for the simplicity.

Adding an event

public static function on($event, $callback) {
  $options = explode('/', $event, 2);
  // my suggestion
  $resource = explode('.', $options[0], 2);

  // Attach the listener
  self::$listeners[$resource[0]][] = [
      'event' => $resource[1],
      'namespace' => $options[1] ?: null,
      'callback' => $callback
  ];

Dispatch an event

public static function dispatchEvent($event, $data = null) {
        // my suggestion
        $resource = explode('.', $options[0], 2);
        // Loop through all listeners
        foreach((array) self::$listeners[$resource[0]] as $listener) {

If we do so, the loop doesn't need to iterate over perhaps 1000 items, it only needs to iterate over the $resource[0] items.
This might not be important at the moment. But if someone has activated 10 Plugins with a lot of listeners, the performance could suffer.

What do you think?

[claviska]

The loop could definitely be optimized, possibly so it only loops over the event name (without the namespace). I'll play around with it.

[claviska]

I benchmarked event dispatching to compare a few different solutions. The results were:

• foreach (first solution)
  • 100,000 events dispatched
  • ~ .27 seconds
• array_filter
  • 100,000 events dispatched
  • ~ .51 seconds
• Splitting by event name (proposed solution)
  • 100,000 events dispatched
  • ~ 3.2 seconds (larger because of the call to explode?)

Thinking a bit more about this, I decided to take @karsasmus' approach and store each event type like this:

self::$listeners[$event] = [
    'namespace' => $namespace ?: null,
    'callback' => $callback
];

Which allows us to iterate over just the events we're targeting in dispatchEvent(). This brought the time down to ~ .23 seconds for 100,000 events.

I also changed the namespace delimiter to # which feels more appropriate than /. I considered following the eventName.namespace convention, but I think being able to categorize event names with dots will keep them organized and might have additional benefits down the road.

[claviska]

I added some post events to experiment with in 5d434ed. For now, you can test listeners by adding them in index.php after Postleaf::run().

Here are a couple examples that append the save time to posts when adding/updating:

Postleaf::on('post.update', function(&$event) {
    $event['post']['content'] .= '<p>Post updated on ' . date('Y-m-d H:i:s') . '</p>';
});

Postleaf::on('post.add', function(&$event) {
    $event['post']['content'] .= '<p>Post created on ' . date('Y-m-d H:i:s') . '</p>';
});

Note that $event needs to be passed by reference for changes to stick. This is the most elegant approach I could come up with for that.

Any thoughts on this before proceeding with other events?

[karsasmus]

I reviewed 5d434ed.
It looks good.
I've a suggestion for naming convention of events.

You fire an event before and after an action. So we should name the event resource.beforeFunctionName and resource.afterFunctionName.

Example:
post.beforeAdd, post.afterAdd

If we do so, we avoid confusion about the data an event returns.

Example:
line 354 of Posts.php (get a post) the event "post.retrieved" gets fired with a post as event data.
line 597 of Posts.php (getMany), the event "post.retrieved" also gets fired but with an array of posts as event data.
One event, two possible returnable values.

If we follow my suggestion, the events would be called "post.afterGet" and "post.afterGetMany"

---

Should we also add an event for startup and shutdown?

Postleaf::dispatch('postleaf.startup', $eventData);
Postleaf::dispatch('postleaf.shutdown', $eventData);

[claviska]

The events use present tense and past tense verbs:

post.add
post.added
post.update 
post.updated
post.retrieve
post.retrieved

The present tense is dispatched as soon as possible and the past tense fires after the operation completes. This is less verbose and similar to the convention Bootstrap uses for their events.

Should we also add an event for startup and shutdown?

Definitely. TODO updated.

[karsasmus]

Ok, present and past tense are fine.

How can I support you?

[harikt]

I suggest using before and after . Present and past tense are very confusing.

[claviska]

@karsasmus if you want to get started on tags/users/navigation/settings that would be great. Just follow my lead on posts. 😀

Will we need to be able to prevent an event from firing? If so, we should think about how to handle that too.

[karsasmus]

Ok, I'll begin tomorrow 😁

I think, the events should always get fired, except we implement a cli.
When we use the cli to communicate with postleaf, the events shouldn't get fired.
We implement the event system to expand postleaf. If we use the cli, we want to use the core functionality so we don't need any expansion.

[harikt]

When we use the cli to communicate with postleaf, the events shouldn't get fired.

I believe the events should be fired. No matter whether it is cli or web.

[claviska]

I agree, any event that happens should dispatch an event. In the case of alternate interfaces, they may want to register their own listeners separate from those of plugins.

But yeah, one thing at a time. 😎

[karsasmus]

A proposal for the plugins database table.

DROP TABLE IF EXISTS `__plugins`;

CREATE TABLE IF NOT EXISTS `__plugins` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `name` varchar(51) NOT NULL,
  `author` varchar(51) NOT NULL,
  `version` varchar(8) NOT NULL,
  `install_date` datetime NOT NULL,
  `enabled` tinyint(4) NOT NULL,
  `enabled_date` datetime NOT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `id` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ;

Should we also save the plugin config (from plugin.json) to database?

I know, we haven't implemented yet the event system, but if this proposal is ok for you, we could check off the todo. 😇