mattisonchao commented on a change in pull request #14179:
URL: https://github.com/apache/pulsar/pull/14179#discussion_r803203384
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -138,15 +139,26 @@ public void getList(
@ApiResponse(code = 404, message = "tenant/namespace/topic doesn't
exit"),
@ApiResponse(code = 412, message = "Topic name is not valid"),
@ApiResponse(code = 500, message = "Internal server error")})
- public Map<String, Set<AuthAction>> getPermissionsOnTopic(
+ public void getPermissionsOnTopic(
+ @Suspended final AsyncResponse asyncResponse,
@ApiParam(value = "Specify the tenant", required = true)
@PathParam("tenant") String tenant,
@ApiParam(value = "Specify the namespace", required = true)
@PathParam("namespace") String namespace,
@ApiParam(value = "Specify topic name", required = true)
@PathParam("topic") @Encoded String encodedTopic) {
- validateTopicName(tenant, namespace, encodedTopic);
- return internalGetPermissionsOnTopic();
+ try {
+ validateTopicName(tenant, namespace, encodedTopic);
+ internalGetPermissionsOnTopic().thenAccept(permissions ->
asyncResponse.resume(permissions))
+ .exceptionally(ex -> {
+ Throwable realCause =
FutureUtil.unwrapCompletionException(ex);
Review comment:
Same above.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
##########
@@ -105,11 +105,23 @@ public void getList(@Suspended final AsyncResponse
asyncResponse, @PathParam("pr
+ "namespace level combined (union) with any eventual
specific permission set on the topic.")
@ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Namespace doesn't exist")})
- public Map<String, Set<AuthAction>>
getPermissionsOnTopic(@PathParam("property") String property,
- @PathParam("cluster") String cluster, @PathParam("namespace")
String namespace,
- @PathParam("topic") @Encoded String encodedTopic) {
- validateTopicName(property, cluster, namespace, encodedTopic);
- return internalGetPermissionsOnTopic();
+ public void getPermissionsOnTopic(@Suspended AsyncResponse asyncResponse,
+
@PathParam("property") String property,
+
@PathParam("cluster") String cluster,
+
@PathParam("namespace") String namespace,
+
@PathParam("topic") @Encoded String encodedTopic) {
+ try {
+ validateTopicName(property, cluster, namespace, encodedTopic);
+ internalGetPermissionsOnTopic().thenAccept(permissions ->
asyncResponse.resume(permissions))
+ .exceptionally(ex -> {
+ Throwable realCause =
FutureUtil.unwrapCompletionException(ex);
+ log.error("[{}] Failed to get permissions for topic
{}", clientAppId(), topicName, ex);
+ return handleCommonRestAsyncException(asyncResponse,
realCause);
Review comment:
You can use ``resumeAsyncResponseExceptionally`` to instead of
``handleCommonRestAsyncException``. because ``handleCommonRestAsyncException``
will be removed.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
##########
@@ -105,11 +105,23 @@ public void getList(@Suspended final AsyncResponse
asyncResponse, @PathParam("pr
+ "namespace level combined (union) with any eventual
specific permission set on the topic.")
@ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Namespace doesn't exist")})
- public Map<String, Set<AuthAction>>
getPermissionsOnTopic(@PathParam("property") String property,
- @PathParam("cluster") String cluster, @PathParam("namespace")
String namespace,
- @PathParam("topic") @Encoded String encodedTopic) {
- validateTopicName(property, cluster, namespace, encodedTopic);
- return internalGetPermissionsOnTopic();
+ public void getPermissionsOnTopic(@Suspended AsyncResponse asyncResponse,
+
@PathParam("property") String property,
+
@PathParam("cluster") String cluster,
+
@PathParam("namespace") String namespace,
+
@PathParam("topic") @Encoded String encodedTopic) {
+ try {
+ validateTopicName(property, cluster, namespace, encodedTopic);
+ internalGetPermissionsOnTopic().thenAccept(permissions ->
asyncResponse.resume(permissions))
+ .exceptionally(ex -> {
+ Throwable realCause =
FutureUtil.unwrapCompletionException(ex);
Review comment:
If you use ``handleCommonRestAsyncException ``, you don't need to unwrap
the exception, because ``handleCommonRestAsyncException`` already does that.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -193,44 +193,39 @@
return getPartitionedTopicList(TopicDomain.getEnum(domain()));
}
- protected Map<String, Set<AuthAction>> internalGetPermissionsOnTopic() {
+ protected CompletableFuture<Map<String, Set<AuthAction>>>
internalGetPermissionsOnTopic() {
// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
validateAdminAccessForTenant(namespaceName.getTenant());
Review comment:
This method can be async.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
