search

salesforce / reactive-grpc

Reactive stubs for gRPC
BSD 3-Clause "New" or "Revised" License
833 stars 118 forks source link

Unwrap incoming `Mono`/`Single` parameters #301

Closed bsideup closed 2 years ago

bsideup commented 2 years ago

It is uncommon to have methods accept Mono as a parameter. The fact that ServerCalls did Mono.just is also a good indicator. This change makes single-input methods look like Mono<Response> foo(Request request) instead of Mono<Response> foo(Mono<Request> request).

It is backward compatible and will delegate to the Mono one (unless overridden, of course).

Closes #78 and #295

salesforce-cla[bot] commented 2 years ago

Thanks for the contribution! Before we can merge this, we need @bsideup to sign the Salesforce.com Contributor License Agreement.

bsideup commented 2 years ago

CLA signed 👍

bsideup commented 2 years ago

Hey @cbornet! Small world :) Done 👍

cbornet commented 2 years ago

Yes, small world 😃 . Could we also add some e2e tests in Reactor EndToEndIntegrationTest and RxJava EndToEndIntegrationTest. Maybe using the "Mono-unwrapped" client call ?

bsideup commented 2 years ago

Hi @cbornet! Don't want to sound rude, but, before I do another turn, is there anything else you would want me to do for this PR to be accepted? I have an extremely limited time right now given my new role so it is pretty hard to iterate on things, I would rather prefer not to. Thanks!

cbornet commented 2 years ago

Sorry, this came to my mind only after the first round. I sincerely apologize for this. LGTM. Thanks a lot.

bsideup commented 2 years ago

@cbornet all good! Thanks for merging it.

It's been a while since the last release but there weren't any changes other than the dependency updates. Would it be possible to cut a release some time soon? Thanks in advance!

cbornet commented 2 years ago

It's been a while since the last release but there weren't any changes other than the dependency updates. Would it be possible to cut a release some time soon?

@rmichela ?

rmichela commented 2 years ago

I've lost my Maven Central access after leaving Salesforce. I'm trying to connect with @nikolay-pshenichny who took over my position as company representative.

bsideup commented 2 years ago

Thank you @rmichela!

bsideup commented 1 year ago

@rmichela thanks for the release! BTW it looks like this PR is missing from the release notes at https://github.com/salesforce/reactive-grpc/releases/tag/v1.2.4 . I don't mind as long as it is released, but I thought some might find the change very helpful :)

rmichela commented 1 year ago

Will fix

sheinbergon commented 1 year ago

@rmichela @bsideup Just out of curiosity - which server were you testing this feature with? When testing wth Lognet gRPC start, I seem to be enocuntering Ambiguous method <METHOD NAME> in service, as it's detecting 2 methods with the same name (but diferent parameters). Is there a way I can disable this feature in the protoc plugin?

at org.lognet.springboot.grpc.GRpcServicesRegistry.descriptorToServiceMethod(GRpcServicesRegistry.java:204) ~[grpc-spring-boot-starter-5.1.1.jar:?]
    at org.springframework.util.function.SingletonSupplier.get(SingletonSupplier.java:97) ~[spring-core-6.0.8.jar:6.0.8]
    at org.lognet.springboot.grpc.GRpcServicesRegistry.getGrpServiceMethod(GRpcServicesRegistry.java:124) ~[grpc-spring-boot-starter-5.1.1.jar:?]
    at org.lognet.springboot.grpc.security.SecurityInterceptor.setupGRpcSecurityContext(SecurityInterceptor.java:261) ~[grpc-spring-boot-starter-5.1.1.jar:?]
    at org.lognet.springboot.grpc.security.SecurityInterceptor.interceptCall(SecurityInterceptor.java:143) ~[grpc-spring-boot-starter-5.1.1.jar:?]
    ... 14 more