grpc-kotlin icon indicating copy to clipboard operation
grpc-kotlin copied to clipboard

Client stub & server ImplBase share the same interface

Open xiaodongw opened this issue 4 years ago • 6 comments

This is my personal opinion, I feel if the Client stub & server ImplBase are organized in this way:

interface Greeter {
  suspend fun sayHello(request: HelloRequest): HelloReply
}

class GreeterCoroutineStub(...): AbstractCoroutineStub<GreeterCoroutineStub>(...), Greeter
abstract class GreeterCoroutineImplBase(...): AbstractCoroutineServerImpl(...), Greeter

Then we can get some benefits like:

  1. The dependency will be declared with the interface, makes it easier to mock in unit tests.
  2. Stub & ImplBase is interchangeable with injection, so you can use a stub to call a remote service, but use ImplBase directly if it's living in the same JVM (I do have this kind of case, if a component is small so I just put it in the current service, but it may be separated out if it grows big).

xiaodongw avatar Apr 07 '20 18:04 xiaodongw

I find myself doing something like this for testing and isolating implementation of methods from the implementation of the server stub:

package io.grpc.health.v1

// one per method
interface CheckMethod {
    suspend operator fun invoke(request: HealthCheckRequest): HealthCheckResponse
}
package io.grpc.health.v1

// one per service
class HealthCheckService(
    private val checkMethod: CheckMethod
) : HealthGrpcKt.HealthCoroutineImplBase() {
    override suspend fun check(request: HealthCheckRequest) =
        checkMethod(request)
}

and so on and so forth for real services with many methods.

This way I can swap method implementations and run clients against mocked behavior in tests that spin up a real in-process server:

// oversimplified to show how it generally works
class HealthCheckGrpcExtension : BeforeEachCallback, AfterEachCallback {
    override fun beforeEach(context: ExtensionContext) {
        // class using extension has an opportunity to specify an implementation
        val checkMethod: CheckMethod? = getCheckMethod(context)

        addService(
            HealthCheckService(
                checkMethod = checkMethod ?: getDefaultCheckMethodImpl()
            )
        )
    }
    
    override fun afterEach(context: ExtensionContext) {
        shutDownService()
    }
}

Ideally the stub implementation generated by grpc-kotlin can support easy swapping of implementations, or else I hope to eventually find a better strategy that doesn't involve manually writing this boilerplate every time.

andrewparmet avatar Apr 09 '20 02:04 andrewparmet

I've thought about this some. It's definitely appealing, though there might be some weirdness if we ever want some things to diverge a little more between server and client -- e.g. the way servers interact with headers is different from the way clients do.

In the meantime, I know gRPC-Java recommends testing only using an in-memory channel; see e.g. https://github.com/grpc/grpc-java/blob/master/examples/src/test/java/io/grpc/examples/helloworld/HelloWorldServerTest.java#L56.

That, for example, correctly handles the stuff that is handled by gRPC, not coroutines, including some gRPC deadline settings. I know I have some helpers to set that up, but it's probably a greater-fidelity representation of how interacting with a gRPC server is likely to actual work.

lowasser avatar May 05 '20 20:05 lowasser

I should mention that above when we call addService(), we're actually adding a service to a JUnit 5 version of the GrpcCleanupRule and using an in-memory channel. We're just changing how the server responds to requests.

What's critical is that we don't have to subclass the generated stub, although now that I look at it it wouldn't be so crazy to do that and simply use the original implementations of the other methods if the test context doesn't specify them.

andrewparmet avatar May 06 '20 00:05 andrewparmet

For testing, I'm more talking about how to test the code that has dependency to another GRPC (or anything) service in unit test.

// service implementation, the dependency to B & C are represented as interface, not a stub
class ServiceAImpl(val serviceB: ServiceBInterface, val serviceC: ServiceCInterface) { ... }

// test
val serviceB = mock<ServiceBInterface>
val serviceC = mock<ServiceCInterface>
val serviceA = ServiceAImpl(serviceB, serviceC)

For the example above, I can easily mock the service B & C, without knowing B is a GRPC service, C is a REST service. If the dependency is a stub, I may have problem mocking it since some mock library doesn't support mock concrete methods; also it pulls in the knowledge about the framework, now I know service B is a GRPC service (because it's a GRPC stub).

We can still have this practice, by manually crafting the interfaces, it's just more boilerplate code. It would save people some time If the compiler can generate these interfaces.

About client & server interface diverging, it leads to another question. When I have a service interface, does it represent the business logic, or the technology to implement it? I prefer it to represent the business logic, then it makes sense that the client & server share the same interface, to prevent them diverge (of course for GRPC this is less a concern since the stub & ImplBase are generated, I had this problem before, added a parameter on server side but forgot to add it in client).

The only case I can think about is, we may want to pass headers to server side method, but since headers are for facility data like authentication, tracing, deadline, they should be handled by server interceptors. With Coroutine, these headers probably can be handled by an interceptor and then stored in the Coroutine context (like a tracing ID). For deadline, I'm not sure, but seems it can be done by cancelling the Coroutine job, so you don't have to think about it when implementing the service method.

xiaodongw avatar May 08 '20 02:05 xiaodongw

The headers are in fact the concern I had in mind. I agree that they should generally be handled by server interceptors, but I'm not certain that will cover all cases?

We might talk to the gRPC Java folks about the decisions for testing there -- they don't support anything like this, but I don't know if it's because of the several different stubs they generate, or what.

lowasser avatar May 08 '20 03:05 lowasser

Hey @lowasser, I just came across this question when starting to implement unit tests for my. Is the in-memory channel approach still the recommended way to go or did something different come up in the meantime?

ln-12 avatar Sep 05 '23 06:09 ln-12