From c866583bcaae8ea70c9dd6260b2daeac317facdf Mon Sep 17 00:00:00 2001 From: Jens Alfke Date: Thu, 2 Jul 2015 14:44:08 -0700 Subject: [PATCH] Fixed error pulling doc with unchanged attachments from CouchDB Nasty edge case where the attachment is unchanged, but the parent rev isn't available (doc was changed multiple times remotely), _and_ the attachment metadata doesn't contain a usable digest (CouchDB's digests are MD5 not SHA-1.) Fixes #802 --- Source/CBLDatabase+Attachments.h | 2 +- Source/CBLDatabase+Attachments.m | 44 ++++++++++++++++++++++----- Source/CBLDatabase+Insertion.m | 6 ++-- Unit-Tests/DatabaseAttachment_Tests.m | 35 +++++++++++++++++++++ 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/Source/CBLDatabase+Attachments.h b/Source/CBLDatabase+Attachments.h index 63c481564..29c5b1b1e 100644 --- a/Source/CBLDatabase+Attachments.h +++ b/Source/CBLDatabase+Attachments.h @@ -30,7 +30,7 @@ typedef enum { /** Scans the rev's _attachments dictionary, adding inline attachment data to the blob-store and turning all the attachments into stubs. */ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev - prevRevID: (NSString*)prevRevID + ancestry: (NSArray*)ancestry // 1st item is parent revID, etc. status: (CBLStatus*)outStatus; /** Modifies a CBL_Revision's _attachments dictionary by adding the "data" property to all diff --git a/Source/CBLDatabase+Attachments.m b/Source/CBLDatabase+Attachments.m index d071afbb5..da4444c07 100644 --- a/Source/CBLDatabase+Attachments.m +++ b/Source/CBLDatabase+Attachments.m @@ -260,7 +260,7 @@ - (BOOL) expandAttachmentsIn: (CBL_MutableRevision*)rev /** Given a revision, updates its _attachments dictionary for storage in the database. */ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev - prevRevID: (NSString*)prevRevID + ancestry: (NSArray*)ancestry status: (CBLStatus*)outStatus { *outStatus = kCBLStatusOK; @@ -276,6 +276,7 @@ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev return YES; } + NSString *prevRevID = (ancestry.count > 0) ? ancestry[0] : nil; unsigned generation = [CBL_Revision generationFromRevID: prevRevID] + 1; __block NSDictionary* parentAttachments = nil; @@ -307,12 +308,20 @@ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev parentAttachments = [self attachmentsForDocID: rev.docID revID: prevRevID status: &status]; if (!parentAttachments) { - if (status == kCBLStatusNotFound - && [_attachments hasBlobForKey: attachment.blobKey]) { - // Parent revision's body isn't known (we are probably pulling a rev along - // with its entire history) but it's OK, we have the attachment already - *outStatus = kCBLStatusOK; - return attachInfo; + if (status == kCBLStatusNotFound) { + if ([_attachments hasBlobForKey: attachment.blobKey]) { + // Parent revision's body isn't known (we are probably pulling a rev along + // with its entire history) but it's OK, we have the attachment already + *outStatus = kCBLStatusOK; + return attachInfo; + } + // If parent rev isn't available, look farther back in ancestry: + NSDictionary* ancestorAttachment = [self findAttachment: name + revpos: attachment->revpos + docID: rev.docID + ancestry: ancestry]; + if (ancestorAttachment) + return ancestorAttachment; } if (status == kCBLStatusOK || status == kCBLStatusNotFound) status = kCBLStatusBadAttachment; @@ -343,6 +352,27 @@ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev } +// Looks for an attachment with the given revpos in the document's ancestry. +- (NSDictionary*) findAttachment: (NSString*)name + revpos: (unsigned)revpos + docID: (NSString*)docID + ancestry: (NSArray*)ancestry +{ + for (NSInteger i = ancestry.count - 1; i >= 0; i--) { + NSString* revID = ancestry[i]; + if ([CBL_Revision generationFromRevID: revID] >= revpos) { + CBLStatus status; + NSDictionary* attachments = [self attachmentsForDocID: docID revID: revID + status: &status]; + NSDictionary* attachment = attachments[name]; + if (attachment) + return attachment; + } + } + return nil; +} + + #pragma mark - MISC.: diff --git a/Source/CBLDatabase+Insertion.m b/Source/CBLDatabase+Insertion.m index 2e84eaf89..014a49b9d 100644 --- a/Source/CBLDatabase+Insertion.m +++ b/Source/CBLDatabase+Insertion.m @@ -181,7 +181,7 @@ - (CBL_Revision*) putDocID: (NSString*)inDocID deleted: deleting]; tmpRev.properties = properties; if (![self processAttachmentsForRevision: tmpRev - prevRevID: prevRevID + ancestry: (prevRevID ? @[prevRevID] : nil) status: outStatus]) { CBLStatusToOutNSError(*outStatus, outError); return nil; @@ -238,9 +238,9 @@ - (CBLStatus) forceInsert: (CBL_Revision*)inRev CBLStatus status; if (inRev.attachments) { CBL_MutableRevision* updatedRev = [inRev mutableCopy]; - NSString* prevRevID = history.count >= 2 ? history[1] : nil; + NSArray* ancestry = [history subarrayWithRange: NSMakeRange(1, history.count-1)]; if (![self processAttachmentsForRevision: updatedRev - prevRevID: prevRevID + ancestry: ancestry status: &status]) { CBLStatusToOutNSError(status, outError); return status; diff --git a/Unit-Tests/DatabaseAttachment_Tests.m b/Unit-Tests/DatabaseAttachment_Tests.m index 12f4de462..96c270c8a 100644 --- a/Unit-Tests/DatabaseAttachment_Tests.m +++ b/Unit-Tests/DatabaseAttachment_Tests.m @@ -483,6 +483,41 @@ - (void) test14_FollowingAttachments { {@"revpos", @1})})); } +// Test for inserting a revision with attachments, when the parent of the new revision isn't local +// and there are attachments that are unchanged in the revision. See issue #802. +- (void) test15_MissingIntermediateRevs { + // Put a revision that includes an _attachments dict: + NSData* attach1 = [@"This is the body of attach1" dataUsingEncoding: NSUTF8StringEncoding]; + NSString* base64 = [CBLBase64 encode: attach1]; + NSDictionary* attachmentDict = $dict({@"attach", $dict({@"content_type", @"text/plain"}, + {@"data", base64})}); + NSDictionary* props = $dict({@"_id", @"X"}, + {@"_attachments", attachmentDict}); + CBL_Revision* rev1; + CBLStatus status; + NSError* error; + rev1 = [db putRevision: [CBL_MutableRevision revisionWithProperties: props] + prevRevisionID: nil allowConflict: NO status: &status error: &error]; + AssertEq(status, kCBLStatusCreated); + AssertNil(error); + AssertEqual(rev1[@"_attachments"][@"attach"][@"revpos"], @1); + + // Insert a revision several generations advanced but which hasn't changed the attachment: + CBL_MutableRevision* rev4 = [rev1 mutableCopy]; + rev4[@"_rev"] = @"4-4444"; + rev4[@"foo"] = @"bar"; + [rev4 mutateAttachments: ^NSDictionary *(NSString *name, NSDictionary *att) { + NSMutableDictionary* nuAtt = [att mutableCopy]; + [nuAtt removeObjectForKey: @"data"]; + nuAtt[@"stub"] = @YES; + nuAtt[@"digest"] = @"md5-deadbeef"; // CouchDB adds MD5 digests! + return nuAtt; + }]; + NSArray* history = @[@"4-4444", @"3-3333", @"2-2222", rev1.revID]; + status = [db forceInsert: rev4 revisionHistory: history source: nil error: &error]; + AssertEq(status, 200); +} + - (void) test16_FollowWithRevpos { NSDictionary* attachInfo = $dict({@"content_type", @"text/plain"},