Changes for sqlalchemy 2.0

app/dao/fact_billing_dao.py

@@ -779,45 +779,37 @@ def fetch_daily_volumes_for_platform(start_date, end_date):
             FactBilling.local_date,
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.SMS,
                         FactBilling.notifications_sent,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("sms_totals"),
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.SMS,
                         FactBilling.billable_units,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("sms_fragment_totals"),
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.SMS,
                         FactBilling.billable_units * FactBilling.rate_multiplier,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("sms_fragments_times_multiplier"),
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.EMAIL,
                         FactBilling.notifications_sent,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("email_totals"),
@@ -897,34 +889,28 @@ def fetch_volumes_by_service(start_date, end_date):
             FactBilling.service_id,
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.SMS,
                         FactBilling.notifications_sent,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("sms_totals"),
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.SMS,
                         FactBilling.billable_units * FactBilling.rate_multiplier,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("sms_fragments_times_multiplier"),
             func.sum(
                 case(
-                    [
                     (
                         FactBilling.notification_type == NotificationType.EMAIL,
                         FactBilling.notifications_sent,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("email_totals"),

app/dao/fact_notification_status_dao.py

@@ -459,25 +459,21 @@ def get_total_notifications_for_date_range(start_date, end_date):
             FactNotificationStatus.local_date.label("local_date"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_type
                         == NotificationType.EMAIL,
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("emails"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_type
                         == NotificationType.SMS,
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("sms"),
@@ -507,32 +503,27 @@ def fetch_monthly_notification_statuses_per_service(start_date, end_date):
             FactNotificationStatus.notification_type,
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_status.in_(
                             [NotificationStatus.SENDING, NotificationStatus.PENDING]
                         ),
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("count_sending"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_status
                         == NotificationStatus.DELIVERED,
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("count_delivered"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_status.in_(
                             [
@@ -541,44 +532,37 @@ def fetch_monthly_notification_statuses_per_service(start_date, end_date):
                             ]
                         ),
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("count_technical_failure"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_status
                         == NotificationStatus.TEMPORARY_FAILURE,
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("count_temporary_failure"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_status
                         == NotificationStatus.PERMANENT_FAILURE,
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("count_permanent_failure"),
             func.sum(
                 case(
-                    [
                     (
                         FactNotificationStatus.notification_status
                         == NotificationStatus.SENT,
                         FactNotificationStatus.notification_count,
-                        )
+                    ),
-                    ],
                     else_=0,
                 )
             ).label("count_sent"),

app/dao/fact_processing_time_dao.py

@@ -40,7 +40,6 @@ def get_processing_time_percentage_for_date_range(start_date, end_date):
             FactProcessingTime.messages_total,
             FactProcessingTime.messages_within_10_secs,
             case(
-                [
                 (
                     FactProcessingTime.messages_total > 0,
                     (
@@ -52,7 +51,6 @@ def get_processing_time_percentage_for_date_range(start_date, end_date):
                     ),
                 ),
                 (FactProcessingTime.messages_total == 0, 100.0),
-                ]
             ).label("percentage"),
         )
         .filter(

app/dao/inbound_sms_dao.py

@@ -6,7 +6,7 @@ from sqlalchemy.orm import aliased
 from app import db
 from app.dao.dao_utils import autocommit
 from app.enums import NotificationType
-from app.models import InboundSms, InboundSmsHistory, Service, ServiceDataRetention
+from app.models import InboundSms, InboundSmsHistory, ServiceDataRetention
 from app.utils import midnight_n_days_ago
 
 
@@ -122,9 +122,7 @@ def delete_inbound_sms_older_than_retention():
     )
 
     flexible_data_retention = (
-        ServiceDataRetention.query.join(
+        ServiceDataRetention.query.join(ServiceDataRetention.service)
-            ServiceDataRetention.service, Service.inbound_number
-        )
         .filter(ServiceDataRetention.notification_type == NotificationType.SMS)
         .all()
     )

app/dao/notifications_dao.py

@@ -7,7 +7,7 @@ from notifications_utils.recipients import (
     try_validate_and_format_phone_number,
     validate_and_format_email_address,
 )
-from sqlalchemy import asc, desc, or_, select, union
+from sqlalchemy import asc, desc, or_, select, text, union
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.sql import functions
@@ -386,15 +386,15 @@ def insert_notification_history_delete_notifications(
         "qry_limit": qry_limit,
     }
 
-    db.session.execute(select_into_temp_table, input_params)
+    db.session.execute(text(select_into_temp_table), input_params)
 
-    result = db.session.execute("select count(*) from NOTIFICATION_ARCHIVE").fetchone()[
+    result = db.session.execute(
-        0
+        text("select count(*) from NOTIFICATION_ARCHIVE")
-    ]
+    ).fetchone()[0]
 
-    db.session.execute(insert_query)
+    db.session.execute(text(insert_query))
 
-    db.session.execute(delete_query)
+    db.session.execute(text(delete_query))
 
     return result
 
@@ -585,7 +585,7 @@ def dao_get_notifications_processing_time_stats(start_date, end_date):
     )
 
     result = db.session.execute(stmt)
-    return result.scalar_one()
+    return result.one()
 
 
 def dao_get_last_notification_added_for_job_id(job_id):

app/dao/permissions_dao.py

@@ -26,7 +26,9 @@ class PermissionDAO(DAOClass):
     ):
         try:
             if replace:
-                query = self.Meta.model.query.filter_by(user=user, service=service)
+                query = self.Meta.model.query.filter(
+                    self.Meta.model.user == user, self.Meta.model.service == service
+                )
                 query.delete()
             for p in permissions:
                 p.user = user

app/dao/service_user_dao.py

@@ -4,7 +4,12 @@ from app.models import ServiceUser, User
 
 
 def dao_get_service_user(user_id, service_id):
-    return ServiceUser.query.filter_by(user_id=user_id, service_id=service_id).one()
+    # TODO: This has been changed to account for the test case failure
+    # that used this method but have any service user to return. Somehow, this
+    # started to throw an error with one() method in sqlalchemy 2.0 unlike 1.4
+    return ServiceUser.query.filter_by(
+        user_id=user_id, service_id=service_id
+    ).one_or_none()
 
 
 def dao_get_active_service_users(service_id):

app/dao/services_dao.py

@@ -102,23 +102,17 @@ def dao_fetch_live_services_data():
             Service.volume_sms.label("sms_volume_intent"),
             Service.volume_email.label("email_volume_intent"),
             case(
-                [
                 (
-                        this_year_ft_billing.c.notification_type
+                    this_year_ft_billing.c.notification_type == NotificationType.EMAIL,
-                        == NotificationType.EMAIL,
                     func.sum(this_year_ft_billing.c.notifications_sent),
-                    )
+                ),
-                ],
                 else_=0,
             ).label("email_totals"),
             case(
-                [
                 (
-                        this_year_ft_billing.c.notification_type
+                    this_year_ft_billing.c.notification_type == NotificationType.SMS,
-                        == NotificationType.SMS,
                     func.sum(this_year_ft_billing.c.notifications_sent),
-                    )
+                ),
-                ],
                 else_=0,
             ).label("sms_totals"),
             AnnualBilling.free_sms_fragment_limit,
@@ -178,15 +172,15 @@ def dao_fetch_live_services_data():
 def dao_fetch_service_by_id(service_id, only_active=False):
     stmt = (
         select(Service)
-        .options(joinedload(Service.users))
         .where(Service.id == service_id)
+        .options(joinedload(Service.users))
     )
 
     if only_active:
         stmt = stmt.where(Service.active)
 
     result = db.session.execute(stmt)
-    return result.unique().scalars().first()
+    return result.unique().scalars().one()
 
 
 def dao_fetch_service_by_inbound_number(number):
@@ -241,8 +235,7 @@ def dao_archive_service(service_id):
     # to ensure that db.session still contains the models when it comes to creating history objects
     service = (
         Service.query.options(
-            joinedload(Service.templates),
+            joinedload(Service.templates).subqueryload(Template.template_redacted),
-            joinedload(Service.templates.template_redacted),
             joinedload(Service.api_keys),
         )
         .filter(Service.id == service_id)
@@ -329,6 +322,11 @@ def dao_add_user_to_service(service, user, permissions=None, folder_permissions=
     try:
         from app.dao.permissions_dao import permission_dao
 
+        # As per SQLAlchemy 2.0, we need to add the user to the service only if the user is not already added;
+        # otherwise it throws sqlalchemy.exc.IntegrityError:
+        # (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "uix_user_to_service"
+        service_user = dao_get_service_user(user.id, service.id)
+        if service_user is None:
             service.users.append(user)
         permission_dao.set_user_service_permission(
             user, service, permissions, _commit=False

app/dao/users_dao.py

@@ -2,6 +2,8 @@ import uuid
 from datetime import datetime, timedelta
 from secrets import randbelow
 
+import sqlalchemy
+from flask import current_app
 from sqlalchemy import func
 from sqlalchemy.orm import joinedload
 
@@ -11,7 +13,7 @@ from app.dao.permissions_dao import permission_dao
 from app.dao.service_user_dao import dao_get_service_users_by_user_id
 from app.enums import AuthType, PermissionType
 from app.errors import InvalidRequest
-from app.models import User, VerifyCode
+from app.models import Organization, Service, User, VerifyCode
 from app.utils import escape_special_characters, get_archived_db_column_value
 
 
@@ -39,10 +41,21 @@ def get_login_gov_user(login_uuid, email_address):
     user = User.query.filter_by(login_uuid=login_uuid).first()
     if user:
         if user.email_address != email_address:
+            try:
                 save_user_attribute(user, {"email_address": email_address})
+            except sqlalchemy.exc.IntegrityError as ie:
+                # We are trying to change the email address as a courtesy,
+                # based on the assumption that the user has somehow changed their
+                # address in login.gov.
+                # But if we cannot change the email address, at least we don't
+                # want to fail here, otherwise the user will be locked out.
+                current_app.logger.error(ie)
+                db.session.rollback()
+
         return user
     # Remove this 1 July 2025, all users should have login.gov uuids by now
-    user = User.query.filter_by(email_address=email_address).first()
+    user = User.query.filter(User.email_address.ilike(email_address)).first()
+
     if user:
         save_user_attribute(user, {"login_uuid": login_uuid})
         return user
@@ -172,15 +185,14 @@ def update_user_password(user, password):
 
 
 def get_user_and_accounts(user_id):
+    # TODO: With sqlalchemy 2.0 change as below because of the breaking change
+    # at User.organizations.services, we need to verify that the below subqueryload
+    # that we have put is functionally doing the same thing as before
     return (
         User.query.filter(User.id == user_id)
         .options(
-            # eagerly load the user's services and organizations, and also the service's org and vice versa
+            joinedload(User.services).joinedload(Service.organization),
-            # (so we can see if the user knows about it)
+            joinedload(User.organizations).subqueryload(Organization.services),
-            joinedload(User.services),
-            joinedload(User.organizations),
-            joinedload(User.organizations.services),
-            joinedload(User.services.organization),
         )
         .one()
     )

app/status/healthcheck.py

@@ -1,4 +1,5 @@
 from flask import Blueprint, jsonify, request
+from sqlalchemy import text
 
 from app import db, version
 from app.dao.organization_dao import dao_count_organizations_with_live_services
@@ -37,5 +38,5 @@ def live_service_and_organization_counts():
 
 def get_db_version():
     query = "SELECT version_num FROM alembic_version"
-    full_name = db.session.execute(query).fetchone()[0]
+    full_name = db.session.execute(text(query)).fetchone()[0]
     return full_name

app/user/rest.py

@@ -6,6 +6,7 @@ from urllib.parse import urlencode
 from flask import Blueprint, abort, current_app, jsonify, request
 from notifications_utils.recipients import is_us_phone_number, use_numeric_sender
 from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm.exc import NoResultFound
 
 from app import redis_store
 from app.config import QueueNames
@@ -533,6 +534,12 @@ def set_permissions(user_id, service_id):
     # TODO fix security hole, how do we verify that the user
     # who is making this request has permission to make the request.
     service_user = dao_get_service_user(user_id, service_id)
+    # TODO: Below exception is raised to account for the test case failure that got handled
+    # on its own in 1.4 when dao_get_service_user() returned an excpetion in case no result was found
+    if service_user is None:
+        raise NoResultFound(
+            "No ServiceUser found with the provided user_id and service_id"
+        )
     user = get_user_by_id(user_id)
     service = dao_fetch_service_by_id(service_id=service_id)
 

tests/app/dao/test_fact_billing_dao.py

@@ -838,7 +838,7 @@ def test_fetch_sms_billing_for_all_services_with_remainder(notify_db_session):
         },
     ]
 
-    assert [dict(result) for result in results] == expected_results
+    assert [result._asdict() for result in results] == expected_results
 
 
 def test_fetch_sms_billing_for_all_services_without_an_organization_appears(
@@ -893,7 +893,7 @@ def test_fetch_sms_billing_for_all_services_without_an_organization_appears(
         },
     ]
 
-    assert [dict(result) for result in results] == expected_results
+    assert [result._asdict() for result in results] == expected_results
 
 
 @freeze_time("2019-06-01 13:30")
@@ -1228,8 +1228,8 @@ def test_query_organization_sms_usage_for_year_handles_multiple_services(
 
     result = query_organization_sms_usage_for_year(org.id, 2022).all()
 
-    service_1_rows = [row for row in result if row.service_id == service_1.id]
+    service_1_rows = [row._asdict() for row in result if row.service_id == service_1.id]
-    service_2_rows = [row for row in result if row.service_id == service_2.id]
+    service_2_rows = [row._asdict() for row in result if row.service_id == service_2.id]
 
     assert len(service_1_rows) == 2
     assert len(service_2_rows) == 2
@@ -1256,10 +1256,10 @@ def test_query_organization_sms_usage_for_year_handles_multiple_services(
 
     # assert total costs are accurate
     assert (
-        float(sum(row.cost for row in service_1_rows)) == 1
+        float(sum(row["cost"] for row in service_1_rows)) == 1
     )  # rows with 2 and 4, allowance of 5
     assert (
-        float(sum(row.cost for row in service_2_rows)) == 14
+        float(sum(row["cost"] for row in service_2_rows)) == 14
     )  # rows with 8 and 16, allowance of 10
 
 
@@ -1296,7 +1296,10 @@ def test_query_organization_sms_usage_for_year_handles_multiple_rates(
         financial_year_start=current_year,
     )
 
-    result = query_organization_sms_usage_for_year(org.id, 2022).all()
+    result = [
+        row._asdict()
+        for row in query_organization_sms_usage_for_year(org.id, 2022).all()
+    ]
 
     # al lthe free allowance is used on the first day
     assert result[0]["local_date"] == date(2022, 4, 29)

tests/app/template_folder/test_template_folder_rest.py

@@ -237,7 +237,7 @@ def test_update_template_folder_users(admin_request, sample_service):
     [
         ({}, "name is a required property"),
         ({"name": None}, "name None is not of type string"),
-        ({"name": ""}, "name  is too short"),
+        ({"name": ""}, "name  should be non-empty"),
     ],
 )
 def test_update_template_folder_fails_if_missing_name(

tests/app/webauthn/test_rest.py

@@ -89,7 +89,7 @@ def test_create_webauthn_credential_returns_201(admin_request, sample_user):
             "name None is not of type string",
         ),
         # name is empty
-        ({"name": "", "credential_data": "ABC123"}, "name  is too short"),
+        ({"name": "", "credential_data": "ABC123"}, "name  should be non-empty"),
     ],
 )
 def test_create_webauthn_credential_errors_if_schema_violation(
@@ -131,7 +131,7 @@ def test_update_webauthn_credential_returns_200(admin_request, sample_user):
         # name is null
         ({"name": None}, "name None is not of type string"),
         # name is empty
-        ({"name": ""}, "name  is too short"),
+        ({"name": ""}, "name  should be non-empty"),
     ],
 )
 def test_update_webauthn_credential_errors_if_schema_violation(