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