Skip to content

Handle null fileSize SUM, null facilityName, int download_id #83

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 28, 2025
Merged

Handle null fileSize SUM, null facilityName, int download_id #83

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
2 changes: 1 addition & 1 deletion src/main/java/org/icatproject/topcat/FacilityMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public FacilityMap(Properties injectedProperties) throws InternalException{
}
}

private String validateFacilityName(String facility) throws InternalException {
public String validateFacilityName(String facility) throws InternalException {
if (facility == null) {
String defaultFacilityName = properties.getProperty("defaultFacilityName");
if (defaultFacilityName == null) {
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/org/icatproject/topcat/IcatClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.icatproject.topcat.domain.*;

import jakarta.json.*;
import jakarta.json.JsonValue.ValueType;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -246,7 +247,12 @@ public long getDatasetFileCount(long datasetId) throws TopcatException {
public long getDatasetFileSize(long datasetId) throws TopcatException {
String query = "SELECT SUM(datafile.fileSize) FROM Datafile datafile WHERE datafile.dataset.id = " + datasetId;
JsonArray jsonArray = submitQuery(query);
return jsonArray.getJsonNumber(0).longValueExact();
if (jsonArray.get(0).getValueType().equals(ValueType.NUMBER)) {
return jsonArray.getJsonNumber(0).longValueExact();
} else {
// SUM will be null if there are no matching Datafiles, so return 0
return 0L;
}
}

/**
Expand Down
10 changes: 10 additions & 0 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 @@ -881,6 +881,7 @@ public Response queueVisitId(@FormParam("facilityName") String facilityName,
logger.info("queueVisitId called for {}", visitId);
validateTransport(transport);

facilityName = validateFacilityName(facilityName);
String icatUrl = getIcatUrl(facilityName);
IcatClient icatClient = new IcatClient(icatUrl, sessionId);
String transportUrl = getDownloadUrl(facilityName, transport);
Expand Down Expand Up @@ -1004,6 +1005,7 @@ public Response queueFiles(@FormParam("facilityName") String facilityName,
}
logger.info("queueFiles called for {} files", files.size());
validateTransport(transport);
facilityName = validateFacilityName(facilityName);
if (fileName == null) {
fileName = facilityName + "_files";
}
Expand Down Expand Up @@ -1178,6 +1180,14 @@ private Response emptyCart(String facilityName, String userName, Long downloadId
private Response emptyCart(String facilityName, String userName) {
return emptyCart(facilityName, userName, null);
}

private String validateFacilityName(String facilityName) throws BadRequestException {
try {
return FacilityMap.getInstance().validateFacilityName(facilityName);
} catch (InternalException ie){
throw new BadRequestException( ie.getMessage() );
}
}

private String getIcatUrl( String facilityName ) throws BadRequestException{
try {
Expand Down
27 changes: 26 additions & 1 deletion src/test/java/org/icatproject/topcat/IcatClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.junit.*;

import jakarta.json.*;
import jakarta.json.JsonValue.ValueType;
import jakarta.ejb.EJB;

import org.icatproject.topcat.httpclient.HttpClient;
Expand Down Expand Up @@ -240,11 +241,29 @@ public void testCheckUserFound() throws Exception {
}
}

@Test
public void testGetDatasets() throws TopcatException {
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);
JsonArray datasets = icatClient.getDatasets("Proposal 0 - 0 0");
for (JsonValue dataset : datasets) {
JsonArray datasetArray = dataset.asJsonArray();
assertEquals(datasetArray.get(0).getValueType(), ValueType.NUMBER);
assertEquals(datasetArray.get(1).getValueType(), ValueType.NUMBER);
assertEquals(datasetArray.get(2).getValueType(), ValueType.NUMBER);
}
}

@Test
public void testGetDatasetFileCount() throws TopcatException {
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);
long datasetId = icatClient.getEntity("Dataset").getJsonNumber("id").longValueExact();
assertNotEquals(0, icatClient.getDatasetFileCount(datasetId));
assertNotEquals(0L, icatClient.getDatasetFileCount(datasetId));
}

@Test
public void testGetDatasetFileCountNotFound() throws TopcatException {
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);
assertEquals(0L, icatClient.getDatasetFileCount(-1L));
}

@Test
Expand All @@ -254,6 +273,12 @@ public void testGetDatasetFileSize() throws TopcatException {
assertNotEquals(0, icatClient.getDatasetFileSize(datasetId));
}

@Test
public void testGetDatasetFileSizeNotFound() throws TopcatException {
IcatClient icatClient = new IcatClient("https://localhost:8181", sessionId);
assertEquals(0, icatClient.getDatasetFileSize(-1L));
}

/*
* @Test public void testGetSize() throws Exception { IcatClient icatClient =
* new IcatClient("https://localhost:8181", sessionId);
Expand Down
6 changes: 2 additions & 4 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 @@ -283,11 +283,10 @@ public void testQueueVisitId() throws Exception {
System.out.println("DEBUG testQueueVisitId");
List<Long> downloadIds = new ArrayList<>();
try {
String facilityName = "LILS";
String transport = "http";
String email = "";
String visitId = "Proposal 0 - 0 0";
Response response = userResource.queueVisitId(facilityName, sessionId, transport, null, email, visitId);
Response response = userResource.queueVisitId(null, sessionId, transport, null, email, visitId);
assertEquals(200, response.getStatus());

JsonArray downloadIdsArray = Utils.parseJsonArray(response.getEntity().toString());
Expand Down Expand Up @@ -351,7 +350,6 @@ public void testQueueFiles() throws Exception {
System.out.println("DEBUG testQueueFiles");
Long downloadId = null;
try {
String facilityName = "LILS";
String transport = "http";
String email = "";
String file = "abcdefghijklmnopqrstuvwxyz";
Expand All @@ -362,7 +360,7 @@ public void testQueueFiles() throws Exception {
for (JsonObject datafile : datafiles) {
files.add(datafile.getString("location"));
}
Response response = userResource.queueFiles(facilityName, sessionId, transport, null, email, files);
Response response = userResource.queueFiles(null, sessionId, transport, null, email, files);
assertEquals(200, response.getStatus());
JsonObject responseObject = Utils.parseJsonObject(response.getEntity().toString());
downloadId = responseObject.getJsonNumber("downloadId").longValueExact();
Expand Down
10 changes: 5 additions & 5 deletions tools/datagateway_admin
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def show_download():
print(requests.get(topcat_url + "/admin/downloads", params={
"facilityName": facility_name,
"sessionId": session_id,
"queryOffset": "where download.id = " + download_id
"queryOffset": f"where download.id = {download_id}"
}, verify=verifySsl).text)


Expand All @@ -124,7 +124,7 @@ def list_file_locations():
download = json.loads(requests.get(topcat_url + "/admin/downloads", params={
"facilityName": facility_name,
"sessionId": session_id,
"queryOffset": "where download.id = " + download_id
"queryOffset": f"where download.id = {download_id}"
}, verify=verifySsl).text)[0]
download_items = download["downloadItems"]
datafile_locations = []
Expand Down Expand Up @@ -175,7 +175,7 @@ def expire_download():

def _expire_download(download_id):
response = requests.put(
topcat_url + "/admin/download/" + download_id + "/status",
f"{topcat_url}/admin/download/{download_id}/status",
data={
"facilityName": facility_name,
"sessionId": session_id,
Expand Down Expand Up @@ -341,7 +341,7 @@ def start_download(download_id):
"sessionId": session_id,
"value": "PREPARING"
}
url = topcat_url + "/admin/download/" + download_id + "/status"
url = f"{topcat_url}/admin/download/{download_id}/status"
response = requests.put(url=url, data=data, verify=verifySsl)
print(response.status_code, response.text)

Expand Down Expand Up @@ -369,7 +369,7 @@ def requeue_download():
"sessionId": session_id,
"value": "QUEUED"
}
url = topcat_url + "/admin/download/" + download_id + "/status"
url = f"{topcat_url}/admin/download/{download_id}/status"
response = requests.put(url=url, data=data, verify=verifySsl)
print(response.status_code, response.text)

Expand Down