Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Some Record fields don't work when optional_? = false #1183

Closed
wants to merge 1 commit into from

5 participants

@alexmsmartins

Based on the thread https://groups.google.com/d/msg/liftweb/jo3xkhu2Fjs/lKV4Cdzgy9EJ

There is a bug in some Record field classes by which the setFromString method does not handle the empty string when the
field is optional.

Fields with the bug:

  • DoubleTypedField
  • DecimalTypedField
  • LongTypedField
  • BooleanTypedField

Fields without the bug (if it's not here, I didn't check it):

  • BinaryTypedField
  • DateTimeTypedField
  • IntTypedField

EmailTypedField might also have a similar bug (I think) since the validateEmail() doesn't include the empty string as valid when the field is empty.

To correct this bug the following (or equivalent) code can be used:

def setFromString(s: String): Box[Double] = s match {
 case "" if optional_? => setBox(Empty)
 case _ =>setBox(tryo(java.lang.Double.parseDouble(s)))
}
@dpp
Owner

Pointed to David W

@davewhittaker davewhittaker was assigned
@fmpwizard
Owner

+1

@davewhittaker

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 2, 2012
  1. @davewhittaker

    All Fields now handle setFromString(null|"") by setting valueBox to E…

    davewhittaker authored
    …mpty if the field is optional, or Failure if it is not
This page is out of date. Refresh to see the latest.
View
6 persistence/record/src/main/scala/net/liftweb/record/field/BinaryField.scala
@@ -30,11 +30,13 @@ import JE._
trait BinaryTypedField extends TypedField[Array[Byte]] {
+
def setFromAny(in: Any): Box[Array[Byte]] = genericSetFromAny(in)
def setFromString(s: String): Box[Array[Byte]] = s match {
- case "" if optional_? => setBox(Empty)
- case _ => setBox(tryo(s.getBytes("UTF-8")))
+ case null|"" if optional_? => setBox(Empty)
+ case null|"" => setBox(Failure(notOptionalErrorMessage))
+ case _ => setBox(tryo(s.getBytes("UTF-8")))
}
def toForm: Box[NodeSeq] = Empty
View
11 persistence/record/src/main/scala/net/liftweb/record/field/BooleanField.scala
@@ -29,6 +29,7 @@ import S._
import JE._
trait BooleanTypedField extends TypedField[Boolean] {
+
def setFromAny(in: Any): Box[Boolean] = in match{
case b: java.lang.Boolean => setBox(Full(b.booleanValue))
case Full(b: java.lang.Boolean) => setBox(Full(b.booleanValue))
@@ -37,7 +38,15 @@ trait BooleanTypedField extends TypedField[Boolean] {
case _ => genericSetFromAny(in)
}
- def setFromString(s: String): Box[Boolean] = setBox(tryo(toBoolean(s)))
+ def setFromString(s: String): Box[Boolean] =
+ if(s == null || s.isEmpty) {
+ if(optional_?)
+ setBox(Empty)
+ else
+ setBox(Failure(notOptionalErrorMessage))
+ } else {
+ setBox(tryo(toBoolean(s)))
+ }
private def elem(attrs: SHtml.ElemAttr*) =
SHtml.checkbox(valueBox openOr false, (b: Boolean) => this.setBox(Full(b)), (("tabindex" -> tabIndex.toString): SHtml.ElemAttr) :: attrs.toList: _*)
View
5 persistence/record/src/main/scala/net/liftweb/record/field/DateTimeField.scala
@@ -43,8 +43,9 @@ trait DateTimeTypedField extends TypedField[Calendar] {
def setFromAny(in : Any): Box[Calendar] = toDate(in).flatMap(d => setBox(Full(dateToCal(d)))) or genericSetFromAny(in)
def setFromString(s: String): Box[Calendar] = s match {
- case "" if optional_? => setBox(Empty)
- case other => setBox(tryo(dateToCal(parseInternetDate(s))))
+ case null|"" if optional_? => setBox(Empty)
+ case null|"" => setBox(Failure(notOptionalErrorMessage))
+ case other => setBox(tryo(dateToCal(parseInternetDate(s))))
}
private def elem =
View
10 persistence/record/src/main/scala/net/liftweb/record/field/DecimalField.scala
@@ -37,7 +37,15 @@ trait DecimalTypedField extends NumericTypedField[BigDecimal] {
def setFromAny(in : Any): Box[BigDecimal] = setNumericFromAny(in, n => BigDecimal(n.toString))
- def setFromString (s : String) : Box[BigDecimal] = setBox(tryo(BigDecimal(s)))
+ def setFromString (s : String) : Box[BigDecimal] =
+ if(s == null || s.isEmpty) {
+ if(optional_?)
+ setBox(Empty)
+ else
+ setBox(Failure(notOptionalErrorMessage))
+ } else {
+ setBox(tryo(BigDecimal(s)))
+ }
def set_!(in: BigDecimal): BigDecimal = new BigDecimal(in.bigDecimal.setScale(scale, context.getRoundingMode))
View
12 persistence/record/src/main/scala/net/liftweb/record/field/DoubleField.scala
@@ -27,13 +27,23 @@ import Helpers._
import S._
trait DoubleTypedField extends NumericTypedField[Double] {
+
def setFromAny(in: Any): Box[Double] = setNumericFromAny(in, _.doubleValue)
- def setFromString(s: String): Box[Double] = setBox(tryo(java.lang.Double.parseDouble(s)))
+ def setFromString(s: String): Box[Double] =
+ if(s == null || s.isEmpty) {
+ if(optional_?)
+ setBox(Empty)
+ else
+ setBox(Failure(notOptionalErrorMessage))
+ } else {
+ setBox(tryo(java.lang.Double.parseDouble(s)))
+ }
def defaultValue = 0.0
def asJValue = valueBox.map(JDouble) openOr (JNothing: JValue)
+
def setFromJValue(jvalue: JValue) = jvalue match {
case JNothing|JNull if optional_? => setBox(Empty)
case JDouble(d) => setBox(Full(d))
View
10 persistence/record/src/main/scala/net/liftweb/record/field/EnumField.scala
@@ -52,7 +52,15 @@ trait EnumTypedField[EnumType <: Enumeration] extends TypedField[EnumType#Value]
case _ => genericSetFromAny(in)(valueManifest)
}
- def setFromString(s: String): Box[EnumType#Value] = setBox(asInt(s).flatMap(fromInt))
+ def setFromString(s: String): Box[EnumType#Value] =
+ if(s == null || s.isEmpty) {
+ if(optional_?)
+ setBox(Empty)
+ else
+ setBox(Failure(notOptionalErrorMessage))
+ } else {
+ setBox(asInt(s).flatMap(fromInt))
+ }
/** Label for the selection item representing Empty, show when this field is optional. Defaults to the empty string. */
def emptyOptionLabel: String = ""
View
5 persistence/record/src/main/scala/net/liftweb/record/field/EnumNameField.scala
@@ -39,8 +39,9 @@ trait EnumNameTypedField[EnumType <: Enumeration] extends TypedField[EnumType#Va
def setFromAny(in: Any): Box[EnumType#Value] = genericSetFromAny(in)(valueManifest)
def setFromString(s: String): Box[EnumType#Value] = s match {
- case "" if optional_? => setBox(Empty)
- case _ => setBox(enum.values.find(_.toString == s))
+ case null|"" if optional_? => setBox(Empty)
+ case null|"" => setBox(Failure(notOptionalErrorMessage))
+ case _ => setBox(enum.values.find(_.toString == s))
}
/** Label for the selection item representing Empty, show when this field is optional. Defaults to the empty string. */
View
7 persistence/record/src/main/scala/net/liftweb/record/field/IntField.scala
@@ -27,16 +27,19 @@ import Helpers._
import S._
trait IntTypedField extends NumericTypedField[Int] {
+
def setFromAny(in: Any): Box[Int] = setNumericFromAny(in, _.intValue)
def setFromString(s: String): Box[Int] = s match {
- case "" if optional_? => setBox(Empty)
- case _ => setBox(tryo(java.lang.Integer.parseInt(s)))
+ case null|"" if optional_? => setBox(Empty)
+ case null|"" => setBox(Failure(notOptionalErrorMessage))
+ case _ => setBox(tryo(java.lang.Integer.parseInt(s)))
}
def defaultValue = 0
def asJValue: JValue = valueBox.map(i => JInt(BigInt(i))) openOr (JNothing: JValue)
+
def setFromJValue(jvalue: JValue): Box[Int] = jvalue match {
case JNothing|JNull if optional_? => setBox(Empty)
case JInt(i) => setBox(Full(i.intValue))
View
11 persistence/record/src/main/scala/net/liftweb/record/field/LongField.scala
@@ -27,9 +27,18 @@ import Helpers._
import S._
trait LongTypedField extends NumericTypedField[Long] {
+
def setFromAny(in: Any): Box[Long] = setNumericFromAny(in, _.longValue)
- def setFromString(s: String): Box[Long] = setBox(asLong(s))
+ def setFromString(s: String): Box[Long] =
+ if(s == null || s.isEmpty) {
+ if(optional_?)
+ setBox(Empty)
+ else
+ setBox(Failure(notOptionalErrorMessage))
+ } else {
+ setBox(asLong(s))
+ }
def defaultValue = 0L
View
5 persistence/record/src/main/scala/net/liftweb/record/field/PasswordField.scala
@@ -76,8 +76,9 @@ trait PasswordTypedField extends TypedField[String] {
}
def setFromString(s: String): Box[String] = s match {
- case "" if optional_? => setBoxPlain(Empty)
- case _ => setBoxPlain(Full(s))
+ case null|"" if optional_? => setBoxPlain(Empty)
+ case null|"" => setBoxPlain(Failure(notOptionalErrorMessage))
+ case _ => setBoxPlain(Full(s))
}
override def validate: List[FieldError] = {
View
5 persistence/record/src/main/scala/net/liftweb/record/field/StringField.scala
@@ -41,8 +41,9 @@ trait StringTypedField extends TypedField[String] with StringValidators {
}
def setFromString(s: String): Box[String] = s match {
- case "" if optional_? => setBox(Empty)
- case _ => setBox(Full(s))
+ case null|"" if optional_? => setBox(Empty)
+ case null|"" => setBox(Failure(notOptionalErrorMessage))
+ case _ => setBox(Full(s))
}
private def elem = S.fmapFunc(SFuncHolder(this.setFromAny(_))) {
View
27 persistence/record/src/test/scala/net/liftweb/record/FieldSpec.scala
@@ -45,6 +45,7 @@ object FieldSpec extends Specification {
lazy val session = new LiftSession("", randomString(20), Empty)
def passBasicTests[A](example: A, mandatory: MandatoryTypedField[A], legacyOptional: MandatoryTypedField[A], optional: OptionalTypedField[A])(implicit m: scala.reflect.Manifest[A]): Fragment = {
+
val canCheckDefaultValues =
!mandatory.defaultValue.isInstanceOf[Calendar] // don't try to use the default value of date/time typed fields, because it changes from moment to moment!
@@ -74,6 +75,22 @@ object FieldSpec extends Specification {
in.get must_== in.defaultValue
}
}
+
+ if(!in.optional_?) {
+ "which fail when set with an empty string when not optional" in {
+ in.setFromString(null)
+ in.valueBox must beLike { case f: Failure => ok }
+ in.setFromString("")
+ in.valueBox must beLike { case f: Failure => ok }
+ }
+ } else {
+ "which don't fail when set with an empty string when optional" in {
+ in.setFromString(null)
+ in.valueBox must_== Empty
+ in.setFromString("")
+ in.valueBox must_== Empty
+ }
+ }
}
def commonBehaviorsForAllFlavors(in: TypedField[A]): Unit = {
@@ -145,6 +162,7 @@ object FieldSpec extends Specification {
}
"which initialize to some value" in {
+ mandatory.clear
mandatory.valueBox.isDefined must_== true
}
@@ -198,6 +216,15 @@ object FieldSpec extends Specification {
"which initialize to Empty" in {
optional.valueBox must_== Empty
}
+
+ "which don't fail when set with an empty string" in {
+ optional.setFromString(null)
+ optional.value must_== None
+ optional.valueBox must_== Empty
+ optional.setFromString("")
+ optional.value must_== None
+ optional.valueBox must_== Empty
+ }
"which do not fail when set to Empty" in {
optional.set(Some(example))
Something went wrong with that request. Please try again.