***BREAKING CHANGES in Record***

[Ross Mellgren]

Hey all. As foreshadowed by DPP, I just pushed three changes which affect Record. A couple are major. The changes were:

354 - Bug with record field initialisation

479 - Allow for unowned field types in lift-record, to support squeryl integration

594 - overriding Field#label not affecting TextareaField#toFrom

617 - remove undesired display logic from record Fields

(the last two were addressed by the same change)

What was broken/changed follows. Actions you probably need to take or changes that are particularly breaking are in bold.

---

issue354 originally described a bug where new MyRecord() would not properly initialize the field metadata -- MyRecord.createRecord was the proper way to instantiate a record. The easiest way to see this was by new MyRecord().myField.name would be null, when instead it should be "myField". The original fix was to make new MyRecord() work, however it turns out that in Scala 2.8 the initialization of class members was changed and it broke the fix. Moreover, it seems that the fix was very fragile. So, I pulled that out and reversed it back to MyRecord.createRecord being the correct path.

For this change, you should change all your records by deleting

  def createRecord = new MyRecord 

from your MetaRecord objects, and make your record constructors private (to ensure they are not accidentally called).

That is, if you previously had

class MyRecord extends Record[MyRecord] {

  object myField extends StringField(this, 100)

}

object MyRecord extends MyRecord with MetaRecord[MyRecord] {

  def createRecord = new MyRecord

}

you should instead use

class MyRecord private () extends Record[MyRecord] {

  object myField extends StringField(this, 100)

}

object MyRecord extends MyRecord with MetaRecord[MyRecord]

If you were implementing createRecord to do something other than new MyRecord, you can override protected def instantiateRecord to provide other factory mechanics. I made createRecord final to make sure people update.

---

issue479 constitutes a refactoring of the Field type hierarchy for Records. This was made to expose more information into the type system, as well as reify concrete field types as traits that can be checked at runtime.

Previously, the hierarchy looked like this (BooleanField as an example):

BooleanField[OwnerType] <: Field[OwnerType, Boolean] <: OwnedField[OwnerType] <: FieldIdentifier

Now it's more variegated, and there are separate concrete classes for optional and nonoptional (mandatory) fields:

BooleanField[OwnerType] <: BooleanTypedField <: Field[OwnerType, Boolean]

    <: MandatoryTypedField[Boolean] <: TypedField[Boolean] <: BaseField <: FieldIdentifier

    <: OwnedField[OwnerType] <: BaseField <: FieldIdentifier

OptionalBooleanField[OwnerType] <: BooleanTypedField <: Field[OwnerType, Boolean]

    <: OptionalTypedField[Boolean] <: TypedField[Option[Boolean]] <: BaseField <: FieldIdentifier

    <: OwnedField[OwnerType] <: BaseField <: FieldIdentifier

(I've diagrammed this as an attachment to the issue: issue479.pdf)

For normal uses, nothing will have changed -- MandatoryTypedField can be used with optional values by overriding optional_? to true as a backward compatibility measure. However, if you switch to the Optional variant, then value and set will give back/take an Option, which forces you to handle missing values correctly rather than silently defaulting them.

With the new types, you can now refer to fields just by their type (as TypedField[Whatever]) or just by their owner (OwnedField[Owner]) or both (Field[Owner, Whatever]). Most Mere Mortals will not need to concern themselves with this, but if you're doing new and crazy generic programming, there it is.

The new BlahTypedField traits are where the meat of each field type live, and because they are not parameterized you can do type-safe matching by type at runtime. That is, this was never safe:

(field: Field[_, _]) match {

  case Field[_, Int] => ...

}

Because the type parameters are not reified and therefore cannot be checked via reflection. But now it's possible:

(field: Field[_, _]) match {

  case IntTypedField => ...

}

---

issue594 and issue617 are both concerned with form generation, but the change for them is the same -- make Record Fields compatible with Screen/Wizard by making toForm operate like MappedField toForm. That is, instead of returning a div with the label and the form element, now they return Box[NodeSeq] with just the form element and you can wrap it in whatever markup you like. If you want the old behavior back, mix DisplayWithLabel[MyRecord] into your field, viz:

  object myField extends StringField(this, 100) with DisplayWithLabel

To make Records fully compatible with Screen/Wizard, several changes had to be made:

  - Previously, validators were of type Box[MyType] => List[FieldError]. They are now MyType => List[FieldError]. This should not be a problem because setBox already checked the Empty case for optional fields, so you should never have needed to validate Empty specially. Note that with the previous change (issue479), OptionalBlahField validators have type Option[Blah] => List[FieldError]

  - Previously, toForm returned NodeSeq. It now returns Box[NodeSeq].

  - Previously, to define validators you would say override def validators = blah :: super.validators. Now, you say override def valiations = blah :: super.validations.

  - Field.asXHtml has been removed. Use toForm instead.

---

Of course, if anybody has questions or comments, just ask.

Viva la Record,

-Ross

[Jorge Ortiz]

Hey Ross,

I'm currently in the process of upgrading code to Lift 2.1 RC1.

Is there a reason why OptionalTypedFields and the like use Option instead of Box?

--j

--
You received this message because you are subscribed to the Google Groups "Lift" group.
To post to this group, send email to lif...@googlegroups.com.
To unsubscribe from this group, send email to liftweb+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/liftweb?hl=en.

[Ross Mellgren]

Yes, because external DSLs will never be coded to Box and usually won't be coded with view conversion to Option, e.g. from Squeryl there are implicit conversions from Option[String] to DSL nodes.

I felt that the compatibility issue was worth it in this case -- you can get the real underlying Box (with Failure or whatever) with valueBox, and if you have option2Box in scope it's not that common that it'd be an issue (as least, as far as my experience with Option goes)

-Ross

[David Pollak]

On Wed, Sep 8, 2010 at 2:46 PM, Ross Mellgren <dri...@gmail.com> wrote:

Yes, because external DSLs will never be coded to Box and usually won't be coded with view conversion to Option, e.g. from Squeryl there are implicit conversions from Option[String] to DSL nodes.

I felt that the compatibility issue was worth it in this case -- you can get the real underlying Box (with Failure or whatever) with valueBox, and if you have option2Box in scope it's not that common that it'd be an issue (as least, as far as my experience with Option goes)

Hmmmm.... Box autoconverts into option without anything in scope.  I really, really, really wish we could be using Box instead of Option.

--
Lift, the simply functional web framework http://liftweb.net
Beginning Scala http://www.apress.com/book/view/1430219890
Follow me: http://twitter.com/dpp
Blog: http://goodstuff.im
Surf the harmonics

[Ross Mellgren]

On Sep 8, 2010, at 6:12 PM, David Pollak wrote:

On Wed, Sep 8, 2010 at 2:46 PM, Ross Mellgren <dri...@gmail.com> wrote:

Yes, because external DSLs will never be coded to Box and usually won't be coded with view conversion to Option, e.g. from Squeryl there are implicit conversions from Option[String] to DSL nodes.

I felt that the compatibility issue was worth it in this case -- you can get the real underlying Box (with Failure or whatever) with valueBox, and if you have option2Box in scope it's not that common that it'd be an issue (as least, as far as my experience with Option goes)

Hmmmm.... Box autoconverts into option without anything in scope.  I really, really, really wish we could be using Box instead of Option.

It only autoconverts where implicits can be in scope (cannot chain implicits), so the DSLs would have to be like (heck, I don't even know if this works):

implicit def optionStringToDSLString[T <% Option[String]](in: T): DSLString

but they're

implicit def optionStringToDSLString(in: Option[String]): DSLString

Not sure of a clean way around it. If we used box, for Squeryl at least we'd have to copy/paste a pile of implicit conversions into the squeryl integration and replace Option with Box.

-Ross

[David Pollak]

On Wed, Sep 8, 2010 at 3:17 PM, Ross Mellgren <dri...@gmail.com> wrote:

On Sep 8, 2010, at 6:12 PM, David Pollak wrote:

On Wed, Sep 8, 2010 at 2:46 PM, Ross Mellgren <dri...@gmail.com> wrote:

Yes, because external DSLs will never be coded to Box and usually won't be coded with view conversion to Option, e.g. from Squeryl there are implicit conversions from Option[String] to DSL nodes.

I felt that the compatibility issue was worth it in this case -- you can get the real underlying Box (with Failure or whatever) with valueBox, and if you have option2Box in scope it's not that common that it'd be an issue (as least, as far as my experience with Option goes)

Hmmmm.... Box autoconverts into option without anything in scope.  I really, really, really wish we could be using Box instead of Option.

It only autoconverts where implicits can be in scope (cannot chain implicits), so the DSLs would have to be like (heck, I don't even know if this works):

implicit def optionStringToDSLString[T <% Option[String]](in: T): DSLString

but they're

implicit def optionStringToDSLString(in: Option[String]): DSLString

Are these specific DSLs or are they hypothetical DSLs?

 

Not sure of a clean way around it. If we used box, for Squeryl at least we'd have to copy/paste a pile of implicit conversions into the squeryl integration and replace Option with Box.

Yeah... I thought we worked through the Option and Box issue with Max already.

[Ross Mellgren]

On Sep 8, 2010, at 6:33 PM, David Pollak wrote:

On Wed, Sep 8, 2010 at 3:17 PM, Ross Mellgren <dri...@gmail.com> wrote:

On Sep 8, 2010, at 6:12 PM, David Pollak wrote:

On Wed, Sep 8, 2010 at 2:46 PM, Ross Mellgren <dri...@gmail.com> wrote:

Yes, because external DSLs will never be coded to Box and usually won't be coded with view conversion to Option, e.g. from Squeryl there are implicit conversions from Option[String] to DSL nodes.

I felt that the compatibility issue was worth it in this case -- you can get the real underlying Box (with Failure or whatever) with valueBox, and if you have option2Box in scope it's not that common that it'd be an issue (as least, as far as my experience with Option goes)

Hmmmm.... Box autoconverts into option without anything in scope.  I really, really, really wish we could be using Box instead of Option.

It only autoconverts where implicits can be in scope (cannot chain implicits), so the DSLs would have to be like (heck, I don't even know if this works):

implicit def optionStringToDSLString[T <% Option[String]](in: T): DSLString

but they're

implicit def optionStringToDSLString(in: Option[String]): DSLString

Are these specific DSLs or are they hypothetical DSLs?

Squeryl's.

Not sure of a clean way around it. If we used box, for Squeryl at least we'd have to copy/paste a pile of implicit conversions into the squeryl integration and replace Option with Box.

Yeah... I thought we worked through the Option and Box issue with Max already.

I think the TypeArithmetic issue we resolved by not using Box as the return type for value ;-)

But, the code has a storied history, it could be no longer an issue.

http://github.com/max-l/Squeryl/blob/master/src/main/scala/org/squeryl/dsl/TypeArithmetic.scala

-Ross

[Jorge Ortiz]

Ok, I've given up on OptionalTypedField. It's really inconvenient that value returns an Option[T] when a defaultValue exists.

For now I'm sticking with MandatoryTypedField, which is (in theory?) backwards compatible with previous versions of Lift, yet there's a slight change in semantics. In the past, if a StringField had a filter (String => String) which returned null, the field data would be set as "Empty" and everything would work fine. In 2.1RC1, if a filter on a StringField returns null, the data is set as Full(null), which causes downstream havoc.

Is there a workaround for this change in semantics of filters?

--j

[Jorge Ortiz]

Nevermind, I've figured it out.

For posterity: you can override setFilterBox (instead of setFilter) for those filters which need Box[String] => Box[String] transformations, then your filter can properly return Empty.

--j

[Jorge Ortiz]

Another breaking change: .toString on StringField now returns "Full(...)" and "Empty" rather than the String in the field.

--j

[Ross Mellgren]

Please file this one, it was certainly not intentional.

-Ross

[Ross Mellgren]

Unfortunately true. This was due to the wizard/screen implementation.

-Ross

[Ross Mellgren]

On Sep 8, 2010, at 9:15 PM, Jorge Ortiz wrote:

Ok, I've given up on OptionalTypedField. It's really inconvenient that value returns an Option[T] when a defaultValue exists.

I don't understand what you mean by this?

-Ross

[Jorge Ortiz]

Thanks Ross! Filed as #641

--j

[Jorge Ortiz]

I've identified another regression between Lift 2.0/2.1-M1 and Lift 2.1-RC1/RC2, filed here: http://www.assembla.com/spaces/liftweb/tickets/652-regression-in-fields-marked-optional

--j

[elversatile]

Unless, the errors are rendered differently now, I noticed that
DisplayWithLabel misses the error message:
<lift:msg id={id} errorClass="lift_error"/>

It's a quick fix, toForm should yield:

<div id={ id + "_holder" }>
<div><label for={ id + "_field" }>{ displayName }</label></div>
{ control }
<lift:msg id={id} errorClass="lift_error"/>
</div>

-Eldar

[Ross Mellgren]

Yeah, I think that's this one:

http://groups.google.com/group/liftweb/browse_thread/thread/c1adb9a7b08781fc?hl=en#

I also think it's in that round trip though I haven't had time to think about it all the way through to be sure.

-Ross

[Ross Mellgren]

Indeed that is true. Please open a ticket.

-Ross

[Ross Mellgren]

Up on RB: http://reviewboard.liftweb.net/r/448/

I fixed some other bugs and corner cases that were revealed from unit tests.

-Ross

On Sep 17, 2010, at 5:51 PM, Jorge Ortiz wrote:

[David Bernard]

Thanks Ross,