diff --git a/lib/controllers/v1/identifications_controller.js b/lib/controllers/v1/identifications_controller.js index 0ce9237f..b51bc772 100644 --- a/lib/controllers/v1/identifications_controller.js +++ b/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 } ); } diff --git a/lib/controllers/v1/observations_controller.js b/lib/controllers/v1/observations_controller.js index 4cbb6cd9..79360fc9 100644 --- a/lib/controllers/v1/observations_controller.js +++ b/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 => ( diff --git a/lib/controllers/v1/taxa_controller.js b/lib/controllers/v1/taxa_controller.js index c89b7a32..f66bca4b 100644 --- a/lib/controllers/v1/taxa_controller.js +++ b/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 ) { diff --git a/lib/models/es_model.js b/lib/models/es_model.js index cf858740..26a968b8 100644 --- a/lib/models/es_model.js +++ b/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 = { } ) { diff --git a/lib/util.js b/lib/util.js index 87d133c8..04d8a945 100644 --- a/lib/util.js +++ b/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; diff --git a/test/models/es_model.js b/test/models/es_model.js index 78c9a2e1..df46bbd6 100644 --- a/test/models/es_model.js +++ b/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; + } ); + } ); } ); diff --git a/test/util.js b/test/util.js index 5741e4c8..d259b37e 100644 --- a/test/util.js +++ b/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" ); + } ); + } ); } );