Skip to content
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.

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

Block user from starting queued downloads with set status #42 #44

Open
wants to merge 4 commits into
base: 36_queuing
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ jobs:
with:
repository: icatproject-contrib/icat-ansible
path: icat-ansible
ref: payara6
- name: Install Ansible
run: pip install -r icat-ansible/requirements.txt

Expand Down
28 changes: 28 additions & 0 deletions src/main/java/org/icatproject/topcat/IcatClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import org.icatproject.topcat.httpclient.*;
import org.icatproject.topcat.exceptions.*;
import org.apache.commons.lang3.StringUtils;
import org.icatproject.topcat.domain.*;

import jakarta.json.*;
Expand Down Expand Up @@ -93,6 +94,33 @@ public String getFullName() throws TopcatException {
}
}

/**
* Gets a single Entity of the specified type, without any other conditions.
*
* @param entityType Type of ICAT Entity to get
* @return A single ICAT Entity of the specified type as a JsonObject
* @throws TopcatException
*/
public JsonObject getEntity(String entityType) throws TopcatException {
try {
String entityCapital = StringUtils.capitalize(entityType.toLowerCase());
String query = URLEncoder.encode("SELECT o FROM " + entityCapital + " o LIMIT 0, 1", "UTF8");
String url = "entityManager?sessionId=" + URLEncoder.encode(sessionId, "UTF8") + "&query=" + query;
Response response = httpClient.get(url, new HashMap<String, String>());
if(response.getCode() == 404){
throw new NotFoundException("Could not run getEntity got a 404 response");
} else if(response.getCode() >= 400){
throw new BadRequestException(Utils.parseJsonObject(response.toString()).getString("message"));
}
JsonObject entity = Utils.parseJsonArray(response.toString()).getJsonObject(0);
return entity.getJsonObject(entityCapital);
} catch (TopcatException e){
throw e;
} catch (Exception e) {
throw new BadRequestException(e.getMessage());
}
}

public List<JsonObject> getEntities(String entityType, List<Long> entityIds) throws TopcatException {
List<JsonObject> out = new ArrayList<JsonObject>();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ public Response setDownloadStatus(
if (!download.getUserName().equals(cartUserName)) {
throw new ForbiddenException("you do not have permission to delete this download");
}
if (download.getPreparedId() == null && download.getStatus().equals(DownloadStatus.PAUSED)) {
throw new ForbiddenException("Cannot modify status of a queued download");
}

download.setStatus(DownloadStatus.valueOf(value));
if(value.equals("COMPLETE")){
Expand Down
46 changes: 39 additions & 7 deletions src/test/java/org/icatproject/topcat/UserResourceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import org.icatproject.topcat.httpclient.HttpClient;
import org.icatproject.topcat.domain.*;
import org.icatproject.topcat.exceptions.ForbiddenException;

import java.net.URLEncoder;

import org.icatproject.topcat.repository.CacheRepository;
Expand Down Expand Up @@ -88,10 +90,9 @@ public void setup() throws Exception {
public void testGetSize() throws Exception {
String facilityName = "LILS";
String entityType = "investigation";
Long entityId = (long) 1;
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);

List<Long> emptyIds = new ArrayList<Long>();
JsonObject investigation = icatClient.getEntity(entityType);
long entityId = investigation.getInt("id");

Response response = userResource.getSize(facilityName, sessionId, entityType, entityId);

Expand All @@ -105,6 +106,9 @@ public void testGetSize() throws Exception {
@Test
public void testCart() throws Exception {
String facilityName = "LILS";
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);
JsonObject dataset = icatClient.getEntity("dataset");
long entityId = dataset.getInt("id");

Response response;

Expand All @@ -127,7 +131,7 @@ public void testCart() throws Exception {
// We assume that there is a dataset with id = 1, and that simple/root can see
// it.

response = userResource.addCartItems(facilityName, sessionId, "dataset 1", false);
response = userResource.addCartItems(facilityName, sessionId, "dataset " + entityId, false);
assertEquals(200, response.getStatus());

response = userResource.getCart(facilityName, sessionId);
Expand All @@ -138,7 +142,7 @@ public void testCart() throws Exception {
// Again, this ought to be done directly, rather than using the methods we
// should be testing independently!

response = userResource.deleteCartItems(facilityName, sessionId, "dataset 1");
response = userResource.deleteCartItems(facilityName, sessionId, "dataset " + entityId);
assertEquals(200, response.getStatus());
assertEquals(0, getCartSize(response));
}
Expand All @@ -149,6 +153,9 @@ public void testSubmitCart() throws Exception {
Response response;
JsonObject json;
List<Download> downloads;
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);
JsonObject dataset = icatClient.getEntity("dataset");
long entityId = dataset.getInt("id");

// Get the initial state of the downloads - may not be empty
// It appears queryOffset cannot be empty!
Expand All @@ -163,7 +170,7 @@ public void testSubmitCart() throws Exception {
System.out.println("DEBUG testSubmitCart: initial downloads size: " + initialDownloadsSize);

// Put something into the Cart, so we have something to submit
response = userResource.addCartItems(facilityName, sessionId, "dataset 1", false);
response = userResource.addCartItems(facilityName, sessionId, "dataset " + entityId, false);
assertEquals(200, response.getStatus());

// Now submit it
Expand Down Expand Up @@ -250,6 +257,31 @@ public void testSubmitCart() throws Exception {
assertTrue(newDownload.getIsDeleted());
}

@Test
public void testSetDownloadStatus() throws Exception {
Download testDownload = new Download();
String facilityName = "LILS";
testDownload.setFacilityName(facilityName);
testDownload.setSessionId(sessionId);
testDownload.setStatus(DownloadStatus.PAUSED);
testDownload.setIsDeleted(false);
testDownload.setUserName("simple/root");
testDownload.setFileName("testFile.txt");
testDownload.setTransport("http");
downloadRepository.save(testDownload);

assertThrows("Cannot modify status of a queued download", ForbiddenException.class, () -> {
userResource.setDownloadStatus(testDownload.getId(), facilityName, sessionId, DownloadStatus.RESTORING.toString());
});

Response response = userResource.getDownloads(facilityName, sessionId, "1 = 1");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with the previous PR, if null can't be used instead of "1 = 1" then this needs a comment as to what it's for.

assertEquals(200, response.getStatus());
List<Download> downloads = (List<Download>) response.getEntity();

Download unmodifiedDownload = findDownload(downloads, testDownload.getId());
assertEquals(DownloadStatus.PAUSED, unmodifiedDownload.getStatus());
}

@Test
public void testGetDownloadTypeStatus() throws Exception {

Expand Down Expand Up @@ -318,7 +350,7 @@ private int getCartSize(Response response) throws Exception {
private Download findDownload(List<Download> downloads, Long downloadId) {

for (Download download : downloads) {
if (download.getId() == downloadId)
if (download.getId().equals(downloadId))
return download;
}
return null;
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/run.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ facility.LILS.icatUrl = https://localhost:8181
facility.LILS.idsUrl = https://localhost:8181
adminUserNames=simple/root
anonUserName=anon/anon
ids.timeout=10s

# Disable scheduled Download status checks (DO THIS FOR TESTS ONLY!)
test.disableDownloadStatusChecks = true
Loading