Skip to content

Commit

Permalink
Gracefully handle Foreign Key Constraint Errors (#4406)
Browse files Browse the repository at this point in the history
Co-authored-by: Adam Sachs <[email protected]>
  • Loading branch information
ThomasLaPiana and adamsachs authored Nov 22, 2023
1 parent d1ff5f6 commit 15b79ee
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ The types of changes are:
## [2.24.0](https://github.com/ethyca/fides/compare/2.23.3...2.24.0)

### Added

- Adds fides_disable_banner config option to Fides.js [#4378](https://github.com/ethyca/fides/pull/4378)
- Deletions that fail due to foreign key constraints will now be more clearly communicated [#4406](https://github.com/ethyca/fides/pull/4378)
- Added support for a custom get preferences API call provided through Fides.init [#4375](https://github.com/ethyca/fides/pull/4375)
- Hidden custom privacy request fields in the Privacy Center [#4370](https://github.com/ethyca/fides/pull/4370)
- Backend System-level Cookie Support [#4383](https://github.com/ethyca/fides/pull/4383)
Expand All @@ -59,6 +61,7 @@ The types of changes are:
- Adds more granularity to tracking consent method, updates custom savePreferencesFn and FidesUpdated event to take consent method [#4419](https://github.com/ethyca/fides/pull/4419)

### Changed

- Add filtering and pagination to bulk vendor add table [#4351](https://github.com/ethyca/fides/pull/4351)
- Determine if the TCF overlay needs to surface based on backend calculated version hash [#4356](https://github.com/ethyca/fides/pull/4356)
- Moved Experiences and Preferences endpoints to Plus to take advantage of dynamic GVL [#4367](https://github.com/ethyca/fides/pull/4367)
Expand Down
18 changes: 17 additions & 1 deletion src/fides/api/db/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@
from collections import defaultdict
from typing import Any, Dict, List, Tuple

from fastapi import HTTPException
from loguru import logger as log
from sqlalchemy import and_, column
from sqlalchemy import delete as _delete
from sqlalchemy import or_
from sqlalchemy import update as _update
from sqlalchemy.dialects.postgresql import Insert as _insert
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.exc import IntegrityError, SQLAlchemyError
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.future import select
from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY

from fides.api.db.base import Base # type: ignore[attr-defined]
from fides.api.models.sql_models import ( # type: ignore[attr-defined]
Expand Down Expand Up @@ -308,6 +310,7 @@ async def delete_resource(

async with async_session.begin():
try:
# Automatically delete related resources if they are CTL objects
if hasattr(sql_model, "parent_key"):
log.debug("Deleting resource and its children")
query = (
Expand All @@ -324,6 +327,19 @@ async def delete_resource(
log.debug("Deleting resource")
query = _delete(sql_model).where(sql_model.fides_key == fides_key)
await async_session.execute(query)
except IntegrityError as err:
raw_error_text: str = err.orig.args[0]

if "violates foreign key constraint" in raw_error_text:
error_message = "Failed to delete resource! Foreign key constraint found, try deleting related resources first."
else:
error_message = "Failed to delete resource!"

log.bind(error="SQL Query integrity error!").error(raw_error_text)
raise HTTPException(
status_code=HTTP_422_UNPROCESSABLE_ENTITY,
detail=error_message,
)
except SQLAlchemyError:
error = errors.QueryError()
log.bind(error=error.detail["error"]).info( # type: ignore[index]
Expand Down
16 changes: 15 additions & 1 deletion tests/ctl/database/test_crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from uuid import uuid4

import pytest
from fastapi import HTTPException
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.ext.asyncio import AsyncSession

Expand Down Expand Up @@ -31,7 +32,6 @@ def fixture_created_resources(
created_keys = []
resource_type = request.param
to_create = ["foo", "foo.bar", "foo.bar.baz", "foo.boo"]

for key in to_create:
parent_key = ".".join(key.split(".")[:-1]) if "." in key else None

Expand Down Expand Up @@ -73,6 +73,20 @@ async def test_cascade_delete_taxonomy_children(
assert len(set(keys).intersection(remaining_keys)) == 0


@pytest.mark.integration
async def test_delete_fails_linked_resources(
linked_dataset,
async_session: AsyncSession,
) -> None:
"""Deleting a resource should fail if a linked resource (without cascade delete relationship) still exists."""
with pytest.raises(HTTPException) as e:
await delete_resource(
sql_models.Dataset, linked_dataset.fides_key, async_session
)

assert "try deleting related resources first" in str(e)


@pytest.fixture(scope="function")
def custom_field_definition_data_use(db):
custom_field_definition_data = {
Expand Down

0 comments on commit 15b79ee

Please sign in to comment.