Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LIVY-498][REPL] Fix Windows CRLF line ending issue in SparkR interpreter #105

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions core/src/main/scala/org/apache/livy/EOLUtils.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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 syste */
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
}

private 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 = determineEOL(s) == CRLF

def hasUnixEOL(s: String): Boolean = determineEOL(s) == LF

def hasOldMacEOL(s: String): Boolean = determineEOL(s) == CR

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 (Mode.hasWindowsEOL(s)) {
s.replaceAll(EOL_WINDOWS, eol)
} else if (Mode.hasUnixEOL(s)) {
s.replaceAll(EOL_UNIX, eol)
} else if (Mode.hasOldMacEOL(s)) {
s.replaceAll(EOL_OLD_MAC, eol)
} else {
s
}
}
}
57 changes: 57 additions & 0 deletions core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* 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.Mode.hasUnixEOL(s1))
assert(!EOLUtils.Mode.hasOldMacEOL(s1))
assert(EOLUtils.Mode.hasWindowsEOL(s1))

val s2 = "test\ntest"
assert(EOLUtils.Mode.hasUnixEOL(s2))
assert(!EOLUtils.Mode.hasOldMacEOL(s2))
assert(!EOLUtils.Mode.hasWindowsEOL(s2))

val s3 = "test\rtest"
assert(!EOLUtils.Mode.hasUnixEOL(s3))
assert(EOLUtils.Mode.hasOldMacEOL(s3))
assert(!EOLUtils.Mode.hasWindowsEOL(s3))

val s4 = "testtest"
assert(!EOLUtils.Mode.hasUnixEOL(s4))
assert(!EOLUtils.Mode.hasOldMacEOL(s4))
assert(!EOLUtils.Mode.hasWindowsEOL(s4))
}

test("convert EOL") {
val s1 = "test\r\ntest"
val s2 = "test\ntest"
val s3 = "test\rtest"
val s4 = "testtest"

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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand All @@ -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")
}
}

Expand Down
6 changes: 3 additions & 3 deletions repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -55,15 +55,15 @@ 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 = {
session.cancel(msg.id)
}

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)
}

/**
Expand Down