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

Support custom update expressions when update has no other attribute changes #138

Merged
merged 5 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
Unreleased Changes
------------------

* Feature - Allow custom `update_expression` to be passed through to the
underlying client calls. (#137)

2.12.0 (2023-09-28)
------------------

Expand Down
7 changes: 7 additions & 0 deletions lib/aws-record/record/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ class MissingRequiredConfiguration < RuntimeError; end
# the key existance check yourself in your condition expression if you
# wish to do so.
class TransactionalSaveConditionCollision < RuntimeError; end

# Raised when you attempt to combine your own update expression with
# the update expression auto-generated from updates to an item's
# attributes. The path forward until this case is supported is to
# perform attribute updates yourself in your update expression if you
# wish to do so.
class UpdateExpressionCollision < RuntimeError; end
end
end
end
74 changes: 39 additions & 35 deletions lib/aws-record/record/item_operations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,27 +269,22 @@ def _perform_save(opts)
)
end
else
update_opts = {
table_name: self.class.table_name,
key: key_values
}
update_pairs = _dirty_changes_for_update
update_tuple = self.class.send(
update_expression_opts = self.class.send(
:_build_update_expression,
update_pairs
)
if update_tuple
uex, exp_attr_names, exp_attr_values = update_tuple
update_opts = {
table_name: self.class.table_name,
key: key_values,
update_expression: uex,
expression_attribute_names: exp_attr_names
}
update_opts[:expression_attribute_values] = exp_attr_values unless exp_attr_values.empty?
else
update_opts = {
table_name: self.class.table_name,
key: key_values
}
end
dynamodb_client.update_item(opts.merge(update_opts))
opts = self.class.send(
:_merge_update_expression_opts,
update_expression_opts,
opts
)
resp = dynamodb_client.update_item(opts.merge(update_opts))
assign_attributes(resp[:attributes]) if resp[:attributes]
end
data = instance_variable_get('@data')
data.destroyed = false
Expand Down Expand Up @@ -581,19 +576,21 @@ def find_all(keys)
# Aws::DynamoDB::Client#update_item} call immediately on the table,
# using the attribute key/value pairs provided.
#
# @param [Hash] opts attribute-value pairs for the update operation you
# wish to perform. You must include all key attributes for a valid
# @param [Hash] new_params attribute-value pairs for the update operation
Copy link
Contributor

Choose a reason for hiding this comment

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

naming nit: maybe instead of new_params we could just use attributes?

Copy link
Contributor

Choose a reason for hiding this comment

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

new_params is what's used for all of the updates - I think it makes sense to stay consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately attributes would shadow the attributes method already used below:

attr_name = attributes.storage_name_for(attr_sym)
key[attr_name] = attributes.attribute_for(attr_sym).serialize(value)

I used new_params because it was consistent with the ItemOperations#update/ItemOperations#update! signatures also in this file. But maybe that's confusing since in this particular case it also must include the keys?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvm - I think new_params makes sense for consistency then.

# you wish to perform. You must include all key attributes for a valid
# call, then you may optionally include any other attributes that you
# wish to update.
# @param [Hash] opts Options to pass through to the
# {http://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/DynamoDB/Client.html#update_item-instance_method
# Aws::DynamoDB::Client#update_item} call.
#
# @raise [Aws::Record::Errors::KeyMissing] if your option parameters do
# not include all table keys.
def update(opts)
def update(new_params, opts = {})
key = {}
@keys.keys.each_value do |attr_sym|
unless (value = opts.delete(attr_sym))
raise Errors::KeyMissing, "Missing required key #{attr_sym} in #{opts}"

unless (value = new_params.delete(attr_sym))
raise Errors::KeyMissing, "Missing required key #{attr_sym} in #{new_params}"
end

attr_name = attributes.storage_name_for(attr_sym)
Expand All @@ -603,14 +600,9 @@ def update(opts)
table_name: table_name,
key: key
}
update_tuple = _build_update_expression(opts)
unless update_tuple.nil?
uex, exp_attr_names, exp_attr_values = update_tuple
update_opts[:update_expression] = uex
update_opts[:expression_attribute_names] = exp_attr_names
update_opts[:expression_attribute_values] = exp_attr_values unless exp_attr_values.empty?
end
dynamodb_client.update_item(update_opts)
update_expression_opts = _build_update_expression(new_params)
opts = _merge_update_expression_opts(update_expression_opts, opts)
dynamodb_client.update_item(opts.merge(update_opts))
end

private
Expand Down Expand Up @@ -641,10 +633,22 @@ def _build_update_expression(attr_value_pairs)
update_expressions = []
update_expressions << ("SET #{set_expressions.join(', ')}") unless set_expressions.empty?
update_expressions << ("REMOVE #{remove_expressions.join(', ')}") unless remove_expressions.empty?
if update_expressions.empty?
nil
else
[update_expressions.join(' '), exp_attr_names, exp_attr_values]
{
update_expression: update_expressions.join(' '),
expression_attribute_names: exp_attr_names,
expression_attribute_values: exp_attr_values
}.reject { |_, value| value.nil? || value.empty? }
end

def _merge_update_expression_opts(update_expression_opts, pass_through_opts)
update_expression_opts.merge(pass_through_opts) do |key, expression_value, pass_through_value|
case key
when :update_expression
msg = 'Using pass-through update expression with attribute updates is not currently supported.'
lsglick marked this conversation as resolved.
Show resolved Hide resolved
raise Errors::UpdateExpressionCollision, msg
else
expression_value.merge(pass_through_value)
end
end
end

Expand Down
22 changes: 7 additions & 15 deletions lib/aws-record/record/transactions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -284,25 +284,17 @@ def _transform_delete_record(delete_record, opts)
def _transform_update_record(update_record, opts)
# extract dirty attribute changes to perform an update
opts[:table_name] = update_record.class.table_name
opts[:key] = update_record.send(:key_values)
dirty_changes = update_record.send(:_dirty_changes_for_update)
update_tuple = update_record.class.send(
update_expression_opts = update_record.class.send(
:_build_update_expression,
dirty_changes
)
uex, exp_attr_names, exp_attr_values = update_tuple
opts[:key] = update_record.send(:key_values)
opts[:update_expression] = uex
# need to combine expression attribute names and values
opts[:expression_attribute_names] = if (names = opts[:expression_attribute_names])
exp_attr_names.merge(names)
else
exp_attr_names
end
opts[:expression_attribute_values] = if (values = opts[:expression_attribute_values])
exp_attr_values.merge(values)
else
exp_attr_values
end
opts = update_record.class.send(
:_merge_update_expression_opts,
update_expression_opts,
opts
)
{ update: opts }
end

Expand Down
97 changes: 97 additions & 0 deletions spec/aws-record/record/item_operations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,38 @@ module Record
)
end

it 'will call #update_item with pass through update expression for existing items' do
klass.configure_client(client: stub_client)
item = klass.new
item.id = 1
item.date = '2015-12-14'
item.body = 'Hello!'
item.clean! # I'm claiming that it is this way in the DB now.
item.save(
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: { '#S' => 'body' },
expression_attribute_values: { ':s' => 'Goodbye!' }
)
expect(api_requests).to eq(
[
{
table_name: 'TestTable',
key: {
'id' => { n: '1' },
'MyDate' => { s: '2015-12-14' }
},
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: {
'#S' => 'body'
},
expression_attribute_values: {
':s' => { s: 'Goodbye!' }
}
}
]
)
end

it 'passes through options to #update_item and #put_item' do
klass.configure_client(client: stub_client)
item = klass.new
Expand Down Expand Up @@ -290,6 +322,23 @@ module Record
expect(api_requests).to eq([])
end

it 'raises an exception when attribute updates collide with an update expression' do
klass.configure_client(client: stub_client)
item = klass.new
item.id = 1
item.date = '2015-12-14'
item.body = 'Hello!'
item.clean! # I'm claiming that it is this way in the DB now.
item.body = 'Goodbye!'
expect {
item.save(
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: { '#S' => 'body' },
expression_attribute_values: { ':s' => 'Goodbye!' }
)
}.to raise_error(Aws::Record::Errors::UpdateExpressionCollision)
end

context 'modifications to default values' do
let(:klass_with_defaults) do
Class.new do
Expand Down Expand Up @@ -464,6 +513,38 @@ module Record
)
end

it 'can find item and apply update if update expression provided' do
klass.configure_client(client: stub_client)
opts = {
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: {
'#S' => 'body'
},
expression_attribute_values: {
':s' => 'Content'
}
}
klass.update({ id: 1, date: '2016-05-18' }, opts)
expect(api_requests).to eq(
[
{
table_name: 'TestTable',
key: {
'id' => { n: '1' },
'MyDate' => { s: '2016-05-18' }
},
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: {
'#S' => 'body'
},
expression_attribute_values: {
':s' => { s: 'Content' }
}
}
]
)
end

it 'will recognize nil as a removal operation if nil not persisted' do
klass.configure_client(client: stub_client)
klass.update(id: 1, date: '2016-07-20', body: nil, persist_on_nil: nil)
Expand Down Expand Up @@ -531,6 +612,22 @@ module Record
Aws::Record::Errors::KeyMissing
)
end

it 'raises if both attribute updates and update expression provided' do
klass.configure_client(client: stub_client)
opts = {
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: {
'#S' => 'body'
},
expression_attribute_values: {
':s' => 'Content'
}
}
expect { klass.update({ id: 1, date: '2016-05-18', bool: false }, opts) }.to raise_error(
Aws::Record::Errors::UpdateExpressionCollision
)
end
end

describe '#delete!' do
Expand Down
64 changes: 64 additions & 0 deletions spec/aws-record/record/transactions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,45 @@ module Record
)
end

it 'supports custom update expressions' do
Aws::Record::Transactions.configure_client(client: stub_client)
update_item = table_two.new(uuid: 'foo')
update_item.clean! # like we got it from #find
Aws::Record::Transactions.transact_write(
transact_items: [
{
update: update_item,
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: {
'#S' => 'body'
},
expression_attribute_values: {
':s' => 'Content'
}
}
]
)
expect(stub_client.api_requests.size).to eq(1)
request_params = stub_client.api_requests.first[:params]
expect(request_params[:transact_items]).to eq(
[
{
update: {
table_name: 'TableTwo',
key: { 'uuid' => { s: 'foo' } },
update_expression: 'SET #S = if_not_exists(#S, :s)',
expression_attribute_names: {
'#S' => 'body'
},
expression_attribute_values: {
':s' => { s: 'Content' }
}
}
}
]
)
end

it 'raises a validation exception when safe put collides with a condition expression' do
Aws::Record::Transactions.configure_client(client: stub_client)
save_item = table_two.new(uuid: 'bar', body: 'Content')
Expand All @@ -458,6 +497,31 @@ module Record
expect(save_item.dirty?).to be_truthy
end

it 'raises an exception when attribute updates collide with an update expression' do
Aws::Record::Transactions.configure_client(client: stub_client)
update_item = table_two.new(uuid: 'foo')
update_item.clean! # like we got it from #find
update_item.body = 'Content'
expect {
Aws::Record::Transactions.transact_write(
transact_items: [
{
update: update_item,
update_expression: 'SET #H = :v',
expression_attribute_names: {
'#H' => 'has_default'
},
expression_attribute_values: {
':v' => 'other'
}
}
]
)
}.to raise_error(
Aws::Record::Errors::UpdateExpressionCollision
)
end

it 'does not clean items when the transaction fails' do
stub_client.stub_responses(
:transact_write_items,
Expand Down
Loading