Copilot commented on code in PR #24944:
URL: https://github.com/apache/pulsar/pull/24944#discussion_r2496059902
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/oauth2/FlowBase.java:
##########
@@ -77,4 +126,23 @@ static URL parseParameterUrl(Map<String, String> params,
String name) {
throw new IllegalArgumentException("Malformed configuration
parameter: " + name);
}
}
+
+ static Integer parseParameterInt(Map<String, String> params, String name) {
+ String value = params.get(name);
+ if (StringUtils.isNotBlank(value)) {
+ try {
+ return Integer.parseInt(value);
+ } catch (NumberFormatException numberFormatException) {
+ throw new IllegalArgumentException("Malformed configuration
parameter: " + name);
+ }
+ }
+ return null;
+ }
+
+ @Override
+ public void close() throws Exception {
+ if (httpClient != null) {
+ httpClient.close();
+ }
Review Comment:
The null check for httpClient in close() is inconsistent with
TokenClient.close() which doesn't have such a check. Since httpClient is
initialized in the constructor and should never be null, this defensive check
suggests uncertainty about the contract. Consider removing it for consistency
or document why it's needed here but not in TokenClient.
```suggestion
httpClient.close();
```
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/oauth2/protocol/DefaultMetadataResolver.java:
##########
@@ -109,4 +71,30 @@ public static URL getWellKnownMetadataUrl(URL issuerUrl) {
throw new IllegalArgumentException(e);
}
}
+
+ /**
+ * Resolves the authorization metadata.
+ *
+ * @return metadata
+ * @throws IOException if the metadata could not be resolved.
+ */
+ public Metadata resolve() throws IOException {
+
+ try {
+ Response response = httpClient.prepareGet(metadataUrl.toString())
+ .addHeader(HttpHeaderNames.ACCEPT,
HttpHeaderValues.APPLICATION_JSON)
+ .execute()
+ .toCompletableFuture()
+ .get();
+
+ Metadata metadata;
+ try (InputStream inputStream = response.getResponseBodyAsStream())
{
+ metadata = this.objectReader.readValue(inputStream);
+ }
+ return metadata;
+
+ } catch (IOException | InterruptedException | ExecutionException e) {
Review Comment:
InterruptedException is caught but the thread's interrupted status is not
restored. When catching InterruptedException, you should either rethrow it,
wrap it in an unchecked exception, or restore the interrupted status with
Thread.currentThread().interrupt() before throwing the IOException.
```suggestion
} catch (IOException | InterruptedException | ExecutionException e) {
if (e instanceof InterruptedException) {
Thread.currentThread().interrupt();
}
```
--
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]
