Skip to content

Commit 80bbb3d

Browse files
pkalita-lblpkalita-lbl
committed
Add endpoint for acquiring a submission lock; do not automatically lock when getting submission
1 parent 5047827 commit 80bbb3d

File tree

3 files changed

+177
-10
lines changed

3 files changed

+177
-10
lines changed

‎nmdc_server/api.py

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -649,10 +649,10 @@ async def get_submission(
649649
db: Session = Depends(get_db),
650650
user: models.User = Depends(get_current_user),
651651
):
652-
submission = db.query(SubmissionMetadata).get(id)
652+
submission: Optional[models.SubmissionMetadata] = db.query(SubmissionMetadata).get(id)
653653
if submission is None:
654654
raise HTTPException(status_code=404, detail="Submission not found")
655-
_ = crud.try_get_submission_lock(db, submission.id, user.id)
655+
656656
if user.is_admin or crud.can_read_submission(db, id, user.orcid):
657657
permission_level = None
658658
if user.is_admin or user.orcid in submission.owners:
@@ -663,16 +663,19 @@ async def get_submission(
663663
permission_level = models.SubmissionEditorRole.metadata_contributor.value
664664
elif user.orcid in submission.viewers:
665665
permission_level = models.SubmissionEditorRole.viewer.value
666-
return schemas_submission.SubmissionMetadataSchema(
666+
submission_metadata_schema = schemas_submission.SubmissionMetadataSchema(
667667
status=submission.status,
668668
id=submission.id,
669669
metadata_submission=submission.metadata_submission,
670670
author_orcid=submission.author_orcid,
671671
created=submission.created,
672672
author=schemas.User(**submission.author.__dict__),
673-
locked_by=schemas.User(**submission.locked_by.__dict__),
674673
permission_level=permission_level,
675674
)
675+
if submission.locked_by is not None:
676+
submission_metadata_schema.locked_by = schemas.User(**submission.locked_by.__dict__)
677+
return submission_metadata_schema
678+
676679
raise HTTPException(status_code=403, detail="Must have access.")
677680

678681

@@ -862,22 +865,63 @@ async def delete_submission(
862865
return Response(status_code=status.HTTP_204_NO_CONTENT)
863866

864867

868+
@router.put("/metadata_submission/{id}/lock")
869+
async def lock_submission(
870+
response: Response,
871+
id: str,
872+
db: Session = Depends(get_db),
873+
user: models.User = Depends(get_current_user),
874+
) -> schemas.LockOperationResult:
875+
submission: Optional[SubmissionMetadata] = db.query(SubmissionMetadata).get(id)
876+
if not submission:
877+
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Submission not found")
878+
879+
# Attempt to acquire the lock
880+
lock_acquired = crud.try_get_submission_lock(db, submission.id, user.id)
881+
if lock_acquired:
882+
return schemas.LockOperationResult(
883+
success=True,
884+
message=f"Lock successfully acquired for submission with ID {id}.",
885+
locked_by=submission.locked_by,
886+
lock_updated=submission.lock_updated,
887+
)
888+
else:
889+
response.status_code = status.HTTP_409_CONFLICT
890+
return schemas.LockOperationResult(
891+
success=False,
892+
message="This submission is currently locked by a different user.",
893+
locked_by=submission.locked_by,
894+
lock_updated=submission.lock_updated,
895+
)
896+
897+
865898
@router.put("/metadata_submission/{id}/unlock")
866899
async def unlock_submission(
867-
id: str, db: Session = Depends(get_db), user: models.User = Depends(get_current_user)
868-
) -> str:
900+
response: Response,
901+
id: str,
902+
db: Session = Depends(get_db),
903+
user: models.User = Depends(get_current_user),
904+
) -> schemas.LockOperationResult:
869905
submission = db.query(SubmissionMetadata).get(id)
870906
if not submission:
871-
raise HTTPException(status_code=404, detail="Submission not found")
907+
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Submission not found")
908+
872909
# Then verify session user has the lock
873910
has_lock = crud.try_get_submission_lock(db, submission.id, user.id)
874911
if not has_lock:
875-
raise HTTPException(
876-
status_code=400, detail="This submission is currently being edited by a different user."
912+
response.status_code = status.HTTP_409_CONFLICT
913+
return schemas.LockOperationResult(
914+
success=False,
915+
message="This submission is currently locked by a different user.",
916+
locked_by=submission.locked_by,
917+
lock_updated=submission.lock_updated,
877918
)
878919
else:
879920
crud.release_submission_lock(db, submission.id)
880-
return f"Submission lock released successfully for submission with ID {id}"
921+
return schemas.LockOperationResult(
922+
success=True,
923+
message=f"Submission lock released successfully for submission with ID {id}",
924+
)
881925

882926

883927
@router.post(

‎nmdc_server/schemas.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,3 +658,10 @@ class User(BaseModel):
658658

659659
class Config:
660660
orm_mode = True
661+
662+
663+
class LockOperationResult(BaseModel):
664+
success: bool
665+
message: str
666+
locked_by: Optional[User]
667+
lock_updated: Optional[datetime]

‎tests/test_submission.py

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,122 @@ def test_list_submissions(db: Session, client: TestClient, logged_in_user):
2727
assert response.json()["results"][0]["id"] == str(submission.id)
2828

2929

30+
def test_obtain_submission_lock(db: Session, client: TestClient, logged_in_user):
31+
submission = fakes.MetadataSubmissionFactory(
32+
author=logged_in_user, author_orcid=logged_in_user.orcid
33+
)
34+
fakes.SubmissionRoleFactory(
35+
submission=submission,
36+
submission_id=submission.id,
37+
user_orcid=logged_in_user.orcid,
38+
role=SubmissionEditorRole.owner,
39+
)
40+
db.commit()
41+
42+
# Should be able to successfully GET this submission and the lock should not be set
43+
response = client.request(method="get", url=f"/api/metadata_submission/{submission.id}")
44+
assert response.status_code == 200
45+
body = response.json()
46+
assert body.get("locked_by") is None
47+
48+
# Attempt to acquire the lock
49+
response = client.request(method="put", url=f"/api/metadata_submission/{submission.id}/lock")
50+
assert response.status_code == 200
51+
body = response.json()
52+
assert body["success"] is True
53+
assert body["locked_by"]["id"] == str(logged_in_user.id)
54+
55+
# Verify that the lock is set
56+
response = client.request(method="get", url=f"/api/metadata_submission/{submission.id}")
57+
assert response.status_code == 200
58+
body = response.json()
59+
assert body["locked_by"]["id"] == str(logged_in_user.id)
60+
61+
62+
def test_cannot_acquire_lock_on_locked_submission(db: Session, client: TestClient, logged_in_user):
63+
locking_user = fakes.UserFactory()
64+
submission = fakes.MetadataSubmissionFactory(
65+
author=logged_in_user,
66+
author_orcid=logged_in_user.orcid,
67+
locked_by=locking_user,
68+
lock_updated=datetime.utcnow(),
69+
)
70+
fakes.SubmissionRoleFactory(
71+
submission=submission,
72+
submission_id=submission.id,
73+
user_orcid=logged_in_user.orcid,
74+
role=SubmissionEditorRole.owner,
75+
)
76+
db.commit()
77+
78+
# Attempt to acquire the lock, verify that it fails and reports the current lock holder
79+
response = client.request(method="put", url=f"/api/metadata_submission/{submission.id}/lock")
80+
assert response.status_code == 409
81+
body = response.json()
82+
assert body["success"] is False
83+
assert body["locked_by"]["id"] == str(locking_user.id)
84+
85+
86+
def test_release_submission_lock(db: Session, client: TestClient, logged_in_user):
87+
submission = fakes.MetadataSubmissionFactory(
88+
author=logged_in_user,
89+
author_orcid=logged_in_user.orcid,
90+
locked_by=logged_in_user,
91+
lock_updated=datetime.utcnow(),
92+
)
93+
fakes.SubmissionRoleFactory(
94+
submission=submission,
95+
submission_id=submission.id,
96+
user_orcid=logged_in_user.orcid,
97+
role=SubmissionEditorRole.owner,
98+
)
99+
db.commit()
100+
101+
# Verify that the lock is set
102+
response = client.request(method="get", url=f"/api/metadata_submission/{submission.id}")
103+
assert response.status_code == 200
104+
body = response.json()
105+
assert body["locked_by"]["id"] == str(logged_in_user.id)
106+
107+
# Release the lock
108+
response = client.request(method="put", url=f"/api/metadata_submission/{submission.id}/unlock")
109+
assert response.status_code == 200
110+
body = response.json()
111+
assert body["success"] is True
112+
113+
# Verify that the lock is released
114+
response = client.request(method="get", url=f"/api/metadata_submission/{submission.id}")
115+
assert response.status_code == 200
116+
body = response.json()
117+
assert body["locked_by"] is None
118+
119+
120+
def test_cannot_release_other_users_submission_lock(
121+
db: Session, client: TestClient, logged_in_user
122+
):
123+
locking_user = fakes.UserFactory()
124+
submission = fakes.MetadataSubmissionFactory(
125+
author=logged_in_user,
126+
author_orcid=logged_in_user.orcid,
127+
locked_by=locking_user,
128+
lock_updated=datetime.utcnow(),
129+
)
130+
fakes.SubmissionRoleFactory(
131+
submission=submission,
132+
submission_id=submission.id,
133+
user_orcid=logged_in_user.orcid,
134+
role=SubmissionEditorRole.owner,
135+
)
136+
db.commit()
137+
138+
# Attempt to release the lock, verify that it fails
139+
response = client.request(method="put", url=f"/api/metadata_submission/{submission.id}/unlock")
140+
assert response.status_code == 409
141+
body = response.json()
142+
assert body["success"] is False
143+
assert body["locked_by"]["id"] == str(locking_user.id)
144+
145+
30146
def test_try_edit_locked_submission(db: Session, client: TestClient, logged_in_user):
31147
# Locked by a random user at utcnow by default
32148
submission = fakes.MetadataSubmissionFactory(

0 commit comments

Comments
 (0)