Merge pull request #1949 from lift/msf_issue_1912

Minimize surprise NPEs in Loc before SiteMap assignment

web/webkit/src/main/scala/net/liftweb/sitemap/Loc.scala

@@ -57,7 +57,7 @@ trait Loc[T] {
    * By default, this lazy val looks for the MenuCssClass LocParam and
    * uses it.
    */
-  protected lazy val cacheCssClassForMenuItem: Box[() => String] = 
+  protected lazy val cacheCssClassForMenuItem: Box[() => String] =
     allParams.flatMap {
       case a: Loc.MenuCssClass => List(a)
       case _ => Nil
@@ -78,7 +78,7 @@ trait Loc[T] {
 
 
   def defaultValue: Box[T]
-  
+
   /**
   * The value of the Loc based on params (either Loc.Value or Loc.CalcValue)
   */
@@ -90,7 +90,7 @@ trait Loc[T] {
         v.asInstanceOf[T]
     }
   }
-  
+
   private lazy val calcValue: Box[() => Box[T]] = {
     params.collectFirst {
       case Loc.CalcValue(f: Function0[_]) =>
@@ -102,13 +102,13 @@ trait Loc[T] {
    * Calculate the Query parameters
    */
   def queryParameters(what: Box[T]): List[(String, String)] =
-    addlQueryParams.flatMap(_()) ::: 
+    addlQueryParams.flatMap(_()) :::
   calcQueryParams.flatMap(_(what))
 
-  protected def appendQueryParams(what: T)(nodeSeq: NodeSeq): NodeSeq = 
+  protected def appendQueryParams(what: T)(nodeSeq: NodeSeq): NodeSeq =
     Text(appendQueryParameters(nodeSeq.text, Full(what)))
 
-  protected def appendQueryParameters(in: String, what: Box[T]) = 
+  protected def appendQueryParameters(in: String, what: Box[T]) =
     Helpers.appendQueryParameters(in, queryParameters(what))
 
   private lazy val addlQueryParams: List[() => List[(String, String)]] =
@@ -127,16 +127,22 @@ trait Loc[T] {
 
   def params: List[Loc.LocParam[T]]
 
-  def allParams: List[Loc.AnyLocParam] = 
-    (params.asInstanceOf[List[Loc.AnyLocParam]]) ::: 
-     parentParams :::
-     siteMap.globalParams
+  def allParams: List[Loc.AnyLocParam] = {
+    if (_menu == null) {
+      // Params are impossible without a menu
+      Nil
+    } else {
+      (params.asInstanceOf[List[Loc.AnyLocParam]]) :::
+       parentParams :::
+       siteMap.globalParams
+    }
+  }
 
-  private def parentParams: List[Loc.AnyLocParam] = 
+  private def parentParams: List[Loc.AnyLocParam] =
     _menu match {
       case null => Nil
       case menu => menu._parent match {
-        case Full(parentMenu: Menu) => 
+        case Full(parentMenu: Menu) =>
           if (!params.collect{case i: Loc.UseParentParams => true}.isEmpty) {
             parentMenu.loc.allParams.asInstanceOf[List[Loc.LocParam[Any]]]
           } else {
@@ -154,9 +160,12 @@ trait Loc[T] {
 
   def hideIfNoKids_? = placeHolder_? || _hideIfNoKids_?
 
-  def siteMap: SiteMap = _menu.siteMap
+  // This implementation is less than ideal, but changing the type would
+  // have been a breaking API change. This at least avoids this method
+  // throwing an NPE
+  def siteMap: SiteMap = Option(_menu).map(_.siteMap).getOrElse(null)
 
-  def createDefaultLink: Option[NodeSeq] = 
+  def createDefaultLink: Option[NodeSeq] =
     currentValue.flatMap(p => link.createLink(p)).toOption.
     map(ns => Text(appendQueryParameters(ns.text, currentValue)))
 
@@ -204,7 +213,7 @@ trait Loc[T] {
    * Is the Loc marked as Stateless (this will force rendering of
    * the page into stateless mode)
    */
-  def stateless_? : Boolean = 
+  def stateless_? : Boolean =
     if (Props.devMode) (calcStateless() || reqCalcStateless())
     else (_frozenStateless || reqCalcStateless())
 
@@ -401,7 +410,13 @@ trait Loc[T] {
     )
   }
 
-  def breadCrumbs: List[Loc[_]] = _menu.breadCrumbs ::: List(this)
+  def breadCrumbs: List[Loc[_]] = {
+    if (_menu != null) {
+      _menu.breadCrumbs ::: List(this)
+    } else {
+      List(this)
+    }
+  }
 
   def buildKidMenuItems(kids: Seq[Menu]): List[MenuItem] = {
     kids.toList.flatMap(_.loc.buildItem(Nil, false, false)) ::: supplementalKidMenuItems
@@ -463,7 +478,7 @@ trait Loc[T] {
 
 trait ConvertableLoc[T] {
   self: Loc[T] =>
-    
+
   /**
    * Converts the String to T that can then be sent to
    * the Loc in createLink
@@ -513,7 +528,6 @@ object Loc {
     init()
   }
 
-
   /**
    * Algebraic data type for parameters that modify handling of a Loc
    * in a SiteMap
@@ -630,13 +644,13 @@ object Loc {
    * the groups can be specified and recalled at the top level
    */
   case class LocGroup(group: String*) extends AnyLocParam
-  
+
   /**
   * Calculate the value for the Loc.  This is useful for parameterized
   * menus.  It allows you to calculate the value of the Loc.
   */
   case class CalcValue[T](func: () => Box[T]) extends LocParam[T]
-  
+
   /**
   * The value of Loc
   */
@@ -685,7 +699,7 @@ object Loc {
    */
   class Snippet(val name: String, _func: => NodeSeq => NodeSeq) extends ValueSnippets[Any] with AnyLocParam {
     /**
-     * The NodeSeq => NodeSeq function 
+     * The NodeSeq => NodeSeq function
      */
     def func: NodeSeq => NodeSeq = _func
 
@@ -959,4 +973,3 @@ case class MenuItem(text: NodeSeq, uri: NodeSeq,  kids: Seq[MenuItem],
     else this :: kids.toList.flatMap(_.breadCrumbs)
   }
 }
-

web/webkit/src/test/scala/net/liftweb/sitemap/LocSpec.scala

@@ -91,6 +91,15 @@ object LocSpec extends Specification  {
         }
       }
     }
+
+    "not throw Exceptions on param methods before SiteMap assignment" in {
+      val testMenu = Menu.param[Param]("Test", "Test", s => Empty, p => "bacon") / "foo" / "bar" >> Loc.MatchWithoutCurrentValue
+      val testLoc = testMenu.toLoc
+
+      testLoc.allParams must not(throwA[Exception])
+      testLoc.currentValue must not(throwA[Exception])
+      testLoc.siteMap must not(throwA[Exception])
+      testLoc.breadCrumbs must not(throwA[Exception])
+    }
   }
 }
-