Merge pull request #83 from ral-facilities/handle-null-sum

Handle null fileSize SUM, null facilityName, int download_id

Author: patrick-austin

src/main/java/org/icatproject/topcat/FacilityMap.java

@@ -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) {

src/main/java/org/icatproject/topcat/IcatClient.java

@@ -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;
@@ -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;
+		}
 	}
 
 	/**

src/main/java/org/icatproject/topcat/web/rest/UserResource.java

@@ -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);
@@ -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";
 		}
@@ -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 {

src/test/java/org/icatproject/topcat/IcatClientTest.java

@@ -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;
@@ -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
@@ -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);

src/test/java/org/icatproject/topcat/UserResourceTest.java

@@ -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());
@@ -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";
@@ -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();

tools/datagateway_admin

@@ -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)
 
 
@@ -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 = []
@@ -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,
@@ -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)
 
@@ -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)