From 89907be508ce8addc2d7b19b981493506228231a Mon Sep 17 00:00:00 2001 From: James Person Date: Fri, 16 Aug 2024 15:45:51 -0400 Subject: [PATCH 1/2] API Version 1.1.1 - Include the Materialized View (#4190) * Add API version 1.1.1, including materialized view * A comment update, linting * Drop/create views when reupping materialized view * Drop views: typo fix --- .../api/api_v1_1_0/create_schema.sql | 11 +- backend/dissemination/api/api_v1_1_1/base.sql | 29 ++ .../api/api_v1_1_1/create_functions.sql | 59 +++ .../api/api_v1_1_1/create_schema.sql | 48 ++ .../api/api_v1_1_1/create_views.sql | 410 ++++++++++++++++++ .../api/api_v1_1_1/drop_schema.sql | 11 + .../api/api_v1_1_1/drop_views.sql | 18 + backend/dissemination/api_versions.py | 19 +- .../management/commands/create_api_views.py | 1 + .../management/commands/materialized_views.py | 3 + backend/docker-compose-web.yml | 2 +- backend/docker-compose.yml | 2 +- .../api/admin_api_v1_1_0/create_functions.sql | 4 +- 13 files changed, 606 insertions(+), 11 deletions(-) create mode 100644 backend/dissemination/api/api_v1_1_1/base.sql create mode 100644 backend/dissemination/api/api_v1_1_1/create_functions.sql create mode 100644 backend/dissemination/api/api_v1_1_1/create_schema.sql create mode 100644 backend/dissemination/api/api_v1_1_1/create_views.sql create mode 100644 backend/dissemination/api/api_v1_1_1/drop_schema.sql create mode 100644 backend/dissemination/api/api_v1_1_1/drop_views.sql diff --git a/backend/dissemination/api/api_v1_1_0/create_schema.sql b/backend/dissemination/api/api_v1_1_0/create_schema.sql index 9075b4aecb..33b9bd4161 100644 --- a/backend/dissemination/api/api_v1_1_0/create_schema.sql +++ b/backend/dissemination/api/api_v1_1_0/create_schema.sql @@ -33,14 +33,13 @@ end $$ ; --- This is the description +-- https://postgrest.org/en/stable/references/api/openapi.html +-- This is the title (version number) and description (text). COMMENT ON SCHEMA api_v1_1_0 IS - 'The FAC dissemation API version 1.0.3.' -; +$$v1.1.0 + +A RESTful API that serves data from the SF-SAC.$$; --- https://postgrest.org/en/stable/references/api/openapi.html --- This is the title -COMMENT ON SCHEMA api_v1_1_0 IS 'A RESTful API that serves data from the SF-SAC.'; commit; diff --git a/backend/dissemination/api/api_v1_1_1/base.sql b/backend/dissemination/api/api_v1_1_1/base.sql new file mode 100644 index 0000000000..dedabe0cb7 --- /dev/null +++ b/backend/dissemination/api/api_v1_1_1/base.sql @@ -0,0 +1,29 @@ +DO +$do$ +BEGIN + IF EXISTS ( + SELECT FROM pg_catalog.pg_roles + WHERE rolname = 'authenticator') THEN + RAISE NOTICE 'Role "authenticator" already exists. Skipping.'; + ELSE + CREATE ROLE authenticator LOGIN NOINHERIT NOCREATEDB NOCREATEROLE NOSUPERUSER; + END IF; +END +$do$; + +DO +$do$ +BEGIN + IF EXISTS ( + SELECT FROM pg_catalog.pg_roles + WHERE rolname = 'api_fac_gov') THEN + RAISE NOTICE 'Role "api_fac_gov" already exists. Skipping.'; + ELSE + CREATE ROLE api_fac_gov NOLOGIN; + END IF; +END +$do$; + +GRANT api_fac_gov TO authenticator; + +NOTIFY pgrst, 'reload schema'; diff --git a/backend/dissemination/api/api_v1_1_1/create_functions.sql b/backend/dissemination/api/api_v1_1_1/create_functions.sql new file mode 100644 index 0000000000..a3e772cec6 --- /dev/null +++ b/backend/dissemination/api/api_v1_1_1/create_functions.sql @@ -0,0 +1,59 @@ +-- WARNING +-- Under PostgreSQL 12, the functions below work. +-- Under PostgreSQL 14, these will break. +-- +-- Note the differences: +-- +-- raise info 'Works under PostgreSQL 12'; +-- raise info 'request.header.x-magic %', (SELECT current_setting('request.header.x-magic', true)); +-- raise info 'request.jwt.claim.expires %', (SELECT current_setting('request.jwt.claim.expires', true)); +-- raise info 'Works under PostgreSQL 14'; +-- raise info 'request.headers::json->>x-magic %', (SELECT current_setting('request.headers', true)::json->>'x-magic'); +-- raise info 'request.jwt.claims::json->expires %', (SELECT current_setting('request.jwt.claims', true)::json->>'expires'); +-- +-- To quote the work of Dav Pilkey, "remember this now." + + +CREATE OR REPLACE FUNCTION api_v1_1_1_functions.get_header(item text) RETURNS text + AS $get_header$ + declare res text; + begin + SELECT (current_setting('request.headers', true)::json)->>item into res; + return res; + end; +$get_header$ LANGUAGE plpgsql; + +create or replace function api_v1_1_1_functions.get_api_key_uuid() returns TEXT +as $gaku$ +declare uuid text; +begin + select api_v1_1_1_functions.get_header('x-api-user-id') into uuid; + return uuid; +end; +$gaku$ LANGUAGE plpgsql; + +create or replace function api_v1_1_1_functions.has_tribal_data_access() +returns boolean +as $has_tribal_data_access$ +DECLARE + uuid_header UUID; + key_exists boolean; +BEGIN + + SELECT api_v1_1_1_functions.get_api_key_uuid() INTO uuid_header; + SELECT + CASE WHEN EXISTS ( + SELECT key_id + FROM public.dissemination_TribalApiAccessKeyIds taaki + WHERE taaki.key_id = uuid_header::TEXT) + THEN 1::BOOLEAN + ELSE 0::BOOLEAN + END + INTO key_exists; + RAISE INFO 'api_v1_1_1 has_tribal % %', uuid_header, key_exists; + RETURN key_exists; +END; +$has_tribal_data_access$ LANGUAGE plpgsql; + + +NOTIFY pgrst, 'reload schema'; diff --git a/backend/dissemination/api/api_v1_1_1/create_schema.sql b/backend/dissemination/api/api_v1_1_1/create_schema.sql new file mode 100644 index 0000000000..2b6a58d553 --- /dev/null +++ b/backend/dissemination/api/api_v1_1_1/create_schema.sql @@ -0,0 +1,48 @@ +begin; + +do +$$ +begin + DROP SCHEMA IF EXISTS api_v1_1_1 CASCADE; + DROP SCHEMA IF EXISTS api_v1_1_1_functions CASCADE; + + if not exists (select schema_name from information_schema.schemata where schema_name = 'api_v1_1_1') then + create schema api_v1_1_1; + create schema api_v1_1_1_functions; + + grant usage on schema api_v1_1_1_functions to api_fac_gov; + + -- Grant access to tables and views + alter default privileges + in schema api_v1_1_1 + grant select + -- this includes views + on tables + to api_fac_gov; + + -- Grant access to sequences, if we have them + grant usage on schema api_v1_1_1 to api_fac_gov; + grant select, usage on all sequences in schema api_v1_1_1 to api_fac_gov; + alter default privileges + in schema api_v1_1_1 + grant select, usage + on sequences + to api_fac_gov; + end if; +end +$$ +; + +-- https://postgrest.org/en/stable/references/api/openapi.html +-- This is the title (version number) and description (text). +COMMENT ON SCHEMA api_v1_1_1 IS +$$v1.1.1 + +A RESTful API that serves data from the SF-SAC.$$; + + +commit; + +notify pgrst, + 'reload schema'; + diff --git a/backend/dissemination/api/api_v1_1_1/create_views.sql b/backend/dissemination/api/api_v1_1_1/create_views.sql new file mode 100644 index 0000000000..7fdb73e109 --- /dev/null +++ b/backend/dissemination/api/api_v1_1_1/create_views.sql @@ -0,0 +1,410 @@ +begin; + +--------------------------------------- +-- finding_text +--------------------------------------- +create view api_v1_1_1.findings_text as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + ft.finding_ref_number, + ft.contains_chart_or_table, + ft.finding_text + from + dissemination_findingtext ft, + dissemination_general gen + where + ft.report_id = gen.report_id + and + (gen.is_public = true + or (gen.is_public = false and api_v1_1_1_functions.has_tribal_data_access())) + order by ft.id +; + +--------------------------------------- +-- additional_ueis +--------------------------------------- +create view api_v1_1_1.additional_ueis as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + uei.additional_uei + from + dissemination_general gen, + dissemination_additionaluei uei + where + gen.report_id = uei.report_id + order by uei.id +; + +--------------------------------------- +-- finding +--------------------------------------- +create view api_v1_1_1.findings as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + finding.award_reference, + finding.reference_number, + finding.is_material_weakness, + finding.is_modified_opinion, + finding.is_other_findings, + finding.is_other_matters, + finding.prior_finding_ref_numbers, + finding.is_questioned_costs, + finding.is_repeat_finding, + finding.is_significant_deficiency, + finding.type_requirement + from + dissemination_finding finding, + dissemination_general gen + where + finding.report_id = gen.report_id + order by finding.id +; + +--------------------------------------- +-- federal award +--------------------------------------- +create view api_v1_1_1.federal_awards as + select + award.report_id, + gen.auditee_uei, + gen.audit_year, + --- + award.award_reference, + award.federal_agency_prefix, + award.federal_award_extension, + award.additional_award_identification, + award.federal_program_name, + award.amount_expended, + award.cluster_name, + award.other_cluster_name, + award.state_cluster_name, + award.cluster_total, + award.federal_program_total, + award.is_major, + award.is_loan, + award.loan_balance, + award.is_direct, + award.audit_report_type, + award.findings_count, + award.is_passthrough_award, + award.passthrough_amount + from + dissemination_federalaward award, + dissemination_general gen + where + award.report_id = gen.report_id + order by award.id +; + + +--------------------------------------- +-- corrective_action_plan +--------------------------------------- +create view api_v1_1_1.corrective_action_plans as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + ct.finding_ref_number, + ct.contains_chart_or_table, + ct.planned_action + from + dissemination_CAPText ct, + dissemination_General gen + where + ct.report_id = gen.report_id + and + (gen.is_public = true + or (gen.is_public = false and api_v1_1_1_functions.has_tribal_data_access())) + order by ct.id +; + +--------------------------------------- +-- notes_to_sefa +--------------------------------------- +create view api_v1_1_1.notes_to_sefa as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + note.note_title as title, + note.accounting_policies, + note.is_minimis_rate_used, + note.rate_explained, + note.content, + note.contains_chart_or_table + from + dissemination_general gen, + dissemination_note note + where + note.report_id = gen.report_id + and + (gen.is_public = true + or (gen.is_public = false and api_v1_1_1_functions.has_tribal_data_access())) + order by note.id +; + +--------------------------------------- +-- passthrough +--------------------------------------- +create view api_v1_1_1.passthrough as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + pass.award_reference, + pass.passthrough_id, + pass.passthrough_name + from + dissemination_general as gen, + dissemination_passthrough as pass + where + gen.report_id = pass.report_id + order by pass.id +; + + +--------------------------------------- +-- general +--------------------------------------- +create view api_v1_1_1.general as + select + -- every table starts with report_id, UEI, and year + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + gen.auditee_certify_name, + gen.auditee_certify_title, + gen.auditee_contact_name, + gen.auditee_email, + gen.auditee_name, + gen.auditee_phone, + gen.auditee_contact_title, + gen.auditee_address_line_1, + gen.auditee_city, + gen.auditee_state, + gen.auditee_ein, + gen.auditee_zip, + -- auditor + gen.auditor_certify_name, + gen.auditor_certify_title, + gen.auditor_phone, + gen.auditor_state, + gen.auditor_city, + gen.auditor_contact_title, + gen.auditor_address_line_1, + gen.auditor_zip, + gen.auditor_country, + gen.auditor_contact_name, + gen.auditor_email, + gen.auditor_firm_name, + gen.auditor_foreign_address, + gen.auditor_ein, + -- agency + gen.cognizant_agency, + gen.oversight_agency, + -- dates + gen.date_created, + gen.ready_for_certification_date, + gen.auditor_certified_date, + gen.auditee_certified_date, + gen.submitted_date, + gen.fac_accepted_date, + gen.fy_end_date, + gen.fy_start_date, + gen.audit_type, + gen.gaap_results, + gen.sp_framework_basis, + gen.is_sp_framework_required, + gen.sp_framework_opinions, + gen.is_going_concern_included, + gen.is_internal_control_deficiency_disclosed, + gen.is_internal_control_material_weakness_disclosed, + gen.is_material_noncompliance_disclosed, + gen.dollar_threshold, + gen.is_low_risk_auditee, + gen.agencies_with_prior_findings, + gen.entity_type, + gen.number_months, + gen.audit_period_covered, + gen.total_amount_expended, + gen.type_audit_code, + gen.is_public, + gen.data_source, + gen.is_aicpa_audit_guide_included, + gen.is_additional_ueis, + CASE EXISTS(SELECT ein.report_id FROM dissemination_additionalein ein WHERE ein.report_id = gen.report_id) + WHEN FALSE THEN 'No' + ELSE 'Yes' + END AS is_multiple_eins, + CASE EXISTS(SELECT aud.report_id FROM dissemination_secondaryauditor aud WHERE aud.report_id = gen.report_id) + WHEN FALSE THEN 'No' + ELSE 'Yes' + END AS is_secondary_auditors + from + dissemination_general gen + order by gen.id +; + +--------------------------------------- +-- auditor (secondary auditor) +--------------------------------------- +create view api_v1_1_1.secondary_auditors as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + sa.auditor_ein, + sa.auditor_name, + sa.contact_name, + sa.contact_title, + sa.contact_email, + sa.contact_phone, + sa.address_street, + sa.address_city, + sa.address_state, + sa.address_zipcode + from + dissemination_General gen, + dissemination_SecondaryAuditor sa + where + sa.report_id = gen.report_id + order by sa.id +; + +create view api_v1_1_1.additional_eins as + select + gen.report_id, + gen.auditee_uei, + gen.audit_year, + --- + ein.additional_ein + from + dissemination_general gen, + dissemination_additionalein ein + where + gen.report_id = ein.report_id + order by ein.id +; + +-- Specify every field in dissemination_combined, omitting the id. +-- Generated fields like ALN are done in the creation of the table, not here. +create view api_v1_1_1.combined as + select + combined.report_id, + combined.award_reference, + combined.reference_number, + combined.aln, + combined.agencies_with_prior_findings, + combined.audit_period_covered, + combined.audit_type, + combined.audit_year, + combined.auditee_address_line_1, + combined.auditee_certified_date, + combined.auditee_certify_name, + combined.auditee_certify_title, + combined.auditee_city, + combined.auditee_contact_name, + combined.auditee_contact_title, + combined.auditee_ein, + combined.auditee_email, + combined.auditee_name, + combined.auditee_phone, + combined.auditee_state, + combined.auditee_uei, + combined.auditee_zip, + combined.auditor_address_line_1, + combined.auditor_certified_date, + combined.auditor_certify_name, + combined.auditor_certify_title, + combined.auditor_city, + combined.auditor_contact_name, + combined.auditor_contact_title, + combined.auditor_country, + combined.auditor_ein, + combined.auditor_email, + combined.auditor_firm_name, + combined.auditor_foreign_address, + combined.auditor_phone, + combined.auditor_state, + combined.auditor_zip, + combined.cognizant_agency, + combined.data_source, + combined.date_created, + combined.dollar_threshold, + combined.entity_type, + combined.fac_accepted_date, + combined.fy_end_date, + combined.fy_start_date, + combined.gaap_results, + combined.is_additional_ueis, + combined.is_aicpa_audit_guide_included, + combined.is_going_concern_included, + combined.is_internal_control_deficiency_disclosed, + combined.is_internal_control_material_weakness_disclosed, + combined.is_low_risk_auditee, + combined.is_material_noncompliance_disclosed, + combined.is_public, + combined.is_sp_framework_required, + combined.number_months, + combined.oversight_agency, + combined.ready_for_certification_date, + combined.sp_framework_basis, + combined.sp_framework_opinions, + combined.submitted_date, + combined.total_amount_expended, + combined.type_audit_code, + combined.additional_award_identification, + combined.amount_expended, + combined.cluster_name, + combined.cluster_total, + combined.federal_agency_prefix, + combined.federal_award_extension, + combined.federal_program_name, + combined.federal_program_total, + combined.findings_count, + combined.is_direct, + combined.is_loan, + combined.is_major, + combined.is_passthrough_award, + combined.loan_balance, + combined.audit_report_type, + combined.other_cluster_name, + combined.passthrough_amount, + combined.state_cluster_name, + combined.is_material_weakness, + combined.is_modified_opinion, + combined.is_other_findings, + combined.is_other_matters, + combined.is_questioned_costs, + combined.is_repeat_finding, + combined.is_significant_deficiency, + combined.prior_finding_ref_numbers, + combined.type_requirement, + combined.passthrough_name, + combined.passthrough_id + from + dissemination_combined combined + where + (combined.is_public = true + or (combined.is_public = false and api_v1_1_1_functions.has_tribal_data_access())) + order by combined.id +; + +commit; + +notify pgrst, + 'reload schema'; + diff --git a/backend/dissemination/api/api_v1_1_1/drop_schema.sql b/backend/dissemination/api/api_v1_1_1/drop_schema.sql new file mode 100644 index 0000000000..705153e878 --- /dev/null +++ b/backend/dissemination/api/api_v1_1_1/drop_schema.sql @@ -0,0 +1,11 @@ + +begin; + +DROP SCHEMA IF EXISTS api_v1_1_1 CASCADE; +-- DROP ROLE IF EXISTS authenticator; +-- DROP ROLE IF EXISTS api_fac_gov; + +commit; + +notify pgrst, + 'reload schema'; diff --git a/backend/dissemination/api/api_v1_1_1/drop_views.sql b/backend/dissemination/api/api_v1_1_1/drop_views.sql new file mode 100644 index 0000000000..c95c0fc139 --- /dev/null +++ b/backend/dissemination/api/api_v1_1_1/drop_views.sql @@ -0,0 +1,18 @@ +begin; + drop table if exists api_v1_1_1.metadata; + drop view if exists api_v1_1_1.general; + drop view if exists api_v1_1_1.auditor; + drop view if exists api_v1_1_1.federal_awards; + drop view if exists api_v1_1_1.findings; + drop view if exists api_v1_1_1.findings_text; + drop view if exists api_v1_1_1.corrective_action_plans; + drop view if exists api_v1_1_1.additional_ueis; + drop view if exists api_v1_1_1.notes_to_sefa; + drop view if exists api_v1_1_1.passthrough; + drop view if exists api_v1_1_1.secondary_auditors; + drop view if exists api_v1_1_1.additional_eins; + drop view if exists api_v1_1_1.combined; +commit; + +notify pgrst, + 'reload schema'; diff --git a/backend/dissemination/api_versions.py b/backend/dissemination/api_versions.py index ccd6c7ec61..574f015b0f 100644 --- a/backend/dissemination/api_versions.py +++ b/backend/dissemination/api_versions.py @@ -6,7 +6,10 @@ logger = logging.getLogger(__name__) # These are API versions we want live. -live = {"dissemination": ["api_v1_0_3", "api_v1_1_0"], "support": ["admin_api_v1_1_0"]} +live = { + "dissemination": ["api_v1_0_3", "api_v1_1_0", "api_v1_1_1"], + "support": ["admin_api_v1_1_0"], +} # These are API versions we have deprecated. # They will be removed. It should be safe to leave them @@ -44,6 +47,20 @@ def exec_sql(location, version, filename): curs.execute(sql) +def create_materialized_view(location): + """ + Create or recreate the dissemination_combined materialized view. + We only want this done once on startup, regardless of the API version. + """ + conn = connection(get_conn_string()) + conn.autocommit = True + with conn.cursor() as curs: + path = f"{location}/sql/create_materialized_views.sql" + logger.info("EXEC SQL create_materialized_views.sql") + sql = open(path, "r").read() + curs.execute(sql) + + def create_views(location, version): exec_sql(location, version, "create_views.sql") diff --git a/backend/dissemination/management/commands/create_api_views.py b/backend/dissemination/management/commands/create_api_views.py index 105b87a780..4db39d481e 100644 --- a/backend/dissemination/management/commands/create_api_views.py +++ b/backend/dissemination/management/commands/create_api_views.py @@ -8,6 +8,7 @@ class Command(BaseCommand): """ def handle(self, *args, **kwargs): + api_versions.create_materialized_view("dissemination") api_versions.create_functions("dissemination") api_versions.create_functions("support") api_versions.create_live_views("dissemination") diff --git a/backend/dissemination/management/commands/materialized_views.py b/backend/dissemination/management/commands/materialized_views.py index 67acc95b5d..11c0b8ab14 100644 --- a/backend/dissemination/management/commands/materialized_views.py +++ b/backend/dissemination/management/commands/materialized_views.py @@ -15,7 +15,10 @@ def add_arguments(self, parser): def handle(self, *args, **options): path = "dissemination/sql" if options["create"]: + # Other API views may rely on the materialized view. So, drop and recreate them. + api_versions.drop_live_views("dissemination") api_versions.exec_sql_at_path(path, "create_materialized_views.sql") + api_versions.create_live_views("dissemination") elif options["drop"]: api_versions.exec_sql_at_path(path, "drop_materialized_views.sql") elif options["refresh"]: diff --git a/backend/docker-compose-web.yml b/backend/docker-compose-web.yml index 98deabef2e..f7f9c6017e 100644 --- a/backend/docker-compose-web.yml +++ b/backend/docker-compose-web.yml @@ -99,7 +99,7 @@ services: PGRST_OPENAPI_SERVER_PROXY_URI: http://127.0.0.1:3000 PGRST_DB_ANON_ROLE: anon # See https://postgrest.org/en/stable/references/api/schemas.html#multiple-schemas for multiple schemas - PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, admin_api_v1_1_0" + PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, api_v1_1_1, admin_api_v1_1_0" PGRST_JWT_SECRET: ${PGRST_JWT_SECRET:-32_chars_fallback_secret_testing} # Fallback value for testing environments depends_on: db: diff --git a/backend/docker-compose.yml b/backend/docker-compose.yml index f8ee53cada..ace79bc281 100644 --- a/backend/docker-compose.yml +++ b/backend/docker-compose.yml @@ -126,7 +126,7 @@ services: PGRST_OPENAPI_SERVER_PROXY_URI: http://127.0.0.1:3000 PGRST_DB_ANON_ROLE: anon # See https://postgrest.org/en/stable/references/api/schemas.html#multiple-schemas for multiple schemas - PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, admin_api_v1_1_0" + PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, api_v1_1_1, admin_api_v1_1_0" PGRST_JWT_SECRET: ${PGRST_JWT_SECRET:-32_chars_fallback_secret_testing} # Fallback value for testing environments # Enable this to inspect the DB plans for queries via EXPLAIN PGRST_DB_PLAN_ENABLED: ${PGRST_DB_PLAN_ENABLED:-false} diff --git a/backend/support/api/admin_api_v1_1_0/create_functions.sql b/backend/support/api/admin_api_v1_1_0/create_functions.sql index 0434cce5cf..336b2e2ccf 100644 --- a/backend/support/api/admin_api_v1_1_0/create_functions.sql +++ b/backend/support/api/admin_api_v1_1_0/create_functions.sql @@ -56,8 +56,8 @@ $log_admin_api_event$ LANGUAGE plpgsql; -- has_admin_data_access :: permission -> bool --- The permissions (insert, select, delete) allow us to have users who can --- read administrative data in addition to users who can (say) update +-- The permissions (insert, select, delete) allow us to have users who can +-- read administrative data in addition to users who can (say) update -- select tables like the tribal access lists. create or replace function admin_api_v1_1_0_functions.has_admin_data_access(perm TEXT) returns boolean as $has_admin_data_access$ From 16ce26476499f167727294a75420b309999d17e8 Mon Sep 17 00:00:00 2001 From: Dan Swick <2365503+danswick@users.noreply.github.com> Date: Fri, 16 Aug 2024 17:10:11 -0700 Subject: [PATCH 2/2] don't delete ExcelFile entries from the db (#4200) --- .../remove_workbook_artifacts.py | 141 +++++++++--------- .../test_remove_workbook_artifacts.py | 103 ++++++------- 2 files changed, 114 insertions(+), 130 deletions(-) diff --git a/backend/dissemination/remove_workbook_artifacts.py b/backend/dissemination/remove_workbook_artifacts.py index 33d74c0495..c5cf4085d0 100644 --- a/backend/dissemination/remove_workbook_artifacts.py +++ b/backend/dissemination/remove_workbook_artifacts.py @@ -1,74 +1,67 @@ -import logging - -from django.conf import settings -from audit.models.models import ExcelFile -from boto3 import client as boto3_client -from botocore.client import ClientError, Config - -logger = logging.getLogger(__name__) - - -def remove_workbook_artifacts(sac): - """ - Remove all workbook artifacts associated with the given sac. - """ - try: - excel_files = ExcelFile.objects.filter(sac=sac) - files = [f"excel/{excel_file.filename}" for excel_file in excel_files] - - if files: - # Delete the records from the database - count = excel_files.count() - excel_files.delete() - logger.info( - f"Deleted {count} excelfile records from fac-db for report: {sac.report_id}" - ) - - # Delete the files from S3 in bulk - delete_files_in_bulk(files, sac) - - except ExcelFile.DoesNotExist: - logger.info(f"No files found to delete for report: {sac.report_id}") - except Exception as e: - logger.error( - f"Failed to delete files from fac-db and S3 for report: {sac.report_id}. Error: {e}" - ) - - -def delete_files_in_bulk(filenames, sac): - """Delete files from S3 in bulk.""" - # This client uses the internal endpoint URL because we're making a request to S3 from within the app - s3_client = boto3_client( - service_name="s3", - region_name=settings.AWS_S3_PRIVATE_REGION_NAME, - aws_access_key_id=settings.AWS_PRIVATE_ACCESS_KEY_ID, - aws_secret_access_key=settings.AWS_PRIVATE_SECRET_ACCESS_KEY, - endpoint_url=settings.AWS_S3_PRIVATE_INTERNAL_ENDPOINT, - config=Config(signature_version="s3v4"), - ) - - try: - delete_objects = [{"Key": filename} for filename in filenames] - - response = s3_client.delete_objects( - Bucket=settings.AWS_PRIVATE_STORAGE_BUCKET_NAME, - Delete={"Objects": delete_objects}, - ) - - deleted_files = response.get("Deleted", []) - for deleted in deleted_files: - logger.info( - f"Successfully deleted {deleted['Key']} from S3 for report: {sac.report_id}" - ) - - errors = response.get("Errors", []) - if errors: - for error in errors: - logger.error( - f"Failed to delete {error['Key']} from S3 for report: {sac.report_id}. Error: {error['Message']}" # nosec B608 - ) - - except ClientError as e: - logger.error( - f"Failed to delete files from S3 for report: {sac.report_id}. Error: {e}" - ) +import logging + +from django.conf import settings +from audit.models.models import ExcelFile +from boto3 import client as boto3_client +from botocore.client import ClientError, Config + +logger = logging.getLogger(__name__) + + +def remove_workbook_artifacts(sac): + """ + Remove all workbook artifacts associated with the given sac. + """ + try: + excel_files = ExcelFile.objects.filter(sac=sac) + files = [f"excel/{excel_file.filename}" for excel_file in excel_files] + + if files: + # Delete the files from S3 in bulk + delete_files_in_bulk(files, sac) + + except ExcelFile.DoesNotExist: + logger.info(f"No files found to delete for report: {sac.report_id}") + except Exception as e: + logger.error( + f"Failed to delete files from S3 for report: {sac.report_id}. Error: {e}" + ) + + +def delete_files_in_bulk(filenames, sac): + """Delete files from S3 in bulk.""" + # This client uses the internal endpoint URL because we're making a request to S3 from within the app + s3_client = boto3_client( + service_name="s3", + region_name=settings.AWS_S3_PRIVATE_REGION_NAME, + aws_access_key_id=settings.AWS_PRIVATE_ACCESS_KEY_ID, + aws_secret_access_key=settings.AWS_PRIVATE_SECRET_ACCESS_KEY, + endpoint_url=settings.AWS_S3_PRIVATE_INTERNAL_ENDPOINT, + config=Config(signature_version="s3v4"), + ) + + try: + delete_objects = [{"Key": filename} for filename in filenames] + + response = s3_client.delete_objects( + Bucket=settings.AWS_PRIVATE_STORAGE_BUCKET_NAME, + Delete={"Objects": delete_objects}, + ) + + deleted_files = response.get("Deleted", []) + for deleted in deleted_files: + logger.info( + f"Successfully deleted {deleted['Key']} from S3 for report: {sac.report_id}" + ) + + errors = response.get("Errors", []) + if errors: + for error in errors: + logger.error( + f"Failed to delete {error['Key']} from S3 for report: {sac.report_id}. Error: {error['Message']}" # nosec B608 + ) + + except ClientError as e: + logger.error( + f"Failed to delete files from S3 for report: {sac.report_id}. Error: {e}" + ) diff --git a/backend/dissemination/test_remove_workbook_artifacts.py b/backend/dissemination/test_remove_workbook_artifacts.py index 157a93c9b2..6749897983 100644 --- a/backend/dissemination/test_remove_workbook_artifacts.py +++ b/backend/dissemination/test_remove_workbook_artifacts.py @@ -1,56 +1,47 @@ -from django.test import TestCase -from unittest.mock import patch -from audit.models.models import ExcelFile, SingleAuditChecklist -from model_bakery import baker - -from dissemination.remove_workbook_artifacts import remove_workbook_artifacts - - -class RemovedWorkbookArtifactsTestCase(TestCase): - - @patch("dissemination.remove_workbook_artifacts.delete_files_in_bulk") - def test_removed_workbook_artifacts_success(self, mock_delete_files_in_bulk): - sac = baker.make( - SingleAuditChecklist, - submission_status=SingleAuditChecklist.STATUS.IN_PROGRESS, - report_id="test_report_id", - ) - - # Create ExcelFile instances - excel_file_1 = baker.make(ExcelFile, sac=sac, form_section="fake_section") - excel_file_2 = baker.make( - ExcelFile, sac=sac, form_section="another_fake_section" - ) - - remove_workbook_artifacts(sac) - - # Assert that the ExcelFile instances are deleted - self.assertFalse(ExcelFile.objects.filter(sac=sac).exists()) - - # Assert S3 bulk delete was called with the correct filenames - mock_delete_files_in_bulk.assert_called_once_with( - [ - f"excel/{sac.report_id}--{excel_file_1.form_section}.xlsx", - f"excel/{sac.report_id}--{excel_file_2.form_section}.xlsx", - ], - sac, - ) - - @patch("dissemination.remove_workbook_artifacts.delete_files_in_bulk") - def test_removed_workbook_artifacts_no_files(self, mock_delete_files_in_bulk): - sac = baker.make( - SingleAuditChecklist, - submission_status=SingleAuditChecklist.STATUS.IN_PROGRESS, - report_id="test_report_id", - ) - - # Ensure no ExcelFile instances exist for this SAC - ExcelFile.objects.filter(sac=sac).delete() - - remove_workbook_artifacts(sac) - - # Assert that no ExcelFile instances exist - self.assertFalse(ExcelFile.objects.filter(sac=sac).exists()) - - # Assert S3 bulk delete was not called - mock_delete_files_in_bulk.assert_not_called() +from django.test import TestCase +from unittest.mock import patch +from audit.models.models import ExcelFile, SingleAuditChecklist +from model_bakery import baker + +from dissemination.remove_workbook_artifacts import remove_workbook_artifacts + + +class RemovedWorkbookArtifactsTestCase(TestCase): + + @patch("dissemination.remove_workbook_artifacts.delete_files_in_bulk") + def test_removed_workbook_artifacts_success(self, mock_delete_files_in_bulk): + sac = baker.make( + SingleAuditChecklist, + submission_status=SingleAuditChecklist.STATUS.IN_PROGRESS, + report_id="test_report_id", + ) + + # Create ExcelFile instances + excel_file_1 = baker.make(ExcelFile, sac=sac, form_section="fake_section") + excel_file_2 = baker.make( + ExcelFile, sac=sac, form_section="another_fake_section" + ) + + remove_workbook_artifacts(sac) + + # Assert S3 bulk delete was called with the correct filenames + mock_delete_files_in_bulk.assert_called_once_with( + [ + f"excel/{sac.report_id}--{excel_file_1.form_section}.xlsx", + f"excel/{sac.report_id}--{excel_file_2.form_section}.xlsx", + ], + sac, + ) + + @patch("dissemination.remove_workbook_artifacts.delete_files_in_bulk") + def test_removed_workbook_artifacts_no_files(self, mock_delete_files_in_bulk): + sac = baker.make( + SingleAuditChecklist, + submission_status=SingleAuditChecklist.STATUS.IN_PROGRESS, + report_id="test_report_id", + ) + + remove_workbook_artifacts(sac) + + # Assert S3 bulk delete was not called + mock_delete_files_in_bulk.assert_not_called()