Merge pull request #72 from ral-facilities/36_sessionIds

Use functional credentials for prepareData queue calls to IDS

Author: patrick-austin

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.

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
-	 *                   <code>{"plugin":"db", "credentials:[{"username":"root"}, {"password":"guess"}]}</code>
+	 * @param plugin   ICAT authentication plugin
+	 * @param username ICAT username
+	 * @param password ICAT password
 	 * @return json with sessionId of the form
 	 *         <samp>{"sessionId","0d9a3706-80d4-4d29-9ff3-4d65d4308a24"}</samp>
 	 * @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<String, String>(), 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();

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

@@ -236,18 +236,22 @@ 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;
       if( idsClient == null ) {
     	  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<String, String> 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,43 +323,48 @@ 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<Download> activeDownloadsQuery = em.createQuery(activeQueryString, Download.class);
       List<Download> 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";
     queuedQueryString += " order by download.createdAt";
     TypedQuery<Download> queuedDownloadsQuery = em.createQuery(queuedQueryString, Download.class);
     List<Download> queuedDownloads = queuedDownloadsQuery.getResultList();
 
+    Map<String, String> 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<Integer, List<Download>> 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<Download> 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;
           }
         }

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

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

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}