Skip to content

Commit

Permalink
Try to make research_database_info less singleton-like; fix test suit…
Browse files Browse the repository at this point in the history
…e for RNC environment
  • Loading branch information
RudolfCardinal committed Jan 9, 2025
1 parent cf7f1f4 commit 2f459c7
Show file tree
Hide file tree
Showing 12 changed files with 405 additions and 103 deletions.
18 changes: 10 additions & 8 deletions crate_anon/anonymise/tests/researcher_report_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,24 @@
"""

import os.path
import random
from tempfile import NamedTemporaryFile
from tempfile import TemporaryDirectory
from typing import List, TYPE_CHECKING
from unittest import mock

import factory
from pypdf import PdfReader
import pytest

from sqlalchemy import (
Column,
DateTime,
ForeignKey,
Integer,
Text,
)

from sqlalchemy.orm import relationship


from crate_anon.anonymise.researcher_report import (
mk_researcher_report_pdf,
ResearcherReportConfig,
Expand Down Expand Up @@ -160,6 +158,8 @@ def setUp(self) -> None:
)
self.anon_dbsession.commit()

self.tempdir = TemporaryDirectory()

@pytest.mark.usefixtures("django_test_settings")
def test_report_has_pages_for_each_table(self) -> None:
def index_of_list_substring(items: List[str], substr: str) -> int:
Expand All @@ -171,7 +171,9 @@ def index_of_list_substring(items: List[str], substr: str) -> int:

anon_config = mock.Mock()

with NamedTemporaryFile(delete=False, mode="w") as f:
reportfilename = os.path.join(self.tempdir.name, "tmpreport.pdf")

with open(reportfilename, mode="w") as f:
mock_db = mock.Mock(
table_names=["anon_patient", "anon_note"],
metadata=AnonTestBase.metadata,
Expand All @@ -182,15 +184,15 @@ def index_of_list_substring(items: List[str], substr: str) -> int:
__post_init__=mock.Mock(),
):
report_config = ResearcherReportConfig(
output_filename=f.name,
output_filename=reportfilename,
anonconfig=anon_config,
use_dd=False,
)
report_config.db_session = self.anon_dbsession
report_config.db = mock_db
mk_researcher_report_pdf(report_config)

with open(f.name, "rb") as f:
with open(reportfilename, "rb") as f:
reader = PdfReader(f)

patient_found = False
Expand All @@ -212,7 +214,7 @@ def index_of_list_substring(items: List[str], substr: str) -> int:
num_rows = int(lines[rows_index + 1])
table_name = lines[0]

if table_name == "patient":
if table_name == "anon_patient":
patient_found = True
self.assertEqual(num_rows, self.num_patients)

Expand Down
2 changes: 2 additions & 0 deletions crate_anon/crateweb/anonymise_api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@

@override_settings(ANONYMISE_API=DEFAULT_SETTINGS)
class AnonymisationTests(TestCase):
databases = {"default", "research"}

def setUp(self) -> None:
super().setUp()

Expand Down
3 changes: 2 additions & 1 deletion crate_anon/crateweb/config/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@
and EnvVar.RUNNING_TESTS not in os.environ
):
from crate_anon.crateweb.research.research_db_info import (
research_database_info,
get_research_db_info,
)

research_database_info = get_research_db_info()
research_database_info.get_colinfolist()

log = logging.getLogger(__name__)
Expand Down
3 changes: 2 additions & 1 deletion crate_anon/crateweb/consent/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
)
from crate_anon.crateweb.research.models import get_mpid
from crate_anon.crateweb.research.research_db_info import (
research_database_info,
get_research_db_info,
)
from crate_anon.crateweb.userprofile.models import UserProfile

Expand Down Expand Up @@ -2038,6 +2038,7 @@ def process_request_main(self) -> None:
) # delayed import

# Translate to an NHS number
research_database_info = get_research_db_info()
dbinfo = research_database_info.dbinfo_for_contact_lookup
if self.lookup_nhs_number is not None:
self.nhs_number = self.lookup_nhs_number
Expand Down
4 changes: 3 additions & 1 deletion crate_anon/crateweb/consent/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
)
from crate_anon.crateweb.extra.pdf import serve_html_or_pdf
from crate_anon.crateweb.research.research_db_info import (
research_database_info,
get_research_db_info,
)
from crate_anon.crateweb.research.models import PidLookup, get_mpid
from crate_anon.crateweb.userprofile.models import UserProfile
Expand Down Expand Up @@ -504,6 +504,7 @@ def submit_contact_request(request: HttpRequest) -> HttpResponse:
Args:
request: the :class:`django.http.request.HttpRequest`
"""
research_database_info = get_research_db_info()
dbinfo = research_database_info.dbinfo_for_contact_lookup
if request.user.is_superuser:
form = SuperuserSubmitContactRequestForm(
Expand Down Expand Up @@ -579,6 +580,7 @@ def clinician_initiated_contact_request(request: HttpRequest) -> HttpResponse:
Args:
request: the :class:`django.http.request.HttpRequest`
"""
research_database_info = get_research_db_info()
dbinfo = research_database_info.dbinfo_for_contact_lookup
email = request.user.email
userprofile = UserProfile.objects.get(user=request.user)
Expand Down
3 changes: 2 additions & 1 deletion crate_anon/crateweb/research/archive_func.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
archive_template_url,
)
from crate_anon.crateweb.research.research_db_info import (
research_database_info,
get_research_db_info,
)
from crate_anon.crateweb.research.views import (
FN_SRCDB,
Expand Down Expand Up @@ -151,6 +151,7 @@ def delimit_sql_identifier(identifer: str) -> str:
"""
Delimits (quotes) an SQL identifier, if required.
"""
research_database_info = get_research_db_info()
return research_database_info.grammar.quote_identifier_if_required(
identifer
)
Expand Down
37 changes: 20 additions & 17 deletions crate_anon/crateweb/research/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
register_for_json,
)
from cardinal_pythonlib.reprfunc import simple_repr
from cardinal_pythonlib.sql.sql_grammar import format_sql, SqlGrammar
from cardinal_pythonlib.sql.sql_grammar import format_sql
from cardinal_pythonlib.tsv import make_tsv_row
from cardinal_pythonlib.django.function_cache import django_cache_function
from django.db import connections, DatabaseError, models
Expand Down Expand Up @@ -82,7 +82,7 @@
)
from crate_anon.crateweb.research.research_db_info import (
RESEARCH_DB_CONNECTION_NAME,
research_database_info,
get_research_db_info,
SingleResearchDatabase,
)
from crate_anon.crateweb.research.sql_writer import (
Expand Down Expand Up @@ -185,6 +185,7 @@ def database_last_updated(dbname: str) -> Optional[datetime.datetime]:
are null, the function will return the minimum date possible. If there
are no such tables, the function will return None.
"""
research_database_info = get_research_db_info()
try:
dbinfo = research_database_info.get_dbinfo_by_name(dbname)
except ValueError:
Expand Down Expand Up @@ -1704,7 +1705,7 @@ def _get_select_mrid_column(self) -> Optional[ColumnId]:
"""
if not self._patient_conditions:
return None
return research_database_info.get_linked_mrid_column(
return self._research_database_info.get_linked_mrid_column(
self._patient_conditions[0].table_id
)

Expand Down Expand Up @@ -1748,7 +1749,6 @@ def patient_id_query(self, with_order_by: bool = True) -> str:
if not self._patient_conditions:
return ""

grammar = research_database_info.grammar
select_mrid_column = self._get_select_mrid_column()
if not select_mrid_column.is_valid:
log.warning(
Expand All @@ -1758,6 +1758,10 @@ def patient_id_query(self, with_order_by: bool = True) -> str:
# One way this can happen: (1) a user saves a PMQ; (2) the
# administrator removes one of the databases!
return ""

research_database_info = get_research_db_info()
grammar = research_database_info.grammar

mrid_alias = "_mrid"
sql = add_to_select(
"",
Expand Down Expand Up @@ -1847,7 +1851,7 @@ def all_queries(self, mrids: List[Any] = None) -> List[TableQueryArgs]:
return queries

def where_patient_clause(
self, table_id: TableId, grammar: SqlGrammar, mrids: List[Any] = None
self, table_id: TableId, mrids: List[Any] = None
) -> SqlArgsTupleType:
"""
Returns an SQL WHERE clauses similar to ``sometable.mrid IN (1, 2, 3)``
Expand All @@ -1857,15 +1861,13 @@ def where_patient_clause(
Args:
table_id: :class:`crate_anon.common.sql.TableId` for the table
whose MRID column we will apply the ``WHERE`` clause to
grammar: :class:`cardinal_pythonlib.sql.sql_grammar.SqlGrammar`
to use
mrids: list of MRIDs; if this is ``None`` or empty, use the
patients fetched (live) by our :meth:`patient_id_query`.
Returns:
tuple: ``sql, args``
"""
mrid_column = research_database_info.get_mrid_column_from_table(
mrid_column = self._research_database_info.get_mrid_column_from_table(
table_id
)
if mrids:
Expand All @@ -1881,6 +1883,8 @@ def where_patient_clause(
# derived tables, subqueries, ... unless TOP, OFFSET or FOR XML
# is specified."
args = [] # type: List[Any]
research_database_info = get_research_db_info()
grammar = research_database_info.grammar
sql = f"{mrid_column.identifier(grammar)} IN ({in_clause})"
return sql, args

Expand Down Expand Up @@ -1914,6 +1918,7 @@ def make_query(
"""
if not columns:
raise ValueError("No columns specified")
research_database_info = get_research_db_info()
grammar = research_database_info.grammar
mrid_column = research_database_info.get_mrid_column_from_table(
table_id
Expand All @@ -1922,9 +1927,7 @@ def make_query(
for c in columns:
if c not in all_columns:
all_columns.append(c)
where_clause, args = self.where_patient_clause(
table_id, grammar, mrids
)
where_clause, args = self.where_patient_clause(table_id, mrids)
select_elements = [SelectElement(column_id=col) for col in all_columns]
where_conditions = [WhereCondition(raw_sql=where_clause)]
sql = add_to_select(
Expand All @@ -1946,6 +1949,7 @@ def output_cols_html(self) -> str:
"""
Returns all our output columns in HTML format.
"""
research_database_info = get_research_db_info()
grammar = research_database_info.grammar
return prettify_sql_html(
"\n".join(
Expand All @@ -1961,6 +1965,7 @@ def pt_conditions_html(self) -> str:
"""
Returns all our patient WHERE conditions in HTML format.
"""
research_database_info = get_research_db_info()
grammar = research_database_info.grammar
return prettify_sql_html(
"\nAND ".join([wc.sql(grammar) for wc in self.patient_conditions])
Expand Down Expand Up @@ -2030,6 +2035,7 @@ def gen_data_finder_queries(
:class:`TableQueryArgs` objects (q.v.)
"""
research_database_info = get_research_db_info()
grammar = research_database_info.grammar
mrid_alias = "master_research_id"
table_name_alias = "table_name"
Expand All @@ -2053,9 +2059,7 @@ def gen_data_finder_queries(
max_date = "NULL"
# ... OK (at least in MySQL) to do:
# SELECT col1, COUNT(*), NULL FROM table GROUP BY col1;
where_clause, args = self.where_patient_clause(
table_id, grammar, mrids
)
where_clause, args = self.where_patient_clause(table_id, mrids)
table_identifier = table_id.identifier(grammar)
select_elements = [
SelectElement(column_id=mrid_col, alias=mrid_alias),
Expand Down Expand Up @@ -2116,16 +2120,15 @@ def gen_monster_queries(
:class:`TableQueryArgs` objects (q.v.)
"""
research_database_info = get_research_db_info()
grammar = research_database_info.grammar
for (
table_id
) in research_database_info.get_mrid_linkable_patient_tables():
mrid_col = research_database_info.get_mrid_column_from_table(
table=table_id
)
where_clause, args = self.where_patient_clause(
table_id, grammar, mrids
)
where_clause, args = self.where_patient_clause(table_id, mrids)
# We add the WHERE using our magic query machine, to get the joins
# right:
select_elements = [
Expand Down
Loading

0 comments on commit 2f459c7

Please sign in to comment.