View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 423 | PHPOF2 | Default | public | 25 Mar 2013 14:08 | 26 Mar 2013 12:55 |
| Reporter | iarmeanu | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | confirmed | Resolution | open | ||
| Product Version | 0.12.3 | ||||
| Summary | 423: update() overrides fields that were not meant to be updated | ||||
| Description | When calling update() on an object, the current entire state of that object is persisted, rather than only the fields that changed. This may be a big problem if two concurrent processes write on the same record. I suggest caching the properties of the object as it was when it was red last time ( which we probably do already for audit purposes ), and when update() is required/requested, only update the fields that have changed since the last read. | ||||
| Steps To Reproduce | Consider the following scenario: Webpage 1 needs to modify only last_access_time, and webpage 2 needs to modify only only access_number ( on the same record ) ------------------- time is : 10:00:01 webpage 1 reads the record with two properties: - accessed_number: 7 - last_access_time = 10:00:01 webpage 2 reads the same record with two properties: - accessed_number: 7 - last_access_time = 10:00:01 -------------------------------- time is 10:00:02 webpage 1 only changes: - last access_time = 10:00:02 and calls update() webpage 2 only changes: - accessed_number = 8 and calls update() -------------------------------- time is 10:00:03 the record shows: - last access_time = 10:00:01 - accessed_number = 8 overriding the change made by webpage 1, because when webpage 2 called update() the last_access_property was reverted back to what it was when webpage 2 first red it ( 10:00:01 ) | ||||
| Tags | No tags attached. | ||||
|
|
Agreed that this would be a good thing. In fact there is a TODO in the code about exactly this, so would definitely accept a patch. Two points: 1. note that this would increase memory usage/marginally slow down PHPOF, as it would need to save values on *every* row retrieval (at the moment it only does if auditing is enabled/DBRow::$record_changes==true). This would need to be measured. 2. it does not actually solve the stated problem, only makes it slightly less acute. It is still the case that two concurrent changes will result in a "last wins" situation. In fact, it can be argued that in some cases (probably a minority) it will end up with a worse result, because you could end up with a state which is not consistent with the values submitted by either process/user. Without storing full state, the only real way of properly addressing this is something like: a) in each table, have an extra column with last changed time (or use the audit log) b) when retrieving data, cache the last modified time of that row c) if - on submission - the row has been changed by another process in the meantime, if there are any fields that conflict then re-prompt the user to submit (assuming it's a user submitting a form) This is roughly how it's done in Bugzilla (http://bugzilla.org) to good effect. |
|
|
It will solve the stated problem ( because the two pages update different properties on the record ). It will not solve the problem of two processes that write the same property concurrently ( but that should be addressed by using transactions with various locks set for the record - a fix for bug 422 ). |
