cache results of some observation search queries #428

lib/controllers/v1/identifications_controller.js

@@ -93,9 +93,9 @@ const IdentificationsController = class IdentificationsController {
   static async elasticResults( req ) {
     const query = IdentificationsController.reqToElasticQuery( req );
     return ESModel.elasticResults( req, query, "identifications", {
-      track_total_hits: !req.query.skip_total_hits && !req.query.no_total_hits,
-      skip_total_hits: req.query.skip_total_hits,
-      no_total_hits: req.query.no_total_hits
+      trackTotalHits: !req.query.skip_total_hits && !req.query.no_total_hits,
+      skipTotalHits: req.query.skip_total_hits,
+      noTotalHits: req.query.no_total_hits
     } );
   }
 

lib/controllers/v1/observations_controller.js

@@ -111,7 +111,7 @@ ObservationsController.searchCacheWrapper = async req => (
     60 * 60 )
 );
 
-ObservationsController.search = async req => {
+ObservationsController.search = async ( req, options = { } ) => {
   if ( req.query.return_bounds === "true" ) {
     // If we've been asked to return the bounds but also to return obs in a
     // place, assume that the bounds are the bounding box of the place and not
@@ -139,7 +139,7 @@ ObservationsController.search = async req => {
       };
     }
   }
-  const data = await ObservationsController.resultsForRequest( req );
+  const data = await ObservationsController.resultsForRequest( req, options );
   const localeOpts = util.localeOpts( req );
   let preloadMethod;
   if ( req.query.only_id && req.query.only_id !== "false" ) {
@@ -211,14 +211,18 @@ ObservationsController.loadNewProjects = async ( obs, options = {} ) => (
 );
 
 // this needs to remain a named function
-ObservationsController.elasticResults = async function observationElasticResults( req ) {
+ObservationsController.elasticResults = async function observationElasticResults(
+  req, options = { }
+) {
   const query = await ObservationsController.reqToElasticQuery( req );
   req.elastic_query = query;
   const opts = {
     excludes: ["taxon.names", "taxon.photos", "taxon.taxon_photos", "taxon.names_*"],
-    track_total_hits: !req.query.skip_total_hits && !req.query.no_total_hits,
-    skip_total_hits: req.query.skip_total_hits,
-    no_total_hits: req.query.no_total_hits
+    trackTotalHits: !req.query.skip_total_hits && !req.query.no_total_hits,
+    skipTotalHits: req.query.skip_total_hits,
+    noTotalHits: req.query.no_total_hits,
+    redisQueryCacheKey: options.redisQueryCacheKey,
+    cacheSeconds: options.cacheSeconds
   };
   const returnOnlyID = req.query.only_id && req.query.only_id !== "false";
   if ( returnOnlyID ) {
@@ -366,8 +370,8 @@ ObservationsController.prepareElasticDataForResponse = ( data, req ) => {
   return response;
 };
 
-ObservationsController.resultsForRequest = async req => {
-  const data = await ObservationsController.elasticResults( req );
+ObservationsController.resultsForRequest = async ( req, options = { } ) => {
+  const data = await ObservationsController.elasticResults( req, options );
   return ObservationsController.prepareElasticDataForResponse( data, req );
 };
 
@@ -382,7 +386,19 @@ ObservationsController.methodCacheWrapper = async (
 ) => {
   await ObservationQueryBuilder.applyLookupRules( req );
   const redisCacheKey = util.observationSearchRequestCacheKey( req, cacheKey );
-  return RedisCacheClient.getOrSetJSON( redisCacheKey, cacheSeconds, async ( ) => method( req ) );
+  const redisQueryCacheKey = util.observationSearchRequestCacheKey(
+    req,
+    cacheKey,
+    { ignoreLocalization: true }
+  );
+  return RedisCacheClient.getOrSetJSON(
+    redisCacheKey,
+    cacheSeconds,
+    async ( ) => method( req, {
+      redisQueryCacheKey,
+      cacheSeconds
+    } )
+  );
 };
 
 ObservationsController.speciesCountsCacheWrapper = async req => (

lib/controllers/v1/taxa_controller.js

@@ -664,7 +664,7 @@ TaxaController.search = async req => {
       break;
   }
   searchReq.query.sort = sort;
-  searchOptions.track_total_hits = true;
+  searchOptions.trackTotalHits = true;
   return TaxaController.searchQuery( searchReq, searchOptions );
 };
 
@@ -685,7 +685,7 @@ TaxaController.searchQuery = async ( req, opts = { } ) => {
     searchHash.per_page = options.perPageOverride || 500;
   }
   const elasticQuery = req.elastic_query || esClient.searchHash( searchHash );
-  elasticQuery.track_total_hits = req.query.track_total_hits || options.track_total_hits;
+  elasticQuery.track_total_hits = req.query.track_total_hits || options.trackTotalHits;
   const defaultSource = _.clone( Taxon.esReturnFields );
   // we don't want all photos for ancestors or children
   if ( options.details || options.photos ) {

lib/models/es_model.js

@@ -212,7 +212,8 @@ const ESModel = class ESModel {
     return leafCounts;
   }
 
-  static async elasticResults( req, query, index, options = { } ) {
+  static async elasticResults( req, query, index, opts = { } ) {
+    const options = { ...opts };
     req.elastic_query = query;
     if ( req.query && req.query.aggs && _.isObject( req.query.aggs ) ) {
       req.elastic_query.aggs = req.query.aggs;
@@ -226,14 +227,35 @@ const ESModel = class ESModel {
       searchHash._source = searchHash._source || { };
       searchHash._source.includes = options.includes;
     }
-    if ( options.track_total_hits ) {
+    if ( options.trackTotalHits ) {
       searchHash.track_total_hits = true;
-    } else if ( options.no_total_hits ) {
+    } else if ( options.noTotalHits ) {
       searchHash.track_total_hits = false;
     }
-    return esClient.search( index, {
-      body: searchHash
-    } );
+    if ( ESModel.searchHashReferencesPotentiallySensitiveFields( searchHash ) ) {
+      delete options.redisQueryCacheKey;
+      delete options.cacheSeconds;
+    }
+    const searchRequest = async ( ) => (
+      esClient.search( index, {
+        body: searchHash
+      } )
+    );
+    return RedisCacheClient.getOrSetJSON(
+      options.redisQueryCacheKey,
+      options.cacheSeconds,
+      searchRequest
+    );
+  }
+
+  static searchHashReferencesPotentiallySensitiveFields( searchHash ) {
+    const searchString = JSON.stringify( searchHash );
+    if ( searchString.match( /"user\.id":/ )
+      || searchString.match( /"user\.id\.keyword":/ )
+      || searchString.match( /private_/ ) ) {
+      return true;
+    }
+    return false;
   }
 
   static async mgetResults( ids, index, options = { } ) {

lib/util.js

@@ -357,11 +357,17 @@ const util = class util {
   }
 
   static observationSearchRequestCacheKey( req, prefix, options = { } ) {
-    let fileCacheKey;
-    if ( process.env.NODE_ENV === "test" ) { return null; }
+    let fileCacheKey = null;
+    if ( process.env.NODE_ENV === "test" && !options.enableInTestEnv ) { return null; }
     const queryDup = _.pickBy( _.cloneDeep( req.query ), a => ( a !== "any" ) );
     const reqInatDup = _.pickBy( _.cloneDeep( req.inat ), a => ( a !== "any" ) );
 
+    // if the user is logged-in and there is a place_id filter,
+    // do not attempt to cache any results
+    if ( req.userSession && queryDup.place_id ) {
+      return null;
+    }
+
     // if the user is logged-in and has taxon name priorities, add a property
     // to the non-query data to consider. Results from users with different name
     // priorities will be cached separated, even if the original URL is the same
@@ -404,6 +410,7 @@ const util = class util {
     if ( prefix === "ObservationsController.identifiers"
       || prefix === "ObservationsController.observers"
       || prefix === "ESModel.ancestriesSpeciesCounts"
+      || options.ignoreLocalization
     ) {
       delete queryDup.locale;
       delete queryDup.preferred_place_id;
@@ -420,7 +427,7 @@ const util = class util {
     cacheableParams.orderBy = queryDup.order_by ? queryDup.order_by : null;
     cacheableParams.returnBounds = queryDup.return_bounds ? queryDup.return_bounds : null;
     cacheableParams.locale = queryDup.locale ? queryDup.locale : null;
-    cacheableParams.no_total_hits = queryDup.no_total_hits ? queryDup.no_total_hits : null;
+    cacheableParams.noTotalHits = queryDup.no_total_hits ? queryDup.no_total_hits : null;
     cacheableParams.preferredPlaceID = queryDup.preferred_place_id
       ? queryDup.preferred_place_id : null;
     cacheableParams.photos = queryDup.photos ? queryDup.photos : null;

test/models/es_model.js

@@ -49,4 +49,54 @@ describe( "ESModel", ( ) => {
       expect( instances["11"].name ).to.eq( "Junonia hierta" );
     } );
   } );
+
+  describe( "searchHashReferencesPotentiallySensitiveFields", ( ) => {
+    it( "returns false by default", ( ) => {
+      expect( ESModel.searchHashReferencesPotentiallySensitiveFields( {
+        query: { match_all: { } }
+      } ) ).to.be.false;
+    } );
+
+    it( "returns true for filters on user.id", ( ) => {
+      expect( ESModel.searchHashReferencesPotentiallySensitiveFields( {
+        query: {
+          bool: {
+            filter: [{
+              term: {
+                "user.id": 1
+              }
+            }]
+          }
+        }
+      } ) ).to.be.true;
+    } );
+
+    it( "returns true for filters on user.id.keyword", ( ) => {
+      expect( ESModel.searchHashReferencesPotentiallySensitiveFields( {
+        query: {
+          bool: {
+            filter: [{
+              term: {
+                "user.id.keyword": 1
+              }
+            }]
+          }
+        }
+      } ) ).to.be.true;
+    } );
+
+    it( "returns true for references to private_* fields", ( ) => {
+      expect( ESModel.searchHashReferencesPotentiallySensitiveFields( {
+        query: {
+          bool: {
+            filter: [{
+              term: {
+                private_place_id: 1
+              }
+            }]
+          }
+        }
+      } ) ).to.be.true;
+    } );
+  } );
 } );

test/util.js

@@ -163,4 +163,66 @@ describe( "util", ( ) => {
       } ) ).to.be.null;
     } );
   } );
+
+  describe( "observationSearchRequestCacheKey", ( ) => {
+    it( "returns a cache key for cacheable queries", ( ) => {
+      const req = {
+        query: {
+          page: "1",
+          return_bounds: "true"
+        }
+      };
+      expect( util.observationSearchRequestCacheKey( req, "ObservationsController.search", {
+        enableInTestEnv: true
+      } ) ).to.eq( "ObservationsController.search-returnBounds-true" );
+    } );
+
+    it( "allows queries with place_id to be cached for obs search", ( ) => {
+      const req = {
+        query: {
+          place_id: 1
+        }
+      };
+      expect( util.observationSearchRequestCacheKey( req, "ObservationsController.search", {
+        enableInTestEnv: true
+      } ) ).to.eq( "ObservationsController.search-placeID-1" );
+    } );
+
+    it( "does not allow queries with place_id to be cached for obs search when logged in", ( ) => {
+      const req = {
+        query: {
+          place_id: 1
+        },
+        userSession: {
+          user_id: 1
+        }
+      };
+      expect( util.observationSearchRequestCacheKey( req, "ObservationsController.search", {
+        enableInTestEnv: true
+      } ) ).to.be.null;
+    } );
+
+    it( "includes locale in cache key for obs search by default", ( ) => {
+      const req = {
+        query: {
+          locale: "en"
+        }
+      };
+      expect( util.observationSearchRequestCacheKey( req, "ObservationsController.search", {
+        enableInTestEnv: true
+      } ) ).to.eq( "ObservationsController.search-locale-en" );
+    } );
+
+    it( "excludes locale from cache key for obs search when requested", ( ) => {
+      const req = {
+        query: {
+          locale: "en"
+        }
+      };
+      expect( util.observationSearchRequestCacheKey( req, "ObservationsController.search", {
+        enableInTestEnv: true,
+        ignoreLocalization: true
+      } ) ).to.eq( "ObservationsController.search" );
+    } );
+  } );
 } );