[PLT-1205] More defensive programming on input (#1720)

Author: adrian-chang

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(
@@ -288,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}) {

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__])

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 = [
             {
@@ -115,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):
@@ -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(UnprocessableEntityError):
+            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"
@@ -249,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}