[ZEPPELIN-6000] Polish some files mainly in zengine (#4731)

* some misc polish * some misc polish
apache · Mar 14, 2024 · 8febca4 · 8febca4
1 parent 0f366d8
commit 8febca4
Show file tree

Hide file tree

Showing 53 changed files with 532 additions and 586 deletions.
diff --git a/zeppelin-integration/src/test/java/org/apache/zeppelin/ProcessData.java b/zeppelin-integration/src/test/java/org/apache/zeppelin/ProcessData.java
@@ -41,7 +41,6 @@ public enum Types_Of_Data {
 
   private Process checked_process;
   private boolean printToConsole = false;
-  private boolean removeRedundantOutput = true;
 
   public ProcessData(Process connected_process, boolean printToConsole, int silenceTimeout, TimeUnit timeUnit) {
     this.checked_process = connected_process;

diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/ClusterManager.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/ClusterManager.java
@@ -131,7 +131,7 @@
 public abstract class ClusterManager {
   private static final Logger LOGGER = LoggerFactory.getLogger(ClusterManager.class);
 
-  public ZeppelinConfiguration zConf;
+  protected final ZeppelinConfiguration zConf;
 
   protected Collection<Node> clusterNodes = new ArrayList<>();
 
@@ -158,8 +158,8 @@ public abstract class ClusterManager {
   protected boolean isTest = false;
 
   public ClusterManager(ZeppelinConfiguration zConf) {
+    this.zConf = zConf;
     try {
-      this.zConf = zConf;
       zeplServerHost = RemoteInterpreterUtils.findAvailableHostAddress();
       String clusterAddr = this.zConf.getClusterAddress();
       if (!StringUtils.isEmpty(clusterAddr)) {

diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/helium/Application.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/helium/Application.java
@@ -31,7 +31,7 @@ public abstract class Application {
 
   private final ApplicationContext context;
 
-  public Application(ApplicationContext context) {
+  protected Application(ApplicationContext context) {
     this.context = context;
   }
 
@@ -129,7 +129,7 @@ public void printStringAsJavascript(String js) throws IOException {
   }
 
   private void beginJavascript() throws IOException {
-    StringBuffer js = new StringBuffer();
+    StringBuilder js = new StringBuilder();
     js.append("\n<script id=\"app_js_" + js.hashCode() + "\">\n");
     js.append("(function() {\n");
     js.append("let $z = {\n");
@@ -142,7 +142,7 @@ private void beginJavascript() throws IOException {
   }
 
   private void endJavascript() throws IOException {
-    StringBuffer js = new StringBuffer();
+    StringBuilder js = new StringBuilder();
     js.append("\n})();\n");
     js.append("</script>\n");
     context.out.write(js.toString());

diff --git a/...rc/test/java/org/apache/zeppelin/interpreter/launcher/ClusterInterpreterLauncherTest.java b/...rc/test/java/org/apache/zeppelin/interpreter/launcher/ClusterInterpreterLauncherTest.java
@@ -23,18 +23,14 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
 import java.util.Properties;
 
-public class ClusterInterpreterLauncherTest extends ClusterMockTest {
-  private static final Logger LOGGER =
-      LoggerFactory.getLogger(ClusterInterpreterLauncherTest.class);
+class ClusterInterpreterLauncherTest extends ClusterMockTest {
 
   @BeforeAll
   static void startTest() throws IOException, InterruptedException {
@@ -77,6 +73,7 @@ void testConnectExistOnlineIntpProcess() throws IOException {
     assertEquals("127.0.0.1", interpreterProcess.getHost());
     assertEquals("name", interpreterProcess.getInterpreterSettingName());
     assertEquals(5000, interpreterProcess.getConnectTimeout());
+    interpreterProcess.close();
   }
 
   @Test
@@ -104,6 +101,7 @@ void testConnectExistOfflineIntpProcess() throws IOException {
     assertEquals(zconf.getInterpreterRemoteRunnerPath(), interpreterProcess.getInterpreterRunner());
     assertTrue(interpreterProcess.getEnv().size() >= 1);
     assertEquals(true, interpreterProcess.isUserImpersonated());
+    interpreterProcess.close();
   }
 
   @Test

diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumBundleFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumBundleFactory.java
@@ -288,15 +288,17 @@ public boolean accept(File pathname) {
       // if online package
       String version = nameAndVersion[1];
       File tgz = new File(heliumLocalRepoDirectory, pkg.getName() + "-" + version + ".tgz");
-      tgz.delete();
+      FileUtils.deleteQuietly(tgz);
 
       // wget, extract and move dir to `bundles/${pkg.getName()}`, and remove tgz
       npmCommand(fpf, "pack " + pkg.getArtifact());
       File extracted = new File(heliumBundleDirectory, "package");
       FileUtils.deleteDirectory(extracted);
       List<String> entries = unTgz(tgz, heliumBundleDirectory);
-      for (String entry: entries) LOGGER.debug("Extracted " + entry);
-      tgz.delete();
+      for (String entry : entries) {
+        LOGGER.debug("Extracted {}", entry);
+      }
+      FileUtils.deleteQuietly(tgz);
       FileUtils.copyDirectory(extracted, bundleDir);
       FileUtils.deleteDirectory(extracted);
     }

diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java
@@ -100,7 +100,9 @@ public synchronized List<HeliumPackage> getAll() throws IOException {
 
     if (response.getStatusLine().getStatusCode() != 200) {
       // try read from cache
-      logger.error(uri() + " returned " + response.getStatusLine().toString());
+      if (logger.isErrorEnabled()) {
+        logger.error("{} returned {}", uri(), response.getStatusLine());
+      }
       return readFromCache();
     } else {
       List<HeliumPackage> packageList = new LinkedList<>();

diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumRegistry.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumRegistry.java
@@ -26,7 +26,7 @@ public abstract class HeliumRegistry {
   private final String name;
   private final String uri;
 
-  public HeliumRegistry(String name, String uri) {
+  protected HeliumRegistry(String name, String uri) {
     this.name = name;
     this.uri = uri;
   }

diff --git a/...-zengine/src/main/java/org/apache/zeppelin/notebook/exception/CorruptedNoteException.java b/...-zengine/src/main/java/org/apache/zeppelin/notebook/exception/CorruptedNoteException.java
@@ -20,7 +20,10 @@
 import java.io.IOException;
 
 public class CorruptedNoteException extends IOException {
-    public CorruptedNoteException(final String noteId, final String message, Exception e) {
-        super(String.format("noteId: %s - %s", noteId, message), e);
-    }
+
+  private static final long serialVersionUID = 1743308058186542714L;
+
+  public CorruptedNoteException(final String noteId, final String message, Exception e) {
+    super(String.format("noteId: %s - %s", noteId, message), e);
+  }
 }
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java
@@ -23,14 +23,15 @@
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.user.AuthenticationInfo;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 
 /**
  * Notebook repository (persistence layer) abstraction.
  */
-public interface NotebookRepo {
+public interface NotebookRepo extends Closeable {
 
   void init(ZeppelinConfiguration zConf) throws IOException;
 
@@ -115,6 +116,7 @@ void move(String folderPath, String newFolderPath,
   /**
    * Release any underlying resources
    */
+  @Override
   @ZeppelinApi
   void close();
 

diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/search/SearchService.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/search/SearchService.java
@@ -31,7 +31,7 @@
  */
 public abstract class SearchService extends NoteEventAsyncListener {
 
-  public SearchService(String name) {
+  protected SearchService(String name) {
     super(name);
   }
 

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
@@ -63,7 +63,7 @@ private static ConfigStorage createConfigStorage(ZeppelinConfiguration zConf) th
   }
 
 
-  public ConfigStorage(ZeppelinConfiguration zConf) {
+  protected ConfigStorage(ZeppelinConfiguration zConf) {
     this.zConf = zConf;
   }
 

diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java
@@ -29,7 +29,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class ZeppelinConfigurationTest {
+class ZeppelinConfigurationTest {
   @BeforeAll
   public static void clearSystemVariables() {
     ZeppelinConfiguration.reset();
@@ -41,7 +41,7 @@ public void cleanup() {
   }
 
   @Test
-  public void getAllowedOrigins2Test() throws MalformedURLException {
+  void getAllowedOrigins2Test() throws MalformedURLException {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("test-zeppelin-site2.xml");
     List<String> origins = conf.getAllowedOrigins();
@@ -51,7 +51,7 @@ public void getAllowedOrigins2Test() throws MalformedURLException {
   }
 
   @Test
-  public void getAllowedOrigins1Test() throws MalformedURLException {
+  void getAllowedOrigins1Test() throws MalformedURLException {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("test-zeppelin-site1.xml");
     List<String> origins = conf.getAllowedOrigins();
@@ -60,78 +60,78 @@ public void getAllowedOrigins1Test() throws MalformedURLException {
   }
 
   @Test
-  public void getAllowedOriginsNoneTest() throws MalformedURLException {
+  void getAllowedOriginsNoneTest() throws MalformedURLException {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     List<String> origins = conf.getAllowedOrigins();
     assertEquals(1, origins.size());
   }
 
   @Test
-  public void isWindowsPathTestTrue() {
+  void isWindowsPathTestTrue() {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     Boolean isIt = conf.isWindowsPath("c:\\test\\file.txt");
     assertTrue(isIt);
   }
 
   @Test
-  public void isWindowsPathTestFalse() {
+  void isWindowsPathTestFalse() {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     Boolean isIt = conf.isWindowsPath("~/test/file.xml");
     assertFalse(isIt);
   }
 
   @Test
-  public void isPathWithSchemeTestTrue() {
+  void isPathWithSchemeTestTrue() {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     Boolean isIt = conf.isPathWithScheme("hdfs://hadoop.example.com/zeppelin/notebook");
     assertTrue(isIt);
   }
 
   @Test
-  public void isPathWithSchemeTestFalse() {
+  void isPathWithSchemeTestFalse() {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     Boolean isIt = conf.isPathWithScheme("~/test/file.xml");
     assertFalse(isIt);
   }
 
   @Test
-  public void isPathWithInvalidSchemeTest() {
+  void isPathWithInvalidSchemeTest() {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     Boolean isIt = conf.isPathWithScheme("c:\\test\\file.txt");
     assertFalse(isIt);
   }
 
   @Test
-  public void getNotebookDirTest() {
+  void getNotebookDirTest() {
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     String notebookLocation = conf.getNotebookDir();
     assertTrue(notebookLocation.endsWith("notebook"));
   }
 
   @Test
-  public void isNotebookPublicTest() {
+  void isNotebookPublicTest() {
 
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     boolean isIt = conf.isNotebookPublic();
     assertTrue(isIt);
   }
 
   @Test
-  public void getPathTest() {
+  void getPathTest() {
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     conf.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), "/usr/lib/zeppelin");
     assertEquals("/usr/lib/zeppelin", conf.getZeppelinHome());
     assertEquals("/usr/lib/zeppelin/conf", conf.getConfDir());
   }
 
   @Test
-  public void getConfigFSPath() {
+  void getConfigFSPath() {
     ZeppelinConfiguration conf = ZeppelinConfiguration.create("zeppelin-test-site.xml");
     conf.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), "/usr/lib/zeppelin");
     conf.setProperty(ConfVars.ZEPPELIN_CONFIG_FS_DIR.getVarName(), "conf");

diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
@@ -43,7 +43,7 @@
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
-public class HeliumApplicationFactoryTest extends AbstractInterpreterTest {
+class HeliumApplicationFactoryTest extends AbstractInterpreterTest {
 
   private NotebookRepo notebookRepo;
   private Notebook notebook;
@@ -88,7 +88,7 @@ public void tearDown() throws Exception {
 
   @Test
   @Disabled
-  public void testLoadRunUnloadApplication()
+  void testLoadRunUnloadApplication()
       throws IOException, ApplicationException, InterruptedException {
     // given
     HeliumPackage pkg1 = newHeliumPackage(HeliumType.APPLICATION,
@@ -138,7 +138,7 @@ public void testLoadRunUnloadApplication()
 
   @Test
   @Disabled
-  public void testUnloadOnParagraphRemove() throws IOException {
+  void testUnloadOnParagraphRemove() throws IOException {
     // given
     HeliumPackage pkg1 = newHeliumPackage(HeliumType.APPLICATION,
         "name1",
@@ -181,7 +181,7 @@ public void testUnloadOnParagraphRemove() throws IOException {
 
   @Test
   @Disabled
-  public void testUnloadOnInterpreterUnbind() throws IOException {
+  void testUnloadOnInterpreterUnbind() throws IOException {
     // given
     HeliumPackage pkg1 = newHeliumPackage(HeliumType.APPLICATION,
         "name1",
@@ -221,7 +221,7 @@ public void testUnloadOnInterpreterUnbind() throws IOException {
 
   @Test
   @Disabled
-  public void testInterpreterUnbindOfNullReplParagraph() throws IOException {
+  void testInterpreterUnbindOfNullReplParagraph() throws IOException {
     // create note
     String note1Id = notebook.createNote("note1", anonymous);
     Note note1 = notebook.processNote(note1Id,
@@ -248,7 +248,7 @@ public void testInterpreterUnbindOfNullReplParagraph() throws IOException {
 
   @Test
   @Disabled
-  public void testUnloadOnInterpreterRestart() throws IOException, InterpreterException {
+  void testUnloadOnInterpreterRestart() throws IOException, InterpreterException {
     // given
     HeliumPackage pkg1 = newHeliumPackage(HeliumType.APPLICATION,
         "name1",