Repository: zeppelin Updated Branches: refs/heads/master af1899136 -> f6b58ee5a
[HOTFIX] JDBC completions. fix cursor position ### What is this PR for? If text of paragraph contains interpreter name then parser works incorrect (cursor position incorrect). ### What type of PR is it? Bug Fix ### Screenshots (if appropriate) before  after  ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: tinkoff-dwh <tinkoff....@gmail.com> Author: Tinkoff DWH <tinkoff....@gmail.com> Closes #2495 from tinkoff-dwh/completion_hotfix and squashes the following commits: bac1ac2 [tinkoff-dwh] Merge remote-tracking branch 'upstream/master' into completion_hotfix 80c8e2a [Tinkoff DWH] small refactoring 5e9aa73 [tinkoff-dwh] add more data to dataset of test 9f3dbc7 [tinkoff-dwh] added test 43f8ffd [tinkoff-dwh] another solution to fix cursor position + small fix to return table names 4e6347a [Tinkoff DWH] fix cursor position for completions Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/f6b58ee5 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/f6b58ee5 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/f6b58ee5 Branch: refs/heads/master Commit: f6b58ee5a06d32af37903cc768e106079d267b2d Parents: af18991 Author: tinkoff-dwh <tinkoff....@gmail.com> Authored: Mon Jul 31 15:56:34 2017 +0500 Committer: Lee moon soo <m...@apache.org> Committed: Sun Aug 20 22:33:04 2017 -0700 ---------------------------------------------------------------------- .../org/apache/zeppelin/jdbc/SqlCompleter.java | 3 +- .../org/apache/zeppelin/notebook/Paragraph.java | 85 ++++++++++++++------ .../apache/zeppelin/notebook/ParagraphTest.java | 43 ++++++++-- 3 files changed, 97 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f6b58ee5/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java ---------------------------------------------------------------------- diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java index 46cc4bd..6103fc7 100644 --- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java +++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java @@ -179,7 +179,8 @@ public class SqlCompleter { private static void fillTableNames(String schema, DatabaseMetaData meta, Set<String> tables) { - try (ResultSet tbls = meta.getTables(schema, schema, "%", null)) { + try (ResultSet tbls = meta.getTables(schema, schema, "%", + new String[]{"TABLE", "VIEW", "ALIAS", "SYNONYM", "GLOBAL TEMPORARY", "LOCAL TEMPORARY"})) { while (tbls.next()) { String table = tbls.getString("TABLE_NAME"); tables.add(table); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f6b58ee5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java ---------------------------------------------------------------------- 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 ac3d19f..37138e6 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 @@ -17,35 +17,58 @@ package org.apache.zeppelin.notebook; -import com.google.common.collect.Maps; -import com.google.common.base.Strings; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.common.JsonSerializable; import org.apache.zeppelin.completer.CompletionType; import org.apache.zeppelin.display.AngularObject; import org.apache.zeppelin.display.AngularObjectRegistry; -import org.apache.zeppelin.helium.HeliumPackage; -import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; -import org.apache.zeppelin.user.AuthenticationInfo; -import org.apache.zeppelin.user.Credentials; -import org.apache.zeppelin.user.UserCredentials; import org.apache.zeppelin.display.GUI; import org.apache.zeppelin.display.Input; -import org.apache.zeppelin.interpreter.*; +import org.apache.zeppelin.helium.HeliumPackage; +import org.apache.zeppelin.interpreter.Interpreter; import org.apache.zeppelin.interpreter.Interpreter.FormType; +import org.apache.zeppelin.interpreter.InterpreterContext; +import org.apache.zeppelin.interpreter.InterpreterContextRunner; +import org.apache.zeppelin.interpreter.InterpreterException; +import org.apache.zeppelin.interpreter.InterpreterFactory; +import org.apache.zeppelin.interpreter.InterpreterInfo; +import org.apache.zeppelin.interpreter.InterpreterOption; +import org.apache.zeppelin.interpreter.InterpreterOutput; +import org.apache.zeppelin.interpreter.InterpreterOutputListener; +import org.apache.zeppelin.interpreter.InterpreterResult; import org.apache.zeppelin.interpreter.InterpreterResult.Code; +import org.apache.zeppelin.interpreter.InterpreterResultMessage; +import org.apache.zeppelin.interpreter.InterpreterResultMessageOutput; +import org.apache.zeppelin.interpreter.InterpreterSetting; +import org.apache.zeppelin.interpreter.InterpreterSettingManager; +import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; import org.apache.zeppelin.resource.ResourcePool; import org.apache.zeppelin.scheduler.Job; import org.apache.zeppelin.scheduler.JobListener; import org.apache.zeppelin.scheduler.Scheduler; +import org.apache.zeppelin.user.AuthenticationInfo; +import org.apache.zeppelin.user.Credentials; +import org.apache.zeppelin.user.UserCredentials; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; -import java.io.Serializable; -import java.util.*; - import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import com.google.common.collect.Maps; /** * Paragraph is a representation of an execution unit. @@ -204,7 +227,7 @@ public class Paragraph extends Job implements Cloneable, JsonSerializable { } public String getRequiredReplName() { - return getRequiredReplName(text); + return getRequiredReplName(text != null ? text.trim() : text); } public static String getRequiredReplName(String text) { @@ -212,15 +235,14 @@ public class Paragraph extends Job implements Cloneable, JsonSerializable { return null; } - String trimmed = text.trim(); - if (!trimmed.startsWith("%")) { + if (!text.startsWith("%")) { return null; } // get script head int scriptHeadIndex = 0; - for (int i = 0; i < trimmed.length(); i++) { - char ch = trimmed.charAt(i); + for (int i = 0; i < text.length(); i++) { + char ch = text.charAt(i); if (Character.isWhitespace(ch) || ch == '(' || ch == '\n') { break; } @@ -247,11 +269,10 @@ public class Paragraph extends Job implements Cloneable, JsonSerializable { return text; } - String trimmed = text.trim(); - if (magic.length() + 1 >= trimmed.length()) { + if (magic.length() + 1 >= text.length()) { return ""; } - return trimmed.substring(magic.length() + 1).trim(); + return text.substring(magic.length() + 1).trim(); } public Interpreter getRepl(String name) { @@ -288,13 +309,12 @@ public class Paragraph extends Job implements Cloneable, JsonSerializable { return getInterpreterCompletion(); } } + String trimmedBuffer = buffer != null ? buffer.trim() : null; + cursor = calculateCursorPosition(buffer, trimmedBuffer, cursor); - String replName = getRequiredReplName(buffer); - if (replName != null && cursor > replName.length()) { - cursor -= replName.length() + 1; - } + String replName = getRequiredReplName(trimmedBuffer); - String body = getScriptBody(buffer); + String body = getScriptBody(trimmedBuffer); Interpreter repl = getRepl(replName); if (repl == null) { return null; @@ -306,6 +326,21 @@ public class Paragraph extends Job implements Cloneable, JsonSerializable { return completion; } + public int calculateCursorPosition(String buffer, String trimmedBuffer, int cursor) { + int countWhitespacesAtStart = buffer.indexOf(trimmedBuffer); + if (countWhitespacesAtStart > 0) { + cursor -= countWhitespacesAtStart; + } + + String replName = getRequiredReplName(trimmedBuffer); + if (replName != null && cursor > replName.length()) { + String body = trimmedBuffer.substring(replName.length() + 1); + cursor -= replName.length() + 1 + body.indexOf(body.trim()); + } + + return cursor; + } + public void setInterpreterFactory(InterpreterFactory factory) { this.factory = factory; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f6b58ee5/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java index 0e77846..7bd6819 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java @@ -21,10 +21,7 @@ package org.apache.zeppelin.notebook; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -33,7 +30,11 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.collect.Lists; + +import java.util.Arrays; import java.util.List; + +import org.apache.commons.lang3.tuple.Triple; import org.apache.zeppelin.display.AngularObject; import org.apache.zeppelin.display.AngularObjectBuilder; import org.apache.zeppelin.display.AngularObjectRegistry; @@ -41,7 +42,6 @@ import org.apache.zeppelin.display.Input; import org.apache.zeppelin.interpreter.Interpreter; import org.apache.zeppelin.interpreter.Interpreter.FormType; import org.apache.zeppelin.interpreter.InterpreterContext; -import org.apache.zeppelin.interpreter.InterpreterFactory; import org.apache.zeppelin.interpreter.InterpreterGroup; import org.apache.zeppelin.interpreter.InterpreterOption; import org.apache.zeppelin.interpreter.InterpreterResult; @@ -52,14 +52,12 @@ import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.interpreter.InterpreterSetting.Status; import org.apache.zeppelin.interpreter.InterpreterSettingManager; import org.apache.zeppelin.resource.ResourcePool; -import org.apache.zeppelin.scheduler.JobListener; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.user.Credentials; import org.junit.Test; import java.util.HashMap; import java.util.Map; -import org.mockito.ArgumentCaptor; import org.mockito.Mockito; public class ParagraphTest { @@ -228,8 +226,37 @@ public class ParagraphTest { assertNotEquals(p1.getReturn().toString(), p2.getReturn().toString()); assertEquals(p1, spyParagraph.getUserParagraph(user1.getUser())); + } - - + @Test + public void testCursorPosition() { + Paragraph paragraph = spy(new Paragraph()); + doReturn(null).when(paragraph).getRepl(anyString()); + // left = buffer, middle = cursor position into source code, right = cursor position after parse + List<Triple<String, Integer, Integer>> dataSet = Arrays.asList( + Triple.of("%jdbc schema.", 13, 7), + Triple.of(" %jdbc schema.", 16, 7), + Triple.of(" \n%jdbc schema.", 15, 7), + Triple.of("%jdbc schema.table. ", 19, 13), + Triple.of("%jdbc schema.\n\n", 13, 7), + Triple.of(" %jdbc schema.tab\n\n", 18, 10), + Triple.of(" \n%jdbc schema.\n \n", 16, 7), + Triple.of(" \n%jdbc schema.\n \n", 16, 7), + Triple.of(" \n%jdbc\n\n schema\n \n", 17, 6), + Triple.of("%another\n\n schema.", 18, 7), + Triple.of("\n\n schema.", 10, 7), + Triple.of("schema.", 7, 7), + Triple.of("schema. \n", 7, 7), + Triple.of(" \n %jdbc", 11, 0), + Triple.of("\n %jdbc", 9, 0), + Triple.of("%jdbc \n schema", 16, 6), + Triple.of("%jdbc \n \n schema", 20, 6) + ); + + for (Triple<String, Integer, Integer> data : dataSet) { + Integer actual = paragraph.calculateCursorPosition(data.getLeft(), data.getLeft().trim(), data.getMiddle()); + assertEquals(data.getRight(), actual); + } } + }
