You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by gi...@git.apache.org on 2017/08/30 22:40:12 UTC

[GitHub] rdhabalia commented on a change in pull request #721: Parse authParamsString on plugin side

rdhabalia commented on a change in pull request #721: Parse authParamsString on plugin side
URL: https://github.com/apache/incubator-pulsar/pull/721#discussion_r136207869
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/api/AuthenticationFactory.java
 ##########
 @@ -41,17 +41,18 @@
      */
     public static final Authentication create(String authPluginClassName, String authParamsString)
             throws UnsupportedAuthenticationException {
-        Map<String, String> authParams = new HashMap<String, String>();
-        if (isNotBlank(authParamsString)) {
-            String[] params = authParamsString.split(",");
-            for (String p : params) {
-                String[] kv = p.split(":");
-                if (kv.length == 2) {
-                    authParams.put(kv[0], kv[1]);
-                }
+        try {
+            if (isNotBlank(authPluginClassName)) {
+                Class<?> authClass = Class.forName(authPluginClassName);
+                Authentication auth = (Authentication) authClass.newInstance();
+                auth.configure(authParamsString);
 
 Review comment:
   a quick thought on implementation:
   Rather adding `void configure(String authParams)` method in `Authentication.java` and doing parsing, instead can we do parsing here in `AuthenticationFactory`?
   
   1. First try to parse given auth-param into json and prepare `Map<String,String>` from json and call configure(map)
   `Map<String, String> authParamsMap = jsonMapper.readValue(authParamsString, new TypeReference<HashMap<String, String>>() {});`
   2. If fails then try to parse in existing way with `:` and `;` delimiters, and prepare config-map 
   
   
   
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services