This is an automated email from the ASF dual-hosted git repository. pdallig pushed a commit to branch branch-0.9 in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/branch-0.9 by this push: new 083c261 [ZEPPELIN-5425] Polish Interpreter class 083c261 is described below commit 083c261013df25cada846dd25627e1778c12aaaf Author: cuspymd <cusp...@gmail.com> AuthorDate: Fri Jul 2 00:13:07 2021 +0900 [ZEPPELIN-5425] Polish Interpreter class ### What is this PR for? - Move `interpolate()` function into `AbstractInterpreter` class which is only one caller of this function - Fix IDE warning messages ### What type of PR is it? [Refactoring] ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-5425 ### How should this be tested? * Unit tests and CI ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: cuspymd <cusp...@gmail.com> Closes #4150 from cuspymd/polish-interpreter-class and squashes the following commits: 1732197c6 [cuspymd] Apply review comment 019cdd16c [cuspymd] Polish Interpreter class (cherry picked from commit c7b2ea1c41d10978854496f74901c4e3c9a2b393) Signed-off-by: Philipp Dallig <philipp.dal...@gmail.com> --- .../zeppelin/interpreter/AbstractInterpreter.java | 37 ++++++ .../apache/zeppelin/interpreter/Interpreter.java | 43 +------ .../zeppelin/interpreter/ZeppCtxtVariableTest.java | 124 ++++++--------------- 3 files changed, 79 insertions(+), 125 deletions(-) diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/AbstractInterpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/AbstractInterpreter.java index 75d1885..de51bb5 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/AbstractInterpreter.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/AbstractInterpreter.java @@ -18,13 +18,21 @@ package org.apache.zeppelin.interpreter; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; +import org.apache.zeppelin.resource.Resource; +import org.apache.zeppelin.resource.ResourcePool; import java.util.ArrayList; import java.util.List; import java.util.Properties; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public abstract class AbstractInterpreter extends Interpreter { + private static final Pattern VARIABLES = Pattern.compile("([^{}]*)([{]+[^{}]*[}]+)(.*)", Pattern.DOTALL); + private static final Pattern VARIABLE_IN_BRACES = Pattern.compile("[{][^{}]+[}]"); + private static final Pattern VARIABLE_IN_DOUBLE_BRACES = Pattern.compile("[{]{2}[^{}]+[}]{2}"); + public AbstractInterpreter(Properties properties) { super(properties); } @@ -47,6 +55,35 @@ public abstract class AbstractInterpreter extends Interpreter { return internalInterpret(st, context); } + static String interpolate(String cmd, ResourcePool resourcePool) { + + StringBuilder sb = new StringBuilder(); + Matcher m; + String st = cmd; + while ((m = VARIABLES.matcher(st)).matches()) { + sb.append(m.group(1)); + String varPat = m.group(2); + if (VARIABLE_IN_BRACES.matcher(varPat).matches()) { + // substitute {variable} only if 'variable' has a value ... + Resource resource = resourcePool.get(varPat.substring(1, varPat.length() - 1)); + Object variableValue = resource == null ? null : resource.get(); + if (variableValue != null) + sb.append(variableValue); + else + return cmd; + } else if (VARIABLE_IN_DOUBLE_BRACES.matcher(varPat).matches()) { + // escape {{text}} ... + sb.append("{").append(varPat, 2, varPat.length() - 2).append("}"); + } else { + // mismatched {{ }} or more than 2 braces ... + return cmd; + } + st = m.group(3); + } + sb.append(st); + return sb.toString(); + } + public abstract ZeppelinContext getZeppelinContext(); protected boolean isInterpolate() { diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java index 1cff609..bf09cb7 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java @@ -23,8 +23,6 @@ import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.zeppelin.annotation.Experimental; import org.apache.zeppelin.annotation.ZeppelinApi; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; -import org.apache.zeppelin.resource.Resource; -import org.apache.zeppelin.resource.ResourcePool; import org.apache.zeppelin.scheduler.Scheduler; import org.apache.zeppelin.scheduler.SchedulerFactory; import org.slf4j.Logger; @@ -38,8 +36,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Interface for interpreters. @@ -82,35 +78,6 @@ public abstract class Interpreter { return null; } - protected String interpolate(String cmd, ResourcePool resourcePool) { - Pattern zVariablePattern = Pattern.compile("([^{}]*)([{]+[^{}]*[}]+)(.*)", Pattern.DOTALL); - StringBuilder sb = new StringBuilder(); - Matcher m; - String st = cmd; - while ((m = zVariablePattern.matcher(st)).matches()) { - sb.append(m.group(1)); - String varPat = m.group(2); - if (varPat.matches("[{][^{}]+[}]")) { - // substitute {variable} only if 'variable' has a value ... - Resource resource = resourcePool.get(varPat.substring(1, varPat.length() - 1)); - Object variableValue = resource == null ? null : resource.get(); - if (variableValue != null) - sb.append(variableValue); - else - return cmd; - } else if (varPat.matches("[{]{2}[^{}]+[}]{2}")) { - // escape {{text}} ... - sb.append("{").append(varPat.substring(2, varPat.length() - 2)).append("}"); - } else { - // mismatched {{ }} or more than 2 braces ... - return cmd; - } - st = m.group(3); - } - sb.append(st); - return sb.toString(); - } - /** * Run code and return result, in synchronous way. * @@ -414,12 +381,12 @@ public abstract class Interpreter { */ public static class RegisteredInterpreter { - private String group; - private String name; - private String className; + private final String group; + private final String name; + private final String className; private boolean defaultInterpreter; - private Map<String, DefaultInterpreterProperty> properties; - private Map<String, Object> editor; + private final Map<String, DefaultInterpreterProperty> properties; + private final Map<String, Object> editor; private Map<String, Object> config; private String path; private InterpreterOption option; diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/ZeppCtxtVariableTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/ZeppCtxtVariableTest.java index 14b4b6b..e5ea4d2 100644 --- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/ZeppCtxtVariableTest.java +++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/ZeppCtxtVariableTest.java @@ -23,182 +23,132 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; -import java.util.Properties; - -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; public class ZeppCtxtVariableTest { - public static class TestInterpreter extends Interpreter { - - TestInterpreter(Properties property) { - super(property); - } - - @Override - public void open() { - } - - @Override - public void close() { - } - - @Override - public InterpreterResult interpret(String st, InterpreterContext context) { - return null; - } - - @Override - public void cancel(InterpreterContext context) { - } - - @Override - public FormType getFormType() { - return null; - } - - @Override - public int getProgress(InterpreterContext context) { - return 0; - } - } - - private Interpreter interpreter; private ResourcePool resourcePool; @Before public void setUp() throws Exception { - resourcePool = new LocalResourcePool("ZeppelinContextVariableInterpolationTest"); - - InterpreterContext context = InterpreterContext.builder() - .setNoteId("noteId") - .setParagraphId("paragraphId") - .setResourcePool(resourcePool) - .build(); - InterpreterContext.set(context); - - interpreter = new TestInterpreter(new Properties()); - resourcePool.put("PI", "3.1415"); - } @After public void tearDown() throws Exception { - InterpreterContext.remove(); } @Test public void stringWithoutPatterns() { - String result = interpreter.interpolate("The value of PI is not exactly 3.14", resourcePool); - assertTrue("String without patterns", "The value of PI is not exactly 3.14".equals(result)); + String result = AbstractInterpreter.interpolate("The value of PI is not exactly 3.14", resourcePool); + assertEquals("String without patterns", "The value of PI is not exactly 3.14", result); } @Test public void substitutionInTheMiddle() { - String result = interpreter.interpolate("The value of {{PI}} is {PI} now", resourcePool); - assertTrue("Substitution in the middle", "The value of {PI} is 3.1415 now".equals(result)); + String result = AbstractInterpreter.interpolate("The value of {{PI}} is {PI} now", resourcePool); + assertEquals("Substitution in the middle", "The value of {PI} is 3.1415 now", result); } @Test public void substitutionAtTheEnds() { - String result = interpreter.interpolate("{{PI}} is now {PI}", resourcePool); - assertTrue("Substitution at the ends", "{PI} is now 3.1415".equals(result)); + String result = AbstractInterpreter.interpolate("{{PI}} is now {PI}", resourcePool); + assertEquals("Substitution at the ends", "{PI} is now 3.1415", result); } @Test public void multiLineSubstitutionSuccessful1() { - String result = interpreter.interpolate("{{PI}}\n{PI}\n{{PI}}\n{PI}", resourcePool); - assertTrue("multiLineSubstitutionSuccessful1", "{PI}\n3.1415\n{PI}\n3.1415".equals(result)); + String result = AbstractInterpreter.interpolate("{{PI}}\n{PI}\n{{PI}}\n{PI}", resourcePool); + assertEquals("multiLineSubstitutionSuccessful1", "{PI}\n3.1415\n{PI}\n3.1415", result); } @Test public void multiLineSubstitutionSuccessful2() { - String result = interpreter.interpolate("prefix {PI} {{PI\n}} suffix", resourcePool); - assertTrue("multiLineSubstitutionSuccessful2", "prefix 3.1415 {PI\n} suffix".equals(result)); + String result = AbstractInterpreter.interpolate("prefix {PI} {{PI\n}} suffix", resourcePool); + assertEquals("multiLineSubstitutionSuccessful2", "prefix 3.1415 {PI\n} suffix", result); } @Test public void multiLineSubstitutionSuccessful3() { - String result = interpreter.interpolate("prefix {{\nPI}} {PI} suffix", resourcePool); - assertTrue("multiLineSubstitutionSuccessful3", "prefix {\nPI} 3.1415 suffix".equals(result)); + String result = AbstractInterpreter.interpolate("prefix {{\nPI}} {PI} suffix", resourcePool); + assertEquals("multiLineSubstitutionSuccessful3", "prefix {\nPI} 3.1415 suffix", result); } @Test public void multiLineSubstitutionFailure2() { - String result = interpreter.interpolate("prefix {PI\n} suffix", resourcePool); - assertTrue("multiLineSubstitutionFailure2", "prefix {PI\n} suffix".equals(result)); + String result = AbstractInterpreter.interpolate("prefix {PI\n} suffix", resourcePool); + assertEquals("multiLineSubstitutionFailure2", "prefix {PI\n} suffix", result); } @Test public void multiLineSubstitutionFailure3() { - String result = interpreter.interpolate("prefix {\nPI} suffix", resourcePool); - assertTrue("multiLineSubstitutionFailure3", "prefix {\nPI} suffix".equals(result)); + String result = AbstractInterpreter.interpolate("prefix {\nPI} suffix", resourcePool); + assertEquals("multiLineSubstitutionFailure3", "prefix {\nPI} suffix", result); } @Test public void noUndefinedVariableError() { - String result = interpreter.interpolate("This {pi} will pass silently", resourcePool); - assertTrue("No partial substitution", "This {pi} will pass silently".equals(result)); + String result = AbstractInterpreter.interpolate("This {pi} will pass silently", resourcePool); + assertEquals("No partial substitution", "This {pi} will pass silently", result); } @Test public void noPartialSubstitution() { - String result = interpreter.interpolate("A {PI} and a {PIE} are different", resourcePool); - assertTrue("No partial substitution", "A {PI} and a {PIE} are different".equals(result)); + String result = AbstractInterpreter.interpolate("A {PI} and a {PIE} are different", resourcePool); + assertEquals("No partial substitution", "A {PI} and a {PIE} are different", result); } @Test public void substitutionAndEscapeMixed() { - String result = interpreter.interpolate("A {PI} is not a {{PIE}}", resourcePool); - assertTrue("Substitution and escape mixed", "A 3.1415 is not a {PIE}".equals(result)); + String result = AbstractInterpreter.interpolate("A {PI} is not a {{PIE}}", resourcePool); + assertEquals("Substitution and escape mixed", "A 3.1415 is not a {PIE}", result); } @Test public void unbalancedBracesOne() { - String result = interpreter.interpolate("A {PI} and a {{PIE} remain unchanged", resourcePool); - assertTrue("Unbalanced braces - one", "A {PI} and a {{PIE} remain unchanged".equals(result)); + String result = AbstractInterpreter.interpolate("A {PI} and a {{PIE} remain unchanged", resourcePool); + assertEquals("Unbalanced braces - one", "A {PI} and a {{PIE} remain unchanged", result); } @Test public void unbalancedBracesTwo() { - String result = interpreter.interpolate("A {PI} and a {PIE}} remain unchanged", resourcePool); - assertTrue("Unbalanced braces - one", "A {PI} and a {PIE}} remain unchanged".equals(result)); + String result = AbstractInterpreter.interpolate("A {PI} and a {PIE}} remain unchanged", resourcePool); + assertEquals("Unbalanced braces - one", "A {PI} and a {PIE}} remain unchanged", result); } @Test public void tooManyBraces() { - String result = interpreter.interpolate("This {{{PI}}} remain unchanged", resourcePool); - assertTrue("Too many braces", "This {{{PI}}} remain unchanged".equals(result)); + String result = AbstractInterpreter.interpolate("This {{{PI}}} remain unchanged", resourcePool); + assertEquals("Too many braces", "This {{{PI}}} remain unchanged", result); } @Test public void randomBracesOne() { - String result = interpreter.interpolate("A {{ starts an escaped sequence", resourcePool); - assertTrue("Random braces - one", "A {{ starts an escaped sequence".equals(result)); + String result = AbstractInterpreter.interpolate("A {{ starts an escaped sequence", resourcePool); + assertEquals("Random braces - one", "A {{ starts an escaped sequence", result); } @Test public void randomBracesTwo() { - String result = interpreter.interpolate("A }} ends an escaped sequence", resourcePool); - assertTrue("Random braces - two", "A }} ends an escaped sequence".equals(result)); + String result = AbstractInterpreter.interpolate("A }} ends an escaped sequence", resourcePool); + assertEquals("Random braces - two", "A }} ends an escaped sequence", result); } @Test public void randomBracesThree() { - String result = interpreter.interpolate("Paired { begin an escaped sequence", resourcePool); - assertTrue("Random braces - three", "Paired { begin an escaped sequence".equals(result)); + String result = AbstractInterpreter.interpolate("Paired { begin an escaped sequence", resourcePool); + assertEquals("Random braces - three", "Paired { begin an escaped sequence", result); } @Test public void randomBracesFour() { - String result = interpreter.interpolate("Paired } end an escaped sequence", resourcePool); - assertTrue("Random braces - four", "Paired } end an escaped sequence".equals(result)); + String result = AbstractInterpreter.interpolate("Paired } end an escaped sequence", resourcePool); + assertEquals("Random braces - four", "Paired } end an escaped sequence", result); } }