Task/WG-177 handle duplicate users in ds projects (#148)

* Ensure we don't have duplicate project users

* Improve log statement

* Only throw ObservableProjectAlreadyExists if really that error

geoapi/custom/designsafe/project_users.py

@@ -20,11 +20,15 @@ def get_system_users(tenant_id, jwt, system_id: str):
     resp = client.get(quote(f'/projects/v2/{uuid}/'))
     resp.raise_for_status()
     project = resp.json()["value"]
-    users = []
+    users = {}
     if "pi" in project:
-        users.append(SystemUser(username=project["pi"], admin=True))
+        users[project["pi"]] = SystemUser(username=project["pi"], admin=True)
     for u in project["coPis"]:
-        users.append(SystemUser(username=u, admin=True))
+        # check if we have already added this user before adding it
+        if u not in users:
+            users[u] = SystemUser(username=u, admin=True)
     for u in project["teamMembers"]:
-        users.append(SystemUser(username=u, admin=False))
-    return users
+        # check if we have already added this user before adding it
+        if u not in users:
+            users[u] = SystemUser(username=u, admin=False)
+    return list(users.values())

geoapi/models/observable_data.py

@@ -10,7 +10,7 @@
 class ObservableDataProject(Base):
     __tablename__ = 'observable_data_projects'
     __table_args__ = (
-        UniqueConstraint('system_id', 'path'),
+        UniqueConstraint('system_id', 'path', name="unique_system_id_path"),
     )
     id = Column(Integer, primary_key=True)
     project_id = Column(ForeignKey('projects.id', ondelete="CASCADE", onupdate="CASCADE"), index=True)

geoapi/services/projects.py

@@ -77,7 +77,7 @@ def makeObservable(database_session,
         )
 
         users = get_system_users(proj.tenant_id, user.jwt, proj.system_id)
-        logger.info("Updating project:{} to have the following users: {}".format(name, users))
+        logger.info("Initial update of project:{} to have the following users: {}".format(name, users))
         project_users = [UserService.getOrCreateUser(database_session, u.username, tenant=proj.tenant_id) for u in users]
         proj.users = project_users
 
@@ -86,10 +86,13 @@ def makeObservable(database_session,
         try:
             database_session.add(obs)
             database_session.commit()
-        except IntegrityError:
+        except IntegrityError as e:
             database_session.rollback()
-            logger.exception("User:{} tried to create an observable project that already exists: '{}'".format(user.username, name))
-            raise ObservableProjectAlreadyExists("'{}' project already exists".format(name))
+            if 'unique_system_id_path' in str(e.orig):
+                logger.exception("User:{} tried to create an observable project that already exists: '{}'".format(user.username, name))
+                raise ObservableProjectAlreadyExists("'{}' project already exists".format(name))
+            else:
+                raise e
 
         if watch_content:
             import_from_agave.apply_async(args=[obs.project.tenant_id, user.id, obs.system_id, obs.path, obs.project_id])

geoapi/tests/custom/test_designsafe.py

@@ -11,6 +11,13 @@ def project_response():
         yield json.loads(f.read())
 
 
+@pytest.fixture()
+def project_response_with_duplicate_users():
+    home = os.path.dirname(__file__)
+    with open(os.path.join(home, '../fixtures/designsafe_api_project_with_duplicate_users.json'), 'rb') as f:
+        yield json.loads(f.read())
+
+
 def test_get_system_users(requests_mock, project_response):
     uuid = "5752672753351626260-242ac118-0001-014"
     requests_mock.get(f"https://agave.designsafe-ci.org/projects/v2/{uuid}/", json=project_response)
@@ -22,3 +29,16 @@ def test_get_system_users(requests_mock, project_response):
     users = get_system_users(tenant_id="designsafe", jwt="dummy", system_id=f"project-{uuid}")
     users_as_list_of_dict = [{u.username: u.admin} for u in users]
     assert users_as_list_of_dict == [{'user_pi': True}, {'user_copi': True}, {'user3': False}, {'user4': False}]
+
+
+def test_get_system_users_duplicate(requests_mock, project_response_with_duplicate_users):
+    uuid = "5752672753351626260-242ac118-0001-014"
+    requests_mock.get(f"https://agave.designsafe-ci.org/projects/v2/{uuid}/", json=project_response_with_duplicate_users)
+
+    users = get_system_users(tenant_id="DESIGNSAFE", jwt="dummy", system_id=f"project-{uuid}")
+    users_as_list_of_dict = [{u.username: u.admin} for u in users]
+    assert users_as_list_of_dict == [{'user_pi': True},
+                                     {'user_copi_1': True},
+                                     {'user_copi_2': True},
+                                     {'user4': False},
+                                     {'user5': False}]

geoapi/tests/fixtures/designsafe_api_project_with_duplicate_users.json

@@ -0,0 +1,75 @@
+{
+    "uuid": "5752672753351626260-242ac118-0001-014",
+    "schemaId": null,
+    "internalUsername": null,
+    "associationIds": [],
+    "lastUpdated": "2022-07-15T14:12:13.776000-05:00",
+    "created": "2022-04-22T15:06:02.127000-05:00",
+    "owner": "ds_admin",
+    "name": "designsafe.project",
+    "_links": {
+        "self": {
+            "href": "https://agave.designsafe-ci.org/meta/v2/data/5752672753351626260-242ac118-0001-014"
+        },
+        "permissions": {
+            "href": "https://agave.designsafe-ci.org/meta/v2/data/5752672753351626260-242ac118-0001-014/pems"
+        },
+        "owner": {
+            "href": "https://agave.designsafe-ci.org/profiles/v2/ds_admin"
+        },
+        "associationIds": []
+    },
+    "value": {
+        "teamMembers": [
+            "user4",
+            "user5",
+            "user_pi",
+            "user_copi_1",
+            "user_copi_2"
+        ],
+        "guestMembers": [],
+        "coPis": ["user_copi_1","user_copi_2"],
+        "projectType": "None",
+        "dataType": "",
+        "teamOrder": [],
+        "projectId": "PRJ-1234",
+        "description": "",
+        "title": "Some project",
+        "pi": "user_pi",
+        "awardNumber": [
+            "[",
+            "{",
+            "u",
+            "'",
+            "n",
+            "a",
+            "m",
+            "e",
+            ":",
+            " ",
+            "]",
+            ",",
+            "b",
+            "r",
+            "}"
+        ],
+        "awardNumbers": [],
+        "associatedProjects": [
+            {
+                "href": "",
+                "title": ""
+            }
+        ],
+        "ef": "",
+        "keywords": "",
+        "fileTags": [],
+        "nhTypes": [],
+        "dois": [],
+        "hazmapperMaps": []
+    },
+    "_ui": {
+        "uuid": "5752672753351626260-242ac118-0001-014",
+        "orders": []
+    },
+    "_related": {}
+}