Range check for lock #267

src/main/java/org/icatproject/core/manager/search/LuceneApi.java

@@ -200,16 +200,45 @@ public SearchResult getResults(JsonObject query, JsonValue searchAfter, Integer
 	 * locked, document modifications will fail (excluding addNow as a result of a
 	 * populate thread).
 	 * 
+	 * A check is also performed against the minId and maxId used for population.
+	 * This ensures that no data is duplicated in the index.
+	 * 
 	 * @param entityName Index to lock.
+	 * @param minId      The exclusive minimum ICAT id being populated for. If
+	 *                   Documents already exist with an id greater than this, the
+	 *                   lock will fail. If null, treated as if it were
+	 *                   Long.MIN_VALUE
+	 * @param maxId      The inclusive maximum ICAT id being populated for. If
+	 *                   Documents already exist with an id less than or equal to
+	 *                   this, the lock will fail. If null, treated as if it were
+	 *                   Long.MAX_VALUE
 	 * @param delete     If true, all existing documents of entityName are deleted.
-	 * @return The largest ICAT id currently stored in the index.
 	 * @throws IcatException
 	 */
 	@Override
-	public long lock(String entityName, boolean delete) throws IcatException {
-		String json = Json.createObjectBuilder().add("delete", delete).build().toString();
-		JsonObject postResponse = postResponse(basePath + "/lock/" + entityName, json);
-		return postResponse.getJsonNumber("currentId").longValueExact();
+	public void lock(String entityName, Long minId, Long maxId, Boolean delete) throws IcatException {
+		String path = basePath + "/lock/" + entityName;
+		try (CloseableHttpClient httpclient = HttpClients.createDefault()) {
+			URIBuilder builder = new URIBuilder(server).setPath(path);
+			if (minId != null) {
+				builder.addParameter("minId", minId.toString());
+			}
+			if (maxId != null) {
+				builder.addParameter("maxId", maxId.toString());
+			}
+			if (delete != null) {
+				builder.addParameter("delete", delete.toString());
+			}
+			URI uri = builder.build();
+			logger.debug("Making call {}", uri);
+			HttpPost httpPost = new HttpPost(uri);
+			try (CloseableHttpResponse response = httpclient.execute(httpPost)) {
+				int code = response.getStatusLine().getStatusCode();
+				Rest.checkStatus(response, code == 400 ? IcatExceptionType.BAD_PARAMETER : IcatExceptionType.INTERNAL);
+			}
+		} catch (URISyntaxException | IOException e) {
+			throw new IcatException(IcatExceptionType.INTERNAL, e.getClass() + " " + e.getMessage());
+		}
 	}
 
 	/**

src/main/java/org/icatproject/core/manager/search/SearchApi.java

@@ -398,13 +398,13 @@ public abstract SearchResult getResults(JsonObject query, JsonValue searchAfter,
 	 * Not implemented.
 	 * 
 	 * @param entityName
+	 * @param minId
+	 * @param maxId
 	 * @param delete
-	 * @return long
 	 * @throws IcatException
 	 */
-	public long lock(String entityName, boolean delete) throws IcatException {
+	public void lock(String entityName, Long minId, Long maxId, Boolean delete) throws IcatException {
 		logger.info("Manually locking index not supported, no request sent");
-		return 0;
 	}
 
 	/**

src/main/java/org/icatproject/core/manager/search/SearchManager.java

@@ -269,13 +269,7 @@ public void run() {
 					if (populatingClassEntry != null) {
 						PopulateBucket bucket = populatingClassEntry.getValue();
 						Long start = bucket.minId != null && bucket.minId > 0 ? bucket.minId : 0;
-						long currentId = searchApi.lock(populatingClassEntry.getKey(), bucket.delete);
-						if (currentId > start) {
-							searchApi.unlock(populatingClassEntry.getKey());
-							populateMap.remove(populatingClassEntry.getKey());
-							throw new IcatException(IcatExceptionType.BAD_PARAMETER, "Populate called starting from id "
-									+ start + " but current id is greater: " + currentId);
-						}
+						searchApi.lock(populatingClassEntry.getKey(), bucket.minId, bucket.maxId, bucket.delete);
 
 						logger.info("Search engine populating " + populatingClassEntry);
 
@@ -327,7 +321,7 @@ public void run() {
 							}
 
 							logger.debug("About to submit {} {} documents from id {} onwards", ids.size(),
-									populatingClassEntry, start);
+									populatingClassEntry.getKey(), start);
 							threads.submit(
 									new IndexSome(populatingClassEntry.getKey(), ids, entityManagerFactory, start));
 							tasks.add(start);
@@ -357,6 +351,7 @@ public void run() {
 				}
 			} catch (Throwable t) {
 				logger.error("Problem encountered in", t);
+				populateMap.remove(populatingClassEntry.getKey());
 			} finally {
 				manager.close();
 				popState = PopState.STOPPED;