From 159858a962d26c8180f0e6c8fea79154b31afed9 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Thu, 25 Jan 2024 20:38:58 -0500 Subject: [PATCH 1/8] Replace links with fulfill links in item dict and single link dict --- CHANGELOG.md | 1 + api/utils.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3672337095..61c6c71ad8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Added New /fulfill endpoint with ability to check for NYPL login in Bearer authorization header Fulfill endpoint returns pre-signed URLs for objects in private buckets when user is logged in +JSON from all endpoints (search, OPDS search, etc) includes fulfill URL when object requires NYPL login ## unreleased version -- v0.12.4 ## Added diff --git a/api/utils.py b/api/utils.py index c918bfe04f..c6c08d9802 100644 --- a/api/utils.py +++ b/api/utils.py @@ -336,6 +336,9 @@ def formatEdition( 'flags': flags }) + # Replace item links with fulfillment links where necessary + itemDict['links'] = list(map(APIUtils.replacePrivateLinkUrl, itemDict['links'])) + itemDict['links'].sort(key=cls.sortByMediaType) itemDict['rights'] = [ @@ -446,6 +449,9 @@ def formatLinkOutput(cls, link): linkDict['work']['editions'] = [linkEdition] linkDict['work']['editions'][0]['items'] = [linkItem] + # Amend link to include /fulfill link if appropriate + linkDict = APIUtils.replacePrivateLinkUrl(linkDict) + return linkDict @classmethod @@ -570,3 +576,15 @@ def getPresignedUrlFromObjectUrl(s3Client, url): {'Bucket': bucketName,'Key': objectKey}, timeValid ) + + @staticmethod + def replacePrivateLinkUrl(link): + """ + Given a link object, return a link object with the url replaced if + the link has flags indicating it should be fulfilled via /fulfill + """ + if link['flags'].get("edd") or not link['flags'].get("nypl_login"): + return link + else: + link['url'] = "/fulfill/" + str(link['link_id']) + return link From baad89b65d0d6afc57f3a1bda1dfb84034fb94b1 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Tue, 30 Jan 2024 23:48:10 -0500 Subject: [PATCH 2/8] Utilize request and pass it all the way to utility operations --- api/blueprints/drbEdition.py | 4 ++-- api/blueprints/drbLink.py | 4 ++-- api/blueprints/drbSearch.py | 2 +- api/blueprints/drbWork.py | 6 +++--- api/utils.py | 29 +++++++++++++++++++---------- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/api/blueprints/drbEdition.py b/api/blueprints/drbEdition.py index 587af41499..9229adefa5 100644 --- a/api/blueprints/drbEdition.py +++ b/api/blueprints/drbEdition.py @@ -27,13 +27,13 @@ def editionFetch(editionID): filteredFormats = APIUtils.formatFilters(terms) - edition = dbClient.fetchSingleEdition(editionID) + edition = dbClient.fetchSingleEdition(editionID) if edition: statusCode = 200 records = dbClient.fetchRecordsByUUID(edition.dcdw_uuids) responseBody = APIUtils.formatEditionOutput( - edition, records=records, dbClient=dbClient, showAll=showAll, formats=filteredFormats, reader=readerVersion + edition, request=request, records=records, dbClient=dbClient, showAll=showAll, formats=filteredFormats, reader=readerVersion ) else: statusCode = 404 diff --git a/api/blueprints/drbLink.py b/api/blueprints/drbLink.py index 02e361876b..5c55c091d5 100644 --- a/api/blueprints/drbLink.py +++ b/api/blueprints/drbLink.py @@ -1,4 +1,4 @@ -from flask import Blueprint, current_app +from flask import Blueprint, current_app, request from ..db import DBClient from ..utils import APIUtils from logger import createLog @@ -18,7 +18,7 @@ def linkFetch(linkID): if link: statusCode = 200 - responseObject = APIUtils.formatLinkOutput(link) + responseObject = APIUtils.formatLinkOutput(link, request=request) else: statusCode = 404 responseObject = {'message': 'Unable to locate link #{}'.format(linkID)} diff --git a/api/blueprints/drbSearch.py b/api/blueprints/drbSearch.py index e18b4b54ef..7bc8b70b5f 100644 --- a/api/blueprints/drbSearch.py +++ b/api/blueprints/drbSearch.py @@ -77,7 +77,7 @@ def standardQuery(): dataBlock = { 'totalWorks': searchResult.hits.total.value, 'works': APIUtils.formatWorkOutput( - works, results, dbClient=dbClient, formats=filteredFormats, reader=readerVersion + works, results, request=request, dbClient=dbClient, formats=filteredFormats, reader=readerVersion ), 'paging': paging, 'facets': facets diff --git a/api/blueprints/drbWork.py b/api/blueprints/drbWork.py index 46de1fa1d2..0ff0231f86 100644 --- a/api/blueprints/drbWork.py +++ b/api/blueprints/drbWork.py @@ -20,7 +20,7 @@ def workFetch(uuid): terms = {} for param in ['filter']: terms[param] = APIUtils.extractParamPairs(param, searchParams) - + showAll = searchParams.get('showAll', ['true'])[0].lower() != 'false' readerVersion = searchParams.get('readerVersion', [None])[0]\ or current_app.config['READER_VERSION'] @@ -30,8 +30,8 @@ def workFetch(uuid): work = dbClient.fetchSingleWork(uuid) if work: statusCode = 200 - responseBody = APIUtils.formatWorkOutput(work, None, showAll=showAll, dbClient=dbClient, formats=filteredFormats, reader=readerVersion) - + responseBody = APIUtils.formatWorkOutput(work, None, showAll=showAll, request=request, dbClient=dbClient, formats=filteredFormats, reader=readerVersion) + else: statusCode = 404 responseBody = { diff --git a/api/utils.py b/api/utils.py index c6c08d9802..3d83550c3c 100644 --- a/api/utils.py +++ b/api/utils.py @@ -2,6 +2,7 @@ from datetime import datetime from hashlib import scrypt from flask import jsonify +from itertools import repeat from math import ceil from model import Collection, Edition import re @@ -139,7 +140,7 @@ def formatPagingOptions(page, pageSize, totalHits): @classmethod def formatWorkOutput( - cls, works, identifiers, dbClient, showAll=True, formats=None, reader=None + cls, works, identifiers, dbClient, request, showAll=True, formats=None, reader=None, ): #Multiple formatted works with formats specified if isinstance(works, list): @@ -232,16 +233,27 @@ def addWorkMeta(cls, work, **kwargs): @classmethod def formatEditionOutput( - cls, edition, records=None, dbClient=None, showAll=False, formats=None, reader=None + cls, edition, request, records=None, dbClient=None, showAll=False, formats=None, reader=None ): editionWorkTitle = edition.work.title editionWorkAuthors = edition.work.authors editionInCollection = cls.checkEditionInCollection(None, edition, dbClient) - return cls.formatEdition( + # Replace item links with fulfillment links where necessary + + #edition['links'] = list(map(APIUtils.replacePrivateLinkUrl, edition['links'], repeat(request))) + + formattedEdition = cls.formatEdition( edition, editionWorkTitle, editionWorkAuthors, editionInCollection, records, formats, showAll=showAll, reader=reader ) + for instance in formattedEdition['instances']: + for item in instance['items']: + item['links']= list(map(APIUtils.replacePrivateLinkUrl, item['links'], repeat(request))) + + return formattedEdition + + @classmethod def checkEditionInCollection(cls, work, edition, dbClient): @@ -336,9 +348,6 @@ def formatEdition( 'flags': flags }) - # Replace item links with fulfillment links where necessary - itemDict['links'] = list(map(APIUtils.replacePrivateLinkUrl, itemDict['links'])) - itemDict['links'].sort(key=cls.sortByMediaType) itemDict['rights'] = [ @@ -432,7 +441,7 @@ def formatRecord(cls, record, itemsByLink): return outRecord @classmethod - def formatLinkOutput(cls, link): + def formatLinkOutput(cls, link, request): linkItem = dict(link.items[0]) linkItem['item_id'] = link.items[0].id @@ -450,7 +459,7 @@ def formatLinkOutput(cls, link): linkDict['work']['editions'][0]['items'] = [linkItem] # Amend link to include /fulfill link if appropriate - linkDict = APIUtils.replacePrivateLinkUrl(linkDict) + linkDict = APIUtils.replacePrivateLinkUrl(linkDict, request) return linkDict @@ -578,7 +587,7 @@ def getPresignedUrlFromObjectUrl(s3Client, url): ) @staticmethod - def replacePrivateLinkUrl(link): + def replacePrivateLinkUrl(link, request): """ Given a link object, return a link object with the url replaced if the link has flags indicating it should be fulfilled via /fulfill @@ -586,5 +595,5 @@ def replacePrivateLinkUrl(link): if link['flags'].get("edd") or not link['flags'].get("nypl_login"): return link else: - link['url'] = "/fulfill/" + str(link['link_id']) + link['url'] = request.host + "/fulfill/" + str(link['link_id']) return link From 1c7b89414b83a365147c7ea8dc35356469f5d696 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Wed, 31 Jan 2024 09:41:44 -0500 Subject: [PATCH 3/8] Fix edition output and API utility tests --- api/utils.py | 11 +++++------ tests/unit/test_api_utils.py | 23 ++++++++++++++--------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/api/utils.py b/api/utils.py index 3d83550c3c..86f68567c2 100644 --- a/api/utils.py +++ b/api/utils.py @@ -239,16 +239,14 @@ def formatEditionOutput( editionWorkAuthors = edition.work.authors editionInCollection = cls.checkEditionInCollection(None, edition, dbClient) - # Replace item links with fulfillment links where necessary - - #edition['links'] = list(map(APIUtils.replacePrivateLinkUrl, edition['links'], repeat(request))) - formattedEdition = cls.formatEdition( edition, editionWorkTitle, editionWorkAuthors, editionInCollection, records, formats, showAll=showAll, reader=reader ) - for instance in formattedEdition['instances']: + if formattedEdition.get("instances"): + for instance in formattedEdition['instances']: for item in instance['items']: + # Map over item links and patch with pre-signed URL where necessary item['links']= list(map(APIUtils.replacePrivateLinkUrl, item['links'], repeat(request))) return formattedEdition @@ -559,7 +557,8 @@ def generate_presigned_url(s3_client, client_method, method_parameters, expires_ def getPresignedUrlFromObjectUrl(s3Client, url): """ Given the URL of an S3 resource, generate a presigned Amazon S3 URL - that can be used to access that resource. + that can be used to access that resource. This function assumes S3 + URLs in the "virtual hosted bucket" style, not deprecated path style. :param s3_client: A Boto3 Amazon S3 client :param url: The URL of the desired resource diff --git a/tests/unit/test_api_utils.py b/tests/unit/test_api_utils.py index f22a40249e..6fe91b04c6 100644 --- a/tests/unit/test_api_utils.py +++ b/tests/unit/test_api_utils.py @@ -60,6 +60,10 @@ def __iter__(self): return MockDB + @pytest.fixture + def MockRequestObject(self, mocker): + return {"hostname": "localhost:5000"} + @pytest.fixture def testRecord(self, MockDBObject): return MockDBObject( @@ -255,7 +259,7 @@ def test_formatPagingOptions_next_null(self): 'lastPage': 6 } - def test_formatWorkOutput_single_work(self, mocker): + def test_formatWorkOutput_single_work(self, mocker, MockRequestObject): mockFormat = mocker.patch.object(APIUtils, 'formatWork') mockFormat.return_value = { 'uuid': 1, @@ -266,14 +270,14 @@ def test_formatWorkOutput_single_work(self, mocker): ] } - outWork = APIUtils.formatWorkOutput('testWork', None, mocker.sentinel.dbClient) + outWork = APIUtils.formatWorkOutput('testWork', None, mocker.sentinel.dbClient, request=MockRequestObject) assert outWork['uuid'] == 1 assert outWork['editions'][0]['id'] == 'ed3' assert outWork['editions'][2]['id'] == 'ed1' mockFormat.assert_called_once_with('testWork', None, True, mocker.sentinel.dbClient, reader=None) - def test_formatWorkOutput_multiple_works(self, mocker): + def test_formatWorkOutput_multiple_works(self, mocker, MockRequestObject): mockFormat = mocker.patch.object(APIUtils, 'formatWork') mockFormat.side_effect = ['formattedWork1', 'formattedWork2'] @@ -291,6 +295,7 @@ def test_formatWorkOutput_multiple_works(self, mocker): ('uuid3', 3, 'highlight3') ], dbClient=mocker.sentinel.dbClient, + request=MockRequestObject ) assert outWorks == ['formattedWork1', 'formattedWork2'] @@ -381,9 +386,9 @@ def test_formatWork_ordered_editions(self, testWork, mocker): assert testWorkDict['editions'][0]['edition_id'] == 'ed2' assert testWorkDict['editions'][1]['edition_id'] == 'ed1' - def test_formatEditionOutput(self, mocker): + def test_formatEditionOutput(self, mocker, MockRequestObject): mockFormatEdition = mocker.patch.object(APIUtils, 'formatEdition') - mockFormatEdition.return_value = 'testEdition' + mockFormatEdition.return_value = {"testEdition": "test"} mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbEdition.DBClient') @@ -394,8 +399,8 @@ def test_formatEditionOutput(self, mocker): mockDB.fetchSingleEdition.return_value = mockEdition assert APIUtils.formatEditionOutput( - mockEdition, records = 'testRecords', dbClient=mockDBClient, showAll=True - ) == 'testEdition' + mockEdition, records = 'testRecords', dbClient=mockDBClient, request=MockRequestObject, showAll=True + ) == {"testEdition": "test"} mockFormatEdition.assert_called_once_with( mockEdition, mockEdition.work.title, mockEdition.work.authors, [], 'testRecords', None, showAll=True, reader=None @@ -519,12 +524,12 @@ def test_formatRecord(self, testRecord, mocker): {'item_id': 1, 'url': 'url1'}, {'item_id': 2, 'url': 'url2'} ] - def test_formatLinkOutput(self, testLink, testWork, testEdition, testItem): + def test_formatLinkOutput(self, testLink, testWork, testEdition, testItem, MockRequestObject): testEdition.work = testWork testItem.edition = testEdition testLink.items = [testItem] - testLink = APIUtils.formatLinkOutput(testLink) + testLink = APIUtils.formatLinkOutput(testLink, MockRequestObject) assert testLink['link_id'] == 'li1' assert testLink['work']['uuid'] == 'testUUID' From 5f80dfae9972ef2676ed09a7d03de7fdbd428ba7 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:27:03 -0500 Subject: [PATCH 4/8] Add stand-in Flask object to fix "called with" tests --- tests/helper.py | 11 +++++++++++ tests/unit/test_api_edition_blueprint.py | 14 +++++++++----- tests/unit/test_api_link_blueprint.py | 6 ++++-- tests/unit/test_api_search_blueprint.py | 7 +++++-- tests/unit/test_api_work_blueprint.py | 16 +++++++++++++--- 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/tests/helper.py b/tests/helper.py index 291646bae3..0705686905 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -1,5 +1,16 @@ import os +from flask import Request +# Because many Blueprint tests rely on checking to see if specific arguments were +# passed to output processing functions, and these output processing functions now +# require a request object to determine the host, this class provides an object that +# will compare "true" to any instantiated Flask request object, for contexts where a +# valid request object must be passed in but none of its content or attributes matter. +class AnyFlaskRequest: + def __eq__(self, __value: object) -> bool: + if isinstance(__value, Request): # any Flask request will compare true + return True + return False class TestHelpers: ENV_VARS = { diff --git a/tests/unit/test_api_edition_blueprint.py b/tests/unit/test_api_edition_blueprint.py index 65f1bdcdc7..ea15f76db6 100644 --- a/tests/unit/test_api_edition_blueprint.py +++ b/tests/unit/test_api_edition_blueprint.py @@ -1,9 +1,9 @@ -from flask import Flask +from flask import Flask, Request import pytest from api.blueprints.drbEdition import editionFetch from api.utils import APIUtils - +from tests.helper import AnyFlaskRequest class TestEditionBlueprint: @pytest.fixture @@ -28,6 +28,7 @@ def test_editionFetch_success_noFormat(self, mockUtils, testApp, mocker): mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbEdition.DBClient') mockDBClient.return_value = mockDB + someFlaskRequest = AnyFlaskRequest() mockUtils['normalizeQueryParams'].return_value = {'showAll': ['true']} @@ -48,7 +49,8 @@ def test_editionFetch_success_noFormat(self, mockUtils, testApp, mocker): mockUtils['normalizeQueryParams'].assert_called_once() mockUtils['formatEditionOutput'].assert_called_once_with( - mockEdition, records='testRecords', dbClient=mockDB, showAll=True, formats=[], reader='test' + mockEdition, records='testRecords', dbClient=mockDB, showAll=True, formats=[], + reader='test', request=someFlaskRequest ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleEdition', 'testEdition' @@ -58,6 +60,7 @@ def test_editionFetch_success_format(self, mockUtils, testApp, mocker): mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbEdition.DBClient') mockDBClient.return_value = mockDB + someFlaskRequest = AnyFlaskRequest() queryParams = {'showAll': ['true']} mockUtils['normalizeQueryParams'].return_value = {'showAll': ['true']} @@ -86,8 +89,9 @@ def test_editionFetch_success_format(self, mockUtils, testApp, mocker): mocker.call('filter', queryParams) ]) mockUtils['formatEditionOutput'].assert_called_once_with( - mockEdition, records='testRecords', showAll=True, dbClient=mockDB, - formats=['application/html+edd', 'application/x.html+edd'], reader='test' + mockEdition, records='testRecords', showAll=True, + dbClient=mockDB,request=someFlaskRequest, + formats=['application/html+edd', 'application/x.html+edd'], reader='test' ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleEdition', 'testEdition' diff --git a/tests/unit/test_api_link_blueprint.py b/tests/unit/test_api_link_blueprint.py index f948f337e8..60b17aa081 100644 --- a/tests/unit/test_api_link_blueprint.py +++ b/tests/unit/test_api_link_blueprint.py @@ -1,8 +1,9 @@ -from flask import Flask +from flask import Flask, Request import pytest from api.blueprints.drbLink import linkFetch from api.utils import APIUtils +from tests.helper import AnyFlaskRequest class TestLinkBlueprint: @pytest.fixture @@ -24,6 +25,7 @@ def test_linkFetch_success(self, mockUtils, testApp, mocker): mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbLink.DBClient') mockDBClient.return_value = mockDB + someFlaskRequest = AnyFlaskRequest() mockDB.fetchSingleLink.return_value = 'dbLinkRecord' @@ -36,7 +38,7 @@ def test_linkFetch_success(self, mockUtils, testApp, mocker): assert testAPIResponse == 'singleLinkResponse' mockDBClient.assert_called_once_with('testDBClient') - mockUtils['formatLinkOutput'].assert_called_once_with('dbLinkRecord') + mockUtils['formatLinkOutput'].assert_called_once_with('dbLinkRecord', request=someFlaskRequest) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleLink', 'testLink' ) diff --git a/tests/unit/test_api_search_blueprint.py b/tests/unit/test_api_search_blueprint.py index 4d2cd920f3..b5c2a079a1 100644 --- a/tests/unit/test_api_search_blueprint.py +++ b/tests/unit/test_api_search_blueprint.py @@ -4,7 +4,7 @@ from api.blueprints.drbSearch import standardQuery from api.utils import APIUtils from api.elastic import ElasticClientError - +from tests.helper import AnyFlaskRequest class TestSearchBlueprint: @pytest.fixture @@ -100,6 +100,8 @@ def test_standardQuery(self, mockUtils, mockHits, testApp, mocker): mockUtils['formatResponseObject'].return_value = 'mockAPIResponse' + someFlaskRequest = AnyFlaskRequest() + with testApp.test_request_context('/?testing=true'): testAPIResponse = standardQuery() @@ -139,7 +141,8 @@ def test_standardQuery(self, mockUtils, mockHits, testApp, mocker): testResultIds, dbClient=mockDB, formats=['text/html'], - reader='test' + reader='test', + request=someFlaskRequest ) mockUtils['formatResponseObject'].assert_called_once_with( diff --git a/tests/unit/test_api_work_blueprint.py b/tests/unit/test_api_work_blueprint.py index 386f9df317..b45cdaf51f 100644 --- a/tests/unit/test_api_work_blueprint.py +++ b/tests/unit/test_api_work_blueprint.py @@ -3,6 +3,7 @@ from api.blueprints.drbWork import workFetch from api.utils import APIUtils +from tests.helper import AnyFlaskRequest class TestWorkBlueprint: @@ -36,6 +37,8 @@ def test_workFetch_success_showAll_true(self, mockUtils, testApp, mocker): mockUtils['formatWorkOutput'].return_value = 'testWork' mockUtils['formatResponseObject'].return_value = 'singleWorkResponse' + someFlaskRequest = AnyFlaskRequest() + with testApp.test_request_context('/?showAll=true'): testAPIResponse = workFetch('testUUID') @@ -44,7 +47,8 @@ def test_workFetch_success_showAll_true(self, mockUtils, testApp, mocker): mockUtils['normalizeQueryParams'].assert_called_once mockUtils['formatWorkOutput'].assert_called_once_with( - 'dbWorkRecord', None, showAll=True, dbClient=mockDB, formats=[], reader='test' + 'dbWorkRecord', None, showAll=True, dbClient=mockDB, formats=[], + reader='test', request=someFlaskRequest ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleWork', 'testWork' @@ -67,6 +71,8 @@ def test_workFetch_success_format(self, mockUtils, testApp, mocker): mockUtils['formatWorkOutput'].return_value = 'testWork' mockUtils['formatResponseObject'].return_value = 'singleWorkResponse' + someFlaskRequest = AnyFlaskRequest() + with testApp.test_request_context('/?showAll=true'): testAPIResponse = workFetch('testUUID') @@ -78,7 +84,8 @@ def test_workFetch_success_format(self, mockUtils, testApp, mocker): mocker.call('filter', queryParams) ]) mockUtils['formatWorkOutput'].assert_called_once_with( - 'dbWorkRecord', None, showAll=True, dbClient=mockDB, formats=['application/html+edd', 'application/x.html+edd'], reader='test' + 'dbWorkRecord', None, showAll=True, dbClient=mockDB, formats=['application/html+edd', + 'application/x.html+edd'], reader='test', request=someFlaskRequest ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleWork', 'testWork' @@ -96,6 +103,8 @@ def test_workFetch_success_showAll_false(self, mockUtils, testApp, mocker): mockUtils['formatWorkOutput'].return_value = 'testWork' mockUtils['formatResponseObject'].return_value = 'singleWorkResponse' + someFlaskRequest = AnyFlaskRequest() + with testApp.test_request_context('/?showAll=false'): testAPIResponse = workFetch('testUUID') @@ -104,7 +113,8 @@ def test_workFetch_success_showAll_false(self, mockUtils, testApp, mocker): mockUtils['normalizeQueryParams'].assert_called_once mockUtils['formatWorkOutput'].assert_called_once_with( - 'dbWorkRecord', None, showAll=False, dbClient=mockDB, formats=[], reader='test' + 'dbWorkRecord', None, showAll=False, dbClient=mockDB, formats=[], reader='test', + request=someFlaskRequest ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleWork', 'testWork' From c7849f888acfe69b8b80d3b184cf0ed84e1b5de5 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:31:22 -0500 Subject: [PATCH 5/8] Remove unused imports after refactor --- tests/unit/test_api_edition_blueprint.py | 2 +- tests/unit/test_api_link_blueprint.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_api_edition_blueprint.py b/tests/unit/test_api_edition_blueprint.py index ea15f76db6..b550d11646 100644 --- a/tests/unit/test_api_edition_blueprint.py +++ b/tests/unit/test_api_edition_blueprint.py @@ -1,4 +1,4 @@ -from flask import Flask, Request +from flask import Flask import pytest from api.blueprints.drbEdition import editionFetch diff --git a/tests/unit/test_api_link_blueprint.py b/tests/unit/test_api_link_blueprint.py index 60b17aa081..c349bb80fe 100644 --- a/tests/unit/test_api_link_blueprint.py +++ b/tests/unit/test_api_link_blueprint.py @@ -1,4 +1,4 @@ -from flask import Flask, Request +from flask import Flask import pytest from api.blueprints.drbLink import linkFetch From 3b8d4409e4d58ac6ecb869ba61132f6f73cc3486 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:53:34 -0500 Subject: [PATCH 6/8] Fix lengthy lines --- api/blueprints/drbWork.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/blueprints/drbWork.py b/api/blueprints/drbWork.py index 0ff0231f86..3551aa77f0 100644 --- a/api/blueprints/drbWork.py +++ b/api/blueprints/drbWork.py @@ -30,7 +30,9 @@ def workFetch(uuid): work = dbClient.fetchSingleWork(uuid) if work: statusCode = 200 - responseBody = APIUtils.formatWorkOutput(work, None, showAll=showAll, request=request, dbClient=dbClient, formats=filteredFormats, reader=readerVersion) + responseBody = APIUtils.formatWorkOutput(work, None, showAll=showAll, + request=request, dbClient=dbClient, formats=filteredFormats, reader=readerVersion + ) else: statusCode = 404 From 33842a81599f19eb312400073716fe46a7aacabc Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Wed, 31 Jan 2024 23:11:10 -0500 Subject: [PATCH 7/8] Remove unneeded mock and replace with request from request context --- tests/unit/test_api_utils.py | 91 +++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/tests/unit/test_api_utils.py b/tests/unit/test_api_utils.py index 6fe91b04c6..e79e7268f6 100644 --- a/tests/unit/test_api_utils.py +++ b/tests/unit/test_api_utils.py @@ -1,12 +1,16 @@ from hashlib import scrypt import pytest from random import shuffle -from flask import Flask +from flask import Flask, request from api.utils import APIUtils from datetime import datetime class TestAPIUtils: + @pytest.fixture + def testApp(self): + return Flask('test') + @pytest.fixture def testAggregationResp(self): return { @@ -60,10 +64,6 @@ def __iter__(self): return MockDB - @pytest.fixture - def MockRequestObject(self, mocker): - return {"hostname": "localhost:5000"} - @pytest.fixture def testRecord(self, MockDBObject): return MockDBObject( @@ -259,7 +259,7 @@ def test_formatPagingOptions_next_null(self): 'lastPage': 6 } - def test_formatWorkOutput_single_work(self, mocker, MockRequestObject): + def test_formatWorkOutput_single_work(self, mocker, testApp): mockFormat = mocker.patch.object(APIUtils, 'formatWork') mockFormat.return_value = { 'uuid': 1, @@ -270,14 +270,15 @@ def test_formatWorkOutput_single_work(self, mocker, MockRequestObject): ] } - outWork = APIUtils.formatWorkOutput('testWork', None, mocker.sentinel.dbClient, request=MockRequestObject) + with testApp.test_request_context('/'): + outWork = APIUtils.formatWorkOutput('testWork', None, mocker.sentinel.dbClient, request=request) assert outWork['uuid'] == 1 assert outWork['editions'][0]['id'] == 'ed3' assert outWork['editions'][2]['id'] == 'ed1' mockFormat.assert_called_once_with('testWork', None, True, mocker.sentinel.dbClient, reader=None) - def test_formatWorkOutput_multiple_works(self, mocker, MockRequestObject): + def test_formatWorkOutput_multiple_works(self, mocker, testApp): mockFormat = mocker.patch.object(APIUtils, 'formatWork') mockFormat.side_effect = ['formattedWork1', 'formattedWork2'] @@ -287,16 +288,17 @@ def test_formatWorkOutput_multiple_works(self, mocker, MockRequestObject): mocker.MagicMock(uuid='uuid1'), mocker.MagicMock(uuid='uuid2') ] - outWorks = APIUtils.formatWorkOutput( - testWorks, - [ - ('uuid1', 1, 'highlight1'), - ('uuid2', 2, 'highlight2'), - ('uuid3', 3, 'highlight3') - ], - dbClient=mocker.sentinel.dbClient, - request=MockRequestObject - ) + with testApp.test_request_context('/'): + outWorks = APIUtils.formatWorkOutput( + testWorks, + [ + ('uuid1', 1, 'highlight1'), + ('uuid2', 2, 'highlight2'), + ('uuid3', 3, 'highlight3') + ], + dbClient=mocker.sentinel.dbClient, + request=request + ) assert outWorks == ['formattedWork1', 'formattedWork2'] mockFormat.assert_has_calls([ @@ -386,7 +388,7 @@ def test_formatWork_ordered_editions(self, testWork, mocker): assert testWorkDict['editions'][0]['edition_id'] == 'ed2' assert testWorkDict['editions'][1]['edition_id'] == 'ed1' - def test_formatEditionOutput(self, mocker, MockRequestObject): + def test_formatEditionOutput(self, mocker, testApp): mockFormatEdition = mocker.patch.object(APIUtils, 'formatEdition') mockFormatEdition.return_value = {"testEdition": "test"} @@ -398,9 +400,14 @@ def test_formatEditionOutput(self, mocker, MockRequestObject): mockDB.fetchSingleEdition.return_value = mockEdition - assert APIUtils.formatEditionOutput( - mockEdition, records = 'testRecords', dbClient=mockDBClient, request=MockRequestObject, showAll=True - ) == {"testEdition": "test"} + with testApp.test_request_context('/'): + assert APIUtils.formatEditionOutput( + mockEdition, + records = 'testRecords', + dbClient=mockDBClient, + request=request, + showAll=True + ) == {"testEdition": "test"} mockFormatEdition.assert_called_once_with( mockEdition, mockEdition.work.title, mockEdition.work.authors, [], 'testRecords', None, showAll=True, reader=None @@ -524,12 +531,13 @@ def test_formatRecord(self, testRecord, mocker): {'item_id': 1, 'url': 'url1'}, {'item_id': 2, 'url': 'url2'} ] - def test_formatLinkOutput(self, testLink, testWork, testEdition, testItem, MockRequestObject): + def test_formatLinkOutput(self, testLink, testWork, testEdition, testItem, testApp): testEdition.work = testWork testItem.edition = testEdition testLink.items = [testItem] - testLink = APIUtils.formatLinkOutput(testLink, MockRequestObject) + with testApp.test_request_context('/'): + testLink = APIUtils.formatLinkOutput(testLink, request) assert testLink['link_id'] == 'li1' assert testLink['work']['uuid'] == 'testUUID' @@ -579,8 +587,7 @@ def test_flatten_nested(self): assert flatArray == [1, 2, 3, 4, 5] - def test_formatResponseObject(self, mocker): - testApp = Flask('test') + def test_formatResponseObject(self, mocker, testApp): with testApp.test_request_context('/'): mockDatetime = mocker.patch('api.utils.datetime') mockDatetime.utcnow.return_value = 'presentTimestamp' @@ -661,3 +668,35 @@ def test_getPresignedUrlFromNons3Url(self): with pytest.raises(ValueError): APIUtils.getPresignedUrlFromObjectUrl({"Some Client"}, "https://example.com") + def test_ReplaceWithPrivateLink(self): + testApp = Flask('test') + with testApp.test_request_context('/', base_url="http://localhost:5000"): + testLoginLinkObj = { + 'link_id':'12345', + 'media_type':'application/pdf', + 'url':'https://doc-example-bucket1.s3.us-west-2.amazonaws.com/puppy.pdf', + 'flags':{'nypl_login': True} + } + assert APIUtils.replacePrivateLinkUrl(testLoginLinkObj, request) == { + 'link_id':'12345', + 'media_type' : 'application/pdf', + 'url':'localhost:5000/fulfill/12345', + 'flags':{'nypl_login': True} + } + + def test_noLinkReplacement(self): + testElectronicDeliveryLink = { + 'link_id':'6789', + 'media_type' : "application/html+edd", + 'flags' : { + "catalog": False, + "download": False, + "edd": True, + "reader": False + } + } + testApp = Flask('test') + with testApp.test_request_context('/'): + assert APIUtils.replacePrivateLinkUrl( + testElectronicDeliveryLink, request + ) == testElectronicDeliveryLink From 53208864f6daa9ddeccce454027c6da1202e9358 Mon Sep 17 00:00:00 2001 From: Lyndsey Jane Moulds <2042238+Apophenia@users.noreply.github.com> Date: Thu, 1 Feb 2024 08:36:26 -0500 Subject: [PATCH 8/8] Remove Flask request helper object and utilize with_context feature --- tests/helper.py | 12 ------------ tests/unit/test_api_edition_blueprint.py | 11 ++++------- tests/unit/test_api_link_blueprint.py | 6 ++---- tests/unit/test_api_search_blueprint.py | 7 ++----- tests/unit/test_api_work_blueprint.py | 15 ++++----------- 5 files changed, 12 insertions(+), 39 deletions(-) diff --git a/tests/helper.py b/tests/helper.py index 0705686905..3499c7385e 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -1,16 +1,4 @@ import os -from flask import Request - -# Because many Blueprint tests rely on checking to see if specific arguments were -# passed to output processing functions, and these output processing functions now -# require a request object to determine the host, this class provides an object that -# will compare "true" to any instantiated Flask request object, for contexts where a -# valid request object must be passed in but none of its content or attributes matter. -class AnyFlaskRequest: - def __eq__(self, __value: object) -> bool: - if isinstance(__value, Request): # any Flask request will compare true - return True - return False class TestHelpers: ENV_VARS = { diff --git a/tests/unit/test_api_edition_blueprint.py b/tests/unit/test_api_edition_blueprint.py index b550d11646..3f05bff918 100644 --- a/tests/unit/test_api_edition_blueprint.py +++ b/tests/unit/test_api_edition_blueprint.py @@ -1,9 +1,8 @@ -from flask import Flask +from flask import Flask, request import pytest from api.blueprints.drbEdition import editionFetch from api.utils import APIUtils -from tests.helper import AnyFlaskRequest class TestEditionBlueprint: @pytest.fixture @@ -15,7 +14,7 @@ def mockUtils(self, mocker): formatResponseObject=mocker.DEFAULT, formatEditionOutput=mocker.DEFAULT ) - + @pytest.fixture def testApp(self): flaskApp = Flask('test') @@ -28,7 +27,6 @@ def test_editionFetch_success_noFormat(self, mockUtils, testApp, mocker): mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbEdition.DBClient') mockDBClient.return_value = mockDB - someFlaskRequest = AnyFlaskRequest() mockUtils['normalizeQueryParams'].return_value = {'showAll': ['true']} @@ -50,7 +48,7 @@ def test_editionFetch_success_noFormat(self, mockUtils, testApp, mocker): mockUtils['normalizeQueryParams'].assert_called_once() mockUtils['formatEditionOutput'].assert_called_once_with( mockEdition, records='testRecords', dbClient=mockDB, showAll=True, formats=[], - reader='test', request=someFlaskRequest + reader='test', request=request ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleEdition', 'testEdition' @@ -60,7 +58,6 @@ def test_editionFetch_success_format(self, mockUtils, testApp, mocker): mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbEdition.DBClient') mockDBClient.return_value = mockDB - someFlaskRequest = AnyFlaskRequest() queryParams = {'showAll': ['true']} mockUtils['normalizeQueryParams'].return_value = {'showAll': ['true']} @@ -90,7 +87,7 @@ def test_editionFetch_success_format(self, mockUtils, testApp, mocker): ]) mockUtils['formatEditionOutput'].assert_called_once_with( mockEdition, records='testRecords', showAll=True, - dbClient=mockDB,request=someFlaskRequest, + dbClient=mockDB,request=request, formats=['application/html+edd', 'application/x.html+edd'], reader='test' ) mockUtils['formatResponseObject'].assert_called_once_with( diff --git a/tests/unit/test_api_link_blueprint.py b/tests/unit/test_api_link_blueprint.py index c349bb80fe..4714a00711 100644 --- a/tests/unit/test_api_link_blueprint.py +++ b/tests/unit/test_api_link_blueprint.py @@ -1,9 +1,8 @@ -from flask import Flask +from flask import Flask, request import pytest from api.blueprints.drbLink import linkFetch from api.utils import APIUtils -from tests.helper import AnyFlaskRequest class TestLinkBlueprint: @pytest.fixture @@ -25,7 +24,6 @@ def test_linkFetch_success(self, mockUtils, testApp, mocker): mockDB = mocker.MagicMock() mockDBClient = mocker.patch('api.blueprints.drbLink.DBClient') mockDBClient.return_value = mockDB - someFlaskRequest = AnyFlaskRequest() mockDB.fetchSingleLink.return_value = 'dbLinkRecord' @@ -38,7 +36,7 @@ def test_linkFetch_success(self, mockUtils, testApp, mocker): assert testAPIResponse == 'singleLinkResponse' mockDBClient.assert_called_once_with('testDBClient') - mockUtils['formatLinkOutput'].assert_called_once_with('dbLinkRecord', request=someFlaskRequest) + mockUtils['formatLinkOutput'].assert_called_once_with('dbLinkRecord', request=request) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleLink', 'testLink' ) diff --git a/tests/unit/test_api_search_blueprint.py b/tests/unit/test_api_search_blueprint.py index b5c2a079a1..5a32ab5751 100644 --- a/tests/unit/test_api_search_blueprint.py +++ b/tests/unit/test_api_search_blueprint.py @@ -1,10 +1,9 @@ -from flask import Flask +from flask import Flask, request import pytest from api.blueprints.drbSearch import standardQuery from api.utils import APIUtils from api.elastic import ElasticClientError -from tests.helper import AnyFlaskRequest class TestSearchBlueprint: @pytest.fixture @@ -100,8 +99,6 @@ def test_standardQuery(self, mockUtils, mockHits, testApp, mocker): mockUtils['formatResponseObject'].return_value = 'mockAPIResponse' - someFlaskRequest = AnyFlaskRequest() - with testApp.test_request_context('/?testing=true'): testAPIResponse = standardQuery() @@ -142,7 +139,7 @@ def test_standardQuery(self, mockUtils, mockHits, testApp, mocker): dbClient=mockDB, formats=['text/html'], reader='test', - request=someFlaskRequest + request=request ) mockUtils['formatResponseObject'].assert_called_once_with( diff --git a/tests/unit/test_api_work_blueprint.py b/tests/unit/test_api_work_blueprint.py index b45cdaf51f..50b2fed45b 100644 --- a/tests/unit/test_api_work_blueprint.py +++ b/tests/unit/test_api_work_blueprint.py @@ -1,9 +1,8 @@ -from flask import Flask +from flask import Flask, request import pytest from api.blueprints.drbWork import workFetch from api.utils import APIUtils -from tests.helper import AnyFlaskRequest class TestWorkBlueprint: @@ -37,8 +36,6 @@ def test_workFetch_success_showAll_true(self, mockUtils, testApp, mocker): mockUtils['formatWorkOutput'].return_value = 'testWork' mockUtils['formatResponseObject'].return_value = 'singleWorkResponse' - someFlaskRequest = AnyFlaskRequest() - with testApp.test_request_context('/?showAll=true'): testAPIResponse = workFetch('testUUID') @@ -48,7 +45,7 @@ def test_workFetch_success_showAll_true(self, mockUtils, testApp, mocker): mockUtils['normalizeQueryParams'].assert_called_once mockUtils['formatWorkOutput'].assert_called_once_with( 'dbWorkRecord', None, showAll=True, dbClient=mockDB, formats=[], - reader='test', request=someFlaskRequest + reader='test', request=request ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleWork', 'testWork' @@ -71,8 +68,6 @@ def test_workFetch_success_format(self, mockUtils, testApp, mocker): mockUtils['formatWorkOutput'].return_value = 'testWork' mockUtils['formatResponseObject'].return_value = 'singleWorkResponse' - someFlaskRequest = AnyFlaskRequest() - with testApp.test_request_context('/?showAll=true'): testAPIResponse = workFetch('testUUID') @@ -85,7 +80,7 @@ def test_workFetch_success_format(self, mockUtils, testApp, mocker): ]) mockUtils['formatWorkOutput'].assert_called_once_with( 'dbWorkRecord', None, showAll=True, dbClient=mockDB, formats=['application/html+edd', - 'application/x.html+edd'], reader='test', request=someFlaskRequest + 'application/x.html+edd'], reader='test', request=request ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleWork', 'testWork' @@ -103,8 +98,6 @@ def test_workFetch_success_showAll_false(self, mockUtils, testApp, mocker): mockUtils['formatWorkOutput'].return_value = 'testWork' mockUtils['formatResponseObject'].return_value = 'singleWorkResponse' - someFlaskRequest = AnyFlaskRequest() - with testApp.test_request_context('/?showAll=false'): testAPIResponse = workFetch('testUUID') @@ -114,7 +107,7 @@ def test_workFetch_success_showAll_false(self, mockUtils, testApp, mocker): mockUtils['normalizeQueryParams'].assert_called_once mockUtils['formatWorkOutput'].assert_called_once_with( 'dbWorkRecord', None, showAll=False, dbClient=mockDB, formats=[], reader='test', - request=someFlaskRequest + request=request ) mockUtils['formatResponseObject'].assert_called_once_with( 200, 'singleWork', 'testWork'