[SPARK-12186] [WEB UI] Send the complete request URI including the query string when redirecting.

[rohitagarwal003]

No description provided.

[rohitagarwal003]

cc - @vanzin

[rohitagarwal003]

cc - @srowen, @andrewor14 Can you please ask Jenkins to run the tests. Thanks!

[andrewor14]

ok to test

[SparkQA]

Test build #47555 has finished for PR 10180 at commit 7625c88.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[rohitagarwal003]

Can someone review this or if it seems okay, merge it?

[srowen]

I think this is fine, but maybe slightly less repetitious like ...

val requestURI = req.getRequestURI + (if (req.getQueryString == null) "" else ("?" + req.getQueryString))
res.sendRedirect(res.encodeRedirectURL(requestURI))

[andrewor14]

or even

val requestURI = req.getRequestURI + req.getQueryString.map("?" + _).getOrElse("")

[rohitagarwal003]

@andrewor14 Can't use getOrElse:

value getOrElse is not a member of scala.collection.immutable.IndexedSeq[String]

[rohitagarwal003]

@srowen I think having an if statement like that makes it a little harder to read. How about the following, it has lesser repetition.

val requestURI = if (req.getQueryString == null) {
  req.getRequestURI
} else {
  req.getRequestURI + "?" + req.getQueryString
}
res.sendRedirect(res.encodeRedirectURL(requestURI))

[andrewor14]

what? Isn't req.getQueryString a string?

[rohitagarwal003]

It is. I am not sure what you are trying to do in that statement:

scala> "spark".map("?" + _)
res0: scala.collection.immutable.IndexedSeq[String] = Vector(?s, ?p, ?a, ?r, ?k)

[andrewor14]

oh sorry, I meant

val requestURI = req.getRequestURI + Option(req.getQueryString).map("?" + _).getOrElse("")

where

Option(null) == None
Option("something") == Some("something")

[rohitagarwal003]

Updated.

[andrewor14]

LGTM once you address the comments.

[SparkQA]

Test build #47839 has finished for PR 10180 at commit 780ee62.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[andrewor14]

retest this please

[SparkQA]

Test build #47853 has finished for PR 10180 at commit 780ee62.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[andrewor14]

Merged into master 1.6