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

TransactionWrite update() with a block #846

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,7 @@ model
* `#destroy`/`#destroy!` - remove an model
* `#upsert` - add a new model or update an existing one, no callbacks
* `#update_fields` - update a model without its instantiation
* `#update` - update a model using a block that supports set, add, and delete operations on fields
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `#update` - update a model using a block that supports set, add, and delete operations on fields
* `#update` - update a model using a block that supports set, add, remove, and delete operations on fields


These methods are supposed to behave exactly like their
non-transactional counterparts.
Expand Down Expand Up @@ -1166,6 +1167,19 @@ Dynamoid::TransactionWrite.execute do |txn|
# sets the name and title for a user
# The user is found by id (that equals 1)
txn.update_fields(User, '1', name: 'bob', title: 'mister')

# sets the name, increments a count and deletes a field
txn.update(user) do |u| # a User instance is provided
u.set(name: 'bob')
u.add(article_count: 1)
u.delete(:title)
end

# adds to a set of integers and deletes from a set of strings
txn.update(user) do |u|
u.add(friend_ids: [1, 2])
u.delete(child_names: ['bebe'])
end
end
```

Expand Down
22 changes: 22 additions & 0 deletions lib/dynamoid/transaction_write.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'dynamoid/transaction_write/delete_with_instance'
require 'dynamoid/transaction_write/destroy'
require 'dynamoid/transaction_write/save'
require 'dynamoid/transaction_write/update'
require 'dynamoid/transaction_write/update_fields'
require 'dynamoid/transaction_write/update_attributes'
require 'dynamoid/transaction_write/upsert'
Expand Down Expand Up @@ -394,6 +395,27 @@ def update_attributes!(model, attributes)
register_action action
end

# Update attributes using a block.
#
# Dynamoid::TransactionWrite.execute do |t|
# t.update(user) do |u|
# u.set age: 27, last_name: 'Tylor'
# u.add article_count: 1 # increment a counter
# u.delete favorite_colors: 'green' # remove from a set
# u.delete :first_name # clear a field
# end
# end
#
# Returns +true+ if saving is successful and +false+
# otherwise.
#
# @param model [Dynamoid::Document] a model
# @return [true|false] Whether updating successful or not
def update(model, &block)
action = Dynamoid::TransactionWrite::Update.new(model, raise_error: false, &block)
register_action action
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new transactional method #update behaves a bit differently than the non-transactional one. Non-transactional updates a model and its fields with new values. It's easy enough because UpdateItem may be configured to return all attributes of an updated item. TransactWriteItems doesn't support this AFAIK. And manually implement logic of remove/add operations to update attributes in memory also doesn't appeal to me. So accepting a model as a parameter seems incorrect to me.

Instead I would propose to accept a primary key as a parameter. There is no such non-transactional method but I will add it later to complement existing #update method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. Because of that should we just change update_fields to optionally allow a block or do you think adding an entirely new method to TransactionWrite would be better?

Copy link
Member

@andrykonchin andrykonchin Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer adding new method update in the TransactionWrite module (like it was already done here).

I overlooked that semantic of existing .update method is find && update_attributes. And indeed .update_fields looks very close to the #update method.

So go ahead with adding a block parameter to the existing transactional method .update_fields.


# Delete a model.
#
# Can be called either with a model:
Expand Down
88 changes: 86 additions & 2 deletions lib/dynamoid/transaction_write/save.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
module Dynamoid
class TransactionWrite
class Save < Base
def initialize(model, **options)
def initialize(model, **options, &block)
super()

@model = model
Expand All @@ -15,6 +15,12 @@ def initialize(model, **options)
@aborted = false
@was_new_record = model.new_record?
@valid = nil

@additions = {}
@deletions = {}
@removals = []

yield(self) if block_given?
end

def on_registration
Expand Down Expand Up @@ -71,7 +77,7 @@ def aborted?
end

def skipped?
@model.persisted? && [email protected]?
@model.persisted? && [email protected]? && @additions.blank? && @deletions.blank? && @removals.blank?
end

def observable_by_user_result
Expand All @@ -86,6 +92,30 @@ def action_request
end
end

# sets a value in the attributes
def set(attributes)
@model.assign_attributes(attributes)
end

# adds to array of fields for use in REMOVE update expression
def remove(field)
@removals << field
end

# increments a number or adds to a set, starts at 0 or [] if it doesn't yet exist
def add(values)
@additions.merge!(values)
end

# deletes a value or values from a set type or simply sets a field to nil
def delete(field_or_values)
if field_or_values.is_a?(Hash)
@deletions.merge!(field_or_values)
else
remove(field_or_values)
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to have a separate class to collect user actions (I mean to set/add/delete), similar to ItemUpdater (https://github.com/Dynamoid/dynamoid/blob/master/lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb)


private

def validate_model!
Expand Down Expand Up @@ -141,6 +171,10 @@ def action_request_to_update

update_expression = "SET #{update_expression_statements.join(', ')}"

update_expression = set_additions(expression_attribute_values, update_expression)
update_expression = set_deletions(expression_attribute_values, update_expression)
expression_attribute_names, update_expression = set_removals(expression_attribute_names, update_expression)

{
update: {
key: key,
Expand All @@ -159,6 +193,56 @@ def touch_model_timestamps(skip_created_at:)
@model.updated_at = timestamp unless @options[:touch] == false && !@was_new_record
@model.created_at ||= timestamp unless skip_created_at
end

# adds all of the ADD statements to the update_expression and returns it
def set_additions(expression_attribute_values, update_expression)
return update_expression unless @additions.present?

# ADD statements can be used to increment a counter:
# txn.update!(UserCount, "UserCount#Red", {}, options: {add: {record_count: 1}})
add_keys = @additions.keys
update_expression += " ADD #{add_keys.each_with_index.map { |k, i| "#{k} :_a#{i}" }.join(', ')}"
# convert any enumerables into sets
add_values = @additions.transform_values do |v|
if !v.is_a?(Set) && v.is_a?(Enumerable)
Set.new(v)
else
v
end
end
add_keys.each_with_index { |k, i| expression_attribute_values[":_a#{i}"] = add_values[k] }
update_expression
end

# adds all of the DELETE statements to the update_expression and returns it
def set_deletions(expression_attribute_values, update_expression)
return update_expression unless @deletions.present?

delete_keys = @deletions.keys
update_expression += " DELETE #{delete_keys.each_with_index.map { |k, i| "#{k} :_d#{i}" }.join(', ')}"
# values must be sets
delete_values = @deletions.transform_values do |v|
if v.is_a?(Set)
v
else
Set.new(v.is_a?(Enumerable) ? v : [v])
end
end
delete_keys.each_with_index { |k, i| expression_attribute_values[":_d#{i}"] = delete_values[k] }
update_expression
end

# adds all of the removals as a REMOVE clause
def set_removals(expression_attribute_names, update_expression)
return expression_attribute_names, update_expression unless @removals.present?

update_expression += " REMOVE #{@removals.each_with_index.map { |_k, i| "#_r#{i}" }.join(', ')}"
expression_attribute_names = expression_attribute_names.merge(
@removals.each_with_index.map { |k, i| ["#_r#{i}", k.to_s] }.to_h
)
[expression_attribute_names, update_expression]
end

end
end
end
45 changes: 45 additions & 0 deletions lib/dynamoid/transaction_write/update.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

require_relative 'base'
require 'dynamoid/persistence/update_validations'

module Dynamoid
class TransactionWrite
class Update < Base
def initialize(model, **options, &block)
super()

@model = model
@save_action = Save.new(model, **options, &block)
end

def on_registration
@save_action.on_registration
end

def on_commit
@save_action.on_commit
end

def on_rollback
@save_action.on_rollback
end

def aborted?
@save_action.aborted?
end

def skipped?
@save_action.skipped?
end

def observable_by_user_result
@save_action.observable_by_user_result
end

def action_request
@save_action.action_request
end
end
end
end
Loading