Skip to content

Commit

Permalink
[HOTFIX] JDBC completions. fix cursor position
Browse files Browse the repository at this point in the history 
### 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
![before](https://user-images.githubusercontent.com/25951039/28326867-55195f50-6bfb-11e7-8f93-c381658d598a.png)
after
![after](https://user-images.githubusercontent.com/25951039/28326904-6f053fa6-6bfb-11e7-8586-00da4fa95b21.png)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: tinkoff-dwh <[email protected]>
Author: Tinkoff DWH <[email protected]>

Closes apache#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
  • Loading branch information
@tinkoff-dwh @Leemoonsoo
tinkoff-dwh authored and Leemoonsoo committed Aug 21, 2017
1 parent af18991 commit f6b58ee
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 34 deletions.
3 changes: 2 additions & 1 deletion jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ private static Set<String> getCatalogNames(DatabaseMetaData meta, List<String> s


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);
Expand Down
85 changes: 60 additions & 25 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -204,23 +227,22 @@ public boolean isEnabled() {
}

public String getRequiredReplName() {
return getRequiredReplName(text);
return getRequiredReplName(text != null ? text.trim() : text);
}

public static String getRequiredReplName(String text) {
if (text == null) {
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;
}
Expand All @@ -247,11 +269,10 @@ public static String getScriptBody(String text) {
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) {
Expand Down Expand Up @@ -288,13 +309,12 @@ public List<InterpreterCompletion> completion(String buffer, int cursor) {
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;
Expand All @@ -306,6 +326,21 @@ public List<InterpreterCompletion> completion(String buffer, int cursor) {
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;
}
Expand Down
43 changes: 35 additions & 8 deletions zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@
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;
Expand All @@ -33,15 +30,18 @@
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;
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;
Expand All @@ -52,14 +52,12 @@
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 {
Expand Down Expand Up @@ -228,8 +226,37 @@ public void returnUnchangedResultsWithDifferentUser() throws Throwable {
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);
}
}

}

0 comments on commit f6b58ee

Please sign in to comment.