From 4d9dcff78c9605eaa0ff50669bd2dd3ed38a3119 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Thu, 24 Oct 2024 09:59:05 -0400 Subject: [PATCH 01/31] test files --- .../test_file_1.txt | 1 + .../test_file_2.txt | 1 + .../test_file_3.txt | 1 + .../test_file_4.pdf | Bin 0 -> 1409 bytes .../test_file_5.pdf | Bin 0 -> 1409 bytes 5 files changed, 3 insertions(+) create mode 100644 api/tests/lib/opportunity_attachment_test_files/test_file_1.txt create mode 100644 api/tests/lib/opportunity_attachment_test_files/test_file_2.txt create mode 100644 api/tests/lib/opportunity_attachment_test_files/test_file_3.txt create mode 100644 api/tests/lib/opportunity_attachment_test_files/test_file_4.pdf create mode 100644 api/tests/lib/opportunity_attachment_test_files/test_file_5.pdf diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_1.txt b/api/tests/lib/opportunity_attachment_test_files/test_file_1.txt new file mode 100644 index 000000000..dbe9dba55 --- /dev/null +++ b/api/tests/lib/opportunity_attachment_test_files/test_file_1.txt @@ -0,0 +1 @@ +Hello, world \ No newline at end of file diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt b/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt new file mode 100644 index 000000000..dbe9dba55 --- /dev/null +++ b/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt @@ -0,0 +1 @@ +Hello, world \ No newline at end of file diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt b/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt new file mode 100644 index 000000000..dbe9dba55 --- /dev/null +++ b/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt @@ -0,0 +1 @@ +Hello, world \ No newline at end of file diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_4.pdf b/api/tests/lib/opportunity_attachment_test_files/test_file_4.pdf new file mode 100644 index 0000000000000000000000000000000000000000..4bb6fbe25130515781d467c2cf72c2aae6defdd1 GIT binary patch literal 1409 zcmb7E-BKDc6u!?>oYtYm5zX$xvQQBRL3GDI1RUFZ`d zP)6-I-C;=1$@%^Aon%^#=DNF)Cz;>Be*S?rpD1k(Sp<7ra?K1+04*R<@pR0kfuS*z z50u{A+-&HX?TAGiu^JN>98l3UsZ z&|Mj;M92Zqb0KRo6_1KzHs)Z1(#|w8cM}x4_>Zz(@C0KmrOGd*s^bxsokC#i0eZv} z!FJRw^iX!7=z0*8J-7tgV%qjsytixfR84ivQ<(K*i?t16gqBuuhZ_t)EikmpZ?TbZ z%m#dCzSie!2$xtE(6%xNB)ZeCW^xwN>Ip0wXrc2`0|hi)U}cYvRDxZerTkb)+*nn* zDD)nIIx}pf21_ygr5LrT87d7~CY8J!t7*Clw5~ZbLdk}$EZg|tIbOkaygaVHS6p+P z_xM}bS?5P)*tQQvq?x{hb$G@EbUNU7H%ccdWKqECUrU0RCexVzUlt1=^5L4t2C`F` zPIxT(f+v`=E6fPHno46gGmU1G338b>*(hDc+TQm9>$*Y@yk~kKu=XBUBDF0y?m?d6 zz67zQ)WWqXMh5FgQq(&2Ff92Ej&!!%93jAS0V5r9IWR+Toxlc44c@|-?6Jw|(AnK; z=A5&O{e1Uga{Q@#b$al1+r-tdmD@aDIdiYPyco82kG^cz&kF}(N0gnT>%A*4*X*oR z+I4>|&dJr!n^}i7qPKq7tGv1DHdY4W^4*sIG1n{RPHG!JsyL1Fr1QDF)x1v}C=uu3 zgQD-}edxo3E^!|I?u%!d7>-4U{?t6HF#nd6id%^B@ZE9*cr{li0j$= z5;lPJDxMbvCC}?85s$o}6fnliC0@=aj7KbAPO7h4T$0SvlQ($z$y;BCMBvCF&HnZJ qtr9{**;io4u1#>_l>s}(xbKMX9Pyo9H`8VYI8IuDWHP%aP4X9fqLGvU literal 0 HcmV?d00001 diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_5.pdf b/api/tests/lib/opportunity_attachment_test_files/test_file_5.pdf new file mode 100644 index 0000000000000000000000000000000000000000..adf79e600ee5910802cb5b5dd0dcfa245138c1e9 GIT binary patch literal 1409 zcmb7ETT>b_6n@XIIITmABbvQ%St{Zni0*hpz_BfAC%YRU77`{Ks`jCOpndCme``+= zlu{s(9$aIaDxG;1%`I{EjAL4 z*?{lN*ZO=7;S$RN+ExaEM0eWNOwK}DJ%L37Ep%RLAdjXCtnAT|O0dhblpiaJ8>>ne zh2A4jXNHZ`U@1mnDMoE-hDt-0NhR;bYMQPBt!vJVP_khw%QilEj+b{G&$p{zUUQuH z_*>Xn=SODPwhu+5nZAQ{c*X>DI^cIVN+&5~QNZe7OM;jt)0qEX77HKp;hM(=vQwE( zcr5yYCz!G;%m}-hN@F)Ojb@Vxa+x;SC|${Hc3&dhm7I#MQ8s+dN-6bFVyK3|qTLU$*P#`Gc?{%FfaC-j$bYc2+9w zdSNZj$<@!BS%)>Ew}r4*d2`imtPI5EyRE{r8X8(Hq qRtce@>?^Qi*Csge%77hX+;_xxj`+^5n`tux949SLGMU|zCix4r`H`#u literal 0 HcmV?d00001 From 7728a06658656581ee015ae949d7b644761bbc46 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Thu, 24 Oct 2024 10:00:24 -0400 Subject: [PATCH 02/31] update factory and seed local db --- api/tests/lib/seed_local_db.py | 29 +++++++++++++++++++++++++++- api/tests/src/db/models/factories.py | 13 ++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 6a2ab9d0a..831c513eb 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -1,7 +1,10 @@ import logging +import os +import pathlib import random - +import boto3 import click +from botocore.exceptions import ClientError from sqlalchemy import func import src.adapters.db as db @@ -13,9 +16,31 @@ from src.db.models.opportunity_models import Opportunity from src.db.models.transfer.topportunity_models import TransferTopportunity from src.util.local import error_if_not_local +from src.adapters.aws import S3Config, get_s3_client logger = logging.getLogger(__name__) +# from reportlab.lib.pagesizes import letter +# from reportlab.pdfgen import canvas + +TESTS_FOLDER = pathlib.Path(__file__).parent.resolve() + +def upload_opportunity_attachments_s3(): + s3_config = S3Config() + s3_client = get_s3_client( + s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") + ) + test_folder_path = TESTS_FOLDER / "opportunity_attachment_test_files" + for root, _, files in os.walk(test_folder_path): + for file in files: + file_path = os.path.join(root, file) + object_name = os.path.relpath(file_path, test_folder_path) + + try: + s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) + print(f'Successfully uploaded {file_path} to s3://{s3_config.s3_opportunity_bucket}/{object_name}') #log? + except ClientError as e: + print(f'Error uploading {file_path}: {e}') def _add_history( opps: list[Opportunity], @@ -173,6 +198,8 @@ def seed_local_db(iterations: int, include_history: bool) -> None: logger.info("Running seed script for local DB") error_if_not_local() + upload_opportunity_attachments_s3() + db_client = PostgresDBClient() with db_client.get_session() as db_session: diff --git a/api/tests/src/db/models/factories.py b/api/tests/src/db/models/factories.py index 4f64ae033..bcecbb02c 100644 --- a/api/tests/src/db/models/factories.py +++ b/api/tests/src/db/models/factories.py @@ -147,6 +147,14 @@ class CustomProvider(BaseProvider): YN_YESNO_BOOLEAN_VALUES = ["Y", "N", "Yes", "No"] + OPPORTUNITY_ATTACHMENT_S3_PATHS = [ + "s3://local-opportunities/test_file_1.txt", + "s3://local-opportunities/test_file_2.txt", + "s3://local-opportunities/test_file_3.txt", + "s3://local-opportunities/test_file_4.pdf", + "s3://local-opportunities/test_file_5.pdf", + ] + def agency(self) -> str: return self.random_element(self.AGENCIES) @@ -188,6 +196,9 @@ def yn_boolean(self) -> str: def yn_yesno_boolean(self) -> str: return self.random_element(self.YN_YESNO_BOOLEAN_VALUES) + def opportunity_attachment(self) -> str: + return self.random_element(self.OPPORTUNITY_ATTACHMENT_S3_PATHS) + fake = faker.Faker() fake.add_provider(CustomProvider) @@ -239,7 +250,7 @@ class OpportunityAttachmentFactory(BaseFactory): class Meta: model = opportunity_models.OpportunityAttachment - file_location = factory.Faker("url") + file_location = factory.Faker("opportunity_attachment") mime_type = factory.Faker("mime_type") file_name = factory.Faker("file_name") file_description = factory.Faker("sentence") From 3b29af1b5ece0083dd79f309da8535c856e5ddcf Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Thu, 24 Oct 2024 10:00:59 -0400 Subject: [PATCH 03/31] update opp with presigned url and return --- .../opportunities_v1/get_opportunity.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 5df09951a..084daf8e9 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -1,10 +1,13 @@ +import copy from datetime import date +import boto3 from sqlalchemy import select from sqlalchemy.orm import noload, selectinload import src.adapters.db as db import src.util.datetime_util as datetime_util +from src.adapters.aws import get_s3_client, S3Config from src.api.route_utils import raise_flask_error from src.db.models.opportunity_models import Opportunity, OpportunitySummary @@ -31,8 +34,22 @@ def _fetch_opportunity( def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: - return _fetch_opportunity(db_session, opportunity_id, load_all_opportunity_summaries=False) + opportunity = _fetch_opportunity(db_session, opportunity_id, load_all_opportunity_summaries=False) + opportunity_copy = copy.deepcopy(opportunity) + s3_config = S3Config() + s3_config.s3_endpoint_url = "http://localstack:4566" + s3_client = get_s3_client( + s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") + ) + + for opp_attachment in opportunity_copy.opportunity_attachments: + file_loc= opp_attachment.file_location + object_key = file_loc.split(f"{s3_config.s3_opportunity_bucket}/")[1] # should be same as filename in db ? + pre_sign_file_loc = s3_client.generate_presigned_url("get_object", Params={"Bucket": s3_config.s3_opportunity_bucket, "Key": object_key},ExpiresIn=1800) # do we want to put limit + opp_attachment.file_location = pre_sign_file_loc + + return opportunity_copy def get_opportunity_versions(db_session: db.Session, opportunity_id: int) -> dict: opportunity = _fetch_opportunity( From c5bee755a3e927bd7ad2be8fb95101152d60904f Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Thu, 24 Oct 2024 10:03:16 -0400 Subject: [PATCH 04/31] clean up --- .../opportunities_v1/get_opportunity.py | 21 +++++++++++++------ api/tests/lib/seed_local_db.py | 11 +++++++--- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 084daf8e9..66e82650c 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -7,7 +7,7 @@ import src.adapters.db as db import src.util.datetime_util as datetime_util -from src.adapters.aws import get_s3_client, S3Config +from src.adapters.aws import S3Config, get_s3_client from src.api.route_utils import raise_flask_error from src.db.models.opportunity_models import Opportunity, OpportunitySummary @@ -34,8 +34,10 @@ def _fetch_opportunity( def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: - opportunity = _fetch_opportunity(db_session, opportunity_id, load_all_opportunity_summaries=False) - opportunity_copy = copy.deepcopy(opportunity) + opportunity = _fetch_opportunity( + db_session, opportunity_id, load_all_opportunity_summaries=False + ) + opportunity_copy = copy.deepcopy(opportunity) s3_config = S3Config() s3_config.s3_endpoint_url = "http://localstack:4566" @@ -44,13 +46,20 @@ def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: ) for opp_attachment in opportunity_copy.opportunity_attachments: - file_loc= opp_attachment.file_location - object_key = file_loc.split(f"{s3_config.s3_opportunity_bucket}/")[1] # should be same as filename in db ? - pre_sign_file_loc = s3_client.generate_presigned_url("get_object", Params={"Bucket": s3_config.s3_opportunity_bucket, "Key": object_key},ExpiresIn=1800) # do we want to put limit + file_loc = opp_attachment.file_location + object_key = file_loc.split(f"{s3_config.s3_opportunity_bucket}/")[ + 1 + ] # should be same as filename in db ? + pre_sign_file_loc = s3_client.generate_presigned_url( + "get_object", + Params={"Bucket": s3_config.s3_opportunity_bucket, "Key": object_key}, + ExpiresIn=1800, + ) # do we want to put limit opp_attachment.file_location = pre_sign_file_loc return opportunity_copy + def get_opportunity_versions(db_session: db.Session, opportunity_id: int) -> dict: opportunity = _fetch_opportunity( db_session, opportunity_id, load_all_opportunity_summaries=True diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 831c513eb..4575c9e34 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -2,6 +2,7 @@ import os import pathlib import random + import boto3 import click from botocore.exceptions import ClientError @@ -11,12 +12,12 @@ import src.logging import src.util.datetime_util as datetime_util import tests.src.db.models.factories as factories +from src.adapters.aws import S3Config, get_s3_client from src.adapters.db import PostgresDBClient from src.db.models.agency_models import Agency from src.db.models.opportunity_models import Opportunity from src.db.models.transfer.topportunity_models import TransferTopportunity from src.util.local import error_if_not_local -from src.adapters.aws import S3Config, get_s3_client logger = logging.getLogger(__name__) # from reportlab.lib.pagesizes import letter @@ -24,6 +25,7 @@ TESTS_FOLDER = pathlib.Path(__file__).parent.resolve() + def upload_opportunity_attachments_s3(): s3_config = S3Config() s3_client = get_s3_client( @@ -38,9 +40,12 @@ def upload_opportunity_attachments_s3(): try: s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) - print(f'Successfully uploaded {file_path} to s3://{s3_config.s3_opportunity_bucket}/{object_name}') #log? + print( + f"Successfully uploaded {file_path} to s3://{s3_config.s3_opportunity_bucket}/{object_name}" + ) # log? except ClientError as e: - print(f'Error uploading {file_path}: {e}') + print(f"Error uploading {file_path}: {e}") + def _add_history( opps: list[Opportunity], From 66cc9159c3074ffd4a5c62aee5295775670c5b67 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Thu, 24 Oct 2024 10:16:31 -0400 Subject: [PATCH 05/31] clean --- api/tests/lib/seed_local_db.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 4575c9e34..3e915da12 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -20,8 +20,6 @@ from src.util.local import error_if_not_local logger = logging.getLogger(__name__) -# from reportlab.lib.pagesizes import letter -# from reportlab.pdfgen import canvas TESTS_FOLDER = pathlib.Path(__file__).parent.resolve() From 387655b5f8a009888e1c2878507c38826c2db333 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Thu, 24 Oct 2024 12:02:57 -0400 Subject: [PATCH 06/31] cleanup --- .../opportunities_v1/get_opportunity.py | 30 ++++++++++++------- api/tests/lib/seed_local_db.py | 13 ++++---- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 66e82650c..5de332b3a 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -1,5 +1,6 @@ import copy from datetime import date +from typing import List import boto3 from sqlalchemy import select @@ -9,7 +10,7 @@ import src.util.datetime_util as datetime_util from src.adapters.aws import S3Config, get_s3_client from src.api.route_utils import raise_flask_error -from src.db.models.opportunity_models import Opportunity, OpportunitySummary +from src.db.models.opportunity_models import Opportunity, OpportunityAttachment, OpportunitySummary def _fetch_opportunity( @@ -33,20 +34,16 @@ def _fetch_opportunity( return opportunity -def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: - opportunity = _fetch_opportunity( - db_session, opportunity_id, load_all_opportunity_summaries=False - ) - opportunity_copy = copy.deepcopy(opportunity) - +def pre_sign_opportunity_file_location( + opp_atts: List[OpportunityAttachment], +) -> List[OpportunityAttachment]: s3_config = S3Config() s3_config.s3_endpoint_url = "http://localstack:4566" s3_client = get_s3_client( s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") ) - - for opp_attachment in opportunity_copy.opportunity_attachments: - file_loc = opp_attachment.file_location + for opp_att in opp_atts: + file_loc = opp_att.file_location object_key = file_loc.split(f"{s3_config.s3_opportunity_bucket}/")[ 1 ] # should be same as filename in db ? @@ -55,7 +52,18 @@ def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: Params={"Bucket": s3_config.s3_opportunity_bucket, "Key": object_key}, ExpiresIn=1800, ) # do we want to put limit - opp_attachment.file_location = pre_sign_file_loc + opp_att.file_location = pre_sign_file_loc + + return opp_atts + + +def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: + opportunity = _fetch_opportunity( + db_session, opportunity_id, load_all_opportunity_summaries=False + ) + opportunity_copy = copy.deepcopy(opportunity) + + pre_sign_opportunity_file_location(opportunity_copy.opportunity_attachments) return opportunity_copy diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 3e915da12..c634f280e 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -24,7 +24,7 @@ TESTS_FOLDER = pathlib.Path(__file__).parent.resolve() -def upload_opportunity_attachments_s3(): +def _upload_opportunity_attachments_s3(): s3_config = S3Config() s3_client = get_s3_client( s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") @@ -38,11 +38,12 @@ def upload_opportunity_attachments_s3(): try: s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) - print( - f"Successfully uploaded {file_path} to s3://{s3_config.s3_opportunity_bucket}/{object_name}" - ) # log? + logger.info("Successfully uploaded files") except ClientError as e: - print(f"Error uploading {file_path}: {e}") + logger.error( + "Error uploading to s3: %s", + extra={"object_name": object_name, "file_path": file_path, "error": e}, + ) def _add_history( @@ -201,7 +202,7 @@ def seed_local_db(iterations: int, include_history: bool) -> None: logger.info("Running seed script for local DB") error_if_not_local() - upload_opportunity_attachments_s3() + _upload_opportunity_attachments_s3() db_client = PostgresDBClient() From 7da3f4c517b22bf431c7ed39ad3e35e2a0434452 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Fri, 25 Oct 2024 16:05:17 -0400 Subject: [PATCH 07/31] add test --- api/src/adapters/aws/s3_adapter.py | 3 ++ .../opportunities_v1/get_opportunity.py | 15 ++++------ api/tests/conftest.py | 28 +++++++++++++++++++ .../test_file_2.txt | 2 +- .../test_file_3.txt | 2 +- .../src/api/opportunities_v1/conftest.py | 3 +- .../test_opportunity_route_get.py | 15 +++++++--- api/tests/src/db/models/factories.py | 4 +-- 8 files changed, 54 insertions(+), 18 deletions(-) diff --git a/api/src/adapters/aws/s3_adapter.py b/api/src/adapters/aws/s3_adapter.py index 4a5f13fb8..125b28aa8 100644 --- a/api/src/adapters/aws/s3_adapter.py +++ b/api/src/adapters/aws/s3_adapter.py @@ -1,3 +1,5 @@ +from typing import Literal + import boto3 import botocore.client @@ -8,6 +10,7 @@ class S3Config(PydanticBaseEnvConfig): # We should generally not need to set this except # locally to use localstack s3_endpoint_url: str | None = None + expires_in: int | Literal[1800] = 1800 ### S3 Buckets # note that we default these to None diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 86162c8a7..c3284b78b 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -1,3 +1,4 @@ +from copy import deepcopy from datetime import date from typing import List @@ -11,6 +12,7 @@ from src.api.route_utils import raise_flask_error from src.db.models.agency_models import Agency from src.db.models.opportunity_models import Opportunity, OpportunityAttachment, OpportunitySummary +from src.util.file_util import split_s3_url def _fetch_opportunity( @@ -42,20 +44,15 @@ def pre_sign_opportunity_file_location( opp_atts: List[OpportunityAttachment], ) -> List[OpportunityAttachment]: s3_config = S3Config() - s3_config.s3_endpoint_url = "http://localstack:4566" s3_client = get_s3_client( s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") ) for opp_att in opp_atts: file_loc = opp_att.file_location - object_key = file_loc.split(f"{s3_config.s3_opportunity_bucket}/")[ - 1 - ] # should be same as filename in db ? + split_url = split_s3_url(file_loc) pre_sign_file_loc = s3_client.generate_presigned_url( - "get_object", - Params={"Bucket": s3_config.s3_opportunity_bucket, "Key": object_key}, - ExpiresIn=1800, - ) # do we want to put limit + "get_object", Params={"Bucket": split_url[0], "Key": split_url[1]} + ) opp_att.file_location = pre_sign_file_loc return opp_atts @@ -65,7 +62,7 @@ def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: opportunity = _fetch_opportunity( db_session, opportunity_id, load_all_opportunity_summaries=False ) - opportunity_copy = copy.deepcopy(opportunity) + opportunity_copy = deepcopy(opportunity) pre_sign_opportunity_file_location(opportunity_copy.opportunity_attachments) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index ba8fde520..9959646da 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -19,6 +19,7 @@ from src.db.models.lookup.sync_lookup_values import sync_lookup_values from src.db.models.opportunity_models import Opportunity from src.db.models.staging import metadata as staging_metadata +from src.util.file_util import split_s3_url from src.util.local import load_local_env_vars from tests.lib import db_testing @@ -253,6 +254,14 @@ def mock_s3(reset_aws_env_vars): yield boto3.resource("s3") +@pytest.fixture +def mock_s3_client(reset_aws_env_vars, monkeypatch): + with moto.mock_aws(config={"core": {"service_whitelist": ["s3"]}}): + # Create a mock S3 client + s3 = boto3.client("s3") + yield s3 + + @pytest.fixture def mock_s3_bucket_resource(mock_s3): bucket = mock_s3.Bucket("test_bucket") @@ -265,6 +274,25 @@ def mock_s3_bucket(mock_s3_bucket_resource): yield mock_s3_bucket_resource.name +@pytest.fixture +def s3_presigned_url(monkeypatch, mock_s3_client, mock_s3_bucket): + """Fixture to generate a presigned URL for a specified S3 object key.""" + + def _generate_presigned_url(file_loc): + split_url = split_s3_url(file_loc) + # Upload a dummy object to the bucket + mock_s3_client.put_object(Bucket=mock_s3_bucket, Key=split_url[1], Body="Hello, world!") + + # Generate a presigned URL for the uploaded object + presigned_url = mock_s3_client.generate_presigned_url( + "get_object", Params={"Bucket": mock_s3_bucket, "Key": split_url[1]} + ) + + return presigned_url + + return _generate_presigned_url + + #################### # Class-based testing #################### diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt b/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt index dbe9dba55..781090c1a 100644 --- a/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt +++ b/api/tests/lib/opportunity_attachment_test_files/test_file_2.txt @@ -1 +1 @@ -Hello, world \ No newline at end of file +Hello, world - again! \ No newline at end of file diff --git a/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt b/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt index dbe9dba55..f94a7922b 100644 --- a/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt +++ b/api/tests/lib/opportunity_attachment_test_files/test_file_3.txt @@ -1 +1 @@ -Hello, world \ No newline at end of file +Hello, motto \ No newline at end of file diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index 65273c62b..f49321491 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -12,6 +12,7 @@ OpportunityAttachment, OpportunitySummary, ) +from src.util.file_util import is_s3_path @pytest.fixture @@ -153,7 +154,7 @@ def validate_opportunity_attachments( assert len(db_attachments) == len(resp_attachments) for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): - assert db_attachment.file_location == resp_attachment["file_location"] + assert not is_s3_path(resp_attachment["file_location"]) # this might be enough test assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index d02950794..066ae0ebe 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -1,5 +1,8 @@ +from copy import deepcopy + import pytest +from src.db.models.opportunity_models import Opportunity from tests.src.api.opportunities_v1.conftest import ( validate_opportunity, validate_opportunity_with_attachments, @@ -47,7 +50,6 @@ def test_get_opportunity_200( db_opportunity = OpportunityFactory.create( **opportunity_params, current_opportunity_summary=None ) # We'll set the current opportunity below - if opportunity_summary_params is not None: db_opportunity_summary = OpportunitySummaryFactory.create( **opportunity_summary_params, opportunity=db_opportunity @@ -66,14 +68,18 @@ def test_get_opportunity_200( def test_get_opportunity_with_attachment_200( - client, api_auth_token, enable_factory_create, db_session + client, api_auth_token, enable_factory_create, db_session, s3_presigned_url ): # Create an opportunity with an attachment opportunity = OpportunityFactory.create() - - # Ensure the opportunity is committed to the database db_session.commit() + # Update file_location with s3 pre-signed url + # opportunity_cp: Opportunity = db_session.query(Opportunity).filter(Opportunity.opportunity_id == opportunity.opportunity_id).first() + opportunity_cp = deepcopy(opportunity) + for opp_att in opportunity_cp.opportunity_attachments: + opp_att.file_location = s3_presigned_url(opp_att.file_location) + # Make the GET request resp = client.get( f"/v1/opportunities/{opportunity.opportunity_id}", headers={"X-Auth": api_auth_token} @@ -85,6 +91,7 @@ def test_get_opportunity_with_attachment_200( # Validate the opportunity data assert len(response_data["attachments"]) > 0 + print(response_data) validate_opportunity_with_attachments(opportunity, response_data) diff --git a/api/tests/src/db/models/factories.py b/api/tests/src/db/models/factories.py index bcecbb02c..b0fed7d11 100644 --- a/api/tests/src/db/models/factories.py +++ b/api/tests/src/db/models/factories.py @@ -196,7 +196,7 @@ def yn_boolean(self) -> str: def yn_yesno_boolean(self) -> str: return self.random_element(self.YN_YESNO_BOOLEAN_VALUES) - def opportunity_attachment(self) -> str: + def s3_file_location(self) -> str: return self.random_element(self.OPPORTUNITY_ATTACHMENT_S3_PATHS) @@ -250,7 +250,7 @@ class OpportunityAttachmentFactory(BaseFactory): class Meta: model = opportunity_models.OpportunityAttachment - file_location = factory.Faker("opportunity_attachment") + file_location = factory.Faker("s3_file_location") mime_type = factory.Faker("mime_type") file_name = factory.Faker("file_name") file_description = factory.Faker("sentence") From d7510b85e49a6065b7c661dbb61819289461120b Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Fri, 25 Oct 2024 16:09:24 -0400 Subject: [PATCH 08/31] cleanup --- api/src/adapters/aws/s3_adapter.py | 2 +- .../src/api/opportunities_v1/test_opportunity_route_get.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/adapters/aws/s3_adapter.py b/api/src/adapters/aws/s3_adapter.py index 125b28aa8..d1651a9d8 100644 --- a/api/src/adapters/aws/s3_adapter.py +++ b/api/src/adapters/aws/s3_adapter.py @@ -10,7 +10,7 @@ class S3Config(PydanticBaseEnvConfig): # We should generally not need to set this except # locally to use localstack s3_endpoint_url: str | None = None - expires_in: int | Literal[1800] = 1800 + expires_in: Literal[1800] = 1800 ### S3 Buckets # note that we default these to None diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index 066ae0ebe..37b5b883f 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -91,7 +91,6 @@ def test_get_opportunity_with_attachment_200( # Validate the opportunity data assert len(response_data["attachments"]) > 0 - print(response_data) validate_opportunity_with_attachments(opportunity, response_data) From c8ee7ef22696c2891f50965c7fda006eb572ab15 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 10:59:15 -0400 Subject: [PATCH 09/31] unit test --- api/tests/conftest.py | 21 +++++++++++ .../test_opportunity_route_get.py | 35 ++++++++++++++----- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 9959646da..c63abe7b2 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -1,4 +1,6 @@ import logging +import os +import pathlib import uuid import _pytest.monkeypatch @@ -51,6 +53,25 @@ def test_example(monkeypatch): load_local_env_vars() +### Uploads test file to localstack s3 bucket +@pytest.fixture +def upload_opportunity_attachment_s3(): + s3_client = boto3.client("s3", endpoint_url=os.environ["S3_ENDPOINT_URL"]) + s3_client.bucket_name = "test-bucket" + s3_client.create_bucket(Bucket=s3_client.bucket_name) + file_path = ( + pathlib.Path(__file__).parent.resolve() + / "lib/opportunity_attachment_test_files/test_file_1.txt" + ) + + # Upload opportunity attachment file to the bucket + s3_client.upload_file(file_path, Bucket=s3_client.bucket_name, Key="test_file_1.txt") + + # Check file was uploaded to mock s3 + s3_files = s3_client.list_objects_v2(Bucket=s3_client.bucket_name) + assert len(s3_files["Contents"]) == 1 + + #################### # Test DB session #################### diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index 37b5b883f..c2a99179a 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -1,14 +1,13 @@ -from copy import deepcopy - import pytest +import requests -from src.db.models.opportunity_models import Opportunity from tests.src.api.opportunities_v1.conftest import ( validate_opportunity, validate_opportunity_with_attachments, ) from tests.src.db.models.factories import ( CurrentOpportunitySummaryFactory, + OpportunityAttachmentFactory, OpportunityFactory, OpportunitySummaryFactory, ) @@ -74,12 +73,6 @@ def test_get_opportunity_with_attachment_200( opportunity = OpportunityFactory.create() db_session.commit() - # Update file_location with s3 pre-signed url - # opportunity_cp: Opportunity = db_session.query(Opportunity).filter(Opportunity.opportunity_id == opportunity.opportunity_id).first() - opportunity_cp = deepcopy(opportunity) - for opp_att in opportunity_cp.opportunity_attachments: - opp_att.file_location = s3_presigned_url(opp_att.file_location) - # Make the GET request resp = client.get( f"/v1/opportunities/{opportunity.opportunity_id}", headers={"X-Auth": api_auth_token} @@ -94,6 +87,30 @@ def test_get_opportunity_with_attachment_200( validate_opportunity_with_attachments(opportunity, response_data) +def test_get_opportunity_s3_endpoint_url_200( + upload_opportunity_attachment_s3, client, api_auth_token, enable_factory_create, db_session +): + # Create an opportunity with a specific attachment + opportunity = OpportunityFactory.create(opportunity_attachments=[]) + file_loc = "s3://test-bucket/test_file_1.txt" + OpportunityAttachmentFactory.create(file_location=file_loc, opportunity=opportunity) + + # Make the GET request + resp = client.get( + f"/v1/opportunities/{opportunity.opportunity_id}", headers={"X-Auth": api_auth_token} + ) + + # Check the response + assert resp.status_code == 200 + response_data = resp.get_json()["data"] + file_loc = response_data["attachments"][0]["file_location"] + + # Validate the s3 endpoint url + response = requests.get(file_loc) + assert response.status_code == 200 + assert "Hello, world" in response.text + + def test_get_opportunity_404_not_found(client, api_auth_token, truncate_opportunities): resp = client.get("/v1/opportunities/1", headers={"X-Auth": api_auth_token}) assert resp.status_code == 404 From c454096fc4e4d7d82e50aa50419d583c537f77d4 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 11:09:38 -0400 Subject: [PATCH 10/31] cleanup --- api/tests/conftest.py | 28 ------------------- .../src/api/opportunities_v1/conftest.py | 3 +- .../test_opportunity_route_get.py | 2 +- 3 files changed, 2 insertions(+), 31 deletions(-) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index c63abe7b2..13f9bd132 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -21,7 +21,6 @@ from src.db.models.lookup.sync_lookup_values import sync_lookup_values from src.db.models.opportunity_models import Opportunity from src.db.models.staging import metadata as staging_metadata -from src.util.file_util import split_s3_url from src.util.local import load_local_env_vars from tests.lib import db_testing @@ -275,14 +274,6 @@ def mock_s3(reset_aws_env_vars): yield boto3.resource("s3") -@pytest.fixture -def mock_s3_client(reset_aws_env_vars, monkeypatch): - with moto.mock_aws(config={"core": {"service_whitelist": ["s3"]}}): - # Create a mock S3 client - s3 = boto3.client("s3") - yield s3 - - @pytest.fixture def mock_s3_bucket_resource(mock_s3): bucket = mock_s3.Bucket("test_bucket") @@ -295,25 +286,6 @@ def mock_s3_bucket(mock_s3_bucket_resource): yield mock_s3_bucket_resource.name -@pytest.fixture -def s3_presigned_url(monkeypatch, mock_s3_client, mock_s3_bucket): - """Fixture to generate a presigned URL for a specified S3 object key.""" - - def _generate_presigned_url(file_loc): - split_url = split_s3_url(file_loc) - # Upload a dummy object to the bucket - mock_s3_client.put_object(Bucket=mock_s3_bucket, Key=split_url[1], Body="Hello, world!") - - # Generate a presigned URL for the uploaded object - presigned_url = mock_s3_client.generate_presigned_url( - "get_object", Params={"Bucket": mock_s3_bucket, "Key": split_url[1]} - ) - - return presigned_url - - return _generate_presigned_url - - #################### # Class-based testing #################### diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index f49321491..65273c62b 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -12,7 +12,6 @@ OpportunityAttachment, OpportunitySummary, ) -from src.util.file_util import is_s3_path @pytest.fixture @@ -154,7 +153,7 @@ def validate_opportunity_attachments( assert len(db_attachments) == len(resp_attachments) for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): - assert not is_s3_path(resp_attachment["file_location"]) # this might be enough test + assert db_attachment.file_location == resp_attachment["file_location"] assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index c2a99179a..55280e165 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -67,7 +67,7 @@ def test_get_opportunity_200( def test_get_opportunity_with_attachment_200( - client, api_auth_token, enable_factory_create, db_session, s3_presigned_url + client, api_auth_token, enable_factory_create, db_session ): # Create an opportunity with an attachment opportunity = OpportunityFactory.create() From 88487ae5a86de8815ec6effea3e6f531515b72ad Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 11:43:46 -0400 Subject: [PATCH 11/31] add timeout to request --- .../src/api/opportunities_v1/test_opportunity_route_get.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index 55280e165..b60123b6d 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -106,7 +106,7 @@ def test_get_opportunity_s3_endpoint_url_200( file_loc = response_data["attachments"][0]["file_location"] # Validate the s3 endpoint url - response = requests.get(file_loc) + response = requests.get(file_loc, timeout=5) assert response.status_code == 200 assert "Hello, world" in response.text From a866aa6dfa2d5030788aff2c8e17b4979de809eb Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 12:03:22 -0400 Subject: [PATCH 12/31] update attachment test --- api/tests/conftest.py | 2 +- api/tests/src/api/opportunities_v1/conftest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 13f9bd132..584423a30 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -55,7 +55,7 @@ def test_example(monkeypatch): ### Uploads test file to localstack s3 bucket @pytest.fixture def upload_opportunity_attachment_s3(): - s3_client = boto3.client("s3", endpoint_url=os.environ["S3_ENDPOINT_URL"]) + s3_client = boto3.client("s3", endpoint_url=os.environ["S3_ENDPOINT_URL"], aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") s3_client.bucket_name = "test-bucket" s3_client.create_bucket(Bucket=s3_client.bucket_name) file_path = ( diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index 65273c62b..f006d0dfb 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -153,7 +153,7 @@ def validate_opportunity_attachments( assert len(db_attachments) == len(resp_attachments) for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): - assert db_attachment.file_location == resp_attachment["file_location"] + assert db_attachment.file_location != resp_attachment["file_location"] #file_location is the pre-signed url assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] From 698687e4cdcd5f05e2617f8e60b9147d41a49ca0 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 12:15:54 -0400 Subject: [PATCH 13/31] fomrat --- api/tests/conftest.py | 7 ++++++- api/tests/src/api/opportunities_v1/conftest.py | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 584423a30..49022db74 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -55,7 +55,12 @@ def test_example(monkeypatch): ### Uploads test file to localstack s3 bucket @pytest.fixture def upload_opportunity_attachment_s3(): - s3_client = boto3.client("s3", endpoint_url=os.environ["S3_ENDPOINT_URL"], aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") + s3_client = boto3.client( + "s3", + endpoint_url=os.environ["S3_ENDPOINT_URL"], + aws_access_key_id="NO_CREDS", + aws_secret_access_key="NO_CREDS", + ) s3_client.bucket_name = "test-bucket" s3_client.create_bucket(Bucket=s3_client.bucket_name) file_path = ( diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index f006d0dfb..9368769c7 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -153,7 +153,9 @@ def validate_opportunity_attachments( assert len(db_attachments) == len(resp_attachments) for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): - assert db_attachment.file_location != resp_attachment["file_location"] #file_location is the pre-signed url + assert ( + db_attachment.file_location != resp_attachment["file_location"] + ) # file_location is the pre-signed url assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] From bc7b60528095769a7eb3e36c403d18bb19367a81 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 17:11:43 -0400 Subject: [PATCH 14/31] update test fixture set new local env --- api/local.env | 3 ++ api/src/adapters/aws/s3_adapter.py | 4 +-- .../opportunities_v1/get_opportunity.py | 18 +++++++--- api/tests/conftest.py | 36 +++++++++---------- api/tests/lib/seed_local_db.py | 16 ++++----- .../src/api/opportunities_v1/conftest.py | 2 +- 6 files changed, 44 insertions(+), 35 deletions(-) diff --git a/api/local.env b/api/local.env index d6248be61..b8d78493a 100644 --- a/api/local.env +++ b/api/local.env @@ -109,3 +109,6 @@ IS_LOCAL_FOREIGN_TABLE=true # File path for the export_opportunity_data task EXPORT_OPP_DATA_FILE_PATH=/tmp + +#This env var is used to set local AWS credentials +IS_LOCAL_AWS=1 \ No newline at end of file diff --git a/api/src/adapters/aws/s3_adapter.py b/api/src/adapters/aws/s3_adapter.py index d1651a9d8..6eae7f034 100644 --- a/api/src/adapters/aws/s3_adapter.py +++ b/api/src/adapters/aws/s3_adapter.py @@ -1,5 +1,3 @@ -from typing import Literal - import boto3 import botocore.client @@ -10,7 +8,7 @@ class S3Config(PydanticBaseEnvConfig): # We should generally not need to set this except # locally to use localstack s3_endpoint_url: str | None = None - expires_in: Literal[1800] = 1800 + presigned_s3_duration: int = 1800 ### S3 Buckets # note that we default these to None diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index c3284b78b..59aa6827f 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -1,3 +1,4 @@ +import os from copy import deepcopy from datetime import date from typing import List @@ -40,18 +41,25 @@ def _fetch_opportunity( return opportunity +def get_boto_session() -> boto3.Session: + is_local = bool(os.getenv("IS_LOCAL_AWS", False)) + if is_local: + return boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") + + return boto3.Session() + + def pre_sign_opportunity_file_location( opp_atts: List[OpportunityAttachment], ) -> List[OpportunityAttachment]: s3_config = S3Config() - s3_client = get_s3_client( - s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") - ) + + s3_client = get_s3_client(s3_config, get_boto_session()) for opp_att in opp_atts: file_loc = opp_att.file_location - split_url = split_s3_url(file_loc) + bucket, key = split_s3_url(file_loc) pre_sign_file_loc = s3_client.generate_presigned_url( - "get_object", Params={"Bucket": split_url[0], "Key": split_url[1]} + "get_object", Params={"Bucket": bucket, "Key": key} ) opp_att.file_location = pre_sign_file_loc diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 49022db74..9c6edd1e3 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -53,27 +53,26 @@ def test_example(monkeypatch): ### Uploads test file to localstack s3 bucket -@pytest.fixture -def upload_opportunity_attachment_s3(): - s3_client = boto3.client( - "s3", - endpoint_url=os.environ["S3_ENDPOINT_URL"], - aws_access_key_id="NO_CREDS", - aws_secret_access_key="NO_CREDS", - ) - s3_client.bucket_name = "test-bucket" - s3_client.create_bucket(Bucket=s3_client.bucket_name) - file_path = ( - pathlib.Path(__file__).parent.resolve() - / "lib/opportunity_attachment_test_files/test_file_1.txt" +@pytest.fixture() +def upload_opportunity_attachment_s3(reset_aws_env_vars, mock_s3_bucket): + s3_client = boto3.client("s3") + test_folder_path = ( + pathlib.Path(__file__).parent.resolve() / "lib/opportunity_attachment_test_files" ) - # Upload opportunity attachment file to the bucket - s3_client.upload_file(file_path, Bucket=s3_client.bucket_name, Key="test_file_1.txt") + for root, _, files in os.walk(test_folder_path): + for file in files: + file_path = os.path.join(root, file) + object_name = os.path.relpath(file_path, test_folder_path) + + # Upload opportunity attachment file to the bucket + s3_client.upload_file( + file_path, Bucket=mock_s3_bucket, Key=os.path.relpath(file_path, object_name) + ) # Check file was uploaded to mock s3 - s3_files = s3_client.list_objects_v2(Bucket=s3_client.bucket_name) - assert len(s3_files["Contents"]) == 1 + s3_files = s3_client.list_objects_v2(Bucket=mock_s3_bucket) + assert len(s3_files["Contents"]) == 5 #################### @@ -261,7 +260,7 @@ def api_auth_token(monkeypatch, all_api_auth_tokens): #################### -@pytest.fixture +@pytest.fixture() def reset_aws_env_vars(monkeypatch): # Reset the env vars so you can't accidentally connect # to a real AWS account if you were doing some local testing @@ -270,6 +269,7 @@ def reset_aws_env_vars(monkeypatch): monkeypatch.setenv("AWS_SECURITY_TOKEN", "testing") monkeypatch.setenv("AWS_SESSION_TOKEN", "testing") monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1") + monkeypatch.setenv("S3_ENDPOINT_URL", "http://localstack:4566") @pytest.fixture diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index c634f280e..2b4549303 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -36,14 +36,14 @@ def _upload_opportunity_attachments_s3(): file_path = os.path.join(root, file) object_name = os.path.relpath(file_path, test_folder_path) - try: - s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) - logger.info("Successfully uploaded files") - except ClientError as e: - logger.error( - "Error uploading to s3: %s", - extra={"object_name": object_name, "file_path": file_path, "error": e}, - ) + try: + s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) + logger.info("Successfully uploaded files") + except ClientError as e: + logger.error( + "Error uploading to s3: %s", + extra={"object_name": object_name, "file_path": file_path, "error": e}, + ) def _add_history( diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index 9368769c7..ec16fb097 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -155,7 +155,7 @@ def validate_opportunity_attachments( for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): assert ( db_attachment.file_location != resp_attachment["file_location"] - ) # file_location is the pre-signed url + ) # response file_location is the pre-signed url not S3 url assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] From 2cc04661feb8bb0c40b38e227653fdeae8579dc9 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 22:09:24 -0400 Subject: [PATCH 15/31] updatew unit test --- .../opportunities_v1/opportunity_schemas.py | 4 ++-- .../opportunities_v1/get_opportunity.py | 14 +++++++------- api/tests/conftest.py | 6 ++---- .../src/api/opportunities_v1/conftest.py | 4 +--- .../test_opportunity_route_get.py | 19 ++++++++++++------- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/api/src/api/opportunities_v1/opportunity_schemas.py b/api/src/api/opportunities_v1/opportunity_schemas.py index a66ef8da7..117627b89 100644 --- a/api/src/api/opportunities_v1/opportunity_schemas.py +++ b/api/src/api/opportunities_v1/opportunity_schemas.py @@ -318,9 +318,9 @@ class OpportunityV1Schema(Schema): class OpportunityAttachmentV1Schema(Schema): - file_location = fields.String( + s3_file = fields.String( metadata={ - "description": "The URL to download the attachment", + "description": "The pre-signed s3 URL to download the attachment", "example": "https://...", } ) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 59aa6827f..8449b9fa0 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -1,5 +1,4 @@ import os -from copy import deepcopy from datetime import date from typing import List @@ -50,7 +49,7 @@ def get_boto_session() -> boto3.Session: def pre_sign_opportunity_file_location( - opp_atts: List[OpportunityAttachment], + opp_atts: List, ) -> List[OpportunityAttachment]: s3_config = S3Config() @@ -59,9 +58,11 @@ def pre_sign_opportunity_file_location( file_loc = opp_att.file_location bucket, key = split_s3_url(file_loc) pre_sign_file_loc = s3_client.generate_presigned_url( - "get_object", Params={"Bucket": bucket, "Key": key} + "get_object", + Params={"Bucket": bucket, "Key": key}, + ExpiresIn=s3_config.presigned_s3_duration, ) - opp_att.file_location = pre_sign_file_loc + opp_att.s3_file = pre_sign_file_loc return opp_atts @@ -70,11 +71,10 @@ def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity: opportunity = _fetch_opportunity( db_session, opportunity_id, load_all_opportunity_summaries=False ) - opportunity_copy = deepcopy(opportunity) - pre_sign_opportunity_file_location(opportunity_copy.opportunity_attachments) + pre_sign_opportunity_file_location(opportunity.opportunity_attachments) - return opportunity_copy + return opportunity def get_opportunity_versions(db_session: db.Session, opportunity_id: int) -> dict: diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 9c6edd1e3..ea6a852c6 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -63,11 +63,9 @@ def upload_opportunity_attachment_s3(reset_aws_env_vars, mock_s3_bucket): for root, _, files in os.walk(test_folder_path): for file in files: file_path = os.path.join(root, file) - object_name = os.path.relpath(file_path, test_folder_path) - # Upload opportunity attachment file to the bucket s3_client.upload_file( - file_path, Bucket=mock_s3_bucket, Key=os.path.relpath(file_path, object_name) + file_path, Bucket=mock_s3_bucket, Key=os.path.relpath(file_path, test_folder_path) ) # Check file was uploaded to mock s3 @@ -269,7 +267,7 @@ def reset_aws_env_vars(monkeypatch): monkeypatch.setenv("AWS_SECURITY_TOKEN", "testing") monkeypatch.setenv("AWS_SESSION_TOKEN", "testing") monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1") - monkeypatch.setenv("S3_ENDPOINT_URL", "http://localstack:4566") + monkeypatch.setenv("IS_LOCAL_AWS", "False") @pytest.fixture diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index ec16fb097..65273c62b 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -153,9 +153,7 @@ def validate_opportunity_attachments( assert len(db_attachments) == len(resp_attachments) for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): - assert ( - db_attachment.file_location != resp_attachment["file_location"] - ) # response file_location is the pre-signed url not S3 url + assert db_attachment.file_location == resp_attachment["file_location"] assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index b60123b6d..cd0941762 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -1,5 +1,5 @@ +import boto3 import pytest -import requests from tests.src.api.opportunities_v1.conftest import ( validate_opportunity, @@ -92,7 +92,9 @@ def test_get_opportunity_s3_endpoint_url_200( ): # Create an opportunity with a specific attachment opportunity = OpportunityFactory.create(opportunity_attachments=[]) - file_loc = "s3://test-bucket/test_file_1.txt" + bucket = "test_bucket" + object_name = "test_file_1.txt" + file_loc = f"s3://{bucket}/{object_name}" OpportunityAttachmentFactory.create(file_location=file_loc, opportunity=opportunity) # Make the GET request @@ -103,12 +105,15 @@ def test_get_opportunity_s3_endpoint_url_200( # Check the response assert resp.status_code == 200 response_data = resp.get_json()["data"] - file_loc = response_data["attachments"][0]["file_location"] + presigned_url = response_data["attachments"][0]["s3_file"] - # Validate the s3 endpoint url - response = requests.get(file_loc, timeout=5) - assert response.status_code == 200 - assert "Hello, world" in response.text + # Validate pre-signed URL + assert bucket in presigned_url + assert object_name in presigned_url + + # Validate content of file + response = boto3.client("s3").get_object(Bucket=bucket, Key=object_name) + assert response["Body"].read() == b"Hello, world" def test_get_opportunity_404_not_found(client, api_auth_token, truncate_opportunities): From 39f048db612a7e46b36115d363240c3212d026b1 Mon Sep 17 00:00:00 2001 From: nava-platform-bot Date: Tue, 29 Oct 2024 02:11:36 +0000 Subject: [PATCH 16/31] Create ERD diagram and Update OpenAPI spec --- api/openapi.generated.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml index 9ccc945b0..a7e036c72 100644 --- a/api/openapi.generated.yml +++ b/api/openapi.generated.yml @@ -2036,9 +2036,9 @@ components: OpportunityAttachmentV1: type: object properties: - file_location: + s3_file: type: string - description: The URL to download the attachment + description: The pre-signed s3 URL to download the attachment example: https://... mime_type: type: string From 292f88601b11613e3b8e416dbe8c89bf0f3705e9 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Mon, 28 Oct 2024 22:19:50 -0400 Subject: [PATCH 17/31] clean --- api/local.env | 2 +- api/tests/conftest.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/api/local.env b/api/local.env index b8d78493a..b02c535b8 100644 --- a/api/local.env +++ b/api/local.env @@ -110,5 +110,5 @@ IS_LOCAL_FOREIGN_TABLE=true # File path for the export_opportunity_data task EXPORT_OPP_DATA_FILE_PATH=/tmp -#This env var is used to set local AWS credentials +# This env var is used to set local AWS credentials IS_LOCAL_AWS=1 \ No newline at end of file diff --git a/api/tests/conftest.py b/api/tests/conftest.py index ea6a852c6..9b58ccaf8 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -68,7 +68,7 @@ def upload_opportunity_attachment_s3(reset_aws_env_vars, mock_s3_bucket): file_path, Bucket=mock_s3_bucket, Key=os.path.relpath(file_path, test_folder_path) ) - # Check file was uploaded to mock s3 + # Check files were uploaded to mock s3 s3_files = s3_client.list_objects_v2(Bucket=mock_s3_bucket) assert len(s3_files["Contents"]) == 5 @@ -267,8 +267,6 @@ def reset_aws_env_vars(monkeypatch): monkeypatch.setenv("AWS_SECURITY_TOKEN", "testing") monkeypatch.setenv("AWS_SESSION_TOKEN", "testing") monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1") - monkeypatch.setenv("IS_LOCAL_AWS", "False") - @pytest.fixture def mock_s3(reset_aws_env_vars): From cc6bcac200357d7fbcca5e923cff060e698fe91e Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 10:28:16 -0400 Subject: [PATCH 18/31] update response schema and test --- api/src/api/opportunities_v1/opportunity_schemas.py | 2 +- .../services/opportunities_v1/get_opportunity.py | 2 +- api/tests/conftest.py | 2 ++ .../opportunities_v1/test_opportunity_route_get.py | 13 +++++-------- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/api/src/api/opportunities_v1/opportunity_schemas.py b/api/src/api/opportunities_v1/opportunity_schemas.py index 117627b89..d52eaa040 100644 --- a/api/src/api/opportunities_v1/opportunity_schemas.py +++ b/api/src/api/opportunities_v1/opportunity_schemas.py @@ -318,7 +318,7 @@ class OpportunityV1Schema(Schema): class OpportunityAttachmentV1Schema(Schema): - s3_file = fields.String( + download_file = fields.String( metadata={ "description": "The pre-signed s3 URL to download the attachment", "example": "https://...", diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 8449b9fa0..92ad675e0 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -62,7 +62,7 @@ def pre_sign_opportunity_file_location( Params={"Bucket": bucket, "Key": key}, ExpiresIn=s3_config.presigned_s3_duration, ) - opp_att.s3_file = pre_sign_file_loc + opp_att.download_file = pre_sign_file_loc return opp_atts diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 9b58ccaf8..e6cf8e1bd 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -267,6 +267,8 @@ def reset_aws_env_vars(monkeypatch): monkeypatch.setenv("AWS_SECURITY_TOKEN", "testing") monkeypatch.setenv("AWS_SESSION_TOKEN", "testing") monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1") + monkeypatch.delenv("S3_ENDPOINT_URL") + @pytest.fixture def mock_s3(reset_aws_env_vars): diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index cd0941762..1ea5bd3b1 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -1,5 +1,5 @@ -import boto3 import pytest +import requests from tests.src.api.opportunities_v1.conftest import ( validate_opportunity, @@ -107,13 +107,10 @@ def test_get_opportunity_s3_endpoint_url_200( response_data = resp.get_json()["data"] presigned_url = response_data["attachments"][0]["s3_file"] - # Validate pre-signed URL - assert bucket in presigned_url - assert object_name in presigned_url - - # Validate content of file - response = boto3.client("s3").get_object(Bucket=bucket, Key=object_name) - assert response["Body"].read() == b"Hello, world" + # Validate pre-signed url + response = requests.get(presigned_url) + assert response.status_code == 200 + assert response.content == b"Hello, world" def test_get_opportunity_404_not_found(client, api_auth_token, truncate_opportunities): From bf19076a283a3167a18caa9fdaff258f54b3d4fb Mon Sep 17 00:00:00 2001 From: nava-platform-bot Date: Tue, 29 Oct 2024 14:31:37 +0000 Subject: [PATCH 19/31] Create ERD diagram and Update OpenAPI spec --- api/openapi.generated.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml index a7e036c72..46d4406af 100644 --- a/api/openapi.generated.yml +++ b/api/openapi.generated.yml @@ -2036,7 +2036,7 @@ components: OpportunityAttachmentV1: type: object properties: - s3_file: + download_file: type: string description: The pre-signed s3 URL to download the attachment example: https://... From 183831501efeee80894046f8536955513477281c Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 10:36:12 -0400 Subject: [PATCH 20/31] clean up --- api/src/api/opportunities_v1/opportunity_schemas.py | 2 +- api/src/services/opportunities_v1/get_opportunity.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/api/opportunities_v1/opportunity_schemas.py b/api/src/api/opportunities_v1/opportunity_schemas.py index d52eaa040..3c208c17d 100644 --- a/api/src/api/opportunities_v1/opportunity_schemas.py +++ b/api/src/api/opportunities_v1/opportunity_schemas.py @@ -318,7 +318,7 @@ class OpportunityV1Schema(Schema): class OpportunityAttachmentV1Schema(Schema): - download_file = fields.String( + download_path = fields.String( metadata={ "description": "The pre-signed s3 URL to download the attachment", "example": "https://...", diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 92ad675e0..fb0f24819 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -62,7 +62,7 @@ def pre_sign_opportunity_file_location( Params={"Bucket": bucket, "Key": key}, ExpiresIn=s3_config.presigned_s3_duration, ) - opp_att.download_file = pre_sign_file_loc + opp_att.download_path = pre_sign_file_loc return opp_atts From 1201d6ac5bf208773f5cc26871cb16606ce78bc1 Mon Sep 17 00:00:00 2001 From: nava-platform-bot Date: Tue, 29 Oct 2024 14:38:42 +0000 Subject: [PATCH 21/31] Create ERD diagram and Update OpenAPI spec --- api/openapi.generated.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml index 46d4406af..384321edb 100644 --- a/api/openapi.generated.yml +++ b/api/openapi.generated.yml @@ -2036,7 +2036,7 @@ components: OpportunityAttachmentV1: type: object properties: - download_file: + download_path: type: string description: The pre-signed s3 URL to download the attachment example: https://... From 6c6a7405b7a25ef40ce24a6ae257b9204279bf94 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 10:55:51 -0400 Subject: [PATCH 22/31] update test --- api/tests/conftest.py | 7 +++---- .../src/api/opportunities_v1/test_opportunity_route_get.py | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index e6cf8e1bd..d751dad58 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -52,8 +52,8 @@ def test_example(monkeypatch): load_local_env_vars() -### Uploads test file to localstack s3 bucket -@pytest.fixture() +### Uploads test files +@pytest.fixture def upload_opportunity_attachment_s3(reset_aws_env_vars, mock_s3_bucket): s3_client = boto3.client("s3") test_folder_path = ( @@ -63,7 +63,6 @@ def upload_opportunity_attachment_s3(reset_aws_env_vars, mock_s3_bucket): for root, _, files in os.walk(test_folder_path): for file in files: file_path = os.path.join(root, file) - # Upload opportunity attachment file to the bucket s3_client.upload_file( file_path, Bucket=mock_s3_bucket, Key=os.path.relpath(file_path, test_folder_path) ) @@ -258,7 +257,7 @@ def api_auth_token(monkeypatch, all_api_auth_tokens): #################### -@pytest.fixture() +@pytest.fixture def reset_aws_env_vars(monkeypatch): # Reset the env vars so you can't accidentally connect # to a real AWS account if you were doing some local testing diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index 1ea5bd3b1..d2893ddd5 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -105,12 +105,12 @@ def test_get_opportunity_s3_endpoint_url_200( # Check the response assert resp.status_code == 200 response_data = resp.get_json()["data"] - presigned_url = response_data["attachments"][0]["s3_file"] + presigned_url = response_data["attachments"][0]["download_path"] # Validate pre-signed url response = requests.get(presigned_url) assert response.status_code == 200 - assert response.content == b"Hello, world" + assert response.text == "Hello, world" def test_get_opportunity_404_not_found(client, api_auth_token, truncate_opportunities): From a443fa690c23ede32bccb4440464d5551dab7084 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 11:17:44 -0400 Subject: [PATCH 23/31] timeout --- .../src/api/opportunities_v1/test_opportunity_route_get.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index d2893ddd5..9be676dbd 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -108,7 +108,7 @@ def test_get_opportunity_s3_endpoint_url_200( presigned_url = response_data["attachments"][0]["download_path"] # Validate pre-signed url - response = requests.get(presigned_url) + response = requests.get(presigned_url, timeout=5) assert response.status_code == 200 assert response.text == "Hello, world" From 6317a45e0fbf343590ef85c49ccecaa3601e59e0 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 11:30:56 -0400 Subject: [PATCH 24/31] rm file_location from test --- api/tests/src/api/opportunities_v1/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index 65273c62b..f08f3e3cd 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -153,7 +153,6 @@ def validate_opportunity_attachments( assert len(db_attachments) == len(resp_attachments) for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True): - assert db_attachment.file_location == resp_attachment["file_location"] assert db_attachment.mime_type == resp_attachment["mime_type"] assert db_attachment.file_name == resp_attachment["file_name"] assert db_attachment.file_description == resp_attachment["file_description"] From f56413c1c489647310125012714586d4c63968af Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 16:57:09 -0400 Subject: [PATCH 25/31] set to localhost when local --- api/local.env | 6 +-- .../opportunities_v1/get_opportunity.py | 3 +- api/tests/lib/seed_local_db.py | 40 +++++++++---------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/api/local.env b/api/local.env index b02c535b8..a93d17aec 100644 --- a/api/local.env +++ b/api/local.env @@ -84,6 +84,9 @@ S3_ENDPOINT_URL=http://localstack:4566 S3_OPPORTUNITY_BUCKET=local-opportunities +# This env var is used to set local AWS credentials +IS_LOCAL_AWS=1 + ############################ # Feature Flags ############################ @@ -109,6 +112,3 @@ IS_LOCAL_FOREIGN_TABLE=true # File path for the export_opportunity_data task EXPORT_OPP_DATA_FILE_PATH=/tmp - -# This env var is used to set local AWS credentials -IS_LOCAL_AWS=1 \ No newline at end of file diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index fb0f24819..aa4b8e208 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -62,7 +62,8 @@ def pre_sign_opportunity_file_location( Params={"Bucket": bucket, "Key": key}, ExpiresIn=s3_config.presigned_s3_duration, ) - opp_att.download_path = pre_sign_file_loc + if s3_config.s3_endpoint_url: + opp_att.download_path = pre_sign_file_loc.replace(s3_config.s3_endpoint_url, "http://localhost:4566") return opp_atts diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 2b4549303..430a864fd 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -36,14 +36,14 @@ def _upload_opportunity_attachments_s3(): file_path = os.path.join(root, file) object_name = os.path.relpath(file_path, test_folder_path) - try: - s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) - logger.info("Successfully uploaded files") - except ClientError as e: - logger.error( - "Error uploading to s3: %s", - extra={"object_name": object_name, "file_path": file_path, "error": e}, - ) + try: + s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name) + logger.info("Successfully uploaded files") + except ClientError as e: + logger.error( + "Error uploading to s3: %s", + extra={"object_name": object_name, "file_path": file_path, "error": e}, + ) def _add_history( @@ -203,15 +203,15 @@ def seed_local_db(iterations: int, include_history: bool) -> None: error_if_not_local() _upload_opportunity_attachments_s3() - - db_client = PostgresDBClient() - - with db_client.get_session() as db_session: - factories._db_session = db_session - - _build_opportunities(db_session, iterations, include_history) - # Need to commit to force any updates made - # after factories created objects - db_session.commit() - - _build_agencies(db_session) + # + # db_client = PostgresDBClient() + # + # with db_client.get_session() as db_session: + # factories._db_session = db_session + # + # _build_opportunities(db_session, iterations, include_history) + # # Need to commit to force any updates made + # # after factories created objects + # db_session.commit() + # + # _build_agencies(db_session) From 2cf28aec4beb850bfa84ddd00994c3a9564a3070 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Tue, 29 Oct 2024 16:59:14 -0400 Subject: [PATCH 26/31] format --- api/src/services/opportunities_v1/get_opportunity.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index aa4b8e208..4653b6e41 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -63,7 +63,9 @@ def pre_sign_opportunity_file_location( ExpiresIn=s3_config.presigned_s3_duration, ) if s3_config.s3_endpoint_url: - opp_att.download_path = pre_sign_file_loc.replace(s3_config.s3_endpoint_url, "http://localhost:4566") + opp_att.download_path = pre_sign_file_loc.replace( + s3_config.s3_endpoint_url, "http://localhost:4566" + ) return opp_atts From 147ea61f0440a026bb6e4665a2213edb2e39c713 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Wed, 30 Oct 2024 09:05:48 -0400 Subject: [PATCH 27/31] clean --- api/tests/lib/seed_local_db.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 430a864fd..31e05dbde 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -13,7 +13,6 @@ import src.util.datetime_util as datetime_util import tests.src.db.models.factories as factories from src.adapters.aws import S3Config, get_s3_client -from src.adapters.db import PostgresDBClient from src.db.models.agency_models import Agency from src.db.models.opportunity_models import Opportunity from src.db.models.transfer.topportunity_models import TransferTopportunity From 3bd6e187b7ed6e1a0de7d638a70ed5f3b7703044 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Wed, 30 Oct 2024 09:42:38 -0400 Subject: [PATCH 28/31] update get_opportunity --- api/src/services/opportunities_v1/get_opportunity.py | 5 ++++- .../src/api/opportunities_v1/test_opportunity_route_get.py | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index 4653b6e41..cabe04099 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -63,10 +63,13 @@ def pre_sign_opportunity_file_location( ExpiresIn=s3_config.presigned_s3_duration, ) if s3_config.s3_endpoint_url: - opp_att.download_path = pre_sign_file_loc.replace( + pre_sign_file_loc = pre_sign_file_loc.replace( s3_config.s3_endpoint_url, "http://localhost:4566" ) + opp_att.download_path = pre_sign_file_loc + + return opp_atts diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py index 9be676dbd..53fea4c85 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_get.py @@ -105,6 +105,7 @@ def test_get_opportunity_s3_endpoint_url_200( # Check the response assert resp.status_code == 200 response_data = resp.get_json()["data"] + print("response_data", response_data) presigned_url = response_data["attachments"][0]["download_path"] # Validate pre-signed url From 071134ea4e5a9452dbd1d6c3b667e214982d3764 Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Wed, 30 Oct 2024 09:43:42 -0400 Subject: [PATCH 29/31] format lint --- api/src/services/opportunities_v1/get_opportunity.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index cabe04099..a0638ae7f 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -69,7 +69,6 @@ def pre_sign_opportunity_file_location( opp_att.download_path = pre_sign_file_loc - return opp_atts From 8a6a622fdc110b3f33b1065450d77b910c86896d Mon Sep 17 00:00:00 2001 From: Bruk Abebe Date: Wed, 30 Oct 2024 10:05:29 -0400 Subject: [PATCH 30/31] reusable func and cleanup --- api/src/adapters/aws/__init__.py | 3 ++- api/src/adapters/aws/aws_session.py | 11 ++++++++ .../opportunities_v1/opportunity_schemas.py | 2 +- .../opportunities_v1/get_opportunity.py | 18 +++---------- api/tests/lib/seed_local_db.py | 25 ++++++++++--------- 5 files changed, 31 insertions(+), 28 deletions(-) create mode 100644 api/src/adapters/aws/aws_session.py diff --git a/api/src/adapters/aws/__init__.py b/api/src/adapters/aws/__init__.py index 3f55ab312..f7484c740 100644 --- a/api/src/adapters/aws/__init__.py +++ b/api/src/adapters/aws/__init__.py @@ -1,3 +1,4 @@ +from .aws_session import get_boto_session from .s3_adapter import S3Config, get_s3_client -__all__ = ["get_s3_client", "S3Config"] +__all__ = ["get_s3_client", "S3Config", "get_boto_session"] diff --git a/api/src/adapters/aws/aws_session.py b/api/src/adapters/aws/aws_session.py new file mode 100644 index 000000000..de781999b --- /dev/null +++ b/api/src/adapters/aws/aws_session.py @@ -0,0 +1,11 @@ +import os + +import boto3 + + +def get_boto_session() -> boto3.Session: + is_local = bool(os.getenv("IS_LOCAL_AWS", False)) + if is_local: + return boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") + + return boto3.Session() diff --git a/api/src/api/opportunities_v1/opportunity_schemas.py b/api/src/api/opportunities_v1/opportunity_schemas.py index 3c208c17d..a6b4347cb 100644 --- a/api/src/api/opportunities_v1/opportunity_schemas.py +++ b/api/src/api/opportunities_v1/opportunity_schemas.py @@ -320,7 +320,7 @@ class OpportunityV1Schema(Schema): class OpportunityAttachmentV1Schema(Schema): download_path = fields.String( metadata={ - "description": "The pre-signed s3 URL to download the attachment", + "description": "The URL to download the attachment", "example": "https://...", } ) diff --git a/api/src/services/opportunities_v1/get_opportunity.py b/api/src/services/opportunities_v1/get_opportunity.py index a0638ae7f..fa1134bae 100644 --- a/api/src/services/opportunities_v1/get_opportunity.py +++ b/api/src/services/opportunities_v1/get_opportunity.py @@ -1,14 +1,11 @@ -import os from datetime import date -from typing import List -import boto3 from sqlalchemy import select from sqlalchemy.orm import noload, selectinload import src.adapters.db as db import src.util.datetime_util as datetime_util -from src.adapters.aws import S3Config, get_s3_client +from src.adapters.aws import S3Config, get_boto_session, get_s3_client from src.api.route_utils import raise_flask_error from src.db.models.agency_models import Agency from src.db.models.opportunity_models import Opportunity, OpportunityAttachment, OpportunitySummary @@ -40,17 +37,9 @@ def _fetch_opportunity( return opportunity -def get_boto_session() -> boto3.Session: - is_local = bool(os.getenv("IS_LOCAL_AWS", False)) - if is_local: - return boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS") - - return boto3.Session() - - def pre_sign_opportunity_file_location( - opp_atts: List, -) -> List[OpportunityAttachment]: + opp_atts: list, +) -> list[OpportunityAttachment]: s3_config = S3Config() s3_client = get_s3_client(s3_config, get_boto_session()) @@ -63,6 +52,7 @@ def pre_sign_opportunity_file_location( ExpiresIn=s3_config.presigned_s3_duration, ) if s3_config.s3_endpoint_url: + # Only relevant when local, due to docker path issues pre_sign_file_loc = pre_sign_file_loc.replace( s3_config.s3_endpoint_url, "http://localhost:4566" ) diff --git a/api/tests/lib/seed_local_db.py b/api/tests/lib/seed_local_db.py index 31e05dbde..c634f280e 100644 --- a/api/tests/lib/seed_local_db.py +++ b/api/tests/lib/seed_local_db.py @@ -13,6 +13,7 @@ import src.util.datetime_util as datetime_util import tests.src.db.models.factories as factories from src.adapters.aws import S3Config, get_s3_client +from src.adapters.db import PostgresDBClient from src.db.models.agency_models import Agency from src.db.models.opportunity_models import Opportunity from src.db.models.transfer.topportunity_models import TransferTopportunity @@ -202,15 +203,15 @@ def seed_local_db(iterations: int, include_history: bool) -> None: error_if_not_local() _upload_opportunity_attachments_s3() - # - # db_client = PostgresDBClient() - # - # with db_client.get_session() as db_session: - # factories._db_session = db_session - # - # _build_opportunities(db_session, iterations, include_history) - # # Need to commit to force any updates made - # # after factories created objects - # db_session.commit() - # - # _build_agencies(db_session) + + db_client = PostgresDBClient() + + with db_client.get_session() as db_session: + factories._db_session = db_session + + _build_opportunities(db_session, iterations, include_history) + # Need to commit to force any updates made + # after factories created objects + db_session.commit() + + _build_agencies(db_session) From 894723129d9edf10c6c68d4795776a9bc63b8e62 Mon Sep 17 00:00:00 2001 From: nava-platform-bot Date: Wed, 30 Oct 2024 14:07:43 +0000 Subject: [PATCH 31/31] Create ERD diagram and Update OpenAPI spec --- api/openapi.generated.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml index 384321edb..91f2f8ccf 100644 --- a/api/openapi.generated.yml +++ b/api/openapi.generated.yml @@ -2038,7 +2038,7 @@ components: properties: download_path: type: string - description: The pre-signed s3 URL to download the attachment + description: The URL to download the attachment example: https://... mime_type: type: string