Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Squeryl-Record integration should be Record "dirty_?" field aware #1259

Merged
merged 1 commit into from

3 participants

@lopex

Many persistence frameworks (including Lift's Mapper, Lift's Mongodb-Record, ActiveRecord) track which fields have changed their values over record's lifetime since it was populated with data. This allows to reduce overhead when updating rows in database.
This also means that first value assignment should not set the "dirty_?" flag to true (so setFromAny shouldn't be called when the record is being populated for the first time ? or just use Empty value as blank one ?)
This also leaves a question what should be the policy of populating Optional*Fields from Record framework, since there's no information if a field was assigned before (not the case with non optional fields where Empty could mean that first assignment takes place)

As a note: Lift's Mapper fields also keep orgData (original data to track the changes, "protected def i_was_! = orgData")

@davewhittaker

Ok, just to be clear, my intention is to make it so that the dirty_? flag will not be set ONLY when a field's value is populated from the database. This fix won't result in the dirty_? flag affecting what fields get sent to the DB when Schema.update(obj) is called. I also plan on leaving the current functionality of TypedField.setBox, which means that dirty_? will be true after the user calls that method, regardless of whether the value has actually changed. Will that be a problem for you?

@lopex

Both of these changes would be insufficient, though I realize how invasive the required changes would be. I think I can work around first one with my own logic (even if it would require me to change squeryl-record or even squeryl itself)
The problem with setBox is that the "data" field is private (now I'm using reflection to get around it). If it was protected then it would be more straight forward by just overriding setBox and using it without reflection hacks. So ultimately, I'll have to change Record on my own too.

I'm well aware of Lift's community and committer policy, and I know that it would be too much to ask for the changes required to make my case work. I'll be happy with the changes you're proposing as well since they'll make my case easier.

@davewhittaker

I'm not understanding what else you need. Retrieving a Record and having all of the dirty_? flags set to false isn't enough, please reply to the thread you had started on the google group and provide use cases.

@davewhittaker davewhittaker Makes Squeryl-Record both aware of the dirty_? flag, which is reset a…
…fter values are loaded from the DB, and utilize runSafe during data loading.
27f5c8b
@dpp dpp merged commit 33d1e34 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 8, 2012
  1. @davewhittaker

    Makes Squeryl-Record both aware of the dirty_? flag, which is reset a…

    davewhittaker authored
    …fter values are loaded from the DB, and utilize runSafe during data loading.
This page is out of date. Refresh to see the latest.
View
25 persistence/squeryl-record/src/main/scala/net/liftweb/squerylrecord/RecordMetaDataFactory.scala
@@ -141,15 +141,30 @@ class RecordMetaDataFactory extends FieldMetaDataFactory {
fieldScale getOrElse super.scale
}
- private def fieldFor(o: AnyRef) = getter.get.invoke(o).asInstanceOf[TypedField[_ <: AnyRef]]
+ private def fieldFor(o: AnyRef) = getter.get.invoke(o) match {
+ case tf: TypedField[_] => tf
+ case other => org.squeryl.internals.Utils.throwError("Field's used with Squeryl must inherit from net.liftweb.record.TypedField : " + other )
+ }
- override def set(target: AnyRef, value: AnyRef) = {
- val typedField: TypedField[_] = fieldFor(target)
- typedField.setFromAny(Box !! value)
+ /**
+ * Sets the value which was retrieved from the DB into the appropriate Record field
+ */
+ override def set(target: AnyRef, value: AnyRef) = target match {
+ case record: Record[_] =>
+ record.runSafe {
+ val typedField: TypedField[_] = fieldFor(target)
+ typedField.setFromAny(Box !! value)
+ typedField.resetDirty
+ }
+ case other =>
+ org.squeryl.internals.Utils.throwError("RecordMetaDataFactory can not set fields on non Record objects : " + other)
}
override def setFromResultSet(target: AnyRef, rs: ResultSet, index: Int) = set(target, resultSetHandler(rs, index))
+ /**
+ * Extracts the value from the field referenced by o that will be stored in the DB
+ */
override def get(o: AnyRef) = fieldFor(o) match {
case enumField: EnumTypedField[_] => enumField.valueBox match {
case Full(enum: Enumeration#Value) => enum.id: java.lang.Integer
@@ -161,7 +176,7 @@ class RecordMetaDataFactory extends FieldMetaDataFactory {
}
case other => other.valueBox match {
case Full(c: Calendar) => new Timestamp(c.getTime.getTime)
- case Full(other) => other
+ case Full(other: AnyRef) => other
case _ => null
}
}
View
6 persistence/squeryl-record/src/test/scala/net/liftweb/squerylrecord/SquerylRecordSpec.scala
@@ -352,6 +352,12 @@ object SquerylRecordSpec extends Specification("SquerylRecord Specification") {
val columnDefinition = new PostgreSqlAdapter().writeColumnDeclaration(fieldMetaData, false, MySchema)
columnDefinition.endsWith("numeric(" + Company.employeeSatisfaction.context.getPrecision() +"," + Company.employeeSatisfaction.scale + ")") must_== true
}
+
+ forExample("Properly reset the dirty_? flag after loading entities") >> inTransaction {
+ val company = from(companies)(company =>
+ select(company)).page(0, 1).single
+ company.allFields map { f => f.dirty_? must_== false }
+ }
}
Something went wrong with that request. Please try again.