This is an automated email from the ASF dual-hosted git repository. zjffdu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push: new 4b269f88fc [ZEPPELIN-5699] Trim space in ConfInterpreter 4b269f88fc is described below commit 4b269f88fc4a9d59a140b10260064a67382b6916 Author: Jeff Zhang <zjf...@apache.org> AuthorDate: Wed Mar 30 11:33:19 2022 +0800 [ZEPPELIN-5699] Trim space in ConfInterpreter ### What is this PR for? Trim the space for the key & value of ConfInterpreter. Unit test is added. ### What type of PR is it? [Bug Fix ] ### Todos * [ ] - Task ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-5669 ### How should this be tested? * CI pass ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jeff Zhang <zjf...@apache.org> Closes #4336 from zjffdu/ZEPPELIN-5699 and squashes the following commits: a4a471e1bf [Jeff Zhang] [ZEPPELIN-5699] Trim space in ConfInterpreter --- .../zeppelin/interpreter/ConfInterpreter.java | 4 +- .../zeppelin/interpreter/ConfInterpreterTest.java | 56 +++++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java index f343790862..9c5d02debb 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java @@ -67,7 +67,9 @@ public class ConfInterpreter extends Interpreter { finalProperties.putAll(getProperties()); Properties newProperties = new Properties(); newProperties.load(new StringReader(st)); - finalProperties.putAll(newProperties); + for (String key : newProperties.stringPropertyNames()) { + finalProperties.put(key.trim(), newProperties.getProperty(key).trim()); + } LOGGER.debug("Properties for InterpreterGroup: {} is {}", interpreterGroupId, finalProperties); interpreterSetting.setInterpreterGroupProperties(interpreterGroupId, finalProperties); return new InterpreterResult(InterpreterResult.Code.SUCCESS); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java index 41f5d73b00..3a8717ff37 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java @@ -21,8 +21,9 @@ import org.apache.zeppelin.interpreter.remote.RemoteInterpreter; import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import java.util.Properties; + +import static org.junit.Assert.*; public class ConfInterpreterTest extends AbstractInterpreterTest { @@ -59,6 +60,57 @@ public class ConfInterpreterTest extends AbstractInterpreterTest { assertEquals(InterpreterResult.Code.ERROR, result.code); } + @Test + public void testPropertyTrim() throws InterpreterException { + assertTrue(interpreterFactory.getInterpreter("test.conf", executionContext) instanceof ConfInterpreter); + ConfInterpreter confInterpreter = (ConfInterpreter) interpreterFactory.getInterpreter("test.conf", executionContext); + + InterpreterContext context = InterpreterContext.builder() + .setNoteId("noteId") + .setParagraphId("paragraphId") + .build(); + + // space before key and space after values + InterpreterResult result = confInterpreter.interpret(" property_1 \tnew_value \n new_property \t dummy_value \n", context); + assertEquals(InterpreterResult.Code.SUCCESS, result.code); + + assertTrue(interpreterFactory.getInterpreter("test", executionContext) instanceof RemoteInterpreter); + RemoteInterpreter remoteInterpreter = (RemoteInterpreter) interpreterFactory.getInterpreter("test", executionContext); + remoteInterpreter.interpret("hello world", context); + Properties intpProperties = remoteInterpreter.getProperties(); + // Total 7 properties, + // 3 built-in properties (zeppelin.interpreter.output.limit, zeppelin.interpreter.localRepo, zeppelin.interpreter.connection.poolsize) + assertEquals(7, intpProperties.size()); + assertNotNull(intpProperties.getProperty("zeppelin.interpreter.output.limit")); + assertNotNull(intpProperties.getProperty("zeppelin.interpreter.localRepo")); + assertNotNull(intpProperties.getProperty("zeppelin.interpreter.connection.poolsize")); + assertEquals("new_value", intpProperties.getProperty("property_1")); + assertEquals("new_value_2", intpProperties.getProperty("property_2")); + assertEquals("value_3", intpProperties.getProperty("property_3")); + assertEquals("dummy_value", intpProperties.getProperty("new_property")); + } + + @Test + public void testEmptyValue() throws InterpreterException { + ConfInterpreter confInterpreter = (ConfInterpreter) interpreterFactory.getInterpreter("test.conf", executionContext); + + InterpreterContext context = InterpreterContext.builder() + .setNoteId("noteId") + .setParagraphId("paragraphId") + .build(); + + InterpreterResult result = confInterpreter.interpret(" property_1\t \n new_property\t \n", context); + assertEquals(result.toString(), InterpreterResult.Code.SUCCESS, result.code); + + assertTrue(interpreterFactory.getInterpreter("test", executionContext) instanceof RemoteInterpreter); + RemoteInterpreter remoteInterpreter = (RemoteInterpreter) interpreterFactory.getInterpreter("test", executionContext); + remoteInterpreter.interpret("hello world", context); + assertEquals(7, remoteInterpreter.getProperties().size()); + assertEquals("", remoteInterpreter.getProperty("property_1")); + assertEquals("", remoteInterpreter.getProperty("new_property")); + assertEquals("value_3", remoteInterpreter.getProperty("property_3")); + } + @Test public void testEmptyConf() throws InterpreterException { assertTrue(interpreterFactory.getInterpreter("test.conf", executionContext) instanceof ConfInterpreter);