This is an automated email from the ASF dual-hosted git repository.
gyeongtae pushed a commit to branch branch-0.12
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/branch-0.12 by this push:
new 0065ba05d7 [ZEPPELIN-6309] Improve method by replacing JsonObject
parameter
0065ba05d7 is described below
commit 0065ba05d788bd2ef3c5996b94128cdf020e09af
Author: YeonKyung Ryu <[email protected]>
AuthorDate: Mon Sep 15 13:05:15 2025 +0900
[ZEPPELIN-6309] Improve method by replacing JsonObject parameter
### What is this PR for?
Refactored the `convertPermissionsFromUsersToOwners` method in
InterpreterSetting.java to improve code readability and maintainability by
separating JSON parsing logic from business logic, addressing the TODO comment
that identified this as "ugly code".
### What type of PR is it?
Refactoring
### Todos
* [x] - Refactor convertPermissionsFromUsersToOwners method to remove
JsonObject parameter
* [x] - Extract JSON parsing logic into separate static helper method
* [x] - Update all callers to use new method signature
### What is the Jira issue?
[ZEPPELIN-6309](https://issues.apache.org/jira/browse/ZEPPELIN-6309)
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5063 from celinayk/ZEPPELIN-6309.
Signed-off-by: ParkGyeongTae <[email protected]>
(cherry picked from commit 75a9caa7b6731e0c1b36aa4ff245b1e461b02d5c)
Signed-off-by: ParkGyeongTae <[email protected]>
---
.../interpreter/InterpreterInfoSaving.java | 7 ++--
.../zeppelin/interpreter/InterpreterSetting.java | 42 ++++++++++++++--------
.../org/apache/zeppelin/storage/ConfigStorage.java | 9 ++---
3 files changed, 37 insertions(+), 21 deletions(-)
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
index 61b58979db..e0b5567cdf 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
@@ -62,9 +62,10 @@ public class InterpreterInfoSaving implements
JsonSerializable {
if (infoSaving != null && infoSaving.interpreterSettings != null) {
for (InterpreterSetting interpreterSetting :
infoSaving.interpreterSettings.values()) {
- interpreterSetting.convertPermissionsFromUsersToOwners(
- jsonObject.getAsJsonObject("interpreterSettings")
- .getAsJsonObject(interpreterSetting.getId()));
+ JsonObject interpreterSettingJson =
jsonObject.getAsJsonObject("interpreterSettings")
+ .getAsJsonObject(interpreterSetting.getId());
+ List<String> users =
InterpreterSetting.extractUsersFromJsonString(interpreterSettingJson.toString());
+ interpreterSetting.convertPermissionsFromUsersToOwners(users);
}
}
}
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
index 62fcb2dfa8..2adaa55a42 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
@@ -1019,22 +1019,36 @@ public class InterpreterSetting {
t.start();
}
- //TODO(zjffdu) ugly code, should not use JsonObject as parameter. not
readable
- public void convertPermissionsFromUsersToOwners(JsonObject jsonObject) {
- 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());
- }
- }
+ 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);
+ }
+ }
+
+ private static class InterpreterSettingData {
+ private OptionData option;
+
+ private static class OptionData {
+ private List<String> users;
+ }
+ }
+
+ public static List<String> extractUsersFromJsonString(String jsonString) {
+ if (jsonString == null || jsonString.trim().isEmpty()) {
+ return new LinkedList<>();
+ }
+
+ Gson gson = new Gson();
+ InterpreterSettingData data = gson.fromJson(jsonString,
InterpreterSettingData.class);
+
+ if (data != null && data.option != null && data.option.users != null) {
+ return new LinkedList<>(data.option.users);
}
+
+ return new LinkedList<>();
}
// For backward compatibility of interpreter.json format after ZEPPELIN-2403
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
index 8925c42d5c..6c9692dd8f 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
@@ -28,6 +28,7 @@ import org.apache.zeppelin.util.ReflectionUtils;
import java.io.IOException;
+import java.util.List;
/**
* Interface for storing zeppelin configuration.
*
@@ -69,7 +70,6 @@ public abstract class ConfigStorage {
public abstract void saveCredentials(String credentials) throws IOException;
protected InterpreterInfoSaving buildInterpreterInfoSaving(String json) {
- //TODO(zjffdu) This kind of post processing is ugly.
JsonObject jsonObject = JsonParser.parseString(json).getAsJsonObject();
InterpreterInfoSaving infoSaving = InterpreterInfoSaving.fromJson(json);
for (InterpreterSetting interpreterSetting :
infoSaving.interpreterSettings.values()) {
@@ -78,9 +78,10 @@ public abstract class ConfigStorage {
// enable/disable option on GUI).
// previously created setting should turn this feature on here.
interpreterSetting.getOption();
- interpreterSetting.convertPermissionsFromUsersToOwners(
- jsonObject.getAsJsonObject("interpreterSettings")
- .getAsJsonObject(interpreterSetting.getId()));
+ JsonObject interpreterSettingJson =
jsonObject.getAsJsonObject("interpreterSettings")
+ .getAsJsonObject(interpreterSetting.getId());
+ List<String> users =
InterpreterSetting.extractUsersFromJsonString(interpreterSettingJson.toString());
+ interpreterSetting.convertPermissionsFromUsersToOwners(users);
}
return infoSaving;
}
