Split the action API

[JDGrimes]

In https://github.com/WordPoints/hooks-api/issues/123#issuecomment-209631801 and https://github.com/WordPoints/hooks-api/issues/123#issuecomment-216276994 we discussed how the event/action API that we have created is really composed of two separate APIs, that we are likely going to want to more-or-less split at some point. The biggest question is how far we want to split them in the initial version. A part of this question is the question of how much of an entity change API we want to introduce in the initial version. At this point, the ideal would be for us to do as little on that front as possible, because of the extra work that would require. We'd like to just get the initial version out ASAP. As long as we provide a robust framework for the events, we should be able to completely switch out the API later if needed.

This ticket therefore intertwines with #128 and #126.

Main action points:

Currently, there are two issues with the hooks API:

1. It is too specific to the WordPress action and filter API. Certainly that should be a main focus, but a more generally interface with broader application would be nice.
2. It is too generic in the way that it forces entity changes to use the same API as events. There may be room for some meta API here but I think that right now both of these are suffering.

Perhaps then, the router should trigger the event API, but not encompass it in any way. Right now it does. So splitting that out might be a first step.

[JDGrimes]

Here is what I'm thinking. We can't introduce the MySQL CRUD API in the initial version, because it would be too much work, and it should really have a lot of testing. I also doubt that we really want to pursue a new UI at this time either. But possibly we would want to introduce the entity change API, but continue to use the existing UI for now. In fact, I suppose that the best way to do that with the least work would be to trigger it through the events API, since the UI is already designed around events. The main question is whether that would require us to leave the action types/hit types around, and whether we want to do that. I'm also not entirely sure that we can introduce the entity change API itself without a considerable amount of work, so that's another thing that we need to consider.

Alternatively, perhaps we could skip the events API and just hook into the action API directly to trigger the entity change API. But then, without the events we'd still have to do some work to prepare the UI.

[JDGrimes]

What to do with action types and hit types?

So, what do we want to do in regard to the action types/hit types? The reason for this question is that currently our action API is far more complex than necessary for it to just handle simple events. The only purpose for the action types and multi-response reactors is for the entity change events. Once we move these entirely to the entity change API designed specifically for them, there will no longer be a need for this added event complexity. Each event would just trigger one type of action, and each reaction would trigger just one reactor, and each reactor would have just one response/hit type.

So there will come a time when this added complexity is no longer needed. But if we keep it around, how will we ever be able get rid of it? It would likely have to stay around indefinitely, even when it was no longer necessary. Which isn't a pleasant thought.

What harm does this added complexity really do though? Well, it requires maintenance, and it makes things more complex, which in itself is bad for many reasons. Of course, the simple events could still work even with this added complexity, but it would also be prone to abuse by APIs that would essentially be _doing_it_wrong().

[JDGrimes]

Isolate hit types

Perhaps there is a way that we could isolate the hit types from the basic events API, thus allowing the hit types to be deprecated and eventually removed in the future without the events API proper being affected.

One potential method of doing this would be to reintroduce the concept of firers or "action type handlers" into the router. The basic events API would then be one thing triggered by the router, while the version with the action types and hit types would be another. Ultimately though, both of these would still have to use the events API, if we're still using that in the UI and with the reactors. So the question is whether it would be possible to extract things this way and have everything still work, but I think it would definitely be worth exploring. It makes a lot of sense to make the router a more generic thing, I think. The main purpose of the router is simply to provide a means of loading code that needs to be hooked to actions on the fly, instead of having to load the code on every single load just to hook it to an action that might not be called. That is something that is much broader than the hooks API itself (though it is more keenly felt there, of course).

[JDGrimes]

Will we ever want action types?

By removing the action types though, we are saying that we don't think that event actions should ever be allowed to fire relative to one another. Or, that is, we are saying that we aren't going to make that a permanent integral part of the events API. This means that we could never introduce a generic spam API for example, to allow any event hit that has occurred to be marked as spam. That is, without a custom action type API anyway.

But would such a thing make sense anyway? Since these events would be endlessly repeatable, like visiting a site. But I guess the question is, is there ever going to be a time when we'd want to have an API that did support more toggle-like events. Like when an editor publishes a post, they aren't related to the post in any way, they just published it. But what would indicate that such an action was "spam"? That doesn't really make a lot of sense. Although the action can be reversed. But that doesn't mean that it didn't happen, we may not want to reverse whatever effect it had. I suppose that we would probably be better off just introducing a per-implementation reversal API in that case [e.g., allow points logs to be reversed from the points logs admin screen]. Or else have a hit log page where the admin could reverse any hit [from any implementation]. And like we said above, we could reintroduce a custom action API if we really needed to.

But then, if we're eventually going to want such an API anyway, is there really any reason to split out the action types/hit types at all? Is having the option of removing it worth it?

[JDGrimes]

Problem: condition settings are stored keyed by the action type (see #103). So the extension code for the API and the UI are both closely tied to the action types, and there would be no easy way to make that optional in a manner that was isolated without much code duplication, I think.

Ironically, the reactors aren't very closely tied to the action types/hit types in a way that couldn't be isolated.

[JDGrimes]

But we were doing this somehow without the hit types before. Let's just go back to that, maybe? What would the implications of that be?

Well, I suppose it would mean that extensions wouldn't be able to be "firer-agnostic" anymore (#103 again). Or at least, that would have to be achieved in a different way. But currently that isn't a feature that we absolutely have to have for the points hooks anyway. It isn't really a feature that is much use to us at the moment at all.

[JDGrimes]

Even if we remove the hit types, we still have always had the action types in some form, and that's something that we may ultimately want to remove. So we'd still need to figure out what we're going to do with these toggle events and the logic related to action types.

There are two parts to this. First, we'd like to isolate the logic that relates to the action types, so that it could ultimately be removed. This includes the extra data about "toggle off" actions that events are currently registered with.

However, just registering those "reversal" actions separate from the events isn't the whole solution, because we still end up stuck with these events hanging around after we deprecate the action types part of the API. This is the second part of the problem.

In other words, these toggle events themselves will really be no longer needed after we transition away from this. They are a fundamentally different type of event (or at least we'll be using them in a fundamentally different way). We might want events similar to them—for example, we might want to award the user who publishes a post. But that would be a different behavior, and to make the transition we'd have to register an entirely different event than the current post publish event. We'd have to do this because we'd need to keep the old event around with the old behavior for legacy reasons. (We could in theory accomplish the same thing by just using a single event, but making it behave the new way on new installs and the old way on legacy installs. However, that would mean that legacy installs wouldn't be able to use the new behavior at all, which probably isn't the path we want to take.)

[JDGrimes]

So it would seem it isn't just the logic that we'd need to isolate, it is even the events themselves. But this doesn't mean that we have to introduce separate event registries, although I originally thought of this. Actually, I doubt that separate event registries would work anyway. But all we really need to do is provide a way to stop registering these events when the time comes, and to possibly register similar events beside/instead of them. Of course, that's really already possible, so we don't actually have to do anything.

In preparation though, we might want to go ahead and "isolate" these events by registering them within a separate function, that could then easily be unhooked as needed when the time comes.

[JDGrimes]

Generic Router?

As far as just converting the router to something totally generic, I don't think that that is actually a good idea. I'm sure that we might need such a generic router in the future, but all of this router's logic is dedicated to the action objects which connect actions with entities. So its action registry is designed around registering not just an action but also information about its args, that is then used to handle entities using generic logic. It is possible that these features in and of themselves will have other uses for other APIs in the future, but I think moving to a generic router at this point might simply be introducing an extra layer of complexity. We'd end up with two action registries (one for the basic router and one for the action args router), which would be a waste of memory. So I think that we should just treat this as a specialized case, separate from a generic router.

[JDGrimes]

Action objects needed?

Are the action objects themselves necessary to a simple events API?

In theory, I suppose that the API could just be triggered directly from functions hooked to the actions, instead of having to use a router and the action objects. The main benefit of doing it with the action objects, I guess, would be that the code for the action objects can then be lazy-loaded.

[JDGrimes]

Move event mapping to handlers

Originally I was thinking to insert the handlers where the router currently calls $this->fire_event(). However, I'm thinking now that it would be better to insert it before the events are brought into play at all. Meaning that the event handling logic will be in the handlers instead.

The reason for this being that it would allow us to have a separate event index/action index for the toggle events. They'd still use the same registry, as we said, but they'd be mapped to actions based on different indexes.

What is the benefit of this isolation? With the action types removed from the main logic, we'd otherwise have to register all of the all of the actions in a single group, both toggle on and toggle off, for each of the toggle events. That is the only way to make the toggle off actions trigger that event and ultimately our handler with action type mapping, because of the way the router logic is constructed. In other words, if the event mapping is before the handlers are called, the toggle off actions will have to map to the events just like any other action would. They'd be indistinguishable from toggle on actions, until we were inside our handler with the action mapping.

While I'm having difficulty formulating the exact problem with this action mixing, it doesn't seem good. One potential side affect, if we just loop through all of the handlers when each action is called, would be that other handlers wouldn't realize this wasn't a toggle off fire of the event instead of a regular fire, since they wouldn't have the benefit of the action type map. Of course, that could be avoided by having the actions be registered on a per-handler basis, so that only the handler specifically listening for that action would be called (very much like we used to do with the firers).

Nonetheless, this action mixing doesn't seem ideal.

(We could split the different action types from all being registered together this way by registering separate events for the toggle on and toggle off actions. However, I'm not sure that this would be a complete solution, or that we want to make all of those events available in the UI, which would be one side effect of that.)

[JDGrimes]

Logs and action types

Would we still need some kind of log with the entity change API? Well, the entity change API itself wouldn't use a log, it would just accept whatever triggered it in good faith. However, this means that we have to take care when triggering it from the hooks API. Which means that we'll have to continue to keep logs there.

The problem is that one of the columns in the log table is for the action type. So if we are ultimately going to remove the action type, we need to remove that column from the table. I guess actually we wouldn't have to, but eventually it might be unused. Perhaps though it would be better for us to go ahead and remove it now. Maybe the action type could be stored in the hit meta if needed. Or else we could use an entirely separate table.

[JDGrimes]

Completely discarding action types?

Are we really going to want to get rid of action types completely though? It seems that there is that possibility that we'll actually find them useful. Might we not even want to introduce other action types for the repeatable events? Well, I guess truly repeatable events couldn't really have other actions that were relative to them, because they are entirely circumstantial, not relational. Event so, I just keep thinking that we might come across some other event type in the future that we'll want to use action types with. We can remove them from being an integral part of the main router and API, but there is no getting around the fact that this will cramp their use.

For example, it will mean that we have to rethink our logs queries. I'm not sure how that will affect extensions like the periods. It also means that we can't use the extensions relative to the reversal actions or other action types. At least, not with the way the code currently stands. Though as we said above, we don't currently need that.

Actually, the periods extension doesn't take the action type into account anyway, so we are OK. Perhaps it should, but if we're removing handling for reversal actions anyway, then it really doesn't matter.

[JDGrimes]

Splitting the hooks API?

Basically what we're proposing here is an isolation of the action types by splitting not just the router but the hooks API itself. We intend to reuse most of the API constructs, understand, but ultimately each handler/sub-router will be in essence a fork of the hooks API. We'll be reusing reactors and extensions, but that starts to seem strange with things split this way. It would seem better to make two separate APIs, rather than sharing constructs this way when the semantic meaning of those constructs in each fork is slightly different.

[JDGrimes]

Split events API from hooks API?

I think we need to start thinking of the event API as something separate from the router itself. Separate even from the hooks API. See #128. Because we can decide in the future to build a different UI/API if we want to, reusing the same events. The problem with that is that we wouldn't be able to import the reactions from the old API, because we wouldn't be able to automatically fork over the extensions and reactors, either on the API side or the UI side. In other words, it is important to get this right the first time. Maybe what we really need to do is make the reactors/extensions more generic. Or, if they are already really generic enough, we need to begin thinking of them as being more generic.

Which kind of brings us back to the previous comment, where we were suggesting that we were using these to generically. But maybe that's not a bad thing after all.

[JDGrimes]

Keeping action types

I guess the truth is that we still don't know for sure where we'll be taking this API in the future. So it seems to me that it would still be a good idea to make it as flexible as possible. We will probably be able to find some way that we could refactor it to make things simpler in the future if we needed to.

In other words, let's retain the action types and hit types. This is the easiest and quickest thing to do anyway. If we had more time, we'd probably do something different, but right now I think this is best.

We'd do better to pursue #128, and only make any changes which that will require.

[JDGrimes]

Related: https://github.com/WordPoints/hooks-api/issues/128#issuecomment-218289502

So here is another thing that is complicating the API just for the sake of the toggle events. Without them we could probably combine the actions and events together, since there would be a one-to-one relationship between actions and events. In other words, the repeatable events probably wouldn't have to relate different actions to one another at all, we only have the events doing that in the API mainly because of the toggle events.

[JDGrimes]

Since #128 has now been maybelater'd, we'll have to take another look at this. But the basic assumption there was that we'll sort of let things stand as they are until the entity change API is introduced, at which point we'll have a much better feel of how to move forward on this split. One particularly interesting and relevant observation in that regard is https://github.com/WordPoints/hooks-api/issues/128#issuecomment-218576379, which relates to https://github.com/WordPoints/hooks-api/issues/129#issuecomment-217006481 above.

[JDGrimes]

No entity change API

Ultimately, I'm planning to leave things pretty much as they are. We're not going to pursue the introduction of the entity change API yet, as originally proposed, because the idea of triggering it from the hooks API wouldn't really work anyway. That would require either that the entity change API would somehow utilize the reaction API, or that the reactor would somehow pass the reaction settings through the entity change API and to its "reactor" code (which we've yet to create). Neither option really makes a whole lot of sense. And the basic structure of the entity change API isn't something that we should throw together as a rush-job anyway.

[JDGrimes]

Random thoughts

For posterity: this is just a collection of random observations collected during the course of this ticket.

---

What if we let the events compose the actions dynamically? That's an idea that we've toyed with, however, it only really makes sense for the entity change events, not the repeatable events.

---

Maybe the reversal actions should be triggered entirely separately. That is, they shouldn't be use the regular router. But that would lead to unnecessary duplication. Possibly there could be something at a higher level. Which is I guess where the proposed action handlers come in. Maybe we should have a different handler for the reverse actions from the toggle on actions. I'm not sure that there is really any need though.

---

Maybe then the actions could actually be registered as part of the event object itself. That is, we wouldn't have to have a separate event map. But that doesn't make sense, we have to pull up the event map before we can pull up the event, so we can't put the event map info in the event object itself.

---

One specific example of this is how we map action types to hit types (see #109). With just a simple event API, the action types and hit types go away, which means that we can just query for reactions for a particular event in the router when an action occurs. But with the action and hit types we end up needing to query not just for an event but action type as well. If we don't change this, we'll be pulling up excess reactions. If we do, we'll be introducing a feature that is unnecessary for the basic events API.

[JDGrimes]

Generic interface

Above we discussed making the fact that the events API is really broader than just WordPress action, and could be triggered by other means as well in the future, perhaps. So we contemplated the possibility of making the interface more generic. At present, it is possible to trigger the events API sans the actions because the fire_event() method on the router is public. While the router might not seem the ideal place for this, if we are thinking of it as an event router, then I suppose it does make sense for anything that wants to trigger an event to go through that router. Then again, it seems that upon inspection the code in those methods is really separate from the action handling and the main brunt of the router itself. (Although this hasn't always been true, and we may want to consider whether it always will be.) So perhaps it does make sense to split it off, perhaps into the events app. Anyway, that would really be something for another ticket though, I think.

[JDGrimes]

Since we haven't really done anything much here, but might revisit this in the future, I'm going to close this as maybelater.