-
Notifications
You must be signed in to change notification settings - Fork 29
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
#448 Implement dispatching of actions on next model-update #141
Conversation
and update `WorkflowLayoutEngine` to dispatch a `CenterAction` after the next model update. Fixes eclipse-glsp/glsp#448
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me!
However, I'm not sure the layout engine is in general the best example usage of dispatchAfterNextUpdate
. At least to me this is quite surprising.
Other than that, this looks great!
@Override | ||
public void layout() { | ||
if (modelState.getRoot() instanceof GGraph) { | ||
GLSPLayoutConfigurator configurator = new GLSPLayoutConfigurator(); | ||
configurator.configureByType("graph"); | ||
this.layout((GGraph) modelState.getRoot(), configurator); | ||
actionDispatcher.dispatchAfterNextUpdate(new CenterAction()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't expect that the layout engine queues an action after layouting -- so maybe this isn't the best example use case for dispatchAfterNextUpdate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think it's not an uncommon usecase to want to trigger a Center action after (manually) autolayouting the model. However, I happily change this to a better example. Any suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An Idea could be: Select a node after the creation operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case to center after layout makes sense, but I find it a bit odd that the layout engine invokes this action. I'd rather expect the center action to be invoked from the layout handler. But I'm fine keeping it this way.
I like the idea by @ivy-cst though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also like the idea by @ivy-cst and updated the PR accordingly.
There is a CI error, other than that, it looks good! Thanks! |
Verson 4.14 no longer exists in p2 repository. Update to new 4.44 Version
…cture (eclipse-glsp#127) * [WIP] eclipse-glsp#150 eclipse-glsp#141 Rework dependency injection architecture eclipse-glsp#150 Rework DI architecture No longer use one global injector per application connection. Instead use a global server injector and dedicated child injectors for each client session. This removes a couple of flaws/downsides we had with the current approach. To achieve this the `GLSPModule` has been split into two separate modules. The `ServerModule` provides the bindings for base infrastructure & networking (used for creating the server injector) while `DiagramModules` provide language & session specific bindings (used for creating the session specific child injectors). The main benefits are the following: - Each session now uses its own `ActionDispatcher` this means action messages intended for different client sessions can now be handled in parallel. Previously they were processed one after the other essentially creating a bottleneck. - One `GLSPServer` can now handler multiple different diagram implementations. The diagram language specific bindings are provided with a `DiagramModule`. When creating a new client session injector the corresponding `DiagramModule` is identified via the given diagram type. This can for instance be used in UML-GLSP to provide all different diagram types (class,package,state machine etc.) with one server. - Apart from the initial "process" method it is no longer necessary to keep track of the diagramid/client session id. Each session has its own instances of action handlers, operation handlers, model state, diagram configuration etc. and the clientId no longer needs to be passed as method argument. In addition, provider classes with the purpose of mapping a class instance to the correct digramId like `ModelStateProvider` or the `DiagramConfigurationRegistry` are now obsolete and have been removed. - It is now possible to create stateful action/operation handlers. Previously handlers had to be stateless because they were reused for all client sessions. Now each session has its own set own set of handlers . - Use the new approach of injecting optionals (provided by Guice 5) instead of binding NullImpls. Note that this PR only provides the DI architecture rework. Existing API whose functionality does not break after the rework as not been touched yet even tough there a various aspects of the existing API that can now be simplified/improved. For instance, with the new approach the Action/Operation Handler API can be simplified. It is no longer necessary to pass the model state argument as part of the interface methods (see also eclipse-glsp#120). I have identified some major aspects of the API that can be improved (corresponding issues will be created after this PR is merged). eclipse-glsp#141 Refactor `ClientSessionListener` API - Extract listener methods for server connection from `ClientSessionListener` interface into own `ServerConnectionListener` interface. Server connection listeners are handled directly by the `GLSPServer`. - It is now possible to define a set of clientIds for which a listener should be registered when adding a new `ClientSessionListener` to the 'ClientSessionManager`. - The `DefaultGLSPClientSessionManager` now autodisposes all `ClientSessionListeners` for the corresponding clientSessionId when a session is disposed. - Improved null-checks: The client session manager now checks for disposed listeners (i.e. null values in the map) and removes them before notifying (i.e invoking a listener method) them. Miscellaneous - Reduce public API exposure. Interface implementations that typically are not expected to be customized by implementing projects haven been moved to the `internal` API. - Improve documentation. Add javadoc for all newly added interfaces and relevant public classes. Also improve documentation of some existing components. - Merge `GLSPServerStatusAction` and `ServerStatusAction` - Introduce `ModuleUtil` class with provides the functionality to mixin modules similar to how it is done in Xtext. - Remove uncessary `.gitkeep` file. - Rename `DefaultGLSPServerLauncher` to `SocketGLSPServerLauncher` - Rename `ClientOptions` to `ClientOptionsUtil` to conform to the standard naming scheme for util classes - Follow Guice best practices and document all public bindings of a module Fixes eclipse-glsp/glsp/issues/150 Fixes eclipse-glsp/glsp/issues/141 * Remove clientId argument where obsolete * Address review feedback Co-authored-by: Philip Langer <[email protected]>
…cture (eclipse-glsp#127) * [WIP] eclipse-glsp#150 eclipse-glsp#141 Rework dependency injection architecture eclipse-glsp#150 Rework DI architecture No longer use one global injector per application connection. Instead use a global server injector and dedicated child injectors for each client session. This removes a couple of flaws/downsides we had with the current approach. To achieve this the `GLSPModule` has been split into two separate modules. The `ServerModule` provides the bindings for base infrastructure & networking (used for creating the server injector) while `DiagramModules` provide language & session specific bindings (used for creating the session specific child injectors). The main benefits are the following: - Each session now uses its own `ActionDispatcher` this means action messages intended for different client sessions can now be handled in parallel. Previously they were processed one after the other essentially creating a bottleneck. - One `GLSPServer` can now handler multiple different diagram implementations. The diagram language specific bindings are provided with a `DiagramModule`. When creating a new client session injector the corresponding `DiagramModule` is identified via the given diagram type. This can for instance be used in UML-GLSP to provide all different diagram types (class,package,state machine etc.) with one server. - Apart from the initial "process" method it is no longer necessary to keep track of the diagramid/client session id. Each session has its own instances of action handlers, operation handlers, model state, diagram configuration etc. and the clientId no longer needs to be passed as method argument. In addition, provider classes with the purpose of mapping a class instance to the correct digramId like `ModelStateProvider` or the `DiagramConfigurationRegistry` are now obsolete and have been removed. - It is now possible to create stateful action/operation handlers. Previously handlers had to be stateless because they were reused for all client sessions. Now each session has its own set own set of handlers . - Use the new approach of injecting optionals (provided by Guice 5) instead of binding NullImpls. Note that this PR only provides the DI architecture rework. Existing API whose functionality does not break after the rework as not been touched yet even tough there a various aspects of the existing API that can now be simplified/improved. For instance, with the new approach the Action/Operation Handler API can be simplified. It is no longer necessary to pass the model state argument as part of the interface methods (see also eclipse-glsp#120). I have identified some major aspects of the API that can be improved (corresponding issues will be created after this PR is merged). eclipse-glsp#141 Refactor `ClientSessionListener` API - Extract listener methods for server connection from `ClientSessionListener` interface into own `ServerConnectionListener` interface. Server connection listeners are handled directly by the `GLSPServer`. - It is now possible to define a set of clientIds for which a listener should be registered when adding a new `ClientSessionListener` to the 'ClientSessionManager`. - The `DefaultGLSPClientSessionManager` now autodisposes all `ClientSessionListeners` for the corresponding clientSessionId when a session is disposed. - Improved null-checks: The client session manager now checks for disposed listeners (i.e. null values in the map) and removes them before notifying (i.e invoking a listener method) them. Miscellaneous - Reduce public API exposure. Interface implementations that typically are not expected to be customized by implementing projects haven been moved to the `internal` API. - Improve documentation. Add javadoc for all newly added interfaces and relevant public classes. Also improve documentation of some existing components. - Merge `GLSPServerStatusAction` and `ServerStatusAction` - Introduce `ModuleUtil` class with provides the functionality to mixin modules similar to how it is done in Xtext. - Remove uncessary `.gitkeep` file. - Rename `DefaultGLSPServerLauncher` to `SocketGLSPServerLauncher` - Rename `ClientOptions` to `ClientOptionsUtil` to conform to the standard naming scheme for util classes - Follow Guice best practices and document all public bindings of a module Fixes eclipse-glsp/glsp/issues/150 Fixes eclipse-glsp/glsp/issues/141 * Remove clientId argument where obsolete * Address review feedback Co-authored-by: Philip Langer <[email protected]>
…lsp#141) * #448 Implement dispatching of actions on next model-update and update `WorkflowLayoutEngine` to dispatch a `CenterAction` after the next model update. Fixes eclipse-glsp/glsp#448 * Address review feedback * Update jetty-bundles version Verson 4.14 no longer exists in p2 repository. Update to new 4.44 Version
and update
WorkflowLayoutEngine
to dispatch aCenterAction
after the next model update.Fixes eclipse-glsp/glsp#448