diff --git a/lib/couch_potato/database.rb b/lib/couch_potato/database.rb index 64dc75d..374695b 100644 --- a/lib/couch_potato/database.rb +++ b/lib/couch_potato/database.rb @@ -112,18 +112,22 @@ def first!(spec) # if passed a block will: # * yield the object to be saved to the block and run if once before saving # * on conflict: reload the document, run the block again and retry saving + # + # The block is run against a copy of the given + # document, leaving the original untouched while the block runs. Once saving + # has succeeded the resulting attributes and revision are copied back onto + # the original instance. def save_document(document, options = {}, retries = 0, &block) cache&.clear - begin - block&.call document - save_document_without_conflict_handling(document, options) - rescue CouchRest::Conflict - if block - handle_write_conflict document, options, retries, &block - else - raise CouchPotato::Conflict - end + if !block + return save_document_with_conflict_handling(document, options, retries, &block) end + + working_copy = document.dup + result = save_document_with_conflict_handling(working_copy, options, retries, &block) + document.attributes = working_copy.attributes + document._rev = working_copy._rev + result end alias save save_document @@ -150,7 +154,7 @@ def destroy_document(document) # could not be found these are omitted from the returned array def load_document(id) return load_documents(id) if id.is_a?(Array) - + cached = cache && cache[id] if cache if cache.key?(id) @@ -301,16 +305,21 @@ def view_cache_id(spec) spec.send(:klass).to_s + spec.view_name.to_s + spec.view_parameters.to_s end - def handle_write_conflict(document, options, retries, &block) - cache&.clear - if retries == 5 - raise CouchPotato::Conflict - else - reloaded = document.reload - document.attributes = reloaded.attributes - document._rev = reloaded._rev - save_document document, options, retries + 1, &block - end + # Saves the given document, running the block before each attempt. On a + # conflict the block is re-run against a freshly reloaded instance so that + # it can read the current state from the database and the save is retried. + # Without a block a conflict is surfaced as a CouchPotato::Conflict. + def save_document_with_conflict_handling(document, options, retries, &block) + block&.call document + save_document_without_conflict_handling document, options + rescue CouchRest::Conflict + raise CouchPotato::Conflict unless block + raise CouchPotato::Conflict if retries == 5 + + reloaded = document.reload + document.attributes = reloaded.attributes + document._rev = reloaded._rev + save_document_with_conflict_handling document, options, retries + 1, &block end def destroy_document_without_conflict_handling(document) diff --git a/spec/conflict_handling_spec.rb b/spec/conflict_handling_spec.rb index a640a9c..bbc010c 100644 --- a/spec/conflict_handling_spec.rb +++ b/spec/conflict_handling_spec.rb @@ -21,6 +21,19 @@ class Measurement expect(measurement.value).to eql(3) end + it 'does not modify the original instance before save but after it' do + measurement = Measurement.new value: 1 + db.save! measurement + + db.couchrest_database.save_doc measurement.reload._document.merge('value' => 2) + + db.save measurement do |m| + m.value = measurement.value + 3 # measurement.value is still 1 + end + + expect(measurement.value).to eql(4) # now it's updated + end + it 'raises an error after 5 tries' do couchrest_database = double(:couchrest_database, info: double.as_null_object) allow(couchrest_database).to receive(:save_doc).and_raise(CouchRest::Conflict)