From daf4869f3cebd842da7f2905a39f261627de523d Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Mon, 30 Dec 2019 13:57:22 -0600 Subject: [PATCH 1/2] Remove batchSize --- src/ParseObject.js | 88 ++++++---------- src/RESTController.js | 1 - src/__tests__/ParseObject-test.js | 160 ++---------------------------- 3 files changed, 37 insertions(+), 212 deletions(-) diff --git a/src/ParseObject.js b/src/ParseObject.js index e2d6b57a0..379c13dc8 100644 --- a/src/ParseObject.js +++ b/src/ParseObject.js @@ -19,7 +19,7 @@ import ParseACL from './ParseACL'; import parseDate from './parseDate'; import ParseError from './ParseError'; import ParseFile from './ParseFile'; -import { when, continueWhile } from './promiseUtils'; +import { when, continueWhile, resolvingPromise } from './promiseUtils'; import { DEFAULT_PIN, PIN_PREFIX } from './LocalDatastoreUtils'; import { @@ -59,8 +59,6 @@ type SaveOptions = FullOptions & { cascadeSave?: boolean } -const DEFAULT_BATCH_SIZE = 20; - // Mapping of class names to constructors, so we can populate objects from the // server with appropriate subclasses of ParseObject const classMap = {}; @@ -1608,7 +1606,6 @@ class ParseObject { * be used for this request. *
  • sessionToken: A valid session token, used for making a request on * behalf of a specific user. - *
  • batchSize: Number of objects to process per request * * @return {Promise} A promise that is fulfilled when the destroyAll * completes. @@ -1621,9 +1618,6 @@ class ParseObject { if (options.hasOwnProperty('sessionToken')) { destroyOptions.sessionToken = options.sessionToken; } - if (options.hasOwnProperty('batchSize') && typeof options.batchSize === 'number') { - destroyOptions.batchSize = options.batchSize; - } return CoreManager.getObjectController().destroy( list, destroyOptions @@ -1651,7 +1645,6 @@ class ParseObject { * be used for this request. *
  • sessionToken: A valid session token, used for making a request on * behalf of a specific user. - *
  • batchSize: Number of objects to process per request * */ static saveAll(list: Array, options: RequestOptions = {}) { @@ -1662,9 +1655,6 @@ class ParseObject { if (options.hasOwnProperty('sessionToken')) { saveOptions.sessionToken = options.sessionToken; } - if (options.hasOwnProperty('batchSize') && typeof options.batchSize === 'number') { - saveOptions.batchSize = options.batchSize; - } return CoreManager.getObjectController().save( list, saveOptions @@ -2141,7 +2131,6 @@ const DefaultController = { }, async destroy(target: ParseObject | Array, options: RequestOptions): Promise | ParseObject> { - const batchSize = (options && options.batchSize) ? options.batchSize : DEFAULT_BATCH_SIZE; const localDatastore = CoreManager.getLocalDatastore(); const RESTController = CoreManager.getRESTController(); @@ -2149,45 +2138,33 @@ const DefaultController = { if (target.length < 1) { return Promise.resolve([]); } - const batches = [[]]; + const objects = []; target.forEach((obj) => { if (!obj.id) { return; } - batches[batches.length - 1].push(obj); - if (batches[batches.length - 1].length >= batchSize) { - batches.push([]); - } + objects.push(obj); }); - if (batches[batches.length - 1].length === 0) { - // If the last batch is empty, remove it - batches.pop(); - } - let deleteCompleted = Promise.resolve(); const errors = []; - batches.forEach((batch) => { - deleteCompleted = deleteCompleted.then(() => { - return RESTController.request('POST', 'batch', { - requests: batch.map((obj) => { - return { - method: 'DELETE', - path: getServerUrlPath() + 'classes/' + obj.className + '/' + obj._getId(), - body: {} - }; - }) - }, options).then((results) => { - for (let i = 0; i < results.length; i++) { - if (results[i] && results[i].hasOwnProperty('error')) { - const err = new ParseError( - results[i].error.code, - results[i].error.error - ); - err.object = batch[i]; - errors.push(err); - } - } - }); - }); + const deleteCompleted = RESTController.request('POST', 'batch', { + requests: objects.map((obj) => { + return { + method: 'DELETE', + path: getServerUrlPath() + 'classes/' + obj.className + '/' + obj._getId(), + body: {} + }; + }) + }, options).then((results) => { + for (let i = 0; i < results.length; i++) { + if (results[i] && results[i].hasOwnProperty('error')) { + const err = new ParseError( + results[i].error.code, + results[i].error.error + ); + err.object = objects[i]; + errors.push(err); + } + } }); return deleteCompleted.then(async () => { if (errors.length) { @@ -2216,7 +2193,6 @@ const DefaultController = { }, save(target: ParseObject | Array, options: RequestOptions) { - const batchSize = (options && options.batchSize) ? options.batchSize : DEFAULT_BATCH_SIZE; const localDatastore = CoreManager.getLocalDatastore(); const mapIdForPin = {}; @@ -2238,19 +2214,17 @@ const DefaultController = { } unsaved = unique(unsaved); - let filesSaved = Promise.resolve(); + const filesSaved: Array = []; let pending: Array = []; unsaved.forEach((el) => { if (el instanceof ParseFile) { - filesSaved = filesSaved.then(() => { - return el.save(); - }); + filesSaved.push(el.save()); } else if (el instanceof ParseObject) { pending.push(el); } }); - return filesSaved.then(() => { + return Promise.all(filesSaved).then(() => { let objectError = null; return continueWhile(() => { return pending.length > 0; @@ -2258,7 +2232,7 @@ const DefaultController = { const batch = []; const nextPending = []; pending.forEach((el) => { - if (batch.length < batchSize && canBeSerialized(el)) { + if (canBeSerialized(el)) { batch.push(el); } else { nextPending.push(el); @@ -2276,17 +2250,11 @@ const DefaultController = { // Queue up tasks for each object in the batch. // When every task is ready, the API request will execute - let res, rej; - const batchReturned = new Promise((resolve, reject) => { res = resolve; rej = reject; }); - batchReturned.resolve = res; - batchReturned.reject = rej; + const batchReturned = new resolvingPromise(); const batchReady = []; const batchTasks = []; batch.forEach((obj, index) => { - let res, rej; - const ready = new Promise((resolve, reject) => { res = resolve; rej = reject; }); - ready.resolve = res; - ready.reject = rej; + const ready = new resolvingPromise(); batchReady.push(ready); const task = function() { ready.resolve(); diff --git a/src/RESTController.js b/src/RESTController.js index 385742c22..a5fc5de95 100644 --- a/src/RESTController.js +++ b/src/RESTController.js @@ -17,7 +17,6 @@ export type RequestOptions = { sessionToken?: string; installationId?: string; returnStatus?: boolean; - batchSize?: number; include?: any; progress?: any; }; diff --git a/src/__tests__/ParseObject-test.js b/src/__tests__/ParseObject-test.js index 2f25e23ca..698e24164 100644 --- a/src/__tests__/ParseObject-test.js +++ b/src/__tests__/ParseObject-test.js @@ -1681,69 +1681,7 @@ describe('ParseObject', () => { jest.runAllTicks(); }); - it('can saveAll with batchSize', async (done) => { - const xhrs = []; - for (let i = 0; i < 2; i++) { - xhrs[i] = { - setRequestHeader: jest.fn(), - open: jest.fn(), - send: jest.fn(), - status: 200, - readyState: 4 - }; - } let current = 0; - RESTController._setXHR(function() { return xhrs[current++]; }); - const objects = []; - for (let i = 0; i < 22; i++) { - objects[i] = new ParseObject('Person'); - } - ParseObject.saveAll(objects, { batchSize: 20 }).then(() => { - expect(xhrs[0].open.mock.calls[0]).toEqual( - ['POST', 'https://api.parse.com/1/batch', true] - ); - expect(xhrs[1].open.mock.calls[0]).toEqual( - ['POST', 'https://api.parse.com/1/batch', true] - ); - done(); - }); - jest.runAllTicks(); - await flushPromises(); - - xhrs[0].responseText = JSON.stringify([ - { success: { objectId: 'pid0' } }, - { success: { objectId: 'pid1' } }, - { success: { objectId: 'pid2' } }, - { success: { objectId: 'pid3' } }, - { success: { objectId: 'pid4' } }, - { success: { objectId: 'pid5' } }, - { success: { objectId: 'pid6' } }, - { success: { objectId: 'pid7' } }, - { success: { objectId: 'pid8' } }, - { success: { objectId: 'pid9' } }, - { success: { objectId: 'pid10' } }, - { success: { objectId: 'pid11' } }, - { success: { objectId: 'pid12' } }, - { success: { objectId: 'pid13' } }, - { success: { objectId: 'pid14' } }, - { success: { objectId: 'pid15' } }, - { success: { objectId: 'pid16' } }, - { success: { objectId: 'pid17' } }, - { success: { objectId: 'pid18' } }, - { success: { objectId: 'pid19' } }, - ]); - xhrs[0].onreadystatechange(); - jest.runAllTicks(); - await flushPromises(); - - xhrs[1].responseText = JSON.stringify([ - { success: { objectId: 'pid20' } }, - { success: { objectId: 'pid21' } }, - ]); - xhrs[1].onreadystatechange(); - jest.runAllTicks(); - }); - it('returns the first error when saving an array of objects', async (done) => { const xhrs = []; for (let i = 0; i < 2; i++) { @@ -1765,7 +1703,7 @@ describe('ParseObject', () => { // The second batch never ran expect(xhrs[1].open.mock.calls.length).toBe(0); expect(objects[19].dirty()).toBe(false); - expect(objects[20].dirty()).toBe(true); + expect(objects[20].dirty()).toBe(false); expect(error.message).toBe('first error'); done(); @@ -1793,6 +1731,8 @@ describe('ParseObject', () => { { success: { objectId: 'pid17' } }, { success: { objectId: 'pid18' } }, { success: { objectId: 'pid19' } }, + { success: { objectId: 'pid20' } }, + { success: { objectId: 'pid21' } }, ]); xhrs[0].onreadystatechange(); jest.runAllTicks(); @@ -1927,81 +1867,6 @@ describe('ObjectController', () => { await result; }); - it('can destroy an array of objects with batchSize', async () => { - const objectController = CoreManager.getObjectController(); - const xhrs = []; - for (let i = 0; i < 3; i++) { - xhrs[i] = { - setRequestHeader: jest.fn(), - open: jest.fn(), - send: jest.fn() - }; - xhrs[i].status = 200; - xhrs[i].responseText = JSON.stringify({}); - xhrs[i].readyState = 4; - } - let current = 0; - RESTController._setXHR(function() { return xhrs[current++]; }); - let objects = []; - for (let i = 0; i < 5; i++) { - objects[i] = new ParseObject('Person'); - objects[i].id = 'pid' + i; - } - const result = objectController.destroy(objects, { batchSize: 20}).then(async () => { - expect(xhrs[0].open.mock.calls[0]).toEqual( - ['POST', 'https://api.parse.com/1/batch', true] - ); - expect(JSON.parse(xhrs[0].send.mock.calls[0]).requests).toEqual([ - { - method: 'DELETE', - path: '/1/classes/Person/pid0', - body: {} - }, { - method: 'DELETE', - path: '/1/classes/Person/pid1', - body: {} - }, { - method: 'DELETE', - path: '/1/classes/Person/pid2', - body: {} - }, { - method: 'DELETE', - path: '/1/classes/Person/pid3', - body: {} - }, { - method: 'DELETE', - path: '/1/classes/Person/pid4', - body: {} - } - ]); - - objects = []; - for (let i = 0; i < 22; i++) { - objects[i] = new ParseObject('Person'); - objects[i].id = 'pid' + i; - } - const destroy = objectController.destroy(objects, { batchSize: 20 }); - jest.runAllTicks(); - await flushPromises(); - xhrs[1].onreadystatechange(); - jest.runAllTicks(); - await flushPromises(); - expect(xhrs[1].open.mock.calls.length).toBe(1); - xhrs[2].onreadystatechange(); - jest.runAllTicks(); - return destroy; - }).then(() => { - expect(JSON.parse(xhrs[1].send.mock.calls[0]).requests.length).toBe(20); - expect(JSON.parse(xhrs[2].send.mock.calls[0]).requests.length).toBe(2); - }); - jest.runAllTicks(); - await flushPromises(); - - xhrs[0].onreadystatechange(); - jest.runAllTicks(); - await result; - }); - it('can destroy an array of objects', async () => { const objectController = CoreManager.getObjectController(); const xhrs = []; @@ -2062,12 +1927,10 @@ describe('ObjectController', () => { jest.runAllTicks(); await flushPromises(); expect(xhrs[1].open.mock.calls.length).toBe(1); - xhrs[2].onreadystatechange(); jest.runAllTicks(); return destroy; }).then(() => { - expect(JSON.parse(xhrs[1].send.mock.calls[0]).requests.length).toBe(20); - expect(JSON.parse(xhrs[2].send.mock.calls[0]).requests.length).toBe(2); + expect(JSON.parse(xhrs[1].send.mock.calls[0]).requests.length).toBe(22); }); jest.runAllTicks(); await flushPromises(); @@ -2194,14 +2057,15 @@ describe('ObjectController', () => { jest.runAllTicks(); await flushPromises(); - xhrs[1].responseText = JSON.stringify(response.slice(0, 20)); - xhrs[2].responseText = JSON.stringify(response.slice(20)); + xhrs[1].responseText = JSON.stringify(response); + // Objects in the second batch will not be prepared for save yet // This means they can also be modified before the first batch returns expect( SingleInstanceStateController.getState({ className: 'Person', id: objects[20]._getId() }).pendingOps.length - ).toBe(1); + ).toBe(2); + objects[20].set('index', 0); xhrs[1].onreadystatechange(); @@ -2209,15 +2073,9 @@ describe('ObjectController', () => { await flushPromises(); expect(objects[0].dirty()).toBe(false); expect(objects[0].id).toBe('pid0'); - expect(objects[20].dirty()).toBe(true); - expect(objects[20].id).toBe(undefined); - - xhrs[2].onreadystatechange(); - jest.runAllTicks(); - await flushPromises(); expect(objects[20].dirty()).toBe(false); - expect(objects[20].get('index')).toBe(0); expect(objects[20].id).toBe('pid20'); + return save; }).then((results) => { expect(results.length).toBe(22); From 3826dc766cec5b2500f755ba71e2bd2b49c4875b Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Mon, 30 Dec 2019 14:19:56 -0600 Subject: [PATCH 2/2] lint --- src/__tests__/ParseObject-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/__tests__/ParseObject-test.js b/src/__tests__/ParseObject-test.js index 698e24164..c74c852e4 100644 --- a/src/__tests__/ParseObject-test.js +++ b/src/__tests__/ParseObject-test.js @@ -1681,7 +1681,6 @@ describe('ParseObject', () => { jest.runAllTicks(); }); - let current = 0; it('returns the first error when saving an array of objects', async (done) => { const xhrs = []; for (let i = 0; i < 2; i++) {