diff --git a/src/main/config/run.properties.example b/src/main/config/run.properties.example index 68897f4d..00225be5 100644 --- a/src/main/config/run.properties.example +++ b/src/main/config/run.properties.example @@ -93,6 +93,13 @@ defaultPlugin=simple # Facility to use if it is not specified in the login request defaultFacilityName=LILS +# Queued Downloads are authorized with the user's sessionId, but this may expire before +# the Download is ready to be prepared so these will be submitted to the IDS with a +# functional read all account. +queue.account.LILS.plugin=simple +queue.account.LILS.username=username +queue.account.LILS.password=password + # Limit the number maximum of active RESTORING downloads. Does not affect user submitted carts, # but queued requests will only be started when there are less than this many RESTORING downloads. # Negative values will start all queued jobs immediately, regardless of load. diff --git a/src/main/java/org/icatproject/topcat/IcatClient.java b/src/main/java/org/icatproject/topcat/IcatClient.java index 051d4768..3ebd964b 100644 --- a/src/main/java/org/icatproject/topcat/IcatClient.java +++ b/src/main/java/org/icatproject/topcat/IcatClient.java @@ -38,14 +38,26 @@ public IcatClient(String url, String sessionId) { /** * Login to create a session * - * @param jsonString with plugin and credentials which takes the form - * {"plugin":"db", "credentials:[{"username":"root"}, {"password":"guess"}]} + * @param plugin ICAT authentication plugin + * @param username ICAT username + * @param password ICAT password * @return json with sessionId of the form * {"sessionId","0d9a3706-80d4-4d29-9ff3-4d65d4308a24"} * @throws BadRequestException */ - public String login(String jsonString) throws BadRequestException { + public String login(String plugin, String username, String password) throws BadRequestException { try { + JsonObjectBuilder objectBuilder = Json.createObjectBuilder(); + JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); + JsonObjectBuilder usernameBuilder = Json.createObjectBuilder(); + JsonObjectBuilder passwordBuilder = Json.createObjectBuilder(); + usernameBuilder.add("username", username); + passwordBuilder.add("password", password); + arrayBuilder.add(usernameBuilder); + arrayBuilder.add(passwordBuilder); + objectBuilder.add("plugin", plugin); + objectBuilder.add("credentials", arrayBuilder); + String jsonString = "json=" + objectBuilder.build().toString(); Response response = httpClient.post("session", new HashMap(), jsonString); return response.toString(); } catch (Exception e) { @@ -379,7 +391,8 @@ public int getQueuePriority(String userName) throws TopcatException { } } - if (!userName.equals(Properties.getInstance().getProperty("anonUserName"))) { + if (!userName.startsWith(Properties.getInstance().getProperty("anonUserName"))) { + // The anonymous cart username will end with the user's sessionId so cannot do .equals return priorityMap.getAuthenticatedPriority(); } else { return priorityMap.getDefaultPriority(); diff --git a/src/main/java/org/icatproject/topcat/StatusCheck.java b/src/main/java/org/icatproject/topcat/StatusCheck.java index 55176817..15496c00 100644 --- a/src/main/java/org/icatproject/topcat/StatusCheck.java +++ b/src/main/java/org/icatproject/topcat/StatusCheck.java @@ -236,6 +236,10 @@ private void sendDownloadReadyEmail(Download download) throws InternalException{ } private void prepareDownload(Download download, IdsClient injectedIdsClient) throws Exception { + prepareDownload(download, injectedIdsClient, download.getSessionId()); + } + + private void prepareDownload(Download download, IdsClient injectedIdsClient, String sessionId) throws Exception { try { IdsClient idsClient = injectedIdsClient; @@ -243,11 +247,11 @@ private void prepareDownload(Download download, IdsClient injectedIdsClient) thr idsClient = new IdsClient(getDownloadUrl(download.getFacilityName(),download.getTransport())); } logger.info("Requesting prepareData for Download " + download.getFileName()); - String preparedId = idsClient.prepareData(download.getSessionId(), download.getInvestigationIds(), download.getDatasetIds(), download.getDatafileIds()); + String preparedId = idsClient.prepareData(sessionId, download.getInvestigationIds(), download.getDatasetIds(), download.getDatafileIds()); download.setPreparedId(preparedId); try { - Long size = idsClient.getSize(download.getSessionId(), download.getInvestigationIds(), download.getDatasetIds(), download.getDatafileIds()); + Long size = idsClient.getSize(sessionId, download.getInvestigationIds(), download.getDatasetIds(), download.getDatafileIds()); download.setSize(size); } catch(Exception e) { logger.error("prepareDownload: setting size to -1 as getSize threw exception: " + e.getMessage()); @@ -274,7 +278,31 @@ private void prepareDownload(Download download, IdsClient injectedIdsClient) thr } } - + + /** + * Gets a functional sessionId to use for submitting to the queue, logging in if needed. + * + * @param sessionIds Map from Facility to functional sessionId + * @param facilityName Name of ICAT Facility to get the sessionId for + * @return Functional ICAT sessionId + * @throws InternalException If the facilityName cannot be mapped to an ICAT url + * @throws BadRequestException If the login fails + */ + private String getQueueSessionId(Map sessionIds, String facilityName) + throws InternalException, BadRequestException { + String sessionId = sessionIds.get(facilityName); + if (sessionId == null) { + IcatClient icatClient = new IcatClient(FacilityMap.getInstance().getIcatUrl(facilityName)); + Properties properties = Properties.getInstance(); + String plugin = properties.getProperty("queue.account." + facilityName + ".plugin"); + String username = properties.getProperty("queue.account." + facilityName + ".username"); + String password = properties.getProperty("queue.account." + facilityName + ".password"); + sessionId = icatClient.login(plugin, username, password); + sessionIds.put(facilityName, sessionId); + } + return sessionId; + } + /** * Prepares Downloads which are queued (PAUSED with no preparedId) up to the * maxActiveDownloads limit. @@ -295,16 +323,19 @@ public void startQueuedDownloads(int maxActiveDownloads) throws Exception { String restoringCondition = "download.status = org.icatproject.topcat.domain.DownloadStatus.RESTORING"; String pausedCondition = "download.status = org.icatproject.topcat.domain.DownloadStatus.PAUSED"; + int availableDownloads = maxActiveDownloads; if (maxActiveDownloads > 0) { + // Work out how many "available" spaces there are by accounting for the active Downloads String activeQueryString = selectString + " and " + restoringCondition; TypedQuery activeDownloadsQuery = em.createQuery(activeQueryString, Download.class); List activeDownloads = activeDownloadsQuery.getResultList(); - maxActiveDownloads -= activeDownloads.size(); - if (maxActiveDownloads <= 0) { + int activeDownloadsSize = activeDownloads.size(); + if (activeDownloadsSize >= maxActiveDownloads) { String format = "More downloads currently RESTORING {} than maxActiveDownloads {}, cannot prepare queued jobs"; - logger.info(format, activeDownloads.size(), maxActiveDownloads); + logger.info(format, activeDownloadsSize, maxActiveDownloads); return; } + availableDownloads -= activeDownloadsSize; } String queuedQueryString = selectString + " and " + pausedCondition + " and download.preparedId = null"; @@ -312,26 +343,28 @@ public void startQueuedDownloads(int maxActiveDownloads) throws Exception { TypedQuery queuedDownloadsQuery = em.createQuery(queuedQueryString, Download.class); List queuedDownloads = queuedDownloadsQuery.getResultList(); + Map sessionIds = new HashMap<>(); if (maxActiveDownloads <= 0) { - logger.info("Preparing {} queued downloads", queuedDownloads.size()); // No limits on how many to submit + logger.info("Preparing {} queued downloads", queuedDownloads.size()); for (Download queuedDownload : queuedDownloads) { queuedDownload.setStatus(DownloadStatus.PREPARING); - prepareDownload(queuedDownload, null); + prepareDownload(queuedDownload, null, getQueueSessionId(sessionIds, queuedDownload.getFacilityName())); } } else { - logger.info("Preparing up to {} queued downloads", maxActiveDownloads); + logger.info("Preparing up to {} queued downloads", availableDownloads); HashMap> mapping = new HashMap<>(); for (Download queuedDownload : queuedDownloads) { + String sessionId = getQueueSessionId(sessionIds, queuedDownload.getFacilityName()); String icatUrl = FacilityMap.getInstance().getIcatUrl(queuedDownload.getFacilityName()); - IcatClient icatClient = new IcatClient(icatUrl, queuedDownload.getSessionId()); + IcatClient icatClient = new IcatClient(icatUrl, sessionId); int priority = icatClient.getQueuePriority(queuedDownload.getUserName()); if (priority == 1) { // Highest priority, prepare now queuedDownload.setStatus(DownloadStatus.PREPARING); - prepareDownload(queuedDownload, null); - maxActiveDownloads -= 1; - if (maxActiveDownloads <= 0) { + prepareDownload(queuedDownload, null, sessionId); + availableDownloads -= 1; + if (availableDownloads <= 0) { return; } } else { @@ -350,9 +383,9 @@ public void startQueuedDownloads(int maxActiveDownloads) throws Exception { List downloadList = mapping.get(key); for (Download download : downloadList) { download.setStatus(DownloadStatus.PREPARING); - prepareDownload(download, null); - maxActiveDownloads -= 1; - if (maxActiveDownloads <= 0) { + prepareDownload(download, null, getQueueSessionId(sessionIds, download.getFacilityName())); + availableDownloads -= 1; + if (availableDownloads <= 0) { return; } } diff --git a/src/main/java/org/icatproject/topcat/web/rest/UserResource.java b/src/main/java/org/icatproject/topcat/web/rest/UserResource.java index 67d26c1f..6562f732 100644 --- a/src/main/java/org/icatproject/topcat/web/rest/UserResource.java +++ b/src/main/java/org/icatproject/topcat/web/rest/UserResource.java @@ -108,19 +108,7 @@ public String login(@QueryParam("facilityName") String facilityName, @FormParam( } String icatUrl = getIcatUrl(facilityName); IcatClient icatClient = new IcatClient(icatUrl); - - JsonObjectBuilder objectBuilder = Json.createObjectBuilder(); - JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); - JsonObjectBuilder usernameBuilder = Json.createObjectBuilder(); - JsonObjectBuilder passwordBuilder = Json.createObjectBuilder(); - usernameBuilder.add("username", username); - passwordBuilder.add("password", password); - arrayBuilder.add(usernameBuilder); - arrayBuilder.add(passwordBuilder); - objectBuilder.add("plugin", plugin); - objectBuilder.add("credentials", arrayBuilder); - - return icatClient.login("json=" + objectBuilder.build().toString()); + return icatClient.login(plugin, username, password); } /** @@ -330,8 +318,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 && download.getStatus().equals(DownloadStatus.PAUSED)) { - throw new ForbiddenException("Cannot modify status of a queued download"); + if (download.getPreparedId() == null) { + throw new ForbiddenException("Cannot modify status of a download before it's prepared"); } download.setStatus(DownloadStatus.valueOf(value)); diff --git a/src/test/java/org/icatproject/topcat/UserResourceTest.java b/src/test/java/org/icatproject/topcat/UserResourceTest.java index 7942c163..f168eb12 100644 --- a/src/test/java/org/icatproject/topcat/UserResourceTest.java +++ b/src/test/java/org/icatproject/topcat/UserResourceTest.java @@ -240,7 +240,13 @@ public void testSubmitCart() throws Exception { if (newDownload.getStatus().equals(DownloadStatus.valueOf(downloadStatus))) { downloadStatus = "PAUSED"; } - + // We cannot modify the status of a download without a prepared id + // Using downloadRepository will create a new Download with a different id, + // so remove the old one without the preparedId. + newDownload.setPreparedId("preparedId"); + Download preparedDownload = downloadRepository.save(newDownload); + downloadRepository.removeDownload(downloadId); + downloadId = preparedDownload.getId(); response = userResource.setDownloadStatus(downloadId, facilityName, sessionId, downloadStatus); assertEquals(200, response.getStatus()); @@ -368,6 +374,7 @@ public void testQueueAllowed() throws Exception { assertEquals(true, response.getEntity()); } + @Test public void testSetDownloadStatus() throws Exception { Long downloadId = null; try { @@ -383,9 +390,10 @@ public void testSetDownloadStatus() throws Exception { downloadRepository.save(testDownload); downloadId = testDownload.getId(); - assertThrows("Cannot modify status of a queued download", ForbiddenException.class, () -> { + 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()); Response response = userResource.getDownloads(facilityName, sessionId, null); assertEquals(200, response.getStatus()); diff --git a/src/test/resources/run.properties b/src/test/resources/run.properties index cc547dbb..31b93702 100644 --- a/src/test/resources/run.properties +++ b/src/test/resources/run.properties @@ -11,6 +11,11 @@ ids.timeout=10s # Disable scheduled Download status checks (DO THIS FOR TESTS ONLY!) test.disableDownloadStatusChecks = true + +queue.account.LILS.plugin=simple +queue.account.LILS.username=root +queue.account.LILS.password=pw + # Test data has 100 files per Dataset, set this to a small number to ensure coverage of the batching logic queue.maxFileCount = 1 queue.priority.user = {"simple/test": 1}