-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Interfaces are not mapped back to concrete classes #8
Comments
Hi @joelvh The current state was mainly a hacky brain dump honestly. I've just recently come back to this and looking at the code I'm a bit horrified by the spaghetti code. I'm currently in the process of completely rewriting the code parsing. Could you provide me with a/some tangible example(s) of such an event class and what you expect to see in the visualization? You can find the progress here: #9 |
Side note: I'm considering putting some more effort into the visualization after this. Thinking of doing something interactive with Vue, instead of a static SVG generated with mermaid.js. What is your use case with the package? |
Hi @JonasPardon Our system is event-oriented to make things as composable as possible. Some events we want to also log, while others are simply for coordination. Furthermore, some events are also meant for serialization for specific 3rd party integrations. Different listeners listen for different interfaces to achieve these goals.
There are other interfaces that may be implemented in the same event classes to allow different listeners to do different things with the same class. In some cases we might also have a class hierarchy, such as a Our system has grown pretty complex with all the events and listeners, and jobs/listeners that emit more events or dispatch more jobs. So, we need a way to visualize how everything is inter-related. I've done this in the past by creating my own "framework" that required defining the events, listeners, entities, etc... in a particular way in each class in order to basically "register" what happens in the classes, to then be able to generate the relationships visually using graphviz. My hope is not to have to explicitly define these things and rather use type hinting and reflection, as well as possibly AST to detect calls to This package is a great foundation, but I haven't dug into how you are determining relationships. I saw that immediate issue and am assuming determining relationships is somewhat arbitrary? We have much more we want to build on this event system, and in some aspects, we may want to abstract things a bit more to build custom workflows (e.g. no code) on top of this. So visualizations right now are to determine how features in the system currently work, and then to use that to identify ways to refactor into configurable workflows. Aside from refactoring, this visualization would help to simply document how these events are used so we can determine the different use cases we haven't otherwise documented already. In case it's useful to you, I found this workflow package that uses Drawflow, which might be something to consider here (as a read-only UI). I'm exploring using that to build workflows that I alluded to earlier as part of our refactoring. I'm happy to help where I can and using something like Vue would be great. Thanks for making this package! |
That's the exact use case I had and what triggered me to create the package. I started with manual discovery of the relationships between classes in the same way, and disliked the clutter. That's why I wanted a plug-and-play solution that just did auto-discovery of all related classes.
Currently this is being done with some bad AST traversing and that's the part I'm currently working on to improve - pretty unstable right now. The idea is to start from the registered app events and 'worm through' all those classes to find related ones. I want to support the following at least, since that is what I use in my own codebases:
This is currently also on my roadmap for work, but not for this package. For me, the package is supposed to be something that helps with documenting and understanding the flow of the application.
I have come across this before but haven't actually checked it out yet, it looks nice, thanks for the tip!
Will keep that in mind, thanks! I don't think it's workable yet, but once the code parsing is mainly done there might be a solid foundation for some tinkering. |
@JonasPardon ok, great. Sounds like we have similar contexts then. And for sure, we want to have a visualization like this to understand the existing system. Building the custom workflows on top is a whole other beast. I'll watch this repo for now -- let me know when I can help with feedback or input. |
Hi @JonasPardon, Sorry it's been a while, but I'm finally coming back to this. I've been testing out the latest and definitely getting better results!
I'm happy to help implement some of this. Do you have any guidance you can provide on how to navigate the codebase to understand where to implement new features? |
@JonasPardon I've made a PR to support In our case, we call a static method that returns the jobs by mapping the results of a query to a collection of jobs. Then we also add more jobs to the batch recursively in the I presume the work from #19 should work with Example use case I'm solving for -- it's a little contrived, so the goal is to illustrate the code structure. <?php
namespace App\Jobs;
class BatchJob implements ShouldQueue
{
public static function createJobs()
{
return Orders::query()->take(100)->get()->map(fn ($order) => new OrderJob($order));
}
public function handle()
{
Bus::batch(static::createJobs())
->finally(fn ($batch) => $batch->add(static::createJobs()));
}
} |
Hi @joelvh! Thanks for taking the time and checking back in with some contributions! I'm currently a bit swamped but I'll make sure to take some time and check everything you've submitted in the following days. |
We are implementing several interfaces and base classes in our event classes, so that we can have subscribers and listeners target events more granularly that might have multiple purposes. However, the visualization links the interface to the listener, and doesn't translate that to which concrete event classes are actually handled by the listener.
Taking a look at the code, it appears that there isn't any consideration for what types are mapped to listeners. It seems this is where we could determine if the event is an interface and then resolve all classes that implement that interface.
Alternatively, I'm not familiar enough with the internal workings of this package, but maybe a way to determine what concrete event classes are relevant, the introspection into which event classes are broadcast could cross-reference with the interfaces mapped to listeners on-the-fly.
I'm happy to help if you have ideas where this can be best implemented -- there might already be a way to accomplish this that I missed.
Thanks for the great work here!
The text was updated successfully, but these errors were encountered: