LA-165: Fix taxonomy create endpoints (#5533)

src/fides/api/api/v1/endpoints/generic_overrides.py

@@ -1,22 +1,33 @@
-from typing import Dict, List, Optional, Type, Union
+from typing import Dict, List, Optional, Union
 
-from fastapi import APIRouter, Depends, Query, Security
+from fastapi import APIRouter, Depends, HTTPException, Query, Security
 from fastapi_pagination import Page, Params
 from fastapi_pagination.ext.async_sqlalchemy import paginate as async_paginate
 from fideslang.models import Dataset
 from sqlalchemy import not_
 from sqlalchemy.ext.asyncio import AsyncSession
+from sqlalchemy.orm import Session
 from sqlalchemy.sql.expression import select
 from starlette import status
+from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY
 
+from fides.api.api.deps import get_db
+from fides.api.common_exceptions import KeyOrNameAlreadyExists
 from fides.api.db.base_class import get_key_from_data
 from fides.api.db.crud import list_resource_query
 from fides.api.db.ctl_session import get_async_db
 from fides.api.models.connectionconfig import ConnectionConfig
 from fides.api.models.datasetconfig import DatasetConfig
 from fides.api.oauth.utils import verify_oauth_client
 from fides.api.schemas.filter_params import FilterParams
-from fides.api.schemas.taxonomy_extensions import DataCategory, DataSubject, DataUse
+from fides.api.schemas.taxonomy_extensions import (
+    DataCategory,
+    DataCategoryCreate,
+    DataSubject,
+    DataSubjectCreate,
+    DataUse,
+    DataUseCreate,
+)
 from fides.api.util.filter_utils import apply_filters_to_query
 from fides.common.api.scope_registry import (
     DATA_CATEGORY_CREATE,
@@ -28,6 +39,9 @@
 
 from fides.api.models.sql_models import (  # type: ignore[attr-defined] # isort: skip
     Dataset as CtlDataset,
+    DataCategory as DataCategoryDbModel,
+    DataSubject as DataSubjectDbModel,
+    DataUse as DataUseDbModel,
 )
 
 # We create routers to override specific methods in those defined in generic.py
@@ -96,28 +110,6 @@ async def list_dataset_paginated(
     return await async_paginate(db, filtered_query, pagination_params)
 
 
-async def create_with_key(
-    data: Union[DataUse, DataCategory, DataSubject],
-    model: Type[Union[DataUse, DataCategory, DataSubject]],
-    db: AsyncSession,
-) -> Dict:
-    """
-    Helper to create taxonomy resource when not given a fides_key.
-    Automatically re-enables disabled resources with the same name.
-    """
-    # If data with same name exists but is disabled, re-enable it
-    disabled_resource_with_name = db.query(model).filter(
-        model.key == data.name,  # type: ignore[union-attr]
-        model.active is False,
-    )
-    if disabled_resource_with_name:
-        return model.update(db=db, data=data, active=True)  # type: ignore[union-attr]
-    data.fides_key = get_key_from_data(
-        {"key": data.fides_key, "name": data.name}, model.__name__
-    )
-    return model.create(db=db, data=data.model_dump(mode="json"))  # type: ignore[union-attr]
-
-
 @data_use_router.post(
     "/data_use",
     dependencies=[Security(verify_oauth_client, scopes=[DATA_USE_CREATE])],
@@ -126,16 +118,35 @@ async def create_with_key(
     name="Create",
 )
 async def create_data_use(
-    data_use: DataUse,
-    db: AsyncSession = Depends(get_async_db),
+    data_use: DataUseCreate,
+    db: Session = Depends(get_db),
 ) -> Dict:
     """
     Create a data use. Updates existing data use if data use with name already exists and is disabled.
     """
     if data_use.fides_key is None:
-        await create_with_key(data_use, DataUse, db)
-
-    return await DataUse.create(db=db, data=data_use.model_dump(mode="json"))  # type: ignore[attr-defined]
+        disabled_resource_with_name = (
+            db.query(DataUseDbModel)
+            .filter(
+                DataUseDbModel.active.is_(False),
+                DataUseDbModel.name == data_use.name,
+            )
+            .first()
+        )
+        data_use.fides_key = get_key_from_data(
+            {"key": data_use.fides_key, "name": data_use.name}, DataUse.__name__
+        )
+        if disabled_resource_with_name:
+            data_use.active = True
+            return disabled_resource_with_name.update(db, data=data_use.model_dump(mode="json"))  # type: ignore[union-attr]
+        try:
+            return DataUseDbModel.create(db=db, data=data_use.model_dump(mode="json"))  # type: ignore[union-attr]
+        except KeyOrNameAlreadyExists:
+            raise HTTPException(
+                status_code=HTTP_422_UNPROCESSABLE_ENTITY,
+                detail=f"Data use with key {data_use.fides_key} or name {data_use.name} already exists.",
+            )
+    return DataUseDbModel.create(db=db, data=data_use.model_dump(mode="json"))
 
 
 @data_category_router.post(
@@ -146,17 +157,37 @@ async def create_data_use(
     name="Create",
 )
 async def create_data_category(
-    data_category: DataCategory,
-    db: AsyncSession = Depends(get_async_db),
+    data_category: DataCategoryCreate,
+    db: Session = Depends(get_db),
 ) -> Dict:
     """
     Create a data category
     """
 
     if data_category.fides_key is None:
-        await create_with_key(data_category, DataCategory, db)
-
-    return await DataCategory.create(db=db, data=data_category.model_dump(mode="json"))  # type: ignore[attr-defined]
+        disabled_resource_with_name = (
+            db.query(DataCategoryDbModel)
+            .filter(
+                DataCategoryDbModel.active.is_(False),
+                DataCategoryDbModel.name == data_category.name,
+            )
+            .first()
+        )
+        data_category.fides_key = get_key_from_data(
+            {"key": data_category.fides_key, "name": data_category.name},
+            DataCategory.__name__,
+        )
+        if disabled_resource_with_name:
+            data_category.active = True
+            return disabled_resource_with_name.update(db, data=data_category.model_dump(mode="json"))  # type: ignore[union-attr]
+        try:
+            return DataCategoryDbModel.create(db=db, data=data_category.model_dump(mode="json"))  # type: ignore[union-attr]
+        except KeyOrNameAlreadyExists:
+            raise HTTPException(
+                status_code=HTTP_422_UNPROCESSABLE_ENTITY,
+                detail=f"Data category with key {data_category.fides_key} or name {data_category.name} already exists.",
+            )
+    return DataCategoryDbModel.create(db=db, data=data_category.model_dump(mode="json"))
 
 
 @data_subject_router.post(
@@ -167,17 +198,37 @@ async def create_data_category(
     name="Create",
 )
 async def create_data_subject(
-    data_subject: DataSubject,
-    db: AsyncSession = Depends(get_async_db),
+    data_subject: DataSubjectCreate,
+    db: Session = Depends(get_db),
 ) -> Dict:
     """
     Create a data subject
     """
 
     if data_subject.fides_key is None:
-        await create_with_key(data_subject, DataSubject, db)
-
-    return await DataSubject.create(db=db, data=data_subject.model_dump(mode="json"))  # type: ignore[attr-defined]
+        disabled_resource_with_name = (
+            db.query(DataSubjectDbModel)
+            .filter(
+                DataSubjectDbModel.active.is_(False),
+                DataSubjectDbModel.name == data_subject.name,
+            )
+            .first()
+        )
+        data_subject.fides_key = get_key_from_data(
+            {"key": data_subject.fides_key, "name": data_subject.name},
+            DataSubject.__name__,
+        )
+        if disabled_resource_with_name:
+            data_subject.active = True
+            return disabled_resource_with_name.update(db, data=data_subject.model_dump(mode="json"))  # type: ignore[union-attr]
+        try:
+            return DataSubjectDbModel.create(db=db, data=data_subject.model_dump(mode="json"))  # type: ignore[union-attr]
+        except KeyOrNameAlreadyExists:
+            raise HTTPException(
+                status_code=HTTP_422_UNPROCESSABLE_ENTITY,
+                detail=f"Data subject with key {data_subject.fides_key} or name {data_subject.name} already exists.",
+            )
+    return DataSubjectDbModel.create(db=db, data=data_subject.model_dump(mode="json"))
 
 
 GENERIC_OVERRIDES_ROUTER = APIRouter()

src/fides/api/schemas/taxonomy_extensions.py

@@ -2,10 +2,14 @@
 Fides-specific extensions to the pydantic models of taxonomy elements as defined in fideslang.
 """
 
+from typing import List, Optional
+
 from fideslang.models import DataCategory as BaseDataCategory
 from fideslang.models import DataSubject as BaseDataSubject
+from fideslang.models import DataSubjectRights
 from fideslang.models import DataUse as BaseDataUse
-from pydantic import Field
+from fideslang.validation import FidesKey
+from pydantic import BaseModel, Field
 
 active_field = Field(
     default=True, description="Indicates whether the resource is currently 'active'."
@@ -22,3 +26,26 @@ class DataCategory(BaseDataCategory):
 
 class DataSubject(BaseDataSubject):
     active: bool = active_field
+
+
+class TaxonomyCreateBase(BaseModel):
+    name: Optional[str] = None
+    description: str
+    active: bool = True
+    fides_key: Optional[FidesKey] = None
+    is_default: bool = False
+    tags: Optional[List[str]] = None
+    organization_fides_key: Optional[FidesKey] = "default_organization"
+
+
+class DataUseCreate(TaxonomyCreateBase):
+    parent_key: Optional[FidesKey] = None
+
+
+class DataCategoryCreate(TaxonomyCreateBase):
+    parent_key: Optional[FidesKey] = None
+
+
+class DataSubjectCreate(TaxonomyCreateBase):
+    rights: Optional[DataSubjectRights] = None
+    automated_decisions_or_profiling: Optional[bool] = None

tests/ctl/core/test_api.py

@@ -3022,133 +3022,6 @@ def test_system_manager_gets_403_if_system_not_found(
         assert result.status_code == HTTP_403_FORBIDDEN
 
 
-@pytest.mark.integration
-class TestDefaultTaxonomyCrudOverrides:
-    @pytest.mark.parametrize("endpoint", TAXONOMY_ENDPOINTS)
-    def test_api_cannot_create_if_generated_fides_key_conflicts_with_existing(
-        self,
-        test_config: FidesConfig,
-        generate_auth_header,
-        endpoint: str,
-    ) -> None:
-        """Ensure we cannot create taxonomy elements if fides key conflicts with existing key"""
-        # get a default taxonomy element as a sample resource
-        resource = getattr(DEFAULT_TAXONOMY, endpoint)[0]
-        resource = TAXONOMY_EXTENSIONS[endpoint](
-            **resource.model_dump(mode="json")
-        )  # cast resource to extended model
-        # This name will conflict with existing resource
-        resource.name = resource.fides_key
-        resource.fides_key = None
-        json_resource = resource.json(exclude_none=True)
-        token_scopes: List[str] = [f"{CLI_SCOPE_PREFIX_MAPPING[endpoint]}:{CREATE}"]
-        auth_header = generate_auth_header(scopes=token_scopes)
-        result = _api.create(
-            url=test_config.cli.server_url,
-            headers=auth_header,
-            resource_type=endpoint,
-            json_resource=json_resource,
-        )
-        assert result.status_code == HTTP_409_CONFLICT
-
-    @pytest.mark.parametrize("endpoint", TAXONOMY_ENDPOINTS)
-    def test_api_can_create_without_explicit_fides_key(
-        self,
-        test_config: FidesConfig,
-        generate_auth_header,
-        endpoint: str,
-    ) -> None:
-        """Ensure we can create taxonomy elements without specifying a fides_key"""
-        # get a default taxonomy element as a sample resource
-        resource = getattr(DEFAULT_TAXONOMY, endpoint)[0]
-        resource = TAXONOMY_EXTENSIONS[endpoint](
-            **resource.model_dump(mode="json")
-        )  # cast resource to extended model
-        # Build unique name based on sample name
-        resource.name = resource.name + "my new resource"
-        resource.fides_key = None
-        json_resource = resource.json(exclude_none=True)
-        token_scopes: List[str] = [f"{CLI_SCOPE_PREFIX_MAPPING[endpoint]}:{CREATE}"]
-        auth_header = generate_auth_header(scopes=token_scopes)
-        result = _api.create(
-            url=test_config.cli.server_url,
-            headers=auth_header,
-            resource_type=endpoint,
-            json_resource=json_resource,
-        )
-        assert result.status_code == 201
-        assert result.json()["active"] is True
-        new_key = result.json()["fides_key"]
-        assert "my_new_resource" in new_key
-
-        result = _api.get(
-            url=test_config.cli.server_url,
-            headers=test_config.user.auth_header,
-            resource_type=endpoint,
-            resource_id=new_key,
-        )
-        assert result.json()["active"] is True
-
-    @pytest.mark.parametrize("endpoint", TAXONOMY_ENDPOINTS)
-    def test_api_can_update_active_when_creating_with_same_name(
-        self,
-        test_config: FidesConfig,
-        endpoint: str,
-    ) -> None:
-        """
-        If we attempt to create a new resource with the same name as an existing inactive resource,
-        but with no explicit fides_key, we should update the existing resource to be active
-        """
-        resource = getattr(DEFAULT_TAXONOMY, endpoint)[0]
-        resource = TAXONOMY_EXTENSIONS[endpoint](
-            **resource.model_dump(mode="json")
-        )  # cast resource to extended model
-        resource.active = False
-        json_resource = resource.json(exclude_none=True)
-        # First, update the existing resource as inactive so we can use it to test
-        result = _api.update(
-            url=test_config.cli.server_url,
-            headers=test_config.user.auth_header,
-            resource_type=endpoint,
-            json_resource=json_resource,
-        )
-        assert result.status_code == 200
-        assert result.json()["active"] is False
-
-        # Confirm it was updated to inactive
-        result = _api.get(
-            url=test_config.cli.server_url,
-            headers=test_config.user.auth_header,
-            resource_type=endpoint,
-            resource_id=resource.fides_key,
-        )
-        assert result.json()["active"] is False
-
-        # Now attempt to create another resource with a name that will generate the same fides_key
-        # as the inactive resource
-
-        resource.name = resource.name  # explicitly using the same name
-        resource.fides_key = None
-        json_resource = resource.json(exclude_none=True)
-        result = _api.create(
-            url=test_config.cli.server_url,
-            headers=test_config.user.auth_header,
-            resource_type=endpoint,
-            json_resource=json_resource,
-        )
-        assert result.status_code == 200
-        assert result.json()["active"] is True
-
-        # Confirm the existing resource was updated to active
-        result = _api.get(
-            url=test_config.cli.server_url,
-            headers=test_config.user.auth_header,
-            resource_type=endpoint,
-            resource_id=resource.fides_key,
-        )
-        assert result.json()["active"] is True
-
-
 @pytest.mark.integration
 class TestDefaultTaxonomyCrud:
     @pytest.mark.parametrize("endpoint", TAXONOMY_ENDPOINTS)