[SPARK-14025][STREAMING][WEBUI] Fix streaming job descriptions on the…

… event timeline ## What changes were proposed in this pull request? Removed the extra `<a href=...>...</a>` for each streaming job's description on the event timeline. ### [Before] ![before](https://cloud.githubusercontent.com/assets/15843379/13898653/0a6c1838-ee13-11e5-9761-14bb7b114c13.png) ### [After] ![after](https://cloud.githubusercontent.com/assets/15843379/13898650/012b8808-ee13-11e5-92a6-64aff0799c83.png) ## How was this patch tested? test suits, manual checks (see screenshots above) Author: Liwei Lin <[email protected]> Author: proflin <[email protected]> Closes apache#11845 from lw-lin/description-event-line.
leonelfolmer · Mar 23, 2016 · de4e48b · de4e48b
1 parent 69bc2c1
commit de4e48b
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 23 deletions.
diff --git a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -416,8 +416,16 @@ private[spark] object UIUtils extends Logging {
    * Note: In terms of security, only anchor tags with root relative links are supported. So any
    * attempts to embed links outside Spark UI, or other tags like <script> will cause in the whole
    * description to be treated as plain text.
+   *
+   * @param desc        the original job or stage description string, which may contain html tags.
+   * @param basePathUri with which to prepend the relative links; this is used when plainText is
+   *                    false.
+   * @param plainText   whether to keep only plain text (i.e. remove html tags) from the original
+   *                    description string.
+   * @return the HTML rendering of the job or stage description, which will be a Text when plainText
+   *         is true, and an Elem otherwise.
    */
-  def makeDescription(desc: String, basePathUri: String): NodeSeq = {
+  def makeDescription(desc: String, basePathUri: String, plainText: Boolean = false): NodeSeq = {
     import scala.language.postfixOps
 
     // If the description can be parsed as HTML and has only relative links, then render
@@ -445,22 +453,37 @@ private[spark] object UIUtils extends Logging {
           "Links in job descriptions must be root-relative:\n" + allLinks.mkString("\n\t"))
       }
 
-      // Prepend the relative links with basePathUri
-      val rule = new RewriteRule() {
-        override def transform(n: Node): Seq[Node] = {
-          n match {
-            case e: Elem if e \ "@href" nonEmpty =>
-              val relativePath = e.attribute("href").get.toString
-              val fullUri = s"${basePathUri.stripSuffix("/")}/${relativePath.stripPrefix("/")}"
-              e % Attribute(null, "href", fullUri, Null)
-            case _ => n
+      val rule =
+        if (plainText) {
+          // Remove all tags, retaining only their texts
+          new RewriteRule() {
+            override def transform(n: Node): Seq[Node] = {
+              n match {
+                case e: Elem if e.child isEmpty => Text(e.text)
+                case e: Elem if e.child nonEmpty => Text(e.child.flatMap(transform).text)
+                case _ => n
+              }
+            }
+          }
+        }
+        else {
+          // Prepend the relative links with basePathUri
+          new RewriteRule() {
+            override def transform(n: Node): Seq[Node] = {
+              n match {
+                case e: Elem if e \ "@href" nonEmpty =>
+                  val relativePath = e.attribute("href").get.toString
+                  val fullUri = s"${basePathUri.stripSuffix("/")}/${relativePath.stripPrefix("/")}"
+                  e % Attribute(null, "href", fullUri, Null)
+                case _ => n
+              }
+            }
           }
         }
-      }
       new RuleTransformer(rule).transform(xml)
     } catch {
       case NonFatal(e) =>
-        <span class="description-input">{desc}</span>
+        if (plainText) Text(desc) else <span class="description-input">{desc}</span>
     }
   }
 

diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala b/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
@@ -71,7 +71,12 @@ private[ui] class AllJobsPage(parent: JobsTab) extends WebUIPage("") {
       val jobId = jobUIData.jobId
       val status = jobUIData.status
       val (jobName, jobDescription) = getLastStageNameAndDescription(jobUIData)
-      val displayJobDescription = if (jobDescription.isEmpty) jobName else jobDescription
+      val displayJobDescription =
+        if (jobDescription.isEmpty) {
+          jobName
+        } else {
+          UIUtils.makeDescription(jobDescription, "", plainText = true).text
+        }
       val submissionTime = jobUIData.submissionTime.get
       val completionTimeOpt = jobUIData.completionTime
       val completionTime = completionTimeOpt.getOrElse(System.currentTimeMillis())
@@ -225,7 +230,8 @@ private[ui] class AllJobsPage(parent: JobsTab) extends WebUIPage("") {
       val formattedDuration = duration.map(d => UIUtils.formatDuration(d)).getOrElse("Unknown")
       val formattedSubmissionTime = job.submissionTime.map(UIUtils.formatDate).getOrElse("Unknown")
       val basePathUri = UIUtils.prependBaseUri(parent.basePath)
-      val jobDescription = UIUtils.makeDescription(lastStageDescription, basePathUri)
+      val jobDescription =
+        UIUtils.makeDescription(lastStageDescription, basePathUri, plainText = false)
 
       val detailUrl = "%s/jobs/job?id=%s".format(basePathUri, job.jobId)
       <tr id={"job-" + job.jobId}>

diff --git a/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala b/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala
@@ -17,43 +17,95 @@
 
 package org.apache.spark.ui
 
-import scala.xml.Elem
+import scala.xml.{Node, Text}
 
 import org.apache.spark.SparkFunSuite
 
 class UIUtilsSuite extends SparkFunSuite {
   import UIUtils._
 
-  test("makeDescription") {
+  test("makeDescription(plainText = false)") {
     verify(
       """test <a href="/link"> text </a>""",
       <span class="description-input">test <a href="/link"> text </a></span>,
-      "Correctly formatted text with only anchors and relative links should generate HTML"
+      "Correctly formatted text with only anchors and relative links should generate HTML",
+      plainText = false
     )
 
     verify(
       """test <a href="/link" text </a>""",
       <span class="description-input">{"""test <a href="/link" text </a>"""}</span>,
-      "Badly formatted text should make the description be treated as a streaming instead of HTML"
+      "Badly formatted text should make the description be treated as a string instead of HTML",
+      plainText = false
     )
 
     verify(
       """test <a href="link"> text </a>""",
       <span class="description-input">{"""test <a href="link"> text </a>"""}</span>,
-      "Non-relative links should make the description be treated as a string instead of HTML"
+      "Non-relative links should make the description be treated as a string instead of HTML",
+      plainText = false
     )
 
     verify(
       """test<a><img></img></a>""",
       <span class="description-input">{"""test<a><img></img></a>"""}</span>,
-      "Non-anchor elements should make the description be treated as a string instead of HTML"
+      "Non-anchor elements should make the description be treated as a string instead of HTML",
+      plainText = false
     )
 
     verify(
       """test <a href="/link"> text </a>""",
       <span class="description-input">test <a href="base/link"> text </a></span>,
       baseUrl = "base",
-      errorMsg = "Base URL should be prepended to html links"
+      errorMsg = "Base URL should be prepended to html links",
+      plainText = false
+    )
+  }
+
+  test("makeDescription(plainText = true)") {
+    verify(
+      """test <a href="/link"> text </a>""",
+      Text("test  text "),
+      "Correctly formatted text with only anchors and relative links should generate a string " +
+      "without any html tags",
+      plainText = true
+    )
+
+    verify(
+      """test <a href="/link"> text1 </a> <a href="/link"> text2 </a>""",
+      Text("test  text1   text2 "),
+      "Correctly formatted text with multiple anchors and relative links should generate a " +
+      "string without any html tags",
+      plainText = true
+    )
+
+    verify(
+      """test <a href="/link"><span> text </span></a>""",
+      Text("test  text "),
+      "Correctly formatted text with nested anchors and relative links and/or spans should " +
+      "generate a string without any html tags",
+      plainText = true
+    )
+
+    verify(
+      """test <a href="/link" text </a>""",
+      Text("""test <a href="/link" text </a>"""),
+      "Badly formatted text should make the description be as the same as the original text",
+      plainText = true
+    )
+
+    verify(
+      """test <a href="link"> text </a>""",
+      Text("""test <a href="link"> text </a>"""),
+      "Non-relative links should make the description be as the same as the original text",
+      plainText = true
+    )
+
+    verify(
+      """test<a><img></img></a>""",
+      Text("""test<a><img></img></a>"""),
+      "Non-anchor elements should make the description be as the same as the original text",
+      plainText = true
     )
   }
 
@@ -82,8 +134,12 @@ class UIUtilsSuite extends SparkFunSuite {
   }
 
   private def verify(
-      desc: String, expected: Elem, errorMsg: String = "", baseUrl: String = ""): Unit = {
-    val generated = makeDescription(desc, baseUrl)
+      desc: String,
+      expected: Node,
+      errorMsg: String = "",
+      baseUrl: String = "",
+      plainText: Boolean): Unit = {
+    val generated = makeDescription(desc, baseUrl, plainText)
     assert(generated.sameElements(expected),
       s"\n$errorMsg\n\nExpected:\n$expected\nGenerated:\n$generated")
   }