fix: Make aggregation query requests run properly inside a transaction (#1166)

* Add a test that ensures the read is txn read

When doing a readOnly transaction, the code should do reads from a consistent snapshot so that if a write is done mid-transaction then that doesn’t affect the read value.

* Code change that lets test pass

This code change ensures transactions that run aggregate queries actually make the request as an aggregate query.

* Add a test to verify run query results

Verify run query results at a particular time so that we know what data we are working with for the rest of the aggregate query tests.

* linting fix

Author: danieljbruce

src/request.ts

@@ -1000,7 +1000,12 @@ class DatastoreRequest {
       reqOpts.transaction = this.id;
     }
 
-    if (isTransaction && (method === 'lookup' || method === 'runQuery')) {
+    if (
+      isTransaction &&
+      (method === 'lookup' ||
+        method === 'runQuery' ||
+        method === 'runAggregationQuery')
+    ) {
       if (reqOpts.readOptions && reqOpts.readOptions.readConsistency) {
         throw new Error(
           'Read consistency cannot be specified in a transaction.'

system-test/datastore.ts

@@ -17,7 +17,7 @@ import {readFileSync} from 'fs';
 import * as path from 'path';
 import {after, before, describe, it} from 'mocha';
 import * as yaml from 'js-yaml';
-import {Datastore, DatastoreOptions, Index} from '../src';
+import {Datastore, DatastoreOptions, Index, Transaction} from '../src';
 import {google} from '../protos/protos';
 import {Storage} from '@google-cloud/storage';
 import {AggregateField} from '../src/aggregate';
@@ -1857,6 +1857,16 @@ async.each(
         });
 
         describe('aggregate query within a transaction', async () => {
+          it('should run a query and return the results', async () => {
+            // Add a test here to verify what the data is at this time.
+            // This will be a valuable reference for tests in this describe block.
+            const query = datastore.createQuery('Company');
+            const [results] = await datastore.runQuery(query);
+            assert.deepStrictEqual(
+              results.map(result => result.rating),
+              [100, 100]
+            );
+          });
           it('should aggregate query within a count transaction', async () => {
             const transaction = datastore.transaction();
             await transaction.run();
@@ -1914,6 +1924,40 @@ async.each(
             assert.deepStrictEqual(result, [{'average rating': 100}]);
             await transaction.commit();
           });
+          it('readOnly transaction should see consistent snapshot of database', async () => {
+            async function getResults(transaction: Transaction) {
+              const query = transaction.createQuery('Company');
+              const aggregateQuery = transaction
+                .createAggregationQuery(query)
+                .count('total');
+              let result;
+              try {
+                [result] = await aggregateQuery.run();
+              } catch (e) {
+                await transaction.rollback();
+                assert.fail(
+                  'The aggregation query run should have been successful'
+                );
+              }
+              return result;
+            }
+            const key = datastore.key(['Company', 'Google']);
+            const transaction = datastore.transaction({readOnly: true});
+            await transaction.run();
+            const results = await getResults(transaction);
+            assert.deepStrictEqual(results, [{total: 2}]);
+            await datastore.save([
+              {
+                key,
+                data: {
+                  rating: 100,
+                },
+              },
+            ]);
+            const resultsAgain = await getResults(transaction);
+            assert.deepStrictEqual(resultsAgain, [{total: 2}]);
+            await transaction.commit();
+          });
         });
 
         it('should read in a readOnly transaction', async () => {