Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to the protoc code generator to avoid generating deprecated code #3089

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions servicetalk-examples/grpc/protoc-options/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ protobuf {
outputSubDir = "java"
// Option to append a suffix to type names to avoid naming collisions with other code generation.
option 'typeNameSuffix=St'
// Option to tell the compiler to exclude all Deprecated fields, types, and methods from the output
option 'skipDeprecated=true'
mgodave marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//
// Copyright © 2019 Apple Inc. and the ServiceTalk project authors
//
// 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.

syntax = "proto3";

option java_multiple_files = true;
option java_package = "io.servicetalk.grpc.netty";
option java_outer_classname = "TesterProto";

package grpc.netty;

service Tester {
Copy link
Member

@idelpivnitskiy idelpivnitskiy Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was handy to use this file for validation of all API variants, but we should not add it to this example if we don't use it. Let's remove for now and we can reconsider the approach for the future similar to what you discussed with Bryce on how to observe the differences in generated code as part of PR review (follow-up).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it was handy for testing I feel like we should leave it or add another project just for this. I'm happy to add a usage here but it seems kind of counterproductive to have to copy/paste something in here every time we want to generate a comprehensive example for testing/comparison.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove it at the end before merging PR. It just doesn't fit into this example. And creating another module is a bit out of scope of this PR, we will plan that for the follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

rpc test (TestRequest) returns (TestResponse) {}
rpc testBiDiStream (stream TestRequest) returns (stream TestResponse) {}
rpc testResponseStream (TestRequest) returns (stream TestResponse) {}
rpc testRequestStream (stream TestRequest) returns (TestResponse) {}
}

message TestRequest {
string name = 1;
}

message TestResponse {
string message = 1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ final Single<GrpcServerContext> bind(final ServerBinder binder, final GrpcExecut
* In case you have a use-case, let us know.
*/
@Deprecated
protected abstract void registerRoutes(Service service); // FIXME: 0.43 - remove deprecated method
protected void registerRoutes(Service service) { // FIXME: 0.43 - remove deprecated method
throw new UnsupportedOperationException("This method is not used starting from version 0.42.0");
}

/**
* Create a new {@link Service} from the passed {@link AllGrpcRoutes}.
Expand All @@ -150,7 +152,9 @@ final Single<GrpcServerContext> bind(final ServerBinder binder, final GrpcExecut
* In case you have a use-case, let us know.
*/
@Deprecated
protected abstract Service newServiceFromRoutes(AllGrpcRoutes routes); // FIXME: 0.43 - remove deprecated method
protected Service newServiceFromRoutes(AllGrpcRoutes routes) { // FIXME: 0.43 - remove deprecated method
throw new UnsupportedOperationException("This method is not used starting from version 0.42.0");
}

static GrpcRoutes<?> merge(GrpcRoutes<?>... allRoutes) {
final GrpcRouter.Builder[] builders = new GrpcRouter.Builder[allRoutes.length];
Expand Down
37 changes: 35 additions & 2 deletions servicetalk-grpc-protoc/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ link:{source-root}/servicetalk-examples/grpc/helloworld[gRPC HelloWorld Example]
This plugin supports the following
link:https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.compiler.command_line_interface[options]:

==== `javaDocs=<true|false>`
==== `javaDocs=<true|false>` (default: true)
Control if `javadoc` is generated by this plugin. Here is an example of how you can specify this option with gradle:

[source,gradle]
Expand All @@ -47,7 +47,7 @@ And with Maven:
</protocPlugin>
----

==== `typeNameSuffix=<value>`
==== `typeNameSuffix=<value>` (default: not enabled)
Appends the <value> onto service type names generated by this plugin. This helps to avoid service type name
collisions if other protoc plugins are generating code in the same gradle project, or in the same package name.

Expand Down Expand Up @@ -78,3 +78,36 @@ And with Maven:
</args>
</protocPlugin>
----

==== `skipDeprecated=<true|false>` (default: false)
Instructs the generator to remove all deprecated methods and fields from the generated code, as well as remove calls
to deprecated code either to internal ServiceTalk libraries, or outside. This will almost certainly cause breaking changes
to code that uses these deprecated methods but will be forward compatible.

If you are using the
link:https://github.com/google/protobuf-gradle-plugin#configure-what-to-generate[protobuf-gradle-plugin] this is how you
can specify an option:

[source,gradle]
----
task.plugins {
servicetalk_grpc {
option 'skipDeprecated=true'
}
}
----

And with Maven:

And with Maven:

[source, xml]
----
<protocPlugin>
<id>servicetalk-grpc-protoc</id>
<!-- more params -->
<args>
<arg>skipDeprecated=true</arg>
</args>
</protocPlugin>
----
Loading
Loading