Skip to content
This repository has been archived by the owner on Mar 11, 2022. It is now read-only.

Avoid assertion in allDocsQuery:options: when a docId isn't found. #207

Merged
merged 2 commits into from
Nov 2, 2015

Conversation

sdrew
Copy link
Contributor

@sdrew sdrew commented Oct 14, 2015

When using allDocsQuery:options: with an array of docIds, a missing document will cause an 'Invalid parameter not satisfying: properties' assertion in [TD_Body initWithProperties:].

When using `allDocsQuery:options:` with an array of docIds, a missing document will cause an 'Invalid parameter not satisfying: properties' assertion in `[TD_Body initWithProperties:]`.
@mikerhodes
Copy link
Member

Could you add a failing test so I can be sure I'm properly understanding the issue at hand?

Also, you'll need to complete and send in a CLA before I can accept this. The process is reasonably painless and can be done electronically, see https://github.com/cloudant/CDTDatastore/blob/master/CONTRIBUTING.md#contributor-license-agreement.

@sdrew
Copy link
Contributor Author

sdrew commented Oct 19, 2015

Apologies. I should have checked the requirements beforehand. I've added the tests as requested and have signed the CLA, which I will be emailing shortly.

The first test is in CDTDatastoreQueryTests.m under can find documents and ignore non-existent documents. It uses [CDTDatastore find:] with the following query:

@{ @"_id" : @{ @"$in" : @[@"mike12", @"mike34", @"mike72", @"mike-not-found"] } }

This method works as expected, fetching 3 documents and ignoring the non-existent document. It doesn't require the change to CDTDatastore in this pull request.

The other test is in DatastoreCRUD.m under testGetDocumentsWithIds_NonExistentDocument. It attempts to use [CDTDatastore getDocumentsWithIds:] with 4 existing docIds and a non-existent one.

@[docIds[5], docIds[7], docIds[12], docIds[170], @"i_do_not_exist"];

Without the change in this pull-request, this test will fail with an assertion in the [TD_Body initWithProperties:] call at line 364 of CDTDatastore.m. This is because [TD_Database getDocsWithIds:options:] will return a dictionary with the following format when the docId doesn't exist:

@{ @"key": docID, @"error": @"not_found" }

To avoid any unintended side-effects, this pull request only checks for the specific case of docId and revId being nil, as well as row containing an @"error" key with the value @"not_found". Once applied, [CDTDatastore getDocumentsWithIds:] will ignore non-existent docIds and return only the found documents, instead of throwing an assertion.

@rhyshort
Copy link
Member

@sdrew thanks for adding those tests, however we have yet to receive your signed CLA.

@sdrew
Copy link
Contributor Author

sdrew commented Oct 27, 2015

@rhyshort @mikerhodes It was sent on October 19th to github@cloudant.com, the email address listed for the organization.

Is there another email I should send it to?

@rhyshort
Copy link
Member

@sdrew it should have been sent to mike.rhodes@uk.ibm.com as per the instructions in the CLA text.

@rhyshort
Copy link
Member

rhyshort commented Nov 2, 2015

@sdrew We have your signed CLA, thanks for that :)

@rhyshort
Copy link
Member

rhyshort commented Nov 2, 2015

Looks good, merging

rhyshort added a commit that referenced this pull request Nov 2, 2015
Avoid assertion in allDocsQuery:options: when a docId isn't found.
@rhyshort rhyshort merged commit e23eb9f into cloudant:master Nov 2, 2015
@rhyshort
Copy link
Member

rhyshort commented Nov 3, 2015

@sdrew This has been released as part of 0.19.2, thanks for the hard work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants