bindService() should be final

[zhangkun83]

We just found out a method in the generated code

The method is "bindService()" on the generated server interface is supposed to be "final", but is currently not.

For example

public abstract class RouteGuideImplBase {
  // Empty implementations, to be overridden by the application
  public StreamObserver routeSingle(StreamObserver) {
    failWithUnimplemented();
  }
  public StreamObserver routeMultiple(StreamObserver) {
    failWithUnimplemented();
  }

  // Glue code called by gRPC server to register the service.
  // Not supposed to be overridden.
  public bindService() {
    ...
  }
}

Why is it an issue

The non-final bindService() adds difficulty for users who want to mock the server interface in their tests. Mockito by default overrides all non-final methods with its no-op and null-returning impl. When a Mockito user creates a mock for RouteGuideImplBase, it can't be correctly registered to the server. It can be worked around by explicitly telling Mockito to use the original bindService(), but it's still a friction in the user experience.

What should we do

bindService() should have been defined as "final" in the first place. It was an oversight that we didn't make it so. This should be considered as a bug and be fixed.

We will make the change in master, which will be in 1.1.0.

Risk

This is technically an incompatible API change. Anyone overrides bindService() on the generated interface will be broken. However, I don't find anyone doing so in google3, or any legitimate reason for it in general. The chance of breaking anyone is very low.