diff --git a/src/collection.js b/src/collection.js index a836e0c04..5bc928165 100644 --- a/src/collection.js +++ b/src/collection.js @@ -727,10 +727,20 @@ export default class Collection { lastModified: this.lastModified, headers: {}, }, options); + + // Optionally ignore records when pulling for changes. + // (avoid redownloading our own changes on last step of #sync()) + let filters; + if (options.exclude) { + // Take the first 50 records to limit size of querystring (~2000 chars). + const exclude_id = options.exclude.slice(0, 50).map((r) => r.id).join(","); + filters = {exclude_id}; + } // First fetch remote changes from the server return this._apiCollection.listRecords({ since: options.lastModified || undefined, - headers: options.headers + headers: options.headers, + filters }) .then(({data, last_modified}) => { // last_modified is the ETag header value (string). @@ -745,7 +755,7 @@ export default class Collection { const localSynced = options.lastModified; const serverChanged = unquoted > options.lastModified; const emptyCollection = data.length === 0; - if (localSynced && serverChanged && emptyCollection) { + if (!options.exclude && localSynced && serverChanged && emptyCollection) { throw Error("Server has been flushed."); } @@ -820,25 +830,29 @@ export default class Collection { const conflicts = synced.conflicts.map((c) => { return {type: c.type, local: c.local.data, remote: c.remote}; }); + // Merge outgoing conflicts into sync result object + syncResultObject.add("conflicts", conflicts); + + // Reflect publication results locally using the response from + // the batch request. + // For created and updated records, the last_modified coming from server + // will be stored locally. const published = synced.published.map((c) => c.data); const skipped = synced.skipped.map((c) => c.data); - // Merge outgoing conflicts into sync result object - syncResultObject.add("conflicts", conflicts); - // Reflect publication results locally + // Records that must be deleted are either deletions that were pushed + // to server (published) or deleted records that were never pushed (skipped). const missingRemotely = skipped.map(r => Object.assign({}, r, {deleted: true})); const toApplyLocally = published.concat(missingRemotely); - // Deleted records are distributed accross local and missing records - // XXX: When tackling the issue to avoid downloading our own changes - // from the server. `toDeleteLocally` should be obtained from local db. - // See https://github.com/Kinto/kinto.js/issues/144 + const toDeleteLocally = toApplyLocally.filter((r) => r.deleted); const toUpdateLocally = toApplyLocally.filter((r) => !r.deleted); + // First, apply the decode transformers, if any return Promise.all(toUpdateLocally.map(record => { return this._decodeRecord("remote", record); })) - // Process everything within a single transaction + // Process everything within a single transaction. .then((results) => { return this.db.execute((transaction) => { const updated = results.map((record) => { @@ -965,7 +979,9 @@ export default class Collection { if (result.published.length === 0) { return result; } - return this.pullChanges(result, options); + // Avoid redownloading our own changes during the last pull. + const pullOpts = Object.assign({}, options, {exclude: result.published}); + return this.pullChanges(result, pullOpts); }); // Ensure API default remote is reverted if a custom one's been used return pFinally(syncPromise, () => this.api.remote = previousRemote); diff --git a/test/collection_test.js b/test/collection_test.js index 805c421b5..c2025a1d5 100644 --- a/test/collection_test.js +++ b/test/collection_test.js @@ -1190,6 +1190,7 @@ describe("Collection", () => { sinon.assert.calledOnce(listRecords); sinon.assert.calledWithExactly(listRecords, { since: undefined, + filters: undefined, headers: {} }); }); @@ -1201,6 +1202,20 @@ describe("Collection", () => { sinon.assert.calledOnce(listRecords); sinon.assert.calledWithExactly(listRecords, { since: 42, + filters: undefined, + headers: {} + }); + }); + }); + + it("should pass provided filters when polling changes from server", () => { + const exclude = [{id: 1}, {id: 2}, {id: 3}]; + return articles.pullChanges(result, {lastModified: 42, exclude}) + .then(_ => { + sinon.assert.calledOnce(listRecords); + sinon.assert.calledWithExactly(listRecords, { + since: 42, + filters: {exclude_id: "1,2,3"}, headers: {} }); }); @@ -1851,6 +1866,20 @@ describe("Collection", () => { }); }); + it("should not redownload pushed changes", () => { + const record1 = {id: uuid4(), title: "blog"}; + const record2 = {id: uuid4(), title: "post"}; + const syncResult = new SyncResultObject(); + syncResult.add("published", record1); + syncResult.add("published", record2); + sandbox.stub(articles, "pullChanges").returns(Promise.resolve(syncResult)); + sandbox.stub(articles, "pushChanges").returns(Promise.resolve(syncResult)); + return articles.sync().then(res => { + expect(res.published).to.have.length(2); + expect(articles.pullChanges.lastCall.args[1].exclude).eql([record1, record2]); + }); + }); + describe("Options", () => { let pullChanges;