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

feat: add FunctionalInterface annotation #1515

Merged
merged 7 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
*
* <p>It is similar to Guava's {@code AsyncFunction}, redeclared so that Guava can be shaded.
*/
@FunctionalInterface
public interface ApiAsyncFunction<I, O> {
/**
* Returns an output Future to use in place of the given input. The output Future need not be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
*
* <p>It is similar to Guava's {@code Function}, redeclared so that Guava can be shaded.
*/
@FunctionalInterface
public interface ApiFunction<F, T> {
T apply(F input);
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
*/
public class ValidationException extends IllegalArgumentException {

@FunctionalInterface
Copy link
Collaborator

@blakeli0 blakeli0 Mar 28, 2023

Choose a reason for hiding this comment

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

This and above inner-public interface are so weird, probably because we wanted to mimic the same behavior of java.util.function.Supplier in Java 7. That being said, I think adding FunctionalInterface to them are appropriate because they are exposed on the surface through other public methods. But we may want to consider replacing these Supplier with java.util.function.Supplier as part of the Java 8 adoption project. CC: @suztomo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the comments.

I'll do a research on replacing Supplier with java.util.function.Supplier. Leave it as-is as this is out of scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would break binary compatibility, and so we'd need a major rev bump.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would break binary compatibility, and so we'd need a major rev bump.

Are you referring to the idea of replacing Supplier with java.util.function.Supplier? I think it depends on how we approach it, if we do it naively yes it would be a breaking change. We could approach it in steps like adding alternatives first, replacing the ones that are not on the surface first etc.

public interface Supplier<T> {
T get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* A functional interface to be implemented for each request message to extract specific fields from
* it. For advanced usage only.
*/
@FunctionalInterface
public interface FieldsExtractor<RequestT, ParamsT> {
ParamsT extract(RequestT request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,7 @@ public void call(final BidiStreamObserver<RequestT, ResponseT> bidiObserver) {
/** Listens to server responses and send requests when the network is free. */
public void call(
final BidiStreamObserver<RequestT, ResponseT> bidiObserver, ApiCallContext context) {
internalCall(
bidiObserver,
new ClientStreamReadyObserver<RequestT>() {
@Override
public void onReady(ClientStream<RequestT> stream) {
bidiObserver.onReady(stream);
}
},
context);
internalCall(bidiObserver, bidiObserver, context);
Copy link
Member

Choose a reason for hiding this comment

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

Nice.

}

/**
Expand Down Expand Up @@ -183,11 +175,8 @@ public ClientStream<RequestT> splitCall(
ResponseObserver<ResponseT> responseObserver, ApiCallContext context) {
return internalCall(
responseObserver,
new ClientStreamReadyObserver<RequestT>() {
@Override
public void onReady(ClientStream<RequestT> stream) {
// no op
}
stream -> {
// no op
},
context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
package com.google.api.gax.rpc;

/** A callback used to report that the {@link ClientStream} is ready to send more messages. */
@FunctionalInterface
public interface ClientStreamReadyObserver<V> {
void onReady(ClientStream<V> stream);
}