Standardize Database Query Parameter Handling Across Repository Classes

CHANGELOG.md

@@ -12,6 +12,7 @@ ENHANCEMENTS:
 * Streamline Azure bootstrap by replacing legacy dependency checks with `bootstrap_azure_env.sh` ([#2993](https://github.com/microsoft/AzureTRE/issues/2993))
 * Create Azure provider registration script ([#2993](https://github.com/microsoft/AzureTRE/issues/4653))
 * Update oauth2-proxy and Tomcat versions to latest in Guacamole container ([#4688](https://github.com/microsoft/AzureTRE/pull/4688)])
+* Standardize database query parameter handling across repository classes ([#4697](https://github.com/microsoft/AzureTRE/issues/4697))
 
 BUG FIXES:
 * Fix disable public network access for stwebcertsTREID is still flagging in Defender ([#4640](https://github.com/microsoft/AzureTRE/issues/4640))

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.24.10"
+__version__ = "0.25.0"

api_app/api/routes/workspaces.py

@@ -1,6 +1,7 @@
 import asyncio
 
-from fastapi import APIRouter, Depends, HTTPException, Header, status, Request, Response
+from fastapi import APIRouter, Depends, HTTPException, Header, Path, status, Request, Response
+from pydantic import UUID4
 
 from jsonschema.exceptions import ValidationError
 
@@ -362,8 +363,8 @@ async def retrieve_workspace_service_history_by_workspace_service_id(workspace_s
 # USER RESOURCE ROUTES
 @user_resources_workspace_router.get("/workspaces/{workspace_id}/workspace-services/{service_id}/user-resources", response_model=UserResourcesInList, name=strings.API_GET_MY_USER_RESOURCES, dependencies=[Depends(get_workspace_by_id_from_path)])
 async def retrieve_user_resources_for_workspace_service(
-        workspace_id: str,
-        service_id: str,
+        workspace_id: UUID4 = Path(...),
+        service_id: UUID4 = Path(...),
         user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager),
         resource_template_repo=Depends(get_repository(ResourceTemplateRepository)),
         user_resource_repo=Depends(get_repository(UserResourceRepository))) -> UserResourcesInList:

api_app/db/repositories/resource_templates.py

@@ -22,7 +22,12 @@ async def create(cls):
 
     @staticmethod
     def _template_by_name_query(name: str, resource_type: ResourceType) -> str:
-        return f'SELECT * FROM c WHERE c.resourceType = "{resource_type}" AND c.name = "{name}"'
+        query = 'SELECT * FROM c WHERE c.resourceType = @resourceType AND c.name = @name'
+        parameters = [
+            {'name': '@resourceType', 'value': resource_type},
+            {'name': '@name', 'value': name}
+        ]
+        return query, parameters
 
     @staticmethod
     def enrich_template(template: ResourceTemplate, is_update: bool = False) -> dict:
@@ -42,10 +47,14 @@ async def get_templates_information(self, resource_type: ResourceType, user_role
         :param user_roles: If set, only return templates that the user is authorized to use.
                            template.authorizedRoles should contain at least one of user_roles
         """
-        query = f'SELECT c.name, c.title, c.description, c.authorizedRoles FROM c WHERE c.resourceType = "{resource_type}" AND c.current = true'
+        query = 'SELECT c.name, c.title, c.description, c.authorizedRoles FROM c WHERE c.resourceType = @resourceType AND c.current = true'
+        parameters = [
+            {'name': '@resourceType', 'value': resource_type}
+        ]
         if resource_type == ResourceType.UserResource:
-            query += f' AND c.parentWorkspaceService = "{parent_service_name}"'
-        template_infos = await self.query(query=query)
+            query += ' AND c.parentWorkspaceService = @parentWorkspaceService'
+            parameters.append({'name': '@parentWorkspaceService', 'value': parent_service_name})
+        template_infos = await self.query(query=query, parameters=parameters)
         templates = [parse_obj_as(ResourceTemplateInformation, info) for info in template_infos]
 
         if not user_roles:
@@ -57,10 +66,12 @@ async def get_current_template(self, template_name: str, resource_type: Resource
         """
         Returns full template for the current version of the 'template_name' template
         """
-        query = self._template_by_name_query(template_name, resource_type) + ' AND c.current = true'
+        query, parameters = self._template_by_name_query(template_name, resource_type)
+        query += ' AND c.current = true'
         if resource_type == ResourceType.UserResource:
-            query += f' AND c.parentWorkspaceService = "{parent_service_name}"'
-        templates = await self.query(query=query)
+            query += ' AND c.parentWorkspaceService = @parentWorkspaceService'
+            parameters.append({'name': '@parentWorkspaceService', 'value': parent_service_name})
+        templates = await self.query(query=query, parameters=parameters)
         if len(templates) == 0:
             raise EntityDoesNotExist
         if len(templates) > 1:
@@ -76,17 +87,20 @@ async def get_template_by_name_and_version(self, name: str, version: str, resour
 
         For UserResource templates, you also need to pass in 'parent_service_name' as a parameter
         """
-        query = self._template_by_name_query(name, resource_type) + f' AND c.version = "{version}"'
+        query, parameters = self._template_by_name_query(name, resource_type)
+        query += ' AND c.version = @version'
+        parameters.append({'name': '@version', 'value': version})
 
         # If querying for a user resource, we also need to add the parentWorkspaceService (name) to the query
         if resource_type == ResourceType.UserResource:
             if parent_service_name:
-                query += f' AND c.parentWorkspaceService = "{parent_service_name}"'
+                query += ' AND c.parentWorkspaceService = @parentWorkspaceService'
+                parameters.append({'name': '@parentWorkspaceService', 'value': parent_service_name})
             else:
                 raise Exception("When getting a UserResource template, you must pass in a 'parent_service_name'")
 
         # Execute the query and handle results
-        templates = await self.query(query=query)
+        templates = await self.query(query=query, parameters=parameters)
         if len(templates) != 1:
             raise EntityDoesNotExist
         if resource_type == ResourceType.UserResource:

api_app/db/repositories/resources.py

@@ -30,16 +30,21 @@ async def create(cls):
         await super().create(config.STATE_STORE_RESOURCES_CONTAINER)
         return cls
 
-    @staticmethod
-    def _active_resources_query():
-        # get active docs (not deleted)
-        return f'SELECT * FROM c WHERE {IS_NOT_DELETED_CLAUSE}'
-
     def _active_resources_by_type_query(self, resource_type: ResourceType):
-        return self._active_resources_query() + f' AND c.resourceType = "{resource_type}"'
+        query = 'SELECT * FROM c WHERE c.deploymentStatus != @deletedStatus AND c.resourceType = @resourceType'
+        parameters = [
+            {'name': '@deletedStatus', 'value': Status.Deleted},
+            {'name': '@resourceType', 'value': resource_type}
+        ]
+        return query, parameters
 
     def _active_resources_by_id_query(self, resource_id: str):
-        return self._active_resources_query() + f' AND c.id = "{resource_id}"'
+        query = 'SELECT * FROM c WHERE c.deploymentStatus != @deletedStatus AND c.id = @resourceId'
+        parameters = [
+            {'name': '@deletedStatus', 'value': Status.Deleted},
+            {'name': '@resourceId', 'value': resource_id}
+        ]
+        return query, parameters
 
     @staticmethod
     def _validate_resource_parameters(resource_input, resource_template):
@@ -76,8 +81,13 @@ async def get_resource_by_id(self, resource_id: UUID4) -> Resource:
         return parse_obj_as(Resource, resource)
 
     async def get_active_resource_by_template_name(self, template_name: str) -> Resource:
-        query = f"SELECT TOP 1 * FROM c WHERE c.templateName = '{template_name}' AND {IS_ACTIVE_RESOURCE}"
-        resources = await self.query(query=query)
+        query = "SELECT TOP 1 * FROM c WHERE c.templateName = @templateName AND c.deploymentStatus != @deletedStatus AND c.deploymentStatus != @failedStatus"
+        parameters = [
+            {'name': '@templateName', 'value': template_name},
+            {'name': '@deletedStatus', 'value': Status.Deleted},
+            {'name': '@failedStatus', 'value': Status.DeploymentFailed}
+        ]
+        resources = await self.query(query=query, parameters=parameters)
         if not resources:
             raise EntityDoesNotExist
         return parse_obj_as(Resource, resources[0])
@@ -130,8 +140,12 @@ async def get_resource_dependency_list(self, resource: Resource) -> List:
         dependent_resources_list = []
 
         # Get all related resources
-        related_resources_query = f"SELECT * FROM c WHERE CONTAINS(c.resourcePath, '{parent_resource_path}') AND c.deploymentStatus != '{Status.Deleted}'"
-        related_resources = await self.query(query=related_resources_query)
+        related_resources_query = "SELECT * FROM c WHERE CONTAINS(c.resourcePath, @resourcePath) AND c.deploymentStatus != @deletedStatus"
+        parameters = [
+            {'name': '@resourcePath', 'value': parent_resource_path},
+            {'name': '@deletedStatus', 'value': Status.Deleted}
+        ]
+        related_resources = await self.query(query=related_resources_query, parameters=parameters)
         for resource in related_resources:
             resource_path = resource["resourcePath"]
             resource_level = resource_path.count("/")
@@ -185,8 +199,3 @@ def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo:
 
     def get_timestamp(self) -> float:
         return datetime.utcnow().timestamp()
-
-
-# Cosmos query consts
-IS_NOT_DELETED_CLAUSE = f'c.deploymentStatus != "{Status.Deleted}"'
-IS_ACTIVE_RESOURCE = f'c.deploymentStatus != "{Status.Deleted}" and c.deploymentStatus != "{Status.DeploymentFailed}"'

api_app/db/repositories/resources_history.py

@@ -26,13 +26,17 @@ def is_valid_uuid(resourceId):
     def resource_history_query(self, resourceId: str):
         logger.debug("Validate sanity of resourceId")
         self.is_valid_uuid(resourceId)
-        return f'SELECT * FROM c WHERE c.resourceId = "{resourceId}"'
+        query = 'SELECT * FROM c WHERE c.resourceId = @resourceId'
+        parameters = [
+            {'name': '@resourceId', 'value': resourceId}
+        ]
+        return query, parameters
 
     async def get_resource_history_by_resource_id(self, resource_id: str) -> List[ResourceHistoryItem]:
-        query = self.resource_history_query(resource_id)
+        query, parameters = self.resource_history_query(resource_id)
         try:
             logger.info(f"Fetching history for resource {resource_id}")
-            resource_history_items = await self.query(query=query)
+            resource_history_items = await self.query(query=query, parameters=parameters)
             logger.debug(f"Got {len(resource_history_items)} history items for resource {resource_id}")
         except EntityDoesNotExist:
             logger.info(f"No history for resource {resource_id}")

api_app/db/repositories/shared_services.py

@@ -8,12 +8,13 @@
 from models.domain.authentication import User
 from db.repositories.resource_templates import ResourceTemplateRepository
 from db.repositories.resources_history import ResourceHistoryRepository
-from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE, IS_ACTIVE_RESOURCE
+from db.repositories.resources import ResourceRepository
 from db.errors import DuplicateEntity, EntityDoesNotExist
 from models.domain.shared_service import SharedService
 from models.schemas.resource import ResourcePatch
 from models.schemas.shared_service_template import SharedServiceTemplateInCreate
 from models.domain.resource import ResourceType
+from models.domain.operation import Status
 
 
 class SharedServiceRepository(ResourceRepository):
@@ -25,18 +26,36 @@ async def create(cls):
 
     @staticmethod
     def shared_service_query(shared_service_id: str):
-        return f'SELECT * FROM c WHERE c.resourceType = "{ResourceType.SharedService}" AND c.id = "{shared_service_id}"'
+        query = 'SELECT * FROM c WHERE c.resourceType = @resourceType AND c.id = @sharedServiceId'
+        parameters = [
+            {'name': '@resourceType', 'value': ResourceType.SharedService},
+            {'name': '@sharedServiceId', 'value': shared_service_id}
+        ]
+        return query, parameters
 
     @staticmethod
     def active_shared_services_query():
-        return f'SELECT * FROM c WHERE {IS_NOT_DELETED_CLAUSE} AND c.resourceType = "{ResourceType.SharedService}"'
+        query = 'SELECT * FROM c WHERE c.deploymentStatus != @deletedStatus AND c.resourceType = @resourceType'
+        parameters = [
+            {'name': '@deletedStatus', 'value': Status.Deleted},
+            {'name': '@resourceType', 'value': ResourceType.SharedService}
+        ]
+        return query, parameters
 
     @staticmethod
     def active_shared_service_with_template_name_query(template_name: str):
-        return f'SELECT * FROM c WHERE {IS_ACTIVE_RESOURCE} AND c.resourceType = "{ResourceType.SharedService}" AND c.templateName = "{template_name}"'
+        query = 'SELECT * FROM c WHERE c.deploymentStatus != @deletedStatus AND c.deploymentStatus != @failedStatus AND c.resourceType = @resourceType AND c.templateName = @templateName'
+        parameters = [
+            {'name': '@deletedStatus', 'value': Status.Deleted},
+            {'name': '@failedStatus', 'value': Status.DeploymentFailed},
+            {'name': '@resourceType', 'value': ResourceType.SharedService},
+            {'name': '@templateName', 'value': template_name}
+        ]
+        return query, parameters
 
     async def get_shared_service_by_id(self, shared_service_id: str):
-        shared_services = await self.query(self.shared_service_query(shared_service_id))
+        query, parameters = self.shared_service_query(str(shared_service_id))
+        shared_services = await self.query(query=query, parameters=parameters)
         if not shared_services:
             raise EntityDoesNotExist
         return parse_obj_as(SharedService, shared_services[0])
@@ -45,8 +64,8 @@ async def get_active_shared_services(self) -> List[SharedService]:
         """
         returns list of "non-deleted" shared services linked to this shared
         """
-        query = SharedServiceRepository.active_shared_services_query()
-        shared_services = await self.query(query=query)
+        query, parameters = SharedServiceRepository.active_shared_services_query()
+        shared_services = await self.query(query=query, parameters=parameters)
         return parse_obj_as(List[SharedService], shared_services)
 
     def get_shared_service_spec_params(self):
@@ -55,7 +74,8 @@ def get_shared_service_spec_params(self):
     async def create_shared_service_item(self, shared_service_input: SharedServiceTemplateInCreate, user_roles: List[str]) -> Tuple[SharedService, ResourceTemplate]:
         shared_service_id = str(uuid.uuid4())
 
-        existing_shared_services = await self.query(self.active_shared_service_with_template_name_query(shared_service_input.templateName))
+        query, parameters = self.active_shared_service_with_template_name_query(shared_service_input.templateName)
+        existing_shared_services = await self.query(query=query, parameters=parameters)
 
         # Duplicate is same template (=id), same version and deployed
         if existing_shared_services:

api_app/db/repositories/user_resources.py

@@ -9,7 +9,8 @@
 import resources.strings as strings
 from db.errors import EntityDoesNotExist
 from db.repositories.resource_templates import ResourceTemplateRepository
-from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE
+from db.repositories.resources import ResourceRepository
+from models.domain.operation import Status
 from models.domain.resource import ResourceType
 from models.domain.user_resource import UserResource
 from models.schemas.resource import ResourcePatch
@@ -25,11 +26,24 @@ async def create(cls):
 
     @staticmethod
     def user_resources_query(workspace_id: str, service_id: str):
-        return f'SELECT * FROM c WHERE c.resourceType = "{ResourceType.UserResource}" AND c.parentWorkspaceServiceId = "{service_id}" AND c.workspaceId = "{workspace_id}"'
+        query = 'SELECT * FROM c WHERE c.resourceType = @resourceType AND c.parentWorkspaceServiceId = @serviceId AND c.workspaceId = @workspaceId'
+        parameters = [
+            {'name': '@resourceType', 'value': ResourceType.UserResource},
+            {'name': '@serviceId', 'value': service_id},
+            {'name': '@workspaceId', 'value': workspace_id}
+        ]
+        return query, parameters
 
     @staticmethod
     def active_user_resources_query(workspace_id: str, service_id: str):
-        return f'SELECT * FROM c WHERE {IS_NOT_DELETED_CLAUSE} AND c.resourceType = "{ResourceType.UserResource}" AND c.parentWorkspaceServiceId = "{service_id}" AND c.workspaceId = "{workspace_id}"'
+        query = 'SELECT * FROM c WHERE c.deploymentStatus != @deletedStatus AND c.resourceType = @resourceType AND c.parentWorkspaceServiceId = @serviceId AND c.workspaceId = @workspaceId'
+        parameters = [
+            {'name': '@deletedStatus', 'value': Status.Deleted},
+            {'name': '@resourceType', 'value': ResourceType.UserResource},
+            {'name': '@serviceId', 'value': service_id},
+            {'name': '@workspaceId', 'value': workspace_id}
+        ]
+        return query, parameters
 
     async def create_user_resource_item(self, user_resource_input: UserResourceInCreate, workspace_id: str, parent_workspace_service_id: str, parent_template_name: str, user_id: str, user_roles: List[str], owner_id: str = None) -> Tuple[UserResource, ResourceTemplate]:
         full_user_resource_id = str(uuid.uuid4())
@@ -57,13 +71,15 @@ async def get_user_resources_for_workspace_service(self, workspace_id: str, serv
         """
         returns a list of "non-deleted" user resources linked to this workspace service
         """
-        query = self.active_user_resources_query(workspace_id, service_id)
-        user_resources = await self.query(query=query)
+        query, parameters = self.active_user_resources_query(str(workspace_id), str(service_id))
+        user_resources = await self.query(query=query, parameters=parameters)
         return parse_obj_as(List[UserResource], user_resources)
 
     async def get_user_resource_by_id(self, workspace_id: str, service_id: str, resource_id: str) -> UserResource:
-        query = self.user_resources_query(workspace_id, service_id) + f' AND c.id = "{resource_id}"'
-        user_resources = await self.query(query=query)
+        query, parameters = self.user_resources_query(str(workspace_id), str(service_id))
+        query += ' AND c.id = @resourceId'
+        parameters.append({'name': '@resourceId', 'value': str(resource_id)})
+        user_resources = await self.query(query=query, parameters=parameters)
         if not user_resources:
             raise EntityDoesNotExist
         return parse_obj_as(UserResource, user_resources[0])