Skip to content

Commit 36960e6

Browse files
JustKuzyakeithmanvillejkglasbrenner
committed
refactor(restapi): add EntityType enum to encapsulate entity constants
This commit introduces an EntityType enum that encapsulates constants associated with Dioptra Entities. Entities are top level Dioptra concepts such as Resources and ResourceSnapshots (as well as specific Resource types such as Queue), and Users, Groups, and Tags. The EntityType enum replaces the scattered throughout the controller and service layers. As part of this refactor new errors are introduced, as existing errors such as EntityDoesNotExistError were being over used for objects that are not Entities. A new helper function, get_resource_type, is added to the repository utilities to assist with retrieving the EntityType from a db_table_name string. Normalize raw and mixed resource lookup failures to use resource and add a draft-specific base resource not found error. Add a dedicated mlflow run not found handler and cover the artifact registration regression with a focused test. Co-authored-by: Keith Manville <kmanville@mitre.org> Co-authored-by: James K. Glasbrenner <jglasbrenner@mitre.org>
1 parent fddc3e3 commit 36960e6

39 files changed

Lines changed: 782 additions & 431 deletions

src/dioptra/restapi/db/repository/drafts.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,19 @@
5151
get_resource_snapshot_id,
5252
get_user_id,
5353
)
54+
from dioptra.restapi.db.repository.utils.common import get_resource_type
5455
from dioptra.restapi.errors import (
5556
DraftAlreadyExistsError,
5657
DraftBaseInvalidError,
58+
DraftBaseResourceDoesNotExistError,
5759
DraftDoesNotExistError,
5860
DraftModificationRequiredError,
5961
DraftSnapshotIdInvalidError,
6062
DraftTargetOwnerMismatch,
6163
EntityDeletedError,
6264
EntityDoesNotExistError,
6365
)
66+
from dioptra.restapi.v1.entity_types import EntityType
6467

6568

6669
class DraftType(enum.Enum):
@@ -86,13 +89,14 @@ def create_draft_resource(
8689
Raises:
8790
DraftAlreadyExistsError: if the draft already exists
8891
EntityDoesNotExistError: if the draft creator or target owner does
89-
not exist, or if the payload's base_resource_id refers to a
90-
resource which does not exist
92+
not exist
9193
EntityDeletedError: if the draft creator or target owner is deleted,
9294
or if the payload's base_resource_id refers to a deleted
9395
resource
9496
UserNotInGroupError: if the draft creator user is not a member of
9597
the draft target owner group
98+
DraftBaseResourceDoesNotExistError: if the payload's
99+
base_resource_id refers to a resource which does not exist
96100
DraftBaseInvalidError: if the payload's base_resource_id refers
97101
to a resource of a type which is not legal to be a parent
98102
of the draft's resource type
@@ -134,20 +138,21 @@ def create_draft_resource(
134138

135139
if result:
136140
is_deleted, parent_resource_type, child_resource_type = result
141+
parent_entity_type = EntityType.get_from_db_table_name(
142+
parent_resource_type
143+
)
144+
137145
if is_deleted:
138-
raise EntityDeletedError(parent_resource_type, base_resource_id)
146+
raise EntityDeletedError(parent_entity_type, base_resource_id)
139147

140148
if not child_resource_type:
141149
raise DraftBaseInvalidError(
142150
base_resource_id,
143-
parent_resource_type,
144-
draft.resource_type,
151+
parent_entity_type,
152+
get_resource_type(draft),
145153
)
146154
else:
147-
raise EntityDoesNotExistError(
148-
None,
149-
resource_id=base_resource_id,
150-
)
155+
raise DraftBaseResourceDoesNotExistError(base_resource_id)
151156

152157
# TODO: verify that the draft payload is for a draft resource, not a
153158
# draft modification? Any other sanity checks necessary?
@@ -197,9 +202,11 @@ def create_draft_modification(
197202

198203
resource_obj = self.get_resource(resource_id, DeletionPolicy.ANY)
199204
if not resource_obj:
200-
raise EntityDoesNotExistError(None, resource_id=resource_id)
205+
raise EntityDoesNotExistError(EntityType.RESOURCE, resource_id=resource_id)
201206
elif resource_obj.is_deleted:
202-
raise EntityDeletedError(None, resource_id, resource_id=resource_id)
207+
raise EntityDeletedError(
208+
EntityType.RESOURCE, resource_id, resource_id=resource_id
209+
)
203210

204211
assert_snapshot_exists(self._session, resource_snapshot_id)
205212

src/dioptra/restapi/db/repository/groups.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
UserIsManagerError,
5151
UserNeedsAGroupError,
5252
)
53+
from dioptra.restapi.v1.entity_types import EntityType
5354

5455
GROUP_CREATOR_MANAGER_PERMISSIONS: Final[dict[str, bool]] = {
5556
"owner": True,
@@ -105,7 +106,9 @@ def create(self, group) -> None:
105106
if group.name is not None and dupe_group:
106107
# Assume this name uniqueness constraint applies with respect to
107108
# all groups, not just non-deleted groups.
108-
raise EntityExistsError("group", dupe_group.group_id, name=group.name)
109+
raise EntityExistsError(
110+
EntityType.GROUP, dupe_group.group_id, name=group.name
111+
)
109112

110113
# Because we may be creating a new user, we need to check things
111114
# similarly to UserRepository.create(). We can't just invoke that
@@ -118,7 +121,7 @@ def create(self, group) -> None:
118121
elif user_exists_result is ExistenceResult.DELETED:
119122
# Should probably enforce this until instructed otherwise
120123
raise EntityDeletedError(
121-
"user",
124+
EntityType.USER,
122125
group.creator.user_id,
123126
user_id=group.creator.user_id,
124127
)
@@ -150,7 +153,7 @@ def delete(self, group: Group) -> None:
150153

151154
exists_result = group_exists(self.session, group)
152155
if exists_result is ExistenceResult.DOES_NOT_EXIST:
153-
raise EntityDoesNotExistError("group", group_id=group.group_id)
156+
raise EntityDoesNotExistError(EntityType.GROUP, group_id=group.group_id)
154157

155158
if exists_result is ExistenceResult.EXISTS:
156159
lock = GroupLock(GroupLockTypes.DELETE, group)
@@ -214,7 +217,7 @@ def get_one(
214217
else:
215218
existence_result = ExistenceResult.EXISTS
216219

217-
assert_exists(deletion_policy, existence_result, "group", group_id)
220+
assert_exists(deletion_policy, existence_result, EntityType.GROUP, group_id)
218221

219222
# The above assert_exists() function would have raised an exception,
220223
# so group can't be none here.
@@ -601,3 +604,5 @@ def _apply_deletion_policy(
601604
stmt = stmt.where(Group.is_deleted == True) # noqa: E712
602605

603606
return stmt
607+
608+
return stmt

src/dioptra/restapi/db/repository/users.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
user_exists,
4343
)
4444
from dioptra.restapi.errors import EntityDoesNotExistError
45+
from dioptra.restapi.v1.entity_types import EntityType
4546

4647

4748
class UserRepository:
@@ -121,7 +122,7 @@ def delete(self, user: User) -> None:
121122

122123
exists_result = user_exists(self.session, user)
123124
if exists_result is ExistenceResult.DOES_NOT_EXIST:
124-
raise EntityDoesNotExistError("user", user_id=user.user_id)
125+
raise EntityDoesNotExistError(EntityType.USER, user_id=user.user_id)
125126

126127
elif exists_result is ExistenceResult.EXISTS:
127128
lock = UserLock(UserLockTypes.DELETE, user)
@@ -181,7 +182,7 @@ def get_one(
181182
else:
182183
existence_result = ExistenceResult.EXISTS
183184

184-
assert_exists(deletion_policy, existence_result, "user", user_id)
185+
assert_exists(deletion_policy, existence_result, EntityType.USER, user_id)
185186

186187
# The above assert_exists() function would have raised an exception,
187188
# so user can't be none here.
@@ -411,3 +412,4 @@ def _apply_deletion_policy(
411412
stmt = stmt.where(User.is_deleted == True) # noqa: E712
412413

413414
return stmt
415+
return stmt

0 commit comments

Comments
 (0)