Updated RequestVarEM to use TransientRequestVar
Updated 2 years, 2 months ago
| Kris Nuttycombe | Reviewers | ||
| kjn-jpa-fix | |||
| http://github.com/dpp/liftweb/issues/#issue/184 | dchenbecker, dpp, timperrett, marius, charles, heiko, joni, atsuhiko, jorge, naftoli, kris, indrajit, alexb | ||
| None | LiftWeb-archive | ||
Changed TransientRequestVar to have private[liftweb] access. Updated RequestVarEM to use TransientRequestVar to ensure that each HTTP exchange results in a separate transaction when using JTA.
Tested for AJAX/JTA exchange that was previously failing in Gaiam lift app.
Posted 2 years, 2 months ago (November 13th, 2009, 3:09 p.m.)
Did you mean for your Loc changes to make it into this diff/commit?
Review request changed
Updated 2 years, 2 months ago (November 13th, 2009, 3:20 p.m.)
-
- added Diff r2
Fixed the diff; I had rebased my fix against the wrong branch.
Review request changed
Updated 2 years, 2 months ago (November 13th, 2009, 3:22 p.m.)
-
- added Diff r3
Corrected the diff for real this time.
Excellent Smithers (can I say that in a review?)
Posted 2 years, 2 months ago (November 13th, 2009, 4:10 p.m.)
Any chance you can add a one-liner to the scaladoc on TransientRequestVar to indicate what it's for?
Review request changed
Updated 2 years, 2 months ago (November 13th, 2009, 11:01 p.m.)
-
- added Diff r4
Documentation enhancements; added update() method to AnyVar in order to allow such usage as
object myRequestVar extends RequestVar[String]("bar")
myRequestVar = "foo"
At the time when I originally suggested the addition of update(f: T => T) to AnyVar I think that I was ignorant of how the Scala compiler allows rewriting to an invocation of "update" from the equals syntax. The change that I made doesn't conflict (because the type of the parameter is different, of course) but I'm wondering whether it wouldn't make sense to rename the current update(f: T => T) to updateWith. Thoughts?
Also, as a more heretical suggestion, I think that the apply(what: T) method of AnyVar should be deprecated. Having something that looks like function application result in a mutation just seems... wrong. :)
Posted 2 years, 2 months ago (November 13th, 2009, 11:10 p.m.)
The update method results in the following syntax: myRequestVar() = "new value" You must include the () to trigger update, so the update() method you have doesn't get you the syntax you are looking for. The apply() syntax is consistent with the syntax used for setting fields in mapper, so it's not going to be deprecated. I'm all for fixing the RequestVarEM issue, but please don't do scope creep. If there are new features/changes/deprecations, let's talk about them on the Lift list rather than putting them into a change request that not everyone will see.