From f1e59b2273dbc852b3cb5ff323f1ebf8b5d59e02 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Mon, 4 Apr 2016 15:57:38 +0900 Subject: [PATCH 01/19] [ZEPPELIN-707]Automatically adds %.. of previous paragraph's typing --- .../org/apache/zeppelin/notebook/Note.java | 2 + .../notebook/OccupiedInterpreter.java | 135 ++++++++++++++++++ .../notebook/OccupiedInterpreterTest.java | 108 ++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java create mode 100644 zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.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 60ee1a02cfe..0e49005d431 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 @@ -188,6 +188,7 @@ public Map> getAngularObjects() { public Paragraph addParagraph() { Paragraph p = new Paragraph(this, this, replLoader); + OccupiedInterpreter.setInterpreterNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(p); } @@ -231,6 +232,7 @@ public void addCloneParagraph(Paragraph srcParagraph) { */ public Paragraph insertParagraph(int index) { Paragraph p = new Paragraph(this, this, replLoader); + OccupiedInterpreter.setInterpreterNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(index, p); } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java new file mode 100644 index 00000000000..95eaf801e16 --- /dev/null +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zeppelin.notebook; + +import org.apache.commons.lang.StringUtils; + +import java.util.HashMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Current occupied interpreter information for each note + */ +public class OccupiedInterpreter { + private static final Pattern INTERPRETER_NAME_PATTERN = Pattern.compile("^%.*"); + private static final String DEFAULT_INTERPRETER_NAME = "%.."; + private static final HashMap occupiedInterpreterMap = new HashMap<>(); + + private OccupiedInterpreter() { + } + + /** + * Get current occupied interpreter name + * + * @param noteId Note Id + * @return Current occupied interpreter name, or "" if no mapping occupied interpreter name. + */ + public static String getOccupiedInterpreter(String noteId) { + String occupiedInterpreter; + synchronized (occupiedInterpreterMap) { + occupiedInterpreter = occupiedInterpreterMap.get(noteId); + } + return StringUtils.defaultString(occupiedInterpreter, DEFAULT_INTERPRETER_NAME); + } + + static String getDefaultInterpreterName() { + return DEFAULT_INTERPRETER_NAME; + } + + /** + * Remove Occupied interpreter name + * + * @param noteId Note Id + */ + public static void removeOccupiedInterpreter(String noteId) { + if (StringUtils.isBlank(noteId)) { + return; + } + synchronized (occupiedInterpreterMap) { + occupiedInterpreterMap.remove(noteId); + } + } + + /** + * Set current occupied interpreter name + * + * @param noteId Note Id + * @param interpreterName Current occupied interpreter name + */ + public static void setOccupiedInterpreter(String noteId, String interpreterName) { + if (StringUtils.isBlank(interpreterName)) { + return; + } + + synchronized (occupiedInterpreterMap) { + occupiedInterpreterMap.put(noteId, interpreterName); + } + } + + /** + * Set current occupied interpreter name + * + * @param p Paragraph + */ + public static void setOccupiedInterpreter(Paragraph p) { + if (p == null || p.getNote() == null || StringUtils.isEmpty(p.getNote().getId()) + || StringUtils.isEmpty(p.getText())) { + return; + } + + String interpreterName = parseInterpreterName(p.getText()); + if (interpreterName == null) { + return; + } + + setOccupiedInterpreter(p.getNote().getId(), interpreterName); + } + + /** + * Parse interpreter name + * + * @param text Paragraph's text + * @return interpreter name, or {@code null} if text is blank or no interpreter name. + */ + static String parseInterpreterName(String text) { + if (StringUtils.isBlank(text)) { + return null; + } + + Matcher matcher = INTERPRETER_NAME_PATTERN.matcher(text); + if (matcher.find()) { + return matcher.group(); + } + return null; + } + + /** + * Set interpreter name If Paragraph has empty text + * + * @param p Paragraph + */ + public static void setInterpreterNameIfEmptyText(Paragraph p) { + String noteId = p.getNote().getId(); + String interpreterName = getOccupiedInterpreter(noteId); + String text = p.getText(); + if (StringUtils.isEmpty(text)) { + p.setText(interpreterName); + } + } +} diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java new file mode 100644 index 00000000000..12ff2bd995f --- /dev/null +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zeppelin.notebook; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +@RunWith(MockitoJUnitRunner.class) +public class OccupiedInterpreterTest { + @Test + public void getOccupiedInterpreter() throws Exception { + String noteId = "test1"; + assertTrue(OccupiedInterpreter.getDefaultInterpreterName() + .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); + } + + @Test + public void setOccupiedInterpreter() throws Exception { + String noteId = "test2"; + assertTrue(OccupiedInterpreter.getDefaultInterpreterName() + .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); + + String interpreterName = "interpreterName"; + OccupiedInterpreter.setOccupiedInterpreter(noteId, interpreterName); + + assertTrue(interpreterName.equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); + assertFalse(OccupiedInterpreter.getDefaultInterpreterName() + .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); + } + + @Test + public void setOccupiedInterpreterFromParagraph() throws Exception { + Note note = Mockito.mock(Note.class); + + String noteId = "test3"; + String interpreter = "%sh"; + Paragraph p = new Paragraph(); + p.setText(interpreter + System.lineSeparator() + "echo hello"); + + p = Mockito.spy(p); + Mockito.when(p.getNote()).thenReturn(note); + Mockito.when(note.getId()).thenReturn(noteId); + OccupiedInterpreter.setOccupiedInterpreter(p); + + assertTrue(interpreter.equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); + + noteId = "test4"; + Mockito.when(note.getId()).thenReturn(noteId); + p.setText("This is test." + System.lineSeparator() + interpreter); + OccupiedInterpreter.setOccupiedInterpreter(p); + + System.out.println(OccupiedInterpreter.getOccupiedInterpreter(noteId)); + assertTrue(OccupiedInterpreter.getDefaultInterpreterName() + .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); + } + + @Test + public void parseInterpreterName() throws Exception { + String interpreter = "%sh"; + + String text = interpreter + System.lineSeparator() + "echo hello" + interpreter + "hello"; + System.out.println(OccupiedInterpreter.parseInterpreterName(text)); + assertTrue(interpreter.equals(OccupiedInterpreter.parseInterpreterName(text))); + + text = "This is test." + System.lineSeparator() + interpreter; + System.out.println(OccupiedInterpreter.parseInterpreterName(text)); + assertNull(OccupiedInterpreter.parseInterpreterName(text)); + } + + @Test + public void setInterpreterNameIfEmptyText() throws Exception { + String noteId = "test5"; + + Paragraph p = new Paragraph(); + p = Mockito.spy(p); + Note note = Mockito.mock(Note.class); + + Mockito.when(p.getNote()).thenReturn(note); + Mockito.when(note.getId()).thenReturn(noteId); + + OccupiedInterpreter.setInterpreterNameIfEmptyText(p); + + System.out.println(OccupiedInterpreter.getDefaultInterpreterName()); + System.out.println(p.getText()); + assertTrue(OccupiedInterpreter.getDefaultInterpreterName().equals(p.getText())); + } +} \ No newline at end of file From 83003f5dd2d0514c8413c0016c27bb0983e28d0c Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Wed, 6 Apr 2016 11:31:01 +0900 Subject: [PATCH 02/19] [ZEPPELIN-707]Fixed test --- .../java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java | 4 +++- .../org/apache/zeppelin/notebook/OccupiedInterpreter.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) 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 4390d74b495..efe8f1dfe61 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 @@ -29,6 +29,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.Note; +import org.apache.zeppelin.notebook.OccupiedInterpreter; import org.apache.zeppelin.notebook.Paragraph; import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.server.ZeppelinServer; @@ -146,7 +147,8 @@ public void testNotebookCreateWithParagraphs() throws IOException { 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()) + || OccupiedInterpreter.getDefaultInterpreterName().equals(p.getText())) { continue; } assertTrue("paragraph title check failed", p.getTitle().startsWith("title")); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java index 95eaf801e16..2c7d6373a6e 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java @@ -48,7 +48,7 @@ public static String getOccupiedInterpreter(String noteId) { return StringUtils.defaultString(occupiedInterpreter, DEFAULT_INTERPRETER_NAME); } - static String getDefaultInterpreterName() { + public static String getDefaultInterpreterName() { return DEFAULT_INTERPRETER_NAME; } From 011f4e39614f56932d36d764fdc8aa40856f87b7 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Sun, 1 May 2016 16:01:00 +0900 Subject: [PATCH 03/19] [ZEPPELIN-707]Fixed interpreter parsing problems --- .../notebook/OccupiedInterpreter.java | 19 ++++++------ .../notebook/OccupiedInterpreterTest.java | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java index 2c7d6373a6e..5721386cd9a 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java @@ -27,7 +27,7 @@ * Current occupied interpreter information for each note */ public class OccupiedInterpreter { - private static final Pattern INTERPRETER_NAME_PATTERN = Pattern.compile("^%.*"); + private static final Pattern INTERPRETER_NAME_PATTERN = Pattern.compile("^%\\S+"); private static final String DEFAULT_INTERPRETER_NAME = "%.."; private static final HashMap occupiedInterpreterMap = new HashMap<>(); @@ -72,13 +72,16 @@ public static void removeOccupiedInterpreter(String noteId) { * @param noteId Note Id * @param interpreterName Current occupied interpreter name */ + @SuppressWarnings("PointlessBooleanExpression") public static void setOccupiedInterpreter(String noteId, String interpreterName) { - if (StringUtils.isBlank(interpreterName)) { - return; - } - synchronized (occupiedInterpreterMap) { - occupiedInterpreterMap.put(noteId, interpreterName); + if (occupiedInterpreterMap.containsKey(noteId) == false && + StringUtils.isBlank(interpreterName)) { + return; + } + String newInterpreterName = StringUtils.defaultIfEmpty(interpreterName, + DEFAULT_INTERPRETER_NAME); + occupiedInterpreterMap.put(noteId, newInterpreterName); } } @@ -94,10 +97,6 @@ public static void setOccupiedInterpreter(Paragraph p) { } String interpreterName = parseInterpreterName(p.getText()); - if (interpreterName == null) { - return; - } - setOccupiedInterpreter(p.getNote().getId(), interpreterName); } diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java index 12ff2bd995f..3c7d739be5c 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java @@ -88,6 +88,35 @@ public void parseInterpreterName() throws Exception { assertNull(OccupiedInterpreter.parseInterpreterName(text)); } + @Test + public void parseInterpreterName2() throws Exception { + String interpreter = "%md"; + + String text = "%md hello"; + System.out.println(OccupiedInterpreter.parseInterpreterName(text)); + + assertTrue(interpreter.equals(OccupiedInterpreter.parseInterpreterName(text))); + } + + @Test + public void parseInterpreterName3() throws Exception { + Note note = Mockito.mock(Note.class); + Mockito.when(note.getId()).thenReturn("test6"); + + Paragraph p1 = new Paragraph(); + p1 = Mockito.spy(p1); + Mockito.when(p1.getNote()).thenReturn(note); + Mockito.when(p1.getText()).thenReturn("%md" + System.lineSeparator() + "hello"); + OccupiedInterpreter.setOccupiedInterpreter(p1); + + Mockito.when(p1.getText()).thenReturn("println(\"Hello\")"); + OccupiedInterpreter.setOccupiedInterpreter(p1); + + String expectedInterpreter = OccupiedInterpreter.getDefaultInterpreterName(); + System.out.println(OccupiedInterpreter.getOccupiedInterpreter("test6")); + assertTrue(expectedInterpreter.equals(OccupiedInterpreter.getOccupiedInterpreter("test6"))); + } + @Test public void setInterpreterNameIfEmptyText() throws Exception { String noteId = "test5"; From 27f3925cfe861b843ab4976f37c19ac2241753ff Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Wed, 18 May 2016 08:59:24 +0900 Subject: [PATCH 04/19] [ZEPPELIN-707]Fixed ParagraphActionsIT test --- .../org/apache/zeppelin/integration/ParagraphActionsIT.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 d1135c052fb..12e6e8f5a32 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 @@ -21,6 +21,7 @@ import org.apache.zeppelin.AbstractZeppelinIT; import org.apache.zeppelin.WebDriverManager; import org.apache.zeppelin.ZeppelinITUtils; +import org.apache.zeppelin.notebook.OccupiedInterpreter; import org.hamcrest.CoreMatchers; import org.junit.After; import org.junit.Before; @@ -96,7 +97,7 @@ public void testCreateNewButton() throws Exception { collector.checkThat("Paragraph is created above", driver.findElement(By.xpath(getParagraphXPath(1) + "//div[contains(@class, 'editor')]")).getText(), - CoreMatchers.equalTo("")); + CoreMatchers.equalTo(OccupiedInterpreter.getDefaultInterpreterName())); setTextOfParagraph(1, " this is above "); newPara = driver.findElement(By.xpath(getParagraphXPath(2) + "//div[contains(@class,'new-paragraph')][2]")); @@ -106,7 +107,7 @@ public void testCreateNewButton() throws Exception { collector.checkThat("Paragraph is created below", driver.findElement(By.xpath(getParagraphXPath(3) + "//div[contains(@class, 'editor')]")).getText(), - CoreMatchers.equalTo("")); + CoreMatchers.equalTo(OccupiedInterpreter.getDefaultInterpreterName())); setTextOfParagraph(3, " this is below "); collector.checkThat("The output field of paragraph1 contains", From 7dd3f68a8aefa3f5f041aeb30a9225e6a3da0ea0 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Mon, 30 May 2016 21:10:55 +0900 Subject: [PATCH 05/19] [ZEPPELIN-707]Make %.. is default interpreter --- .../org/apache/zeppelin/socket/NotebookServer.java | 1 + .../zeppelin/integration/ParagraphActionsIT.java | 5 +---- .../org/apache/zeppelin/rest/ZeppelinRestApiTest.java | 5 +---- .../{notebook => interpreter}/OccupiedInterpreter.java | 7 ++++++- .../main/java/org/apache/zeppelin/notebook/Note.java | 1 - .../zeppelin/notebook/NoteInterpreterLoader.java | 4 +++- .../java/org/apache/zeppelin/notebook/Paragraph.java | 4 +++- .../OccupiedInterpreterTest.java | 10 ++++++---- 8 files changed, 21 insertions(+), 16 deletions(-) rename zeppelin-zengine/src/main/java/org/apache/zeppelin/{notebook => interpreter}/OccupiedInterpreter.java (95%) rename zeppelin-zengine/src/test/java/org/apache/zeppelin/{notebook => interpreter}/OccupiedInterpreterTest.java (94%) 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 74ab9bd5fac..a65e6e20222 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 @@ -35,6 +35,7 @@ import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.AngularObjectRegistryListener; import org.apache.zeppelin.interpreter.InterpreterGroup; +import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; import org.apache.zeppelin.user.AuthenticationInfo; 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 12e6e8f5a32..158dab10236 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 @@ -21,7 +21,7 @@ import org.apache.zeppelin.AbstractZeppelinIT; import org.apache.zeppelin.WebDriverManager; import org.apache.zeppelin.ZeppelinITUtils; -import org.apache.zeppelin.notebook.OccupiedInterpreter; +import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.hamcrest.CoreMatchers; import org.junit.After; import org.junit.Before; @@ -29,14 +29,11 @@ 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); 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 efe8f1dfe61..d4ec4e2871a 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 @@ -25,13 +25,10 @@ import org.apache.commons.httpclient.methods.DeleteMethod; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.httpclient.methods.PostMethod; -import org.apache.commons.httpclient.methods.PutMethod; import org.apache.commons.lang3.StringUtils; -import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.Note; -import org.apache.zeppelin.notebook.OccupiedInterpreter; +import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.apache.zeppelin.notebook.Paragraph; -import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.server.ZeppelinServer; import org.junit.AfterClass; import org.junit.BeforeClass; diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java similarity index 95% rename from zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java rename to zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java index 5721386cd9a..80e590299c9 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/OccupiedInterpreter.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java @@ -15,9 +15,10 @@ * limitations under the License. */ -package org.apache.zeppelin.notebook; +package org.apache.zeppelin.interpreter; import org.apache.commons.lang.StringUtils; +import org.apache.zeppelin.notebook.Paragraph; import java.util.HashMap; import java.util.regex.Matcher; @@ -52,6 +53,10 @@ public static String getDefaultInterpreterName() { return DEFAULT_INTERPRETER_NAME; } + public static boolean isDefaultInterpreter(String replName) { + return DEFAULT_INTERPRETER_NAME.equals(replName); + } + /** * Remove Occupied interpreter name * 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 0e49005d431..3b6981145c7 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,7 +20,6 @@ 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; 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 d2a1e847ab5..1ba2a0cdc97 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 @@ -28,6 +28,7 @@ import org.apache.zeppelin.interpreter.InterpreterFactory; import org.apache.zeppelin.interpreter.InterpreterGroup; import org.apache.zeppelin.interpreter.InterpreterSetting; +import org.apache.zeppelin.interpreter.OccupiedInterpreter; /** * Interpreter loader per note. @@ -117,7 +118,8 @@ public Interpreter get(String replName) { return null; } - if (replName == null || replName.trim().length() == 0) { + if (replName == null || replName.trim().length() == 0 + || OccupiedInterpreter.isDefaultInterpreter(replName)) { // get default settings (first available) InterpreterSetting defaultSettings = settings.get(0); return createOrGetInterpreterList(defaultSettings).get(0); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java index 4e02fb18806..9a2b0dc0307 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java @@ -168,7 +168,9 @@ public static String getRequiredReplName(String text) { return null; } String head = text.substring(0, scriptHeadIndex); - if (head.startsWith("%")) { + if (OccupiedInterpreter.isDefaultInterpreter(head)) { + return head; + } else if (head.startsWith("%")) { return head.substring(1); } else { return null; diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.java similarity index 94% rename from zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java rename to zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.java index 3c7d739be5c..3ecf2d45a23 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/OccupiedInterpreterTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.java @@ -15,8 +15,10 @@ * limitations under the License. */ -package org.apache.zeppelin.notebook; +package org.apache.zeppelin.interpreter; +import org.apache.zeppelin.notebook.Note; +import org.apache.zeppelin.notebook.Paragraph; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -55,7 +57,7 @@ public void setOccupiedInterpreterFromParagraph() throws Exception { String noteId = "test3"; String interpreter = "%sh"; - Paragraph p = new Paragraph(); + Paragraph p = new Paragraph(null, null, null); p.setText(interpreter + System.lineSeparator() + "echo hello"); p = Mockito.spy(p); @@ -103,7 +105,7 @@ public void parseInterpreterName3() throws Exception { Note note = Mockito.mock(Note.class); Mockito.when(note.getId()).thenReturn("test6"); - Paragraph p1 = new Paragraph(); + Paragraph p1 = new Paragraph(null, null, null); p1 = Mockito.spy(p1); Mockito.when(p1.getNote()).thenReturn(note); Mockito.when(p1.getText()).thenReturn("%md" + System.lineSeparator() + "hello"); @@ -121,7 +123,7 @@ public void parseInterpreterName3() throws Exception { public void setInterpreterNameIfEmptyText() throws Exception { String noteId = "test5"; - Paragraph p = new Paragraph(); + Paragraph p = new Paragraph(null, null, null); p = Mockito.spy(p); Note note = Mockito.mock(Note.class); From 3c7485d7cc9eeac990883e1494dde5f31f6f543d Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Mon, 30 May 2016 22:47:50 +0900 Subject: [PATCH 06/19] [ZEPPELIN-707]Fix go to line end --- zeppelin-web/src/app/notebook/notebook.controller.js | 1 + .../src/app/notebook/paragraph/paragraph.controller.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js index 32c659f33ee..47bdc5b008d 100644 --- a/zeppelin-web/src/app/notebook/notebook.controller.js +++ b/zeppelin-web/src/app/notebook/notebook.controller.js @@ -549,6 +549,7 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl', for (var f=0; f<$scope.note.paragraphs.length; f++) { if (paragraphToBeFocused === $scope.note.paragraphs[f].id) { $scope.note.paragraphs[f].focus = true; + angular.element('#' + paragraphId + '_paragraphColumn_main').scope().goToLineEnd(); } } }; diff --git a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js index 21a17b221de..b6a0300aa1f 100644 --- a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js +++ b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js @@ -1001,6 +1001,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; From 854e96b8cbc0867143480af60546f8cc24e06fec Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Mon, 30 May 2016 23:08:29 +0900 Subject: [PATCH 07/19] [ZEPPELIN-707]Fix 'paragraphId' used out of scope. --- zeppelin-web/src/app/notebook/notebook.controller.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js index 47bdc5b008d..3420ba5a206 100644 --- a/zeppelin-web/src/app/notebook/notebook.controller.js +++ b/zeppelin-web/src/app/notebook/notebook.controller.js @@ -549,7 +549,10 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl', for (var f=0; f<$scope.note.paragraphs.length; f++) { if (paragraphToBeFocused === $scope.note.paragraphs[f].id) { $scope.note.paragraphs[f].focus = true; - angular.element('#' + paragraphId + '_paragraphColumn_main').scope().goToLineEnd(); + var paragraph = angular.element('#' + paragraphToBeFocused + '_paragraphColumn_main').scope(); + if(paragraph) { + paragraph.goToLineEnd(); + } } } }; From caeb9a23ef367de3185abb5ae601df978ef94d22 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Tue, 7 Jun 2016 16:55:05 +0900 Subject: [PATCH 08/19] [ZEPPELIN-707]Use ConcurrentHashMap instead of HashMap --- .../interpreter/OccupiedInterpreter.java | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java index 80e590299c9..61fda306dd5 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java @@ -20,7 +20,8 @@ import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.notebook.Paragraph; -import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -30,7 +31,7 @@ public class OccupiedInterpreter { private static final Pattern INTERPRETER_NAME_PATTERN = Pattern.compile("^%\\S+"); private static final String DEFAULT_INTERPRETER_NAME = "%.."; - private static final HashMap occupiedInterpreterMap = new HashMap<>(); + private static final Map occupiedInterpreterMap = new ConcurrentHashMap<>(); private OccupiedInterpreter() { } @@ -39,13 +40,14 @@ private OccupiedInterpreter() { * Get current occupied interpreter name * * @param noteId Note Id - * @return Current occupied interpreter name, or "" if no mapping occupied interpreter name. + * @return Current occupied interpreter name, or {@link OccupiedInterpreter#DEFAULT_INTERPRETER_NAME} + * if no mapping occupied interpreter name. */ public static String getOccupiedInterpreter(String noteId) { - String occupiedInterpreter; - synchronized (occupiedInterpreterMap) { - occupiedInterpreter = occupiedInterpreterMap.get(noteId); + if (StringUtils.isBlank(noteId)) { + return DEFAULT_INTERPRETER_NAME; } + String occupiedInterpreter = occupiedInterpreterMap.get(noteId); return StringUtils.defaultString(occupiedInterpreter, DEFAULT_INTERPRETER_NAME); } @@ -66,9 +68,7 @@ public static void removeOccupiedInterpreter(String noteId) { if (StringUtils.isBlank(noteId)) { return; } - synchronized (occupiedInterpreterMap) { - occupiedInterpreterMap.remove(noteId); - } + occupiedInterpreterMap.remove(noteId); } /** @@ -77,17 +77,13 @@ public static void removeOccupiedInterpreter(String noteId) { * @param noteId Note Id * @param interpreterName Current occupied interpreter name */ - @SuppressWarnings("PointlessBooleanExpression") public static void setOccupiedInterpreter(String noteId, String interpreterName) { - synchronized (occupiedInterpreterMap) { - if (occupiedInterpreterMap.containsKey(noteId) == false && - StringUtils.isBlank(interpreterName)) { - return; - } - String newInterpreterName = StringUtils.defaultIfEmpty(interpreterName, - DEFAULT_INTERPRETER_NAME); - occupiedInterpreterMap.put(noteId, newInterpreterName); + if (StringUtils.isBlank(noteId)) { + return; } + String newInterpreterName = StringUtils.defaultIfEmpty(interpreterName, + DEFAULT_INTERPRETER_NAME); + occupiedInterpreterMap.put(noteId, newInterpreterName); } /** From 2366709734741cdfdb18ed4845441dd0c8ac073b Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Tue, 7 Jun 2016 17:06:46 +0900 Subject: [PATCH 09/19] [ZEPPELIN-707]Use ConcurrentHashMap instead of HashMap --- .../org/apache/zeppelin/interpreter/OccupiedInterpreter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java index 61fda306dd5..fe7a79f1764 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java @@ -40,8 +40,8 @@ private OccupiedInterpreter() { * Get current occupied interpreter name * * @param noteId Note Id - * @return Current occupied interpreter name, or {@link OccupiedInterpreter#DEFAULT_INTERPRETER_NAME} - * if no mapping occupied interpreter name. + * @return Current occupied interpreter name, or + * {@link OccupiedInterpreter#DEFAULT_INTERPRETER_NAME} if no mapping occupied interpreter name. */ public static String getOccupiedInterpreter(String noteId) { if (StringUtils.isBlank(noteId)) { From 1484d1cf84941e8489963c6a3dc4c12428c714e7 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Fri, 10 Jun 2016 13:40:22 +0900 Subject: [PATCH 10/19] [ZEPPELIN-707]Change cursor of new paragraph go at the end of the line --- zeppelin-web/src/app/notebook/notebook.controller.js | 4 ---- .../src/app/notebook/paragraph/paragraph.controller.js | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js index 3420ba5a206..32c659f33ee 100644 --- a/zeppelin-web/src/app/notebook/notebook.controller.js +++ b/zeppelin-web/src/app/notebook/notebook.controller.js @@ -549,10 +549,6 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl', for (var f=0; f<$scope.note.paragraphs.length; f++) { if (paragraphToBeFocused === $scope.note.paragraphs[f].id) { $scope.note.paragraphs[f].focus = true; - var paragraph = angular.element('#' + paragraphToBeFocused + '_paragraphColumn_main').scope(); - if(paragraph) { - paragraph.goToLineEnd(); - } } } }; diff --git a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js index b6a0300aa1f..2cf0222ae81 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); From 532e98edb615a7e47be3096cfd39017726d2a8b1 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Fri, 10 Jun 2016 13:47:02 +0900 Subject: [PATCH 11/19] [ZEPPELIN-707]Fix cannot find symbol error --- .../java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 d4ec4e2871a..a93bdede65a 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 @@ -25,10 +25,12 @@ import org.apache.commons.httpclient.methods.DeleteMethod; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.httpclient.methods.PostMethod; +import org.apache.commons.httpclient.methods.PutMethod; import org.apache.commons.lang3.StringUtils; +import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.Note; -import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.apache.zeppelin.notebook.Paragraph; +import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.server.ZeppelinServer; import org.junit.AfterClass; import org.junit.BeforeClass; From f3ab5ea7d49e1d1d62d05a9c804d0b54868768da Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Fri, 10 Jun 2016 13:47:35 +0900 Subject: [PATCH 12/19] [ZEPPELIN-707]Fix cannot find symbol error --- .../test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java | 1 + 1 file changed, 1 insertion(+) 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 a93bdede65a..5aa3fd8b92d 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 @@ -29,6 +29,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.Note; +import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.apache.zeppelin.notebook.Paragraph; import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.server.ZeppelinServer; From 334b15ff7a81a083ecff6bcc780fd56b867cbcfe Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 13:56:51 +0900 Subject: [PATCH 13/19] [ZEPPELIN-707]Remove OccupiedInterpreter class. Add lastReplName in Note.java --- .../zeppelin/socket/NotebookServer.java | 2 +- .../integration/ParagraphActionsIT.java | 6 +- .../zeppelin/rest/ZeppelinRestApiTest.java | 4 +- .../interpreter/OccupiedInterpreter.java | 135 ----------------- .../org/apache/zeppelin/notebook/Note.java | 45 +++++- .../notebook/NoteInterpreterLoader.java | 22 ++- .../apache/zeppelin/notebook/Notebook.java | 1 + .../apache/zeppelin/notebook/Paragraph.java | 4 +- .../interpreter/OccupiedInterpreterTest.java | 139 ------------------ 9 files changed, 66 insertions(+), 292 deletions(-) delete mode 100644 zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java delete mode 100644 zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.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 a65e6e20222..17650dd10cc 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 @@ -35,7 +35,6 @@ import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.AngularObjectRegistryListener; import org.apache.zeppelin.interpreter.InterpreterGroup; -import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; import org.apache.zeppelin.user.AuthenticationInfo; @@ -1076,6 +1075,7 @@ private void runParagraph(NotebookSocket conn, HashSet userAndRoles, Not // 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(); } 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 158dab10236..a76706b9ff3 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,10 +18,10 @@ 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; -import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.hamcrest.CoreMatchers; import org.junit.After; import org.junit.Before; @@ -94,7 +94,7 @@ public void testCreateNewButton() throws Exception { collector.checkThat("Paragraph is created above", driver.findElement(By.xpath(getParagraphXPath(1) + "//div[contains(@class, 'editor')]")).getText(), - CoreMatchers.equalTo(OccupiedInterpreter.getDefaultInterpreterName())); + CoreMatchers.equalTo(StringUtils.EMPTY)); setTextOfParagraph(1, " this is above "); newPara = driver.findElement(By.xpath(getParagraphXPath(2) + "//div[contains(@class,'new-paragraph')][2]")); @@ -104,7 +104,7 @@ public void testCreateNewButton() throws Exception { collector.checkThat("Paragraph is created below", driver.findElement(By.xpath(getParagraphXPath(3) + "//div[contains(@class, 'editor')]")).getText(), - CoreMatchers.equalTo(OccupiedInterpreter.getDefaultInterpreterName())); + CoreMatchers.equalTo(StringUtils.EMPTY)); setTextOfParagraph(3, " this is below "); collector.checkThat("The output field of paragraph1 contains", 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 5aa3fd8b92d..4390d74b495 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 @@ -29,7 +29,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.Note; -import org.apache.zeppelin.interpreter.OccupiedInterpreter; import org.apache.zeppelin.notebook.Paragraph; import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.server.ZeppelinServer; @@ -147,8 +146,7 @@ public void testNotebookCreateWithParagraphs() throws IOException { 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()) - || OccupiedInterpreter.getDefaultInterpreterName().equals(p.getText())) { + if (StringUtils.isEmpty(p.getText())) { continue; } assertTrue("paragraph title check failed", p.getTitle().startsWith("title")); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java deleted file mode 100644 index fe7a79f1764..00000000000 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/OccupiedInterpreter.java +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.zeppelin.interpreter; - -import org.apache.commons.lang.StringUtils; -import org.apache.zeppelin.notebook.Paragraph; - -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * Current occupied interpreter information for each note - */ -public class OccupiedInterpreter { - private static final Pattern INTERPRETER_NAME_PATTERN = Pattern.compile("^%\\S+"); - private static final String DEFAULT_INTERPRETER_NAME = "%.."; - private static final Map occupiedInterpreterMap = new ConcurrentHashMap<>(); - - private OccupiedInterpreter() { - } - - /** - * Get current occupied interpreter name - * - * @param noteId Note Id - * @return Current occupied interpreter name, or - * {@link OccupiedInterpreter#DEFAULT_INTERPRETER_NAME} if no mapping occupied interpreter name. - */ - public static String getOccupiedInterpreter(String noteId) { - if (StringUtils.isBlank(noteId)) { - return DEFAULT_INTERPRETER_NAME; - } - String occupiedInterpreter = occupiedInterpreterMap.get(noteId); - return StringUtils.defaultString(occupiedInterpreter, DEFAULT_INTERPRETER_NAME); - } - - public static String getDefaultInterpreterName() { - return DEFAULT_INTERPRETER_NAME; - } - - public static boolean isDefaultInterpreter(String replName) { - return DEFAULT_INTERPRETER_NAME.equals(replName); - } - - /** - * Remove Occupied interpreter name - * - * @param noteId Note Id - */ - public static void removeOccupiedInterpreter(String noteId) { - if (StringUtils.isBlank(noteId)) { - return; - } - occupiedInterpreterMap.remove(noteId); - } - - /** - * Set current occupied interpreter name - * - * @param noteId Note Id - * @param interpreterName Current occupied interpreter name - */ - public static void setOccupiedInterpreter(String noteId, String interpreterName) { - if (StringUtils.isBlank(noteId)) { - return; - } - String newInterpreterName = StringUtils.defaultIfEmpty(interpreterName, - DEFAULT_INTERPRETER_NAME); - occupiedInterpreterMap.put(noteId, newInterpreterName); - } - - /** - * Set current occupied interpreter name - * - * @param p Paragraph - */ - public static void setOccupiedInterpreter(Paragraph p) { - if (p == null || p.getNote() == null || StringUtils.isEmpty(p.getNote().getId()) - || StringUtils.isEmpty(p.getText())) { - return; - } - - String interpreterName = parseInterpreterName(p.getText()); - setOccupiedInterpreter(p.getNote().getId(), interpreterName); - } - - /** - * Parse interpreter name - * - * @param text Paragraph's text - * @return interpreter name, or {@code null} if text is blank or no interpreter name. - */ - static String parseInterpreterName(String text) { - if (StringUtils.isBlank(text)) { - return null; - } - - Matcher matcher = INTERPRETER_NAME_PATTERN.matcher(text); - if (matcher.find()) { - return matcher.group(); - } - return null; - } - - /** - * Set interpreter name If Paragraph has empty text - * - * @param p Paragraph - */ - public static void setInterpreterNameIfEmptyText(Paragraph p) { - String noteId = p.getNote().getId(); - String interpreterName = getOccupiedInterpreter(noteId); - String text = p.getText(); - if (StringUtils.isEmpty(text)) { - p.setText(interpreterName); - } - } -} 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 3b6981145c7..d76720a3b44 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 @@ -23,8 +23,9 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; -import org.apache.zeppelin.conf.ZeppelinConfiguration; +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.display.AngularObject; import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.Input; @@ -39,6 +40,7 @@ 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; @@ -64,6 +66,7 @@ public class Note implements Serializable, JobListener { private String name = ""; private String id; + private AtomicReference lastReplName = new AtomicReference<>(StringUtils.EMPTY); private transient ZeppelinConfiguration conf = ZeppelinConfiguration.create(); @SuppressWarnings("rawtypes") @@ -107,6 +110,17 @@ private void generateId() { id = IdHashes.encode(System.currentTimeMillis() + new Random().nextInt()); } + private String getDefaultInterpreterName() { + Optional 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; } @@ -187,7 +201,7 @@ public Map> getAngularObjects() { public Paragraph addParagraph() { Paragraph p = new Paragraph(this, this, replLoader); - OccupiedInterpreter.setInterpreterNameIfEmptyText(p); + addLastReplNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(p); } @@ -231,13 +245,25 @@ public void addCloneParagraph(Paragraph srcParagraph) { */ public Paragraph insertParagraph(int index) { Paragraph p = new Paragraph(this, this, replLoader); - OccupiedInterpreter.setInterpreterNameIfEmptyText(p); + addLastReplNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(index, p); } return p; } + /** + * 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("%" + replName + " "); + } + } + /** * Remove paragraph by id. * @@ -388,6 +414,9 @@ public List> generateParagraphsInfo (){ 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; @@ -503,6 +532,16 @@ public void persist(AuthenticationInfo subject) throws IOException { 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 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 1ba2a0cdc97..611be5173a3 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; @@ -28,7 +30,6 @@ import org.apache.zeppelin.interpreter.InterpreterFactory; import org.apache.zeppelin.interpreter.InterpreterGroup; import org.apache.zeppelin.interpreter.InterpreterSetting; -import org.apache.zeppelin.interpreter.OccupiedInterpreter; /** * Interpreter loader per note. @@ -118,10 +119,9 @@ public Interpreter get(String replName) { return null; } - if (replName == null || replName.trim().length() == 0 - || OccupiedInterpreter.isDefaultInterpreter(replName)) { + if (replName == null || replName.trim().length() == 0) { // get default settings (first available) - InterpreterSetting defaultSettings = settings.get(0); + InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings); return createOrGetInterpreterList(defaultSettings).get(0); } @@ -158,7 +158,7 @@ public Interpreter get(String replName) { } 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); Interpreter.RegisteredInterpreter registeredInterpreter = Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName); if (registeredInterpreter != null) { @@ -192,4 +192,16 @@ public Interpreter get(String replName) { return null; } + + private InterpreterSetting getDefaultInterpreterSetting(List settings) { + return settings.get(0); + } + + Optional getDefaultInterpreterSetting() { + List settings = getInterpreterSettings(); + if (settings == null || settings.isEmpty()) { + return Optional.absent(); + } + return Optional.of(settings.get(0)); + } } 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 a6f581639ca..adaaa2ac4d0 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 Note createNote(List interpreterIds, AuthenticationInfo subject) } if (interpreterIds != null) { bindInterpretersToNote(note.id(), interpreterIds); + note.putDefaultReplName(); } notebookIndex.addIndexDoc(note); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java index 9a2b0dc0307..4e02fb18806 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java @@ -168,9 +168,7 @@ public static String getRequiredReplName(String text) { return null; } String head = text.substring(0, scriptHeadIndex); - if (OccupiedInterpreter.isDefaultInterpreter(head)) { - return head; - } else if (head.startsWith("%")) { + if (head.startsWith("%")) { return head.substring(1); } else { return null; diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.java deleted file mode 100644 index 3ecf2d45a23..00000000000 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/OccupiedInterpreterTest.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.zeppelin.interpreter; - -import org.apache.zeppelin.notebook.Note; -import org.apache.zeppelin.notebook.Paragraph; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mockito; -import org.mockito.runners.MockitoJUnitRunner; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -@RunWith(MockitoJUnitRunner.class) -public class OccupiedInterpreterTest { - @Test - public void getOccupiedInterpreter() throws Exception { - String noteId = "test1"; - assertTrue(OccupiedInterpreter.getDefaultInterpreterName() - .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); - } - - @Test - public void setOccupiedInterpreter() throws Exception { - String noteId = "test2"; - assertTrue(OccupiedInterpreter.getDefaultInterpreterName() - .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); - - String interpreterName = "interpreterName"; - OccupiedInterpreter.setOccupiedInterpreter(noteId, interpreterName); - - assertTrue(interpreterName.equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); - assertFalse(OccupiedInterpreter.getDefaultInterpreterName() - .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); - } - - @Test - public void setOccupiedInterpreterFromParagraph() throws Exception { - Note note = Mockito.mock(Note.class); - - String noteId = "test3"; - String interpreter = "%sh"; - Paragraph p = new Paragraph(null, null, null); - p.setText(interpreter + System.lineSeparator() + "echo hello"); - - p = Mockito.spy(p); - Mockito.when(p.getNote()).thenReturn(note); - Mockito.when(note.getId()).thenReturn(noteId); - OccupiedInterpreter.setOccupiedInterpreter(p); - - assertTrue(interpreter.equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); - - noteId = "test4"; - Mockito.when(note.getId()).thenReturn(noteId); - p.setText("This is test." + System.lineSeparator() + interpreter); - OccupiedInterpreter.setOccupiedInterpreter(p); - - System.out.println(OccupiedInterpreter.getOccupiedInterpreter(noteId)); - assertTrue(OccupiedInterpreter.getDefaultInterpreterName() - .equals(OccupiedInterpreter.getOccupiedInterpreter(noteId))); - } - - @Test - public void parseInterpreterName() throws Exception { - String interpreter = "%sh"; - - String text = interpreter + System.lineSeparator() + "echo hello" + interpreter + "hello"; - System.out.println(OccupiedInterpreter.parseInterpreterName(text)); - assertTrue(interpreter.equals(OccupiedInterpreter.parseInterpreterName(text))); - - text = "This is test." + System.lineSeparator() + interpreter; - System.out.println(OccupiedInterpreter.parseInterpreterName(text)); - assertNull(OccupiedInterpreter.parseInterpreterName(text)); - } - - @Test - public void parseInterpreterName2() throws Exception { - String interpreter = "%md"; - - String text = "%md hello"; - System.out.println(OccupiedInterpreter.parseInterpreterName(text)); - - assertTrue(interpreter.equals(OccupiedInterpreter.parseInterpreterName(text))); - } - - @Test - public void parseInterpreterName3() throws Exception { - Note note = Mockito.mock(Note.class); - Mockito.when(note.getId()).thenReturn("test6"); - - Paragraph p1 = new Paragraph(null, null, null); - p1 = Mockito.spy(p1); - Mockito.when(p1.getNote()).thenReturn(note); - Mockito.when(p1.getText()).thenReturn("%md" + System.lineSeparator() + "hello"); - OccupiedInterpreter.setOccupiedInterpreter(p1); - - Mockito.when(p1.getText()).thenReturn("println(\"Hello\")"); - OccupiedInterpreter.setOccupiedInterpreter(p1); - - String expectedInterpreter = OccupiedInterpreter.getDefaultInterpreterName(); - System.out.println(OccupiedInterpreter.getOccupiedInterpreter("test6")); - assertTrue(expectedInterpreter.equals(OccupiedInterpreter.getOccupiedInterpreter("test6"))); - } - - @Test - public void setInterpreterNameIfEmptyText() throws Exception { - String noteId = "test5"; - - Paragraph p = new Paragraph(null, null, null); - p = Mockito.spy(p); - Note note = Mockito.mock(Note.class); - - Mockito.when(p.getNote()).thenReturn(note); - Mockito.when(note.getId()).thenReturn(noteId); - - OccupiedInterpreter.setInterpreterNameIfEmptyText(p); - - System.out.println(OccupiedInterpreter.getDefaultInterpreterName()); - System.out.println(p.getText()); - assertTrue(OccupiedInterpreter.getDefaultInterpreterName().equals(p.getText())); - } -} \ No newline at end of file From 85ee14b738c7ceeed2959e41a5ad25fa132de2a9 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 14:36:35 +0900 Subject: [PATCH 14/19] [ZEPPELIN-707]Fix test in error --- .../apache/zeppelin/rest/ZeppelinRestApiTest.java | 3 ++- .../java/org/apache/zeppelin/notebook/Note.java | 14 +++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) 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 4390d74b495..00a81066316 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 void testNotebookCreateWithParagraphs() throws IOException { 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")); 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 d76720a3b44..e0b7b9bc33f 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 @@ -260,10 +260,14 @@ public Paragraph insertParagraph(int index) { private void addLastReplNameIfEmptyText(Paragraph p) { String replName = lastReplName.get(); if (StringUtils.isEmpty(p.getText()) && StringUtils.isNotEmpty(replName)) { - p.setText("%" + replName + " "); + p.setText(getInterpreterName(replName) + " "); } } + private String getInterpreterName(String replName) { + return "%" + replName; + } + /** * Remove paragraph by id. * @@ -607,6 +611,14 @@ public void setInfo(Map info) { 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) { } From 0fd122c34d2821ffbe11c7fd5806f608a2e8d24f Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 17:08:09 +0900 Subject: [PATCH 15/19] [ZEPPELIN-707]Modify checking nullable list location --- .../zeppelin/notebook/NoteInterpreterLoader.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 611be5173a3..3c432cbb12e 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 @@ -121,7 +121,7 @@ public Interpreter get(String replName) { if (replName == null || replName.trim().length() == 0) { // get default settings (first available) - InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings); + InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings).get(); return createOrGetInterpreterList(defaultSettings).get(0); } @@ -158,7 +158,7 @@ public Interpreter get(String replName) { } else { // first assume replName is 'name' of interpreter. ('groupName' is ommitted) // search 'name' from first (default) interpreter group - InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings); + InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings).get(); Interpreter.RegisteredInterpreter registeredInterpreter = Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName); if (registeredInterpreter != null) { @@ -193,15 +193,15 @@ public Interpreter get(String replName) { return null; } - private InterpreterSetting getDefaultInterpreterSetting(List settings) { - return settings.get(0); - } - - Optional getDefaultInterpreterSetting() { - List settings = getInterpreterSettings(); + private Optional + getDefaultInterpreterSetting(List settings) { if (settings == null || settings.isEmpty()) { return Optional.absent(); } return Optional.of(settings.get(0)); } + + Optional getDefaultInterpreterSetting() { + return getDefaultInterpreterSetting(getInterpreterSettings()); + } } From 4c0904bc77106c5dcc6b41524af2fe78050f875e Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 17:19:14 +0900 Subject: [PATCH 16/19] [ZEPPELIN-707]Resolved conflicts --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 1 + 1 file changed, 1 insertion(+) 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 e0b7b9bc33f..8a6e28af161 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 @@ -26,6 +26,7 @@ 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; import org.apache.zeppelin.display.Input; From 720ffba3aa725e04e863dfa89819b530789e2b20 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 17:19:39 +0900 Subject: [PATCH 17/19] [ZEPPELIN-707]Add note.setLastReplName in Rest API --- .../src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java | 1 + 1 file changed, 1 insertion(+) 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 87e7ccef8f5..c10dee87551 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 Response runParagraph(@PathParam("notebookId") String notebookId, if (paramsForUpdating != null) { paragraph.settings.getParams().putAll(paramsForUpdating); AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); + note.setLastReplName(paragraph.getId()); note.persist(subject); } } From a159903a136677faf60d7dc72e8dbf53b0f79b34 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 23:07:37 +0900 Subject: [PATCH 18/19] [ZEPPELIN-707]Modify return empty string when repl name is blank --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8a6e28af161..7108bb34367 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 @@ -266,7 +266,7 @@ private void addLastReplNameIfEmptyText(Paragraph p) { } private String getInterpreterName(String replName) { - return "%" + replName; + return StringUtils.isBlank(replName) ? StringUtils.EMPTY : "%" + replName; } /** From 3652dbbfb0d4f9dd75fa79a8b7591651388a9a17 Mon Sep 17 00:00:00 2001 From: Minwoo Kang Date: Thu, 16 Jun 2016 23:15:12 +0900 Subject: [PATCH 19/19] [ZEPPELIN-707]Add test case --- .../apache/zeppelin/notebook/NoteTest.java | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) 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 33a7ef2ed98..29da0583fcd 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.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 void runJdbcTest() { 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.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