From 2ec538e880bae3c2b418087cf16343dd8efd4906 Mon Sep 17 00:00:00 2001 From: Adrian Chang Date: Tue, 9 Jul 2024 00:27:29 -0700 Subject: [PATCH 1/3] More defensive programming on input --- .../src/labelbox/schema/user_group.py | 8 +++++++ .../tests/unit/schema/test_user_group.py | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/libs/labelbox/src/labelbox/schema/user_group.py b/libs/labelbox/src/labelbox/schema/user_group.py index f515bf1ca..4cf3a88e6 100644 --- a/libs/labelbox/src/labelbox/schema/user_group.py +++ b/libs/labelbox/src/labelbox/schema/user_group.py @@ -110,7 +110,10 @@ def get(self) -> "UserGroup": Raises: ResourceNotFoundError: If the query fails to fetch the group information. + ValueError: If the group ID is not provided. """ + if not self.id: + raise ValueError("Group id is required") query = """ query GetUserGroupPyApi($id: ID!) { userGroup(where: {id: $id}) { @@ -156,7 +159,12 @@ def update(self) -> "UserGroup": Raises: ResourceNotFoundError: If the update fails due to unknown user group UnprocessableEntityError: If the update fails due to a malformed input + ValueError: If the group id or name is not provided """ + if not self.id: + raise ValueError("Group id is required") + if not self.name: + raise ValueError("Group name is required") query = """ mutation UpdateUserGroupPyApi($id: ID!, $name: String!, $color: String!, $projectIds: [String!]!, $userIds: [String!]!) { updateUserGroup( diff --git a/libs/labelbox/tests/unit/schema/test_user_group.py b/libs/labelbox/tests/unit/schema/test_user_group.py index 5b41e336b..74e019ba8 100644 --- a/libs/labelbox/tests/unit/schema/test_user_group.py +++ b/libs/labelbox/tests/unit/schema/test_user_group.py @@ -65,6 +65,13 @@ def test_constructor(self): assert len(group.projects) == 0 assert len(group.users) == 0 + def test_update_with_exception_name(self): + group = self.group + group.id = "" + + with pytest.raises(ValueError): + group.get() + def test_get(self): projects = [ { @@ -170,6 +177,20 @@ def test_update_resource_error_unknown_id(self): with pytest.raises(ResourceNotFoundError) as e: group.update() + def test_update_with_exception_name(self): + group = self.group + group.name = "" + + with pytest.raises(ValueError): + group.update() + + def test_update_with_exception_name(self): + group = self.group + group.id = "" + + with pytest.raises(ValueError): + group.update() + def test_create_with_exception_id(self): group = self.group group.id = "group_id" From 98cdadd93001660bc4827f5569dfc828cd2d969d Mon Sep 17 00:00:00 2001 From: Adrian Chang Date: Tue, 9 Jul 2024 01:32:43 -0700 Subject: [PATCH 2/3] Update tests and validation --- .../src/labelbox/schema/user_group.py | 3 ++ .../integration/schema/test_user_group.py | 34 ++++++++++++++++--- .../tests/unit/schema/test_user_group.py | 9 ++++- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/libs/labelbox/src/labelbox/schema/user_group.py b/libs/labelbox/src/labelbox/schema/user_group.py index 4cf3a88e6..e09c1768e 100644 --- a/libs/labelbox/src/labelbox/schema/user_group.py +++ b/libs/labelbox/src/labelbox/schema/user_group.py @@ -296,7 +296,10 @@ def delete(self) -> bool: Raises: ResourceNotFoundError: If the deletion of the user group fails due to not existing + ValueError: If the group ID is not provided. """ + if not self.id: + raise ValueError("Group id is required") query = """ mutation DeleteUserGroupPyApi($id: ID!) { deleteUserGroup(where: {id: $id}) { diff --git a/libs/labelbox/tests/integration/schema/test_user_group.py b/libs/labelbox/tests/integration/schema/test_user_group.py index 0a027bdf1..e8464ac37 100644 --- a/libs/labelbox/tests/integration/schema/test_user_group.py +++ b/libs/labelbox/tests/integration/schema/test_user_group.py @@ -21,7 +21,7 @@ def user_group(client): user_group.delete() -def test_existing_user_groups(user_group, client): +def test_get_user_group(user_group, client): # Verify that the user group was created successfully user_group_equal = UserGroup(client) user_group_equal.id = user_group.id @@ -31,7 +31,15 @@ def test_existing_user_groups(user_group, client): assert user_group.color == user_group_equal.color -def test_cannot_get_user_group_with_invalid_id(client): +def test_throw_error_get_user_group_no_id(user_group, client): + old_id = user_group.id + with pytest.raises(ValueError): + user_group.id = "" + user_group.get() + user_group.id = old_id + + +def test_throw_error_cannot_get_user_group_with_invalid_id(client): user_group = UserGroup(client=client, id=str(uuid4())) with pytest.raises(ResourceNotFoundError): user_group.get() @@ -108,12 +116,20 @@ def test_update_user_group(user_group): assert user_group.color == UserGroupColor.PURPLE -def test_cannot_update_name_to_empty_string(user_group): - with pytest.raises(UnprocessableEntityError): +def test_throw_error_cannot_update_name_to_empty_string(user_group): + with pytest.raises(ValueError): user_group.name = "" user_group.update() +def test_throw_error_cannot_update_id_to_empty_string(user_group): + old_id = user_group.id + with pytest.raises(ValueError): + user_group.id = "" + user_group.update() + user_group.id = old_id + + def test_cannot_update_group_id(user_group): old_id = user_group.id with pytest.raises(ResourceNotFoundError): @@ -160,7 +176,7 @@ def test_get_user_groups_with_creation_deletion(client): # project_pack creates two projects -def test_update_user_group(user_group, client, project_pack): +def test_update_user_group_users_projects(user_group, client, project_pack): users = list(client.get_users()) projects = project_pack @@ -192,6 +208,14 @@ def test_throw_error_when_deleting_invalid_id_group(client): user_group.delete() +def test_throw_error_delete_user_group_no_id(user_group, client): + old_id = user_group.id + with pytest.raises(ValueError): + user_group.id = "" + user_group.delete() + user_group.id = old_id + + if __name__ == "__main__": import subprocess subprocess.call(["pytest", "-v", __file__]) \ No newline at end of file diff --git a/libs/labelbox/tests/unit/schema/test_user_group.py b/libs/labelbox/tests/unit/schema/test_user_group.py index 74e019ba8..047189e7f 100644 --- a/libs/labelbox/tests/unit/schema/test_user_group.py +++ b/libs/labelbox/tests/unit/schema/test_user_group.py @@ -181,7 +181,7 @@ def test_update_with_exception_name(self): group = self.group group.name = "" - with pytest.raises(ValueError): + with pytest.raises(UnprocessableEntityError): group.update() def test_update_with_exception_name(self): @@ -270,6 +270,13 @@ def test_delete_resource_not_found_error(self): with pytest.raises(ResourceNotFoundError): group.delete() + def test_delete_no_id(self): + group = UserGroup(self.client) + group.id = None + + with pytest.raises(ValueError): + group.delete() + def test_user_groups_empty(self): self.client.execute.return_value = {"userGroups": None} From 2d4dced35620d3c046ee47d0e262a52f26fddb06 Mon Sep 17 00:00:00 2001 From: Adrian Chang Date: Tue, 9 Jul 2024 01:37:04 -0700 Subject: [PATCH 3/3] Correct error --- libs/labelbox/tests/unit/schema/test_user_group.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/labelbox/tests/unit/schema/test_user_group.py b/libs/labelbox/tests/unit/schema/test_user_group.py index 047189e7f..0eb0381d6 100644 --- a/libs/labelbox/tests/unit/schema/test_user_group.py +++ b/libs/labelbox/tests/unit/schema/test_user_group.py @@ -122,12 +122,12 @@ def test_get(self): assert len(group.projects) == 2 assert len(group.users) == 2 - def test_get_resource_not_found_error(self): + def test_get_value_error(self): self.client.execute.return_value = None group = UserGroup(self.client) group.name = "Test Group" - with pytest.raises(ResourceNotFoundError): + with pytest.raises(ValueError): group.get() def test_update(self, group_user, group_project):