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 c816558 [ZEPPELIN-4220] Unable to set notebook head to previous revision c816558 is described below commit c8165588cfabeeb8e0566ed9e15af24399fb5ed8 Author: 自知 <jiachun....@alibaba-inc.com> AuthorDate: Wed Jul 3 21:51:11 2019 +0800 [ZEPPELIN-4220] Unable to set notebook head to previous revision ### What is this PR for? Unable to set notebook head to previous revision. ### What type of PR is it? [Bug Fix] ### Todos * [x] - Task ### What is the Jira issue? https://jira.apache.org/jira/projects/ZEPPELIN/issues/ZEPPELIN-4220 ### How should this be tested? CI pass ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: 自知 <jiachun....@alibaba-inc.com> Closes #3395 from yejiachun/git_repo and squashes the following commits: c381f6619 [自知] add UT for git repo 6b93a55a6 [自知] [ZEPPELIN-4220]. Fix git repo issues --- .../org/apache/zeppelin/socket/NotebookServer.java | 2 +- .../src/app/notebook/notebook.controller.js | 2 +- .../org/apache/zeppelin/notebook/Notebook.java | 6 +++-- .../zeppelin/notebook/repo/GitNotebookRepo.java | 3 ++- .../notebook/repo/GitNotebookRepoTest.java | 30 +++++++++++++++++++++- 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 20527b9..0bfd389 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java @@ -1491,7 +1491,7 @@ public class NotebookServer extends WebSocketServlet revisions))); } else { conn.send(serializeMessage(new Message(OP.ERROR_INFO).put("info", - "Couldn't checkpoint note revision: possibly storage doesn't support versioning. " + "Couldn't checkpoint note revision: possibly no changes found or storage doesn't support versioning. " + "Please check the logs for more details."))); } } diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js index e2a05b1..dfdb071 100644 --- a/zeppelin-web/src/app/notebook/notebook.controller.js +++ b/zeppelin-web/src/app/notebook/notebook.controller.js @@ -304,7 +304,7 @@ function NotebookCtrl($scope, $route, $routeParams, $location, $rootScope, message: 'Set notebook head to current revision?', callback: function(result) { if (result) { - websocketMsgSrv.setNoteRevision($routeParams.noteId, $routeParams.name, $routeParams.revisionId); + websocketMsgSrv.setNoteRevision($routeParams.noteId, $routeParams.revisionId); } }, }); 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 31d5fdc..79bad1e 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 @@ -330,8 +330,10 @@ public class Notebook { public Note setNoteRevision(String noteId, String notePath, String revisionId, AuthenticationInfo subject) throws IOException { if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) { - return ((NotebookRepoWithVersionControl) notebookRepo) - .setNoteRevision(noteId, notePath, revisionId, subject); + Note note = ((NotebookRepoWithVersionControl) notebookRepo) + .setNoteRevision(noteId, notePath, revisionId, subject); + noteManager.saveNote(note); + return note; } else { return null; } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java index 322d692..21d4f6d 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java @@ -131,7 +131,8 @@ public class GitNotebookRepo extends VFSNotebookRepo implements NotebookRepoWith Revision revision = Revision.EMPTY; try { List<DiffEntry> gitDiff = git.diff().call(); - if (!gitDiff.isEmpty()) { + boolean modified = gitDiff.parallelStream().anyMatch(diffEntry -> diffEntry.getNewPath().equals(noteFileName)); + if (modified) { LOGGER.debug("Changes found for pattern '{}': {}", noteFileName, gitDiff); DirCache added = git.add().addFilepattern(noteFileName).call(); LOGGER.debug("{} changes are about to be commited", added.getEntryCount()); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java index a9fa151..7a4b5c7 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java @@ -39,6 +39,7 @@ import org.apache.zeppelin.user.AuthenticationInfo; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -171,7 +172,7 @@ public class GitNotebookRepoTest { } @Test - public void addCheckpointTest() throws IOException { + public void addCheckpointTest() throws IOException, GitAPIException { // initial checks notebookRepo = new GitNotebookRepo(conf); assertThat(notebookRepo.list(null)).isNotEmpty(); @@ -199,6 +200,33 @@ public class GitNotebookRepoTest { // see if commit is added List<Revision> notebookHistoryAfter = notebookRepo.revisionHistory(TEST_NOTE_ID, TEST_NOTE_PATH, null); assertThat(notebookHistoryAfter.size()).isEqualTo(initialCount + 1); + + int revCountBefore = 0; + Iterable<RevCommit> revCommits = notebookRepo.getGit().log().call(); + for (RevCommit revCommit : revCommits) { + revCountBefore++; + } + + // add changes to note2 + Note note2 = notebookRepo.get(TEST_NOTE_ID2, TEST_NOTE_PATH2, null); + note2.setInterpreterFactory(mock(InterpreterFactory.class)); + Paragraph p2 = note2.addNewParagraph(AuthenticationInfo.ANONYMOUS); + Map<String, Object> config2 = p2.getConfig(); + config2.put("enabled", true); + p2.setConfig(config); + p2.setText("%md checkpoint test text"); + + // save note2 and checkpoint this note without changes + notebookRepo.save(note2, null); + notebookRepo.checkpoint(TEST_NOTE_ID, TEST_NOTE_PATH, "third commit", null); + + // should not add more commit + int revCountAfter = 0; + revCommits = notebookRepo.getGit().log().call(); + for (RevCommit revCommit : revCommits) { + revCountAfter++; + } + assertThat(revCountAfter).isEqualTo(revCountBefore); } private boolean containsNote(Map<String, NoteInfo> notes, String noteId) {