Repository: zeppelin Updated Branches: refs/heads/master 8790ba97c -> dec31d69e
[ZEPPELIN-707]Automatically adds %.* of previous paragraph's typing ### What is this PR for? Automatically adds %pyspark in the code area after paragraph created, if previous paragraph's typing is %pyspark. ### What type of PR is it? New Feature ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-707 ### How should this be tested? unit test Run-time checking ### Screenshots (if appropriate) - default interpreter  - while run paragraph, go to line end  Author: Minwoo Kang <[email protected]> Closes #806 from mwkang/ZEPPELIN-707 and squashes the following commits: 3652dbb [Minwoo Kang] [ZEPPELIN-707]Add test case a159903 [Minwoo Kang] [ZEPPELIN-707]Modify return empty string when repl name is blank 720ffba [Minwoo Kang] [ZEPPELIN-707]Add note.setLastReplName in Rest API 4c0904b [Minwoo Kang] [ZEPPELIN-707]Resolved conflicts 0fd122c [Minwoo Kang] [ZEPPELIN-707]Modify checking nullable list location 85ee14b [Minwoo Kang] [ZEPPELIN-707]Fix test in error 334b15f [Minwoo Kang] [ZEPPELIN-707]Remove OccupiedInterpreter class. Add lastReplName in Note.java f3ab5ea [Minwoo Kang] [ZEPPELIN-707]Fix cannot find symbol error 532e98e [Minwoo Kang] [ZEPPELIN-707]Fix cannot find symbol error 1484d1c [Minwoo Kang] [ZEPPELIN-707]Change cursor of new paragraph go at the end of the line 2366709 [Minwoo Kang] [ZEPPELIN-707]Use ConcurrentHashMap instead of HashMap caeb9a2 [Minwoo Kang] [ZEPPELIN-707]Use ConcurrentHashMap instead of HashMap 854e96b [Minwoo Kang] [ZEPPELIN-707]Fix 'paragraphId' used out of scope. 3c7485d [Minwoo Kang] [ZEPPELIN-707]Fix go to line end 7dd3f68 [Minwoo Kang] [ZEPPELIN-707]Make %.. is default interpreter 27f3925 [Minwoo Kang] [ZEPPELIN-707]Fixed ParagraphActionsIT test 011f4e3 [Minwoo Kang] [ZEPPELIN-707]Fixed interpreter parsing problems 83003f5 [Minwoo Kang] [ZEPPELIN-707]Fixed test f1e59b2 [Minwoo Kang] [ZEPPELIN-707]Automatically adds %.. of previous paragraph's typing Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/dec31d69 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/dec31d69 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/dec31d69 Branch: refs/heads/master Commit: dec31d69efc3d167bb1f5ac91b26478f307414fb Parents: 8790ba9 Author: Minwoo Kang <[email protected]> Authored: Thu Jun 16 23:15:12 2016 +0900 Committer: Jongyoul Lee <[email protected]> Committed: Mon Jun 20 23:12:18 2016 +0900 ---------------------------------------------------------------------- .../apache/zeppelin/rest/NotebookRestApi.java | 1 + .../apache/zeppelin/socket/NotebookServer.java | 1 + .../integration/ParagraphActionsIT.java | 8 +-- .../zeppelin/rest/ZeppelinRestApiTest.java | 3 +- .../notebook/paragraph/paragraph.controller.js | 5 ++ .../java/org/apache/zeppelin/notebook/Note.java | 55 ++++++++++++++- .../notebook/NoteInterpreterLoader.java | 18 ++++- .../org/apache/zeppelin/notebook/Notebook.java | 1 + .../org/apache/zeppelin/notebook/NoteTest.java | 74 ++++++++++++++++++++ 9 files changed, 157 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java ---------------------------------------------------------------------- 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 87e7cce..c10dee8 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 @@ -587,6 +587,7 @@ public class NotebookRestApi { if (paramsForUpdating != null) { paragraph.settings.getParams().putAll(paramsForUpdating); AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); + note.setLastReplName(paragraph.getId()); note.persist(subject); } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java ---------------------------------------------------------------------- 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 74ab9bd..17650dd 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 @@ -1075,6 +1075,7 @@ public class NotebookServer extends WebSocketServlet implements // if it's the last paragraph, let's add a new one boolean isTheLastParagraph = note.getLastParagraph().getId() .equals(p.getId()); + note.setLastReplName(paragraphId); if (!Strings.isNullOrEmpty(text) && isTheLastParagraph) { note.addParagraph(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java index d1135c0..a76706b 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java @@ -18,6 +18,7 @@ package org.apache.zeppelin.integration; +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.AbstractZeppelinIT; import org.apache.zeppelin.WebDriverManager; import org.apache.zeppelin.ZeppelinITUtils; @@ -28,14 +29,11 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ErrorCollector; import org.openqa.selenium.*; -import org.openqa.selenium.interactions.Action; import org.openqa.selenium.interactions.Actions; import org.openqa.selenium.support.ui.Select; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; - public class ParagraphActionsIT extends AbstractZeppelinIT { private static final Logger LOG = LoggerFactory.getLogger(ParagraphActionsIT.class); @@ -96,7 +94,7 @@ public class ParagraphActionsIT extends AbstractZeppelinIT { collector.checkThat("Paragraph is created above", driver.findElement(By.xpath(getParagraphXPath(1) + "//div[contains(@class, 'editor')]")).getText(), - CoreMatchers.equalTo("")); + CoreMatchers.equalTo(StringUtils.EMPTY)); setTextOfParagraph(1, " this is above "); newPara = driver.findElement(By.xpath(getParagraphXPath(2) + "//div[contains(@class,'new-paragraph')][2]")); @@ -106,7 +104,7 @@ public class ParagraphActionsIT extends AbstractZeppelinIT { collector.checkThat("Paragraph is created below", driver.findElement(By.xpath(getParagraphXPath(3) + "//div[contains(@class, 'editor')]")).getText(), - CoreMatchers.equalTo("")); + CoreMatchers.equalTo(StringUtils.EMPTY)); setTextOfParagraph(3, " this is below "); collector.checkThat("The output field of paragraph1 contains", http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java index 4390d74..00a8106 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java @@ -146,7 +146,8 @@ public class ZeppelinRestApiTest extends AbstractTestRestApi { assertEquals("compare note name", expectedNoteName, newNoteName); assertEquals("initial paragraph check failed", 3, newNote.getParagraphs().size()); for (Paragraph p : newNote.getParagraphs()) { - if (StringUtils.isEmpty(p.getText())) { + if (StringUtils.isEmpty(p.getText()) || + p.getText().trim().equals(newNote.getLastInterpreterName())) { continue; } assertTrue("paragraph title check failed", p.getTitle().startsWith("title")); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js index 21a17b2..2cf0222 100644 --- a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js +++ b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js @@ -732,6 +732,7 @@ angular.module('zeppelinWebApp') $scope.editor.setTheme('ace/theme/chrome'); if ($scope.paragraphFocused) { $scope.editor.focus(); + $scope.goToLineEnd(); } autoAdjustEditorHeight(_editor.container.id); @@ -1001,6 +1002,10 @@ angular.module('zeppelinWebApp') return false; }; + $scope.goToLineEnd = function () { + $scope.editor.navigateLineEnd(); + }; + $scope.$on('updateProgress', function(event, data) { if (data.id === $scope.paragraph.id) { $scope.currentProgress = data.progress; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java index 60ee1a0..7108bb3 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java @@ -20,11 +20,12 @@ package org.apache.zeppelin.notebook; import java.io.IOException; import java.io.Serializable; import java.util.*; -import java.util.concurrent.Callable; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.display.AngularObject; import org.apache.zeppelin.display.AngularObjectRegistry; @@ -40,6 +41,7 @@ import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.scheduler.JobListener; import org.apache.zeppelin.search.SearchService; +import com.google.common.base.Optional; import com.google.gson.Gson; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.user.Credentials; @@ -65,6 +67,7 @@ public class Note implements Serializable, JobListener { private String name = ""; private String id; + private AtomicReference<String> lastReplName = new AtomicReference<>(StringUtils.EMPTY); private transient ZeppelinConfiguration conf = ZeppelinConfiguration.create(); @SuppressWarnings("rawtypes") @@ -108,6 +111,17 @@ public class Note implements Serializable, JobListener { id = IdHashes.encode(System.currentTimeMillis() + new Random().nextInt()); } + private String getDefaultInterpreterName() { + Optional<InterpreterSetting> settingOptional = replLoader.getDefaultInterpreterSetting(); + return settingOptional.isPresent() ? settingOptional.get().getName() : StringUtils.EMPTY; + } + + void putDefaultReplName() { + String defaultInterpreterName = getDefaultInterpreterName(); + logger.info("defaultInterpreterName is '{}'", defaultInterpreterName); + lastReplName.set(defaultInterpreterName); + } + public String id() { return id; } @@ -188,6 +202,7 @@ public class Note implements Serializable, JobListener { public Paragraph addParagraph() { Paragraph p = new Paragraph(this, this, replLoader); + addLastReplNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(p); } @@ -231,6 +246,7 @@ public class Note implements Serializable, JobListener { */ public Paragraph insertParagraph(int index) { Paragraph p = new Paragraph(this, this, replLoader); + addLastReplNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(index, p); } @@ -238,6 +254,22 @@ public class Note implements Serializable, JobListener { } /** + * Add Last Repl name If Paragraph has empty text + * + * @param p Paragraph + */ + private void addLastReplNameIfEmptyText(Paragraph p) { + String replName = lastReplName.get(); + if (StringUtils.isEmpty(p.getText()) && StringUtils.isNotEmpty(replName)) { + p.setText(getInterpreterName(replName) + " "); + } + } + + private String getInterpreterName(String replName) { + return StringUtils.isBlank(replName) ? StringUtils.EMPTY : "%" + replName; + } + + /** * Remove paragraph by id. * * @param paragraphId @@ -387,6 +419,9 @@ public class Note implements Serializable, JobListener { public void runAll() { String cronExecutingUser = (String) getConfig().get("cronExecutingUser"); synchronized (paragraphs) { + if (!paragraphs.isEmpty()) { + setLastReplName(paragraphs.get(paragraphs.size() - 1)); + } for (Paragraph p : paragraphs) { if (!p.isEnabled()) { continue; @@ -502,6 +537,16 @@ public class Note implements Serializable, JobListener { repo.save(this, subject); } + private void setLastReplName(Paragraph lastParagraphStarted) { + if (StringUtils.isNotEmpty(lastParagraphStarted.getRequiredReplName())) { + lastReplName.set(lastParagraphStarted.getRequiredReplName()); + } + } + + public void setLastReplName(String paragraphId) { + setLastReplName(getParagraph(paragraphId)); + } + /** * Persist this note with maximum delay. * @param maxDelaySec @@ -567,6 +612,14 @@ public class Note implements Serializable, JobListener { this.info = info; } + public String getLastReplName() { + return lastReplName.get(); + } + + public String getLastInterpreterName() { + return getInterpreterName(getLastReplName()); + } + @Override public void beforeStatusChange(Job job, Status before, Status after) { } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java index d2a1e84..3c432cb 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java @@ -17,6 +17,8 @@ package org.apache.zeppelin.notebook; +import com.google.common.base.Optional; + import java.io.IOException; import java.util.LinkedList; import java.util.List; @@ -119,7 +121,7 @@ public class NoteInterpreterLoader { if (replName == null || replName.trim().length() == 0) { // get default settings (first available) - InterpreterSetting defaultSettings = settings.get(0); + InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings).get(); return createOrGetInterpreterList(defaultSettings).get(0); } @@ -156,7 +158,7 @@ public class NoteInterpreterLoader { } else { // first assume replName is 'name' of interpreter. ('groupName' is ommitted) // search 'name' from first (default) interpreter group - InterpreterSetting defaultSetting = settings.get(0); + InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings).get(); Interpreter.RegisteredInterpreter registeredInterpreter = Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName); if (registeredInterpreter != null) { @@ -190,4 +192,16 @@ public class NoteInterpreterLoader { return null; } + + private Optional<InterpreterSetting> + getDefaultInterpreterSetting(List<InterpreterSetting> settings) { + if (settings == null || settings.isEmpty()) { + return Optional.absent(); + } + return Optional.of(settings.get(0)); + } + + Optional<InterpreterSetting> getDefaultInterpreterSetting() { + return getDefaultInterpreterSetting(getInterpreterSettings()); + } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java ---------------------------------------------------------------------- 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 a6f5816..adaaa2a 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 @@ -159,6 +159,7 @@ public class Notebook { } if (interpreterIds != null) { bindInterpretersToNote(note.id(), interpreterIds); + note.putDefaultReplName(); } notebookIndex.addIndexDoc(note); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dec31d69/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java index 33a7ef2..29da058 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java @@ -17,7 +17,11 @@ package org.apache.zeppelin.notebook; +import com.google.common.base.Optional; + +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.interpreter.Interpreter; +import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.repo.NotebookRepo; import org.apache.zeppelin.scheduler.Scheduler; import org.apache.zeppelin.search.SearchService; @@ -26,6 +30,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import static org.junit.Assert.*; @@ -91,4 +96,73 @@ public class NoteTest { assertEquals("Change paragraph text", "%jdbc(mysql) show databases", pCaptor.getValue().getEffectiveText()); assertEquals("Change paragraph text", pText, pCaptor.getValue().getText()); } + + @Test + public void putDefaultReplNameIfInterpreterSettingAbsent() { + when(replLoader.getDefaultInterpreterSetting()) + .thenReturn(Optional.<InterpreterSetting>absent()); + + Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + note.putDefaultReplName(); + + assertEquals(StringUtils.EMPTY, note.getLastReplName()); + assertEquals(StringUtils.EMPTY, note.getLastInterpreterName()); + } + + @Test + public void putDefaultReplNameIfInterpreterSettingPresent() { + InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); + when(interpreterSetting.getName()).thenReturn("spark"); + when(replLoader.getDefaultInterpreterSetting()) + .thenReturn(Optional.of(interpreterSetting)); + + Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + note.putDefaultReplName(); + + assertEquals("spark", note.getLastReplName()); + assertEquals("%spark", note.getLastInterpreterName()); + } + + @Test + public void addParagraphWithLastReplName() { + InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); + when(interpreterSetting.getName()).thenReturn("spark"); + when(replLoader.getDefaultInterpreterSetting()) + .thenReturn(Optional.of(interpreterSetting)); + + Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + note.putDefaultReplName(); //set lastReplName + + Paragraph p = note.addParagraph(); + + assertEquals("%spark ", p.getText()); + } + + @Test + public void insertParagraphWithLastReplName() { + InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); + when(interpreterSetting.getName()).thenReturn("spark"); + when(replLoader.getDefaultInterpreterSetting()) + .thenReturn(Optional.of(interpreterSetting)); + + Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + note.putDefaultReplName(); //set lastReplName + + Paragraph p = note.insertParagraph(note.getParagraphs().size()); + + assertEquals("%spark ", p.getText()); + } + + @Test + public void setLastReplName() { + String paragraphId = "HelloWorld"; + Note note = Mockito.spy(new Note(repo, replLoader, jobListenerFactory, index, credentials)); + Paragraph mockParagraph = Mockito.mock(Paragraph.class); + when(note.getParagraph(paragraphId)).thenReturn(mockParagraph); + when(mockParagraph.getRequiredReplName()).thenReturn("spark"); + + note.setLastReplName(paragraphId); + + assertEquals("spark", note.getLastReplName()); + } } \ No newline at end of file
