Skip to content

Commit

Permalink
[ZEPPELIN-5130] Improve Code
Browse files Browse the repository at this point in the history 
### What is this PR for?
Improve the code during development.

### What type of PR is it?
 - Refactoring

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5130

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Philipp Dallig <[email protected]>

Closes apache#4000 from Reamer/cleanup2 and squashes the following commits:

64af5ae [Philipp Dallig] cleanup
da66de0 [Philipp Dallig] Use commons.lang3
  • Loading branch information
@Reamer
Reamer committed Jan 4, 2021
1 parent 8e3bf30 commit 136140c
Show file tree
Hide file tree
Showing 20 changed files with 120 additions and 124 deletions.
11 changes: 6 additions & 5 deletions zeppelin-common/src/main/java/org/apache/zeppelin/common/Message.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class Message implements JsonSerializable {
/**
* Representation of event type.
*/
public static enum OP {
public enum OP {
GET_HOME_NOTE, // [c-s] load note for home screen

GET_NOTE, // [c-s] client load note
Expand Down Expand Up @@ -138,7 +138,7 @@ public static enum OP {

ANGULAR_OBJECT_UPDATE, // [s-c] add/update angular object
ANGULAR_OBJECT_REMOVE, // [s-c] add angular object del

ANGULAR_OBJECT_UPDATED, // [c-s] angular object value updated,

ANGULAR_OBJECT_CLIENT_BIND, // [c-s] angular object updated from AngularJS z object
Expand Down Expand Up @@ -220,7 +220,7 @@ public static enum OP {

private static final Gson GSON = new Gson();
public static final Message EMPTY = new Message(null);

public OP op;
public Map<String, Object> data = new HashMap<>();
public String ticket = "anonymous";
Expand Down Expand Up @@ -260,11 +260,11 @@ public <T> T getType(String key) {
return (T) data.get(key);
}

public <T> T getType(String key, Logger LOG) {
public <T> T getType(String key, Logger log) {
try {
return getType(key);
} catch (ClassCastException e) {
LOG.error("Failed to get " + key + " from message (Invalid type). " , e);
log.error("Failed to get {} from message (Invalid type). ", key , e);
return null;
}
}
Expand All @@ -278,6 +278,7 @@ public String toString() {
return sb.toString();
}

@Override
public String toJson() {
return GSON.toJson(this);
}
Expand Down
5 changes: 3 additions & 2 deletions ...notebookrepo/azure/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import java.util.List;
import java.util.Map;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.NoteInfo;
Expand All @@ -56,6 +56,7 @@ public AzureNotebookRepo() {

}

@Override
public void init(ZeppelinConfiguration conf) throws IOException {
this.conf = conf;
user = conf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_AZURE_USER);
Expand Down Expand Up @@ -102,7 +103,7 @@ private Map<String, NoteInfo> list(CloudFileDirectory folder) throws IOException
LOGGER.warn(e.getMessage());
}
} else {
LOGGER.debug("Skip invalid note file: " + file.getUri().getPath());
LOGGER.debug("Skip invalid note file: {}", file.getUri().getPath());
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...ebookrepo/azure/src/main/java/org/apache/zeppelin/notebook/repo/OldAzureNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@
import com.microsoft.azure.storage.file.CloudFileShare;
import com.microsoft.azure.storage.file.ListFileItem;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.NoteInfo;
import org.apache.zeppelin.notebook.OldNoteInfo;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.slf4j.Logger;
Expand Down Expand Up @@ -60,6 +59,7 @@ public OldAzureNotebookRepo() {

}

@Override
public void init(ZeppelinConfiguration conf) throws IOException {
this.conf = conf;
user = conf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_AZURE_USER);
Expand Down
9 changes: 5 additions & 4 deletions ...po/filesystem/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ public FileSystemNotebookRepo() {

}

@Override
public void init(ZeppelinConfiguration zConf) throws IOException {
this.fs = new FileSystemStorage(zConf, zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_DIR));
LOGGER.info("Creating FileSystem: " + this.fs.getFs().getClass().getName());
LOGGER.info("Creating FileSystem: {}", this.fs.getFs().getClass().getName());
this.notebookDir = this.fs.makeQualified(new Path(zConf.getNotebookDir()));
LOGGER.info("Using folder {} to store notebook", notebookDir);
this.fs.tryMkDir(notebookDir);
Expand All @@ -63,7 +64,7 @@ public Map<String, NoteInfo> list(AuthenticationInfo subject) throws IOException
getNotePath(notebookDir.toString(), path.toString()));
noteInfos.put(noteInfo.getId(), noteInfo);
} catch (IOException e) {
LOGGER.warn("Fail to get NoteInfo for note: " + path.getName(), e);
LOGGER.warn("Fail to get NoteInfo for note: {}", path.getName(), e);
}
}
return noteInfos;
Expand Down Expand Up @@ -108,14 +109,14 @@ public void move(String folderPath, String newFolderPath, AuthenticationInfo sub
public void remove(String noteId, String notePath, AuthenticationInfo subject)
throws IOException {
if (!this.fs.delete(new Path(notebookDir.toString(), buildNoteFileName(noteId, notePath)))) {
LOGGER.warn("Fail to move note, noteId: " + notePath + ", notePath: " + notePath);
LOGGER.warn("Fail to move note, noteId: {}, notePath: {}", notePath, notePath);
}
}

@Override
public void remove(String folderPath, AuthenticationInfo subject) throws IOException {
if (!this.fs.delete(new Path(notebookDir, folderPath.substring(1)))) {
LOGGER.warn("Fail to remove folder: " + folderPath);
LOGGER.warn("Fail to remove folder: {}", folderPath);
}
}

Expand Down
5 changes: 2 additions & 3 deletions ...filesystem/src/main/java/org/apache/zeppelin/notebook/repo/OldFileSystemNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.notebook.FileSystemStorage;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.NoteInfo;
import org.apache.zeppelin.notebook.OldNoteInfo;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.slf4j.Logger;
Expand Down Expand Up @@ -34,11 +33,11 @@ public OldFileSystemNotebookRepo() {

}

@Override
public void init(ZeppelinConfiguration zConf) throws IOException {
this.fs = new FileSystemStorage(zConf,
zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_DIR));
LOGGER.info("Creating FileSystem: " + this.fs.getFs().getClass().getName() +
" for Zeppelin Notebook.");
LOGGER.info("Creating FileSystem: {} for Zeppelin Notebook.", this.fs.getFs().getClass().getName());
this.notebookDir = this.fs.makeQualified(new Path(zConf.getNotebookDir()));
LOGGER.info("Using folder {} to store notebook", notebookDir);
this.fs.tryMkDir(notebookDir);
Expand Down
14 changes: 7 additions & 7 deletions ...ins/notebookrepo/gcs/src/main/java/org/apache/zeppelin/notebook/repo/GCSNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@
import com.google.cloud.storage.StorageException;
import com.google.cloud.storage.StorageOptions;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.gson.JsonParseException;

import java.io.FileInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
import org.apache.zeppelin.notebook.Note;
Expand Down Expand Up @@ -98,7 +98,7 @@ public void init(ZeppelinConfiguration zConf) throws IOException {

// pathComponents excludes empty string if trailing slash is present
List<String> pathComponents = Arrays.asList(storageDirWithoutScheme.split("/"));
if (pathComponents.size() < 1) {
if (pathComponents.isEmpty()) {
throw new IOException(String.format(
"GCS storage directory '%s' must be in the form gs://bucketname/path/to/dir",
gcsStorageDir));
Expand All @@ -108,7 +108,7 @@ public void init(ZeppelinConfiguration zConf) throws IOException {
this.basePath = Optional.of(StringUtils.join(
pathComponents.subList(1, pathComponents.size()), "/"));
} else {
this.basePath = Optional.absent();
this.basePath = Optional.empty();
}

// Notes are stored at gs://bucketName/basePath/<note-id>/note.json
Expand Down Expand Up @@ -194,7 +194,7 @@ public void save(Note note, AuthenticationInfo subject) throws IOException {
.setContentType("application/json")
.build();
try {
storage.create(info, note.toJson().getBytes("UTF-8"));
storage.create(info, note.toJson().getBytes(StandardCharsets.UTF_8));
} catch (StorageException se) {
throw new IOException("Could not write " + info.toString() + ": " + se.getMessage(), se);
}
Expand All @@ -212,7 +212,7 @@ public void move(String folderPath, String newFolderPath, AuthenticationInfo sub

@Override
public void remove(String noteId, String notePath, AuthenticationInfo subject) throws IOException {
Preconditions.checkArgument(!Strings.isNullOrEmpty(noteId));
Preconditions.checkArgument(StringUtils.isNotEmpty(noteId));
BlobId blobId = makeBlobId(noteId, notePath);
try {
boolean deleted = storage.delete(blobId);
Expand Down
17 changes: 8 additions & 9 deletions .../notebookrepo/gcs/src/main/java/org/apache/zeppelin/notebook/repo/OldGCSNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,26 @@
import com.google.cloud.storage.StorageException;
import com.google.cloud.storage.StorageOptions;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.gson.JsonParseException;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.NoteInfo;
import org.apache.zeppelin.notebook.OldNoteInfo;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.FileInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -67,7 +66,7 @@
*/
public class OldGCSNotebookRepo implements OldNotebookRepo {

private static final Logger LOG = LoggerFactory.getLogger(GCSNotebookRepo.class);
private static final Logger LOG = LoggerFactory.getLogger(OldGCSNotebookRepo.class);
private String encoding;
private String bucketName;
private Optional<String> basePath;
Expand Down Expand Up @@ -99,7 +98,7 @@ public void init(ZeppelinConfiguration zConf) throws IOException {

// pathComponents excludes empty string if trailing slash is present
List<String> pathComponents = Arrays.asList(storageDirWithoutScheme.split("/"));
if (pathComponents.size() < 1) {
if (pathComponents.isEmpty()) {
throw new IOException(String.format(
"GCS storage directory '%s' must be in the form gs://bucketname/path/to/dir",
gcsStorageDir));
Expand All @@ -109,7 +108,7 @@ public void init(ZeppelinConfiguration zConf) throws IOException {
this.basePath = Optional.of(StringUtils.join(
pathComponents.subList(1, pathComponents.size()), "/"));
} else {
this.basePath = Optional.absent();
this.basePath = Optional.empty();
}

// Notes are stored at gs://bucketName/basePath/<note-id>/note.json
Expand Down Expand Up @@ -189,15 +188,15 @@ public void save(Note note, AuthenticationInfo subject) throws IOException {
.setContentType("application/json")
.build();
try {
storage.create(info, note.toJson().getBytes("UTF-8"));
storage.create(info, note.toJson().getBytes(StandardCharsets.UTF_8));
} catch (StorageException se) {
throw new IOException("Could not write " + info.toString() + ": " + se.getMessage(), se);
}
}

@Override
public void remove(String noteId, AuthenticationInfo subject) throws IOException {
Preconditions.checkArgument(!Strings.isNullOrEmpty(noteId));
Preconditions.checkArgument(StringUtils.isNotEmpty(noteId));
BlobId blobId = makeBlobId(noteId);
try {
boolean deleted = storage.delete(blobId);
Expand Down
12 changes: 7 additions & 5 deletions ...ins/notebookrepo/oss/src/main/java/org/apache/zeppelin/notebook/repo/OSSNotebookRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -57,6 +58,7 @@ public class OSSNotebookRepo implements NotebookRepo {
public OSSNotebookRepo() {
}

@Override
public void init(ZeppelinConfiguration conf) throws IOException {
String endpoint = conf.getOSSEndpoint();
bucketName = conf.getOSSBucketName();
Expand Down Expand Up @@ -93,7 +95,7 @@ public Map<String, NoteInfo> list(AuthenticationInfo subject) throws IOException
LOGGER.warn(e.getMessage());
}
} else {
LOGGER.debug("Skip invalid note file: " + s.getKey());
LOGGER.debug("Skip invalid note file: {}", s.getKey());
}
}
nextMarker = objectListing.getNextMarker();
Expand All @@ -109,7 +111,7 @@ public Note get(String noteId, String notePath, AuthenticationInfo subject) thro
InputStream in = null;
try {
in = ossObject.getObjectContent();
return Note.fromJson(IOUtils.toString(in));
return Note.fromJson(IOUtils.toString(in, StandardCharsets.UTF_8));
} finally {
if (in != null) {
in.close();
Expand Down Expand Up @@ -160,7 +162,7 @@ public void move(String folderPath, String newFolderPath, AuthenticationInfo sub
LOGGER.warn(e.getMessage());
}
} else {
LOGGER.debug("Skip invalid note file: " + s.getKey());
LOGGER.debug("Skip invalid note file: {}", s.getKey());
}
}
nextMarker = objectListing.getNextMarker();
Expand All @@ -183,8 +185,8 @@ public void remove(String folderPath, AuthenticationInfo subject) {
.withPrefix(rootFolder + folderPath + "/")
.withMarker(nextMarker);
objectListing = ossClient.listObjects(listObjectsRequest);
if (objectListing.getObjectSummaries().size() > 0) {
List<String> keys = new ArrayList();
if (!objectListing.getObjectSummaries().isEmpty()) {
List<String> keys = new ArrayList<>();
for (OSSObjectSummary s : objectListing.getObjectSummaries()) {
keys.add(s.getKey());
}
Expand Down
Loading

0 comments on commit 136140c

Please sign in to comment.