From ad69b266051a5a74637ae0f3a262f4b9ab613543 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Fri, 8 Nov 2024 08:09:05 -0700 Subject: [PATCH 01/11] revert me: for testing only Signed-off-by: Matt Peterson --- .../pbj/grpc/helidon/PbjProtocolHandler.java | 134 +++++++----------- 1 file changed, 48 insertions(+), 86 deletions(-) diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index 28f8b8a4..11c7351c 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -348,92 +348,54 @@ public void data(@NonNull final Http2FrameHeader header, @NonNull final BufferDa // There is some asynchronous behavior here, but in the worst case, we handle a few more // bytes before the stream is closed. while (data.available() > 0) { - switch (currentReadState) { - case START: - { - // Read whether this message is compressed. We do not currently support - // compression. - final var isCompressed = (data.read() == 1); - if (isCompressed) { - // The error will eventually result in the stream being closed - throw new GrpcException( - GrpcStatus.UNIMPLEMENTED, "Compression is not supported"); - } - currentReadState = ReadState.READ_LENGTH; - numOfPartReadBytes = 0; - break; - } - case READ_LENGTH: - { - // if I have not read a full int yet then read more from available bytes - if (numOfPartReadBytes < Integer.BYTES) { - // we do not have enough bytes yet to read a 4 byte int - // read the bytes we do have and store them for next time - final int bytesToRead = - Math.min( - data.available(), - Integer.BYTES - numOfPartReadBytes); - data.read(partReadLengthBytes, numOfPartReadBytes, bytesToRead); - numOfPartReadBytes += bytesToRead; - } - // check if we have read all the 4 bytes of the length int32 - if (numOfPartReadBytes == Integer.BYTES) { - final long length = - ((long) partReadLengthBytes[0] & 0xFF) << 24 - | ((long) partReadLengthBytes[1] & 0xFF) << 16 - | ((long) partReadLengthBytes[2] & 0xFF) << 8 - | ((long) partReadLengthBytes[3] & 0xFF); - if (length > config.maxMessageSizeBytes()) { - throw new GrpcException( - GrpcStatus.INVALID_ARGUMENT, - "Message size exceeds maximum allowed size"); - } - // Create a buffer to hold the message. We sadly cannot reuse this - // buffer - // because once we have filled it and wrapped it in Bytes and sent - // it to the - // handler, some user code may grab and hold that Bytes object for - // an arbitrary - // amount of time, and if we were to scribble into the same byte - // array, we - // would break the application. So we need a new buffer each time - // :-( - entityBytes = new byte[(int) length]; - entityBytesIndex = 0; - // done with length now, so move on to next state - currentReadState = ReadState.READ_ENTITY_BYTES; - } - break; - } - case READ_ENTITY_BYTES: - { - // By the time we get here, entityBytes is no longer null. It may be - // empty, or it - // may already have been partially populated from a previous iteration. - // It may be - // that the number of bytes available to be read is larger than just - // this one - // message. So we need to be careful to read, from what is available, - // only up to - // the message length, and to leave the rest for the next iteration. - final int available = data.available(); - final int numBytesToRead = - Math.min(entityBytes.length - entityBytesIndex, available); - data.read(entityBytes, entityBytesIndex, numBytesToRead); - entityBytesIndex += numBytesToRead; - - // If we have completed reading the message, then we can proceed. - if (entityBytesIndex == entityBytes.length) { - currentReadState = ReadState.START; - // Grab and wrap the bytes and reset to being reading the next - // message - final var bytes = Bytes.wrap(entityBytes); - pipeline.onNext(bytes); - entityBytesIndex = 0; - entityBytes = null; - } - break; - } + // First chunk of data contains the compression flag and the length of the message + if (entityBytes == null) { + // Read whether this message is compressed. We do not currently support + // compression. + final var isCompressed = (data.read() == 1); + if (isCompressed) { + // The error will eventually result in the stream being closed + throw new GrpcException( + GrpcStatus.UNIMPLEMENTED, "Compression is not supported"); + } + // Read the length of the message. As per the grpc protocol specification, each + // message on the wire is prefixed with the number of bytes for the message. + // However, to prevent a DOS attack where the attacker sends us a very large + // length and exhausts our memory, we have a maximum message size configuration + // setting. Using that, we can detect attempts to exhaust our memory. + final long length = data.readUnsignedInt32(); + if (length > config.maxMessageSizeBytes()) { + throw new GrpcException( + GrpcStatus.INVALID_ARGUMENT, + "Message size exceeds maximum allowed size"); + } + // Create a buffer to hold the message. We sadly cannot reuse this buffer + // because once we have filled it and wrapped it in Bytes and sent it to the + // handler, some user code may grab and hold that Bytes object for an arbitrary + // amount of time, and if we were to scribble into the same byte array, we + // would break the application. So we need a new buffer each time :-( + entityBytes = new byte[(int) length]; + entityBytesIndex = 0; + } + + // By the time we get here, entityBytes is no longer null. It may be empty, or it + // may already have been partially populated from a previous iteration. It may be + // that the number of bytes available to be read is larger than just this one + // message. So we need to be careful to read, from what is available, only up to + // the message length, and to leave the rest for the next iteration. + final int available = data.available(); + final int numBytesToRead = + Math.min(entityBytes.length - entityBytesIndex, available); + data.read(entityBytes, entityBytesIndex, numBytesToRead); + entityBytesIndex += numBytesToRead; + + // If we have completed reading the message, then we can proceed. + if (entityBytesIndex == entityBytes.length) { + // Grab and wrap the bytes and reset to being reading the next message + final var bytes = Bytes.wrap(entityBytes); + pipeline.onNext(bytes); + entityBytesIndex = 0; + entityBytes = null; } } From 3cf41a22672b7ee86c45384d5b6662a4c391ee8e Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Fri, 8 Nov 2024 08:07:30 -0700 Subject: [PATCH 02/11] fix: first draft of oncancel changes Signed-off-by: Matt Peterson --- .../pbj/grpc/helidon/PbjProtocolHandler.java | 16 +- .../pbj/runtime/grpc/PbjEventHandler.java | 28 ++ .../com/hedera/pbj/runtime/grpc/Pipeline.java | 22 +- .../hedera/pbj/runtime/grpc/Pipelines.java | 267 +++++++++++------- 4 files changed, 220 insertions(+), 113 deletions(-) create mode 100644 pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index 11c7351c..32b506ac 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -30,6 +30,7 @@ import com.hedera.pbj.grpc.helidon.config.PbjConfig; import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; +import com.hedera.pbj.runtime.grpc.PbjEventHandler; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.ServiceInterface; import com.hedera.pbj.runtime.io.buffer.Bytes; @@ -640,7 +641,10 @@ protected void send( * The implementation of {@link Flow.Subscriber} used to send messages to the client. It * receives bytes from the handlers to send to the client. */ - private final class SendToClientSubscriber implements Flow.Subscriber { + private final class SendToClientSubscriber implements Flow.Subscriber, PbjEventHandler { + + private Runnable onErrorHandler; + @Override public void onSubscribe(@NonNull final Flow.Subscription subscription) { // FUTURE: Add support for flow control @@ -666,11 +670,16 @@ public void onNext(@NonNull final Bytes response) { new Http2FrameData(header, bufferData), flowControl.outbound()); } catch (final Exception e) { LOGGER.log(ERROR, "Failed to respond to grpc request: " + route.method(), e); + pipeline.onError(e); } } @Override public void onError(@NonNull final Throwable throwable) { + if (onErrorHandler != null) { + onErrorHandler.run(); + } + if (throwable instanceof final GrpcException grpcException) { new TrailerBuilder() .grpcStatus(grpcException.status()) @@ -697,6 +706,11 @@ public void onComplete() { return Http2StreamState.CLOSED; }); } + + @Override + public void registerOnErrorHandler(@NonNull final Runnable handler) { + this.onErrorHandler = requireNonNull(handler); + } } /** Simple implementation of the {@link ServiceInterface.RequestOptions} interface. */ diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java new file mode 100644 index 00000000..3c7cd678 --- /dev/null +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.pbj.runtime.grpc; + +/** Interface for handling events in the PBJ runtime. */ +public interface PbjEventHandler { + + /** + * Register a handler for when an error occurs. + * + * @param runnable the handler to run + */ + void registerOnErrorHandler(Runnable runnable); +} diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java index 3360cf9f..3fe5a016 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java @@ -1,3 +1,19 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.runtime.grpc; import java.util.concurrent.Flow; @@ -8,8 +24,6 @@ * @param The subscribed item type */ public interface Pipeline extends Flow.Subscriber { - /** - * Called when an END_STREAM frame is received from the client. - */ - void clientEndStreamReceived(); + /** Called when an END_STREAM frame is received from the client. */ + default void clientEndStreamReceived() {} } diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java index a25f2bae..0f4c5a07 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java @@ -23,8 +23,9 @@ import java.util.concurrent.Flow; /** - * Utility class for generating a "pipeline" of processing steps for gRPC services. This is not intended to be used - * directly by application code, but rather by the PBJ compiler when generating service interfaces. + * Utility class for generating a "pipeline" of processing steps for gRPC services. This is not + * intended to be used directly by application code, but rather by the PBJ compiler when generating + * service interfaces. */ public final class Pipelines { @@ -33,8 +34,8 @@ private Pipelines() { } /** - * Returns a {@link Flow.Subscriber} that does nothing. This can be used in cases where a subscriber is required - * but no proper implementation is available. + * Returns a {@link Flow.Subscriber} that does nothing. This can be used in cases where a + * subscriber is required but no proper implementation is available. * * @return A No-op subscriber. */ @@ -46,6 +47,7 @@ public void clientEndStreamReceived() { } private Flow.Subscription subscription; + @Override public void onSubscribe(@NonNull final Flow.Subscription subscription) { this.subscription = requireNonNull(subscription); @@ -71,7 +73,8 @@ public void onComplete() { } /** - * Create a new pipeline for a unary gRPC service method. A unary method is a simple request/response method. + * Create a new pipeline for a unary gRPC service method. A unary method is a simple + * request/response method. * * @return A new builder for constructing the pipeline. * @param The type of the request message. @@ -82,8 +85,9 @@ public static UnaryBuilder unary() { } /** - * Create a new pipeline for a bidirectional streaming gRPC service method. A bidirectional streaming method - * allows for a stream of requests and a stream of responses to operate concurrently. + * Create a new pipeline for a bidirectional streaming gRPC service method. A bidirectional + * streaming method allows for a stream of requests and a stream of responses to operate + * concurrently. * * @return A new builder for constructing the pipeline. * @param The type of the request message. @@ -94,8 +98,9 @@ public static BidiStreamingBuilder bidiStreaming() { } /** - * Create a new pipeline for a client streaming gRPC service method. A client streaming method allows for a - * stream of requests to be sent to the server, with a single response returned at the very end. + * Create a new pipeline for a client streaming gRPC service method. A client streaming method + * allows for a stream of requests to be sent to the server, with a single response returned at + * the very end. * * @return A new builder for constructing the pipeline. * @param The type of the request message. @@ -106,8 +111,8 @@ public static ClientStreamingBuilder clientStreaming() { } /** - * Create a new pipeline for a server streaming gRPC service method. A server streaming method allows for a - * single request to be sent to the server, with a stream of responses returned. + * Create a new pipeline for a server streaming gRPC service method. A server streaming method + * allows for a single request to be sent to the server, with a stream of responses returned. * * @return A new builder for constructing the pipeline. * @param The type of the request message. @@ -119,12 +124,14 @@ public static ServerStreamingBuilder serverStreaming() { /** * A builder for constructing the pipeline for a unary gRPC service method. + * * @param The type of the request message. * @param The type of the response message. */ public interface UnaryBuilder { /** - * Configures a lambda for mapping from {@link Bytes} to the request message type. This must be specified. + * Configures a lambda for mapping from {@link Bytes} to the request message type. This must + * be specified. * * @param mapper The mapping function. * @return This builder. @@ -133,8 +140,8 @@ public interface UnaryBuilder { UnaryBuilder mapRequest(@NonNull ExceptionalFunction mapper); /** - * Configures the unary method to be called when a request is received. This method handles the request and - * returns a response. This must be specified. + * Configures the unary method to be called when a request is received. This method handles + * the request and returns a response. This must be specified. * * @param method The method to call. * @return This builder. @@ -143,7 +150,8 @@ public interface UnaryBuilder { UnaryBuilder method(@NonNull ExceptionalFunction method); /** - * Configures a lambda for mapping from the response message type to {@link Bytes}. This must be specified. + * Configures a lambda for mapping from the response message type to {@link Bytes}. This + * must be specified. * * @param mapper The mapping function. * @return This builder. @@ -152,8 +160,9 @@ public interface UnaryBuilder { UnaryBuilder mapResponse(@NonNull ExceptionalFunction mapper); /** - * Configures a subscriber to receive the response messages. This must be specified. This subscriber is - * provided by the web server and is responsible for sending the responses back to the client. + * Configures a subscriber to receive the response messages. This must be specified. This + * subscriber is provided by the web server and is responsible for sending the responses + * back to the client. * * @param replies The subscriber to receive the responses. * @return This builder. @@ -162,8 +171,8 @@ public interface UnaryBuilder { UnaryBuilder respondTo(@NonNull Flow.Subscriber replies); /** - * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, and contains - * the replies that are sent back to the client. + * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, + * and contains the replies that are sent back to the client. * * @return the communication pipeline */ @@ -179,8 +188,9 @@ public interface UnaryBuilder { */ public interface BidiStreamingBuilder { /** - * Configures a lambda for mapping from {@link Bytes} to the request message type. This must be specified. - * This function will be called once for each message arriving from the client. + * Configures a lambda for mapping from {@link Bytes} to the request message type. This must + * be specified. This function will be called once for each message arriving from the + * client. * * @param mapper The mapping function. * @return This builder. @@ -189,9 +199,9 @@ public interface BidiStreamingBuilder { BidiStreamingBuilder mapRequest(@NonNull ExceptionalFunction mapper); /** - * Configures the bidirectional streaming method to be called when a request is received. This method is given - * a subscriber that it can push responses to, and it returns a subscriber that the system can push requests to. - * This must be specified. + * Configures the bidirectional streaming method to be called when a request is received. + * This method is given a subscriber that it can push responses to, and it returns a + * subscriber that the system can push requests to. This must be specified. * * @param method The method to call. * @return This builder. @@ -200,8 +210,9 @@ public interface BidiStreamingBuilder { BidiStreamingBuilder method(@NonNull BidiStreamingMethod method); /** - * Configures a lambda for mapping from the response message type to {@link Bytes}. This must be specified. - * This function will be called once for each message that the method sends back to the client. + * Configures a lambda for mapping from the response message type to {@link Bytes}. This + * must be specified. This function will be called once for each message that the method + * sends back to the client. * * @param mapper The mapping function. * @return This builder. @@ -210,8 +221,9 @@ public interface BidiStreamingBuilder { BidiStreamingBuilder mapResponse(@NonNull ExceptionalFunction mapper); /** - * Configures a subscriber to receive the response messages. This must be specified. This subscriber is - * provided by the web server and is responsible for sending the responses back to the client. + * Configures a subscriber to receive the response messages. This must be specified. This + * subscriber is provided by the web server and is responsible for sending the responses + * back to the client. * * @param replies The subscriber to receive the responses. * @return This builder. @@ -220,8 +232,8 @@ public interface BidiStreamingBuilder { BidiStreamingBuilder respondTo(@NonNull Flow.Subscriber replies); /** - * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, and contains - * the replies that are sent back to the client. + * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, + * and contains the replies that are sent back to the client. * * @return the communication pipeline */ @@ -237,8 +249,9 @@ public interface BidiStreamingBuilder { */ public interface ClientStreamingBuilder { /** - * Configures a lambda for mapping from {@link Bytes} to the request message type. This must be specified. - * This function will be called once for each message arriving from the client. + * Configures a lambda for mapping from {@link Bytes} to the request message type. This must + * be specified. This function will be called once for each message arriving from the + * client. * * @param mapper The mapping function. * @return This builder. @@ -247,21 +260,21 @@ public interface ClientStreamingBuilder { ClientStreamingBuilder mapRequest(@NonNull ExceptionalFunction mapper); /** - * Configures the client streaming method to be called when a request is received. This method is given - * a subscriber that it can push responses to, and it returns a subscriber that the system can push requests to. - * Only a single message is returned through the subscriber. - * This must be specified. + * Configures the client streaming method to be called when a request is received. This + * method is given a subscriber that it can push responses to, and it returns a subscriber + * that the system can push requests to. Only a single message is returned through the + * subscriber. This must be specified. * * @param method The method to call. * @return This builder. */ @NonNull - ClientStreamingBuilder method( - @NonNull ClientStreamingMethod method); + ClientStreamingBuilder method(@NonNull ClientStreamingMethod method); /** - * Configures a lambda for mapping from the response message type to {@link Bytes}. This must be specified. - * This function will be called once for each message that the method sends back to the client. + * Configures a lambda for mapping from the response message type to {@link Bytes}. This + * must be specified. This function will be called once for each message that the method + * sends back to the client. * * @param mapper The mapping function. * @return This builder. @@ -270,8 +283,9 @@ ClientStreamingBuilder method( ClientStreamingBuilder mapResponse(@NonNull ExceptionalFunction mapper); /** - * Configures a subscriber to receive the response messages. This must be specified. This subscriber is - * provided by the web server and is responsible for sending the responses back to the client. + * Configures a subscriber to receive the response messages. This must be specified. This + * subscriber is provided by the web server and is responsible for sending the responses + * back to the client. * * @param replies The subscriber to receive the responses. * @return This builder. @@ -280,8 +294,8 @@ ClientStreamingBuilder method( ClientStreamingBuilder respondTo(@NonNull Flow.Subscriber replies); /** - * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, and contains - * the replies that are sent back to the client. + * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, + * and contains the replies that are sent back to the client. * * @return the communication pipeline */ @@ -297,7 +311,8 @@ ClientStreamingBuilder method( */ public interface ServerStreamingBuilder { /** - * Configures a lambda for mapping from {@link Bytes} to the request message type. This must be specified. + * Configures a lambda for mapping from {@link Bytes} to the request message type. This must + * be specified. * * @param mapper The mapping function. * @return This builder. @@ -306,8 +321,8 @@ public interface ServerStreamingBuilder { ServerStreamingBuilder mapRequest(@NonNull ExceptionalFunction mapper); /** - * Configures the server streaming method to be called when a request is received. This method is given - * a subscriber that it can push responses to. This must be specified. + * Configures the server streaming method to be called when a request is received. This + * method is given a subscriber that it can push responses to. This must be specified. * * @param method The method to call. * @return This builder. @@ -316,8 +331,9 @@ public interface ServerStreamingBuilder { ServerStreamingBuilder method(@NonNull ServerStreamingMethod method); /** - * Configures a lambda for mapping from the response message type to {@link Bytes}. This must be specified. - * This function will be called once for each message that the method sends back to the client. + * Configures a lambda for mapping from the response message type to {@link Bytes}. This + * must be specified. This function will be called once for each message that the method + * sends back to the client. * * @param mapper The mapping function. * @return This builder. @@ -326,8 +342,9 @@ public interface ServerStreamingBuilder { ServerStreamingBuilder mapResponse(@NonNull ExceptionalFunction mapper); /** - * Configures a subscriber to receive the response messages. This must be specified. This subscriber is - * provided by the web server and is responsible for sending the responses back to the client. + * Configures a subscriber to receive the response messages. This must be specified. This + * subscriber is provided by the web server and is responsible for sending the responses + * back to the client. * * @param replies The subscriber to receive the responses. * @return This builder. @@ -336,8 +353,8 @@ public interface ServerStreamingBuilder { ServerStreamingBuilder respondTo(@NonNull Flow.Subscriber replies); /** - * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, and contains - * the replies that are sent back to the client. + * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, + * and contains the replies that are sent back to the client. * * @return the communication pipeline */ @@ -365,8 +382,8 @@ public interface ExceptionalFunction { } /** - * A function that handles a client streaming gRPC service method. Many messages are received from the client, - * but only a single response is sent back to the client when completed. + * A function that handles a client streaming gRPC service method. Many messages are received + * from the client, but only a single response is sent back to the client when completed. * * @param The type of the request message. * @param The type of the response message. @@ -376,8 +393,8 @@ public interface ClientStreamingMethod extends ExceptionalFunction, Flow.Subscriber> {} /** - * A function that handles a server streaming gRPC service method. A single request is received from the client, - * and many responses are sent back to the client. + * A function that handles a server streaming gRPC service method. A single request is received + * from the client, and many responses are sent back to the client. * * @param The type of the request message. * @param The type of the response message. @@ -390,12 +407,13 @@ public interface ServerStreamingMethod { * @param replies The subscriber to send responses to. * @throws Exception If an error occurs during processing. */ - void apply(@NonNull T request, @NonNull Flow.Subscriber replies) throws Exception; + void apply(@NonNull T request, @NonNull Flow.Subscriber replies) + throws Exception; } /** - * A function that handles a bidirectional streaming gRPC service method. Many messages are received from the - * client, and many responses are sent back to the client. + * A function that handles a bidirectional streaming gRPC service method. Many messages are + * received from the client, and many responses are sent back to the client. * * @param The type of the request message. * @param The type of the response message. @@ -404,19 +422,20 @@ public interface BidiStreamingMethod extends ExceptionalFunction, Flow.Subscriber> {} /** - * A convenient base class for the different builders. All builders have to hold state for request and - * response mapping functions, as well as the subscriber to send responses to, so we have a base class. - * This class also implements the {@link Flow.Subscriber} and {@link Flow.Subscription} interfaces, to - * reduce the overall number of instances created. + * A convenient base class for the different builders. All builders have to hold state for + * request and response mapping functions, as well as the subscriber to send responses to, so we + * have a base class. This class also implements the {@link Flow.Subscriber} and {@link + * Flow.Subscription} interfaces, to reduce the overall number of instances created. * - *

A {@link Flow.Subscription} is provided to each subscriber at the time they subscribe. Technically - * this can be a many-to-one relationship, but in our case, there is only going to be one subscriber for - * this {@link Flow.Subscription}, so we can simplify things a bit. + *

A {@link Flow.Subscription} is provided to each subscriber at the time they subscribe. + * Technically this can be a many-to-one relationship, but in our case, there is only going to + * be one subscriber for this {@link Flow.Subscription}, so we can simplify things a bit. * * @param The type of the request message. * @param The type of the response message. */ - private abstract static class PipelineBuilderImpl implements Pipeline, Flow.Subscription { + private abstract static class PipelineBuilderImpl + implements Pipeline, Flow.Subscription { protected ExceptionalFunction requestMapper; protected ExceptionalFunction responseMapper; protected Flow.Subscriber replies; @@ -425,7 +444,8 @@ private abstract static class PipelineBuilderImpl implements Pipeline The type of the request message. * @param The type of the response message. */ - private static final class UnaryBuilderImpl extends PipelineBuilderImpl implements UnaryBuilder { + private static final class UnaryBuilderImpl extends PipelineBuilderImpl + implements UnaryBuilder { private ExceptionalFunction method; @Override @@ -522,9 +543,12 @@ public Pipeline build() { @Override public void onNext(@NonNull final Bytes message) { - // A unary method call is pretty simple. We take the incoming bytes, convert them into the request - // message type, call the method, and then convert the response message back into bytes. If there - // are any exceptions, we forward that along. Otherwise, we just do the work and complete. + // A unary method call is pretty simple. We take the incoming bytes, convert them into + // the request + // message type, call the method, and then convert the response message back into bytes. + // If there + // are any exceptions, we forward that along. Otherwise, we just do the work and + // complete. if (completed) { replies.onError(new IllegalStateException("Unary method already called.")); @@ -541,11 +565,6 @@ public void onNext(@NonNull final Bytes message) { replies.onError(e); } } - - @Override - public void clientEndStreamReceived() { - // nothing to do, as onComplete is always called inside onNext - } } /** @@ -561,28 +580,32 @@ private static final class BidiStreamingBuilderImpl extends PipelineBuilde @Override @NonNull - public BidiStreamingBuilderImpl mapRequest(@NonNull final ExceptionalFunction mapper) { + public BidiStreamingBuilderImpl mapRequest( + @NonNull final ExceptionalFunction mapper) { this.requestMapper = mapper; return this; } @Override @NonNull - public BidiStreamingBuilderImpl method(@NonNull final BidiStreamingMethod method) { + public BidiStreamingBuilderImpl method( + @NonNull final BidiStreamingMethod method) { this.method = method; return this; } @Override @NonNull - public BidiStreamingBuilderImpl mapResponse(@NonNull final ExceptionalFunction mapper) { + public BidiStreamingBuilderImpl mapResponse( + @NonNull final ExceptionalFunction mapper) { this.responseMapper = mapper; return this; } @Override @NonNull - public BidiStreamingBuilderImpl respondTo(@NonNull final Flow.Subscriber replies) { + public BidiStreamingBuilderImpl respondTo( + @NonNull final Flow.Subscriber replies) { this.replies = replies; return this; } @@ -597,10 +620,14 @@ public Pipeline build() { replies.onSubscribe(this); - // This subscriber maps from the response type to bytes and sends them back to the client. Whenever - // the "onNext" method produces a new response, it will pass through this subscriber before being - // forwarded to the "replies" subscriber, where the webserver will return it to the client. - final var responseConverter = new MapSubscriber(replies, item -> responseMapper.apply(item)); + // This subscriber maps from the response type to bytes and sends them back to the + // client. Whenever + // the "onNext" method produces a new response, it will pass through this subscriber + // before being + // forwarded to the "replies" subscriber, where the webserver will return it to the + // client. + final var responseConverter = + new MapSubscriber(replies, item -> responseMapper.apply(item)); try { incoming = method.apply(responseConverter); @@ -631,6 +658,15 @@ public void onComplete() { super.onComplete(); } + @Override + public void onError(@NonNull final Throwable t) { + if (incoming != null) { + incoming.onError(t); + } + + super.onError(t); + } + @Override public void clientEndStreamReceived() { // if the client stream is ended, the entire pipeline is ended @@ -640,6 +676,7 @@ public void clientEndStreamReceived() { /** * The implementation of the {@link ClientStreamingBuilder} interface. + * * @param The type of the request message. * @param The type of the response message. */ @@ -650,28 +687,32 @@ private static final class ClientStreamingBuilderImpl extends PipelineBuil @Override @NonNull - public ClientStreamingBuilderImpl mapRequest(@NonNull final ExceptionalFunction mapper) { + public ClientStreamingBuilderImpl mapRequest( + @NonNull final ExceptionalFunction mapper) { this.requestMapper = mapper; return this; } @Override @NonNull - public ClientStreamingBuilderImpl method(@NonNull final ClientStreamingMethod method) { + public ClientStreamingBuilderImpl method( + @NonNull final ClientStreamingMethod method) { this.method = method; return this; } @Override @NonNull - public ClientStreamingBuilderImpl mapResponse(@NonNull final ExceptionalFunction mapper) { + public ClientStreamingBuilderImpl mapResponse( + @NonNull final ExceptionalFunction mapper) { this.responseMapper = mapper; return this; } @Override @NonNull - public ClientStreamingBuilderImpl respondTo(@NonNull final Flow.Subscriber replies) { + public ClientStreamingBuilderImpl respondTo( + @NonNull final Flow.Subscriber replies) { this.replies = replies; return this; } @@ -684,7 +725,8 @@ public Pipeline build() { throw new IllegalStateException("The method must be specified."); } replies.onSubscribe(this); - final var responseConverter = new MapSubscriber(replies, item -> responseMapper.apply(item)); + final var responseConverter = + new MapSubscriber(replies, item -> responseMapper.apply(item)); try { incoming = method.apply(responseConverter); @@ -697,7 +739,8 @@ public Pipeline build() { @Override public void onNext(@NonNull final Bytes message) { if (completed) { - replies.onError(new IllegalStateException("ClientStreaming method already called.")); + replies.onError( + new IllegalStateException("ClientStreaming method already called.")); return; } @@ -734,28 +777,32 @@ private static final class ServerStreamingBuilderImpl extends PipelineBuil @Override @NonNull - public ServerStreamingBuilderImpl mapRequest(@NonNull final ExceptionalFunction mapper) { + public ServerStreamingBuilderImpl mapRequest( + @NonNull final ExceptionalFunction mapper) { this.requestMapper = mapper; return this; } @Override @NonNull - public ServerStreamingBuilderImpl method(@NonNull final ServerStreamingMethod method) { + public ServerStreamingBuilderImpl method( + @NonNull final ServerStreamingMethod method) { this.method = method; return this; } @Override @NonNull - public ServerStreamingBuilderImpl mapResponse(@NonNull final ExceptionalFunction mapper) { + public ServerStreamingBuilderImpl mapResponse( + @NonNull final ExceptionalFunction mapper) { this.responseMapper = mapper; return this; } @Override @NonNull - public ServerStreamingBuilderImpl respondTo(@NonNull final Flow.Subscriber replies) { + public ServerStreamingBuilderImpl respondTo( + @NonNull final Flow.Subscriber replies) { this.replies = replies; return this; } @@ -770,14 +817,16 @@ public Pipeline build() { responseConverter = new MapSubscriber<>(replies, item -> responseMapper.apply(item)); responseConverter.onSubscribe( - this); // Theoretically this should be done. But now I'm subscribing to this AND replies! + this); // Theoretically this should be done. But now I'm subscribing to this AND + // replies! return this; } @Override public void onNext(@NonNull final Bytes message) { if (completed) { - replies.onError(new IllegalStateException("ServerStreaming method already called.")); + replies.onError( + new IllegalStateException("ServerStreaming method already called.")); return; } @@ -788,25 +837,20 @@ public void onNext(@NonNull final Bytes message) { replies.onError(e); } } - - @Override - public void clientEndStreamReceived() { - // nothing to do - // the server will continue streaming, since the message coming from the client is a subscription request - } } /** - * A subscriber that maps from one type to another. It is like a Java "map" operation on a stream, but as a - * subscriber. + * A subscriber that maps from one type to another. It is like a Java "map" operation on a + * stream, but as a subscriber. * * @param next The subscriber to send the mapped values to. * @param mapper The function to map from one type to another. * @param The type of the input. * @param The type of the output. */ - private record MapSubscriber(Flow.Subscriber next, ExceptionalFunction mapper) - implements Flow.Subscriber, Flow.Subscription { + private record MapSubscriber( + Flow.Subscriber next, ExceptionalFunction mapper) + implements Flow.Subscriber, Flow.Subscription, PbjEventHandler { private MapSubscriber { next.onSubscribe(this); @@ -846,5 +890,12 @@ public void onError(Throwable throwable) { public void onComplete() { next.onComplete(); } + + @Override + public void registerOnErrorHandler(Runnable handler) { + if (next instanceof PbjEventHandler pbjEventHandler) { + pbjEventHandler.registerOnErrorHandler(handler); + } + } } } From 00a3952ee47d4bc1062513c945cafb44c236eaa8 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Mon, 11 Nov 2024 10:50:45 -0700 Subject: [PATCH 03/11] comment Signed-off-by: Matt Peterson --- .../java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index 32b506ac..d29c576a 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -677,6 +677,9 @@ public void onNext(@NonNull final Bytes response) { @Override public void onError(@NonNull final Throwable throwable) { if (onErrorHandler != null) { + // Invoke the handlers registered by + // the application code integrated + // with the PBJ Helidon Plugin. onErrorHandler.run(); } From 9ce0e5c1bf145c83c22e0cc75ed07ac088928ae1 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Mon, 11 Nov 2024 14:13:24 -0700 Subject: [PATCH 04/11] wip: first stab at a unit test for the handler Signed-off-by: Matt Peterson --- pbj-core/pbj-grpc-helidon/build.gradle.kts | 2 + .../pbj/grpc/helidon/PbjProtocolHandler.java | 18 +- .../grpc/helidon/PbjProtocolHandlerTest.java | 265 ++++++++++++++++++ 3 files changed, 281 insertions(+), 4 deletions(-) create mode 100644 pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java diff --git a/pbj-core/pbj-grpc-helidon/build.gradle.kts b/pbj-core/pbj-grpc-helidon/build.gradle.kts index c736f90c..a6ee5cac 100644 --- a/pbj-core/pbj-grpc-helidon/build.gradle.kts +++ b/pbj-core/pbj-grpc-helidon/build.gradle.kts @@ -38,6 +38,8 @@ testModuleInfo { requires("io.grpc.protobuf") requires("io.grpc.netty") requires("io.grpc.stub") + requires("org.mockito") + requires("org.mockito.junit.jupiter") requiresStatic("com.github.spotbugs.annotations") requiresStatic("java.annotation") } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index d29c576a..c763fd6f 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -73,7 +73,7 @@ * between Helidon and the generated PBJ service handler endpoints. An instance of this class is * created for each new connection, and each connection is made to a specific method endpoint. */ -final class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHandler { +class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHandler { private static final System.Logger LOGGER = System.getLogger(PbjProtocolHandler.class.getName()); @@ -170,7 +170,7 @@ enum ReadState { * *

Method calls on this object are thread-safe. */ - private Pipeline pipeline; + protected Pipeline pipeline; /** Create a new instance */ PbjProtocolHandler( @@ -293,7 +293,7 @@ public void init() { // Setup the subscribers. The "outgoing" subscriber will send messages to the client. // This is given to the "open" method on the service to allow it to send messages to // the client. - final Flow.Subscriber outgoing = new SendToClientSubscriber(); + final Flow.Subscriber outgoing = getOutgoing(); pipeline = route.service().open(route.method(), options, outgoing); } catch (final GrpcException grpcException) { route.failedGrpcRequestCounter().increment(); @@ -317,6 +317,11 @@ public void init() { } } + // Visible for testing + protected Flow.Subscriber getOutgoing() { + return new SendToClientSubscriber(); + } + @Override @NonNull public Http2StreamState streamState() { @@ -343,6 +348,11 @@ public void data(@NonNull final Http2FrameHeader header, @NonNull final BufferDa Objects.requireNonNull(header); Objects.requireNonNull(data); + processData(header, data); + } + + protected void processData(@NonNull final Http2FrameHeader header, @NonNull final BufferData data) { + try { // NOTE: if the deadline is exceeded, then the stream will be closed and data will no // longer flow. @@ -717,7 +727,7 @@ public void registerOnErrorHandler(@NonNull final Runnable handler) { } /** Simple implementation of the {@link ServiceInterface.RequestOptions} interface. */ - private record Options( + protected record Options( Optional authority, boolean isProtobuf, boolean isJson, String contentType) implements ServiceInterface.RequestOptions {} diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java new file mode 100644 index 00000000..e12a38a0 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -0,0 +1,265 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.pbj.grpc.helidon; + +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; +import com.hedera.pbj.grpc.helidon.config.PbjConfig; +import com.hedera.pbj.runtime.grpc.PbjEventHandler; +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.grpc.Pipelines; +import com.hedera.pbj.runtime.grpc.ServiceInterface; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import edu.umd.cs.findbugs.annotations.NonNull; +import greeter.HelloReply; +import greeter.HelloRequest; +import io.helidon.common.buffers.BufferData; +import io.helidon.http.http2.Http2Headers; +import io.helidon.http.http2.Http2StreamState; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.Flow; + +import static com.hedera.pbj.grpc.helidon.PbjProtocolHandlerTest.TestGreeterService.TestGreeterMethod.sayHelloStreamReply; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.withSettings; + +@ExtendWith(MockitoExtension.class) +public class PbjProtocolHandlerTest { + + @Mock private Http2Headers http2Headers; + + @Mock private Http2StreamWriter http2StreamWriter; + + @Mock private StreamFlowControl streamFlowControl; + + @Mock private Http2StreamState http2StreamState; + + @Mock private PbjConfig pbjConfig; + + @Mock private PbjMethodRoute pbjMethodRoute; + + @Mock private DeadlineDetector deadlineDetector; + + @Mock private BufferData bufferData; + + @Test + public void testOnErrorHandlerCalledOnException() { + + // We're testing the onError() routing from PbjProtocolHandler into + // an Application defined method. To confirm the registered handler + // gets called when there's an exception. + final Flow.Subscriber subscriber = mock(Flow.Subscriber.class, withSettings().extraInterfaces(PbjEventHandler.class)); + final PbjProtocolHandler testPbjProtocolHandler = + new TestPbjProtocolHandler( + http2Headers, + http2StreamWriter, + 1, + streamFlowControl, + http2StreamState, + pbjConfig, + pbjMethodRoute, + deadlineDetector, + subscriber); + + doThrow(IllegalArgumentException.class).when(bufferData).available(); + testPbjProtocolHandler.processData(null, bufferData); + } + + static class TestGreeterService implements ServiceInterface { + + private final Flow.Subscriber subscriber; + + public TestGreeterService(final Flow.Subscriber subscriber) { + this.subscriber = subscriber; + } + + enum TestGreeterMethod implements Method { + sayHelloStreamReply, + sayHelloStreamBidi + } + + @NonNull + public String serviceName() { + return "Greeter"; + } + + @NonNull + public String fullName() { + return "greeter.Greeter"; + } + + @NonNull + public List methods() { + return Arrays.asList(GreeterService.GreeterMethod.values()); + } + + @Override + @NonNull + public Pipeline open( + final @NonNull Method method, + final @NonNull RequestOptions options, + final @NonNull Flow.Subscriber replies) { + + final var m = (TestGreeterMethod) method; + try { + return switch (m) { + // Client sends a single request and the server sends many responses + case sayHelloStreamReply -> Pipelines.serverStreaming() + .mapRequest(bytes -> parseRequest(bytes, options)) + .method((request, replies) -> { + + if (replies instanceof PbjEventHandler pbjEventHandler) { + pbjEventHandler.registerOnErrorHandler(() -> { + System.out.println("Error handler called"); + subscriber.onError(new NoSuchAlgorithmException()); + }); + } + }) + .mapResponse(reply -> createReply(reply, options)) + .respondTo(replies) + .build(); + // Client and server are sending messages back and forth. + case sayHelloStreamBidi -> Pipelines.bidiStreaming() + .mapRequest(bytes -> parseRequest(bytes, options)) + .method(this::sayHelloStreamBidi) + .mapResponse(reply -> createReply(reply, options)) + .respondTo(replies) + .build(); + }; + } catch (Exception e) { + replies.onError(e); + return Pipelines.noop(); + } + } + + @NonNull + private HelloRequest parseRequest( + @NonNull final Bytes message, @NonNull final RequestOptions options) + throws InvalidProtocolBufferException { + Objects.requireNonNull(message); + + final HelloRequest request; + if (options.isProtobuf()) { + request = HelloRequest.parseFrom(message.toByteArray()); + } else if (options.isJson()) { + final var builder = HelloRequest.newBuilder(); + JsonFormat.parser().merge(message.asUtf8String(), builder); + request = builder.build(); + } else { + request = HelloRequest.newBuilder().setName(message.asUtf8String()).build(); + } + return request; + } + + @NonNull + private Bytes createReply( + @NonNull final HelloReply reply, @NonNull final RequestOptions options) + throws InvalidProtocolBufferException { + Objects.requireNonNull(reply); + + if (options.isProtobuf()) { + return Bytes.wrap(reply.toByteArray()); + } else if (options.isJson()) { + return Bytes.wrap(JsonFormat.printer().print(reply)); + } else { + return Bytes.wrap(reply.getMessage().getBytes()); + } + } + + void sayHelloStreamReply( + HelloRequest request, Flow.Subscriber replies) { + if (replies instanceof PbjEventHandler pbjEventHandler) { + pbjEventHandler.registerOnErrorHandler(() -> { + System.out.println("Error handler called"); + subscriber.onError(new NoSuchAlgorithmException()); + }); + } + } + + @NonNull + Pipeline sayHelloStreamBidi( + Flow.Subscriber replies) { + return null; + } + } + +// private static class TestGreeterProxy implements TestGreeterService { +// +// private final Flow.Subscriber subscriber; +// +// public TestGreeterProxy(final Flow.Subscriber subscriber) { +// this.subscriber = subscriber; +// } +// +// @Override +// public void sayHelloStreamReply( +// HelloRequest request, Flow.Subscriber replies) { +// if (replies instanceof PbjEventHandler pbjEventHandler) { +// pbjEventHandler.registerOnErrorHandler(() -> { +// System.out.println("Error handler called"); +// subscriber.onError(new NoSuchAlgorithmException()); +// }); +// } +// } +// +// @Override +// @NonNull +// public Pipeline sayHelloStreamBidi( +// Flow.Subscriber replies) { +// return null; +// } +// } + + + // Subclass PbjProtocolHandler to expose the pipeline for testing + private static class TestPbjProtocolHandler extends PbjProtocolHandler { + + TestPbjProtocolHandler( + @NonNull Http2Headers headers, + @NonNull Http2StreamWriter streamWriter, + int streamId, + @NonNull StreamFlowControl flowControl, + @NonNull Http2StreamState currentStreamState, + @NonNull PbjConfig config, + @NonNull PbjMethodRoute route, + @NonNull DeadlineDetector deadlineDetector, + @NonNull Flow.Subscriber subscriber) { + super( + headers, + streamWriter, + streamId, + flowControl, + currentStreamState, + config, + route, + deadlineDetector); + + final TestGreeterService greeterService = new TestGreeterService(subscriber); + super.pipeline = greeterService.open(sayHelloStreamReply, null, getOutgoing()); } + } +} From ac6638d68429b30cb1864d54d7ef15933387daed Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Mon, 11 Nov 2024 14:28:05 -0700 Subject: [PATCH 05/11] fix: fixed test compilation error but the test is still broken Signed-off-by: Matt Peterson --- .../pbj/grpc/helidon/PbjProtocolHandlerTest.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index e12a38a0..221cd127 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -131,15 +131,7 @@ public Pipeline open( // Client sends a single request and the server sends many responses case sayHelloStreamReply -> Pipelines.serverStreaming() .mapRequest(bytes -> parseRequest(bytes, options)) - .method((request, replies) -> { - - if (replies instanceof PbjEventHandler pbjEventHandler) { - pbjEventHandler.registerOnErrorHandler(() -> { - System.out.println("Error handler called"); - subscriber.onError(new NoSuchAlgorithmException()); - }); - } - }) + .method(this::sayHelloStreamReply) .mapResponse(reply -> createReply(reply, options)) .respondTo(replies) .build(); From 9802fbea8fe64bb284dc5fe1044ab14de3a357f2 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Mon, 11 Nov 2024 16:07:24 -0700 Subject: [PATCH 06/11] fix: switched back to the Pipeline interface approach Signed-off-by: Matt Peterson --- .../pbj/grpc/helidon/PbjProtocolHandler.java | 9 ++-- .../pbj/grpc/helidon/GreeterService.java | 8 +-- .../grpc/helidon/PbjProtocolHandlerTest.java | 49 +++---------------- .../com/hedera/pbj/grpc/helidon/PbjTest.java | 22 ++++----- .../pbj/runtime/grpc/PbjEventHandler.java | 28 ----------- .../com/hedera/pbj/runtime/grpc/Pipeline.java | 2 + .../hedera/pbj/runtime/grpc/Pipelines.java | 42 ++++++++-------- .../pbj/runtime/grpc/ServiceInterface.java | 2 +- .../pbj/runtime/grpc/PipelinesTest.java | 16 +++--- 9 files changed, 58 insertions(+), 120 deletions(-) delete mode 100644 pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index c763fd6f..7ba107af 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -30,7 +30,6 @@ import com.hedera.pbj.grpc.helidon.config.PbjConfig; import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; -import com.hedera.pbj.runtime.grpc.PbjEventHandler; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.ServiceInterface; import com.hedera.pbj.runtime.io.buffer.Bytes; @@ -293,7 +292,7 @@ public void init() { // Setup the subscribers. The "outgoing" subscriber will send messages to the client. // This is given to the "open" method on the service to allow it to send messages to // the client. - final Flow.Subscriber outgoing = getOutgoing(); + final Pipeline outgoing = getOutgoing(); pipeline = route.service().open(route.method(), options, outgoing); } catch (final GrpcException grpcException) { route.failedGrpcRequestCounter().increment(); @@ -318,7 +317,7 @@ public void init() { } // Visible for testing - protected Flow.Subscriber getOutgoing() { + protected Pipeline getOutgoing() { return new SendToClientSubscriber(); } @@ -648,10 +647,10 @@ protected void send( } /** - * The implementation of {@link Flow.Subscriber} used to send messages to the client. It + * The implementation of {@link Pipeline} used to send messages to the client. It * receives bytes from the handlers to send to the client. */ - private final class SendToClientSubscriber implements Flow.Subscriber, PbjEventHandler { + private final class SendToClientSubscriber implements Pipeline { private Runnable onErrorHandler; diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java index ae4b4caf..60d9bc12 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java @@ -48,14 +48,14 @@ enum GreeterMethod implements Method { // A stream of messages coming from the client, with a single response from the server. Pipeline sayHelloStreamRequest( - Flow.Subscriber replies); + Pipeline replies); // A single request from the client, with a stream of responses from the server. - void sayHelloStreamReply(HelloRequest request, Flow.Subscriber replies); + void sayHelloStreamReply(HelloRequest request, Pipeline replies); // A bidirectional stream of requests and responses between the client and the server. Pipeline sayHelloStreamBidi( - Flow.Subscriber replies); + Pipeline replies); @NonNull default String serviceName() { @@ -77,7 +77,7 @@ default List methods() { default Pipeline open( final @NonNull Method method, final @NonNull RequestOptions options, - final @NonNull Flow.Subscriber replies) { + final @NonNull Pipeline replies) { final var m = (GreeterMethod) method; try { diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index 221cd127..843c050e 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -19,7 +19,6 @@ import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; import com.hedera.pbj.grpc.helidon.config.PbjConfig; -import com.hedera.pbj.runtime.grpc.PbjEventHandler; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.Pipelines; import com.hedera.pbj.runtime.grpc.ServiceInterface; @@ -41,12 +40,10 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.concurrent.Flow; import static com.hedera.pbj.grpc.helidon.PbjProtocolHandlerTest.TestGreeterService.TestGreeterMethod.sayHelloStreamReply; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.withSettings; @ExtendWith(MockitoExtension.class) public class PbjProtocolHandlerTest { @@ -73,7 +70,7 @@ public void testOnErrorHandlerCalledOnException() { // We're testing the onError() routing from PbjProtocolHandler into // an Application defined method. To confirm the registered handler // gets called when there's an exception. - final Flow.Subscriber subscriber = mock(Flow.Subscriber.class, withSettings().extraInterfaces(PbjEventHandler.class)); + final Pipeline subscriber = mock(Pipeline.class); final PbjProtocolHandler testPbjProtocolHandler = new TestPbjProtocolHandler( http2Headers, @@ -92,9 +89,9 @@ public void testOnErrorHandlerCalledOnException() { static class TestGreeterService implements ServiceInterface { - private final Flow.Subscriber subscriber; + private final Pipeline subscriber; - public TestGreeterService(final Flow.Subscriber subscriber) { + public TestGreeterService(final Pipeline subscriber) { this.subscriber = subscriber; } @@ -123,7 +120,7 @@ public List methods() { public Pipeline open( final @NonNull Method method, final @NonNull RequestOptions options, - final @NonNull Flow.Subscriber replies) { + final @NonNull Pipeline replies) { final var m = (TestGreeterMethod) method; try { @@ -184,50 +181,20 @@ private Bytes createReply( } void sayHelloStreamReply( - HelloRequest request, Flow.Subscriber replies) { - if (replies instanceof PbjEventHandler pbjEventHandler) { - pbjEventHandler.registerOnErrorHandler(() -> { + HelloRequest request, Pipeline replies) { + replies.registerOnErrorHandler(() -> { System.out.println("Error handler called"); subscriber.onError(new NoSuchAlgorithmException()); }); - } } @NonNull Pipeline sayHelloStreamBidi( - Flow.Subscriber replies) { + Pipeline replies) { return null; } } -// private static class TestGreeterProxy implements TestGreeterService { -// -// private final Flow.Subscriber subscriber; -// -// public TestGreeterProxy(final Flow.Subscriber subscriber) { -// this.subscriber = subscriber; -// } -// -// @Override -// public void sayHelloStreamReply( -// HelloRequest request, Flow.Subscriber replies) { -// if (replies instanceof PbjEventHandler pbjEventHandler) { -// pbjEventHandler.registerOnErrorHandler(() -> { -// System.out.println("Error handler called"); -// subscriber.onError(new NoSuchAlgorithmException()); -// }); -// } -// } -// -// @Override -// @NonNull -// public Pipeline sayHelloStreamBidi( -// Flow.Subscriber replies) { -// return null; -// } -// } - - // Subclass PbjProtocolHandler to expose the pipeline for testing private static class TestPbjProtocolHandler extends PbjProtocolHandler { @@ -240,7 +207,7 @@ private static class TestPbjProtocolHandler extends PbjProtocolHandler { @NonNull PbjConfig config, @NonNull PbjMethodRoute route, @NonNull DeadlineDetector deadlineDetector, - @NonNull Flow.Subscriber subscriber) { + @NonNull Pipeline subscriber) { super( headers, streamWriter, diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjTest.java index 95705827..a4e58c5d 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjTest.java @@ -513,7 +513,7 @@ void exceptionThrownWhileOpening() { public Pipeline open( @NonNull Method method, @NonNull RequestOptions options, - @NonNull Flow.Subscriber replies) { + @NonNull Pipeline replies) { throw ex; } }; @@ -806,7 +806,7 @@ public HelloReply sayHello(HelloRequest request) { // Streams of stuff coming from the client, with a single response. @Override public Pipeline sayHelloStreamRequest( - Flow.Subscriber replies) { + Pipeline replies) { final var names = new ArrayList(); return new Pipeline<>() { @Override @@ -843,7 +843,7 @@ public void onComplete() { @Override public void sayHelloStreamReply( - HelloRequest request, Flow.Subscriber replies) { + HelloRequest request, Pipeline replies) { for (int i = 0; i < 10; i++) { replies.onNext(HelloReply.newBuilder().setMessage("Hello!").build()); } @@ -853,7 +853,7 @@ public void sayHelloStreamReply( @Override public Pipeline sayHelloStreamBidi( - Flow.Subscriber replies) { + Pipeline replies) { // Here we receive info from the client. In this case, it is a stream of requests with // names. We will respond with a stream of replies. return new Pipeline<>() { @@ -894,17 +894,17 @@ default HelloReply sayHello(HelloRequest request) { @Override default Pipeline sayHelloStreamRequest( - Flow.Subscriber replies) { + Pipeline replies) { return null; } @Override default void sayHelloStreamReply( - HelloRequest request, Flow.Subscriber replies) {} + HelloRequest request, Pipeline replies) {} @Override default Pipeline sayHelloStreamBidi( - Flow.Subscriber replies) { + Pipeline replies) { return null; } } @@ -921,20 +921,20 @@ public HelloReply sayHello(HelloRequest request) { @Override @NonNull public Pipeline sayHelloStreamRequest( - Flow.Subscriber replies) { + Pipeline replies) { return svc.sayHelloStreamRequest(replies); } @Override public void sayHelloStreamReply( - HelloRequest request, Flow.Subscriber replies) { + HelloRequest request, Pipeline replies) { svc.sayHelloStreamReply(request, replies); } @Override @NonNull public Pipeline sayHelloStreamBidi( - Flow.Subscriber replies) { + Pipeline replies) { return svc.sayHelloStreamBidi(replies); } @@ -943,7 +943,7 @@ public Pipeline sayHelloStreamBidi( public Pipeline open( @NonNull Method method, @NonNull RequestOptions options, - @NonNull Flow.Subscriber replies) { + @NonNull Pipeline replies) { return svc.open(method, options, replies); } } diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java deleted file mode 100644 index 3c7cd678..00000000 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/PbjEventHandler.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright (C) 2024 Hedera Hashgraph, LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.hedera.pbj.runtime.grpc; - -/** Interface for handling events in the PBJ runtime. */ -public interface PbjEventHandler { - - /** - * Register a handler for when an error occurs. - * - * @param runnable the handler to run - */ - void registerOnErrorHandler(Runnable runnable); -} diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java index 3fe5a016..d8debc6e 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipeline.java @@ -26,4 +26,6 @@ public interface Pipeline extends Flow.Subscriber { /** Called when an END_STREAM frame is received from the client. */ default void clientEndStreamReceived() {} + + default void registerOnErrorHandler(Runnable runnable) {} } diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java index 0f4c5a07..1b22a33b 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java @@ -34,7 +34,7 @@ private Pipelines() { } /** - * Returns a {@link Flow.Subscriber} that does nothing. This can be used in cases where a + * Returns a {@link Pipeline} that does nothing. This can be used in cases where a * subscriber is required but no proper implementation is available. * * @return A No-op subscriber. @@ -168,7 +168,7 @@ public interface UnaryBuilder { * @return This builder. */ @NonNull - UnaryBuilder respondTo(@NonNull Flow.Subscriber replies); + UnaryBuilder respondTo(@NonNull Pipeline replies); /** * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, @@ -229,7 +229,7 @@ public interface BidiStreamingBuilder { * @return This builder. */ @NonNull - BidiStreamingBuilder respondTo(@NonNull Flow.Subscriber replies); + BidiStreamingBuilder respondTo(@NonNull Pipeline replies); /** * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, @@ -291,7 +291,7 @@ public interface ClientStreamingBuilder { * @return This builder. */ @NonNull - ClientStreamingBuilder respondTo(@NonNull Flow.Subscriber replies); + ClientStreamingBuilder respondTo(@NonNull Pipeline replies); /** * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, @@ -350,7 +350,7 @@ public interface ServerStreamingBuilder { * @return This builder. */ @NonNull - ServerStreamingBuilder respondTo(@NonNull Flow.Subscriber replies); + ServerStreamingBuilder respondTo(@NonNull Pipeline replies); /** * Builds the pipeline and returns it. The returned pipeline receives the incoming messages, @@ -390,7 +390,7 @@ public interface ExceptionalFunction { */ @FunctionalInterface public interface ClientStreamingMethod - extends ExceptionalFunction, Flow.Subscriber> {} + extends ExceptionalFunction, Pipeline> {} /** * A function that handles a server streaming gRPC service method. A single request is received @@ -407,7 +407,7 @@ public interface ServerStreamingMethod { * @param replies The subscriber to send responses to. * @throws Exception If an error occurs during processing. */ - void apply(@NonNull T request, @NonNull Flow.Subscriber replies) + void apply(@NonNull T request, @NonNull Pipeline replies) throws Exception; } @@ -419,12 +419,12 @@ void apply(@NonNull T request, @NonNull Flow.Subscriber replies) * @param The type of the response message. */ public interface BidiStreamingMethod - extends ExceptionalFunction, Flow.Subscriber> {} + extends ExceptionalFunction, Pipeline> {} /** * A convenient base class for the different builders. All builders have to hold state for * request and response mapping functions, as well as the subscriber to send responses to, so we - * have a base class. This class also implements the {@link Flow.Subscriber} and {@link + * have a base class. This class also implements the {@link Pipeline} and {@link * Flow.Subscription} interfaces, to reduce the overall number of instances created. * *

A {@link Flow.Subscription} is provided to each subscriber at the time they subscribe. @@ -438,7 +438,7 @@ private abstract static class PipelineBuilderImpl implements Pipeline, Flow.Subscription { protected ExceptionalFunction requestMapper; protected ExceptionalFunction responseMapper; - protected Flow.Subscriber replies; + protected Pipeline replies; private Flow.Subscription sourceSubscription; protected boolean completed = false; @@ -524,7 +524,7 @@ public UnaryBuilder mapResponse(@NonNull final ExceptionalFunction respondTo(@NonNull final Flow.Subscriber replies) { + public UnaryBuilder respondTo(@NonNull final Pipeline replies) { this.replies = requireNonNull(replies); return this; } @@ -576,7 +576,7 @@ public void onNext(@NonNull final Bytes message) { private static final class BidiStreamingBuilderImpl extends PipelineBuilderImpl implements BidiStreamingBuilder { private BidiStreamingMethod method; - private Flow.Subscriber incoming; + private Pipeline incoming; @Override @NonNull @@ -605,7 +605,7 @@ public BidiStreamingBuilderImpl mapResponse( @Override @NonNull public BidiStreamingBuilderImpl respondTo( - @NonNull final Flow.Subscriber replies) { + @NonNull final Pipeline replies) { this.replies = replies; return this; } @@ -683,7 +683,7 @@ public void clientEndStreamReceived() { private static final class ClientStreamingBuilderImpl extends PipelineBuilderImpl implements ClientStreamingBuilder { private ClientStreamingMethod method; - private Flow.Subscriber incoming; + private Pipeline incoming; @Override @NonNull @@ -712,7 +712,7 @@ public ClientStreamingBuilderImpl mapResponse( @Override @NonNull public ClientStreamingBuilderImpl respondTo( - @NonNull final Flow.Subscriber replies) { + @NonNull final Pipeline replies) { this.replies = replies; return this; } @@ -773,7 +773,7 @@ public void clientEndStreamReceived() { private static final class ServerStreamingBuilderImpl extends PipelineBuilderImpl implements ServerStreamingBuilder { private ServerStreamingMethod method; - private Flow.Subscriber responseConverter; + private Pipeline responseConverter; @Override @NonNull @@ -802,7 +802,7 @@ public ServerStreamingBuilderImpl mapResponse( @Override @NonNull public ServerStreamingBuilderImpl respondTo( - @NonNull final Flow.Subscriber replies) { + @NonNull final Pipeline replies) { this.replies = replies; return this; } @@ -849,8 +849,8 @@ public void onNext(@NonNull final Bytes message) { * @param The type of the output. */ private record MapSubscriber( - Flow.Subscriber next, ExceptionalFunction mapper) - implements Flow.Subscriber, Flow.Subscription, PbjEventHandler { + Pipeline next, ExceptionalFunction mapper) + implements Pipeline, Flow.Subscription { private MapSubscriber { next.onSubscribe(this); @@ -893,9 +893,7 @@ public void onComplete() { @Override public void registerOnErrorHandler(Runnable handler) { - if (next instanceof PbjEventHandler pbjEventHandler) { - pbjEventHandler.registerOnErrorHandler(handler); - } + next.registerOnErrorHandler(handler); } } } diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java index a7e11d67..367164a2 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java @@ -142,6 +142,6 @@ interface RequestOptions { */ @NonNull Pipeline open( - @NonNull Method method, @NonNull RequestOptions opts, @NonNull Flow.Subscriber responses) + @NonNull Method method, @NonNull RequestOptions opts, @NonNull Pipeline responses) throws GrpcException; } diff --git a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java index 7acd3900..1d3d480d 100644 --- a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java +++ b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java @@ -71,7 +71,7 @@ void noopOnCompleteDoesNothing() { @Nested @ExtendWith(MockitoExtension.class) class UnaryTest { - @Mock Flow.Subscriber replies; + @Mock Pipeline replies; @Mock Flow.Subscription subscription; @Test @@ -184,8 +184,8 @@ void positive() { @Nested @ExtendWith(MockitoExtension.class) class BidiTest { - @Mock Flow.Subscriber client; - @Mock Flow.Subscriber replies; + @Mock Pipeline client; + @Mock Pipeline replies; @Mock Flow.Subscription subscription; @Test @@ -341,7 +341,7 @@ void positive() { @Nested @ExtendWith(MockitoExtension.class) class ServerStreamingTest { - @Mock Flow.Subscriber replies; + @Mock Pipeline replies; @Mock Flow.Subscription subscription; @Test @@ -470,7 +470,7 @@ void positive() { @Nested @ExtendWith(MockitoExtension.class) class ClientStreamingTest { - @Mock Flow.Subscriber replies; + @Mock Pipeline replies; @Mock Flow.Subscription subscription; @Test @@ -595,11 +595,11 @@ void positive() { verify(replies).onNext(Bytes.wrap("hello:world")); } - private static final class ConcatenatingHandler implements Flow.Subscriber { + private static final class ConcatenatingHandler implements Pipeline { private final List strings = new ArrayList<>(); - private final Flow.Subscriber sink; + private final Pipeline sink; - private ConcatenatingHandler(Flow.Subscriber sink) { + private ConcatenatingHandler(Pipeline sink) { this.sink = sink; } From f3002ce4620ee9ebbd1433586111ec923864a11a Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Tue, 12 Nov 2024 13:23:44 -0700 Subject: [PATCH 07/11] wip: broken but first phase of refactor Signed-off-by: Matt Peterson --- .../hedera/pbj/grpc/helidon/Constants.java | 15 + .../pbj/grpc/helidon/GrpcDataProcessor.java | 14 + .../grpc/helidon/GrpcDataProcessorImpl.java | 169 +++++ .../pbj/grpc/helidon/HeadersProcessor.java | 356 ++++++++++ .../pbj/grpc/helidon/PbjProtocolHandler.java | 663 +----------------- .../pbj/grpc/helidon/PbjProtocolSelector.java | 31 +- .../pbj/grpc/helidon/PipelineBuilder.java | 31 + .../grpc/helidon/SendToClientSubscriber.java | 132 ++++ .../pbj/grpc/helidon/TrailerBuilder.java | 97 +++ .../pbj/grpc/helidon/TrailerOnlyBuilder.java | 60 ++ .../grpc/helidon/PbjProtocolHandlerTest.java | 64 +- 11 files changed, 934 insertions(+), 698 deletions(-) create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java new file mode 100644 index 00000000..bd9b3788 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java @@ -0,0 +1,15 @@ +package com.hedera.pbj.grpc.helidon; + +import io.helidon.http.Header; +import io.helidon.http.HeaderValues; + +public final class Constants { + private Constants() {} + + /** The only grpc-encoding supported by this implementation. */ + public static final String IDENTITY = "identity"; + + /** A pre-created and cached *response* header for "grpc-encoding: identity". */ + public static final Header GRPC_ENCODING_IDENTITY = + HeaderValues.createCached("grpc-encoding", IDENTITY); +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java new file mode 100644 index 00000000..3ecdab05 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java @@ -0,0 +1,14 @@ +package com.hedera.pbj.grpc.helidon; + +import edu.umd.cs.findbugs.annotations.NonNull; +import io.helidon.common.buffers.BufferData; +import io.helidon.http.http2.Http2FrameHeader; +import io.helidon.http.http2.Http2StreamState; + +import java.util.function.UnaryOperator; + +public interface GrpcDataProcessor { + void data(@NonNull final Http2FrameHeader header, @NonNull final BufferData data); + void setCurrentStreamState(UnaryOperator operator); + Http2StreamState getCurrentStreamState(); +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java new file mode 100644 index 00000000..b8374c10 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java @@ -0,0 +1,169 @@ +package com.hedera.pbj.grpc.helidon; + +import com.hedera.pbj.grpc.helidon.config.PbjConfig; +import com.hedera.pbj.runtime.grpc.GrpcException; +import com.hedera.pbj.runtime.grpc.GrpcStatus; +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import edu.umd.cs.findbugs.annotations.NonNull; +import io.helidon.common.buffers.BufferData; +import io.helidon.http.http2.Http2FrameHeader; +import io.helidon.http.http2.Http2FrameTypes; +import io.helidon.http.http2.Http2StreamState; + +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.UnaryOperator; + +import static java.util.Objects.requireNonNull; + +public class GrpcDataProcessorImpl implements GrpcDataProcessor { + + /** + * The bytes of the next incoming message. This is created dynamically as a message is received, + * and is never larger than the system configured {@link PbjConfig#maxMessageSizeBytes()}. + * + *

This member is only accessed by the {@link #data} method, which is called sequentially. + */ + private byte[] entityBytes; + + /** + * The current index into {@link #entityBytes} into which data is to be read. + * + *

This member is only accessed by the {@link #data} method, which is called sequentially. + */ + private int entityBytesIndex; + + /** States for currentReadState state ,machine */ + enum ReadState { + /** + * Start state, when we are looking for first byte that says if data is compressed or not + */ + START, + /** + * State were we are reading length, can be partial length of final point when we have all + * length bytes + */ + READ_LENGTH, + /** State where we are reading the protobuf entity bytes */ + READ_ENTITY_BYTES + } + + /** State machine as we read bytes from incoming data */ + private ReadState currentReadState = ReadState.START; + + /** Number of read bytes between 0 and {@code Integer.BYTES} = 4 */ + private int numOfPartReadBytes; + + /** Byte array to store bytes as we build up to a full 4 byte integer */ + private final byte[] partReadLengthBytes = new byte[Integer.BYTES]; + + private final PbjConfig config; + private final AtomicReference currentStreamState; + + /** + * The communication pipeline between server and client + * + *

Method calls on this object are thread-safe. + */ + private Pipeline pipeline; + + public GrpcDataProcessorImpl( + @NonNull final PbjConfig config, + @NonNull final Http2StreamState currentStreamState) { + + this.config = requireNonNull(config); + this.currentStreamState = new AtomicReference<>(requireNonNull(currentStreamState)); + } + + public void setPipeline(@NonNull final Pipeline pipeline) { + this.pipeline = requireNonNull(pipeline); + } + + @Override + public void data(@NonNull final Http2FrameHeader header, @NonNull final BufferData data) { + + try { + // NOTE: if the deadline is exceeded, then the stream will be closed and data will no + // longer flow. + // There is some asynchronous behavior here, but in the worst case, we handle a few more + // bytes before the stream is closed. + while (data.available() > 0) { + // First chunk of data contains the compression flag and the length of the message + if (entityBytes == null) { + // Read whether this message is compressed. We do not currently support + // compression. + final var isCompressed = (data.read() == 1); + if (isCompressed) { + // The error will eventually result in the stream being closed + throw new GrpcException( + GrpcStatus.UNIMPLEMENTED, "Compression is not supported"); + } + // Read the length of the message. As per the grpc protocol specification, each + // message on the wire is prefixed with the number of bytes for the message. + // However, to prevent a DOS attack where the attacker sends us a very large + // length and exhausts our memory, we have a maximum message size configuration + // setting. Using that, we can detect attempts to exhaust our memory. + final long length = data.readUnsignedInt32(); + if (length > config.maxMessageSizeBytes()) { + throw new GrpcException( + GrpcStatus.INVALID_ARGUMENT, + "Message size exceeds maximum allowed size"); + } + // Create a buffer to hold the message. We sadly cannot reuse this buffer + // because once we have filled it and wrapped it in Bytes and sent it to the + // handler, some user code may grab and hold that Bytes object for an arbitrary + // amount of time, and if we were to scribble into the same byte array, we + // would break the application. So we need a new buffer each time :-( + entityBytes = new byte[(int) length]; + entityBytesIndex = 0; + } + + // By the time we get here, entityBytes is no longer null. It may be empty, or it + // may already have been partially populated from a previous iteration. It may be + // that the number of bytes available to be read is larger than just this one + // message. So we need to be careful to read, from what is available, only up to + // the message length, and to leave the rest for the next iteration. + final int available = data.available(); + final int numBytesToRead = + Math.min(entityBytes.length - entityBytesIndex, available); + data.read(entityBytes, entityBytesIndex, numBytesToRead); + entityBytesIndex += numBytesToRead; + + // If we have completed reading the message, then we can proceed. + if (entityBytesIndex == entityBytes.length) { + // Grab and wrap the bytes and reset to being reading the next message + final var bytes = Bytes.wrap(entityBytes); + pipeline.onNext(bytes); + entityBytesIndex = 0; + entityBytes = null; + } + } + + // The end of the stream has been reached! It is possible that a bad client will send + // end of stream before all the message data we sent. In that case, it is as if the + // message were never sent. + if (header.flags(Http2FrameTypes.DATA).endOfStream()) { + entityBytesIndex = 0; + entityBytes = null; + currentStreamState.set(Http2StreamState.HALF_CLOSED_REMOTE); + + pipeline.clientEndStreamReceived(); + } + } catch (final Exception e) { + // I have to propagate this error through the service interface, so it can respond to + // errors in the connection, tear down resources, etc. It will also forward this on + // to the client, causing the connection to be torn down. + pipeline.onError(e); + } + } + + @Override + public void setCurrentStreamState(UnaryOperator operator) { + this.currentStreamState.getAndUpdate(operator); + } + + @Override + public Http2StreamState getCurrentStreamState() { + return currentStreamState.get(); + } +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java new file mode 100644 index 00000000..2f41cdaa --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java @@ -0,0 +1,356 @@ +package com.hedera.pbj.grpc.helidon; + +import com.hedera.pbj.runtime.grpc.GrpcException; +import com.hedera.pbj.runtime.grpc.GrpcStatus; +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.grpc.ServiceInterface; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import io.helidon.http.Header; +import io.helidon.http.HeaderNames; +import io.helidon.http.HttpException; +import io.helidon.http.HttpMediaType; +import io.helidon.http.HttpMediaTypes; +import io.helidon.http.Status; +import io.helidon.http.WritableHeaders; +import io.helidon.http.http2.Http2Flag; +import io.helidon.http.http2.Http2Headers; +import io.helidon.http.http2.Http2StreamState; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; + +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Delayed; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; + +import static com.hedera.pbj.grpc.helidon.Constants.GRPC_ENCODING_IDENTITY; +import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ENCODING; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_TIMEOUT; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_JSON; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_PROTO; +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + +class HeadersProcessor { + private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); + + /** The regular expression used to parse the grpc-timeout header. */ + private static final String GRPC_TIMEOUT_REGEX = "(\\d{1,8})([HMSmun])"; + private static final Pattern GRPC_TIMEOUT_PATTERN = Pattern.compile(GRPC_TIMEOUT_REGEX); + + /** + * A future representing the background task detecting deadlines. If there is a deadline, then + * this future will represent the task that will be executed when the deadline is reached. If + * there is no deadline, then we default to a non-null no-op future that exists in the infinite + * future. + * + *

Method calls on this object are thread-safe. + */ + private ScheduledFuture deadlineFuture; + + /** + * If there is a timeout defined for the request, then this detector is used to determine when + * the timeout deadline has been met. The detector runs on a background thread/timer. + */ + private final DeadlineDetector deadlineDetector; + + /** + * The service method that this connection was created for. The route has information about the + * {@link ServiceInterface} and method to invoke, as well as metrics, and other information. + */ + private final PbjMethodRoute route; + private final Http2StreamWriter streamWriter; + private final StreamFlowControl flowControl; + private final int streamId; + private ServiceInterface.RequestOptions options; + private Pipeline pipeline; + + HeadersProcessor( + @NonNull final Http2Headers headers, + @NonNull final Http2StreamWriter streamWriter, + final int streamId, + @NonNull final StreamFlowControl flowControl, + @NonNull final PbjMethodRoute route, + @NonNull final DeadlineDetector deadlineDetector) { + + this.streamWriter = streamWriter; + this.streamId = streamId; + this.flowControl = flowControl; + this.route = route; + this.deadlineDetector = deadlineDetector; + + try { + // If Content-Type does not begin with "application/grpc", gRPC servers SHOULD respond + // with HTTP status of 415 (Unsupported Media Type). This will prevent other HTTP/2 + // clients from interpreting a gRPC error response, which uses status 200 (OK), as + // successful. + // See https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md + // In addition, "application/grpc" is interpreted as "application/grpc+proto". + final var requestHeaders = headers.httpHeaders(); + final var requestContentType = + requestHeaders.contentType().orElse(HttpMediaTypes.PLAINTEXT_UTF_8); + final var ct = requestContentType.text(); + final var contentType = + switch (ct) { + case APPLICATION_GRPC, APPLICATION_GRPC_PROTO -> APPLICATION_GRPC_PROTO; + case APPLICATION_GRPC_JSON -> APPLICATION_GRPC_JSON; + default -> { + if (ct.startsWith(APPLICATION_GRPC)) { + yield ct; + } + throw new HttpException( + "Unsupported", Status.UNSUPPORTED_MEDIA_TYPE_415); + } + }; + + // This implementation currently only supports "identity" compression. + // + // As per the documentation: + // If a client message is compressed by an algorithm that is not supported by a server, + // the message will result in an UNIMPLEMENTED error status on the server. The server + // will include a grpc-accept-encoding header [in] the response which specifies the + // algorithms that the server accepts. + // + // Note that in the HeadersBuilder we ALWAYS set the grpc-accept-encoding header. + // FUTURE: Add support for the other compression schemes and let the response be in the + // same scheme that was sent to us, or another scheme in "grpc-accept-encoding" that + // we support, or identity. + final var encodingHeader = requestHeaders.value(GRPC_ENCODING).orElse(IDENTITY); + if (!IDENTITY.equals(encodingHeader)) { + throw new GrpcException(GrpcStatus.UNIMPLEMENTED); + } + + // The client may have sent a "grpc-accept-encoding" header. Note that + // "grpc-accept-encoding" is not well specified. I am following what I see work with + // the grpc.io client library, and the definition of "accept-encoding" for HTTP, such + // that, identity is *always* safe, but only compression algorithms supported by + // "grpc-accept-encoding" should be used if any compression algorithm will be used. + // + // To support this claim, the spec says: + // "A Compressed-Flag value of 1 indicates that the binary octet sequence of Message is + // compressed using the mechanism declared by the Message-Encoding header. A value of + // 0 indicates that no encoding of Message bytes has occurred. + // + // This seems to support the notion that compression can be enabled or disabled + // irrespective of the grpc-accept-encoding header. + + // FUTURE: If the client sends a "grpc-accept-encoding", and if we support one of them, + // then we should pick one and use it in the response. Otherwise, we should not have + // any compression. + + // If the grpc-timeout header is present, determine when that timeout would occur, or + // default to a future that is so far in the future it will never happen. + final var timeout = requestHeaders.value(GRPC_TIMEOUT); + + deadlineFuture = + timeout.map(this::scheduleDeadline).orElse(new NoopScheduledFuture<>()); + + // At this point, the request itself is valid. Maybe it will still fail to be handled by + // the service interface, but as far as the protocol is concerned, this was a valid + // request. Send the headers back to the client (the messages and trailers will be sent + // later). + sendResponseHeaders(GRPC_ENCODING_IDENTITY, requestContentType, emptyList()); + + // NOTE: The specification mentions the "Message-Type" header. Like everyone else, we're + // just going to ignore that header. See https://github.com/grpc/grpc/issues/12468. + + // FUTURE: Should we support custom metadata, we would extract it here and pass it along + // via "options". + // We should have a wrapper around them, such that we don't process the custom headers + // ourselves, but allow the service interface to look up special headers based on key. + + // Create the "options" to make available to the ServiceInterface. These options are + // used to decide on the best way to parse or handle the request. + options = + new Options( + Optional.ofNullable(headers.authority()), // the client (see http2 spec) + contentType.equals(APPLICATION_GRPC_PROTO), + contentType.equals(APPLICATION_GRPC_JSON), + contentType); + + } catch (final GrpcException grpcException) { + route.failedGrpcRequestCounter().increment(); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .grpcStatus(grpcException.status()) + .statusMessage(grpcException.getMessage()) + .send(); +// error(); + } catch (final HttpException httpException) { + route.failedHttpRequestCounter().increment(); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .httpStatus(httpException.status()) + .grpcStatus(GrpcStatus.INVALID_ARGUMENT) + .send(); +// error(); + } catch (final Exception unknown) { + route.failedUnknownRequestCounter().increment(); + LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .grpcStatus(GrpcStatus.UNKNOWN).send(); +// error(); + } + } + + public void setPipeline(@NonNull final Pipeline pipeline) { + this.pipeline = requireNonNull(pipeline); + } + + public void cancelDeadlineFuture(boolean isCancelled) { + deadlineFuture.cancel(isCancelled); + } + + public ServiceInterface.RequestOptions options() { + return options; + } + + /** + * According to the specification, the "Response-Headers" are: + * + * 

HTTP-Status [Message-Encoding] [Message-Accept-Encoding] Content-Type *Custom-Metadata
+     *
+ * + *

Where "Status" is always 200 OK. + * + *

The Response-Headers are normally sent, followed by the data, followed by trailers. But if + * the request fails right away, before any handling, then it is possible to send Trailers-Only + * instead. + */ + private void sendResponseHeaders( + @Nullable final Header messageEncoding, + @NonNull final HttpMediaType contentType, + @NonNull final List

customMetadata) { + + // Some headers are http2 specific, the rest are used for the grpc protocol + final var grpcHeaders = WritableHeaders.create(); + // FUTURE: I think to support custom headers in the response, we would have to list them + // here. + // Since this has to be sent before we have any data to send, we must know ahead of time + // which custom headers are to be returned. + grpcHeaders.set(HeaderNames.TRAILER, "grpc-status, grpc-message"); + grpcHeaders.set(Http2Headers.STATUS_NAME, Status.OK_200.code()); + grpcHeaders.contentType(contentType); + grpcHeaders.set(GRPC_ACCEPT_ENCODING, IDENTITY); + customMetadata.forEach(grpcHeaders::set); + if (messageEncoding != null) { + grpcHeaders.set(messageEncoding); + } + + final var http2Headers = Http2Headers.create(grpcHeaders); + + streamWriter.writeHeaders( + http2Headers, + streamId, + Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS), + flowControl.outbound()); + } + + /** + * Helper function. Given a string of digits followed by a unit, schedule a callback to be + * invoked when the deadline is exceeded, and return the associated future. The proper format of + * the string is defined in the specification as: + * + * 
+     *      Timeout → "grpc-timeout" TimeoutValue TimeoutUnit
+     *      TimeoutValue → {positive integer as ASCII string of at most 8 digits}
+     *      TimeoutUnit → Hour / Minute / Second / Millisecond / Microsecond / Nanosecond
+     *             Hour → "H"
+     *             Minute → "M"
+     *             Second → "S"
+     *             Millisecond → "m"
+     *             Microsecond → "u"
+     *             Nanosecond → "n"
+     *
+ * + *

Illegal values result in the deadline being ignored. + * + * @param timeout The timeout value. Cannot be null. + * @return The future representing the task that will be executed if/when the deadline is + * reached. + */ + @NonNull + private ScheduledFuture scheduleDeadline(@NonNull final String timeout) { + final var matcher = GRPC_TIMEOUT_PATTERN.matcher(timeout); + if (matcher.matches()) { + final var num = Integer.parseInt(matcher.group(1)); + final var unit = matcher.group(2); + final var deadline = + System.nanoTime() + * TimeUnit.NANOSECONDS.convert( + num, + switch (unit) { + case "H" -> TimeUnit.HOURS; + case "M" -> TimeUnit.MINUTES; + case "S" -> TimeUnit.SECONDS; + case "m" -> TimeUnit.MILLISECONDS; + case "u" -> TimeUnit.MICROSECONDS; + case "n" -> TimeUnit.NANOSECONDS; + // This should NEVER be reachable, because the matcher + // would not have matched. + default -> throw new GrpcException( + GrpcStatus.INTERNAL, "Invalid unit: " + unit); + }); + return deadlineDetector.scheduleDeadline( + deadline, + () -> { + route.deadlineExceededCounter().increment(); + pipeline.onError(new GrpcException(GrpcStatus.DEADLINE_EXCEEDED)); + }); + } + + return new NoopScheduledFuture<>(); + } + + /** + * A {@link ScheduledFuture} that does nothing. This is used when there is no deadline set for + * the request. A new instance of this must be created (or we need a "reset" method) for each + * {@link PbjProtocolHandler} instance, because it can become "corrupted" if canceled from any + * particular call. + */ + private static final class NoopScheduledFuture extends CompletableFuture + implements ScheduledFuture { + @Override + public long getDelay(@NonNull final TimeUnit unit) { + return 0; + } + + @Override + public int compareTo(@NonNull final Delayed o) { + // Since all NoopScheduledFuture instances have "0" as the delay, any other Delayed + // instance with a non-0 + // delay will come after this one. + return (int) (o.getDelay(TimeUnit.NANOSECONDS)); + } + } + + /** + * An error has occurred. Cancel the deadline future if it's still active, and set the stream + * state accordingly. + * + *

May be called by different threads concurrently. + */ + private void error() { + // Canceling a future that has already completed has no effect. So by canceling here, we are + // saying: + // "If you have not yet executed, never execute. If you have already executed, then just + // ignore me". + // The "isCancelled" flag is set if the future was canceled before it was executed. + + // cancel is threadsafe + cancelDeadlineFuture(false); + grpcDataProcessor.setCurrentStreamState(current -> Http2StreamState.CLOSED); + } + + /** Simple implementation of the {@link ServiceInterface.RequestOptions} interface. */ + protected record Options( + Optional authority, boolean isProtobuf, boolean isJson, String contentType) + implements ServiceInterface.RequestOptions {} +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index 7ba107af..81d0c547 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -16,84 +16,44 @@ package com.hedera.pbj.grpc.helidon; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.APPLICATION_GRPC_PROTO_TYPE; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ENCODING; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_TIMEOUT; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_JSON; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_PROTO; -import static java.lang.System.Logger.Level.ERROR; -import static java.util.Collections.emptyList; -import static java.util.Objects.requireNonNull; - -import com.hedera.pbj.grpc.helidon.config.PbjConfig; import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.ServiceInterface; import com.hedera.pbj.runtime.io.buffer.Bytes; import edu.umd.cs.findbugs.annotations.NonNull; -import edu.umd.cs.findbugs.annotations.Nullable; import io.helidon.common.buffers.BufferData; -import io.helidon.common.uri.UriEncoding; -import io.helidon.http.Header; -import io.helidon.http.HeaderNames; -import io.helidon.http.HeaderValues; -import io.helidon.http.HttpException; -import io.helidon.http.HttpMediaType; -import io.helidon.http.HttpMediaTypes; -import io.helidon.http.Status; -import io.helidon.http.WritableHeaders; import io.helidon.http.http2.Http2Flag; import io.helidon.http.http2.Http2FrameData; import io.helidon.http.http2.Http2FrameHeader; import io.helidon.http.http2.Http2FrameTypes; -import io.helidon.http.http2.Http2Headers; import io.helidon.http.http2.Http2RstStream; import io.helidon.http.http2.Http2StreamState; import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.Http2WindowUpdate; import io.helidon.http.http2.StreamFlowControl; import io.helidon.webserver.http2.spi.Http2SubProtocolSelector; -import java.util.List; + import java.util.Objects; -import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Delayed; import java.util.concurrent.Flow; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Pattern; + +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Objects.requireNonNull; /** * Implementation of gRPC based on PBJ. This class specifically contains the glue logic for bridging * between Helidon and the generated PBJ service handler endpoints. An instance of this class is * created for each new connection, and each connection is made to a specific method endpoint. */ -class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHandler { +final class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHandler { private static final System.Logger LOGGER = System.getLogger(PbjProtocolHandler.class.getName()); - /** The only grpc-encoding supported by this implementation. */ - private static final String IDENTITY = "identity"; - - /** A pre-created and cached *response* header for "grpc-encoding: identity". */ - private static final Header GRPC_ENCODING_IDENTITY = - HeaderValues.createCached("grpc-encoding", IDENTITY); - - /** The regular expression used to parse the grpc-timeout header. */ - private static final String GRPC_TIMEOUT_REGEX = "(\\d{1,8})([HMSmun])"; - - private static final Pattern GRPC_TIMEOUT_PATTERN = Pattern.compile(GRPC_TIMEOUT_REGEX); - // Helidon-specific fields related to the connection itself - private final Http2Headers headers; private final Http2StreamWriter streamWriter; private final int streamId; private final StreamFlowControl flowControl; - private final AtomicReference currentStreamState; + private final HeadersProcessor headersProcessor; /** * The service method that this connection was created for. The route has information about the @@ -101,94 +61,25 @@ class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHandler */ private final PbjMethodRoute route; - private final PbjConfig config; - - /** - * If there is a timeout defined for the request, then this detector is used to determine when - * the timeout deadline has been met. The detector runs on a background thread/timer. - */ - private final DeadlineDetector deadlineDetector; - - /** - * A future representing the background task detecting deadlines. If there is a deadline, then - * this future will represent the task that will be executed when the deadline is reached. If - * there is no deadline, then we default to a non-null no-op future that exists in the infinite - * future. - * - *

This member isn't final because it is set in the {@link #init()} method. It should not be - * set at any other time. - * - *

Method calls on this object are thread-safe. - */ - private ScheduledFuture deadlineFuture; - - /** - * The bytes of the next incoming message. This is created dynamically as a message is received, - * and is never larger than the system configured {@link PbjConfig#maxMessageSizeBytes()}. - * - *

This member is only accessed by the {@link #data} method, which is called sequentially. - */ - private byte[] entityBytes = null; - - /** - * The current index into {@link #entityBytes} into which data is to be read. - * - *

This member is only accessed by the {@link #data} method, which is called sequentially. - */ - private int entityBytesIndex = 0; - - /** States for currentReadState state ,machine */ - enum ReadState { - /** - * Start state, when we are looking for first byte that says if data is compressed or not - */ - START, - /** - * State were we are reading length, can be partial length of final point when we have all - * length bytes - */ - READ_LENGTH, - /** State where we are reading the protobuf entity bytes */ - READ_ENTITY_BYTES - } - - /** State machine as we read bytes from incoming data */ - private ReadState currentReadState = ReadState.START; - - /** Number of read bytes between 0 and {@code Integer.BYTES} = 4 */ - private int numOfPartReadBytes = 0; - - /** Byte array to store bytes as we build up to a full 4 byte integer */ - private final byte[] partReadLengthBytes = new byte[Integer.BYTES]; - - /** - * The communication pipeline between server and client - * - *

This member isn't final because it is set in the {@link #init()} method. It should not be - * set at any other time. - * - *

Method calls on this object are thread-safe. - */ - protected Pipeline pipeline; + private final Pipeline pipeline; + private final GrpcDataProcessor grpcDataProcessor; /** Create a new instance */ PbjProtocolHandler( - @NonNull final Http2Headers headers, @NonNull final Http2StreamWriter streamWriter, final int streamId, @NonNull final StreamFlowControl flowControl, - @NonNull final Http2StreamState currentStreamState, - @NonNull final PbjConfig config, @NonNull final PbjMethodRoute route, - @NonNull final DeadlineDetector deadlineDetector) { - this.headers = requireNonNull(headers); + @NonNull final GrpcDataProcessor grpcDataProcessor, + @NonNull final HeadersProcessor headersProcessor, + @NonNull final Pipeline pipeline) { this.streamWriter = requireNonNull(streamWriter); this.streamId = streamId; this.flowControl = requireNonNull(flowControl); - this.currentStreamState = new AtomicReference<>(requireNonNull(currentStreamState)); - this.config = requireNonNull(config); this.route = requireNonNull(route); - this.deadlineDetector = requireNonNull(deadlineDetector); + this.grpcDataProcessor = requireNonNull(grpcDataProcessor); + this.headersProcessor = requireNonNull(headersProcessor); + this.pipeline = requireNonNull(pipeline); } /** @@ -199,132 +90,12 @@ enum ReadState { @Override public void init() { route.requestCounter().increment(); - - try { - // If Content-Type does not begin with "application/grpc", gRPC servers SHOULD respond - // with HTTP status of 415 (Unsupported Media Type). This will prevent other HTTP/2 - // clients from interpreting a gRPC error response, which uses status 200 (OK), as - // successful. - // See https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md - // In addition, "application/grpc" is interpreted as "application/grpc+proto". - final var requestHeaders = headers.httpHeaders(); - final var requestContentType = - requestHeaders.contentType().orElse(HttpMediaTypes.PLAINTEXT_UTF_8); - final var ct = requestContentType.text(); - final var contentType = - switch (ct) { - case APPLICATION_GRPC, APPLICATION_GRPC_PROTO -> APPLICATION_GRPC_PROTO; - case APPLICATION_GRPC_JSON -> APPLICATION_GRPC_JSON; - default -> { - if (ct.startsWith(APPLICATION_GRPC)) { - yield ct; - } - throw new HttpException( - "Unsupported", Status.UNSUPPORTED_MEDIA_TYPE_415); - } - }; - - // This implementation currently only supports "identity" compression. - // - // As per the documentation: - // If a client message is compressed by an algorithm that is not supported by a server, - // the message will result in an UNIMPLEMENTED error status on the server. The server - // will include a grpc-accept-encoding header [in] the response which specifies the - // algorithms that the server accepts. - // - // Note that in the HeadersBuilder we ALWAYS set the grpc-accept-encoding header. - // FUTURE: Add support for the other compression schemes and let the response be in the - // same scheme that was sent to us, or another scheme in "grpc-accept-encoding" that - // we support, or identity. - final var encodingHeader = requestHeaders.value(GRPC_ENCODING).orElse(IDENTITY); - if (!IDENTITY.equals(encodingHeader)) { - throw new GrpcException(GrpcStatus.UNIMPLEMENTED); - } - - // The client may have sent a "grpc-accept-encoding" header. Note that - // "grpc-accept-encoding" is not well specified. I am following what I see work with - // the grpc.io client library, and the definition of "accept-encoding" for HTTP, such - // that, identity is *always* safe, but only compression algorithms supported by - // "grpc-accept-encoding" should be used if any compression algorithm will be used. - // - // To support this claim, the spec says: - // "A Compressed-Flag value of 1 indicates that the binary octet sequence of Message is - // compressed using the mechanism declared by the Message-Encoding header. A value of - // 0 indicates that no encoding of Message bytes has occurred. - // - // This seems to support the notion that compression can be enabled or disabled - // irrespective of the grpc-accept-encoding header. - - // FUTURE: If the client sends a "grpc-accept-encoding", and if we support one of them, - // then we should pick one and use it in the response. Otherwise, we should not have - // any compression. - - // If the grpc-timeout header is present, determine when that timeout would occur, or - // default to a future that is so far in the future it will never happen. - final var timeout = requestHeaders.value(GRPC_TIMEOUT); - - deadlineFuture = - timeout.map(this::scheduleDeadline).orElse(new NoopScheduledFuture<>()); - - // At this point, the request itself is valid. Maybe it will still fail to be handled by - // the service interface, but as far as the protocol is concerned, this was a valid - // request. Send the headers back to the client (the messages and trailers will be sent - // later). - sendResponseHeaders(GRPC_ENCODING_IDENTITY, requestContentType, emptyList()); - - // NOTE: The specification mentions the "Message-Type" header. Like everyone else, we're - // just going to ignore that header. See https://github.com/grpc/grpc/issues/12468. - - // FUTURE: Should we support custom metadata, we would extract it here and pass it along - // via "options". - // We should have a wrapper around them, such that we don't process the custom headers - // ourselves, but allow the service interface to look up special headers based on key. - - // Create the "options" to make available to the ServiceInterface. These options are - // used to decide on the best way to parse or handle the request. - final var options = - new Options( - Optional.ofNullable(headers.authority()), // the client (see http2 spec) - contentType.equals(APPLICATION_GRPC_PROTO), - contentType.equals(APPLICATION_GRPC_JSON), - contentType); - - // Setup the subscribers. The "outgoing" subscriber will send messages to the client. - // This is given to the "open" method on the service to allow it to send messages to - // the client. - final Pipeline outgoing = getOutgoing(); - pipeline = route.service().open(route.method(), options, outgoing); - } catch (final GrpcException grpcException) { - route.failedGrpcRequestCounter().increment(); - new TrailerOnlyBuilder() - .grpcStatus(grpcException.status()) - .statusMessage(grpcException.getMessage()) - .send(); - error(); - } catch (final HttpException httpException) { - route.failedHttpRequestCounter().increment(); - new TrailerOnlyBuilder() - .httpStatus(httpException.status()) - .grpcStatus(GrpcStatus.INVALID_ARGUMENT) - .send(); - error(); - } catch (final Exception unknown) { - route.failedUnknownRequestCounter().increment(); - LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); - new TrailerOnlyBuilder().grpcStatus(GrpcStatus.UNKNOWN).send(); - error(); - } - } - - // Visible for testing - protected Pipeline getOutgoing() { - return new SendToClientSubscriber(); } @Override @NonNull public Http2StreamState streamState() { - return currentStreamState.get(); + return grpcDataProcessor.getCurrentStreamState(); } @Override @@ -347,408 +118,6 @@ public void data(@NonNull final Http2FrameHeader header, @NonNull final BufferDa Objects.requireNonNull(header); Objects.requireNonNull(data); - processData(header, data); - } - - protected void processData(@NonNull final Http2FrameHeader header, @NonNull final BufferData data) { - - try { - // NOTE: if the deadline is exceeded, then the stream will be closed and data will no - // longer flow. - // There is some asynchronous behavior here, but in the worst case, we handle a few more - // bytes before the stream is closed. - while (data.available() > 0) { - // First chunk of data contains the compression flag and the length of the message - if (entityBytes == null) { - // Read whether this message is compressed. We do not currently support - // compression. - final var isCompressed = (data.read() == 1); - if (isCompressed) { - // The error will eventually result in the stream being closed - throw new GrpcException( - GrpcStatus.UNIMPLEMENTED, "Compression is not supported"); - } - // Read the length of the message. As per the grpc protocol specification, each - // message on the wire is prefixed with the number of bytes for the message. - // However, to prevent a DOS attack where the attacker sends us a very large - // length and exhausts our memory, we have a maximum message size configuration - // setting. Using that, we can detect attempts to exhaust our memory. - final long length = data.readUnsignedInt32(); - if (length > config.maxMessageSizeBytes()) { - throw new GrpcException( - GrpcStatus.INVALID_ARGUMENT, - "Message size exceeds maximum allowed size"); - } - // Create a buffer to hold the message. We sadly cannot reuse this buffer - // because once we have filled it and wrapped it in Bytes and sent it to the - // handler, some user code may grab and hold that Bytes object for an arbitrary - // amount of time, and if we were to scribble into the same byte array, we - // would break the application. So we need a new buffer each time :-( - entityBytes = new byte[(int) length]; - entityBytesIndex = 0; - } - - // By the time we get here, entityBytes is no longer null. It may be empty, or it - // may already have been partially populated from a previous iteration. It may be - // that the number of bytes available to be read is larger than just this one - // message. So we need to be careful to read, from what is available, only up to - // the message length, and to leave the rest for the next iteration. - final int available = data.available(); - final int numBytesToRead = - Math.min(entityBytes.length - entityBytesIndex, available); - data.read(entityBytes, entityBytesIndex, numBytesToRead); - entityBytesIndex += numBytesToRead; - - // If we have completed reading the message, then we can proceed. - if (entityBytesIndex == entityBytes.length) { - // Grab and wrap the bytes and reset to being reading the next message - final var bytes = Bytes.wrap(entityBytes); - pipeline.onNext(bytes); - entityBytesIndex = 0; - entityBytes = null; - } - } - - // The end of the stream has been reached! It is possible that a bad client will send - // end of stream before all the message data we sent. In that case, it is as if the - // message were never sent. - if (header.flags(Http2FrameTypes.DATA).endOfStream()) { - entityBytesIndex = 0; - entityBytes = null; - currentStreamState.set(Http2StreamState.HALF_CLOSED_REMOTE); - pipeline.clientEndStreamReceived(); - } - } catch (final Exception e) { - // I have to propagate this error through the service interface, so it can respond to - // errors in the connection, tear down resources, etc. It will also forward this on - // to the client, causing the connection to be torn down. - pipeline.onError(e); - } - } - - /** - * An error has occurred. Cancel the deadline future if it's still active, and set the stream - * state accordingly. - * - *

May be called by different threads concurrently. - */ - private void error() { - // Canceling a future that has already completed has no effect. So by canceling here, we are - // saying: - // "If you have not yet executed, never execute. If you have already executed, then just - // ignore me". - // The "isCancelled" flag is set if the future was canceled before it was executed. - - // cancel is threadsafe - deadlineFuture.cancel(false); - currentStreamState.set(Http2StreamState.CLOSED); - } - - /** - * Helper function. Given a string of digits followed by a unit, schedule a callback to be - * invoked when the deadline is exceeded, and return the associated future. The proper format of - * the string is defined in the specification as: - * - * 

-     *      Timeout → "grpc-timeout" TimeoutValue TimeoutUnit
-     *      TimeoutValue → {positive integer as ASCII string of at most 8 digits}
-     *      TimeoutUnit → Hour / Minute / Second / Millisecond / Microsecond / Nanosecond
-     *             Hour → "H"
-     *             Minute → "M"
-     *             Second → "S"
-     *             Millisecond → "m"
-     *             Microsecond → "u"
-     *             Nanosecond → "n"
-     *
- * - *

Illegal values result in the deadline being ignored. - * - * @param timeout The timeout value. Cannot be null. - * @return The future representing the task that will be executed if/when the deadline is - * reached. - */ - @NonNull - private ScheduledFuture scheduleDeadline(@NonNull final String timeout) { - final var matcher = GRPC_TIMEOUT_PATTERN.matcher(timeout); - if (matcher.matches()) { - final var num = Integer.parseInt(matcher.group(1)); - final var unit = matcher.group(2); - final var deadline = - System.nanoTime() - * TimeUnit.NANOSECONDS.convert( - num, - switch (unit) { - case "H" -> TimeUnit.HOURS; - case "M" -> TimeUnit.MINUTES; - case "S" -> TimeUnit.SECONDS; - case "m" -> TimeUnit.MILLISECONDS; - case "u" -> TimeUnit.MICROSECONDS; - case "n" -> TimeUnit.NANOSECONDS; - // This should NEVER be reachable, because the matcher - // would not have matched. - default -> throw new GrpcException( - GrpcStatus.INTERNAL, "Invalid unit: " + unit); - }); - return deadlineDetector.scheduleDeadline( - deadline, - () -> { - route.deadlineExceededCounter().increment(); - pipeline.onError(new GrpcException(GrpcStatus.DEADLINE_EXCEEDED)); - }); - } - - return new NoopScheduledFuture<>(); - } - - /** - * According to the specification, the "Response-Headers" are: - * - * 

HTTP-Status [Message-Encoding] [Message-Accept-Encoding] Content-Type *Custom-Metadata
-     *
- * - *

Where "Status" is always 200 OK. - * - *

The Response-Headers are normally sent, followed by the data, followed by trailers. But if - * the request fails right away, before any handling, then it is possible to send Trailers-Only - * instead. - */ - private void sendResponseHeaders( - @Nullable final Header messageEncoding, - @NonNull final HttpMediaType contentType, - @NonNull final List

customMetadata) { - - // Some headers are http2 specific, the rest are used for the grpc protocol - final var grpcHeaders = WritableHeaders.create(); - // FUTURE: I think to support custom headers in the response, we would have to list them - // here. - // Since this has to be sent before we have any data to send, we must know ahead of time - // which custom headers are to be returned. - grpcHeaders.set(HeaderNames.TRAILER, "grpc-status, grpc-message"); - grpcHeaders.set(Http2Headers.STATUS_NAME, Status.OK_200.code()); - grpcHeaders.contentType(contentType); - grpcHeaders.set(GRPC_ACCEPT_ENCODING, IDENTITY); - customMetadata.forEach(grpcHeaders::set); - if (messageEncoding != null) { - grpcHeaders.set(messageEncoding); - } - - final var http2Headers = Http2Headers.create(grpcHeaders); - - streamWriter.writeHeaders( - http2Headers, - streamId, - Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS), - flowControl.outbound()); - } - - /** - * A convenience class for building the trailers. In the specification, it says: - * - * 
-     *     Trailers → Status [Status-Message] *Custom-Metadata
-     *     Status → "grpc-status" 1*DIGIT ; 0-9
-     *     Status-Message → "grpc-message" Percent-Encoded
-     *     Percent-Encoded → 1*(Percent-Byte-Unencoded / Percent-Byte-Encoded)
-     *     Percent-Byte-Unencoded → 1*( %x20-%x24 / %x26-%x7E ) ; space and VCHAR, except %
-     *     Percent-Byte-Encoded → "%" 2HEXDIGIT ; 0-9 A-F
-     *
- */ - private class TrailerBuilder { - @NonNull private GrpcStatus grpcStatus = GrpcStatus.OK; - @Nullable private String statusMessage; - @NonNull private final List
customMetadata = emptyList(); // Never set - - /** - * Sets the gRPC status to return. Normally, the HTTP status will always be 200, while the - * gRPC status can be anything. - */ - @NonNull - public TrailerBuilder grpcStatus(@NonNull final GrpcStatus grpcStatus) { - this.grpcStatus = grpcStatus; - return this; - } - - /** Optionally, set the status message. May be null. */ - @NonNull - public TrailerBuilder statusMessage(@Nullable final String statusMessage) { - this.statusMessage = statusMessage; - return this; - } - - /** Send the headers to the client */ - public final void send() { - final var httpHeaders = WritableHeaders.create(); - final var http2Headers = Http2Headers.create(httpHeaders); - send(httpHeaders, http2Headers); - } - - /** - * Actually sends the headers. This method exists so that "trailers-only" can call it to - * send the normal headers. - */ - protected void send( - @NonNull final WritableHeaders httpHeaders, - @NonNull final Http2Headers http2Headers) { - httpHeaders.set(requireNonNull(GrpcHeaders.header(requireNonNull(grpcStatus)))); - httpHeaders.set(GRPC_ACCEPT_ENCODING, IDENTITY); - customMetadata.forEach(httpHeaders::set); - if (statusMessage != null) { - final var percentEncodedMessage = UriEncoding.encodeUri(statusMessage); - httpHeaders.set(GrpcHeaders.GRPC_MESSAGE, percentEncodedMessage); - } - - streamWriter.writeHeaders( - http2Headers, - streamId, - Http2Flag.HeaderFlags.create( - Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM), - flowControl.outbound()); - } - } - - /** - * A convenience class for building the trailers in the event of a catastrophic error before any - * headers could be sent to the client in response. In the specification, it says: - * - * 
-     *     Response-Headers & Trailers-Only are each delivered in a single HTTP2 HEADERS frame block.
-     *     Most responses are expected to have both headers and trailers but Trailers-Only is permitted
-     *     for calls that produce an immediate error. Status must be sent in Trailers even if the status
-     *     code is OK.
-     *
- * - * It extends {@link TrailerBuilder} and delegates to its parent to send common headers. - */ - private class TrailerOnlyBuilder extends TrailerBuilder { - private Status httpStatus = Status.OK_200; - private final HttpMediaType contentType = APPLICATION_GRPC_PROTO_TYPE; - - /** The HTTP Status to return in these trailers. The status will default to 200 OK. */ - @NonNull - public TrailerOnlyBuilder httpStatus(@Nullable final Status httpStatus) { - this.httpStatus = httpStatus; - return this; - } - - /** - * Send the headers back to the client - * - * @param httpHeaders The normal HTTP headers (also grpc headers) - * @param http2Headers The HTTP2 pseudo-headers - */ - @Override - protected void send( - @NonNull final WritableHeaders httpHeaders, - @NonNull final Http2Headers http2Headers) { - http2Headers.status(httpStatus); - httpHeaders.contentType(requireNonNull(contentType)); - super.send(httpHeaders, http2Headers); - } - } - - /** - * The implementation of {@link Pipeline} used to send messages to the client. It - * receives bytes from the handlers to send to the client. - */ - private final class SendToClientSubscriber implements Pipeline { - - private Runnable onErrorHandler; - - @Override - public void onSubscribe(@NonNull final Flow.Subscription subscription) { - // FUTURE: Add support for flow control - subscription.request(Long.MAX_VALUE); - } - - @Override - public void onNext(@NonNull final Bytes response) { - try { - final int length = (int) response.length(); - final var bufferData = BufferData.create(5 + length); - bufferData.write(0); // 0 means no compression - bufferData.writeUnsignedInt32(length); - bufferData.write(response.toByteArray()); - final var header = - Http2FrameHeader.create( - bufferData.available(), - Http2FrameTypes.DATA, - Http2Flag.DataFlags.create(0), - streamId); - - streamWriter.writeData( - new Http2FrameData(header, bufferData), flowControl.outbound()); - } catch (final Exception e) { - LOGGER.log(ERROR, "Failed to respond to grpc request: " + route.method(), e); - pipeline.onError(e); - } - } - - @Override - public void onError(@NonNull final Throwable throwable) { - if (onErrorHandler != null) { - // Invoke the handlers registered by - // the application code integrated - // with the PBJ Helidon Plugin. - onErrorHandler.run(); - } - - if (throwable instanceof final GrpcException grpcException) { - new TrailerBuilder() - .grpcStatus(grpcException.status()) - .statusMessage(grpcException.getMessage()) - .send(); - } else { - LOGGER.log(ERROR, "Failed to send response", throwable); - new TrailerBuilder().grpcStatus(GrpcStatus.INTERNAL).send(); - } - error(); - } - - @Override - public void onComplete() { - new TrailerBuilder().send(); - - deadlineFuture.cancel(false); - - currentStreamState.getAndUpdate( - currentValue -> { - if (requireNonNull(currentValue) == Http2StreamState.OPEN) { - return Http2StreamState.HALF_CLOSED_LOCAL; - } - return Http2StreamState.CLOSED; - }); - } - - @Override - public void registerOnErrorHandler(@NonNull final Runnable handler) { - this.onErrorHandler = requireNonNull(handler); - } - } - - /** Simple implementation of the {@link ServiceInterface.RequestOptions} interface. */ - protected record Options( - Optional authority, boolean isProtobuf, boolean isJson, String contentType) - implements ServiceInterface.RequestOptions {} - - /** - * A {@link ScheduledFuture} that does nothing. This is used when there is no deadline set for - * the request. A new instance of this must be created (or we need a "reset" method) for each - * {@link PbjProtocolHandler} instance, because it can become "corrupted" if canceled from any - * particular call. - */ - private static final class NoopScheduledFuture extends CompletableFuture - implements ScheduledFuture { - @Override - public long getDelay(@NonNull final TimeUnit unit) { - return 0; - } - - @Override - public int compareTo(@NonNull final Delayed o) { - // Since all NoopScheduledFuture instances have "0" as the delay, any other Delayed - // instance with a non-0 - // delay will come after this one. - return (int) (o.getDelay(TimeUnit.NANOSECONDS)); - } + grpcDataProcessor.data(header, data); } } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java index bc076381..cdfa64cd 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java @@ -19,21 +19,15 @@ import static java.util.Objects.requireNonNull; import com.hedera.pbj.grpc.helidon.config.PbjConfig; +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.io.buffer.Bytes; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.common.buffers.BufferData; import io.helidon.http.HttpPrologue; import io.helidon.http.Method; -import io.helidon.http.Status; -import io.helidon.http.WritableHeaders; -import io.helidon.http.http2.FlowControl; -import io.helidon.http.http2.Http2Flag; -import io.helidon.http.http2.Http2FrameHeader; import io.helidon.http.http2.Http2Headers; -import io.helidon.http.http2.Http2RstStream; import io.helidon.http.http2.Http2Settings; import io.helidon.http.http2.Http2StreamState; import io.helidon.http.http2.Http2StreamWriter; -import io.helidon.http.http2.Http2WindowUpdate; import io.helidon.http.http2.StreamFlowControl; import io.helidon.metrics.api.Counter; import io.helidon.metrics.api.Metrics; @@ -133,17 +127,30 @@ public SubProtocolResult subProtocol( true, new RouteNotFoundHandler(streamWriter, streamId, currentStreamState)); } + final HeadersProcessor headersProcessor = new HeadersProcessor( + headers, streamWriter, streamId, flowControl, route, deadlineDetector); + final var grpcDataProcessor = new GrpcDataProcessorImpl(config, currentStreamState); + + final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( + streamWriter, streamId, flowControl, route, grpcDataProcessor, headersProcessor); + final PipelineBuilder pipelineBuilder = new PipelineBuilder(route, headersProcessor.options(), sendToClientSubscriber.subscriber()); + final Pipeline pipeline = pipelineBuilder.createPipeline(); + + grpcDataProcessor.setPipeline(pipeline); + sendToClientSubscriber.setPipeline(pipeline); + headersProcessor.setPipeline(pipeline); + + // This is a valid call! return new SubProtocolResult( true, new PbjProtocolHandler( - headers, streamWriter, streamId, flowControl, - currentStreamState, - config, route, - deadlineDetector)); + grpcDataProcessor, + headersProcessor, + pipeline)); } } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java new file mode 100644 index 00000000..50a85e4d --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java @@ -0,0 +1,31 @@ +package com.hedera.pbj.grpc.helidon; + +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.grpc.ServiceInterface; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import edu.umd.cs.findbugs.annotations.NonNull; + +import static java.util.Objects.requireNonNull; + +public class PipelineBuilder { + + private final PbjMethodRoute route; + private final ServiceInterface.RequestOptions options; + private final Pipeline outgoing; + + PipelineBuilder( + @NonNull final PbjMethodRoute route, + @NonNull final ServiceInterface.RequestOptions options, + @NonNull final Pipeline outgoing) { + this.route = requireNonNull(route); + this.options = requireNonNull(options); + this.outgoing = requireNonNull(outgoing); + } + + public Pipeline createPipeline() { + // Setup the subscribers. The "outgoing" subscriber will send messages to the client. + // This is given to the "open" method on the service to allow it to send messages to + // the client. + return route.service().open(route.method(), options, outgoing); + } +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java new file mode 100644 index 00000000..043d2589 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java @@ -0,0 +1,132 @@ +package com.hedera.pbj.grpc.helidon; + +import com.hedera.pbj.runtime.grpc.GrpcException; +import com.hedera.pbj.runtime.grpc.GrpcStatus; +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import edu.umd.cs.findbugs.annotations.NonNull; +import io.helidon.common.buffers.BufferData; +import io.helidon.http.http2.Http2Flag; +import io.helidon.http.http2.Http2FrameData; +import io.helidon.http.http2.Http2FrameHeader; +import io.helidon.http.http2.Http2FrameTypes; +import io.helidon.http.http2.Http2StreamState; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; + +import java.util.concurrent.Flow; + +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Objects.requireNonNull; + +/** + * The implementation of {@link Pipeline} used to send messages to the client. It + * receives bytes from the handlers to send to the client. + */ +final class SendToClientSubscriber implements Pipeline { + + private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); + + private Runnable onErrorHandler; + private final Http2StreamWriter streamWriter; + private final int streamId; + private final StreamFlowControl flowControl; + private final PbjMethodRoute route; + private final GrpcDataProcessor grpcDataProcessor; + private final HeadersProcessor headersProcessor; + private Pipeline pipeline; + + SendToClientSubscriber( + @NonNull final Http2StreamWriter streamWriter, + final int streamId, + @NonNull final StreamFlowControl flowControl, + @NonNull final PbjMethodRoute route, + @NonNull final GrpcDataProcessor grpcDataProcessor, + @NonNull final HeadersProcessor headersProcessor) { + this.streamWriter = requireNonNull(streamWriter); + this.streamId = streamId; + this.flowControl = requireNonNull(flowControl); + this.route = requireNonNull(route); + this.grpcDataProcessor = requireNonNull(grpcDataProcessor); + this.headersProcessor = requireNonNull(headersProcessor); + } + + public void setPipeline(@NonNull final Pipeline pipeline) { + this.pipeline = requireNonNull(pipeline); + } + + public Pipeline subscriber() { + return this; + } + + @Override + public void onSubscribe(@NonNull final Flow.Subscription subscription) { + // FUTURE: Add support for flow control + subscription.request(Long.MAX_VALUE); + } + + @Override + public void onNext(@NonNull final Bytes response) { + try { + final int length = (int) response.length(); + final var bufferData = BufferData.create(5 + length); + bufferData.write(0); // 0 means no compression + bufferData.writeUnsignedInt32(length); + bufferData.write(response.toByteArray()); + final var header = + Http2FrameHeader.create( + bufferData.available(), + Http2FrameTypes.DATA, + Http2Flag.DataFlags.create(0), + streamId); + + streamWriter.writeData( + new Http2FrameData(header, bufferData), flowControl.outbound()); + } catch (final Exception e) { + LOGGER.log(ERROR, "Failed to respond to grpc request: " + route.method(), e); + pipeline.onError(e); + } + } + + @Override + public void onError(@NonNull final Throwable throwable) { + if (onErrorHandler != null) { + // Invoke the handlers registered by + // the application code integrated + // with the PBJ Helidon Plugin. + onErrorHandler.run(); + } + + if (throwable instanceof final GrpcException grpcException) { + new TrailerBuilder(streamWriter, streamId, flowControl) + .grpcStatus(grpcException.status()) + .statusMessage(grpcException.getMessage()) + .send(); + } else { + LOGGER.log(ERROR, "Failed to send response", throwable); + new TrailerBuilder(streamWriter, streamId, flowControl) + .grpcStatus(GrpcStatus.INTERNAL).send(); + } +// error(); + } + + @Override + public void onComplete() { + new TrailerBuilder(streamWriter, streamId, flowControl) + .send(); + + headersProcessor.cancelDeadlineFuture(false); + + grpcDataProcessor.setCurrentStreamState(currentValue -> { + if (requireNonNull(currentValue) == Http2StreamState.OPEN) { + return Http2StreamState.HALF_CLOSED_LOCAL; + } + return Http2StreamState.CLOSED; + }); + } + + @Override + public void registerOnErrorHandler(@NonNull final Runnable handler) { + this.onErrorHandler = requireNonNull(handler); + } +} \ No newline at end of file diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java new file mode 100644 index 00000000..0cb39ada --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java @@ -0,0 +1,97 @@ +package com.hedera.pbj.grpc.helidon; + +import com.hedera.pbj.runtime.grpc.GrpcStatus; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import io.helidon.common.uri.UriEncoding; +import io.helidon.http.Header; +import io.helidon.http.WritableHeaders; +import io.helidon.http.http2.Http2Flag; +import io.helidon.http.http2.Http2Headers; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; + +import java.util.List; + +import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + +/** + * A convenience class for building the trailers. In the specification, it says: + * + * 
+ *     Trailers → Status [Status-Message] *Custom-Metadata
+ *     Status → "grpc-status" 1*DIGIT ; 0-9
+ *     Status-Message → "grpc-message" Percent-Encoded
+ *     Percent-Encoded → 1*(Percent-Byte-Unencoded / Percent-Byte-Encoded)
+ *     Percent-Byte-Unencoded → 1*( %x20-%x24 / %x26-%x7E ) ; space and VCHAR, except %
+ *     Percent-Byte-Encoded → "%" 2HEXDIGIT ; 0-9 A-F
+ *
+ */ +class TrailerBuilder { + @NonNull private GrpcStatus grpcStatus = GrpcStatus.OK; + @Nullable private String statusMessage; + @NonNull private final List
customMetadata = emptyList(); // Never set + + private final Http2StreamWriter streamWriter; + private final StreamFlowControl flowControl; + final int streamId; + + TrailerBuilder( + @NonNull final Http2StreamWriter streamWriter, + final int streamId, + @NonNull final StreamFlowControl flowControl) { + this.streamWriter = requireNonNull(streamWriter); + this.streamId = streamId; + this.flowControl = flowControl; + } + + /** + * Sets the gRPC status to return. Normally, the HTTP status will always be 200, while the + * gRPC status can be anything. + */ + @NonNull + public TrailerBuilder grpcStatus(@NonNull final GrpcStatus grpcStatus) { + this.grpcStatus = grpcStatus; + return this; + } + + /** Optionally, set the status message. May be null. */ + @NonNull + public TrailerBuilder statusMessage(@Nullable final String statusMessage) { + this.statusMessage = statusMessage; + return this; + } + + /** Send the headers to the client */ + public final void send() { + final var httpHeaders = WritableHeaders.create(); + final var http2Headers = Http2Headers.create(httpHeaders); + send(httpHeaders, http2Headers); + } + + /** + * Actually sends the headers. This method exists so that "trailers-only" can call it to + * send the normal headers. + */ + protected void send( + @NonNull final WritableHeaders httpHeaders, + @NonNull final Http2Headers http2Headers) { + httpHeaders.set(requireNonNull(GrpcHeaders.header(requireNonNull(grpcStatus)))); + httpHeaders.set(GRPC_ACCEPT_ENCODING, IDENTITY); + customMetadata.forEach(httpHeaders::set); + if (statusMessage != null) { + final var percentEncodedMessage = UriEncoding.encodeUri(statusMessage); + httpHeaders.set(GrpcHeaders.GRPC_MESSAGE, percentEncodedMessage); + } + + streamWriter.writeHeaders( + http2Headers, + streamId, + Http2Flag.HeaderFlags.create( + Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM), + flowControl.outbound()); + } +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java new file mode 100644 index 00000000..f033b2d7 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java @@ -0,0 +1,60 @@ +package com.hedera.pbj.grpc.helidon; + +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import io.helidon.http.HttpMediaType; +import io.helidon.http.Status; +import io.helidon.http.WritableHeaders; +import io.helidon.http.http2.Http2Headers; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; + +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.APPLICATION_GRPC_PROTO_TYPE; +import static java.util.Objects.requireNonNull; + +/** + * A convenience class for building the trailers in the event of a catastrophic error before any + * headers could be sent to the client in response. In the specification, it says: + * + * 
+ *     Response-Headers & Trailers-Only are each delivered in a single HTTP2 HEADERS frame block.
+ *     Most responses are expected to have both headers and trailers but Trailers-Only is permitted
+ *     for calls that produce an immediate error. Status must be sent in Trailers even if the status
+ *     code is OK.
+ *
+ * + * It extends {@link TrailerBuilder} and delegates to its parent to send common headers. + */ +class TrailerOnlyBuilder extends TrailerBuilder { + private Status httpStatus = Status.OK_200; + private final HttpMediaType contentType = APPLICATION_GRPC_PROTO_TYPE; + + TrailerOnlyBuilder( + @NonNull final Http2StreamWriter streamWriter, + final int streamId, + @NonNull final StreamFlowControl flowControl) { + super(streamWriter, streamId, flowControl); + } + + /** The HTTP Status to return in these trailers. The status will default to 200 OK. */ + @NonNull + public TrailerOnlyBuilder httpStatus(@Nullable final Status httpStatus) { + this.httpStatus = httpStatus; + return this; + } + + /** + * Send the headers back to the client + * + * @param httpHeaders The normal HTTP headers (also grpc headers) + * @param http2Headers The HTTP2 pseudo-headers + */ + @Override + protected void send( + @NonNull final WritableHeaders httpHeaders, + @NonNull final Http2Headers http2Headers) { + http2Headers.status(httpStatus); + httpHeaders.contentType(requireNonNull(contentType)); + super.send(httpHeaders, http2Headers); + } +} diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index 843c050e..4ab5e9a7 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -44,6 +44,7 @@ import static com.hedera.pbj.grpc.helidon.PbjProtocolHandlerTest.TestGreeterService.TestGreeterMethod.sayHelloStreamReply; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class PbjProtocolHandlerTest { @@ -64,6 +65,9 @@ public class PbjProtocolHandlerTest { @Mock private BufferData bufferData; + + @Mock private Pipeline pipeline; + @Test public void testOnErrorHandlerCalledOnException() { @@ -71,20 +75,29 @@ public void testOnErrorHandlerCalledOnException() { // an Application defined method. To confirm the registered handler // gets called when there's an exception. final Pipeline subscriber = mock(Pipeline.class); - final PbjProtocolHandler testPbjProtocolHandler = - new TestPbjProtocolHandler( - http2Headers, - http2StreamWriter, - 1, - streamFlowControl, - http2StreamState, - pbjConfig, - pbjMethodRoute, - deadlineDetector, - subscriber); + final HeadersProcessor headersProcessor = new HeadersProcessor( + http2Headers, http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, deadlineDetector); + final var grpcDataProcessor = new GrpcDataProcessorImpl(pbjConfig, http2StreamState); + + final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( + http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, grpcDataProcessor, headersProcessor); + final PipelineBuilder pipelineBuilder = new PipelineBuilder(pbjMethodRoute, headersProcessor.options(), sendToClientSubscriber.subscriber()); + + grpcDataProcessor.setPipeline(pipeline); + sendToClientSubscriber.setPipeline(pipeline); + headersProcessor.setPipeline(pipeline); + + final PbjProtocolHandler testPbjProtocolHandler = new PbjProtocolHandler( + http2StreamWriter, + 1, + streamFlowControl, + pbjMethodRoute, + grpcDataProcessor, + headersProcessor, + pipeline); doThrow(IllegalArgumentException.class).when(bufferData).available(); - testPbjProtocolHandler.processData(null, bufferData); + testPbjProtocolHandler.data(null, bufferData); } static class TestGreeterService implements ServiceInterface { @@ -194,31 +207,4 @@ Pipeline sayHelloStreamBidi( return null; } } - - // Subclass PbjProtocolHandler to expose the pipeline for testing - private static class TestPbjProtocolHandler extends PbjProtocolHandler { - - TestPbjProtocolHandler( - @NonNull Http2Headers headers, - @NonNull Http2StreamWriter streamWriter, - int streamId, - @NonNull StreamFlowControl flowControl, - @NonNull Http2StreamState currentStreamState, - @NonNull PbjConfig config, - @NonNull PbjMethodRoute route, - @NonNull DeadlineDetector deadlineDetector, - @NonNull Pipeline subscriber) { - super( - headers, - streamWriter, - streamId, - flowControl, - currentStreamState, - config, - route, - deadlineDetector); - - final TestGreeterService greeterService = new TestGreeterService(subscriber); - super.pipeline = greeterService.open(sayHelloStreamReply, null, getOutgoing()); } - } } From 42c7d3a93d113d67ccff4a79c05e7a8b849a3711 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Tue, 12 Nov 2024 15:00:52 -0700 Subject: [PATCH 08/11] wip: second step of refactor Signed-off-by: Matt Peterson --- .../pbj/grpc/helidon/HeadersProcessor.java | 352 +---------------- .../grpc/helidon/HeadersProcessorImpl.java | 360 ++++++++++++++++++ .../pbj/grpc/helidon/PbjProtocolHandler.java | 17 - .../pbj/grpc/helidon/PbjProtocolSelector.java | 8 +- .../grpc/helidon/SendToClientSubscriber.java | 4 +- .../grpc/helidon/PbjProtocolHandlerTest.java | 92 +++-- 6 files changed, 426 insertions(+), 407 deletions(-) create mode 100644 pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java index 2f41cdaa..8a57d120 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java @@ -1,356 +1,12 @@ package com.hedera.pbj.grpc.helidon; -import com.hedera.pbj.runtime.grpc.GrpcException; -import com.hedera.pbj.runtime.grpc.GrpcStatus; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.ServiceInterface; import com.hedera.pbj.runtime.io.buffer.Bytes; import edu.umd.cs.findbugs.annotations.NonNull; -import edu.umd.cs.findbugs.annotations.Nullable; -import io.helidon.http.Header; -import io.helidon.http.HeaderNames; -import io.helidon.http.HttpException; -import io.helidon.http.HttpMediaType; -import io.helidon.http.HttpMediaTypes; -import io.helidon.http.Status; -import io.helidon.http.WritableHeaders; -import io.helidon.http.http2.Http2Flag; -import io.helidon.http.http2.Http2Headers; -import io.helidon.http.http2.Http2StreamState; -import io.helidon.http.http2.Http2StreamWriter; -import io.helidon.http.http2.StreamFlowControl; -import java.util.List; -import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Delayed; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; -import java.util.regex.Pattern; - -import static com.hedera.pbj.grpc.helidon.Constants.GRPC_ENCODING_IDENTITY; -import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ENCODING; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_TIMEOUT; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_JSON; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_PROTO; -import static java.lang.System.Logger.Level.ERROR; -import static java.util.Collections.emptyList; -import static java.util.Objects.requireNonNull; - -class HeadersProcessor { - private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); - - /** The regular expression used to parse the grpc-timeout header. */ - private static final String GRPC_TIMEOUT_REGEX = "(\\d{1,8})([HMSmun])"; - private static final Pattern GRPC_TIMEOUT_PATTERN = Pattern.compile(GRPC_TIMEOUT_REGEX); - - /** - * A future representing the background task detecting deadlines. If there is a deadline, then - * this future will represent the task that will be executed when the deadline is reached. If - * there is no deadline, then we default to a non-null no-op future that exists in the infinite - * future. - * - *

Method calls on this object are thread-safe. - */ - private ScheduledFuture deadlineFuture; - - /** - * If there is a timeout defined for the request, then this detector is used to determine when - * the timeout deadline has been met. The detector runs on a background thread/timer. - */ - private final DeadlineDetector deadlineDetector; - - /** - * The service method that this connection was created for. The route has information about the - * {@link ServiceInterface} and method to invoke, as well as metrics, and other information. - */ - private final PbjMethodRoute route; - private final Http2StreamWriter streamWriter; - private final StreamFlowControl flowControl; - private final int streamId; - private ServiceInterface.RequestOptions options; - private Pipeline pipeline; - - HeadersProcessor( - @NonNull final Http2Headers headers, - @NonNull final Http2StreamWriter streamWriter, - final int streamId, - @NonNull final StreamFlowControl flowControl, - @NonNull final PbjMethodRoute route, - @NonNull final DeadlineDetector deadlineDetector) { - - this.streamWriter = streamWriter; - this.streamId = streamId; - this.flowControl = flowControl; - this.route = route; - this.deadlineDetector = deadlineDetector; - - try { - // If Content-Type does not begin with "application/grpc", gRPC servers SHOULD respond - // with HTTP status of 415 (Unsupported Media Type). This will prevent other HTTP/2 - // clients from interpreting a gRPC error response, which uses status 200 (OK), as - // successful. - // See https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md - // In addition, "application/grpc" is interpreted as "application/grpc+proto". - final var requestHeaders = headers.httpHeaders(); - final var requestContentType = - requestHeaders.contentType().orElse(HttpMediaTypes.PLAINTEXT_UTF_8); - final var ct = requestContentType.text(); - final var contentType = - switch (ct) { - case APPLICATION_GRPC, APPLICATION_GRPC_PROTO -> APPLICATION_GRPC_PROTO; - case APPLICATION_GRPC_JSON -> APPLICATION_GRPC_JSON; - default -> { - if (ct.startsWith(APPLICATION_GRPC)) { - yield ct; - } - throw new HttpException( - "Unsupported", Status.UNSUPPORTED_MEDIA_TYPE_415); - } - }; - - // This implementation currently only supports "identity" compression. - // - // As per the documentation: - // If a client message is compressed by an algorithm that is not supported by a server, - // the message will result in an UNIMPLEMENTED error status on the server. The server - // will include a grpc-accept-encoding header [in] the response which specifies the - // algorithms that the server accepts. - // - // Note that in the HeadersBuilder we ALWAYS set the grpc-accept-encoding header. - // FUTURE: Add support for the other compression schemes and let the response be in the - // same scheme that was sent to us, or another scheme in "grpc-accept-encoding" that - // we support, or identity. - final var encodingHeader = requestHeaders.value(GRPC_ENCODING).orElse(IDENTITY); - if (!IDENTITY.equals(encodingHeader)) { - throw new GrpcException(GrpcStatus.UNIMPLEMENTED); - } - - // The client may have sent a "grpc-accept-encoding" header. Note that - // "grpc-accept-encoding" is not well specified. I am following what I see work with - // the grpc.io client library, and the definition of "accept-encoding" for HTTP, such - // that, identity is *always* safe, but only compression algorithms supported by - // "grpc-accept-encoding" should be used if any compression algorithm will be used. - // - // To support this claim, the spec says: - // "A Compressed-Flag value of 1 indicates that the binary octet sequence of Message is - // compressed using the mechanism declared by the Message-Encoding header. A value of - // 0 indicates that no encoding of Message bytes has occurred. - // - // This seems to support the notion that compression can be enabled or disabled - // irrespective of the grpc-accept-encoding header. - - // FUTURE: If the client sends a "grpc-accept-encoding", and if we support one of them, - // then we should pick one and use it in the response. Otherwise, we should not have - // any compression. - - // If the grpc-timeout header is present, determine when that timeout would occur, or - // default to a future that is so far in the future it will never happen. - final var timeout = requestHeaders.value(GRPC_TIMEOUT); - - deadlineFuture = - timeout.map(this::scheduleDeadline).orElse(new NoopScheduledFuture<>()); - - // At this point, the request itself is valid. Maybe it will still fail to be handled by - // the service interface, but as far as the protocol is concerned, this was a valid - // request. Send the headers back to the client (the messages and trailers will be sent - // later). - sendResponseHeaders(GRPC_ENCODING_IDENTITY, requestContentType, emptyList()); - - // NOTE: The specification mentions the "Message-Type" header. Like everyone else, we're - // just going to ignore that header. See https://github.com/grpc/grpc/issues/12468. - - // FUTURE: Should we support custom metadata, we would extract it here and pass it along - // via "options". - // We should have a wrapper around them, such that we don't process the custom headers - // ourselves, but allow the service interface to look up special headers based on key. - - // Create the "options" to make available to the ServiceInterface. These options are - // used to decide on the best way to parse or handle the request. - options = - new Options( - Optional.ofNullable(headers.authority()), // the client (see http2 spec) - contentType.equals(APPLICATION_GRPC_PROTO), - contentType.equals(APPLICATION_GRPC_JSON), - contentType); - - } catch (final GrpcException grpcException) { - route.failedGrpcRequestCounter().increment(); - new TrailerOnlyBuilder(streamWriter, streamId, flowControl) - .grpcStatus(grpcException.status()) - .statusMessage(grpcException.getMessage()) - .send(); -// error(); - } catch (final HttpException httpException) { - route.failedHttpRequestCounter().increment(); - new TrailerOnlyBuilder(streamWriter, streamId, flowControl) - .httpStatus(httpException.status()) - .grpcStatus(GrpcStatus.INVALID_ARGUMENT) - .send(); -// error(); - } catch (final Exception unknown) { - route.failedUnknownRequestCounter().increment(); - LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); - new TrailerOnlyBuilder(streamWriter, streamId, flowControl) - .grpcStatus(GrpcStatus.UNKNOWN).send(); -// error(); - } - } - - public void setPipeline(@NonNull final Pipeline pipeline) { - this.pipeline = requireNonNull(pipeline); - } - - public void cancelDeadlineFuture(boolean isCancelled) { - deadlineFuture.cancel(isCancelled); - } - - public ServiceInterface.RequestOptions options() { - return options; - } - - /** - * According to the specification, the "Response-Headers" are: - * - * 

HTTP-Status [Message-Encoding] [Message-Accept-Encoding] Content-Type *Custom-Metadata
-     *
- * - *

Where "Status" is always 200 OK. - * - *

The Response-Headers are normally sent, followed by the data, followed by trailers. But if - * the request fails right away, before any handling, then it is possible to send Trailers-Only - * instead. - */ - private void sendResponseHeaders( - @Nullable final Header messageEncoding, - @NonNull final HttpMediaType contentType, - @NonNull final List

customMetadata) { - - // Some headers are http2 specific, the rest are used for the grpc protocol - final var grpcHeaders = WritableHeaders.create(); - // FUTURE: I think to support custom headers in the response, we would have to list them - // here. - // Since this has to be sent before we have any data to send, we must know ahead of time - // which custom headers are to be returned. - grpcHeaders.set(HeaderNames.TRAILER, "grpc-status, grpc-message"); - grpcHeaders.set(Http2Headers.STATUS_NAME, Status.OK_200.code()); - grpcHeaders.contentType(contentType); - grpcHeaders.set(GRPC_ACCEPT_ENCODING, IDENTITY); - customMetadata.forEach(grpcHeaders::set); - if (messageEncoding != null) { - grpcHeaders.set(messageEncoding); - } - - final var http2Headers = Http2Headers.create(grpcHeaders); - - streamWriter.writeHeaders( - http2Headers, - streamId, - Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS), - flowControl.outbound()); - } - - /** - * Helper function. Given a string of digits followed by a unit, schedule a callback to be - * invoked when the deadline is exceeded, and return the associated future. The proper format of - * the string is defined in the specification as: - * - * 
-     *      Timeout → "grpc-timeout" TimeoutValue TimeoutUnit
-     *      TimeoutValue → {positive integer as ASCII string of at most 8 digits}
-     *      TimeoutUnit → Hour / Minute / Second / Millisecond / Microsecond / Nanosecond
-     *             Hour → "H"
-     *             Minute → "M"
-     *             Second → "S"
-     *             Millisecond → "m"
-     *             Microsecond → "u"
-     *             Nanosecond → "n"
-     *
- * - *

Illegal values result in the deadline being ignored. - * - * @param timeout The timeout value. Cannot be null. - * @return The future representing the task that will be executed if/when the deadline is - * reached. - */ - @NonNull - private ScheduledFuture scheduleDeadline(@NonNull final String timeout) { - final var matcher = GRPC_TIMEOUT_PATTERN.matcher(timeout); - if (matcher.matches()) { - final var num = Integer.parseInt(matcher.group(1)); - final var unit = matcher.group(2); - final var deadline = - System.nanoTime() - * TimeUnit.NANOSECONDS.convert( - num, - switch (unit) { - case "H" -> TimeUnit.HOURS; - case "M" -> TimeUnit.MINUTES; - case "S" -> TimeUnit.SECONDS; - case "m" -> TimeUnit.MILLISECONDS; - case "u" -> TimeUnit.MICROSECONDS; - case "n" -> TimeUnit.NANOSECONDS; - // This should NEVER be reachable, because the matcher - // would not have matched. - default -> throw new GrpcException( - GrpcStatus.INTERNAL, "Invalid unit: " + unit); - }); - return deadlineDetector.scheduleDeadline( - deadline, - () -> { - route.deadlineExceededCounter().increment(); - pipeline.onError(new GrpcException(GrpcStatus.DEADLINE_EXCEEDED)); - }); - } - - return new NoopScheduledFuture<>(); - } - - /** - * A {@link ScheduledFuture} that does nothing. This is used when there is no deadline set for - * the request. A new instance of this must be created (or we need a "reset" method) for each - * {@link PbjProtocolHandler} instance, because it can become "corrupted" if canceled from any - * particular call. - */ - private static final class NoopScheduledFuture extends CompletableFuture - implements ScheduledFuture { - @Override - public long getDelay(@NonNull final TimeUnit unit) { - return 0; - } - - @Override - public int compareTo(@NonNull final Delayed o) { - // Since all NoopScheduledFuture instances have "0" as the delay, any other Delayed - // instance with a non-0 - // delay will come after this one. - return (int) (o.getDelay(TimeUnit.NANOSECONDS)); - } - } - - /** - * An error has occurred. Cancel the deadline future if it's still active, and set the stream - * state accordingly. - * - *

May be called by different threads concurrently. - */ - private void error() { - // Canceling a future that has already completed has no effect. So by canceling here, we are - // saying: - // "If you have not yet executed, never execute. If you have already executed, then just - // ignore me". - // The "isCancelled" flag is set if the future was canceled before it was executed. - - // cancel is threadsafe - cancelDeadlineFuture(false); - grpcDataProcessor.setCurrentStreamState(current -> Http2StreamState.CLOSED); - } - - /** Simple implementation of the {@link ServiceInterface.RequestOptions} interface. */ - protected record Options( - Optional authority, boolean isProtobuf, boolean isJson, String contentType) - implements ServiceInterface.RequestOptions {} +interface HeadersProcessor { + void setPipeline(@NonNull final Pipeline pipeline); + void cancelDeadlineFuture(boolean isCancelled); + ServiceInterface.RequestOptions options(); } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java new file mode 100644 index 00000000..b0cc3319 --- /dev/null +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java @@ -0,0 +1,360 @@ +package com.hedera.pbj.grpc.helidon; + +import com.hedera.pbj.runtime.grpc.GrpcException; +import com.hedera.pbj.runtime.grpc.GrpcStatus; +import com.hedera.pbj.runtime.grpc.Pipeline; +import com.hedera.pbj.runtime.grpc.ServiceInterface; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import io.helidon.http.Header; +import io.helidon.http.HeaderNames; +import io.helidon.http.HttpException; +import io.helidon.http.HttpMediaType; +import io.helidon.http.HttpMediaTypes; +import io.helidon.http.Status; +import io.helidon.http.WritableHeaders; +import io.helidon.http.http2.Http2Flag; +import io.helidon.http.http2.Http2Headers; +import io.helidon.http.http2.Http2StreamState; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; + +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Delayed; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; + +import static com.hedera.pbj.grpc.helidon.Constants.GRPC_ENCODING_IDENTITY; +import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ENCODING; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_TIMEOUT; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_JSON; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_PROTO; +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + +public class HeadersProcessorImpl implements HeadersProcessor { + private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); + + /** The regular expression used to parse the grpc-timeout header. */ + private static final String GRPC_TIMEOUT_REGEX = "(\\d{1,8})([HMSmun])"; + private static final Pattern GRPC_TIMEOUT_PATTERN = Pattern.compile(GRPC_TIMEOUT_REGEX); + + /** + * A future representing the background task detecting deadlines. If there is a deadline, then + * this future will represent the task that will be executed when the deadline is reached. If + * there is no deadline, then we default to a non-null no-op future that exists in the infinite + * future. + * + *

Method calls on this object are thread-safe. + */ + private ScheduledFuture deadlineFuture; + + /** + * If there is a timeout defined for the request, then this detector is used to determine when + * the timeout deadline has been met. The detector runs on a background thread/timer. + */ + private final DeadlineDetector deadlineDetector; + + /** + * The service method that this connection was created for. The route has information about the + * {@link ServiceInterface} and method to invoke, as well as metrics, and other information. + */ + private final PbjMethodRoute route; + private final Http2StreamWriter streamWriter; + private final StreamFlowControl flowControl; + private final int streamId; + private ServiceInterface.RequestOptions options; + private final GrpcDataProcessor grpcDataProcessor; + private Pipeline pipeline; + + + HeadersProcessorImpl( + @NonNull final Http2Headers headers, + @NonNull final Http2StreamWriter streamWriter, + final int streamId, + @NonNull final StreamFlowControl flowControl, + @NonNull final PbjMethodRoute route, + @NonNull final DeadlineDetector deadlineDetector, + @NonNull final GrpcDataProcessor grpcDataProcessor) { + + this.streamWriter = requireNonNull(streamWriter); + this.streamId = streamId; + this.flowControl = requireNonNull(flowControl); + this.route = requireNonNull(route); + this.deadlineDetector = requireNonNull(deadlineDetector); + this.grpcDataProcessor = requireNonNull(grpcDataProcessor); + + try { + // If Content-Type does not begin with "application/grpc", gRPC servers SHOULD respond + // with HTTP status of 415 (Unsupported Media Type). This will prevent other HTTP/2 + // clients from interpreting a gRPC error response, which uses status 200 (OK), as + // successful. + // See https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md + // In addition, "application/grpc" is interpreted as "application/grpc+proto". + final var requestHeaders = headers.httpHeaders(); + final var requestContentType = + requestHeaders.contentType().orElse(HttpMediaTypes.PLAINTEXT_UTF_8); + final var ct = requestContentType.text(); + final var contentType = + switch (ct) { + case APPLICATION_GRPC, APPLICATION_GRPC_PROTO -> APPLICATION_GRPC_PROTO; + case APPLICATION_GRPC_JSON -> APPLICATION_GRPC_JSON; + default -> { + if (ct.startsWith(APPLICATION_GRPC)) { + yield ct; + } + throw new HttpException( + "Unsupported", Status.UNSUPPORTED_MEDIA_TYPE_415); + } + }; + + // This implementation currently only supports "identity" compression. + // + // As per the documentation: + // If a client message is compressed by an algorithm that is not supported by a server, + // the message will result in an UNIMPLEMENTED error status on the server. The server + // will include a grpc-accept-encoding header [in] the response which specifies the + // algorithms that the server accepts. + // + // Note that in the HeadersBuilder we ALWAYS set the grpc-accept-encoding header. + // FUTURE: Add support for the other compression schemes and let the response be in the + // same scheme that was sent to us, or another scheme in "grpc-accept-encoding" that + // we support, or identity. + final var encodingHeader = requestHeaders.value(GRPC_ENCODING).orElse(IDENTITY); + if (!IDENTITY.equals(encodingHeader)) { + throw new GrpcException(GrpcStatus.UNIMPLEMENTED); + } + + // The client may have sent a "grpc-accept-encoding" header. Note that + // "grpc-accept-encoding" is not well specified. I am following what I see work with + // the grpc.io client library, and the definition of "accept-encoding" for HTTP, such + // that, identity is *always* safe, but only compression algorithms supported by + // "grpc-accept-encoding" should be used if any compression algorithm will be used. + // + // To support this claim, the spec says: + // "A Compressed-Flag value of 1 indicates that the binary octet sequence of Message is + // compressed using the mechanism declared by the Message-Encoding header. A value of + // 0 indicates that no encoding of Message bytes has occurred. + // + // This seems to support the notion that compression can be enabled or disabled + // irrespective of the grpc-accept-encoding header. + + // FUTURE: If the client sends a "grpc-accept-encoding", and if we support one of them, + // then we should pick one and use it in the response. Otherwise, we should not have + // any compression. + + // If the grpc-timeout header is present, determine when that timeout would occur, or + // default to a future that is so far in the future it will never happen. + final var timeout = requestHeaders.value(GRPC_TIMEOUT); + + deadlineFuture = + timeout.map(this::scheduleDeadline).orElse(new NoopScheduledFuture<>()); + + // At this point, the request itself is valid. Maybe it will still fail to be handled by + // the service interface, but as far as the protocol is concerned, this was a valid + // request. Send the headers back to the client (the messages and trailers will be sent + // later). + sendResponseHeaders(GRPC_ENCODING_IDENTITY, requestContentType, emptyList()); + + // NOTE: The specification mentions the "Message-Type" header. Like everyone else, we're + // just going to ignore that header. See https://github.com/grpc/grpc/issues/12468. + + // FUTURE: Should we support custom metadata, we would extract it here and pass it along + // via "options". + // We should have a wrapper around them, such that we don't process the custom headers + // ourselves, but allow the service interface to look up special headers based on key. + + // Create the "options" to make available to the ServiceInterface. These options are + // used to decide on the best way to parse or handle the request. + options = + new Options( + Optional.ofNullable(headers.authority()), // the client (see http2 spec) + contentType.equals(APPLICATION_GRPC_PROTO), + contentType.equals(APPLICATION_GRPC_JSON), + contentType); + + } catch (final GrpcException grpcException) { + route.failedGrpcRequestCounter().increment(); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .grpcStatus(grpcException.status()) + .statusMessage(grpcException.getMessage()) + .send(); + error(); + } catch (final HttpException httpException) { + route.failedHttpRequestCounter().increment(); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .httpStatus(httpException.status()) + .grpcStatus(GrpcStatus.INVALID_ARGUMENT) + .send(); + error(); + } catch (final Exception unknown) { + route.failedUnknownRequestCounter().increment(); + LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .grpcStatus(GrpcStatus.UNKNOWN).send(); + error(); + } + } + + public void setPipeline(@NonNull final Pipeline pipeline) { + this.pipeline = requireNonNull(pipeline); + } + + public void cancelDeadlineFuture(boolean isCancelled) { + deadlineFuture.cancel(isCancelled); + } + + public ServiceInterface.RequestOptions options() { + return options; + } + + /** + * According to the specification, the "Response-Headers" are: + * + * 

HTTP-Status [Message-Encoding] [Message-Accept-Encoding] Content-Type *Custom-Metadata
+     *
+ * + *

Where "Status" is always 200 OK. + * + *

The Response-Headers are normally sent, followed by the data, followed by trailers. But if + * the request fails right away, before any handling, then it is possible to send Trailers-Only + * instead. + */ + private void sendResponseHeaders( + @Nullable final Header messageEncoding, + @NonNull final HttpMediaType contentType, + @NonNull final List

customMetadata) { + + // Some headers are http2 specific, the rest are used for the grpc protocol + final var grpcHeaders = WritableHeaders.create(); + // FUTURE: I think to support custom headers in the response, we would have to list them + // here. + // Since this has to be sent before we have any data to send, we must know ahead of time + // which custom headers are to be returned. + grpcHeaders.set(HeaderNames.TRAILER, "grpc-status, grpc-message"); + grpcHeaders.set(Http2Headers.STATUS_NAME, Status.OK_200.code()); + grpcHeaders.contentType(contentType); + grpcHeaders.set(GRPC_ACCEPT_ENCODING, IDENTITY); + customMetadata.forEach(grpcHeaders::set); + if (messageEncoding != null) { + grpcHeaders.set(messageEncoding); + } + + final var http2Headers = Http2Headers.create(grpcHeaders); + + streamWriter.writeHeaders( + http2Headers, + streamId, + Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS), + flowControl.outbound()); + } + + /** + * Helper function. Given a string of digits followed by a unit, schedule a callback to be + * invoked when the deadline is exceeded, and return the associated future. The proper format of + * the string is defined in the specification as: + * + * 
+     *      Timeout → "grpc-timeout" TimeoutValue TimeoutUnit
+     *      TimeoutValue → {positive integer as ASCII string of at most 8 digits}
+     *      TimeoutUnit → Hour / Minute / Second / Millisecond / Microsecond / Nanosecond
+     *             Hour → "H"
+     *             Minute → "M"
+     *             Second → "S"
+     *             Millisecond → "m"
+     *             Microsecond → "u"
+     *             Nanosecond → "n"
+     *
+ * + *

Illegal values result in the deadline being ignored. + * + * @param timeout The timeout value. Cannot be null. + * @return The future representing the task that will be executed if/when the deadline is + * reached. + */ + @NonNull + private ScheduledFuture scheduleDeadline(@NonNull final String timeout) { + final var matcher = GRPC_TIMEOUT_PATTERN.matcher(timeout); + if (matcher.matches()) { + final var num = Integer.parseInt(matcher.group(1)); + final var unit = matcher.group(2); + final var deadline = + System.nanoTime() + * TimeUnit.NANOSECONDS.convert( + num, + switch (unit) { + case "H" -> TimeUnit.HOURS; + case "M" -> TimeUnit.MINUTES; + case "S" -> TimeUnit.SECONDS; + case "m" -> TimeUnit.MILLISECONDS; + case "u" -> TimeUnit.MICROSECONDS; + case "n" -> TimeUnit.NANOSECONDS; + // This should NEVER be reachable, because the matcher + // would not have matched. + default -> throw new GrpcException( + GrpcStatus.INTERNAL, "Invalid unit: " + unit); + }); + return deadlineDetector.scheduleDeadline( + deadline, + () -> { + route.deadlineExceededCounter().increment(); + pipeline.onError(new GrpcException(GrpcStatus.DEADLINE_EXCEEDED)); + }); + } + + return new NoopScheduledFuture<>(); + } + + /** + * A {@link ScheduledFuture} that does nothing. This is used when there is no deadline set for + * the request. A new instance of this must be created (or we need a "reset" method) for each + * {@link PbjProtocolHandler} instance, because it can become "corrupted" if canceled from any + * particular call. + */ + private static final class NoopScheduledFuture extends CompletableFuture + implements ScheduledFuture { + @Override + public long getDelay(@NonNull final TimeUnit unit) { + return 0; + } + + @Override + public int compareTo(@NonNull final Delayed o) { + // Since all NoopScheduledFuture instances have "0" as the delay, any other Delayed + // instance with a non-0 + // delay will come after this one. + return (int) (o.getDelay(TimeUnit.NANOSECONDS)); + } + } + + /** + * An error has occurred. Cancel the deadline future if it's still active, and set the stream + * state accordingly. + * + *

May be called by different threads concurrently. + */ + private void error() { + // Canceling a future that has already completed has no effect. So by canceling here, we are + // saying: + // "If you have not yet executed, never execute. If you have already executed, then just + // ignore me". + // The "isCancelled" flag is set if the future was canceled before it was executed. + + // cancel is threadsafe + cancelDeadlineFuture(false); + grpcDataProcessor.setCurrentStreamState(current -> Http2StreamState.CLOSED); + } + + /** Simple implementation of the {@link ServiceInterface.RequestOptions} interface. */ + private record Options( + Optional authority, boolean isProtobuf, boolean isJson, String contentType) + implements ServiceInterface.RequestOptions {} +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index 81d0c547..5fa1bbba 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -46,39 +46,22 @@ * created for each new connection, and each connection is made to a specific method endpoint. */ final class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHandler { - private static final System.Logger LOGGER = - System.getLogger(PbjProtocolHandler.class.getName()); - - // Helidon-specific fields related to the connection itself - private final Http2StreamWriter streamWriter; - private final int streamId; - private final StreamFlowControl flowControl; - private final HeadersProcessor headersProcessor; /** * The service method that this connection was created for. The route has information about the * {@link ServiceInterface} and method to invoke, as well as metrics, and other information. */ private final PbjMethodRoute route; - private final Pipeline pipeline; private final GrpcDataProcessor grpcDataProcessor; /** Create a new instance */ PbjProtocolHandler( - @NonNull final Http2StreamWriter streamWriter, - final int streamId, - @NonNull final StreamFlowControl flowControl, @NonNull final PbjMethodRoute route, @NonNull final GrpcDataProcessor grpcDataProcessor, - @NonNull final HeadersProcessor headersProcessor, @NonNull final Pipeline pipeline) { - this.streamWriter = requireNonNull(streamWriter); - this.streamId = streamId; - this.flowControl = requireNonNull(flowControl); this.route = requireNonNull(route); this.grpcDataProcessor = requireNonNull(grpcDataProcessor); - this.headersProcessor = requireNonNull(headersProcessor); this.pipeline = requireNonNull(pipeline); } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java index cdfa64cd..7807391b 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java @@ -127,9 +127,9 @@ public SubProtocolResult subProtocol( true, new RouteNotFoundHandler(streamWriter, streamId, currentStreamState)); } - final HeadersProcessor headersProcessor = new HeadersProcessor( - headers, streamWriter, streamId, flowControl, route, deadlineDetector); final var grpcDataProcessor = new GrpcDataProcessorImpl(config, currentStreamState); + final HeadersProcessor headersProcessor = new HeadersProcessorImpl( + headers, streamWriter, streamId, flowControl, route, deadlineDetector, grpcDataProcessor); final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( streamWriter, streamId, flowControl, route, grpcDataProcessor, headersProcessor); @@ -145,12 +145,8 @@ public SubProtocolResult subProtocol( return new SubProtocolResult( true, new PbjProtocolHandler( - streamWriter, - streamId, - flowControl, route, grpcDataProcessor, - headersProcessor, pipeline)); } } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java index 043d2589..096086c1 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java @@ -107,7 +107,9 @@ public void onError(@NonNull final Throwable throwable) { new TrailerBuilder(streamWriter, streamId, flowControl) .grpcStatus(GrpcStatus.INTERNAL).send(); } -// error(); + + headersProcessor.cancelDeadlineFuture(false); + grpcDataProcessor.setCurrentStreamState(current -> Http2StreamState.CLOSED); } @Override diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index 4ab5e9a7..baf3f67f 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -18,7 +18,6 @@ import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; -import com.hedera.pbj.grpc.helidon.config.PbjConfig; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.Pipelines; import com.hedera.pbj.runtime.grpc.ServiceInterface; @@ -27,7 +26,7 @@ import greeter.HelloReply; import greeter.HelloRequest; import io.helidon.common.buffers.BufferData; -import io.helidon.http.http2.Http2Headers; +import io.helidon.http.http2.Http2FrameHeader; import io.helidon.http.http2.Http2StreamState; import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; @@ -36,76 +35,100 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import java.security.NoSuchAlgorithmException; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.function.Consumer; +import java.util.function.UnaryOperator; import static com.hedera.pbj.grpc.helidon.PbjProtocolHandlerTest.TestGreeterService.TestGreeterMethod.sayHelloStreamReply; +import static java.util.Objects.requireNonNull; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class PbjProtocolHandlerTest { - @Mock private Http2Headers http2Headers; - @Mock private Http2StreamWriter http2StreamWriter; @Mock private StreamFlowControl streamFlowControl; - @Mock private Http2StreamState http2StreamState; - - @Mock private PbjConfig pbjConfig; - @Mock private PbjMethodRoute pbjMethodRoute; - @Mock private DeadlineDetector deadlineDetector; - @Mock private BufferData bufferData; + @Mock private HeadersProcessor headersProcessor; + + @Mock private ServiceInterface.RequestOptions options; + + @Mock private static Consumer testConsumer; - @Mock private Pipeline pipeline; @Test public void testOnErrorHandlerCalledOnException() { - // We're testing the onError() routing from PbjProtocolHandler into - // an Application defined method. To confirm the registered handler - // gets called when there's an exception. - final Pipeline subscriber = mock(Pipeline.class); - final HeadersProcessor headersProcessor = new HeadersProcessor( - http2Headers, http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, deadlineDetector); - final var grpcDataProcessor = new GrpcDataProcessorImpl(pbjConfig, http2StreamState); - + final HelloRequest helloRequest = HelloRequest.newBuilder().setName("Alice").build(); + final var grpcDataProcessor = new TestGrpcDataProcessor(helloRequest); + when(headersProcessor.options()).thenReturn(options); final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, grpcDataProcessor, headersProcessor); + + final Pipeline subscriber = mock(Pipeline.class); + when(pbjMethodRoute.service()).thenReturn(new TestGreeterService(subscriber)); + when(pbjMethodRoute.method()).thenReturn(sayHelloStreamReply); + final PipelineBuilder pipelineBuilder = new PipelineBuilder(pbjMethodRoute, headersProcessor.options(), sendToClientSubscriber.subscriber()); + final Pipeline pipeline = pipelineBuilder.createPipeline(); grpcDataProcessor.setPipeline(pipeline); sendToClientSubscriber.setPipeline(pipeline); - headersProcessor.setPipeline(pipeline); - final PbjProtocolHandler testPbjProtocolHandler = new PbjProtocolHandler( - http2StreamWriter, - 1, - streamFlowControl, - pbjMethodRoute, - grpcDataProcessor, - headersProcessor, - pipeline); + when(bufferData.available()).thenReturn(1, 0); + grpcDataProcessor.data(null, bufferData); doThrow(IllegalArgumentException.class).when(bufferData).available(); - testPbjProtocolHandler.data(null, bufferData); + grpcDataProcessor.data(null, bufferData); + + verify(testConsumer, timeout(50).times(1)).accept("TEST"); } - static class TestGreeterService implements ServiceInterface { + private static class TestGrpcDataProcessor implements GrpcDataProcessor { + + private Pipeline pipeline; + private final byte[] helloRequestBytes; - private final Pipeline subscriber; + private TestGrpcDataProcessor(final HelloRequest helloRequest) { + this.helloRequestBytes = helloRequest.toByteArray(); + } + + public void data(@NonNull final Http2FrameHeader header, @NonNull final BufferData data) { + try { + while (data.available() > 0) { + pipeline.onNext(Bytes.wrap(helloRequestBytes)); + } + } catch (Exception e) { + pipeline.onError(e); + } + } + + public void setPipeline(@NonNull final Pipeline pipeline) { + this.pipeline = requireNonNull(pipeline); + } + + public void setCurrentStreamState(UnaryOperator operator) {} + + public Http2StreamState getCurrentStreamState() { + return null; + } + } + + static class TestGreeterService implements ServiceInterface { public TestGreeterService(final Pipeline subscriber) { - this.subscriber = subscriber; +// this.subscriber = subscriber; } enum TestGreeterMethod implements Method { @@ -196,8 +219,7 @@ private Bytes createReply( void sayHelloStreamReply( HelloRequest request, Pipeline replies) { replies.registerOnErrorHandler(() -> { - System.out.println("Error handler called"); - subscriber.onError(new NoSuchAlgorithmException()); + testConsumer.accept("TEST"); }); } From d3f92a39c78f24e324356bfc8573eba8afd93264 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Tue, 12 Nov 2024 15:29:18 -0700 Subject: [PATCH 09/11] fix: fixing exceptions tests Signed-off-by: Matt Peterson --- .../pbj/grpc/helidon/PbjProtocolSelector.java | 15 +++-- .../pbj/grpc/helidon/PipelineBuilder.java | 57 ++++++++++++++++++- .../grpc/helidon/PbjProtocolHandlerTest.java | 3 +- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java index 7807391b..54ffa233 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java @@ -127,20 +127,27 @@ public SubProtocolResult subProtocol( true, new RouteNotFoundHandler(streamWriter, streamId, currentStreamState)); } - final var grpcDataProcessor = new GrpcDataProcessorImpl(config, currentStreamState); + final GrpcDataProcessorImpl grpcDataProcessor = new GrpcDataProcessorImpl(config, currentStreamState); final HeadersProcessor headersProcessor = new HeadersProcessorImpl( headers, streamWriter, streamId, flowControl, route, deadlineDetector, grpcDataProcessor); final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( streamWriter, streamId, flowControl, route, grpcDataProcessor, headersProcessor); - final PipelineBuilder pipelineBuilder = new PipelineBuilder(route, headersProcessor.options(), sendToClientSubscriber.subscriber()); - final Pipeline pipeline = pipelineBuilder.createPipeline(); + final PipelineBuilder pipelineBuilder = new PipelineBuilder( + streamWriter, + streamId, + flowControl, + route, + headersProcessor.options(), + sendToClientSubscriber.subscriber(), + grpcDataProcessor, + headersProcessor); + final Pipeline pipeline = pipelineBuilder.createPipeline(); grpcDataProcessor.setPipeline(pipeline); sendToClientSubscriber.setPipeline(pipeline); headersProcessor.setPipeline(pipeline); - // This is a valid call! return new SubProtocolResult( true, diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java index 50a85e4d..d3f47781 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java @@ -1,31 +1,84 @@ package com.hedera.pbj.grpc.helidon; +import com.hedera.pbj.runtime.grpc.GrpcException; +import com.hedera.pbj.runtime.grpc.GrpcStatus; import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.ServiceInterface; import com.hedera.pbj.runtime.io.buffer.Bytes; import edu.umd.cs.findbugs.annotations.NonNull; +import io.helidon.http.HttpException; +import io.helidon.http.http2.Http2StreamState; +import io.helidon.http.http2.Http2StreamWriter; +import io.helidon.http.http2.StreamFlowControl; +import static java.lang.System.Logger.Level.ERROR; import static java.util.Objects.requireNonNull; public class PipelineBuilder { + private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); + + private final Http2StreamWriter streamWriter; + private final int streamId; + private final StreamFlowControl flowControl; private final PbjMethodRoute route; private final ServiceInterface.RequestOptions options; private final Pipeline outgoing; + private final GrpcDataProcessor grpcDataProcessor; + private final HeadersProcessor headersProcessor; PipelineBuilder( + @NonNull final Http2StreamWriter streamWriter, + final int streamId, + @NonNull final StreamFlowControl flowControl, @NonNull final PbjMethodRoute route, @NonNull final ServiceInterface.RequestOptions options, - @NonNull final Pipeline outgoing) { + @NonNull final Pipeline outgoing, + @NonNull final GrpcDataProcessor grpcDataProcessor, + @NonNull final HeadersProcessor headersProcessor) { + this.streamWriter = requireNonNull(streamWriter); + this.streamId = streamId; + this.flowControl = requireNonNull(flowControl); this.route = requireNonNull(route); this.options = requireNonNull(options); this.outgoing = requireNonNull(outgoing); + this.grpcDataProcessor = requireNonNull(grpcDataProcessor); + this.headersProcessor = requireNonNull(headersProcessor); } public Pipeline createPipeline() { // Setup the subscribers. The "outgoing" subscriber will send messages to the client. // This is given to the "open" method on the service to allow it to send messages to // the client. - return route.service().open(route.method(), options, outgoing); + try { + return route.service().open(route.method(), options, outgoing); + } catch (final GrpcException grpcException) { + route.failedGrpcRequestCounter().increment(); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .grpcStatus(grpcException.status()) + .statusMessage(grpcException.getMessage()) + .send(); + error(); + } catch (final HttpException httpException) { + route.failedHttpRequestCounter().increment(); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .httpStatus(httpException.status()) + .grpcStatus(GrpcStatus.INVALID_ARGUMENT) + .send(); + error(); + } catch (final Exception unknown) { + route.failedUnknownRequestCounter().increment(); + LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); + new TrailerOnlyBuilder(streamWriter, streamId, flowControl) + .grpcStatus(GrpcStatus.UNKNOWN).send(); + error(); + } + + return null; + } + + private void error() { + headersProcessor.cancelDeadlineFuture(false); + grpcDataProcessor.setCurrentStreamState(current -> Http2StreamState.CLOSED); } } diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index baf3f67f..0f781751 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -80,7 +80,8 @@ public void testOnErrorHandlerCalledOnException() { when(pbjMethodRoute.service()).thenReturn(new TestGreeterService(subscriber)); when(pbjMethodRoute.method()).thenReturn(sayHelloStreamReply); - final PipelineBuilder pipelineBuilder = new PipelineBuilder(pbjMethodRoute, headersProcessor.options(), sendToClientSubscriber.subscriber()); + final PipelineBuilder pipelineBuilder = new PipelineBuilder( + http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, headersProcessor.options(), sendToClientSubscriber.subscriber(), grpcDataProcessor, headersProcessor); final Pipeline pipeline = pipelineBuilder.createPipeline(); grpcDataProcessor.setPipeline(pipeline); From c8d7f81ec9e72da29c1f984d416b287259c178cc Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Tue, 12 Nov 2024 15:51:15 -0700 Subject: [PATCH 10/11] fix: added test comments Signed-off-by: Matt Peterson --- .../grpc/helidon/PbjProtocolHandlerTest.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index 0f781751..79473816 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -70,16 +70,17 @@ public class PbjProtocolHandlerTest { @Test public void testOnErrorHandlerCalledOnException() { - final HelloRequest helloRequest = HelloRequest.newBuilder().setName("Alice").build(); - final var grpcDataProcessor = new TestGrpcDataProcessor(helloRequest); + // Create a fake HelloRequest which will initialize the route + final var grpcDataProcessor = new TestGrpcDataProcessor(HelloRequest.newBuilder().setName("Alice").build()); when(headersProcessor.options()).thenReturn(options); - final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( + final var sendToClientSubscriber = new SendToClientSubscriber( http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, grpcDataProcessor, headersProcessor); - final Pipeline subscriber = mock(Pipeline.class); - when(pbjMethodRoute.service()).thenReturn(new TestGreeterService(subscriber)); + // Stub the route so our test service is used + when(pbjMethodRoute.service()).thenReturn(new TestGreeterService()); when(pbjMethodRoute.method()).thenReturn(sayHelloStreamReply); + // Create the pipeline final PipelineBuilder pipelineBuilder = new PipelineBuilder( http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, headersProcessor.options(), sendToClientSubscriber.subscriber(), grpcDataProcessor, headersProcessor); final Pipeline pipeline = pipelineBuilder.createPipeline(); @@ -87,12 +88,17 @@ public void testOnErrorHandlerCalledOnException() { grpcDataProcessor.setPipeline(pipeline); sendToClientSubscriber.setPipeline(pipeline); + // Use bufferData to simulate data being available in the first + // pass to initialize the downstream service. + // On the second while-loop pass, simulate the data being consumed (0). + // On the third while-loop pass, throw an exception. when(bufferData.available()).thenReturn(1, 0); grpcDataProcessor.data(null, bufferData); doThrow(IllegalArgumentException.class).when(bufferData).available(); grpcDataProcessor.data(null, bufferData); + // Verify the testConsumer was invoked inside the registered runnable. verify(testConsumer, timeout(50).times(1)).accept("TEST"); } @@ -105,6 +111,9 @@ private TestGrpcDataProcessor(final HelloRequest helloRequest) { this.helloRequestBytes = helloRequest.toByteArray(); } + // Implement the core pieces of the GrpcDataProcessorImpl class: + // a while loop which will call pipeline.onNext() while data is available + // and throw an exception if an error occurs. public void data(@NonNull final Http2FrameHeader header, @NonNull final BufferData data) { try { while (data.available() > 0) { @@ -128,10 +137,6 @@ public Http2StreamState getCurrentStreamState() { static class TestGreeterService implements ServiceInterface { - public TestGreeterService(final Pipeline subscriber) { -// this.subscriber = subscriber; - } - enum TestGreeterMethod implements Method { sayHelloStreamReply, sayHelloStreamBidi From ea87a8768b213b3b9972558b4fe56e6bed518458 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Wed, 13 Nov 2024 12:17:48 -0700 Subject: [PATCH 11/11] spotlessApply changes Signed-off-by: Matt Peterson --- .../hedera/pbj/grpc/helidon/Constants.java | 16 + .../pbj/grpc/helidon/GrpcDataProcessor.java | 19 +- .../grpc/helidon/GrpcDataProcessorImpl.java | 24 +- .../pbj/grpc/helidon/HeadersProcessor.java | 18 + .../grpc/helidon/HeadersProcessorImpl.java | 73 +-- .../pbj/grpc/helidon/PbjProtocolHandler.java | 15 +- .../pbj/grpc/helidon/PbjProtocolSelector.java | 49 +- .../pbj/grpc/helidon/PipelineBuilder.java | 25 +- .../grpc/helidon/SendToClientSubscriber.java | 51 +- .../pbj/grpc/helidon/TrailerBuilder.java | 38 +- .../pbj/grpc/helidon/TrailerOnlyBuilder.java | 22 +- .../pbj/grpc/helidon/GreeterService.java | 7 +- .../grpc/helidon/PbjProtocolHandlerTest.java | 70 +-- .../hedera/pbj/runtime/grpc/Pipelines.java | 10 +- .../pbj/runtime/grpc/ServiceInterface.java | 86 ++-- .../pbj/runtime/grpc/PipelinesTest.java | 447 ++++++++++-------- 16 files changed, 601 insertions(+), 369 deletions(-) diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java index bd9b3788..eea92966 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/Constants.java @@ -1,3 +1,19 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; import io.helidon.http.Header; diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java index 3ecdab05..65689d0b 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessor.java @@ -1,14 +1,31 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; import edu.umd.cs.findbugs.annotations.NonNull; import io.helidon.common.buffers.BufferData; import io.helidon.http.http2.Http2FrameHeader; import io.helidon.http.http2.Http2StreamState; - import java.util.function.UnaryOperator; public interface GrpcDataProcessor { void data(@NonNull final Http2FrameHeader header, @NonNull final BufferData data); + void setCurrentStreamState(UnaryOperator operator); + Http2StreamState getCurrentStreamState(); } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java index b8374c10..03938771 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/GrpcDataProcessorImpl.java @@ -1,5 +1,23 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; +import static java.util.Objects.requireNonNull; + import com.hedera.pbj.grpc.helidon.config.PbjConfig; import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; @@ -10,12 +28,9 @@ import io.helidon.http.http2.Http2FrameHeader; import io.helidon.http.http2.Http2FrameTypes; import io.helidon.http.http2.Http2StreamState; - import java.util.concurrent.atomic.AtomicReference; import java.util.function.UnaryOperator; -import static java.util.Objects.requireNonNull; - public class GrpcDataProcessorImpl implements GrpcDataProcessor { /** @@ -68,8 +83,7 @@ enum ReadState { private Pipeline pipeline; public GrpcDataProcessorImpl( - @NonNull final PbjConfig config, - @NonNull final Http2StreamState currentStreamState) { + @NonNull final PbjConfig config, @NonNull final Http2StreamState currentStreamState) { this.config = requireNonNull(config); this.currentStreamState = new AtomicReference<>(requireNonNull(currentStreamState)); diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java index 8a57d120..27062f44 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessor.java @@ -1,3 +1,19 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; import com.hedera.pbj.runtime.grpc.Pipeline; @@ -7,6 +23,8 @@ interface HeadersProcessor { void setPipeline(@NonNull final Pipeline pipeline); + void cancelDeadlineFuture(boolean isCancelled); + ServiceInterface.RequestOptions options(); } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java index b0cc3319..8d472df1 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/HeadersProcessorImpl.java @@ -1,5 +1,33 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; +import static com.hedera.pbj.grpc.helidon.Constants.GRPC_ENCODING_IDENTITY; +import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ENCODING; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_TIMEOUT; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_JSON; +import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_PROTO; +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; import com.hedera.pbj.runtime.grpc.Pipeline; @@ -19,7 +47,6 @@ import io.helidon.http.http2.Http2StreamState; import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; - import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -28,23 +55,12 @@ import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; -import static com.hedera.pbj.grpc.helidon.Constants.GRPC_ENCODING_IDENTITY; -import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ENCODING; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_TIMEOUT; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_JSON; -import static com.hedera.pbj.runtime.grpc.ServiceInterface.RequestOptions.APPLICATION_GRPC_PROTO; -import static java.lang.System.Logger.Level.ERROR; -import static java.util.Collections.emptyList; -import static java.util.Objects.requireNonNull; - public class HeadersProcessorImpl implements HeadersProcessor { private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); /** The regular expression used to parse the grpc-timeout header. */ private static final String GRPC_TIMEOUT_REGEX = "(\\d{1,8})([HMSmun])"; + private static final Pattern GRPC_TIMEOUT_PATTERN = Pattern.compile(GRPC_TIMEOUT_REGEX); /** @@ -68,6 +84,7 @@ public class HeadersProcessorImpl implements HeadersProcessor { * {@link ServiceInterface} and method to invoke, as well as metrics, and other information. */ private final PbjMethodRoute route; + private final Http2StreamWriter streamWriter; private final StreamFlowControl flowControl; private final int streamId; @@ -75,7 +92,6 @@ public class HeadersProcessorImpl implements HeadersProcessor { private final GrpcDataProcessor grpcDataProcessor; private Pipeline pipeline; - HeadersProcessorImpl( @NonNull final Http2Headers headers, @NonNull final Http2StreamWriter streamWriter, @@ -199,7 +215,8 @@ public class HeadersProcessorImpl implements HeadersProcessor { route.failedUnknownRequestCounter().increment(); LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); new TrailerOnlyBuilder(streamWriter, streamId, flowControl) - .grpcStatus(GrpcStatus.UNKNOWN).send(); + .grpcStatus(GrpcStatus.UNKNOWN) + .send(); error(); } } @@ -289,19 +306,19 @@ private ScheduledFuture scheduleDeadline(@NonNull final String timeout) { final var deadline = System.nanoTime() * TimeUnit.NANOSECONDS.convert( - num, - switch (unit) { - case "H" -> TimeUnit.HOURS; - case "M" -> TimeUnit.MINUTES; - case "S" -> TimeUnit.SECONDS; - case "m" -> TimeUnit.MILLISECONDS; - case "u" -> TimeUnit.MICROSECONDS; - case "n" -> TimeUnit.NANOSECONDS; - // This should NEVER be reachable, because the matcher - // would not have matched. - default -> throw new GrpcException( - GrpcStatus.INTERNAL, "Invalid unit: " + unit); - }); + num, + switch (unit) { + case "H" -> TimeUnit.HOURS; + case "M" -> TimeUnit.MINUTES; + case "S" -> TimeUnit.SECONDS; + case "m" -> TimeUnit.MILLISECONDS; + case "u" -> TimeUnit.MICROSECONDS; + case "n" -> TimeUnit.NANOSECONDS; + // This should NEVER be reachable, because the matcher + // would not have matched. + default -> throw new GrpcException( + GrpcStatus.INTERNAL, "Invalid unit: " + unit); + }); return deadlineDetector.scheduleDeadline( deadline, () -> { diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java index 5fa1bbba..80c90442 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandler.java @@ -16,29 +16,19 @@ package com.hedera.pbj.grpc.helidon; -import com.hedera.pbj.runtime.grpc.GrpcException; -import com.hedera.pbj.runtime.grpc.GrpcStatus; +import static java.util.Objects.requireNonNull; + import com.hedera.pbj.runtime.grpc.Pipeline; import com.hedera.pbj.runtime.grpc.ServiceInterface; import com.hedera.pbj.runtime.io.buffer.Bytes; import edu.umd.cs.findbugs.annotations.NonNull; import io.helidon.common.buffers.BufferData; -import io.helidon.http.http2.Http2Flag; -import io.helidon.http.http2.Http2FrameData; import io.helidon.http.http2.Http2FrameHeader; -import io.helidon.http.http2.Http2FrameTypes; import io.helidon.http.http2.Http2RstStream; import io.helidon.http.http2.Http2StreamState; -import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.Http2WindowUpdate; -import io.helidon.http.http2.StreamFlowControl; import io.helidon.webserver.http2.spi.Http2SubProtocolSelector; - import java.util.Objects; -import java.util.concurrent.Flow; - -import static java.lang.System.Logger.Level.ERROR; -import static java.util.Objects.requireNonNull; /** * Implementation of gRPC based on PBJ. This class specifically contains the glue logic for bridging @@ -52,6 +42,7 @@ final class PbjProtocolHandler implements Http2SubProtocolSelector.SubProtocolHa * {@link ServiceInterface} and method to invoke, as well as metrics, and other information. */ private final PbjMethodRoute route; + private final Pipeline pipeline; private final GrpcDataProcessor grpcDataProcessor; diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java index 54ffa233..ff621c04 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PbjProtocolSelector.java @@ -127,21 +127,36 @@ public SubProtocolResult subProtocol( true, new RouteNotFoundHandler(streamWriter, streamId, currentStreamState)); } - final GrpcDataProcessorImpl grpcDataProcessor = new GrpcDataProcessorImpl(config, currentStreamState); - final HeadersProcessor headersProcessor = new HeadersProcessorImpl( - headers, streamWriter, streamId, flowControl, route, deadlineDetector, grpcDataProcessor); + final GrpcDataProcessorImpl grpcDataProcessor = + new GrpcDataProcessorImpl(config, currentStreamState); + final HeadersProcessor headersProcessor = + new HeadersProcessorImpl( + headers, + streamWriter, + streamId, + flowControl, + route, + deadlineDetector, + grpcDataProcessor); - final SendToClientSubscriber sendToClientSubscriber = new SendToClientSubscriber( - streamWriter, streamId, flowControl, route, grpcDataProcessor, headersProcessor); - final PipelineBuilder pipelineBuilder = new PipelineBuilder( - streamWriter, - streamId, - flowControl, - route, - headersProcessor.options(), - sendToClientSubscriber.subscriber(), - grpcDataProcessor, - headersProcessor); + final SendToClientSubscriber sendToClientSubscriber = + new SendToClientSubscriber( + streamWriter, + streamId, + flowControl, + route, + grpcDataProcessor, + headersProcessor); + final PipelineBuilder pipelineBuilder = + new PipelineBuilder( + streamWriter, + streamId, + flowControl, + route, + headersProcessor.options(), + sendToClientSubscriber.subscriber(), + grpcDataProcessor, + headersProcessor); final Pipeline pipeline = pipelineBuilder.createPipeline(); grpcDataProcessor.setPipeline(pipeline); @@ -150,10 +165,6 @@ public SubProtocolResult subProtocol( // This is a valid call! return new SubProtocolResult( - true, - new PbjProtocolHandler( - route, - grpcDataProcessor, - pipeline)); + true, new PbjProtocolHandler(route, grpcDataProcessor, pipeline)); } } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java index d3f47781..0b03a5f8 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/PipelineBuilder.java @@ -1,5 +1,24 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Objects.requireNonNull; + import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; import com.hedera.pbj.runtime.grpc.Pipeline; @@ -11,9 +30,6 @@ import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; -import static java.lang.System.Logger.Level.ERROR; -import static java.util.Objects.requireNonNull; - public class PipelineBuilder { private final System.Logger LOGGER = System.getLogger(this.getClass().getName()); @@ -70,7 +86,8 @@ public Pipeline createPipeline() { route.failedUnknownRequestCounter().increment(); LOGGER.log(ERROR, "Failed to initialize grpc protocol handler", unknown); new TrailerOnlyBuilder(streamWriter, streamId, flowControl) - .grpcStatus(GrpcStatus.UNKNOWN).send(); + .grpcStatus(GrpcStatus.UNKNOWN) + .send(); error(); } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java index 096086c1..ffafcac9 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/SendToClientSubscriber.java @@ -1,5 +1,24 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; +import static java.lang.System.Logger.Level.ERROR; +import static java.util.Objects.requireNonNull; + import com.hedera.pbj.runtime.grpc.GrpcException; import com.hedera.pbj.runtime.grpc.GrpcStatus; import com.hedera.pbj.runtime.grpc.Pipeline; @@ -13,15 +32,11 @@ import io.helidon.http.http2.Http2StreamState; import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; - import java.util.concurrent.Flow; -import static java.lang.System.Logger.Level.ERROR; -import static java.util.Objects.requireNonNull; - /** - * The implementation of {@link Pipeline} used to send messages to the client. It - * receives bytes from the handlers to send to the client. + * The implementation of {@link Pipeline} used to send messages to the client. It receives bytes + * from the handlers to send to the client. */ final class SendToClientSubscriber implements Pipeline { @@ -80,8 +95,7 @@ public void onNext(@NonNull final Bytes response) { Http2Flag.DataFlags.create(0), streamId); - streamWriter.writeData( - new Http2FrameData(header, bufferData), flowControl.outbound()); + streamWriter.writeData(new Http2FrameData(header, bufferData), flowControl.outbound()); } catch (final Exception e) { LOGGER.log(ERROR, "Failed to respond to grpc request: " + route.method(), e); pipeline.onError(e); @@ -105,7 +119,8 @@ public void onError(@NonNull final Throwable throwable) { } else { LOGGER.log(ERROR, "Failed to send response", throwable); new TrailerBuilder(streamWriter, streamId, flowControl) - .grpcStatus(GrpcStatus.INTERNAL).send(); + .grpcStatus(GrpcStatus.INTERNAL) + .send(); } headersProcessor.cancelDeadlineFuture(false); @@ -114,21 +129,21 @@ public void onError(@NonNull final Throwable throwable) { @Override public void onComplete() { - new TrailerBuilder(streamWriter, streamId, flowControl) - .send(); + new TrailerBuilder(streamWriter, streamId, flowControl).send(); headersProcessor.cancelDeadlineFuture(false); - grpcDataProcessor.setCurrentStreamState(currentValue -> { - if (requireNonNull(currentValue) == Http2StreamState.OPEN) { - return Http2StreamState.HALF_CLOSED_LOCAL; - } - return Http2StreamState.CLOSED; - }); + grpcDataProcessor.setCurrentStreamState( + currentValue -> { + if (requireNonNull(currentValue) == Http2StreamState.OPEN) { + return Http2StreamState.HALF_CLOSED_LOCAL; + } + return Http2StreamState.CLOSED; + }); } @Override public void registerOnErrorHandler(@NonNull final Runnable handler) { this.onErrorHandler = requireNonNull(handler); } -} \ No newline at end of file +} diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java index 0cb39ada..042ead9b 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerBuilder.java @@ -1,5 +1,26 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; +import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + import com.hedera.pbj.runtime.grpc.GrpcStatus; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; @@ -10,14 +31,8 @@ import io.helidon.http.http2.Http2Headers; import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; - import java.util.List; -import static com.hedera.pbj.grpc.helidon.Constants.IDENTITY; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.GRPC_ACCEPT_ENCODING; -import static java.util.Collections.emptyList; -import static java.util.Objects.requireNonNull; - /** * A convenience class for building the trailers. In the specification, it says: * @@ -49,8 +64,8 @@ class TrailerBuilder { } /** - * Sets the gRPC status to return. Normally, the HTTP status will always be 200, while the - * gRPC status can be anything. + * Sets the gRPC status to return. Normally, the HTTP status will always be 200, while the gRPC + * status can be anything. */ @NonNull public TrailerBuilder grpcStatus(@NonNull final GrpcStatus grpcStatus) { @@ -73,8 +88,8 @@ public final void send() { } /** - * Actually sends the headers. This method exists so that "trailers-only" can call it to - * send the normal headers. + * Actually sends the headers. This method exists so that "trailers-only" can call it to send + * the normal headers. */ protected void send( @NonNull final WritableHeaders httpHeaders, @@ -90,8 +105,7 @@ protected void send( streamWriter.writeHeaders( http2Headers, streamId, - Http2Flag.HeaderFlags.create( - Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM), + Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM), flowControl.outbound()); } } diff --git a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java index f033b2d7..92784a14 100644 --- a/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java +++ b/pbj-core/pbj-grpc-helidon/src/main/java/com/hedera/pbj/grpc/helidon/TrailerOnlyBuilder.java @@ -1,5 +1,24 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.grpc.helidon; +import static com.hedera.pbj.grpc.helidon.GrpcHeaders.APPLICATION_GRPC_PROTO_TYPE; +import static java.util.Objects.requireNonNull; + import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import io.helidon.http.HttpMediaType; @@ -9,9 +28,6 @@ import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; -import static com.hedera.pbj.grpc.helidon.GrpcHeaders.APPLICATION_GRPC_PROTO_TYPE; -import static java.util.Objects.requireNonNull; - /** * A convenience class for building the trailers in the event of a catastrophic error before any * headers could be sent to the client in response. In the specification, it says: diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java index 60d9bc12..7cd7d1a0 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/GreeterService.java @@ -28,7 +28,6 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.concurrent.Flow; /** * This service doesn't rely on any PBJ objects, because the build right now doesn't have a good way @@ -47,15 +46,13 @@ enum GreeterMethod implements Method { HelloReply sayHello(HelloRequest request); // A stream of messages coming from the client, with a single response from the server. - Pipeline sayHelloStreamRequest( - Pipeline replies); + Pipeline sayHelloStreamRequest(Pipeline replies); // A single request from the client, with a stream of responses from the server. void sayHelloStreamReply(HelloRequest request, Pipeline replies); // A bidirectional stream of requests and responses between the client and the server. - Pipeline sayHelloStreamBidi( - Pipeline replies); + Pipeline sayHelloStreamBidi(Pipeline replies); @NonNull default String serviceName() { diff --git a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java index 79473816..85f00c71 100644 --- a/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java +++ b/pbj-core/pbj-grpc-helidon/src/test/java/com/hedera/pbj/grpc/helidon/PbjProtocolHandlerTest.java @@ -16,6 +16,13 @@ package com.hedera.pbj.grpc.helidon; +import static com.hedera.pbj.grpc.helidon.PbjProtocolHandlerTest.TestGreeterService.TestGreeterMethod.sayHelloStreamReply; +import static java.util.Objects.requireNonNull; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; import com.hedera.pbj.runtime.grpc.Pipeline; @@ -30,24 +37,15 @@ import io.helidon.http.http2.Http2StreamState; import io.helidon.http.http2.Http2StreamWriter; import io.helidon.http.http2.StreamFlowControl; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.function.Consumer; import java.util.function.UnaryOperator; - -import static com.hedera.pbj.grpc.helidon.PbjProtocolHandlerTest.TestGreeterService.TestGreeterMethod.sayHelloStreamReply; -import static java.util.Objects.requireNonNull; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.timeout; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) public class PbjProtocolHandlerTest { @@ -66,23 +64,37 @@ public class PbjProtocolHandlerTest { @Mock private static Consumer testConsumer; - @Test public void testOnErrorHandlerCalledOnException() { // Create a fake HelloRequest which will initialize the route - final var grpcDataProcessor = new TestGrpcDataProcessor(HelloRequest.newBuilder().setName("Alice").build()); + final var grpcDataProcessor = + new TestGrpcDataProcessor(HelloRequest.newBuilder().setName("Alice").build()); when(headersProcessor.options()).thenReturn(options); - final var sendToClientSubscriber = new SendToClientSubscriber( - http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, grpcDataProcessor, headersProcessor); + final var sendToClientSubscriber = + new SendToClientSubscriber( + http2StreamWriter, + 1, + streamFlowControl, + pbjMethodRoute, + grpcDataProcessor, + headersProcessor); // Stub the route so our test service is used when(pbjMethodRoute.service()).thenReturn(new TestGreeterService()); when(pbjMethodRoute.method()).thenReturn(sayHelloStreamReply); // Create the pipeline - final PipelineBuilder pipelineBuilder = new PipelineBuilder( - http2StreamWriter, 1, streamFlowControl, pbjMethodRoute, headersProcessor.options(), sendToClientSubscriber.subscriber(), grpcDataProcessor, headersProcessor); + final PipelineBuilder pipelineBuilder = + new PipelineBuilder( + http2StreamWriter, + 1, + streamFlowControl, + pbjMethodRoute, + headersProcessor.options(), + sendToClientSubscriber.subscriber(), + grpcDataProcessor, + headersProcessor); final Pipeline pipeline = pipelineBuilder.createPipeline(); grpcDataProcessor.setPipeline(pipeline); @@ -167,14 +179,15 @@ public Pipeline open( final var m = (TestGreeterMethod) method; try { return switch (m) { - // Client sends a single request and the server sends many responses - case sayHelloStreamReply -> Pipelines.serverStreaming() + // Client sends a single request and the server sends many responses + case sayHelloStreamReply -> Pipelines + .serverStreaming() .mapRequest(bytes -> parseRequest(bytes, options)) .method(this::sayHelloStreamReply) .mapResponse(reply -> createReply(reply, options)) .respondTo(replies) .build(); - // Client and server are sending messages back and forth. + // Client and server are sending messages back and forth. case sayHelloStreamBidi -> Pipelines.bidiStreaming() .mapRequest(bytes -> parseRequest(bytes, options)) .method(this::sayHelloStreamBidi) @@ -222,16 +235,15 @@ private Bytes createReply( } } - void sayHelloStreamReply( - HelloRequest request, Pipeline replies) { - replies.registerOnErrorHandler(() -> { - testConsumer.accept("TEST"); - }); + void sayHelloStreamReply(HelloRequest request, Pipeline replies) { + replies.registerOnErrorHandler( + () -> { + testConsumer.accept("TEST"); + }); } @NonNull - Pipeline sayHelloStreamBidi( - Pipeline replies) { + Pipeline sayHelloStreamBidi(Pipeline replies) { return null; } } diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java index 1b22a33b..5749b471 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/Pipelines.java @@ -34,8 +34,8 @@ private Pipelines() { } /** - * Returns a {@link Pipeline} that does nothing. This can be used in cases where a - * subscriber is required but no proper implementation is available. + * Returns a {@link Pipeline} that does nothing. This can be used in cases where a subscriber is + * required but no proper implementation is available. * * @return A No-op subscriber. */ @@ -407,8 +407,7 @@ public interface ServerStreamingMethod { * @param replies The subscriber to send responses to. * @throws Exception If an error occurs during processing. */ - void apply(@NonNull T request, @NonNull Pipeline replies) - throws Exception; + void apply(@NonNull T request, @NonNull Pipeline replies) throws Exception; } /** @@ -848,8 +847,7 @@ public void onNext(@NonNull final Bytes message) { * @param The type of the input. * @param The type of the output. */ - private record MapSubscriber( - Pipeline next, ExceptionalFunction mapper) + private record MapSubscriber(Pipeline next, ExceptionalFunction mapper) implements Pipeline, Flow.Subscription { private MapSubscriber { diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java index 367164a2..a3e2c6d7 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/grpc/ServiceInterface.java @@ -18,19 +18,17 @@ import com.hedera.pbj.runtime.io.buffer.Bytes; import edu.umd.cs.findbugs.annotations.NonNull; -import edu.umd.cs.findbugs.annotations.Nullable; import java.util.List; import java.util.Optional; -import java.util.concurrent.Flow; /** - * Defines a common interface for all implementations of a gRPC {@code service}. PBJ will generate a sub-interface - * for each {@code service} in the protobuf schema definition files, with default implementations of each of the - * given methods in this interface. + * Defines a common interface for all implementations of a gRPC {@code service}. PBJ will generate a + * sub-interface for each {@code service} in the protobuf schema definition files, with default + * implementations of each of the given methods in this interface. * *

For example, suppose I have the following protobuf file: - * 

- * {@code
+ *
+ * 
{@code
  * package example;
  *
  * service HelloService {
@@ -44,12 +42,12 @@
  * message HelloResponse {
  *   string reply = 1;
  * }
- * }
- * 

+ * }
* - *

From this file, PBJ will generate a {@code HelloService} interface that extends {@code ServiceInterface}: - * 

- * {@code
+ * 
From this file, PBJ will generate a {@code HelloService} interface that extends {@code
+ * ServiceInterface}:
+ *
+ * 
{@code
  * public interface HelloService extends ServiceInterface {
  *    // ...
  *
@@ -61,35 +59,33 @@
  *
  *    // ...
  * }
- * }
- * 

+ * }
* - * In the application code, you will simply create a new class implementing the {@code HelloService} interface, and - * register it with your webserver in whatever way is appropriate for your webserver. + * In the application code, you will simply create a new class implementing the {@code HelloService} + * interface, and register it with your webserver in whatever way is appropriate for your webserver. */ public interface ServiceInterface { - /** - * Represents the metadata of a method in a gRPC service. - */ + /** Represents the metadata of a method in a gRPC service. */ interface Method { String name(); } - /** - * The options that are passed to the service when a new connection is opened. - */ + /** The options that are passed to the service when a new connection is opened. */ interface RequestOptions { /** A constant for the gRPC content type "application/grpc". */ String APPLICATION_GRPC = "application/grpc"; + /** A constant for the gRPC content type "application/grpc+proto". */ String APPLICATION_GRPC_PROTO = "application/grpc+proto"; + /** A constant for the gRPC content type "application/grpc+json". */ String APPLICATION_GRPC_JSON = "application/grpc+json"; /** - * The authority of the client that is connecting to the service. This is the value of the ":authority" header - * in the HTTP/2 request. This value is used by the service to determine the client's identity. It may be that - * no authority is provided, in which case this method will return an empty optional. + * The authority of the client that is connecting to the service. This is the value of the + * ":authority" header in the HTTP/2 request. This value is used by the service to determine + * the client's identity. It may be that no authority is provided, in which case this method + * will return an empty optional. * * @return the authority of the client */ @@ -97,22 +93,23 @@ interface RequestOptions { Optional authority(); /** - * Gets whether the content type describes a protobuf message. This will be true if the {@link #contentType()} - * is equal to {@link #APPLICATION_GRPC_PROTO} or {@link #APPLICATION_GRPC}. + * Gets whether the content type describes a protobuf message. This will be true if the + * {@link #contentType()} is equal to {@link #APPLICATION_GRPC_PROTO} or {@link + * #APPLICATION_GRPC}. */ boolean isProtobuf(); /** - * Gets whether the content type describes a JSON message. This will be true if the {@link #contentType()} - * is equal to {@link #APPLICATION_GRPC_JSON}. + * Gets whether the content type describes a JSON message. This will be true if the {@link + * #contentType()} is equal to {@link #APPLICATION_GRPC_JSON}. */ boolean isJson(); /** - * Gets the content type of the request. This is the value of the "content-type" header in the HTTP/2 request. - * This value is used by the service to determine how to parse the request. Since gRPC supports custom content - * types, it is possible that the content type will be something other than the constants defined in this - * interface. + * Gets the content type of the request. This is the value of the "content-type" header in + * the HTTP/2 request. This value is used by the service to determine how to parse the + * request. Since gRPC supports custom content types, it is possible that the content type + * will be something other than the constants defined in this interface. * * @return the content type of the request */ @@ -123,25 +120,32 @@ interface RequestOptions { /** Gets the simple name of the service. For example, "HelloService". */ @NonNull String serviceName(); + /** Gets the full name of the service. For example, "example.HelloService". */ @NonNull String fullName(); - /** Gets a list of each method in the service. This list may be empty but should never be null. */ + + /** + * Gets a list of each method in the service. This list may be empty but should never be null. + */ @NonNull List methods(); /** - * Called by the webserver to open a new connection between the client and the service. This method may be called - * many times concurrently, once per connection. The implementation must therefore be thread-safe. A default - * implementation is provided by the generated PBJ code, which will handle the dispatching of messages to the - * appropriate methods in the correct way (unary, server-side streaming, etc.). + * Called by the webserver to open a new connection between the client and the service. This + * method may be called many times concurrently, once per connection. The implementation must + * therefore be thread-safe. A default implementation is provided by the generated PBJ code, + * which will handle the dispatching of messages to the appropriate methods in the correct way + * (unary, server-side streaming, etc.). * - * @param method The method that was called by the client. - * @param opts Any options from the request, such as the content type. + * @param method The method that was called by the client. + * @param opts Any options from the request, such as the content type. * @param responses The subscriber used by the service to push responses back to the client. */ @NonNull Pipeline open( - @NonNull Method method, @NonNull RequestOptions opts, @NonNull Pipeline responses) + @NonNull Method method, + @NonNull RequestOptions opts, + @NonNull Pipeline responses) throws GrpcException; } diff --git a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java index 1d3d480d..60317cad 100644 --- a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java +++ b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/grpc/PipelinesTest.java @@ -1,3 +1,19 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.pbj.runtime.grpc; import static org.assertj.core.api.Assertions.assertThat; @@ -65,7 +81,6 @@ void noopOnCompleteDoesNothing() { noop.onComplete(); assertThat(noop).isNotNull(); // if we get here, all is well. } - } @Nested @@ -76,10 +91,11 @@ class UnaryTest { @Test void requestMapperIsRequired() { - final var builder = Pipelines.unary() - .method(String::toUpperCase) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.unary() + .method(String::toUpperCase) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The request mapper must be specified.") @@ -88,10 +104,11 @@ void requestMapperIsRequired() { @Test void methodIsRequired() { - final var builder = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The method must be specified.") @@ -100,10 +117,11 @@ void methodIsRequired() { @Test void responseMapperIsRequired() { - final var builder = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .method(String::toUpperCase) - .respondTo(replies); + final var builder = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .method(String::toUpperCase) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The response mapper must be specified.") @@ -112,10 +130,11 @@ void responseMapperIsRequired() { @Test void respondToIsRequired() { - final var builder = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .method(String::toUpperCase) - .mapResponse(Bytes::wrap); + final var builder = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .method(String::toUpperCase) + .mapResponse(Bytes::wrap); assertThatThrownBy(builder::build) .hasMessage("The replies subscriber must be specified.") @@ -124,12 +143,13 @@ void respondToIsRequired() { @Test void nullSubscriptionThrowsNPE() { - final var pipeline = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .method(String::toUpperCase) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .method(String::toUpperCase) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); assertThatThrownBy(() -> pipeline.onSubscribe(null)) .isInstanceOf(NullPointerException.class); @@ -137,12 +157,13 @@ void nullSubscriptionThrowsNPE() { @Test void onNextTwiceThrowsISE() { - final var pipeline = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .method(String::toUpperCase) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .method(String::toUpperCase) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -154,12 +175,13 @@ void onNextTwiceThrowsISE() { void exceptionDuring_onNext_IsHandled() { final var ex = new RuntimeException("Some exception"); doThrow(ex).when(replies).onNext(any()); - final var pipeline = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .method(String::toUpperCase) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .method(String::toUpperCase) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -168,12 +190,13 @@ void exceptionDuring_onNext_IsHandled() { @Test void positive() { - final var pipeline = Pipelines.unary() - .mapRequest(Bytes::asUtf8String) - .method(String::toUpperCase) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.unary() + .mapRequest(Bytes::asUtf8String) + .method(String::toUpperCase) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(subscription); pipeline.onNext(Bytes.wrap("hello")); @@ -190,10 +213,11 @@ class BidiTest { @Test void requestMapperIsRequired() { - final var builder = Pipelines.bidiStreaming() - .method(sink -> client) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.bidiStreaming() + .method(sink -> client) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The request mapper must be specified.") @@ -202,10 +226,11 @@ void requestMapperIsRequired() { @Test void methodIsRequired() { - final var builder = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The method must be specified.") @@ -214,10 +239,11 @@ void methodIsRequired() { @Test void responseMapperIsRequired() { - final var builder = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .method(sink -> client) - .respondTo(replies); + final var builder = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .method(sink -> client) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The response mapper must be specified.") @@ -226,10 +252,11 @@ void responseMapperIsRequired() { @Test void respondToIsRequired() { - final var builder = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .method(sink -> client) - .mapResponse(Bytes::wrap); + final var builder = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .method(sink -> client) + .mapResponse(Bytes::wrap); assertThatThrownBy(builder::build) .hasMessage("The replies subscriber must be specified.") @@ -238,12 +265,13 @@ void respondToIsRequired() { @Test void nullSubscriptionThrowsNPE() { - final var pipeline = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .method(sink -> client) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .method(sink -> client) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); assertThatThrownBy(() -> pipeline.onSubscribe(null)) .isInstanceOf(NullPointerException.class); @@ -251,19 +279,27 @@ void nullSubscriptionThrowsNPE() { @Test void onCompleteNextThrowsISE() { - final var pipeline = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .method(sink -> { - lenient().doAnswer(invocation -> { - final var msg = invocation.getArgument(0, String.class); - sink.onNext(msg.toUpperCase()); - return null; - }).when(client).onNext(any()); - return client; - }) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .method( + sink -> { + lenient() + .doAnswer( + invocation -> { + final var msg = + invocation.getArgument( + 0, String.class); + sink.onNext(msg.toUpperCase()); + return null; + }) + .when(client) + .onNext(any()); + return client; + }) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -286,12 +322,13 @@ void onCompleteNextThrowsISE() { void exceptionDuring_onNext_IsHandled() { final var ex = new RuntimeException("Some exception"); doThrow(ex).when(client).onNext(any()); - final var pipeline = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .method(sink -> client) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .method(sink -> client) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -303,7 +340,10 @@ void exceptionDuring_responseConverter_IsHandled() { final var ex = new RuntimeException("Some exception"); Pipelines.bidiStreaming() .mapRequest(Bytes::asUtf8String) - .method(sink -> { throw ex; }) + .method( + sink -> { + throw ex; + }) .mapResponse(Bytes::wrap) .respondTo(replies) .build(); @@ -313,28 +353,34 @@ void exceptionDuring_responseConverter_IsHandled() { @Test void positive() { - final var pipeline = Pipelines.bidiStreaming() - .mapRequest(Bytes::asUtf8String) - .method(sink -> { - doAnswer(invocation -> { - final var msg = invocation.getArgument(0, String.class); - sink.onNext(msg.toUpperCase()); - return null; - }).when(client).onNext(any()); - return client; - }) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.bidiStreaming() + .mapRequest(Bytes::asUtf8String) + .method( + sink -> { + doAnswer( + invocation -> { + final var msg = + invocation.getArgument( + 0, String.class); + sink.onNext(msg.toUpperCase()); + return null; + }) + .when(client) + .onNext(any()); + return client; + }) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); final var argCaptor = ArgumentCaptor.forClass(Bytes.class); pipeline.onSubscribe(subscription); pipeline.onNext(Bytes.wrap("hello")); pipeline.onNext(Bytes.wrap("world")); verify(replies, times(2)).onNext(argCaptor.capture()); - assertThat(argCaptor.getAllValues()).containsExactly( - Bytes.wrap("HELLO"), - Bytes.wrap("WORLD")); + assertThat(argCaptor.getAllValues()) + .containsExactly(Bytes.wrap("HELLO"), Bytes.wrap("WORLD")); } } @@ -346,10 +392,11 @@ class ServerStreamingTest { @Test void requestMapperIsRequired() { - final var builder = Pipelines.serverStreaming() - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.serverStreaming() + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The request mapper must be specified.") @@ -358,10 +405,11 @@ void requestMapperIsRequired() { @Test void methodIsRequired() { - final var builder = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The method must be specified.") @@ -370,10 +418,11 @@ void methodIsRequired() { @Test void responseMapperIsRequired() { - final var builder = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .respondTo(replies); + final var builder = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The response mapper must be specified.") @@ -382,10 +431,11 @@ void responseMapperIsRequired() { @Test void respondToIsRequired() { - final var builder = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .mapResponse(Bytes::wrap); + final var builder = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .mapResponse(Bytes::wrap); assertThatThrownBy(builder::build) .hasMessage("The replies subscriber must be specified.") @@ -394,12 +444,13 @@ void respondToIsRequired() { @Test void nullSubscriptionThrowsNPE() { - final var pipeline = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); assertThatThrownBy(() -> pipeline.onSubscribe(null)) .isInstanceOf(NullPointerException.class); @@ -407,12 +458,13 @@ void nullSubscriptionThrowsNPE() { @Test void onCompleteNextThrowsISE() { - final var pipeline = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -425,12 +477,16 @@ void onCompleteNextThrowsISE() { @Test void badRequestMapperCallsOnError() { final var ex = new RuntimeException("Bad bad bad"); - final var pipeline = Pipelines.serverStreaming() - .mapRequest(bytes -> { throw ex; }) - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.serverStreaming() + .mapRequest( + bytes -> { + throw ex; + }) + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -440,12 +496,16 @@ void badRequestMapperCallsOnError() { @Test void badMethodCallsOnError() { final var ex = new RuntimeException("Bad bad bad"); - final var pipeline = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .method((msg, sink) -> { throw ex; }) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .method( + (msg, sink) -> { + throw ex; + }) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -454,12 +514,13 @@ void badMethodCallsOnError() { @Test void positive() { - final var pipeline = Pipelines.serverStreaming() - .mapRequest(Bytes::asUtf8String) - .method((msg, sink) -> sink.onNext(msg.toUpperCase())) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.serverStreaming() + .mapRequest(Bytes::asUtf8String) + .method((msg, sink) -> sink.onNext(msg.toUpperCase())) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(subscription); pipeline.onNext(Bytes.wrap("hello")); @@ -475,10 +536,11 @@ class ClientStreamingTest { @Test void requestMapperIsRequired() { - final var builder = Pipelines.clientStreaming() - .method(ConcatenatingHandler::new) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.clientStreaming() + .method(ConcatenatingHandler::new) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The request mapper must be specified.") @@ -487,10 +549,11 @@ void requestMapperIsRequired() { @Test void methodIsRequired() { - final var builder = Pipelines.clientStreaming() - .mapRequest(Bytes::asUtf8String) - .mapResponse(Bytes::wrap) - .respondTo(replies); + final var builder = + Pipelines.clientStreaming() + .mapRequest(Bytes::asUtf8String) + .mapResponse(Bytes::wrap) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The method must be specified.") @@ -499,10 +562,11 @@ void methodIsRequired() { @Test void responseMapperIsRequired() { - final var builder = Pipelines.clientStreaming() - .mapRequest(Bytes::asUtf8String) - .method(ConcatenatingHandler::new) - .respondTo(replies); + final var builder = + Pipelines.clientStreaming() + .mapRequest(Bytes::asUtf8String) + .method(ConcatenatingHandler::new) + .respondTo(replies); assertThatThrownBy(builder::build) .hasMessage("The response mapper must be specified.") @@ -511,10 +575,11 @@ void responseMapperIsRequired() { @Test void respondToIsRequired() { - final var builder = Pipelines.clientStreaming() - .mapRequest(Bytes::asUtf8String) - .method(ConcatenatingHandler::new) - .mapResponse(Bytes::wrap); + final var builder = + Pipelines.clientStreaming() + .mapRequest(Bytes::asUtf8String) + .method(ConcatenatingHandler::new) + .mapResponse(Bytes::wrap); assertThatThrownBy(builder::build) .hasMessage("The replies subscriber must be specified.") @@ -523,12 +588,13 @@ void respondToIsRequired() { @Test void nullSubscriptionThrowsNPE() { - final var pipeline = Pipelines.clientStreaming() - .mapRequest(Bytes::asUtf8String) - .method(ConcatenatingHandler::new) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.clientStreaming() + .mapRequest(Bytes::asUtf8String) + .method(ConcatenatingHandler::new) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); assertThatThrownBy(() -> pipeline.onSubscribe(null)) .isInstanceOf(NullPointerException.class); @@ -536,12 +602,13 @@ void nullSubscriptionThrowsNPE() { @Test void onCompleteNextThrowsISE() { - final var pipeline = Pipelines.clientStreaming() - .mapRequest(Bytes::asUtf8String) - .method(ConcatenatingHandler::new) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.clientStreaming() + .mapRequest(Bytes::asUtf8String) + .method(ConcatenatingHandler::new) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -554,12 +621,16 @@ void onCompleteNextThrowsISE() { @Test void badRequestMapperCallsOnError() { final var ex = new RuntimeException("Bad bad bad"); - final var pipeline = Pipelines.clientStreaming() - .mapRequest(bytes -> { throw ex; }) - .method(ConcatenatingHandler::new) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.clientStreaming() + .mapRequest( + bytes -> { + throw ex; + }) + .method(ConcatenatingHandler::new) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(mock(Flow.Subscription.class)); pipeline.onNext(Bytes.wrap("hello")); @@ -571,7 +642,10 @@ void badMethodCallsOnError() { final var ex = new RuntimeException("Bad bad bad"); Pipelines.clientStreaming() .mapRequest(Bytes::asUtf8String) - .method(sink -> { throw ex; }) + .method( + sink -> { + throw ex; + }) .mapResponse(Bytes::wrap) .respondTo(replies) .build(); @@ -581,12 +655,13 @@ void badMethodCallsOnError() { @Test void positive() { - final var pipeline = Pipelines.clientStreaming() - .mapRequest(Bytes::asUtf8String) - .method(ConcatenatingHandler::new) - .mapResponse(Bytes::wrap) - .respondTo(replies) - .build(); + final var pipeline = + Pipelines.clientStreaming() + .mapRequest(Bytes::asUtf8String) + .method(ConcatenatingHandler::new) + .mapResponse(Bytes::wrap) + .respondTo(replies) + .build(); pipeline.onSubscribe(subscription); pipeline.onNext(Bytes.wrap("hello"));