-
Notifications
You must be signed in to change notification settings - Fork 17
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
add ActiveModel::Datastore.save_all feature #27
base: main
Are you sure you want to change the base?
Changes from all commits
c642b86
0e06ae2
ac3f0fb
eeebc06
04f5bdf
65afb12
050bfc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,6 +161,18 @@ class User | |
end | ||
``` | ||
|
||
To improve performance, you can save many entities at once. (Batch operation) | ||
|
||
```ruby | ||
group = Group.find(params[:group_id]) | ||
users = %w{ alice bob charlie }.map{ |name| User.new(name: name) } | ||
if ActiveModel::Datastore.save_all(users, parent: group) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe passing in the group model object to |
||
format.html { redirect_to action: :index, notice: "#{users.count} users were successfully created.' } | ||
else | ||
format.html { render :new } | ||
end | ||
``` | ||
|
||
## <a name="controller"></a>Controller Example | ||
|
||
Now on to the controller! A scaffold generated controller works out of the box: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
## | ||
# Batch operations | ||
# | ||
# | ||
# Such batch calls are faster than making separate calls for each individual entity because they | ||
# incur the overhead for only one service call. | ||
# | ||
# reference: https://cloud.google.com/datastore/docs/concepts/entities#batch_operations | ||
# | ||
# group = Group.find(params[:group_id]) | ||
# users = %w{ alice bob charlie }.map{ |name| User.new(name: name) } | ||
# saved_users = ActiveModel::Datastore.save_all(users, parent: group) | ||
# | ||
module ActiveModel::Datastore | ||
def self.save_all(entries, parent: nil) | ||
return if entries.reject(&:valid?).present? | ||
entries.each_slice(500).map do |sliced_entries| | ||
entities = [] | ||
results = nil | ||
fn = lambda do |n| | ||
entry = sliced_entries[n] | ||
entry.run_callbacks(:save) do | ||
entities << entry.build_entity(parent) | ||
if n + 1 < sliced_entries.count | ||
# recursive call | ||
fn.call(n + 1) | ||
else | ||
# batch insert | ||
results = entry.class.retry_on_exception? { CloudDatastore.dataset.save entities } | ||
end | ||
sliced_entries[n].fill_id_from_entity(results&.fetch(n)) | ||
results.present? | ||
end | ||
end | ||
fn.call(0) | ||
sliced_entries | ||
end.flatten | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would we expect this method to return? It is currently returning an array of model objects. Should it return an array of Google::Cloud::Datastore::Entity instead? Also, is the |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
require 'test_helper' | ||
|
||
class ActiveModel::BatchOperationTest < ActiveSupport::TestCase | ||
def setup | ||
super | ||
@mock_models = %w[alice bob charlie].map do |name| | ||
MockModel.new(name: name, parent_key_id: MOCK_PARENT_ID) | ||
end | ||
end | ||
|
||
test 'batch save' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also add a test where the |
||
count = MockModel.count_test_entities | ||
assert ActiveModel::Datastore.save_all(@mock_models) | ||
assert_equal count + @mock_models.count, MockModel.count_test_entities | ||
@mock_models.each do |mock_model| | ||
assert_not_nil mock_model.id | ||
key = CloudDatastore.dataset.key 'MockModel', mock_model.id | ||
key.parent = CloudDatastore.dataset.key('ParentMockModel', MOCK_PARENT_ID) | ||
entity = CloudDatastore.dataset.find key | ||
assert_equal mock_model.id, entity.key.id | ||
assert_equal 'MockModel', entity.key.kind | ||
assert_equal 'ParentMockModel', entity.key.parent.kind | ||
assert_equal MOCK_PARENT_ID, entity.key.parent.id | ||
end | ||
end | ||
|
||
test 'before validation callback on batch save' do | ||
@mock_models.each do |mock_model| | ||
class << mock_model | ||
before_validation { self.name = nil } | ||
end | ||
end | ||
refute ActiveModel::Datastore.save_all(@mock_models) | ||
@mock_models.each do |mock_model| | ||
assert_nil mock_model.name | ||
end | ||
assert_equal 0, MockModel.count_test_entities | ||
end | ||
|
||
test 'after validation callback on batch save' do | ||
@mock_models.each do |mock_model| | ||
class << mock_model | ||
after_validation { self.name = nil } | ||
end | ||
end | ||
assert ActiveModel::Datastore.save_all(@mock_models) | ||
@mock_models.each do |mock_model| | ||
assert_nil mock_model.name | ||
end | ||
assert_equal @mock_models.count, MockModel.count_test_entities | ||
end | ||
|
||
test 'before save callback on batch save' do | ||
@mock_models.each do |mock_model| | ||
class << mock_model | ||
before_save { self.name = name.upcase } | ||
end | ||
end | ||
assert ActiveModel::Datastore.save_all(@mock_models) | ||
@mock_models.each_with_index do |mock_model, index| | ||
assert_equal mock_model.name, %w[alice bob charlie][index].upcase | ||
assert_equal MockModel.all[index].name, %w[alice bob charlie][index].upcase | ||
end | ||
end | ||
|
||
test 'after save callback on batch save' do | ||
@mock_models.each do |mock_model| | ||
class << mock_model | ||
after_save { self.name = name.upcase } | ||
end | ||
end | ||
assert ActiveModel::Datastore.save_all(@mock_models) | ||
@mock_models.each_with_index do |mock_model, index| | ||
assert_equal mock_model.name, %w[alice bob charlie][index].upcase | ||
assert_equal MockModel.all[index].name, %w[alice bob charlie][index] | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this position in the README we have not yet mentioned the concept of parent/ancestor. Let's move this new section down after Datastore Consistency.