Merge pull request #116 from NYPL/NOREF-fix-advanced-search

NOREF Fix Advanced Search

CHANGELOG.md

@@ -1,5 +1,10 @@
 # CHANGELOG
 
+## unreleased -- v0.6.6
+### Fixed
+- Default sorting options
+- Added Redis client to `utils/languages` endpoint to resolve error
+
 ## 2021-06-24 -- v0.6.5
 ### Fixed
 - Handling of empty result sets from ElasticSearch

api/blueprints/drbUtils.py

@@ -1,4 +1,4 @@
-from flask import Blueprint, jsonify, current_app, request
+from flask import Blueprint, current_app, request
 
 from ..db import DBClient
 from ..elastic import ElasticClient
@@ -12,7 +12,7 @@
 
 @utils.route('/languages', methods=['GET'])
 def languageCounts():
-    esClient = ElasticClient()
+    esClient = ElasticClient(current_app.config['REDIS_CLIENT'])
 
     reqParams = APIUtils.normalizeQueryParams(request.args)
     workCounts = reqParams.get('totals', ['true'])[0].lower() != 'false'

api/elastic.py

@@ -99,7 +99,6 @@ def executeSearchQuery(self, params, page, perPage):
         else:
             res = self.query[startPos:endPos].execute()
 
-
         if not searchFromStr:
             try:
                 lastSort = list(res.hits[-1].meta.sort)
@@ -141,7 +140,7 @@ def executeDeepQuery(self, startPos, perPage):
     def setPageResultCache(self, cacheKey, sort):
         self.redis.set(
             '{}/queryPaging/{}'.format(self.environment, cacheKey),
-            '|'.join(sort),
+            '|'.join([str(s) for s in sort]),
             ex=60*60*24
         )
 
@@ -171,7 +170,7 @@ def makeDictHashable(cls, object):
 
     @staticmethod
     def escapeSearchQuery(query):
-        return re.sub(r'[\+\-\&\|\!\(\)\[\]\{\}\^\~\?\:\\]{1}', '\\\\\g<0>', query)
+        return re.sub(r'[\+\-\&\|\!\(\)\[\]\{\}\^\~\?\:\\\/]{1}', '\\\\\g<0>', query)
 
     def languageQuery(self, workTotals):
         search = self.createSearch()
@@ -190,7 +189,7 @@ def languageQuery(self, workTotals):
     def titleQuery(cls, titleText):
         return Q('bool',
             should=[
-                Q('query_string', query=titleText, fields=['title', 'alt_titles'], default_operator='and'),
+                Q('query_string', query=titleText, fields=['title^3', 'alt_titles'], default_operator='and'),
                 Q('nested', path='editions', query=Q('query_string', query=titleText, fields=['editions.title'], default_operator='and'))
             ]
         )
@@ -199,7 +198,7 @@ def titleQuery(cls, titleText):
     def authorQuery(cls, authorText):
         workAgentQuery = Q('bool',
             must=[
-                Q('query_string', query=authorText, fields=['agents.name'], default_operator='and'),
+                Q('query_string', query=authorText, fields=['agents.name^2'], default_operator='and'),
                 Q('terms', agents__roles=cls.ROLE_ALLOWLIST)
             ]
         )
@@ -276,6 +275,9 @@ def addSortClause(self, sortParams, reverse=False):
 
                 sortValues.append(self.dateSort)
 
+        if len(sortValues) < 1:
+            sortValues.append({'_score': 'desc'})
+
         sortValues.append({'uuid': 'asc' if reverse is False else 'desc'})
 
         self.query = self.query.sort(*sortValues)

task-definition.json

@@ -26,7 +26,7 @@
                 },
                 {
                     "name": "ELASTICSEARCH_HOST",
-                    "value": "https://vpc-sfr-search-api-production-tom4zg45o45pfomgtasgskvx4e.us-east-1.es.amazonaws.com"
+                    "value": "https://vpc-drb-search-production-5ptfzwisshcadbbaph35dqilva.us-east-1.es.amazonaws.com"
                 },
                 {
                     "name": "ELASTICSEARCH_INDEX",

tests/unit/test_api_es.py

@@ -429,7 +429,7 @@ def test_makeDictHashable(self, mocker):
         )
 
     def test_escapeSearchQuery_changed(self):
-        assert ElasticClient.escapeSearchQuery('[test]+a:thing!') == '\[test\]\+a\:thing\!'
+        assert ElasticClient.escapeSearchQuery('/[test]+a:thing!') == '\/\[test\]\+a\:thing\!'
 
     def test_escapeSearchQuery_unchanged(self):
         assert ElasticClient.escapeSearchQuery('a simple query') == 'a simple query'
@@ -439,7 +439,7 @@ def test_titleQuery(self):
         testQuery = testQueryES.to_dict()
 
         assert testQuery['bool']['should'][0]['query_string']['query'] == 'testTitle'
-        assert testQuery['bool']['should'][0]['query_string']['fields'] == ['title', 'alt_titles']
+        assert testQuery['bool']['should'][0]['query_string']['fields'] == ['title^3', 'alt_titles']
         assert testQuery['bool']['should'][1]['nested']['path'] == 'editions'
         assert testQuery['bool']['should'][1]['nested']['query']['query_string']['query'] == 'testTitle'
         assert testQuery['bool']['should'][1]['nested']['query']['query_string']['fields'] == ['editions.title']
@@ -451,7 +451,7 @@ def test_authorQuery(self):
 
         assert testQuery['bool']['should'][0]['nested']['path'] == 'agents'
         assert testQuery['bool']['should'][0]['nested']['query']['bool']['must'][0]['query_string']['query'] == 'testAuthor'
-        assert testQuery['bool']['should'][0]['nested']['query']['bool']['must'][0]['query_string']['fields'] == ['agents.name']
+        assert testQuery['bool']['should'][0]['nested']['query']['bool']['must'][0]['query_string']['fields'] == ['agents.name^2']
         assert testQuery['bool']['should'][0]['nested']['query']['bool']['must'][1]['terms']['agents.roles'] == ElasticClient.ROLE_ALLOWLIST
         assert testQuery['bool']['should'][1]['nested']['path'] == 'editions.agents'
         assert testQuery['bool']['should'][1]['nested']['query']['bool']['must'][0]['query_string']['query'] == 'testAuthor'
@@ -586,7 +586,7 @@ def test_addSortClause_root_sort_only(self, testInstance, mocker):
         testInstance.addSortClause([])
 
         assert testInstance.query == 'sortQuery'
-        mockQuery.sort.assert_called_once_with({'uuid': 'asc'})
+        mockQuery.sort.assert_called_once_with({'_score': 'desc'}, {'uuid': 'asc'})
 
     def test_addSortClause_reverse_true(self, testInstance, mocker):
         mockQuery = mocker.MagicMock()

tests/unit/test_api_utils_blueprint.py

@@ -19,6 +19,7 @@ def mockUtils(self, mocker):
     def testApp(self):
         flaskApp = Flask('test')
         flaskApp.config['DB_CLIENT'] = 'testDBClient'
+        flaskApp.config['REDIS_CLIENT'] = 'testRedisClient'
 
         return flaskApp
 
@@ -38,7 +39,7 @@ def test_languageCounts(self, mockUtils, testApp, mocker):
             testAPIResponse = languageCounts()
 
             assert testAPIResponse == 'languageListResponse'
-            mockESClient.assert_called_once()
+            mockESClient.assert_called_once_with('testRedisClient')
 
             mockUtils['normalizeQueryParams'].assert_called_once()
             mockES.languageQuery.assert_called_once_with(True)