celinayk commented on code in PR #5063:
URL: https://github.com/apache/zeppelin/pull/5063#discussion_r2315554125


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java:
##########
@@ -1016,22 +1016,29 @@ public void run() {
     t.start();
   }
 
-  //TODO(zjffdu) ugly code, should not use JsonObject as parameter. not 
readable
-  public void convertPermissionsFromUsersToOwners(JsonObject jsonObject) {
+  public void convertPermissionsFromUsersToOwners(List<String> users) {
+    if (users != null && !users.isEmpty()) {
+      if (this.option.getOwners() == null) {
+        this.option.owners = new LinkedList<>();
+      }
+      this.option.getOwners().addAll(users);
+    }
+  }
+
+  public static List<String> extractUsersFromJsonObject(JsonObject jsonObject) 
{
+    List<String> users = new ArrayList<>();
     if (jsonObject != null) {
       JsonObject option = jsonObject.getAsJsonObject("option");
       if (option != null) {
-        JsonArray users = option.getAsJsonArray("users");
-        if (users != null) {
-          if (this.option.getOwners() == null) {
-            this.option.owners = new LinkedList<>();
-          }
-          for (JsonElement user : users) {
-            this.option.getOwners().add(user.getAsString());
+        JsonArray usersArray = option.getAsJsonArray("users");
+        if (usersArray != null) {
+          for (JsonElement userElement : usersArray) {
+            users.add(userElement.getAsString());
           }
         }
       }
     }
+    return users;

Review Comment:
   Good catch! Updated to use LinkedList for consistency with the rest of the 
class. This maintains consistency with the owners field initialization on line.



-- 
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: reviews-unsubscr...@zeppelin.apache.org

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

Reply via email to