Re: [PR] KAFKA-16629: Add broker-related tests to ConfigCommandIntegrationTest [kafka]

[via GitHub]

m1a2st commented on code in PR #15840:
URL: https://github.com/apache/kafka/pull/15840#discussion_r1616061164


##########
core/src/main/scala/kafka/admin/ConfigCommand.scala:
##########
@@ -210,15 +210,19 @@ object ConfigCommand extends Logging {
     }
   }
 
-  def createPasswordEncoder(encoderConfigs: Map[String, String]): 
PasswordEncoder = {
+  def createPasswordEncoder(encoderConfigs: java.util.Map[String, String]): 
PasswordEncoder = {
     encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_SECRET_CONFIG)
-    val encoderSecret = 
encoderConfigs.getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_SECRET_CONFIG,
-      throw new IllegalArgumentException("Password encoder secret not 
specified"))
+    val encoderSecret = 
Optional.ofNullable(encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_SECRET_CONFIG))
+      .orElseThrow(() => new IllegalArgumentException("Password encoder secret 
not specified"))
     PasswordEncoder.encrypting(new Password(encoderSecret),
       null,
-      
encoderConfigs.getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_CONFIG,
 PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_DEFAULT),
-      
encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_CONFIG).map(_.toInt).getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_DEFAULT),
-      
encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_ITERATIONS_CONFIG).map(_.toInt).getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_ITERATIONS_DEFAULT))
+      
encoderConfigs.getOrDefault(PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_CONFIG,
 PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_DEFAULT),
+      
Optional.ofNullable(encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_CONFIG))
+        .map(Integer.parseInt)
+        .orElse(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_DEFAULT),
+      
Optional.ofNullable(Integer.parseInt(encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_ITERATIONS_CONFIG)))

Review Comment:
   No, I don't think so. If I use main function to test, It won't NPE.
   ![CleanShot 2024-05-27 at 21 31 
37@2x](https://github.com/apache/kafka/assets/100591800/c4992264-cc8c-400e-93ae-b89ac7ae6294)
   Sorry. I didn't see the last line.



##########
core/src/main/scala/kafka/admin/ConfigCommand.scala:
##########
@@ -210,15 +210,19 @@ object ConfigCommand extends Logging {
     }
   }
 
-  def createPasswordEncoder(encoderConfigs: Map[String, String]): 
PasswordEncoder = {
+  def createPasswordEncoder(encoderConfigs: java.util.Map[String, String]): 
PasswordEncoder = {
     encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_SECRET_CONFIG)
-    val encoderSecret = 
encoderConfigs.getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_SECRET_CONFIG,
-      throw new IllegalArgumentException("Password encoder secret not 
specified"))
+    val encoderSecret = 
Optional.ofNullable(encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_SECRET_CONFIG))
+      .orElseThrow(() => new IllegalArgumentException("Password encoder secret 
not specified"))
     PasswordEncoder.encrypting(new Password(encoderSecret),
       null,
-      
encoderConfigs.getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_CONFIG,
 PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_DEFAULT),
-      
encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_CONFIG).map(_.toInt).getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_DEFAULT),
-      
encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_ITERATIONS_CONFIG).map(_.toInt).getOrElse(PasswordEncoderConfigs.PASSWORD_ENCODER_ITERATIONS_DEFAULT))
+      
encoderConfigs.getOrDefault(PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_CONFIG,
 PasswordEncoderConfigs.PASSWORD_ENCODER_CIPHER_ALGORITHM_DEFAULT),
+      
Optional.ofNullable(encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_CONFIG))
+        .map(Integer.parseInt)
+        .orElse(PasswordEncoderConfigs.PASSWORD_ENCODER_KEY_LENGTH_DEFAULT),
+      
Optional.ofNullable(Integer.parseInt(encoderConfigs.get(PasswordEncoderConfigs.PASSWORD_ENCODER_ITERATIONS_CONFIG)))

Review Comment:
   No, I don't think so. If I use main function to test, It won't NPE.
   ![CleanShot 2024-05-27 at 21 31 
37@2x](https://github.com/apache/kafka/assets/100591800/c4992264-cc8c-400e-93ae-b89ac7ae6294)
   Sorry. I didn't see the last line. You are right!



-- 
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: jira-unsubscr...@kafka.apache.org

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