MLIBZ-2131 Remove implicit push() calls

src/core/datastore/cachestore.js

@@ -7,7 +7,7 @@ import { NetworkStore } from './networkstore';
 import { OperationType } from './operations';
 import { processorFactory } from './processors';
 import { syncManagerProvider } from './sync';
-import { formTaggedCollectionName } from './utils';
+import { formTaggedCollectionName, getEntitiesPendingPushError } from './utils';
 
 /**
  * The CacheStore class is used to find, create, update, remove, count and group entities. Entities are stored
@@ -114,7 +114,8 @@ export class CacheStore extends NetworkStore {
     return this.syncManager.getSyncItemCountByEntityQuery(this.collection, query)
       .then((count) => {
         if (count > 0) {
-          return this.syncManager.push(this.collection, query);
+          const err = getEntitiesPendingPushError(count, 'fetch the entities');
+          return Promise.reject(err);
         }
         return Promise.resolve();
       })

src/core/datastore/cachestore.spec.js

@@ -13,6 +13,7 @@ import { NodeHttpMiddleware } from '../../node/http';
 import { User } from '../user';
 
 const collection = 'Books';
+const pendingPushEntitiesErrMsg = 'There is 1 entity, matching the provided query or id, pending push to the backend.';
 
 describe('CacheStore', () => {
   let client;
@@ -97,10 +98,7 @@ describe('CacheStore', () => {
             .subscribe(null, (error) => {
               try {
                 expect(error).toBeA(KinveyError);
-                expect(error.message).toEqual(
-                  'Unable to fetch the entities on the backend.'
-                  + ' There are 1 entities that need to be synced.'
-                );
+                expect(error.message).toInclude(pendingPushEntitiesErrMsg);
                 done();
               } catch (e) {
                 done(e);
@@ -263,10 +261,7 @@ describe('CacheStore', () => {
             .subscribe(null, (error) => {
               try {
                 expect(error).toBeA(KinveyError);
-                expect(error.message).toEqual(
-                  'Unable to find the entity on the backend.'
-                  + ' There are 1 entities that need to be synced.'
-                );
+                expect(error.message).toInclude(pendingPushEntitiesErrMsg);
                 done();
               } catch (e) {
                 done(e);
@@ -402,31 +397,6 @@ describe('CacheStore', () => {
         });
     });
 
-    it('should throw an error if there are entities to sync', (done) => {
-      const entity = { _id: randomString() };
-      const syncStore = new SyncStore(collection);
-      syncStore.save(entity)
-        .then(() => {
-          const aggregation = new Aggregation();
-          const store = new CacheStore(collection);
-          store.group(aggregation)
-            .subscribe(null, (error) => {
-              try {
-                expect(error).toBeA(KinveyError);
-                expect(error.message).toEqual(
-                  'Unable to group entities on the backend.'
-                  + ' There are 1 entities that need to be synced.'
-                );
-                done();
-              } catch (e) {
-                done(e);
-              }
-            }, () => {
-              done(new Error('This test should fail.'));
-            });
-        });
-    });
-
     it('should return the count of all unique properties on the collection', (done) => {
       const entity1 = { _id: randomString(), title: randomString() };
       const entity2 = { _id: randomString(), title: randomString() };
@@ -497,30 +467,6 @@ describe('CacheStore', () => {
         });
     });
 
-    it('should throw an error if there are entities to sync', (done) => {
-      const entity = { _id: randomString() };
-      const syncStore = new SyncStore(collection);
-      syncStore.save(entity)
-        .then(() => {
-          const store = new CacheStore(collection);
-          store.count()
-            .subscribe(null, (error) => {
-              try {
-                expect(error).toBeA(KinveyError);
-                expect(error.message).toEqual(
-                  'Unable to count entities on the backend.'
-                  + ' There are 1 entities that need to be synced.'
-                );
-                done();
-              } catch (e) {
-                done(e);
-              }
-            }, () => {
-              done(new Error('This test should fail.'));
-            });
-        });
-    });
-
     it('should return the count for the collection', (done) => {
       const entity1 = { _id: randomString() };
       const entity2 = { _id: randomString() };

src/core/datastore/processors/cache-offline-data-processor.js

@@ -2,12 +2,12 @@ import { Promise } from 'es6-promise';
 import clone from 'lodash/clone';
 
 import { Query } from '../../query';
-import { KinveyError, NotFoundError } from '../../errors';
+import { NotFoundError } from '../../errors';
 
 import { OfflineDataProcessor } from './offline-data-processor';
 import { ensureArray } from '../../utils';
 import { wrapInObservable } from '../../observable';
-import { isLocalEntity, isNotEmpty, isEmpty } from '../utils';
+import { isLocalEntity, isNotEmpty, isEmpty, getEntitiesPendingPushError } from '../utils';
 
 // imported for type info
 // import { NetworkRepository } from '../repositories';
@@ -79,13 +79,13 @@ export class CacheOfflineDataProcessor extends OfflineDataProcessor {
   _processRead(collection, query, options) {
     let offlineEntities;
     return wrapInObservable((observer) => {
-      return this._ensureCountBeforeRead(collection, 'fetch the entities', query)
-        .then(() => super._processRead(collection, query, options))
+      return super._processRead(collection, query, options)
         .then((entities) => {
           offlineEntities = entities;
           observer.next(offlineEntities);
-          return this._networkRepository.read(collection, query, options);
+          return this._ensureCountBeforeRead(collection, 'fetch the entities', query);
         })
+        .then(() => this._networkRepository.read(collection, query, options))
         .then((networkEntities) => {
           observer.next(networkEntities);
           return this._replaceOfflineEntities(collection, offlineEntities, networkEntities);
@@ -97,14 +97,14 @@ export class CacheOfflineDataProcessor extends OfflineDataProcessor {
     let offlineEntity;
     return wrapInObservable((observer) => {
       const query = new Query().equalTo('_id', entityId);
-      return this._ensureCountBeforeRead(collection, 'find the entity', query)
-        .then(() => super._processReadById(collection, entityId, options))
+      return super._processReadById(collection, entityId, options)
         .catch(err => this._catchNotFoundError(err)) // backwards compatibility
         .then((entity) => {
           observer.next(entity);
           offlineEntity = entity;
-          return this._networkRepository.readById(collection, entityId, options);
+          return this._ensureCountBeforeRead(collection, 'find the entity', query);
         })
+        .then(() => this._networkRepository.readById(collection, entityId, options))
         .then((entity) => {
           observer.next(entity);
           return this._replaceOfflineEntities(collection, offlineEntity, ensureArray(entity));
@@ -125,15 +125,12 @@ export class CacheOfflineDataProcessor extends OfflineDataProcessor {
 
   _processCount(collection, query, options) {
     return wrapInObservable((observer) => {
-      return this._ensureCountBeforeRead(collection, 'count entities', query)
-        .then(() => super._processCount(collection, query, options))
+      return super._processCount(collection, query, options)
         .then((offlineCount) => {
           observer.next(offlineCount);
           return this._networkRepository.count(collection, query, options);
         })
-        .then((networkCount) => {
-          observer.next(networkCount);
-        });
+        .then(networkCount => observer.next(networkCount));
     });
   }
 
@@ -143,9 +140,8 @@ export class CacheOfflineDataProcessor extends OfflineDataProcessor {
         .catch(() => []) // backwards compatibility
         .then((offlineResult) => {
           observer.next(offlineResult);
-          return this._ensureCountBeforeRead(collection, 'group entities');
+          return this._networkRepository.group(collection, aggregationQuery, options);
         })
-        .then(() => this._networkRepository.group(collection, aggregationQuery, options))
         .then(networkResult => observer.next(networkResult));
     });
   }
@@ -213,20 +209,11 @@ export class CacheOfflineDataProcessor extends OfflineDataProcessor {
 
   _ensureCountBeforeRead(collection, prefix, query) {
     return this._syncManager.getSyncItemCountByEntityQuery(collection, query)
-      .then((count) => {
-        if (count > 0) { // backwards compatibility
-          return this._syncManager.push(collection, query)
-            .then(() => this._syncManager.getSyncItemCountByEntityQuery(collection, query));
-        }
-        return count;
-      })
       .then((count) => {
         if (count === 0) {
           return count;
         }
-        const countMsg = `There are ${count} entities that need to be synced.`;
-        const err = new KinveyError(`Unable to ${prefix} on the backend. ${countMsg}`);
-        return Promise.reject(err);
+        return Promise.reject(getEntitiesPendingPushError(count, prefix));
       });
   }
 

src/core/datastore/utils/utils.js

@@ -1,9 +1,21 @@
 import _isEmpty from 'lodash/isEmpty';
 
+import { KinveyError } from '../../errors';
 import { isNonemptyString } from '../../utils';
 
 export const dataStoreTagSeparator = '.';
 
+export function getEntitiesPendingPushError(entityCount, prefix) {
+  let countMsg = `There are ${entityCount} entities, matching the provided query, pending push to the backend.`;
+
+  if (entityCount === 1) {
+    countMsg = `There is ${entityCount} entity, matching the provided query or id, pending push to the backend.`;
+  }
+
+  const errMsg = `Unable to ${prefix} on the backend. The result will overwrite your local changes. ${countMsg}`;
+  return new KinveyError(errMsg);
+}
+
 export function generateEntityId(length = 24) {
   const chars = 'abcdef0123456789';
   let objectId = '';

src/core/datastore/working-with-data-tests/cache-offline-data-processor.spec.js

@@ -75,48 +75,12 @@ describe('CacheOfflineDataProcessor', () => {
             });
         });
 
-        it('should call SyncManager.push() if there are entities to sync', () => {
+        it('should return an error if there are entities to sync', () => {
           syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(123);
-          syncManagerMock.push = createPromiseSpy().andCall(() => {
-            syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(0);
-            return Promise.resolve();
-          });
-          return dataProcessor.process(operation).toPromise()
-            .then(() => {
-              validateSpyCalls(syncManagerMock.push, 1, [collection, getExpectedQuery()]);
-            });
-        });
-
-        it('should call SyncManager.getSyncItemCountByEntityQuery() again, after push()', () => {
-          syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(123);
-          syncManagerMock.push = createPromiseSpy().andCall(() => {
-            syncManagerMock.getSyncItemCountByEntityQuery.andReturn(Promise.resolve(0));
-            return Promise.resolve();
-          });
-          return dataProcessor.process(operation).toPromise()
-            .then(() => {
-              validateSpyCalls(syncManagerMock.push, 1, [collection, getExpectedQuery()]);
-              const secondCountSpy = syncManagerMock.getSyncItemCountByEntityQuery;
-              validateSpyCalls(secondCountSpy, 2, [collection, getExpectedQuery()], [collection, getExpectedQuery()]);
-            });
-        });
-
-        it('should return an error if there are still entities to sync, after push()', () => {
-          syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(123);
-          syncManagerMock.push = createPromiseSpy().andCall(() => {
-            return Promise.resolve();
-          });
+          syncManagerMock.push = createPromiseSpy();
           return dataProcessor.process(operation).toPromise()
             .catch((err) => {
-              validateError(err, KinveyError, 'entities that need to be synced');
-            });
-        });
-
-        it('should NOT call SyncManager.push() if there are NO entities to sync', () => {
-          syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(0);
-          return dataProcessor.process(operation).toPromise()
-            .then(() => {
-              validateSpyCalls(syncManagerMock.push, 0);
+              validateError(err, KinveyError, 'pending push to the backend');
             });
         });
       }
@@ -244,8 +208,6 @@ describe('CacheOfflineDataProcessor', () => {
           expect(result).toBeA(KinveyObservable);
         });
 
-        addSyncQueueTests();
-
         it('should call OfflineRepo.count()', () => {
           return dataProcessor.process(operation, options).toPromise()
             .then(() => {
@@ -278,8 +240,6 @@ describe('CacheOfflineDataProcessor', () => {
             });
         });
 
-        addSyncQueueTests();
-
         it('should call NetworkRepo.group()', () => {
           return dataProcessor.process(operation, options).toPromise()
             .then(() => {

src/core/datastore/working-with-data-tests/data-stores-sync-manager.spec.js

@@ -2,7 +2,8 @@ import expect from 'expect';
 
 import { DataStoreType } from '../datastore';
 import { Query } from '../../query';
-import { datastoreFactory, createPromiseSpy, validateSpyCalls } from './utils';
+import { datastoreFactory, createPromiseSpy, validateSpyCalls, validateError } from './utils';
+import { KinveyError } from '../../errors';
 
 const collection = 'books';
 const optionKeyName = 'test';
@@ -64,7 +65,7 @@ describe('Data stores delegate correctly to sync manager', () => {
         return store.pendingSyncCount()
           .then(() => {
             const spy = syncManagerMock.getSyncItemCountByEntityQuery;
-            validateSpyCalls(spy, 1, [collection, undefined])
+            validateSpyCalls(spy, 1, [collection, undefined]);
           });
       });
 
@@ -90,15 +91,18 @@ describe('Data stores delegate correctly to sync manager', () => {
       it('pull() when there are pending sync items', () => {
         const query = new Query();
         const options = { [optionKeyName]: true };
-        syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(123);
+        const pendingSyncEntityCount = 123;
+        syncManagerMock.getSyncItemCountByEntityQuery = createPromiseSpy(pendingSyncEntityCount);
         return store.pull(query, options)
-          .then(() => {
-            const pushSpy = syncManagerMock.push;
-            validateSyncManagerCall(pushSpy, query);
+          .then(() => Promise.reject(new Error('should not happen')))
+          .catch((err) => {
+            const countSpy = syncManagerMock.getSyncItemCountByEntityQuery;
+            validateSyncManagerCall(countSpy, query);
+
+            validateError(err, KinveyError, `There are ${pendingSyncEntityCount} entities`);
 
             const pullSpy = syncManagerMock.pull;
-            validateSyncManagerCall(pullSpy, query, options);
-            expect(pullSpy.calls[0].arguments[2].useDeltaFetch).toBe(store.useDeltaFetch);
+            expect(pullSpy.calls.length).toBe(0);
           });
       });
 

test/integration/tests/sync.test.js

@@ -424,12 +424,12 @@ function testFunc() {
               .catch(done);
           });
 
-          it('should make a silent push before the pull operation', (done) => {
+          it('should return an error if there are entities awaiting to be pushed to the backend', (done) => {
             syncStore.save(entity3)
               .then(() => storeToTest.pull())
-              .then(() => networkStore.findById(entity3._id).toPromise())
-              .then((result) => {
-                expect(result._id).to.equal(entity3._id);
+              .then(() => Promise.reject(new Error('should not happen')))
+              .catch((err) => {
+                expect(err.message).to.contain('There are 1 entities');
                 done();
               })
               .catch(done);