Fix pulling documents with sub attachments failure

A few related issues have been fixed:
1. Correct minRevPos value when stripping the (non modified) attachments. This corrects the performance optimization when pushing documents that have non-modified attachments.
2. Applying a patch from @snej that "if the attachment is a stub, and the parent revision's JSON isn't known, but the attachment exists locally (based on its digest), just leave the stub alone".
3. Fix the issue that atts_since parameter has never been sent.

#672

Source/CBLBulkDownloader.m

@@ -48,10 +48,10 @@ - (instancetype) initWithDbURL: (NSURL*)dbURL
 {
     // Build up a JSON body describing what revisions we want:
     NSArray* keys = [revs my_map: ^(CBL_Revision* rev) {
-        NSArray* attsSince = [_db.storage getPossibleAncestorRevisionIDs: rev
+        NSArray* attsSince = [database.storage getPossibleAncestorRevisionIDs: rev
                                                            limit: kMaxNumberOfAttsSince
                                                  onlyAttachments: YES];
-        if (!attsSince.count == 0)
+        if (attsSince.count == 0)
             attsSince = nil;
         return $dict({@"id", rev.docID},
                      {@"rev", rev.revID},

Source/CBLDatabase+Attachments.m

@@ -279,7 +279,7 @@ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev
     unsigned generation = [CBL_Revision generationFromRevID: prevRevID] + 1;
     __block NSDictionary* parentAttachments = nil;
 
-    return [rev mutateAttachments: ^NSDictionary *(NSString *name, NSDictionary *attachInfo) {
+    [rev mutateAttachments: ^NSDictionary *(NSString *name, NSDictionary *attachInfo) {
         CBL_Attachment* attachment = [[CBL_Attachment alloc] initWithName: name
                                                                      info: attachInfo
                                                                    status: outStatus];
@@ -303,11 +303,20 @@ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev
         } else if ([attachInfo[@"stub"] isEqual: $true]) {
             // "stub" on an incoming revision means the attachment is the same as in the parent.
             if (!parentAttachments && prevRevID) {
+                CBLStatus status;
                 parentAttachments = [self attachmentsForDocID: rev.docID revID: prevRevID
-                                                       status: outStatus];
+                                                       status: &status];
                 if (!parentAttachments) {
-                    if (*outStatus == kCBLStatusOK || *outStatus == kCBLStatusNotFound)
-                        *outStatus = kCBLStatusBadAttachment;
+                    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 == kCBLStatusOK || status == kCBLStatusNotFound)
+                        status = kCBLStatusBadAttachment;
+                    *outStatus = status;
                     return nil;
                 }
             }
@@ -329,6 +338,8 @@ - (BOOL) processAttachmentsForRevision: (CBL_MutableRevision*)rev
         Assert(attachment.isValid);
         return attachment.asStubDictionary;
     }];
+
+    return !CBLStatusIsError(*outStatus);
 }
 
 

Source/CBLDatabase+Insertion.m

@@ -229,7 +229,7 @@ - (CBLStatus) forceInsert: (CBL_Revision*)inRev
         if (![self processAttachmentsForRevision: updatedRev
                                        prevRevID: prevRevID
                                           status: &status])
-        return status;
+            return status;
         inRev = updatedRev;
     }
 

Source/CBL_Pusher.m

@@ -293,7 +293,7 @@ - (void) processInbox: (CBL_RevisionList*)changes {
                             // Look for the latest common ancestor and stub out older attachments:
                             int minRevPos = CBLFindCommonAncestor(populatedRev, possibleAncestors);
                             if (![db expandAttachmentsIn: populatedRev
-                                             minRevPos: minRevPos
+                                               minRevPos: minRevPos + 1
                                             allowFollows: !_dontSendMultipart
                                                   decode: NO
                                                   status: &status]) {

Source/CBL_SQLiteStorage.m

@@ -614,9 +614,12 @@ - (CBLStatus) loadRevisionBody: (CBL_MutableRevision*)rev {
     CBLStatus status = kCBLStatusNotFound;
     if ([r next]) {
         // Found the rev. But the JSON still might be null if the database has been compacted.
-        status = kCBLStatusOK;
-        rev.sequence = [r longLongIntForColumnIndex: 0];
-        rev.asJSON = [r dataNoCopyForColumnIndex: 1];
+        NSData* json = [r dataNoCopyForColumnIndex: 1];
+        if (json) {
+            status = kCBLStatusOK;
+            rev.sequence = [r longLongIntForColumnIndex: 0];
+            rev.asJSON = json;
+        }
     }
     [r close];
     return status;

Unit-Tests/Replication_Tests.m

@@ -777,4 +777,131 @@ - (void) test13_StopIdlePullReplication {
     Assert(stopped);
 }
 
+- (void) test14_PullDocWithStubAttachment {
+    NSURL* remoteDbURL = [self remoteTestDBURL: kPushThenPullDBName];
+    if (!remoteDbURL)
+        return;
+    [self eraseRemoteDB: remoteDbURL];
+
+    NSMutableDictionary* properties;
+    CBLUnsavedRevision* newRev;
+
+    NSError* error;
+    CBLDatabase* pushDB = [dbmgr createEmptyDatabaseNamed: @"prepopdb" error: &error];
+
+    // Create a document:
+    CBLDocument* doc = [pushDB documentWithID: @"mydoc"];
+    CBLSavedRevision* rev1 = [doc putProperties: @{@"foo": @"bar"} error: &error];
+    Assert(rev1);
+
+    // Attach an attachment:
+    NSUInteger size = 50 * 1024;
+    unsigned char attachbytes[size];
+    for (NSUInteger i = 0; i < size; i++) {
+        attachbytes[i] = 1;
+    }
+    NSData* attachment = [NSData dataWithBytes: attachbytes length: size];
+    newRev = [doc newRevision];
+    [newRev setAttachmentNamed: @"myattachment"
+               withContentType: @"text/plain; charset=utf-8"
+                       content: attachment];
+    CBLSavedRevision* rev2 = [newRev save: &error];
+    Assert(rev2);
+
+    // Push:
+    CBLReplication* pusher = [pushDB createPushReplication: remoteDbURL];
+    [self runReplication:pusher expectedChangesCount:1];
+
+    // Pull (The db now has a base doc with an attachment.):
+    CBLReplication* puller = [db createPullReplication: remoteDbURL];
+    [self runReplication: puller expectedChangesCount: 1];
+
+    // Create a new revision and push:
+    properties = doc.userProperties.mutableCopy;
+    properties[@"tag"] = @3;
+
+    newRev = [rev2 createRevision];
+    newRev.userProperties = properties;
+    CBLSavedRevision* rev3 = [newRev save: &error];
+    Assert(rev3);
+
+    pusher = [pushDB createPushReplication: remoteDbURL];
+    [self runReplication: pusher expectedChangesCount: 1];
+
+    // Create another revision and push:
+    properties = doc.userProperties.mutableCopy;
+    properties[@"tag"] = @4;
+
+    newRev = [rev3 createRevision];
+    newRev.userProperties = properties;
+    CBLSavedRevision* rev4 = [newRev save: &error];
+    Assert(rev4);
+
+    pusher = [pushDB createPushReplication: remoteDbURL];
+    [self runReplication: pusher expectedChangesCount: 1];
+
+    // Pull without any errors:
+    puller = [db createPullReplication: remoteDbURL];
+    [self runReplication: puller expectedChangesCount: 1];
+
+    Assert([pushDB deleteDatabase: &error], @"Couldn't delete db: %@", error);
+}
+
+- (void) test15_PushShouldNotSendNonModifiedAttachment {
+    NSURL* remoteDbURL = [self remoteTestDBURL: kPushThenPullDBName];
+    if (!remoteDbURL)
+        return;
+    [self eraseRemoteDB: remoteDbURL];
+
+    CBLUnsavedRevision* newRev;
+    NSError* error;
+
+    // Create a document:
+    CBLDocument* doc = [db documentWithID: @"mydoc"];
+    CBLSavedRevision* rev1 = [doc putProperties: @{@"foo": @"bar"} error: &error];
+    Assert(rev1);
+
+    // Attach an attachment:
+    NSUInteger size = 50 * 1024;
+    unsigned char attachbytes[size];
+    for (NSUInteger i = 0; i < size; i++) {
+        attachbytes[i] = 1;
+    }
+    NSData* attachment = [NSData dataWithBytes: attachbytes length: size];
+    newRev = [doc newRevision];
+    [newRev setAttachmentNamed: @"myattachment"
+               withContentType: @"text/plain; charset=utf-8"
+                       content: attachment];
+    CBLSavedRevision* rev2 = [newRev save: &error];
+    Assert(rev2);
+
+    // Push:
+    CBLReplication* pusher = [db createPushReplication: remoteDbURL];
+    [self runReplication:pusher expectedChangesCount:1];
+
+    NSMutableDictionary* properties = doc.userProperties.mutableCopy;
+    properties[@"tag"] = @3;
+
+    newRev = [rev2 createRevision];
+    newRev.userProperties = properties;
+    CBLSavedRevision* rev3 = [newRev save: &error];
+    Assert(rev3);
+
+    pusher = [db createPushReplication: remoteDbURL];
+    [self runReplication: pusher expectedChangesCount: 1];
+
+    // Implicitly verify the result by checking the revpos of the document on the Sync Gateway.
+    NSURL* allDocsURL = [remoteDbURL URLByAppendingPathComponent: @"mydoc"];
+    NSData* data = [NSData dataWithContentsOfURL: allDocsURL];
+    Assert(data);
+    NSDictionary* response = [CBLJSON JSONObjectWithData: data options: 0 error: NULL];
+    NSDictionary* attachments = response[@"_attachments"];
+    Assert(attachments);
+    NSDictionary* myAttachment = attachments[@"myattachment"];
+    Assert(myAttachment);
+    Assert(myAttachment[@"revpos"]);
+    int revpos = [myAttachment[@"revpos"] intValue];
+    AssertEq(revpos, 2);
+}
+
 @end