Skip to content
Browse files

Properly handle identical re-creation of deleted document

If a document is deleted and then re-created with the same contents as its former first revision,
the new revision's revID is the same as the former one, causing a SQLite table constraint violation.
Fixed this by re-using the old row and setting its 'current' back to 1.
Fixes #205.
  • Loading branch information...
1 parent f56210b commit 1d95e46f27f76cfcd23134c71b7f822935810aad @snej snej committed
Showing with 61 additions and 36 deletions.
  1. +42 −32 Source/TD_Database+Insertion.m
  2. +18 −3 Source/TD_Database_Tests.m
  3. +1 −1 vendor/fmdb
View
74 Source/TD_Database+Insertion.m
@@ -288,8 +288,19 @@ - (TD_Revision*) putRevision: (TD_Revision*)rev
TD_Revision* winningRev = nil;
@try {
//// PART I: In which are performed lookups and validations prior to the insert...
-
+
+ // Get the doc's numeric ID (doc_id) and its current winning revision:
SInt64 docNumericID = docID ? [self getDocNumericID: docID] : 0;
+ BOOL oldWinnerWasDeletion = NO;
+ NSString* oldWinningRevID = nil;
+ BOOL makeOldWinnerNonCurrent = NO;
+ if (docNumericID > 0) {
+ // Look up which rev is the winner, before this insertion
+ //OPT: This rev ID could be cached in the 'docs' row
+ oldWinningRevID = [self winningRevIDOfDocNumericID: docNumericID
+ isDeleted: &oldWinnerWasDeletion];
+ }
+
SequenceNumber parentSequence = 0;
if (prevRevID) {
// Replacing: make sure given prevRevID is current & find its sequence number:
@@ -345,25 +356,13 @@ - (TD_Revision*) putRevision: (TD_Revision*)rev
return nil;
} else {
// Doc ID exists; check whether current winning revision is deleted:
- r = [_fmdb executeQuery: @"SELECT sequence, deleted FROM revs "
- "WHERE doc_id=? and current=1 ORDER BY revid DESC LIMIT 1",
- @(docNumericID)];
- if (!r)
+ if (oldWinnerWasDeletion) {
+ makeOldWinnerNonCurrent = YES;
+ } else if (oldWinningRevID) {
+ // The current winning revision is not deleted, so this is a conflict
+ *outStatus = kTDStatusConflict;
return nil;
- if ([r next]) {
- if ([r boolForColumnIndex: 1]) {
- // Make the deleted revision no longer current:
- if (![_fmdb executeUpdate: @"UPDATE revs SET current=0 WHERE sequence=?",
- @([r longLongIntForColumnIndex: 0])])
- return nil;
- } else if (!allowConflict) {
- // The current winning revision is not deleted, so this is a conflict
- *outStatus = kTDStatusConflict;
- return nil;
- }
}
- [r close];
- r = nil;
}
} else {
// Inserting first revision, with no docID given (POST): generate a unique docID:
@@ -374,13 +373,7 @@ - (TD_Revision*) putRevision: (TD_Revision*)rev
}
}
- // Look up which rev is the winner, before this insertion
- //OPT: This rev ID could be cached in the 'docs' row
- BOOL oldWinnerWasDeletion;
- NSString* oldWinningRevID = [self winningRevIDOfDocNumericID: docNumericID
- isDeleted: &oldWinnerWasDeletion];
-
- //// PART II: In which insertion occurs...
+ //// PART II: In which we prepare for insertion...
// Get the attachments:
NSDictionary* attachments = [self attachmentsFromRevision: rev status: &status];
@@ -411,29 +404,41 @@ - (TD_Revision*) putRevision: (TD_Revision*)rev
Assert(docID);
rev = [rev copyWithDocID: docID revID: newRevID];
- // Now insert the rev itself:
-
// Don't store a SQL null in the 'json' column -- I reserve it to mean that the revision data
// is missing due to compaction or replication.
// Instead, store an empty zero-length blob.
if (json == nil)
json = [NSData data];
+ //// PART III: In which the actual insertion finally takes place:
+
SequenceNumber sequence = [self insertRevision: rev
docNumericID: docNumericID
parentSequence: parentSequence
current: YES
JSON: json];
if (!sequence) {
- // The insert failed. If it was due to a constraint violation, that means an identical
- // revision already exists; so just return it.
- if (_fmdb.lastErrorCode == SQLITE_CONSTRAINT) {
+ // The insert failed. If it was due to a constraint violation, that means a revision
+ // already exists with identical contents and the same parent rev. We can ignore this
+ // insert call, then, _unless_ it has the effect of un-deleting the document. [#205]
+ if (_fmdb.lastErrorCode != SQLITE_CONSTRAINT)
+ return nil;
+ LogTo(TD_Database, @"Duplicate rev insertion: %@ / %@", docID, newRevID);
+ if (!oldWinnerWasDeletion || deleted) {
+ // no-op
*outStatus = kTDStatusOK;
rev.body = nil;
return rev;
- } else {
- return nil;
}
+ sequence = [_fmdb longLongForQuery: @"SELECT sequence FROM revs "
+ "WHERE doc_id=? and revid=?",
+ @(docNumericID), newRevID];
+ if (sequence <= 0)
+ return nil;
+ rev.sequence = sequence;
+ // Make the old revision current again:
+ if (![_fmdb executeUpdate: @"UPDATE revs SET current=1 WHERE sequence=?", @(sequence)])
+ return nil;
}
// Make replaced rev non-current:
@@ -442,6 +447,11 @@ - (TD_Revision*) putRevision: (TD_Revision*)rev
@(parentSequence)])
return nil;
}
+ if (makeOldWinnerNonCurrent) {
+ if (![_fmdb executeUpdate: @"UPDATE revs SET current=0 WHERE doc_id=? and revid=?",
+ @(docNumericID), oldWinningRevID])
+ return nil;
+ }
// Store any attachments:
status = [self processAttachments: attachments
View
21 Source/TD_Database_Tests.m
@@ -54,10 +54,12 @@
TD_Revision* rev = [[TD_Revision alloc] initWithProperties: props];
TDStatus status;
TD_Revision* result = [db putRevision: rev
- prevRevisionID: props[@"_rev"]
- allowConflict: NO
- status: &status];
+ prevRevisionID: props[@"_rev"]
+ allowConflict: NO
+ status: &status];
CAssert(status < 300);
+ CAssert(result.sequence > 0);
+ CAssert(result.revID != nil);
return result;
}
@@ -210,6 +212,19 @@
}
+TestCase(TD_Database_DeleteAndRecreate) {
+ // Test case for issue #205: Create a doc, delete it, create it again with the same content.
+ TD_Database* db = createDB();
+ TD_Revision* rev1 = putDoc(db, $dict({@"_id", @"dock"}, {@"property", @"value"}));
+ Log(@"Created: %@ -- %@", rev1, rev1.properties);
+ TD_Revision* rev2 = putDoc(db, $dict({@"_id", @"dock"}, {@"_rev", rev1.revID},
+ {@"_deleted", $true}));
+ Log(@"Deleted: %@ -- %@", rev2, rev2.properties);
+ TD_Revision* rev3 = putDoc(db, $dict({@"_id", @"dock"}, {@"property", @"value"}));
+ Log(@"Recreated: %@ -- %@", rev3, rev3.properties);
+}
+
+
TestCase(TD_Database_Validation) {
TD_Database* db = createDB();
__block BOOL validationCalled = NO;
2 vendor/fmdb
@@ -1 +1 @@
-Subproject commit 832c3436f8eef9089d2d3b16d96a5be267675a9f
+Subproject commit 00ac073c151131c2ea4a8f629608b2ed6f19817f

0 comments on commit 1d95e46

Please sign in to comment.
Something went wrong with that request. Please try again.