Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Need a way to customize fail behavior in menus/param menus #1487

Merged
merged 4 commits into from

2 participants

@farmdawgnation
Collaborator

Currently, if a param menu fails because it can't find the parameter referenced, you get a 404. There's no easy way to customize that behavior for situations where you want something else to happen.

We should implement a LocParam or something that you can tack on to a menu definition that allows you to specify custom fail behavior.

farmdawgnation added some commits
@farmdawgnation farmdawgnation Simplify Loc.doesMatch_?
As I was reading over this code to look at this issue, we were calling
Link.isDefinedAt to determine whether or not to call Link.appy.
Link.apply calls Link.isDefinedAt to determine whether or not to throw
an exception.

This seemed redundant in this particular case, so I've reduced this
method to a simple boolean expression that should have the same effect.
244af80
@farmdawgnation farmdawgnation Add and implement the MatchWithoutCurrentValue LocParam.
The MatchWithoutCurrentValue LocParam overrides Lift's default behavior
of considering an Empty currentValue a non-match for a Loc. This default
action is desirable in most circumstances, but occasionally we want to
allow client code to define some custom behavior in the event
currentValue comes up Empty.

Using this new LocParam in combination with IfValue would allow them to
do so.
080dfc8
@farmdawgnation farmdawgnation Add specs to prove MatchWithoutCurrentValue works as expected. 5654beb
@farmdawgnation farmdawgnation Add an example of using MatchWithoutCurrentValue with IfValue. d18c6ef
@fmpwizard
Owner

:+1:

@fmpwizard
Owner

how likely do you think it is that merging this would break any app running 2.6?

if you don't think it is likely, I'd like to merge it and then do 2.6-M1 build this weekend, if you think there is a risk, maybe we can do 2.6-M1 without it, and then merge it right away, so people can start using a somewhat stable 2.6, and then try 2.6-SNAPSHOT to try this out.

I think merging should be ok, but figured to ask just in case :)

@farmdawgnation
Collaborator

how likely do you think it is that merging this would break any app running 2.6?

I don't think it's very likely. There are specs that tested Loc at a basic level, and they still pass. This is mostly an addition of new things.

Let's get it in.

@fmpwizard fmpwizard merged commit 74a951f into from
@fmpwizard fmpwizard deleted the branch
@fmpwizard
Owner

Done :) !

@farmdawgnation
Collaborator

! ! ! ! ! ! ! ! ! ! !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 14, 2013
  1. @farmdawgnation

    Simplify Loc.doesMatch_?

    farmdawgnation authored
    As I was reading over this code to look at this issue, we were calling
    Link.isDefinedAt to determine whether or not to call Link.appy.
    Link.apply calls Link.isDefinedAt to determine whether or not to throw
    an exception.
    
    This seemed redundant in this particular case, so I've reduced this
    method to a simple boolean expression that should have the same effect.
  2. @farmdawgnation

    Add and implement the MatchWithoutCurrentValue LocParam.

    farmdawgnation authored
    The MatchWithoutCurrentValue LocParam overrides Lift's default behavior
    of considering an Empty currentValue a non-match for a Loc. This default
    action is desirable in most circumstances, but occasionally we want to
    allow client code to define some custom behavior in the event
    currentValue comes up Empty.
    
    Using this new LocParam in combination with IfValue would allow them to
    do so.
  3. @farmdawgnation
  4. @farmdawgnation
This page is out of date. Refresh to see the latest.
View
35 web/webkit/src/main/scala/net/liftweb/sitemap/Loc.scala
@@ -383,14 +383,12 @@ trait Loc[T] {
}
def doesMatch_?(req: Req): Boolean = {
- (if (link.isDefinedAt(req)) {
- link(req) match {
- case Full(x) if testAllParams(allParams, req) => x
- case Full(x) => false
- case x => x.openOr(false)
- }
- } else false) && currentValue.isDefined
- // the loc only matches if we've got a current value
+ link.isDefinedAt(req) &&
+ testAllParams(allParams, req) &&
+ (
+ currentValue.isDefined ||
+ params.contains(Loc.MatchWithoutCurrentValue)
+ )
}
def breadCrumbs: List[Loc[_]] = _menu.breadCrumbs ::: List(this)
@@ -737,6 +735,27 @@ object Loc {
case object Hidden extends AnyLocParam
/**
+ * If this parameter is included, the Loc will continue to execute even if
+ * currentValue is not defined.
+ *
+ * By default, Lift will determine that a Loc does not match a given request
+ * if its currentValue comes up Empty, and as a result will return an HTTP 404.
+ * For situations where this is not the desired, "Not Found" behavior, you can
+ * add the MatchWithoutCurrentValue LocParam to a Loc, then use the IfValue
+ * LocParam to define what should happen when the currentValue is Empty.
+ *
+ * For example, given some class Thing, you could do the following to trigger
+ * a redirect when a Thing with a particular ID isn't found.
+ *
+ * {{{
+ * Menu.param[Thing]("Thing", "Thing", Thing.find(_), _.id) >>
+ * MatchWithoutCurrentValue >>
+ * IfValue(_.isDefined, () => RedirectResponse("/page/to/redirect/to"))
+ * }}}
+ */
+ case object MatchWithoutCurrentValue extends AnyLocParam
+
+ /**
* If this is a submenu, use the parent Loc's params
*/
case class UseParentParams() extends AnyLocParam
View
32 web/webkit/src/test/scala/net/liftweb/sitemap/LocSpec.scala
@@ -18,6 +18,10 @@ package net.liftweb
package sitemap
import common._
+import mockweb._
+ import MockWeb._
+import mocks._
+
import org.specs2.mutable.Specification
@@ -40,6 +44,34 @@ object LocSpec extends Specification {
val loc = (Menu.param[Param]("Test", "Test", s => Full(Param(s)), p => p.s) / "foo" / "bar" / *).toLoc
loc.calcHref(Param("myparam")) mustEqual "/foo/bar/myparam"
}
+
+ "should not match a Req matching its Link when currentValue is Empty" in {
+ val testMenu = Menu.param[Param]("Test", "Test", s => Empty, p => "bacon") / "foo" / "bar" / *
+ val testSiteMap = SiteMap(testMenu)
+
+ val testLoc = testMenu.toLoc
+ val mockReq = new MockHttpServletRequest("http://test/foo/bar/123")
+
+ testS(mockReq) {
+ testReq(mockReq) { req =>
+ testLoc.doesMatch_?(req) mustEqual false
+ }
+ }
+ }
+
+ "should match a Req matching its Link when currentValue is Empty and MatchWithoutCurrentValue is a param" in {
+ val testMenu = Menu.param[Param]("Test", "Test", s => Empty, p => "bacon") / "foo" / "bar" / * >> Loc.MatchWithoutCurrentValue
+ val testSiteMap = SiteMap(testMenu)
+
+ val testLoc = testMenu.toLoc
+ val mockReq = new MockHttpServletRequest("http://test/foo/bar/123")
+
+ testS(mockReq) {
+ testReq(mockReq) { req =>
+ testLoc.doesMatch_?(req) mustEqual true
+ }
+ }
+ }
}
}
Something went wrong with that request. Please try again.