Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Record fields should do a == check in setBox before setting dirty_? flag #1265

Merged
merged 1 commit into from

2 participants

@davewhittaker davewhittaker was assigned
@davewhittaker davewhittaker Some updates to Record Fields. Added a check on whether the newly set…
… value differed from the previous value before setting the dirty_? flag in setBox. In the process of setting up unit testing for that feature, I noticed that non-mandatory fields weren't being tested properly, and fixing that caused some tests to fail that required a few other changes. I also tried to add a bit more API doc as I went through.
f3e45ff
@dpp dpp merged commit c6224ce into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 10, 2012
  1. @davewhittaker

    Some updates to Record Fields. Added a check on whether the newly set…

    davewhittaker authored
    … value differed from the previous value before setting the dirty_? flag in setBox. In the process of setting up unit testing for that feature, I noticed that non-mandatory fields weren't being tested properly, and fixing that caused some tests to fail that required a few other changes. I also tried to add a bit more API doc as I went through.
This page is out of date. Refresh to see the latest.
View
36 persistence/record/src/main/scala/net/liftweb/record/Field.scala
@@ -146,6 +146,11 @@ trait OwnedField[OwnerType <: Record[OwnerType]] extends BaseField {
/** Refined trait for fields holding a particular value type */
trait TypedField[ThisType] extends BaseField {
+
+ /*
+ * Unless overriden, MyType is equal to ThisType. Available for
+ * backwards compatibility
+ */
type MyType = ThisType // For backwards compatability
type ValidationFunction = ValueType => List[FieldError]
@@ -212,6 +217,7 @@ trait TypedField[ThisType] extends BaseField {
def setBox(in: Box[MyType]): Box[MyType] = synchronized {
needsDefault = false
+ val oldValue = valueBox
data = in match {
case _ if !canWrite_? => Failure(noValueErrorMessage)
case Full(_) => set_!(in)
@@ -219,12 +225,17 @@ trait TypedField[ThisType] extends BaseField {
case (f: Failure) => set_!(f) // preserve failures set in
case _ => Failure(notOptionalErrorMessage)
}
- dirty_?(true)
+ val same = (oldValue, valueBox) match {
+ case (Full(ov), Full(nv)) => ov == nv
+ case (a, b) => a == b
+ }
+ dirty_?(!same)
data
}
// Helper methods for things to easily use mixins and so on that use ValueType instead of Box[MyType], regardless of the optional-ness of the field
protected def toValueType(in: Box[MyType]): ValueType
+
protected def toBoxMyType(in: ValueType): Box[MyType]
protected def set_!(in: Box[MyType]): Box[MyType] = runFilters(in, setFilterBox)
@@ -313,6 +324,11 @@ trait TypedField[ThisType] extends BaseField {
}
trait MandatoryTypedField[ThisType] extends TypedField[ThisType] with Product1[ThisType] {
+
+ /**
+ * ValueType represents the type that users will work with. For MandatoryTypeField, this is
+ * equal to ThisType.
+ */
type ValueType = ThisType // For util.BaseField
//TODO: fullfil the contract of Product1[ThisType]
@@ -335,6 +351,7 @@ trait MandatoryTypedField[ThisType] extends TypedField[ThisType] with Product1[T
def value: MyType = valueBox openOr defaultValue
def get: MyType = value
+
def is: MyType = value
protected def liftSetFilterToBox(in: Box[MyType]): Box[MyType] = in.map(v => setFilter.foldLeft(v)((prev, f) => f(prev)))
@@ -354,6 +371,11 @@ trait MandatoryTypedField[ThisType] extends TypedField[ThisType] with Product1[T
}
trait OptionalTypedField[ThisType] extends TypedField[ThisType] with Product1[Box[ThisType]] {
+
+ /**
+ * ValueType represents the type that users will work with. For OptionalTypedField, this is
+ * equal to Option[ThisType].
+ */
type ValueType = Option[ThisType] // For util.BaseField
//TODO: fullfil the contract of Product1[ThisType]
@@ -371,15 +393,21 @@ trait OptionalTypedField[ThisType] extends TypedField[ThisType] with Product1[Bo
def set(in: Option[MyType]): Option[MyType] = setBox(in) or defaultValueBox
def toValueType(in: Box[MyType]) = in
+
def toBoxMyType(in: ValueType) = in
def value: Option[MyType] = valueBox
def get: Option[MyType] = value
+
def is: Option[MyType] = value
- protected def liftSetFilterToBox(in: Box[MyType]): Box[MyType] = setFilter.foldLeft(in)((prev, f) => f(prev))
-
+ protected def liftSetFilterToBox(in: Box[MyType]): Box[MyType] = setFilter.foldLeft(in){ (prev, f) =>
+ prev match {
+ case fail: Failure => fail //stop on failure, otherwise some filters will clober it to Empty
+ case other => f(other)
+ }
+ }
def defaultValueBox: Box[MyType] = Empty
@@ -392,7 +420,7 @@ trait OptionalTypedField[ThisType] extends TypedField[ThisType] with Product1[Bo
}
/**
- * A simple field that can store and retreive a value of a given type
+ * A simple field that can store and retrieve a value of a given type
*/
trait Field[ThisType, OwnerType <: Record[OwnerType]] extends OwnedField[OwnerType] with TypedField[ThisType] {
View
7 persistence/record/src/main/scala/net/liftweb/record/field/PostalCodeField.scala
@@ -29,6 +29,7 @@ import S._
trait PostalCodeTypedField extends StringTypedField {
+
protected val country: CountryField[_]
override def setFilter = toUpper _ :: trim _ :: super.setFilter
@@ -52,9 +53,7 @@ trait PostalCodeTypedField extends StringTypedField {
}
}
-class PostalCodeField[OwnerType <: Record[OwnerType]](rec: OwnerType, val country: CountryField[OwnerType])
-extends StringField(rec, 32) with PostalCodeTypedField
+class PostalCodeField[OwnerType <: Record[OwnerType]](rec: OwnerType, val country: CountryField[OwnerType]) extends StringField(rec, 32) with PostalCodeTypedField
-class OptionalPostalCodeField[OwnerType <: Record[OwnerType]](rec: OwnerType, val country: CountryField[OwnerType])
-extends OptionalStringField(rec, 32) with PostalCodeTypedField
+class OptionalPostalCodeField[OwnerType <: Record[OwnerType]](rec: OwnerType, val country: CountryField[OwnerType]) extends OptionalStringField(rec, 32) with PostalCodeTypedField
View
89 persistence/record/src/test/scala/net/liftweb/record/FieldSpec.scala
@@ -42,44 +42,90 @@ object FieldSpec extends Specification("Record Field Specification") {
def passBasicTests[A](example: A, mandatory: MandatoryTypedField[A], legacyOptional: MandatoryTypedField[A], optional: OptionalTypedField[A])(implicit m: scala.reflect.Manifest[A]): Unit = {
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!
+
+ def commonBehaviorsForMandatory(in: MandatoryTypedField[A]): Unit = {
+
+ if (canCheckDefaultValues) {
+ "which have the correct initial value" in {
+ in.get must_== in.defaultValue
+ }
+ }
+
+ "which are readable and writable" in {
+ in.set(example)
+ in.get must_== example
+ in.clear
+ in.get must_!= example
+ in.setBox(Box !! example)
+ in.get must_== example
+ }
+ if (canCheckDefaultValues) {
+ "which correctly clear back to the default value" in {
+ in.set(example)
+ in.clear
+ in.get must_== in.defaultValue
+ }
+ }
+ }
+
def commonBehaviorsForAllFlavors(in: TypedField[A]): Unit = {
+
if (canCheckDefaultValues) {
- "which have the correct initial value" in {
- mandatory.value must_== mandatory.defaultValue
- mandatory.valueBox must_== mandatory.defaultValueBox
+ "which have the correct initial boxed value" in {
+ in match {
+ case mandatory: MandatoryTypedField[_] =>
+ mandatory.value must_== mandatory.defaultValue
+ case _ => ()
+ }
+ in.valueBox must_== in.defaultValueBox
}
}
- "which are readable and writable" in {
- mandatory.valueBox must verify(_.isDefined)
- mandatory.set(example)
- mandatory.value must_== example
- mandatory.valueBox must_== Full(example)
- mandatory.clear
- mandatory.value must_!= example
- mandatory.valueBox must_!= Full(example)
- mandatory.setBox(Full(example))
- mandatory.value must_== example
- mandatory.valueBox must_== Full(example)
+ "which have readable and writable boxed values" in {
+ in.setBox(Full(example))
+ in.valueBox must verify(_.isDefined)
+ in.valueBox must_== Full(example)
+ in.clear
+ in.valueBox must_!= Full(example)
}
if (canCheckDefaultValues) {
- "which correctly clear back to the default" in {
- mandatory.valueBox must verify(_.isDefined)
- mandatory.clear
- mandatory.valueBox must_== mandatory.defaultValueBox
+ "which correctly clear back to the default box value" in {
+ in.setBox(Full(example))
+ in.valueBox must verify(_.isDefined)
+ in.clear
+ in.valueBox must_== in.defaultValueBox
}
}
"which capture error conditions set in" in {
- mandatory.setBox(Failure("my failure"))
- mandatory.valueBox must_== Failure("my failure")
+ in.setBox(Failure("my failure"))
+ in.valueBox must_== Failure("my failure")
+ }
+
+ "which are only flagged as dirty_? when setBox is called with a different value" in {
+ in.clear
+ in match {
+ case owned: OwnedField[_] => owned.owner.runSafe {
+ in.resetDirty
+ }
+ case _ => in.resetDirty
+ }
+ in.dirty_? must_== false
+ val valueBox = in.valueBox
+ in.setBox(valueBox)
+ in.dirty_? must_== false
+ val exampleBox = Full(example)
+ valueBox must verify { v => ! (exampleBox === v) }
+ in.setBox(exampleBox)
+ in.dirty_? must_== true
}
}
"support mandatory fields" in {
commonBehaviorsForAllFlavors(mandatory)
+ commonBehaviorsForMandatory(mandatory)
"which are configured correctly" in {
mandatory.optional_? must_== false
@@ -98,7 +144,8 @@ object FieldSpec extends Specification("Record Field Specification") {
"support 'legacy' optional fields (override optional_?)" in {
commonBehaviorsForAllFlavors(legacyOptional)
-
+ commonBehaviorsForMandatory(legacyOptional)
+
"which are configured correctly" in {
legacyOptional.optional_? must_== true
}
Something went wrong with that request. Please try again.