This is an automated email from the ASF dual-hosted git repository. zjffdu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push: new c26ed1a [ZEPPELIN-5661] Support clone a specific revision of a note (#4299) c26ed1a is described below commit c26ed1a0bc0221141d8d216c54507a72a6580e2a Author: nihua <guanhua...@foxmail.com> AuthorDate: Mon Mar 14 16:11:09 2022 +0800 [ZEPPELIN-5661] Support clone a specific revision of a note (#4299) --- docs/usage/rest_api/notebook.md | 8 ++- .../org/apache/zeppelin/rest/NotebookRestApi.java | 4 +- .../zeppelin/rest/message/NewNoteRequest.java | 5 ++ .../apache/zeppelin/service/NotebookService.java | 23 +++++-- .../apache/zeppelin/rest/NotebookRestApiTest.java | 79 +++++++++++++++------- .../org/apache/zeppelin/notebook/Notebook.java | 32 ++++++--- 6 files changed, 113 insertions(+), 38 deletions(-) diff --git a/docs/usage/rest_api/notebook.md b/docs/usage/rest_api/notebook.md index c49d805..b669213 100644 --- a/docs/usage/rest_api/notebook.md +++ b/docs/usage/rest_api/notebook.md @@ -352,7 +352,8 @@ Notebooks REST API supports the following operations: List, Create, Get, Delete, <tr> <td>Description</td> <td>This ```POST``` method clones a note by the given id and create a new note using the given name - or default name if none given. + or default name if none given. If what you want to copy is a certain version of note, you need + to specify the revisionId. The body field of the returned JSON contains the new note id. </td> </tr> @@ -373,7 +374,10 @@ Notebooks REST API supports the following operations: List, Create, Get, Delete, <td> ```json -{"name": "name of new note"} +{ + "name": "name of new note", + "revisionId": "revisionId of note to be copied (optional)" +} ``` </td> </tr> diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java index 40ad46a..88004f4 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java @@ -555,11 +555,13 @@ public class NotebookRestApi extends AbstractRestApi { checkIfUserCanWrite(noteId, "Insufficient privileges you cannot clone this note"); NewNoteRequest request = NewNoteRequest.fromJson(message); String newNoteName = null; + String revisionId = null; if (request != null) { newNoteName = request.getName(); + revisionId = request.getRevisionId(); } AuthenticationInfo subject = new AuthenticationInfo(authenticationService.getPrincipal()); - String newNoteId = notebookService.cloneNote(noteId, newNoteName, getServiceContext(), + String newNoteId = notebookService.cloneNote(noteId, revisionId, newNoteName, getServiceContext(), new RestServiceCallback<Note>() { @Override public void onSuccess(Note newNote, ServiceContext context) throws IOException { diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/NewNoteRequest.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/NewNoteRequest.java index 85744dc..1ef9da3 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/NewNoteRequest.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/NewNoteRequest.java @@ -33,6 +33,7 @@ public class NewNoteRequest implements JsonSerializable { private String defaultInterpreterGroup; private boolean addingEmptyParagraph = false; private List<NewParagraphRequest> paragraphs; + private String revisionId; public NewNoteRequest (){ } @@ -53,6 +54,10 @@ public class NewNoteRequest implements JsonSerializable { return paragraphs; } + public String getRevisionId() { + return revisionId; + } + public String toJson() { return GSON.toJson(this); } diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java index 02a4bbd..dfc75eb 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java @@ -317,15 +317,30 @@ public class NotebookService { } public String cloneNote(String noteId, - String newNotePath, - ServiceContext context, - ServiceCallback<Note> callback) throws IOException { + String newNotePath, + ServiceContext context, + ServiceCallback<Note> callback) throws IOException { + return cloneNote(noteId, "", newNotePath, context, callback); + } + + + public String cloneNote(String noteId, + String revisionId, + String newNotePath, + ServiceContext context, + ServiceCallback<Note> callback) throws IOException { //TODO(zjffdu) move these to Notebook if (StringUtils.isBlank(newNotePath)) { newNotePath = "/Cloned Note_" + noteId; + if(StringUtils.isNotEmpty(revisionId)) { + // If cloning a revision of the note, + // append the short commit id of revision to newNoteName + // to distinguish which commit to be copied. + newNotePath += "_" + revisionId.substring(0, 7); + } } try { - String newNoteId = notebook.cloneNote(noteId, normalizeNotePath(newNotePath), + String newNoteId = notebook.cloneNote(noteId, revisionId, normalizeNotePath(newNotePath), context.getAutheInfo()); return notebook.processNote(newNoteId, newNote -> { diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java index 62398c0..e186ae9 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java @@ -43,10 +43,7 @@ import org.junit.runners.MethodSorters; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.util.EntityUtils; @@ -895,33 +892,69 @@ public class NotebookRestApiTest extends AbstractTestRestApi { public void testCloneNote() throws IOException { LOG.info("Running testCloneNote"); String note1Id = null; - String clonedNoteId = null; + List<String> clonedNoteIds = new ArrayList<>(); + String text1 = "%text clone note"; + String text2 = "%text clone revision of note"; try { - note1Id = TestUtils.getInstance(Notebook.class).createNote("note1", anonymous); - CloseableHttpResponse post = httpPost("/notebook/" + note1Id, ""); - String postResponse = EntityUtils.toString(post.getEntity(), StandardCharsets.UTF_8); - LOG.info("testCloneNote response\n" + postResponse); - assertThat(post, isAllowed()); - Map<String, Object> resp = gson.fromJson(postResponse, - new TypeToken<Map<String, Object>>() {}.getType()); - clonedNoteId = (String) resp.get("body"); - post.close(); + Notebook notebook = TestUtils.getInstance(Notebook.class); + note1Id = notebook.createNote("note1", anonymous); - CloseableHttpResponse get = httpGet("/notebook/" + clonedNoteId); - assertThat(get, isAllowed()); - Map<String, Object> resp2 = gson.fromJson(EntityUtils.toString(get.getEntity(), StandardCharsets.UTF_8), - new TypeToken<Map<String, Object>>() {}.getType()); - Map<String, Object> resp2Body = (Map<String, Object>) resp2.get("body"); + // add text and commit note + NotebookRepoWithVersionControl.Revision first_commit = + notebook.processNote(note1Id, note -> { + Paragraph p1 = note.addNewParagraph(anonymous); + p1.setText(text2); + notebook.saveNote(note, AuthenticationInfo.ANONYMOUS); + return notebook.checkpointNote(note.getId(), note.getPath(), "first commit", anonymous); + }); - // assertEquals(resp2Body.get("name"), "Note " + clonedNoteId); - get.close(); + // change the text of note + notebook.processNote(note1Id, note -> { + note.getParagraph(0).setText(text1); + return null; + }); + + // Clone a note + CloseableHttpResponse post1 = httpPost("/notebook/" + note1Id, ""); + // Clone a revision of note + CloseableHttpResponse post2 = + httpPost("/notebook/" + note1Id, "{ revisionId: " + first_commit.id + "}"); + + // Verify the responses + for (int i = 0; i < 2; i++) { + CloseableHttpResponse post = Arrays.asList(post1, post2).get(i); + String text = Arrays.asList(text1, text2).get(i); + + String postResponse = EntityUtils.toString(post.getEntity(), StandardCharsets.UTF_8); + LOG.info("testCloneNote response: {}", postResponse); + assertThat(post, isAllowed()); + Map<String, Object> resp = gson.fromJson(postResponse, + new TypeToken<Map<String, Object>>() { + }.getType()); + clonedNoteIds.add((String) resp.get("body")); + post.close(); + + CloseableHttpResponse get = httpGet("/notebook/" + clonedNoteIds.get(clonedNoteIds.size() - 1)); + assertThat(get, isAllowed()); + Map<String, Object> resp2 = gson.fromJson(EntityUtils.toString(get.getEntity(), StandardCharsets.UTF_8), + new TypeToken<Map<String, Object>>() { + }.getType()); + Map<String, Object> resp2Body = (Map<String, Object>) resp2.get("body"); + List<Map<String, String>> paragraphs = (List<Map<String, String>>) resp2Body.get("paragraphs"); + // Verify that the original and copied text are consistent + assertEquals(text, paragraphs.get(0).get("text")); + // assertEquals(resp2Body.get("name"), "Note " + clonedNoteId); + get.close(); + } } finally { // cleanup if (null != note1Id) { TestUtils.getInstance(Notebook.class).removeNote(note1Id, anonymous); } - if (null != clonedNoteId) { - TestUtils.getInstance(Notebook.class).removeNote(clonedNoteId, anonymous); + if (null != clonedNoteIds) { + for (String clonedNoteId : clonedNoteIds) { + TestUtils.getInstance(Notebook.class).removeNote(clonedNoteId, anonymous); + } } } } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java index b985b1e..ad285cc 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java @@ -32,6 +32,7 @@ import java.util.function.Function; import java.util.stream.Collectors; import javax.inject.Inject; +import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; import org.apache.zeppelin.display.AngularObject; @@ -310,25 +311,40 @@ public class Notebook { * @throws IOException */ public String cloneNote(String sourceNoteId, String newNotePath, AuthenticationInfo subject) + throws IOException { + return cloneNote(sourceNoteId, "", newNotePath, subject); + } + + + public String cloneNote(String sourceNoteId, String revisionId, String newNotePath, AuthenticationInfo subject) throws IOException { return processNote(sourceNoteId, sourceNote -> { if (sourceNote == null) { throw new IOException("Source note: " + sourceNoteId + " not found"); } + Note note; + if(StringUtils.isNotEmpty(revisionId)) { + note = getNoteByRevision(sourceNote.getId(), sourceNote.getPath(), revisionId, subject); + } else { + note = sourceNote; + } + if (note == null) { + throw new IOException("Source note: " + sourceNoteId + " revisionId " + revisionId + " not found"); + } String newNoteId = createNote(newNotePath, subject, false); processNote(newNoteId, newNote -> { - List<Paragraph> paragraphs = sourceNote.getParagraphs(); + List<Paragraph> paragraphs = note.getParagraphs(); for (Paragraph p : paragraphs) { newNote.addCloneParagraph(p, subject); } - newNote.setConfig(new HashMap<>(sourceNote.getConfig())); - newNote.setInfo(new HashMap<>(sourceNote.getInfo())); - newNote.setDefaultInterpreterGroup(sourceNote.getDefaultInterpreterGroup()); - newNote.setNoteForms(new HashMap<>(sourceNote.getNoteForms())); - newNote.setNoteParams(new HashMap<>(sourceNote.getNoteParams())); + newNote.setConfig(new HashMap<>(note.getConfig())); + newNote.setInfo(new HashMap<>(note.getInfo())); + newNote.setDefaultInterpreterGroup(note.getDefaultInterpreterGroup()); + newNote.setNoteForms(new HashMap<>(note.getNoteForms())); + newNote.setNoteParams(new HashMap<>(note.getNoteParams())); newNote.setRunning(false); saveNote(newNote, subject); @@ -528,11 +544,11 @@ public class Notebook { } } - public Note getNoteByRevision(String noteId, String noteName, + public Note getNoteByRevision(String noteId, String notePath, String revisionId, AuthenticationInfo subject) throws IOException { if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) { - return ((NotebookRepoWithVersionControl) notebookRepo).get(noteId, noteName, + return ((NotebookRepoWithVersionControl) notebookRepo).get(noteId, notePath, revisionId, subject); } else { return null;