kitar / laravel-dynamodb

A DynamoDB based Eloquent model and Query builder for Laravel.
MIT License
179 stars 27 forks source link

Support Atomic Counters #10

Closed mehedimi closed 3 years ago

mehedimi commented 3 years ago

fixed #7

kitar commented 3 years ago

Hi @mehedimi, thank you for working on this feature! I will look into it. Please wait for a while :)

kitar commented 3 years ago

@mehedimi Thanks again for your commits. Builder looks nice, but I've noticed few things with Model::incrementOrDecrement.

  1. We cannot know the result value of increment/decrement before fetching the record again because this is an atomic counter.
  2. $extra values are not syncing.

To fix 1, how about using the UPDATED_NEW value, which DynamoDB responds, to sync. I've made a commit to make UpdateItem returns response with UPDATED_NEW values. Please take a look at it.

https://github.com/kitar/laravel-dynamodb/commit/7a8edb50923c5323f88f9b32c624ca15641ed6f3

After fixing 1, Model::incrementOrDecrement could look like this. It fixes 2 as well.

protected function incrementOrDecrement($column, $amount, $extra, $method)
{
    $query = $this->newQuery()->key($this->getKey());

    if (! $this->exists) {
        return $query->{$method}($column, $amount, $extra);
    }

    if ($this->fireModelEvent('updating') === false) {
        return false;
    }

    return tap($query->{$method}($column, $amount, $extra), function ($response) use ($column, $extra) {
        $extra[$column] = $response['Attributes'][$column];

        $this->forceFill($extra);

        $this->syncChanges();

        $this->fireModelEvent('updated', false);
    });
}
mehedimi commented 3 years ago

@kitar Thanks for your response. And sorry for the late response. I will push update soon! Please wait....

mehedimi commented 3 years ago

Hi @kitar I have updated PR. Please take a look.

kitar commented 3 years ago

@mehedimi looks nice! Thanks again for working on this issue. I will merge it and tag next version.