opensourcepos
opensourcepos copied to clipboard
opensourcepos
Event generator
This is the CRUD event generator that @jekkos suggested be the driver for third-party integrations. Per #2608 this has been implemented using the CI Events library and is pretty lean in its functionality so as not to bog the code down. The goal is to provide a layer of separation between the base code and third party integrations.
The concept is that "Triggers" are placed throughout the code (in the controllers) which trigger Create, Read, Update and Delete Methods. Simultaneously "Listeners" receive the appropriate data and pass it off to the third-party integrations.
An example of this is that there is a trigger in the Items controller so that when a user successfully creates an item, the trigger puts the event out there with the item_data. The event_create listener defined in the events/Integrations library then picks up the event and sends it to integrations_create defined in the same library which in turn will call any third party functions needing to use the data. The idea is that that integrations_create function would do no heavy lifting but serve to call the third-party integrations functions only. So, for example integrations_create($data) will look at $data[type] and when it's of type CUSTOMERS it might call, say mailchimp_add_email($data[data]) which would be defined separately.
There are a couple of question marks:
- Currently I have not added any "Read" triggers, because it was unclear to me how or if third-party integrations would use a read operation. We can either remove the code or keep it as a place-holder for future use. Alternately if someone can think of a good use case, I'll be glad to insert the triggers in the appropriate controllers.
- ~~Currently I have third-party integrations specific functions being created in the events/Integrations at the bottom of the class, but I'm thinking that will quickly get messy. I'm thinking that we should extrapolate each integration into it's own file. I was thinking that this could be done with a helper or library for each integration. What is the best way to do this? A helper would make the scope of each function global to the whole code, and I don't think we want that. A library would require it be nested into its own class. I'm not sure if that creates it's own scope problems. Thoughts?~~ I went ahead and made changes so that each third party integration needs to be defined in its own library to create better code separation.
- This seems to be consistent with the rest of the code, but the errors on third-party integration fails are written to the log with a hard-coded English error message. Should these be switched to toast notifications and not written in the log? Another question is whether the error messages should be translatable regardless of whether they are toast notifications or written to the error log. It seems that error log notifications are always in English and toast notifications are always translatable.
I thought about this a bit further and think that we should try to make this code as modular as possible. When you add a 3rd party integration, you will need to sync with all data you already have in the ospos db. That's why we should use a publisher / subscriber pattern here. Currently I see following uses
- A publisher that reads existing records from the database and dispatches this data as an 'event' through the defined CRUD hooks. This one will allow you to sync existing data to an integration 'connector'. You'll actually replay all past events through this publisher one-by-one.
- Next we have the same events being fired from our controllers, meaning that once something is changed by a user in ospos, we can immediately dispatch those changes to the 3rd party integration.
- Also how should we hande an event dispatch failure? I guess that we will have to stop the sync at that point as our data could become inconsistent. All messages should be dispatched in order but the process should keep track of what was processed and ideally be able to resume from where it left of.
Same idea for the mailchimp integration: it's failry limited at this time and will only add customers to mailchimp that are created in ospos, if you have the integration setup at that time. It seems there is no way of syncing data that is already there, which is a pity. So once we have the groundwork done here this should be reworked to use the plugin structure.
@jekkos yep, it's push only and not bidirectional syncing. The syncing requires comparisons and policies about what to do in conflicting scenarios.
Agreed. The event generator does not include an integration listener. So that you all know, this is high on my priority list, but I came down with a fever on Monday and am waiting for COVID-19 test results. I'm almost back to 100% and since I work from home, I will likely be back at it tomorrow. The bad news is that the crap has hit the fan and I am trying to get my staff geared to work from home.
I thought about this a bit further and think that we should try to make this code as modular as possible. When you add a 3rd party integration, you will need to sync with all data you already have in the ospos db. That's why we should use a publisher / subscriber pattern here. I apologize, I agree that the 3rd party integrations should be removed from the core code, which is why I wrote the code using event generators to trigger events. The disadvantage of this method rather than a completely modular design is that this design requires the completion of 3rd-party integrations before the main application completes. I can see long-running integrations, slowing the application down significantly. A design that triggers an integration to run a playback of events would get around that, but it is out of my league to design and I would need some help with the code design.
Currently I see following uses
* A publisher that reads existing records from the database and dispatches this data as an 'event' through the defined CRUD hooks. This one will allow you to sync existing data to an integration 'connector'. You'll actually replay all past events through this publisher one-by-one.
I like the idea, but will need help with the design.
* Next we have the same events being fired from our controllers, meaning that once something is changed by a user in ospos, we can immediately dispatch those changes to the 3rd party integration. * Also how should we hande an event dispatch failure? I guess that we will have to stop the sync at that point as our data could become inconsistent. All messages should be dispatched in order but the process should keep track of what was processed and ideally be able to resume from where it left of.
The current method I am taking to handle integration failures is to report but not prevent the core application from processing the data. The downside to this is that it potentially means stale data in the integration side. We could potentially run the trigger as a transaction and fail the operation if the integration fails. This would involve reworking the trigger location itself since I believe most or all are placed in the various controllers only after the application completes its work successfully, but not during its processing of the data.
Short form is that the biggest help I need is that we would reach a decision on exactly how the event generator should work, exactly how the design should be implemented (e.g., what goes in libraries, controllers, etc).
RFC: @jekkos @daN4cat
I was checking whether it would not be better to integrate an existing library to take care of this example: https://eventsauce.io/
I was checking whether it would not be better to integrate an existing library to take care of this example: https://eventsauce.io/
The code in this PR is based off the existing library https://github.com/nathanmac/CodeIgniter-Events. I'll take a look at Event Sauce.
I think the eventsauce looks like it has some more functionality like support for testing, exception handling, event persistence, mesage generation from definition.. it feels like if we use something like this we don't need to reinvent the wheel as the current lib seems rather limited in that regard (just dispatching events but no clear way on how to do these things). Of course the specific integration with CI will remain (in this case wiring the 3rd party controllers as consumers) and handling event registration.
I think the eventsauce looks like it has some more functionality like support for testing, exception handling, event persistence, mesage generation from definition.. it feels like if we use something like this we don't need to reinvent the wheel as the current lib seems rather limited in that regard (just dispatching events but no clear way on how to do these things). Of course the specific integration with CI will remain (in this case wiring the 3rd party controllers as consumers) and handling event registration.
Agreed that EventSauce seems to be more robust. A few questions come to my mind:
- What mechanism do 3rd party plugins need to use to run? The current implementation triggers the event with the data passed to the appropriate integrations. I skimmed the article that you sent and it appears that each 3rd party integration will need a parser to read the event log, but admittedly, I'm fuzzy on this, because it's new to me.
- Will the event logs give enough information for 3rd party integrations to do their work? If the event log simply tells the integrations that an item was sold, without item_ids, sale amounts, etc, that could prove problematic.
- Are you available to help get this running? I'm fine with getting my hands dirty, but I suspect this will go a lot slower if I need to implement all of it because of the learning curve.
- The cost overhead in terms of memory and time for the current implementation is fairly low, because the data is being passed to the 3rd party integrations. Does EventSauce add significant "go get it yourself" overhead against the database for the 3rd party integrations?
Ok I think the next step would be to map out what we have in the PR here against what this library offers, as you suggest.
Basically this means redefining the events in this eventsauce DSL or yaml file and then running a code generator to see how it comes out. Once that is done we need to have some callbacks that push these events to the 3rd party API, and map them out to the correct fields on that side.
But I think that having event persistence could be an important one here, if it comes out of the box then that is quite a leap forward. As once an exception occurs, you know exactly up to which point you are synchronized, and can choose to resolve the error before you start a replay from that snapshot. I'll see if I can free up some time maybe later next week to take a closer look. I can't make any hard promises there though, but I'll try my best.
Ok I think the next step would be to map out what we have in the PR here against what this library offers, as you suggest.
Basically this means redefining the events in this eventsauce DSL or yaml file and then running a code generator to see how it comes out. Once that is done we need to have some callbacks that push these events to the 3rd party API, and map them out to the correct fields on that side.
sounds reasonable.
But I think that having event persistence could be an important one here, if it comes out of the box then that is quite a leap forward. As once an exception occurs, you know exactly up to which point you are synchronized, and can choose to resolve the error before you start a replay from that snapshot.
Agreed, but it's unclear to me if each integration will have it's own log to draw on or if not, how one integration is supposed to keep track of what it has processed and what it hasn't. e.g., integration A processes 10 of 20 events in the logfile does integration B still have those first 10 to process. Also, does each integration need to have a listener? Currently the integration is kicked off by the event generator, but event sourcing sounds more like eventsauce just writes the persistent log and the integrations are on their own.
I'll see if I can free up some time maybe later next week to take a closer look. I can't make any hard promises there though, but I'll try my best.
Thanks. My CLCdesq API 3rd party integration PR is running off this event manager, so merging it is dependent on this being in place. I believe I understand the rough idea of Event sourcing, but my hesitation here is 25% not wanting to spend a bunch of time on another proof of concept if we don't know we are going to use it and 75% the learning curve. Hence asking for your assistance with getting the big pieces in place and I can probably take it from there.
If our bookstore didn't have the quarantine absolutely destroying our sales causing me to rush getting the API code written (CLCdesq API PR), I would have a go at it with just the understanding that this might just be learning a new tool, but I've got a crazy backlog of "urgent".
To clarify, clearly changing this component means I would adapt the CLCdesq API 3rd party integration.
I see what you mean, you might want to have something minimal working first in that case so you can start using it already. I understand the urgency and it's probably more important then to have a nicely designed framework for doing integrations.
The integration you talk about is not something in this PR here I suppose (unless I missed that)? In that case I'd think it'd be just the mapping of these 'events' to the API that you are integrating?
I see what you mean, you might want to have something minimal working first in that case so you can start using it already.
This PR as it stands now works. I'm using it in #2754.
I understand the urgency and it's probably more important then to have a nicely designed framework for doing integrations.
I 100% agree that a nicely designed framework for integrations is absolutely important. It sounds like EventSauce fits that description much better than the PR I have here. We can take one of two routes.
- We can merge this PR as it is, and then rip it out and replace it when we get EventSauce in place. The 3rd party integration code itself ideally wouldn't be affected anyway.
- We can not merge this PR and I can do a rebase dance to the master, this PR and #2754 then use #2754 in production until we can get EventSauce running.
Thoughts?
The integration you talk about is not something in this PR here I suppose (unless I missed that)? In that case I'd think it'd be just the mapping of these 'events' to the API that you are integrating?
#2754 is the 3rd party integration that I referred to. I have just rebased that branch onto this one to make it work.
@jekkos can we sort out whether EventSauce is what we want to use for this instead of CI Events?