Skip to content

[Zeppelin 796] Duplicated notebook names should not be allowed #1536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost wants to merge 3 commits into from
Closed

[Zeppelin 796] Duplicated notebook names should not be allowed #1536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcdcce2
code changes for ZEPPELIN-796
raja-imaginea Oct 18, 2016
c00c440
format changes
raja-imaginea Oct 18, 2016
ce8e4f4
Test fixes
raja-imaginea Nov 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 46 additions & 27 deletions zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.annotation.ZeppelinApi;
import org.apache.zeppelin.exception.DuplicateNameException;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.Notebook;
Expand Down Expand Up @@ -295,13 +296,20 @@ public Response exportNote(@PathParam("noteId") String noteId) throws IOExceptio
* @param req - note Json
* @return JSON with new note ID
* @throws IOException
* @throws DuplicateNameException
*/
@POST
@Path("import")
@ZeppelinApi
public Response importNote(String req) throws IOException {
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
Note newNote = notebook.importNote(req, null, subject);
Note newNote;
try {
newNote = notebook.importNote(req, null, subject);
} catch (DuplicateNameException e) {
return new JsonResponse<>(Status.NOT_ACCEPTABLE,
"Notebook already exists with same name").build();
}
return new JsonResponse<>(Status.CREATED, "", newNote.getId()).build();
}

Expand All @@ -311,33 +319,37 @@ public Response importNote(String req) throws IOException {
* @param message - JSON with new note name
* @return JSON with new note ID
* @throws IOException
* @throws DuplicateNameException
*/
@POST
@Path("/")
@ZeppelinApi
public Response createNote(String message) throws IOException {
LOG.info("Create new note by JSON {}", message);
NewNoteRequest request = gson.fromJson(message, NewNoteRequest.class);
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
Note note = notebook.createNote(subject);
List<NewParagraphRequest> initialParagraphs = request.getParagraphs();
if (initialParagraphs != null) {
for (NewParagraphRequest paragraphRequest : initialParagraphs) {
Paragraph p = note.addParagraph();
p.setTitle(paragraphRequest.getTitle());
p.setText(paragraphRequest.getText());
String noteName = null;
Note note;
try {
NewNoteRequest request = gson.fromJson(message, NewNoteRequest.class);
noteName = request.getName();
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
note = notebook.createNote(subject, noteName);
List<NewParagraphRequest> initialParagraphs = request.getParagraphs();
if (initialParagraphs != null) {
for (NewParagraphRequest paragraphRequest : initialParagraphs) {
Paragraph p = note.addParagraph();
p.setTitle(paragraphRequest.getTitle());
p.setText(paragraphRequest.getText());
}
}
}
note.addParagraph(); // add one paragraph to the last
String noteName = request.getName();
if (noteName.isEmpty()) {
noteName = "Note " + note.getId();
}
note.addParagraph(); // add one paragraph to the last

note.setName(noteName);
note.persist(subject);
notebookServer.broadcastNote(note);
notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
note.persist(subject);
notebookServer.broadcastNote(note);
notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
} catch (DuplicateNameException e) {
return new JsonResponse<>(Status.NOT_ACCEPTABLE, "Notebook with name " + noteName +
"already exists").build();
}
return new JsonResponse<>(Status.CREATED, "", note.getId()).build();
}

Expand Down Expand Up @@ -372,6 +384,7 @@ public Response deleteNote(@PathParam("noteId") String noteId) throws IOExceptio
* @param noteId ID of Note
* @return JSON with status.CREATED
* @throws IOException, CloneNotSupportedException, IllegalArgumentException
* @throws DuplicateNameException
*/
@POST
@Path("{noteId}")
Expand All @@ -380,15 +393,21 @@ public Response cloneNote(@PathParam("noteId") String noteId, String message)
throws IOException, CloneNotSupportedException, IllegalArgumentException {
LOG.info("clone note by JSON {}", message);
checkIfUserCanWrite(noteId, "Insufficient privileges you cannot clone this note");
NewNoteRequest request = gson.fromJson(message, NewNoteRequest.class);
String newNoteName = null;
if (request != null) {
newNoteName = request.getName();
Note newNote;
try {
NewNoteRequest request = gson.fromJson(message, NewNoteRequest.class);
if (request != null) {
newNoteName = request.getName();
}
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
newNote = notebook.cloneNote(noteId, newNoteName, subject);
notebookServer.broadcastNote(newNote);
notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
} catch (DuplicateNameException e) {
return new JsonResponse<>(Status.NOT_ACCEPTABLE, "Note with name " + newNoteName +
"already exists").build();
}
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
Note newNote = notebook.cloneNote(noteId, newNoteName, subject);
notebookServer.broadcastNote(newNote);
notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
return new JsonResponse<>(Status.CREATED, "", newNote.getId()).build();
}

Expand Down
113 changes: 59 additions & 54 deletions zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.apache.zeppelin.display.AngularObject;
import org.apache.zeppelin.display.AngularObjectRegistry;
import org.apache.zeppelin.display.AngularObjectRegistryListener;
import org.apache.zeppelin.exception.DuplicateNameException;
import org.apache.zeppelin.helium.ApplicationEventListener;
import org.apache.zeppelin.helium.HeliumPackage;
import org.apache.zeppelin.interpreter.InterpreterGroup;
Expand Down Expand Up @@ -718,46 +719,39 @@ private boolean isCronUpdated(Map<String, Object> configA,

private void createNote(NotebookSocket conn, HashSet<String> userAndRoles,
Notebook notebook, Message message)
throws IOException {
throws IOException, DuplicateNameException {
AuthenticationInfo subject = new AuthenticationInfo(message.principal);

try {
Note note = null;

String defaultInterpreterId = (String) message.get("defaultInterpreterId");
if (!StringUtils.isEmpty(defaultInterpreterId)) {
List<String> interpreterSettingIds = new LinkedList<>();
interpreterSettingIds.add(defaultInterpreterId);
for (String interpreterSettingId : notebook.getInterpreterFactory().
getDefaultInterpreterSettingList()) {
if (!interpreterSettingId.equals(defaultInterpreterId)) {
interpreterSettingIds.add(interpreterSettingId);
}
Note note = null;
String noteName = null;
if (message != null) {
noteName = (String) message.get("name");
}
String defaultInterpreterId = (String) message.get("defaultInterpreterId");
if (!StringUtils.isEmpty(defaultInterpreterId)) {
List<String> interpreterSettingIds = new LinkedList<>();
interpreterSettingIds.add(defaultInterpreterId);
for (String interpreterSettingId : notebook.getInterpreterFactory().
getDefaultInterpreterSettingList()) {
if (!interpreterSettingId.equals(defaultInterpreterId)) {
interpreterSettingIds.add(interpreterSettingId);
}
note = notebook.createNote(interpreterSettingIds, subject);
} else {
note = notebook.createNote(subject);
}

note.addParagraph(); // it's an empty note. so add one paragraph
if (message != null) {
String noteName = (String) message.get("name");
if (StringUtils.isEmpty(noteName)) {
noteName = "Note " + note.getId();
}
note.setName(noteName);
note = notebook.createNote(interpreterSettingIds, subject);
} else {
try {
note = notebook.createNote(subject, noteName);
} catch (DuplicateNameException dne) {
conn.send(serializeMessage(new Message(OP.ERROR_DIALOG).put("info",
"Please provide a unique notebook name")));
}

note.persist(subject);
addConnectionToNote(note.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", note)));
} catch (FileSystemException e) {
LOG.error("Exception from createNote", e);
conn.send(serializeMessage(new Message(OP.ERROR_INFO).put("info",
"Oops! There is something wrong with the notebook file system. "
+ "Please check the logs for more details.")));
return;
}

note.addParagraph(); // it's an empty note. so add one paragraph

note.setName(noteName);
note.persist(subject);
addConnectionToNote(note.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", note)));
broadcastNoteList(subject, userAndRoles);
}

Expand Down Expand Up @@ -816,13 +810,19 @@ private void updateParagraph(NotebookSocket conn, HashSet<String> userAndRoles,
private void cloneNote(NotebookSocket conn, HashSet<String> userAndRoles,
Notebook notebook, Message fromMessage)
throws IOException, CloneNotSupportedException {
String noteId = getOpenNoteId(conn);
String name = (String) fromMessage.get("name");
Note newNote = notebook.cloneNote(noteId, name, new AuthenticationInfo(fromMessage.principal));
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
addConnectionToNote(newNote.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", newNote)));
broadcastNoteList(subject, userAndRoles);
try {
String noteId = getOpenNoteId(conn);
String name = (String) fromMessage.get("name");
Note newNote = notebook.cloneNote(noteId, name,
new AuthenticationInfo(fromMessage.principal));
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
addConnectionToNote(newNote.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", newNote)));
broadcastNoteList(subject, userAndRoles);
} catch (DuplicateNameException dne) {
conn.send(serializeMessage(new Message(OP.ERROR_DIALOG).put("info",
"Please provide a unique notebook name")));
}
}

private void clearAllParagraphOutput(NotebookSocket conn, HashSet<String> userAndRoles,
Expand All @@ -848,19 +848,24 @@ protected Note importNote(NotebookSocket conn, HashSet<String> userAndRoles,
Notebook notebook, Message fromMessage)
throws IOException {
Note note = null;
if (fromMessage != null) {
String noteName = (String) ((Map) fromMessage.get("note")).get("name");
String noteJson = gson.toJson(fromMessage.get("note"));
AuthenticationInfo subject = null;
if (fromMessage.principal != null) {
subject = new AuthenticationInfo(fromMessage.principal);
} else {
subject = new AuthenticationInfo("anonymous");
try {
if (fromMessage != null) {
String noteName = (String) ((Map) fromMessage.get("note")).get("name");
String noteJson = gson.toJson(fromMessage.get("note"));
AuthenticationInfo subject = null;
if (fromMessage.principal != null) {
subject = new AuthenticationInfo(fromMessage.principal);
} else {
subject = new AuthenticationInfo("anonymous");
}
note = notebook.importNote(noteJson, noteName, subject);
note.persist(subject);
broadcastNote(note);
broadcastNoteList(subject, userAndRoles);
}
note = notebook.importNote(noteJson, noteName, subject);
note.persist(subject);
broadcastNote(note);
broadcastNoteList(subject, userAndRoles);
} catch (DuplicateNameException dne) {
conn.send(serializeMessage(new Message(OP.IMPORT_ERROR_DIALOG).put("info",
"Please provide a unique notebook name")));
}
return note;
}
Expand Down
2 changes: 2 additions & 0 deletions zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,4 +555,6 @@ protected Matcher<? super HttpMethodBase> isNotAllowed() {
return responsesWith(405);
}

protected Matcher<? super HttpMethodBase> isNotAcceptable() { return responsesWith(406); }

}
22 changes: 14 additions & 8 deletions zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

package org.apache.zeppelin.rest;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

import java.io.IOException;
import java.util.List;
import java.util.Map;
Expand All @@ -26,6 +30,7 @@
import org.apache.commons.httpclient.methods.PostMethod;
import org.apache.commons.httpclient.methods.PutMethod;
import org.apache.zeppelin.interpreter.InterpreterOption;
import org.apache.zeppelin.exception.DuplicateNameException;
import org.apache.zeppelin.interpreter.InterpreterSetting;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.Paragraph;
Expand All @@ -42,15 +47,14 @@
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;

import static org.junit.Assert.*;

/**
* Zeppelin interpreter rest api tests
*/
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class InterpreterRestApiTest extends AbstractTestRestApi {
Gson gson = new Gson();
AuthenticationInfo anonymous;
private final String EMPTY_STRING = "";

@BeforeClass
public static void init() throws Exception {
Expand Down Expand Up @@ -138,9 +142,9 @@ public void testSettingsCreateWithEmptyJson() throws IOException {
}

@Test
public void testInterpreterAutoBinding() throws IOException {
public void testInterpreterAutoBinding() throws IOException, DuplicateNameException {
// create note
Note note = ZeppelinServer.notebook.createNote(anonymous);
Note note = ZeppelinServer.notebook.createNote(anonymous, EMPTY_STRING);

// check interpreter is binded
GetMethod get = httpGet("/notebook/interpreter/bind/" + note.getId());
Expand All @@ -157,9 +161,10 @@ public void testInterpreterAutoBinding() throws IOException {
}

@Test
public void testInterpreterRestart() throws IOException, InterruptedException {
public void testInterpreterRestart() throws IOException, InterruptedException,
DuplicateNameException {
// create new note
Note note = ZeppelinServer.notebook.createNote(anonymous);
Note note = ZeppelinServer.notebook.createNote(anonymous, EMPTY_STRING);
note.addParagraph();
Paragraph p = note.getLastParagraph();
Map config = p.getConfig();
Expand Down Expand Up @@ -202,9 +207,10 @@ public void testInterpreterRestart() throws IOException, InterruptedException {
}

@Test
public void testRestartInterpreterPerNote() throws IOException, InterruptedException {
public void testRestartInterpreterPerNote() throws IOException, InterruptedException,
DuplicateNameException {
// create new note
Note note = ZeppelinServer.notebook.createNote(anonymous);
Note note = ZeppelinServer.notebook.createNote(anonymous, EMPTY_STRING);
note.addParagraph();
Paragraph p = note.getLastParagraph();
Map config = p.getConfig();
Expand Down
Loading