core: add accessor for bare method name in MethodDescriptor #7339
Conversation
|
|
| @@ -225,6 +226,7 @@ private MethodDescriptor( | |||
| this.type = Preconditions.checkNotNull(type, "type"); | |||
| this.fullMethodName = Preconditions.checkNotNull(fullMethodName, "fullMethodName"); | |||
| this.serviceName = extractFullServiceName(fullMethodName); | |||
| this.methodName = extractMethodName(fullMethodName); | |||
There was a problem hiding this comment.
I don't think we should pre-compute this.
There was a problem hiding this comment.
Does this really matter? This should be a fairly cheap operation.
There was a problem hiding this comment.
If it's so cheap we shouldn't mind doing it every time. Really, I don't think we want the startup initialization overhead and memory usage for something that may never be used (consider Android for example).
There was a problem hiding this comment.
My concern is that then every caller will need to cache it, but alas. Done.
| */ | ||
| @Nullable | ||
| @ExperimentalApi("https://github.com/grpc/grpc-java/issues/5635") | ||
| public String getMethodName() { |
There was a problem hiding this comment.
We should add something to the name to make users think whether they want the full name or this one. "simple" or "bare" or similar. Ditto for extract method.
I'm not sure if we have any such term today, as method name is normally fully qualified. Maybe another language has such a thing?
There was a problem hiding this comment.
I am not aware of another language having solved this properly. The naming confusion is very real. Generally the full(y qualified) method name gets its own name (also here: getFullMethodName()) and the other one is just MethodName. But from the options I picked Bare for now. Let me know what you think.
| * @since 1.32.0 | ||
| */ | ||
| @Nullable | ||
| public static String extractMethodName(String fullMethodName) { |
Added getBareMethodName and extractBareMethodName for accessing the unqualified method name.
To implement credential restrictions based on not only service names but also (non-qualified) method names, we need to expose this property just like the service name got exposed through the API.
This reuses the same experimental tracking issue as getServiceName, but I can to file a new one if necessary.