From ed97124b653effc1eb52ff8f5ac4ecad16b0c735 Mon Sep 17 00:00:00 2001 From: jerryshao Date: Fri, 17 Aug 2018 21:37:32 +0800 Subject: [PATCH 1/2] Fix Windows line ending issue --- .../main/scala/org/apache/livy/EOLUtils.scala | 116 ++++++++++++++++++ .../scala/org/apache/livy/EOLUtilsSuite.scala | 68 ++++++++++ .../org/apache/livy/test/InteractiveIT.scala | 2 + .../org/apache/livy/repl/ReplDriver.scala | 6 +- 4 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 core/src/main/scala/org/apache/livy/EOLUtils.scala create mode 100644 core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala diff --git a/core/src/main/scala/org/apache/livy/EOLUtils.scala b/core/src/main/scala/org/apache/livy/EOLUtils.scala new file mode 100644 index 000000000..1cc01eb77 --- /dev/null +++ b/core/src/main/scala/org/apache/livy/EOLUtils.scala @@ -0,0 +1,116 @@ +/* + * 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.livy + +/** + * Helper class to deal with end-of-line markers in text files. + */ +object EOLUtils { + /** + * Unix-style end-of-line marker (LF) + */ + private val EOL_UNIX: String = "\n" + + /** + * Windows-style end-of-line marker (CRLF) + */ + private val EOL_WINDOWS: String = "\r\n" + + /** + * "Old Mac"-style end-of-line marker (CR) + */ + private val EOL_OLD_MAC: String = "\r" + + /** + * Default end-of-line marker on current system + */ + private val EOL_SYSTEM_DEFAULT: String = System.getProperty("line.separator") + + object Mode extends Enumeration { + + type Mode = Value + + val LF, CRLF, CR = Value + + lazy val SYSTEM_DEFAULT: Mode = { + val tmp = if (EOL_SYSTEM_DEFAULT == EOL_UNIX) { + LF + } else if (EOL_SYSTEM_DEFAULT == EOL_WINDOWS) { + CRLF + } else if (EOL_SYSTEM_DEFAULT == EOL_OLD_MAC) { + CR + } else { + null + } + + if (tmp == null) { + throw new IllegalStateException("Could not determine system default end-of-line marker") + } + tmp + } + + def determineEOL(s: String): Mode = { + val charArray = s.toCharArray + + var prev: Char = null.asInstanceOf[Char] + for (ch <- charArray) { + if (ch == '\n') { + if (prev == '\r') { + return CRLF + } else { + return LF + } + } else if (prev == '\r') { + return CR + } + + prev = ch + } + + null + } + } + + def hasWindowsEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.CRLF + + def hasUnixEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.LF + + def hasOldMacEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.CR + + def hasSystemDefaultEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.SYSTEM_DEFAULT + + def convertToUnixEOL(s: String): String = convertLineEndings(s, EOL_UNIX) + + def convertToWindowsEOL(s: String): String = convertLineEndings(s, EOL_WINDOWS) + + def convertToOldMacEOL(s: String): String = convertLineEndings(s, EOL_OLD_MAC) + + def convertToSystemEOL(s: String): String = convertLineEndings(s, EOL_SYSTEM_DEFAULT) + + private def convertLineEndings(s: String, eol: String): String = { + if (hasWindowsEOL(s)) { + s.replaceAll(EOL_WINDOWS, eol) + } else if (hasUnixEOL(s)) { + s.replaceAll(EOL_UNIX, eol) + } else if (hasOldMacEOL(s)) { + s.replaceAll(EOL_OLD_MAC, eol) + } else { + s + } + } +} diff --git a/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala b/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala new file mode 100644 index 000000000..2af567069 --- /dev/null +++ b/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala @@ -0,0 +1,68 @@ +/* + * 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.livy + +import org.scalatest.FunSuite + +class EOLUtilsSuite extends FunSuite with LivyBaseUnitTestSuite { + + test("check EOL") { + val s1 = "test\r\ntest" + assert(!EOLUtils.hasUnixEOL(s1)) + assert(!EOLUtils.hasOldMacEOL(s1)) + assert(EOLUtils.hasWindowsEOL(s1)) + + val s2 = "test\ntest" + assert(EOLUtils.hasUnixEOL(s2)) + assert(!EOLUtils.hasOldMacEOL(s2)) + assert(!EOLUtils.hasWindowsEOL(s2)) + + val s3 = "test\rtest" + assert(!EOLUtils.hasUnixEOL(s3)) + assert(EOLUtils.hasOldMacEOL(s3)) + assert(!EOLUtils.hasWindowsEOL(s3)) + + val s4 = "testtest" + assert(!EOLUtils.hasUnixEOL(s4)) + assert(!EOLUtils.hasOldMacEOL(s4)) + assert(!EOLUtils.hasWindowsEOL(s4)) + } + + test("convert EOL") { + val s1 = "test\r\ntest" + val s2 = "test\ntest" + val s3 = "test\rtest" + val s4 = "testtest" + + assert(EOLUtils.convertToUnixEOL(s1) === s2) + assert(EOLUtils.convertToWindowsEOL(s1) === s1) + assert(EOLUtils.convertToOldMacEOL(s1) === s3) + + assert(EOLUtils.convertToUnixEOL(s2) === s2) + assert(EOLUtils.convertToWindowsEOL(s2) === s1) + assert(EOLUtils.convertToOldMacEOL(s2) === s3) + + assert(EOLUtils.convertToUnixEOL(s3) === s2) + assert(EOLUtils.convertToWindowsEOL(s3) === s1) + assert(EOLUtils.convertToOldMacEOL(s3) === s3) + + assert(EOLUtils.convertToUnixEOL(s4) === s4) + assert(EOLUtils.convertToWindowsEOL(s4) === s4) + assert(EOLUtils.convertToOldMacEOL(s4) === s4) + } +} diff --git a/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala b/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala index ff29d9542..853290a78 100644 --- a/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala +++ b/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala @@ -97,6 +97,7 @@ class InteractiveIT extends BaseIntegrationTestSuite { s.run("%table x").verifyResult(".*headers.*type.*name.*data.*") s.run("abcde").verifyError(ename = "NameError", evalue = "name 'abcde' is not defined") s.run("raise KeyError, 'foo'").verifyError(ename = "KeyError", evalue = "'foo'") + s.run("print(1)\r\nprint(1)").verifyResult("1\n1") } } @@ -115,6 +116,7 @@ class InteractiveIT extends BaseIntegrationTestSuite { """|root | |-- name: string (nullable = true) | |-- age: double (nullable = true)""".stripMargin)) + s.run("print(1)\r\nprint(1)").verifyResult(".*1\n.*1") } } diff --git a/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala b/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala index af51e434a..b805a4db1 100644 --- a/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala +++ b/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala @@ -23,7 +23,7 @@ import scala.concurrent.duration.Duration import io.netty.channel.ChannelHandlerContext import org.apache.spark.SparkConf -import org.apache.livy.Logging +import org.apache.livy.{EOLUtils, Logging} import org.apache.livy.client.common.ClientConf import org.apache.livy.rsc.{BaseProtocol, ReplJobResults, RSCConf} import org.apache.livy.rsc.BaseProtocol.ReplState @@ -55,7 +55,7 @@ class ReplDriver(conf: SparkConf, livyConf: RSCConf) } def handle(ctx: ChannelHandlerContext, msg: BaseProtocol.ReplJobRequest): Int = { - session.execute(msg.code, msg.codeType) + session.execute(EOLUtils.convertToSystemEOL(msg.code), msg.codeType) } def handle(ctx: ChannelHandlerContext, msg: BaseProtocol.CancelReplJobRequest): Unit = { @@ -63,7 +63,7 @@ class ReplDriver(conf: SparkConf, livyConf: RSCConf) } def handle(ctx: ChannelHandlerContext, msg: BaseProtocol.ReplCompleteRequest): Array[String] = { - session.complete(msg.code, msg.codeType, msg.cursor) + session.complete(EOLUtils.convertToSystemEOL(msg.code), msg.codeType, msg.cursor) } /** From f6db8ef9e79481b255b21a440bdcd08aa0e48291 Mon Sep 17 00:00:00 2001 From: jerryshao Date: Tue, 21 Aug 2018 14:43:16 +0800 Subject: [PATCH 2/2] Remove several unused methods and simplify the codes --- .../main/scala/org/apache/livy/EOLUtils.scala | 41 ++++++------------ .../scala/org/apache/livy/EOLUtilsSuite.scala | 43 +++++++------------ 2 files changed, 29 insertions(+), 55 deletions(-) diff --git a/core/src/main/scala/org/apache/livy/EOLUtils.scala b/core/src/main/scala/org/apache/livy/EOLUtils.scala index 1cc01eb77..10f2c626b 100644 --- a/core/src/main/scala/org/apache/livy/EOLUtils.scala +++ b/core/src/main/scala/org/apache/livy/EOLUtils.scala @@ -21,28 +21,19 @@ package org.apache.livy * Helper class to deal with end-of-line markers in text files. */ object EOLUtils { - /** - * Unix-style end-of-line marker (LF) - */ + /** Unix-style end-of-line marker (LF) */ private val EOL_UNIX: String = "\n" - /** - * Windows-style end-of-line marker (CRLF) - */ + /** Windows-style end-of-line marker (CRLF) */ private val EOL_WINDOWS: String = "\r\n" - /** - * "Old Mac"-style end-of-line marker (CR) - */ + /** "Old Mac"-style end-of-line marker (CR) */ private val EOL_OLD_MAC: String = "\r" - /** - * Default end-of-line marker on current system - */ + /** Default end-of-line marker on current syste */ private val EOL_SYSTEM_DEFAULT: String = System.getProperty("line.separator") object Mode extends Enumeration { - type Mode = Value val LF, CRLF, CR = Value @@ -64,7 +55,7 @@ object EOLUtils { tmp } - def determineEOL(s: String): Mode = { + private def determineEOL(s: String): Mode = { val charArray = s.toCharArray var prev: Char = null.asInstanceOf[Char] @@ -84,30 +75,24 @@ object EOLUtils { null } - } - - def hasWindowsEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.CRLF - def hasUnixEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.LF + def hasWindowsEOL(s: String): Boolean = determineEOL(s) == CRLF - def hasOldMacEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.CR + def hasUnixEOL(s: String): Boolean = determineEOL(s) == LF - def hasSystemDefaultEOL(s: String): Boolean = Mode.determineEOL(s) == Mode.SYSTEM_DEFAULT + def hasOldMacEOL(s: String): Boolean = determineEOL(s) == CR - def convertToUnixEOL(s: String): String = convertLineEndings(s, EOL_UNIX) - - def convertToWindowsEOL(s: String): String = convertLineEndings(s, EOL_WINDOWS) - - def convertToOldMacEOL(s: String): String = convertLineEndings(s, EOL_OLD_MAC) + def hasSystemDefaultEOL(s: String): Boolean = determineEOL(s) == SYSTEM_DEFAULT + } def convertToSystemEOL(s: String): String = convertLineEndings(s, EOL_SYSTEM_DEFAULT) private def convertLineEndings(s: String, eol: String): String = { - if (hasWindowsEOL(s)) { + if (Mode.hasWindowsEOL(s)) { s.replaceAll(EOL_WINDOWS, eol) - } else if (hasUnixEOL(s)) { + } else if (Mode.hasUnixEOL(s)) { s.replaceAll(EOL_UNIX, eol) - } else if (hasOldMacEOL(s)) { + } else if (Mode.hasOldMacEOL(s)) { s.replaceAll(EOL_OLD_MAC, eol) } else { s diff --git a/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala b/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala index 2af567069..8ee73a117 100644 --- a/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala +++ b/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala @@ -23,24 +23,24 @@ class EOLUtilsSuite extends FunSuite with LivyBaseUnitTestSuite { test("check EOL") { val s1 = "test\r\ntest" - assert(!EOLUtils.hasUnixEOL(s1)) - assert(!EOLUtils.hasOldMacEOL(s1)) - assert(EOLUtils.hasWindowsEOL(s1)) + assert(!EOLUtils.Mode.hasUnixEOL(s1)) + assert(!EOLUtils.Mode.hasOldMacEOL(s1)) + assert(EOLUtils.Mode.hasWindowsEOL(s1)) val s2 = "test\ntest" - assert(EOLUtils.hasUnixEOL(s2)) - assert(!EOLUtils.hasOldMacEOL(s2)) - assert(!EOLUtils.hasWindowsEOL(s2)) + assert(EOLUtils.Mode.hasUnixEOL(s2)) + assert(!EOLUtils.Mode.hasOldMacEOL(s2)) + assert(!EOLUtils.Mode.hasWindowsEOL(s2)) val s3 = "test\rtest" - assert(!EOLUtils.hasUnixEOL(s3)) - assert(EOLUtils.hasOldMacEOL(s3)) - assert(!EOLUtils.hasWindowsEOL(s3)) + assert(!EOLUtils.Mode.hasUnixEOL(s3)) + assert(EOLUtils.Mode.hasOldMacEOL(s3)) + assert(!EOLUtils.Mode.hasWindowsEOL(s3)) val s4 = "testtest" - assert(!EOLUtils.hasUnixEOL(s4)) - assert(!EOLUtils.hasOldMacEOL(s4)) - assert(!EOLUtils.hasWindowsEOL(s4)) + assert(!EOLUtils.Mode.hasUnixEOL(s4)) + assert(!EOLUtils.Mode.hasOldMacEOL(s4)) + assert(!EOLUtils.Mode.hasWindowsEOL(s4)) } test("convert EOL") { @@ -49,20 +49,9 @@ class EOLUtilsSuite extends FunSuite with LivyBaseUnitTestSuite { val s3 = "test\rtest" val s4 = "testtest" - assert(EOLUtils.convertToUnixEOL(s1) === s2) - assert(EOLUtils.convertToWindowsEOL(s1) === s1) - assert(EOLUtils.convertToOldMacEOL(s1) === s3) - - assert(EOLUtils.convertToUnixEOL(s2) === s2) - assert(EOLUtils.convertToWindowsEOL(s2) === s1) - assert(EOLUtils.convertToOldMacEOL(s2) === s3) - - assert(EOLUtils.convertToUnixEOL(s3) === s2) - assert(EOLUtils.convertToWindowsEOL(s3) === s1) - assert(EOLUtils.convertToOldMacEOL(s3) === s3) - - assert(EOLUtils.convertToUnixEOL(s4) === s4) - assert(EOLUtils.convertToWindowsEOL(s4) === s4) - assert(EOLUtils.convertToOldMacEOL(s4) === s4) + assert(EOLUtils.convertToSystemEOL(s1) === EOLUtils.convertToSystemEOL(s2)) + assert(EOLUtils.convertToSystemEOL(s1) === EOLUtils.convertToSystemEOL(s3)) + assert(EOLUtils.convertToSystemEOL(s2) === EOLUtils.convertToSystemEOL(s3)) + assert(EOLUtils.convertToSystemEOL(s4) === s4) } }