[GitHub] [kafka] Fleshgrinder commented on a change in pull request #9374: MINOR: Fix NPE in KafkaAdminClient.describeUserScramCredentials

[GitBox]

Fleshgrinder commented on a change in pull request #9374:
URL: https://github.com/apache/kafka/pull/9374#discussion_r511774755



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -4166,10 +4167,22 @@ public DescribeUserScramCredentialsResult 
describeUserScramCredentials(List<Stri
         Call call = new Call("describeUserScramCredentials", 
calcDeadlineMs(now, options.timeoutMs()),
                 new LeastLoadedNodeProvider()) {
             @Override
-            public DescribeUserScramCredentialsRequest.Builder 
createRequest(int timeoutMs) {
-                return new DescribeUserScramCredentialsRequest.Builder(
-                        new 
DescribeUserScramCredentialsRequestData().setUsers(users.stream().map(user ->
-                                new 
DescribeUserScramCredentialsRequestData.UserName().setName(user)).collect(Collectors.toList())));
+            public DescribeUserScramCredentialsRequest.Builder 
createRequest(final int timeoutMs) {
+                final DescribeUserScramCredentialsRequestData requestData = 
new DescribeUserScramCredentialsRequestData();
+
+                if (users != null && !users.isEmpty()) {
+                    final List<UserName> userNames = new 
ArrayList<>(users.size());
+
+                    for (final String user : users) {
+                        if (user != null) {
+                            userNames.add(new UserName().setName(user));
+                        }
+                    }
+
+                    requestData.setUsers(userNames);

Review comment:
       Not really, if we keep all checks in place it will only be a little 
shorter but definitely less efficient:
   
   ```java
   final DescribeUserScramCredentialsRequestData requestData = new 
DescribeUserScramCredentialsRequestData();
   
   if (users != null && !users.isEmpty()) {
       requestData.setUsers(users.stream()
           .filter(Objects::nonNull)
           .map(it -> new 
DescribeUserScramCredentialsRequestData.UserName().setName(it))
           .collect(Collectors.toList())
       );
   }
   
   return new DescribeUserScramCredentialsRequest.Builder(requestData);
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org