Skip to content

Add explicit QUEUED status #80 #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
patrick-austin merged 1 commit into from
Apr 2, 2025
Merged

Add explicit QUEUED status #80 #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/main/java/org/icatproject/topcat/StatusCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,7 @@ private String getQueueSessionId(Map<String, String> sessionIds, String facility
}

/**
* Prepares Downloads which are queued (PAUSED with no preparedId) up to the
* maxActiveDownloads limit.
* Prepares Downloads which are QUEUED up to the maxActiveDownloads limit.
* Downloads will be prepared in order of priority, with all Downloads from
* Users with a value of 1 being prepared first, then 2 and so on.
*
Expand All @@ -325,7 +324,7 @@ public void startQueuedDownloads(int maxActiveDownloads) throws Exception {

String selectString = "select download from Download download where download.isDeleted != true";
String restoringCondition = "download.status = org.icatproject.topcat.domain.DownloadStatus.RESTORING";
String pausedCondition = "download.status = org.icatproject.topcat.domain.DownloadStatus.PAUSED";
String queuedCondition = "download.status = org.icatproject.topcat.domain.DownloadStatus.QUEUED";

int availableDownloads = maxActiveDownloads;
if (maxActiveDownloads > 0) {
Expand All @@ -342,7 +341,7 @@ public void startQueuedDownloads(int maxActiveDownloads) throws Exception {
availableDownloads -= activeDownloadsSize;
}

String queuedQueryString = selectString + " and " + pausedCondition + " and download.preparedId = null";
String queuedQueryString = selectString + " and " + queuedCondition;
queuedQueryString += " order by download.createdAt";
TypedQuery<Download> queuedDownloadsQuery = em.createQuery(queuedQueryString, Download.class);
List<Download> queuedDownloads = queuedDownloadsQuery.getResultList();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/icatproject/topcat/domain/DownloadStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@

@XmlRootElement
public enum DownloadStatus {
RESTORING, COMPLETE, EXPIRED, PAUSED, PREPARING
RESTORING, COMPLETE, EXPIRED, PAUSED, PREPARING, QUEUED
}
2 changes: 1 addition & 1 deletion src/main/java/org/icatproject/topcat/web/rest/AdminResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public Response setDownloadStatus(
throw new NotFoundException("could not find download");
}

if (download.getPreparedId() == null && value.equals("RESTORING")) {
if (download.getStatus().equals(DownloadStatus.QUEUED) && value.equals("RESTORING")) {
// Queued jobs need to be marked PREPARING first to generate a preparedId before RESTORING
download.setStatus(DownloadStatus.PREPARING);
} else {
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/icatproject/topcat/web/rest/UserResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ public Response setDownloadStatus(
if (!download.getUserName().equals(cartUserName)) {
throw new ForbiddenException("you do not have permission to delete this download");
}
if (download.getPreparedId() == null) {
throw new ForbiddenException("Cannot modify status of a download before it's prepared");
if (download.getStatus() == DownloadStatus.QUEUED) {
throw new ForbiddenException("Cannot modify status of a QUEUED Download");
}

download.setStatus(DownloadStatus.valueOf(value));
Expand Down Expand Up @@ -942,7 +942,7 @@ public Response queueVisitId(@FormParam("facilityName") String facilityName,
for (Download download : downloads) {
String partFilename = formatQueuedFilename(fileName, part, downloads.size());
download.setFileName(partFilename);
downloadId = submitDownload(idsClient, download, DownloadStatus.PAUSED);
downloadId = submitDownload(idsClient, download, DownloadStatus.QUEUED);
jsonArrayBuilder.add(downloadId);
part += 1;
}
Expand Down Expand Up @@ -1029,7 +1029,7 @@ public Response queueFiles(@FormParam("facilityName") String facilityName,
download.setDownloadItems(downloadItems);
download.setSize(response.totalSize);

long downloadId = submitDownload(idsClient, download, DownloadStatus.PAUSED);
long downloadId = submitDownload(idsClient, download, DownloadStatus.QUEUED);

JsonObjectBuilder jsonObjectBuilder = Json.createObjectBuilder();
JsonArrayBuilder jsonArrayBuilder = Json.createArrayBuilder();
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/icatproject/topcat/AdminResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public void testSetDownloadStatus() throws Exception {
String facilityName = "LILS";
testDownload.setFacilityName(facilityName);
testDownload.setSessionId(adminSessionId);
testDownload.setStatus(DownloadStatus.PAUSED);
testDownload.setStatus(DownloadStatus.QUEUED);
testDownload.setIsDeleted(false);
testDownload.setUserName("simple/root");
testDownload.setFileName("testFile.txt");
Expand Down
22 changes: 11 additions & 11 deletions src/test/java/org/icatproject/topcat/StatusCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,9 +754,9 @@ public void testStartQueuedDownloadsNegative() throws Exception {
try {
String transport = "http";
Download dummyDownload1 = TestHelpers.createDummyDownload("DummyUserName", null, transport, true,
DownloadStatus.PAUSED, false, downloadRepository);
DownloadStatus.QUEUED, false, downloadRepository);
Download dummyDownload2 = TestHelpers.createDummyDownload("DummyUserName", null, transport, true,
DownloadStatus.PAUSED, false, downloadRepository);
DownloadStatus.QUEUED, false, downloadRepository);
downloadId1 = dummyDownload1.getId();
downloadId2 = dummyDownload2.getId();

Expand All @@ -783,16 +783,16 @@ public void testStartQueuedDownloadsZero() throws Exception {
try {
String transport = "http";
Download dummyDownload = TestHelpers.createDummyDownload("DummyUserName", null, transport, true,
DownloadStatus.PAUSED, false, downloadRepository);
DownloadStatus.QUEUED, false, downloadRepository);
downloadId = dummyDownload.getId();

statusCheck.startQueuedDownloads(0);

// Download status should still be PAUSED, as we unqueued a max of 0 downloads
// Download status should still be QUEUED, as we unqueued a max of 0 downloads

Download postDownload = TestHelpers.getDummyDownload(downloadId, downloadRepository);

assertEquals(DownloadStatus.PAUSED, postDownload.getStatus());
assertEquals(DownloadStatus.QUEUED, postDownload.getStatus());
assertNull(postDownload.getPreparedId());
} finally {
// clean up
Expand All @@ -808,9 +808,9 @@ public void testStartQueuedDownloadsNonZero() throws Exception {
try {
String transport = "http";
Download dummyDownload1 = TestHelpers.createDummyDownload("DummyUserName", null, transport, true,
DownloadStatus.PAUSED, false, downloadRepository);
DownloadStatus.QUEUED, false, downloadRepository);
Download dummyDownload2 = TestHelpers.createDummyDownload("DummyUserName", null, transport, true,
DownloadStatus.PAUSED, false, downloadRepository);
DownloadStatus.QUEUED, false, downloadRepository);
downloadId1 = dummyDownload1.getId();
downloadId2 = dummyDownload2.getId();

Expand All @@ -821,7 +821,7 @@ public void testStartQueuedDownloadsNonZero() throws Exception {

assertEquals(DownloadStatus.PREPARING, postDownload1.getStatus());
assertNull(postDownload1.getPreparedId());
assertEquals(DownloadStatus.PAUSED, postDownload2.getStatus());
assertEquals(DownloadStatus.QUEUED, postDownload2.getStatus());
assertNull(postDownload2.getPreparedId());
} finally {
// clean up
Expand All @@ -840,7 +840,7 @@ public void testStartQueuedDownloadsNonZeroRestoringDownload() throws Exception
Download dummyDownload1 = TestHelpers.createDummyDownload("DummyUserName", "preparedId", transport, true,
DownloadStatus.RESTORING, false, downloadRepository);
Download dummyDownload2 = TestHelpers.createDummyDownload("DummyUserName", null, transport, true,
DownloadStatus.PAUSED, false, downloadRepository);
DownloadStatus.QUEUED, false, downloadRepository);
downloadId1 = dummyDownload1.getId();
downloadId2 = dummyDownload2.getId();

Expand All @@ -854,8 +854,8 @@ public void testStartQueuedDownloadsNonZeroRestoringDownload() throws Exception

assertEquals(DownloadStatus.RESTORING, postDownload1.getStatus());
assertNotNull("Expected RESTORING Download to still have preparedId set", postDownload1.getPreparedId());
assertEquals(DownloadStatus.PAUSED, postDownload2.getStatus());
assertNull("Expected PAUSED Download to not have preparedId set", postDownload2.getPreparedId());
assertEquals(DownloadStatus.QUEUED, postDownload2.getStatus());
assertNull("Expected QUEUED Download to not have preparedId set", postDownload2.getPreparedId());
} finally {
// clean up
TestHelpers.deleteDummyDownload(downloadId1, downloadRepository);
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/icatproject/topcat/UserResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public void testQueueVisitId() throws Exception {
for (long downloadId : downloadIds) {
Download download = downloadRepository.getDownload(downloadId);
assertNull(download.getPreparedId());
assertEquals(DownloadStatus.PAUSED, download.getStatus());
assertEquals(DownloadStatus.QUEUED, download.getStatus());
assertEquals(0, download.getInvestigationIds().size());
assertEquals(1, download.getDatasetIds().size());
assertEquals(0, download.getDatafileIds().size());
Expand Down Expand Up @@ -371,7 +371,7 @@ public void testQueueFiles() throws Exception {
assertEquals(file, missingArray.getString(0));
Download download = downloadRepository.getDownload(downloadId);
assertNull(download.getPreparedId());
assertEquals(DownloadStatus.PAUSED, download.getStatus());
assertEquals(DownloadStatus.QUEUED, download.getStatus());
assertEquals(0, download.getInvestigationIds().size());
assertEquals(0, download.getDatasetIds().size());
assertEquals(2, download.getDatafileIds().size());
Expand Down Expand Up @@ -444,7 +444,7 @@ public void testSetDownloadStatus() throws Exception {
String facilityName = "LILS";
testDownload.setFacilityName(facilityName);
testDownload.setSessionId(sessionId);
testDownload.setStatus(DownloadStatus.PAUSED);
testDownload.setStatus(DownloadStatus.QUEUED);
testDownload.setIsDeleted(false);
testDownload.setUserName("simple/root");
testDownload.setFileName("testFile.txt");
Expand All @@ -455,14 +455,14 @@ public void testSetDownloadStatus() throws Exception {
ForbiddenException exception = assertThrows(ForbiddenException.class, () -> {
userResource.setDownloadStatus(testDownload.getId(), facilityName, sessionId, DownloadStatus.RESTORING.toString());
});
assertEquals("(403) : Cannot modify status of a download before it's prepared", exception.getMessage());
assertEquals("(403) : Cannot modify status of a QUEUED Download", exception.getMessage());

Response response = userResource.getDownloads(facilityName, sessionId, null);
assertEquals(200, response.getStatus());
List<Download> downloads = (List<Download>) response.getEntity();

Download unmodifiedDownload = findDownload(downloads, downloadId);
assertEquals(DownloadStatus.PAUSED, unmodifiedDownload.getStatus());
assertEquals(DownloadStatus.QUEUED, unmodifiedDownload.getStatus());
} finally {
if (downloadId != null) {
downloadRepository.removeDownload(downloadId);
Expand Down
9 changes: 5 additions & 4 deletions tools/datagateway_admin
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,10 @@ def queue_files():


def get_all_queued_downloads():
query_offset = "WHERE download.isDeleted != true"
query_offset += " AND download.status = org.icatproject.topcat.domain.DownloadStatus.PAUSED"
query_offset += " AND download.preparedId = null"
query_offset = (
"WHERE download.isDeleted != true"
" AND download.status = org.icatproject.topcat.domain.DownloadStatus.QUEUED"
)
params = {
"facilityName": facility_name,
"sessionId": session_id,
Expand Down Expand Up @@ -366,7 +367,7 @@ def requeue_download():
data = {
"facilityName": facility_name,
"sessionId": session_id,
"value": "PAUSED"
"value": "QUEUED"
}
url = topcat_url + "/admin/download/" + download_id + "/status"
requests.put(url=url, data=data, verify=verifySsl)
Expand Down
Loading