Address review feedback

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actions/ActionDispatcher.java

@@ -39,7 +39,7 @@ default CompletableFuture<Void> dispatch(final ActionMessage message) {
     * Processes the given action, received from the specified clientId, by dispatching it to all registered handlers.
     * </p>
     *
-    * @param clientId The client from which the action was received
+    * @param action The action that should be dispatched.
     * @return
     *         A {@link CompletableFuture} indicating when the action processing is complete
     */

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/DiagramModule.java

@@ -36,8 +36,8 @@
 import org.eclipse.glsp.server.actions.SetViewportAction;
 import org.eclipse.glsp.server.actions.TriggerEdgeCreationAction;
 import org.eclipse.glsp.server.actions.TriggerNodeCreationAction;
-import org.eclipse.glsp.server.di.scope.DiagramGlobal;
 import org.eclipse.glsp.server.di.scope.DiagramGlobalScope;
+import org.eclipse.glsp.server.di.scope.DiagramGlobalSingleton;
 import org.eclipse.glsp.server.diagram.DiagramConfiguration;
 import org.eclipse.glsp.server.diagram.RequestTypeHintsActionHandler;
 import org.eclipse.glsp.server.diagram.ServerConfigurationContribution;
@@ -117,7 +117,7 @@
  * <ul>
  * <li>{@link String} annotated with @{@link DiagramType}
  * <li>{@link String} annotated with @{@link ClientId}
- * <li>{@link DiagramGlobal} bound as Scope
+ * <li>{@link DiagramGlobalSingleton} bound as Scope
  * <li>{@link DiagramConfiguration}
  * <li>{@link ServerConfigurationContribution}
  * <li>{@link GModelState}
@@ -163,7 +163,7 @@ protected void configureBase() {
       bindClientId();
       bindDiagramGobalScope();
       // Configurations
-      bind(DiagramConfiguration.class).to(bindDiagramConfiguration()).in(DiagramGlobal.class);
+      bind(DiagramConfiguration.class).to(bindDiagramConfiguration()).in(DiagramGlobalSingleton.class);
       bind(ServerConfigurationContribution.class).to(bindServerConfigurationContribution()).in(Singleton.class);
       // Model-related bindings
       bind(GModelState.class).to(bindGModelState()).in(Singleton.class);
@@ -205,7 +205,6 @@ protected void configureBase() {
       bindOptionally(PopupModelFactory.class, bindPopupModelFactory());
       bindOptionally(LayoutEngine.class, bindLayoutEngine());
       bindOptionally(GraphExtension.class, bindGraphExtension());
-
    }
 
    protected void bindDiagramType() {
@@ -217,7 +216,7 @@ protected void bindClientId() {
    }
 
    protected void bindDiagramGobalScope() {
-      bindScope(DiagramGlobal.class, new DiagramGlobalScope.NullImpl());
+      bindScope(DiagramGlobalSingleton.class, new DiagramGlobalScope.NullImpl());
    }
 
    protected abstract Class<? extends DiagramConfiguration> bindDiagramConfiguration();

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/ServerModule.java

@@ -44,12 +44,12 @@
  * The server module is the central configuration artifact for configuring the server injector (i.e. main injector). For
  * each application connecting to the server process a new server injector is created. The server module provides the
  * base bindings necessary for setting up the base {@link GLSPServer} infrastructure. In addition, it is used to
- * configure the set of {@link DiagramModule}s. Diagram modules are used to create the diagram session specific child
+ * configure the set of {@link DiagramModule}s. Diagram modules are used to create the diagram-session-specific child
  * injector when the
  * {@link GLSPServer#initializeClientSession(org.eclipse.glsp.server.protocol.InitializeClientSessionParameters)}
  * method is called.
  *
- * All bindings inside of the {@link ServerModule#configure()} method are encapsulated into submethods which makes it
+ * All bindings inside of the {@link ServerModule#configure()} method are delegated to separate methods which makes it
  * easy to override a specific binding in subclasses.
  *
  * p>The following bindings are provided:
@@ -71,7 +71,7 @@ public class ServerModule extends GLSPModule {
 
    /**
     * Configure a new {@link DiagramModule} for this server. A diagram module represents the base configuration artifact
-    * for configuring a diagram-language specific client session injector. The diagram type provided
+    * for configuring a diagram-language-specific client session injector. The diagram type provided
     * {@link DiagramModule#getDiagramType()} is used to retrieve the correct diagram module when the {@link GLSPServer}
     * initialises a new client session.
     * The given diagram module and all (optional) additional modules will be combined to one merged module. If bindings

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/scope/DiagramGlobalScope.java

@@ -24,7 +24,7 @@
 /**
  * A {@link Scope} implementation that is used in GLSP for sharing instances across multiple diagram injectors of the
  * same diagram type. A diagram injector is an injector with an installed {@link DiagramModule}. The
- * {@link DiagramGlobal} annotation is used to annotated bindings that should be made available to all injectors with
+ * {@link DiagramGlobalSingleton} annotation is used to annotated bindings that should be made available to all injectors with
  * the same diagram type as Singleton.
  *
  * Use this scope with care. If its used on classes that inject session specific values (e.g. the clientId) it can cause

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/scope/DiagramGlobalScopeModule.java

@@ -38,6 +38,6 @@ public DiagramGlobalScopeModule(final DiagramGlobalScope scope) {
 
    @Override
    protected void configure() {
-      bindScope(DiagramGlobal.class, scope);
+      bindScope(DiagramGlobalSingleton.class, scope);
    }
 }

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/scope/DiagramGlobal.java → plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/scope/DiagramGlobalSingleton.java

@@ -26,12 +26,12 @@
 
 /**
  *
- * Custom scope annotation that is used to declare bindings as diagram global. Diagram global means that one instance of
+ * Scope annotation to declare bindings as diagram global singleton. Diagram global singleton means that one instance of
  * this class is shared across all related injectors of the same diagram type. This is used to declare diagram type
  * specific singletons across all diagram session injectors (i.e. child injectors) of one GLSP server (injector).
  *
  */
 @Target({ TYPE, METHOD })
 @Retention(RUNTIME)
 @ScopeAnnotation
-public @interface DiagramGlobal {}
+public @interface DiagramGlobalSingleton {}

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/diagram/BaseDiagramConfiguration.java

@@ -26,7 +26,7 @@
 import com.google.inject.Inject;
 
 /**
- * A base reusable implementation of {@link DiagramConfiguration} that used DI to provide
+ * A reusable base implementation of {@link DiagramConfiguration} that used DI to provide
  * the diagram type and the graph extension. In addition, default type mappings are configured.
  */
 public abstract class BaseDiagramConfiguration implements DiagramConfiguration {

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/diagram/DiagramConfiguration.java

@@ -28,7 +28,6 @@
 /**
  * Provides configuration constants for a specific diagram implementation (i.e. diagram language),
  * The corresponding configuration for a diagram implementation is identified via its diagram type.
- *
  */
 public interface DiagramConfiguration {
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/disposable/DisposableCollection.java

@@ -19,9 +19,7 @@
 import java.util.List;
 
 /**
- * Helper class to collect multiple {@link IDisposable}s and dispose them
- * all at once.
- *
+ * A collection of {@link IDisposable}s that can be disposed.
  */
 public class DisposableCollection extends Disposable {
    private final List<IDisposable> disposables = new ArrayList<>();

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/features/validation/RequestMarkersHandler.java

@@ -41,7 +41,7 @@ public class RequestMarkersHandler extends BasicActionHandler<RequestMarkersActi
    public List<Action> executeAction(final RequestMarkersAction action, final GModelState modelState) {
       List<String> elementsIDs = action.getElementsIDs();
       if (validator.isEmpty()) {
-         LOG.warn("Cannot compute markers no implementation for: " + ModelValidator.class + " has been bound");
+         LOG.warn("Cannot compute markers! No implementation for " + ModelValidator.class + " has been bound");
          return none();
       }
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/gson/GraphGsonConfigurationFactory.java

@@ -22,10 +22,9 @@
 import com.google.gson.TypeAdapter;
 
 /**
- * This factory is used in client session specific injectors to create
+ * A factory used in the client-session-specific injectors to create
  * {@link GsonBuilder}s that can properly (de)serialize all graph elements (i.e. {@link GModelElement}s
  * that are used in the diagram model.
- *
  */
 public interface GraphGsonConfigurationFactory {
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/gson/ServerGsonConfigurator.java

@@ -23,8 +23,8 @@
 import com.google.gson.TypeAdapter;
 
 /**
- * Is used to configure the {@link Gson} object used by lsp4J for (de)serialization of json-rpc messages. The server
- * gson configurator configures a given {@link GsonBuilder} to be able do properly handle {@link Action}
+ * Configurator of the {@link Gson} object used by LSP4J for (de)serialization of json-rpc messages. The server
+ * gson configurator configures a given {@link GsonBuilder} to be able to properly handle {@link Action}
  * objects and {@link GModelElement}s.
  */
 public interface ServerGsonConfigurator {

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/internal/actions/DefaultActionDispatcher.java

@@ -59,6 +59,7 @@ public class DefaultActionDispatcher extends Disposable implements ActionDispatc
    @Inject
    protected ActionHandlerRegistry actionHandlerRegistry;
 
+   @Inject
    @ClientId
    protected String clientId;
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/internal/di/scope/DefaultDiagramGlobalScope.java

@@ -86,7 +86,6 @@ protected Optional<Injector> getInjector(final Provider<?> unscoped) {
          getInjector.setAccessible(true);
          Injector injector = (Injector) getInjector.invoke(unscoped);
          return Optional.of(injector);
-
       } catch (ReflectiveOperationException e) {
          return Optional.empty();
       }

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/internal/session/DefaultClientSession.java

@@ -55,8 +55,6 @@ protected void doDispose() {
    }
 
    @Override
-   public Injector getInjector() { // TODO Auto-generated method stub
-      return null;
-   }
+   public Injector getInjector() { return injector; }
 
 }

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/internal/session/DefaultClientSessionManager.java

@@ -33,6 +33,7 @@
 
 public final class DefaultClientSessionManager extends Disposable implements ClientSessionManager {
    private static final String ALL_CLIENT_IDS_KEY = "*";
+
    @Inject()
    protected ClientSessionFactory sessionFactory;
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/launch/DefaultGLSPServerLauncher.java → plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/launch/SocketGLSPServerLauncher.java

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2019-2020 EclipseSource and others.
+ * Copyright (c) 2019-2021 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -32,26 +32,27 @@
 import java.util.function.Function;
 
 import org.apache.log4j.Logger;
-import org.eclipse.glsp.server.di.GLSPModule;
-import org.eclipse.glsp.server.jsonrpc.GsonConfigurator;
+import org.eclipse.glsp.server.di.ServerModule;
+import org.eclipse.glsp.server.gson.ServerGsonConfigurator;
 import org.eclipse.glsp.server.protocol.GLSPClient;
 import org.eclipse.glsp.server.protocol.GLSPServer;
 import org.eclipse.lsp4j.jsonrpc.Launcher;
 import org.eclipse.lsp4j.jsonrpc.MessageConsumer;
 
 import com.google.gson.GsonBuilder;
 import com.google.inject.Injector;
+import com.google.inject.Module;
 
-public class DefaultGLSPServerLauncher extends GLSPServerLauncher {
+public class SocketGLSPServerLauncher extends GLSPServerLauncher {
    public static final String START_UP_COMPLETE_MSG = "[GLSP-Server]:Startup completed";
-   private static Logger log = Logger.getLogger(DefaultGLSPServerLauncher.class);
+   private static Logger log = Logger.getLogger(SocketGLSPServerLauncher.class);
 
    private ExecutorService threadPool;
    private AsynchronousServerSocketChannel serverSocket;
    private CompletableFuture<Void> onShutdown;
 
-   public DefaultGLSPServerLauncher(final GLSPModule glspModule) {
-      super(glspModule);
+   public SocketGLSPServerLauncher(final ServerModule serverModule, final Module... additionalModules) {
+      super(serverModule, additionalModules);
    }
 
    @Override
@@ -77,7 +78,7 @@ public Future<Void> asyncRun(final String hostname, final int port)
          @Override
          public void completed(final AsynchronousSocketChannel result, final Void attachment) {
             serverSocket.accept(null, this); // Prepare for the next connection
-            DefaultGLSPServerLauncher.this.createClientConnection(result);
+            SocketGLSPServerLauncher.this.createClientConnection(result);
          }
 
          @Override
@@ -121,7 +122,7 @@ protected void createClientConnection(final AsynchronousSocketChannel socketChan
    }
 
    protected Consumer<GsonBuilder> configureGson(final Injector injector) {
-      GsonConfigurator gsonConf = injector.getInstance(GsonConfigurator.class);
+      ServerGsonConfigurator gsonConf = injector.getInstance(ServerGsonConfigurator.class);
       return (final GsonBuilder builder) -> gsonConf.configureGsonBuilder(builder);
    }
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/model/DefaultGModelState.java

@@ -29,7 +29,7 @@
 
 public class DefaultGModelState implements GModelState {
 
-   @Inject()
+   @Inject
    @ClientId
    protected String clientId;
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/protocol/ServerConnectionListener.java

@@ -18,7 +18,6 @@
 /**
  * A listener to track the connection status of {@link GLSPClient}s (i.e. client applications).
  * Gets notified when a new GLSP client connects or disconnects.
- *
  */
 public interface ServerConnectionListener {
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/registry/MultiRegistry.java

@@ -21,7 +21,7 @@
 import org.eclipse.glsp.server.internal.registry.MapMultiRegistry;
 
 /**
- * Similar to {@link Registry} a multi registry is used to manage a set of key-value pairs. The main difference is that
+ * Multi registry used to manage a set of key-value pairs. The main difference to {@link Registry} is that
  * a multi registry doesn't enforce a 1-1 relation between key and value(s).
  * One key can be associated with multiple values. The default implementation uses a {@link Map} to manage the key value
  * pairs.

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/session/ClientSession.java

@@ -45,7 +45,6 @@ public interface ClientSession extends IDisposable {
    String getDiagramType();
 
    /*
-    * +
     * Return the action dispatcher of this diagram type. The action dispatcher is typically created by the session
     * specific injector and is basically the entrypoint to the session specific injection context.
     * @return
@@ -54,8 +53,7 @@ public interface ClientSession extends IDisposable {
 
    /**
     * Retrieve the session specific {@link Injector}. Use this method with care. Normally it should not be necessary to
-    * construct
-    * additional instances using the session specific injector and this can cause unintended side effects.
+    * construct additional instances using the session specific injector and this can cause unintended side effects.
     *
     * @return The session specific injector.
     */

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/session/ClientSessionManager.java

@@ -22,7 +22,6 @@
 
 /**
  * The central component that manages the lifecycle of client sessions.
- *
  */
 public interface ClientSessionManager extends IDisposable {
 

plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/utils/ModuleUtil.java

@@ -23,19 +23,17 @@ public final class ModuleUtil {
    private ModuleUtil() {}
 
    /**
-    * Mixin additional modules into a base modules. Bindings that are already present in the base module will be
+    * Mix in additional modules into a base modules. Bindings that are already present in the base module will be
     * overwritten with the binding of the additional module.
     *
     * @param baseModule        the base module.
     * @param additionalModules the additional modules
     * @return A merged (mixedin) module
     */
    public static Module mixin(Module baseModule, final Module... additionalModules) {
-
       for (Module additionalModule : additionalModules) {
          baseModule = Modules.override(baseModule).with(additionalModule);
       }
       return baseModule;
    }
-
 }