From 55ea24a010ea80e62cfa37b9aaaab22e70a5d1e2 Mon Sep 17 00:00:00 2001
From: "yevhenii.nadtochii" <yevhenii.nadtochii@teamdev.com>
Date: Fri, 22 Apr 2022 13:59:42 +0300
Subject: [PATCH 1/6] Better docs for `CommandService`

---
 .../java/io/spine/server/CommandService.java  | 35 ++++++++++++++++---
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/server/src/main/java/io/spine/server/CommandService.java b/server/src/main/java/io/spine/server/CommandService.java
index 7a4fe9402b8..bc474dd3c4e 100644
--- a/server/src/main/java/io/spine/server/CommandService.java
+++ b/server/src/main/java/io/spine/server/CommandService.java
@@ -45,8 +45,16 @@
 import static io.spine.server.bus.Acks.reject;
 
 /**
- * The {@code CommandService} allows client applications to post commands and
- * receive updates from the application backend.
+ * The {@code CommandService} allows client applications to post commands to
+ * the application backend.
+ *
+ * <p>This class is an implementation of a corresponding gRPC service.
+ *
+ * <p>Please note, its public API is dictated by the
+ * {@linkplain CommandServiceGrpc.CommandServiceImplBase generated code}. Despite the fact of its
+ * "publicity", it's not meant to be used directly. Use {@link io.spine.client.Client Client}
+ * to post commands to the application. Actual API of the service is declared in its proto
+ * definition. Please take a look on "command_service.proto" file.
  */
 public final class CommandService
         extends CommandServiceGrpc.CommandServiceImplBase
@@ -55,7 +63,7 @@ public final class CommandService
     private final ImmutableMap<CommandClass, BoundedContext> commandToContext;
 
     /**
-     * Constructs new instance using the map from a {@code CommandClass} to
+     * Constructs a new instance using the map from a {@code CommandClass} to
      * a {@code BoundedContext} instance which handles the command.
      */
     private CommandService(Map<CommandClass, BoundedContext> map) {
@@ -70,7 +78,9 @@ public static Builder newBuilder() {
         return new Builder();
     }
 
-    /** Builds the service with a single Bounded Context. **/
+    /**
+     * Builds the service with a single Bounded Context.
+     */
     public static CommandService withSingle(BoundedContext context) {
         CommandService result = newBuilder()
                 .add(context)
@@ -78,6 +88,23 @@ public static CommandService withSingle(BoundedContext context) {
         return result;
     }
 
+    /**
+     * Posts the given command to the application.
+     *
+     * <p>In the original proto definition of this service, this method is blocking unary. Meaning,
+     * its real signature should be {@code Ack post(Command)}. But due to the restrictions,
+     * imposed by gRPC, we have to implement it using {@code StreamObserver}, even when only
+     * a single {@code Ack} is expected.
+     *
+     * <p>As a result, we don't expect any streaming errors since there's no stream at all. We use
+     * a {@code StreamObserver} to return a single value. The corresponding
+     * {@linkplain StreamObserver#onError(Throwable) error handler} is never called by our code.
+     *
+     * <p>The errors, which may occur on a transport layer or within gRPC itself are runtime. They
+     * are not propagated in an observer as well.
+     *
+     * <p>See issue: <a href="https://github.com/grpc/grpc-java/issues/1474">Improve unary server stub</a>
+     */
     @Override
     public void post(Command request, StreamObserver<Ack> responseObserver) {
         CommandClass commandClass = CommandClass.of(request);

From fd46f85b6c298a90f0d7891cecb4a4f9e510f8c7 Mon Sep 17 00:00:00 2001
From: "yevhenii.nadtochii" <yevhenii.nadtochii@teamdev.com>
Date: Fri, 22 Apr 2022 14:14:42 +0300
Subject: [PATCH 2/6] Shorten a linked type

---
 server/src/main/java/io/spine/server/CommandService.java | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/server/src/main/java/io/spine/server/CommandService.java b/server/src/main/java/io/spine/server/CommandService.java
index bc474dd3c4e..90a2c3da3d7 100644
--- a/server/src/main/java/io/spine/server/CommandService.java
+++ b/server/src/main/java/io/spine/server/CommandService.java
@@ -50,11 +50,10 @@
  *
  * <p>This class is an implementation of a corresponding gRPC service.
  *
- * <p>Please note, its public API is dictated by the
- * {@linkplain CommandServiceGrpc.CommandServiceImplBase generated code}. Despite the fact of its
- * "publicity", it's not meant to be used directly. Use {@link io.spine.client.Client Client}
- * to post commands to the application. Actual API of the service is declared in its proto
- * definition. Please take a look on "command_service.proto" file.
+ * <p>Please note, its public API is dictated by the {@linkplain CommandServiceGrpc generated code}.
+ * Despite the fact of its "publicity", it's not meant to be used directly.
+ * Use {@link io.spine.client.Client Client} to post commands to the application. Actual API of
+ * the service is declared in its proto definition. Please take a look on "command_service.proto".
  */
 public final class CommandService
         extends CommandServiceGrpc.CommandServiceImplBase

From 3f33d66e23cfbfa26394bd4fad33359ca7e8b6cd Mon Sep 17 00:00:00 2001
From: "yevhenii.nadtochii" <yevhenii.nadtochii@teamdev.com>
Date: Fri, 22 Apr 2022 14:26:00 +0300
Subject: [PATCH 3/6] Drop duplicated statements from docs

---
 .../java/io/spine/server/CommandService.java  | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/server/src/main/java/io/spine/server/CommandService.java b/server/src/main/java/io/spine/server/CommandService.java
index 90a2c3da3d7..f6a4f34740b 100644
--- a/server/src/main/java/io/spine/server/CommandService.java
+++ b/server/src/main/java/io/spine/server/CommandService.java
@@ -45,15 +45,15 @@
 import static io.spine.server.bus.Acks.reject;
 
 /**
- * The {@code CommandService} allows client applications to post commands to
- * the application backend.
+ * The {@code CommandService} provides a synchronous way to post commands
+ * to the application backend.
  *
  * <p>This class is an implementation of a corresponding gRPC service.
  *
- * <p>Please note, its public API is dictated by the {@linkplain CommandServiceGrpc generated code}.
- * Despite the fact of its "publicity", it's not meant to be used directly.
- * Use {@link io.spine.client.Client Client} to post commands to the application. Actual API of
- * the service is declared in its proto definition. Please take a look on "command_service.proto".
+ * <p>Please note, public API of this class is dictated by the
+ * {@linkplain CommandServiceGrpc generated code}. Despite the fact of its "publicity", it's not
+ * meant to be used directly. Use {@link io.spine.client.Client Client} to post commands to the
+ * application. Actual API of the service is defined in its proto definition.
  */
 public final class CommandService
         extends CommandServiceGrpc.CommandServiceImplBase
@@ -93,11 +93,11 @@ public static CommandService withSingle(BoundedContext context) {
      * <p>In the original proto definition of this service, this method is blocking unary. Meaning,
      * its real signature should be {@code Ack post(Command)}. But due to the restrictions,
      * imposed by gRPC, we have to implement it using {@code StreamObserver}, even when only
-     * a single {@code Ack} is expected.
+     * a single {@code Ack} is returned.
      *
-     * <p>As a result, we don't expect any streaming errors since there's no stream at all. We use
-     * a {@code StreamObserver} to return a single value. The corresponding
-     * {@linkplain StreamObserver#onError(Throwable) error handler} is never called by our code.
+     * <p>As a result, we don't expect any streaming errors since there's no stream at all.
+     * The corresponding {@linkplain StreamObserver#onError(Throwable) error handler} is never
+     * called by our code.
      *
      * <p>The errors, which may occur on a transport layer or within gRPC itself are runtime. They
      * are not propagated in an observer as well.

From 9cedbeb9cdf6046244d598012de866913d4f8390 Mon Sep 17 00:00:00 2001
From: "yevhenii.nadtochii" <yevhenii.nadtochii@teamdev.com>
Date: Tue, 26 Apr 2022 13:19:51 +0300
Subject: [PATCH 4/6] Update docs to `CommandService` definition

---
 .../proto/spine/client/command_service.proto  | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/client/src/main/proto/spine/client/command_service.proto b/client/src/main/proto/spine/client/command_service.proto
index 89f5746dd66..f4189a2f611 100644
--- a/client/src/main/proto/spine/client/command_service.proto
+++ b/client/src/main/proto/spine/client/command_service.proto
@@ -30,8 +30,6 @@ package spine.client;
 import "spine/options.proto";
 
 option (type_url_prefix) = "type.spine.io";
-// We put gRPC-based classes into `grpc` sub-package, which is annotated as `@Internal`
-// to hide implementation details from the public API of the framework.
 option java_package = "io.spine.client.grpc";
 option java_multiple_files = true;
 option java_outer_classname = "CommandServiceProto";
@@ -39,9 +37,24 @@ option java_outer_classname = "CommandServiceProto";
 import "spine/core/command.proto";
 import "spine/core/ack.proto";
 
-// A service for sending commands from clients.
+// A service for posting commands to the application backend.
+//
 service CommandService {
 
-    // Request to handle a command.
+    // Posts the given command to the application backend.
+    //
+    // When the command is successfully received by the application, it responds with
+    // an acknowledgement. The received `Ack` may have one of the following statuses:
+    //
+    //    1. `Ok` meaning the command is accepted for further processing.
+    //    2. `Error` when a technical error occurs on the side of the application.
+    //    3. `Rejection` if no technical error occurred but due to the business rules the command
+    //        should be immediately disqualified from being executed. A typical scenario would be
+    //        when the permissions of a user who made a request aren't broad enough.
+    //
+    // In case of internal gRPC errors or issues on a transport layer, an `Ack` would not be
+    // received. All errors which occur on the side of gRCP should be handled by the means
+    // of the used client stub.
+    //
     rpc Post(core.Command) returns (core.Ack);
 }

From 110c250ebe6d7d059416edbf9f955050cd060cc7 Mon Sep 17 00:00:00 2001
From: "yevhenii.nadtochii" <yevhenii.nadtochii@teamdev.com>
Date: Tue, 26 Apr 2022 15:29:35 +0300
Subject: [PATCH 5/6] Document an implementation of `CommandService`

---
 .../java/io/spine/server/CommandService.java  | 38 +++++++++++--------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/server/src/main/java/io/spine/server/CommandService.java b/server/src/main/java/io/spine/server/CommandService.java
index f6a4f34740b..d3dc1aae191 100644
--- a/server/src/main/java/io/spine/server/CommandService.java
+++ b/server/src/main/java/io/spine/server/CommandService.java
@@ -48,12 +48,10 @@
  * The {@code CommandService} provides a synchronous way to post commands
  * to the application backend.
  *
- * <p>This class is an implementation of a corresponding gRPC service.
- *
- * <p>Please note, public API of this class is dictated by the
- * {@linkplain CommandServiceGrpc generated code}. Despite the fact of its "publicity", it's not
- * meant to be used directly. Use {@link io.spine.client.Client Client} to post commands to the
- * application. Actual API of the service is defined in its proto definition.
+ * <p>This class is an implementation of a corresponding gRPC service. Hence, public API of this
+ * class is dictated by the {@linkplain CommandServiceGrpc generated code}. Despite the fact of its
+ * "publicity", it's not meant to be used directly. Use {@link io.spine.client.Client Client}
+ * to post commands to the application.
  */
 public final class CommandService
         extends CommandServiceGrpc.CommandServiceImplBase
@@ -88,19 +86,27 @@ public static CommandService withSingle(BoundedContext context) {
     }
 
     /**
-     * Posts the given command to the application.
+     * {@inheritDoc}
+     *
+     * <p>In the original proto definition of this service, this method is unary.
+     * Meaning, for every posted command only a single {@code Ack} is received in response:
+     *
+     * <pre>Ack post(Command);</pre>
      *
-     * <p>In the original proto definition of this service, this method is blocking unary. Meaning,
-     * its real signature should be {@code Ack post(Command)}. But due to the restrictions,
-     * imposed by gRPC, we have to implement it using {@code StreamObserver}, even when only
-     * a single {@code Ack} is returned.
+     * <p>But for every service, gRPC generates several client stubs: blocking, asynchronous,
+     * {@link com.google.common.util.concurrent.ListenableFuture ListenableFuture}-based. Each one
+     * has its own signature for this method. And instead of making users to implement three
+     * different signatures of the same method in the
+     * {@linkplain CommandServiceGrpc.CommandServiceImplBase server stub}, they prefer a single
+     * universal method that cover all the cases.
      *
-     * <p>As a result, we don't expect any streaming errors since there's no stream at all.
-     * The corresponding {@linkplain StreamObserver#onError(Throwable) error handler} is never
-     * called by our code.
+     * <p>Although, utilizing of {@link StreamObserver} is quite controversial decision for the
+     * method which does not stream anything. It is still better than implementing this method
+     * three times. For more details on this matter, reference the issue below.
      *
-     * <p>The errors, which may occur on a transport layer or within gRPC itself are runtime. They
-     * are not propagated in an observer as well.
+     * <p>Please note, all the errors, occurring on the side of the application are still
+     * propagated through the {@linkplain Ack acknowledgement}. They are not passed into the
+     * {@linkplain StreamObserver#onError(Throwable) observer}.
      *
      * <p>See issue: <a href="https://github.com/grpc/grpc-java/issues/1474">Improve unary server stub</a>
      */

From 21fcee06b9437d475521df1d25bfd59d87cbfdcf Mon Sep 17 00:00:00 2001
From: "yevhenii.nadtochii" <yevhenii.nadtochii@teamdev.com>
Date: Tue, 26 Apr 2022 15:48:28 +0300
Subject: [PATCH 6/6] Fix a typo

---
 server/src/main/java/io/spine/server/CommandService.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/src/main/java/io/spine/server/CommandService.java b/server/src/main/java/io/spine/server/CommandService.java
index d3dc1aae191..597c41e50a7 100644
--- a/server/src/main/java/io/spine/server/CommandService.java
+++ b/server/src/main/java/io/spine/server/CommandService.java
@@ -89,12 +89,12 @@ public static CommandService withSingle(BoundedContext context) {
      * {@inheritDoc}
      *
      * <p>In the original proto definition of this service, this method is unary.
-     * Meaning, for every posted command only a single {@code Ack} is received in response:
+     * Meaning, for every posted command only a single {@link Ack} is received in response:
      *
      * <pre>Ack post(Command);</pre>
      *
-     * <p>But for every service, gRPC generates several client stubs: blocking, asynchronous,
-     * {@link com.google.common.util.concurrent.ListenableFuture ListenableFuture}-based. Each one
+     * <p>But for every service, gRPC generates several clients: blocking, asynchronous,
+     * {@link com.google.common.util.concurrent.ListenableFuture Future}-based. Each one
      * has its own signature for this method. And instead of making users to implement three
      * different signatures of the same method in the
      * {@linkplain CommandServiceGrpc.CommandServiceImplBase server stub}, they prefer a single