Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AR force update #1611

Closed
genichyar opened this issue Dec 25, 2013 · 8 comments
Closed

AR force update #1611

genichyar opened this issue Dec 25, 2013 · 8 comments

Comments

@genichyar
Copy link
Contributor

Sometimes I need to update optimistic locks's counter even if no attribute was changed. In the current version It's hard to do it.

I propose to set a variable $forceUpdate at \yii\db\BaseActiveRecord class.
Also updateInternal method should be updated.

public $forceUpdate = false;

protected function updateInternal($attributes = null)
{
        if (!$this->beforeSave(false)) {
                return false;
        }
        $values = $this->getDirtyAttributes($attributes);

        $lock = $this->optimisticLock();    
        if (empty($values) && ($lock === null || !$this->forceUpdate)) {
                $this->afterSave(false);
                return 0;
        }
        $condition = $this->getOldPrimaryKey(true);

        if ($lock !== null) {
                if (!isset($values[$lock])) {
                        $values[$lock] = $this->$lock + 1;
                }
                $condition[$lock] = $this->$lock;
        }
        // We do not check the return value of updateAll() because it's possible
        // that the UPDATE statement doesn't change anything and thus returns 0.
        $rows = $this->updateAll($values, $condition);

        if ($lock !== null && !$rows) {
                throw new StaleObjectException('The object being updated is outdated.');
        }

        foreach ($values as $name => $value) {
                $this->_oldAttributes[$name] = $this->_attributes[$name];
        }
        $this->afterSave(false);
        return $rows;
}
@samdark
Copy link
Member

samdark commented Dec 26, 2013

What's your situation exactly?

@qiangxue
Copy link
Member

Added markAttributeDirty(). You can call this method to mark an attribute as dirty to force updating a record.

@genichyar
Copy link
Contributor Author

@samdark , I have a following situation: There is an AR Model Form with a file field. File field is perfomed in beforeSave() method of the AR Model. If two users is editing same record simultaneously and one of them will update only file field, second will not received an StaleObjectException. Solution is increase lock attribute value when file is uploading.

@qiangxue , thank you for your job, but I can't solve an problem with markAttributeDirty()
For example we have a table with following attributes: id, name, version. version is used for optimisticLock.
I have only one way to force update. It is calliing $this->markAttributeDirty('name');
But I will get next sql: UPDATE table SET name='name', version=2 WHERE id=1 AND version=1
I no need to update name. I want to get UPDATE table SET version=2 WHERE id=1 AND version=1

If table has only 2 fields id and version I haven't ways to call markAttributeDirty()

If you don't want to set a variable $forceUpdate, what do you think about following change?

$condition[$lock] = $this->$lock;

to

$condition[$lock] = $this->getOldAttribute($lock);

I think it solves the problem. If I want to force update I will call $this->version++;

@qiangxue
Copy link
Member

If two users is editing same record simultaneously and one of them will update only file field, second will not received an StaleObjectException.

Why the second will not receive the exception? If the first user saves the record first, the lock counter will be incremented by one, and when the second user tries to save the record, he will find the lock counter is outdated.

@genichyar
Copy link
Contributor Author

If the first user saves the record first, the lock counter will be incremented by one

You're right, it would have happened if first user change at least one field. But if user change only file field of the form, AR model will not be saved. So lock counter will not be incremented.

@qiangxue
Copy link
Member

But why don't you modify the AR model (e.g. set the update time) since the model does get changed? It seems your design is problematic...

@genichyar
Copy link
Contributor Author

Setting update time is good idea. But in my task it will be unused field. When model is changed I have to update update_time and lock field instead of a just lock.

@qiangxue
Copy link
Member

I think your use case is uncommon. We certainly don't want to introduce an extra property to solve this uncommon problem because it means extra memory cost (we are very careful at adding any additional new properties to base classes.) For your case, I think markAttributeDirty() should be a sufficient workaround, although you would argue it won't work if there are only id and version columns. I don't think forcing updating a column such as name should be a big issue, since it doesn't really change anything.

tonydspaniard added a commit to tonydspaniard/yii2 that referenced this issue Dec 27, 2013
* upstream: (21 commits)
  Fixes yiisoft#1643: Added default value for `Captcha::options`
  Fixes yiisoft#1654: Fixed the issue that a new message source object is generated for every new message being translated
  Allow dash char in ActionColumn’s button names.
  Added SecurityTest.
  fixed functional test when enablePrettyUrl is false.
  fixed composer.json
  minor doc fix.
  Fixes yiisoft#1634: Use masked CSRF tokens to prevent BREACH exploits
  Use better random CSRF token.
  GII unique indexes avoid autoIncrement columns
  updated debug retry params.
  Added sleep().
  Added unit test for ActiveRecord::updateAttributes().
  Fixes yiisoft#1641: Added `BaseActiveRecord::updateAttributes()`
  Fixed yiisoft#1504: Debug toolbar isn't loaded successfully in some environments when xdebug is enabled
  Mongo README.md updated.
  Fixes yiisoft#1611: Added `BaseActiveRecord::markAttributeDirty()`
  Number validator was missing
  Fixes yiisoft#1638: prevent table names from being enclosed within curly brackets twice.
  Unique indexes rules for single columns into array
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants