Integration tests and fixes #267

src/main/java/org/icatproject/core/entity/Datafile.java

@@ -245,8 +245,7 @@ public static Map<String, Relationship[]> getDocumentFields() throws IcatExcepti
 			Relationship[] textRelationships = {
 					eiHandler.getRelationshipsByName(Datafile.class).get("datafileFormat") };
 			Relationship[] investigationRelationships = {
-					eiHandler.getRelationshipsByName(Datafile.class).get("dataset"),
-					eiHandler.getRelationshipsByName(Dataset.class).get("investigation") }; // TODO check if we need this
+					eiHandler.getRelationshipsByName(Datafile.class).get("dataset") };
 			documentFields.put("text", textRelationships);
 			documentFields.put("name", null);
 			documentFields.put("date", null);

src/main/java/org/icatproject/core/entity/Dataset.java

@@ -191,7 +191,7 @@ public void setType(DatasetType type) {
 	@Override
 	public void getDoc(JsonGenerator gen, SearchApi searchApi) {
 
-		StringBuilder sb = new StringBuilder(name + " " + type.getName() + " " + type.getName());
+		StringBuilder sb = new StringBuilder(name + " " + type.getName() + " " + type.getName()); // TODO duplicate type.getName()
 		if (description != null) {
 			sb.append(" " + description);
 		}
@@ -244,15 +244,13 @@ public void getDoc(JsonGenerator gen, SearchApi searchApi) {
 	public static Map<String, Relationship[]> getDocumentFields() throws IcatException {
 		if (documentFields.size() == 0) {
 			EntityInfoHandler eiHandler = EntityInfoHandler.getInstance();
-			Relationship[] textRelationships = { eiHandler.getRelationshipsByName(Dataset.class).get("type") };
-			Relationship[] investigationRelationships = {
-					eiHandler.getRelationshipsByName(Dataset.class).get("investigation") };
+			Relationship[] textRelationships = { eiHandler.getRelationshipsByName(Dataset.class).get("type"), eiHandler.getRelationshipsByName(Dataset.class).get("sample") };
 			documentFields.put("text", textRelationships);
 			documentFields.put("name", null);
 			documentFields.put("startDate", null);
 			documentFields.put("endDate", null);
 			documentFields.put("id", null);
-			documentFields.put("investigation", investigationRelationships);
+			documentFields.put("investigation", null);
 		}
 		return documentFields;
 	}

src/main/java/org/icatproject/core/manager/EntityBeanManager.java

@@ -790,7 +790,7 @@ private ScoredEntityBaseBean filterReadAccess(List<ScoredEntityBaseBean> results
 			if (beanManaged != null) {
 				try {
 					gateKeeper.performAuthorisation(userId, beanManaged, AccessType.READ, manager);
-					results.add(new ScoredEntityBaseBean(entityId, sr.getScore(), sr.getSource()));
+					results.add(sr);
 					if (results.size() > maxEntities) {
 						throw new IcatException(IcatExceptionType.VALIDATION,
 								"attempt to return more than " + maxEntities + " entities");
@@ -1405,8 +1405,7 @@ public List<ScoredEntityBaseBean> freeTextSearch(String userName, JsonObject jo,
 			int blockSize = Math.max(1000, limit);
 
 			do {
-				List<String> fields = SearchManager.getPublicSearchFields(gateKeeper, klass.getSimpleName());
-				lastSearchResult = searchManager.freeTextSearch(jo, searchAfter, blockSize, sort, fields);
+				lastSearchResult = searchManager.freeTextSearch(jo, searchAfter, blockSize, sort, Arrays.asList("id"));
 				allResults = lastSearchResult.getResults();
 				ScoredEntityBaseBean lastBean = filterReadAccess(results, allResults, limit, userName, manager, klass);
 				if (lastBean == null) {
@@ -1469,14 +1468,14 @@ public SearchResult freeTextSearchDocs(String userName, JsonObject jo, String se
 		if (searchActive) {
 			SearchResult lastSearchResult = null;
 			List<ScoredEntityBaseBean> allResults = Collections.emptyList();
+			List<String> fields = SearchManager.getPublicSearchFields(gateKeeper, klass.getSimpleName());
 			/*
 			 * As results may be rejected and maxCount may be 1 ensure that we
 			 * don't make a huge number of calls to search engine
 			 */
 			int blockSize = Math.max(1000, limit);
 
 			do {
-				List<String> fields = SearchManager.getPublicSearchFields(gateKeeper, klass.getSimpleName());
 				lastSearchResult = searchManager.freeTextSearch(jo, searchAfter, blockSize, sort, fields);
 				allResults = lastSearchResult.getResults();
 				ScoredEntityBaseBean lastBean = filterReadAccess(results, allResults, limit, userName, manager, klass);

src/main/java/org/icatproject/core/manager/GateKeeper.java

@@ -109,7 +109,12 @@ public int compare(String o1, String o2) {
 	 */
 	public boolean allowed(Relationship r) {
 		String beanName = r.getDestinationBean().getSimpleName();
-		if (publicTables.contains(beanName)) {
+		// TODO by using the getter we can update the public tables if needed.
+		// Previous direct access meant we use out of date permissions, so why was there
+		// no update not here before? Needed for the publicSearchFields but if there's a
+		// reason for it to be publicTables and not getPublicTables can manually call
+		// update in SearchManager
+		if (getPublicTables().contains(beanName)) {
 			return true;
 		}
 		String originBeanName = r.getOriginBean().getSimpleName();
@@ -289,7 +294,7 @@ public boolean isAccessAllowed(String user, EntityBaseBean object, AccessType ac
 		if (access == AccessType.CREATE) {
 			qName = Rule.CREATE_QUERY;
 		} else if (access == AccessType.READ) {
-			if (publicTables.contains(simpleName)) {
+			if (getPublicTables().contains(simpleName)) { // TODO see other comment on publicTables vs getPublicTables
 				logger.info("All are allowed " + access + " to " + simpleName);
 				return true;
 			}

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

@@ -78,7 +78,7 @@ public void encodeSortedDocValuesField(JsonGenerator gen, String name, Long valu
 
 	@Override
 	public void encodeSortedDocValuesField(JsonGenerator gen, String name, String value) {
-		encodeStringField(gen, name, value);
+		encodeStringField(gen, name, value); // TODO leading to duplications in the _source, do we need both here?
 		gen.writeStartObject().write("type", "SortedDocValuesField").write("name", name).write("value", value)
 				.writeEnd();
 	}
@@ -156,7 +156,7 @@ public void addNow(String entityName, List<Long> ids, EntityManager manager,
 	@Override
     public String buildSearchAfter(ScoredEntityBaseBean lastBean, String sort) throws IcatException {
 		JsonObjectBuilder builder = Json.createObjectBuilder();
-		builder.add("doc", lastBean.getEntityBaseBeanId());
+		builder.add("doc", lastBean.getEngineDocId());
 		builder.add("shardIndex", -1);
 		if (!Float.isNaN(lastBean.getScore())) {
 			builder.add("score", lastBean.getScore());
@@ -312,13 +312,13 @@ private SearchResult getResults(URI uri, CloseableHttpClient httpclient, String
 					JsonObject responseObject = reader.readObject();
 					List<JsonObject> resultsArray = responseObject.getJsonArray("results").getValuesAs(JsonObject.class);
 					for (JsonObject resultObject: resultsArray) {
-						long id = resultObject.getJsonNumber("id").longValueExact();
+						int luceneDocId = resultObject.getInt("_id");
 						Float score = Float.NaN;
-						if (resultObject.keySet().contains("score")) {
-							score = resultObject.getJsonNumber("score").bigDecimalValue().floatValue();
+						if (resultObject.keySet().contains("_score")) {
+							score = resultObject.getJsonNumber("_score").bigDecimalValue().floatValue();
 						}
-						JsonObject source = resultObject.getJsonObject("source");
-						results.add(new ScoredEntityBaseBean(id, score, source));
+						JsonObject source = resultObject.getJsonObject("_source");
+						results.add(new ScoredEntityBaseBean(luceneDocId, score, source));
 					}
 					if (responseObject.containsKey("search_after")) {
 						lsr.setSearchAfter(responseObject.getJsonObject("search_after").toString());

src/main/java/org/icatproject/core/manager/ScoredEntityBaseBean.java

@@ -2,28 +2,54 @@
 
 import javax.json.JsonObject;
 
+import org.icatproject.core.IcatException;
+import org.icatproject.core.IcatException.IcatExceptionType;
+
 public class ScoredEntityBaseBean {
 
 	private long entityBaseBeanId;
+	private int engineDocId;
 	private float score;
 	private JsonObject source;
 
-	public ScoredEntityBaseBean(String id, float score, JsonObject source) {
-		this.entityBaseBeanId = Long.parseLong(id);
-		this.score = score;
-		this.source = source;
-	}
-
-	public ScoredEntityBaseBean(long id, float score, JsonObject source) {
-		this.entityBaseBeanId = id;
+	/**
+	 * Represents a single entity returned from a search, and relevant search engine
+	 * information.
+	 * 
+	 * @param engineDocId The id of the search engine Document that represents this
+	 *                    entity. This should not be confused with the
+	 *                    entityBaseBeanId. This is needed in order to enable
+	 *                    subsequent searches to "search after" Documents which have
+	 *                    already been returned once.
+	 * @param score       A float generated by the engine to indicate the relevance
+	 *                    of the returned Document to the search term(s). Higher
+	 *                    scores are more relevant. May be null if the results were
+	 *                    not sorted by relevance.
+	 * @param source      JsonObject containing the requested fields of the Document
+	 *                    as key-value pairs. At the very least, this should contain
+	 *                    the ICAT "id" of the entity.
+	 * @throws IcatException If "id" and the corresponding entityBaseBeanId are not
+	 *                       a key-value pair in the source JsonObject.
+	 */
+	public ScoredEntityBaseBean(int engineDocId, float score, JsonObject source) throws IcatException {
+		if (!source.keySet().contains("id")) {
+			throw new IcatException(IcatExceptionType.BAD_PARAMETER,
+					"Document source must have 'id' and the entityBaseBeanId as a key-value pair.");
+		}
+		this.engineDocId = engineDocId;
 		this.score = score;
 		this.source = source;
+		this.entityBaseBeanId = new Long(source.getString("id"));
 	}
 
 	public long getEntityBaseBeanId() {
 		return entityBaseBeanId;
 	}
 
+	public int getEngineDocId() {
+		return engineDocId;
+	}
+
 	public float getScore() {
 		return score;
 	}
@@ -32,8 +58,4 @@ public JsonObject getSource() {
 		return source;
 	}
 
-	public void setSource(JsonObject source) {
-		this.source = source;
-	}
-
 }

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

@@ -8,6 +8,7 @@
 import java.net.URISyntaxException;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
 import java.util.Set;
@@ -243,11 +244,11 @@ public abstract List<FacetDimension> facetSearch(JsonObject facetQuery, int maxR
 			throws IcatException;
 
 	public SearchResult getResults(JsonObject query, int maxResults) throws IcatException {
-		return getResults(query, null, maxResults, null, null);
+		return getResults(query, null, maxResults, null, Arrays.asList("id"));
 	}
 
 	public SearchResult getResults(JsonObject query, int maxResults, String sort) throws IcatException {
-		return getResults(query, null, maxResults, sort, null);
+		return getResults(query, null, maxResults, sort, Arrays.asList("id"));
 	}
 
 	public SearchResult getResults(JsonObject query, String searchAfter, int blockSize, String sort, List<String> fields) throws IcatException {
@@ -369,7 +370,7 @@ private SearchResult getResults(String uid, JsonObject query, int maxResults) th
 				JsonObject jsonObject = jsonReader.readObject();
 				JsonArray hits = jsonObject.getJsonObject("hits").getJsonArray("hits");
 				for (JsonObject hit : hits.getValuesAs(JsonObject.class)) {
-					entities.add(new ScoredEntityBaseBean(hit.getString("_id"), hit.getJsonNumber("_score").bigDecimalValue().floatValue(), null)); // TODO
+					entities.add(new ScoredEntityBaseBean(hit.getInt("_id"), hit.getJsonNumber("_score").bigDecimalValue().floatValue(), null)); // TODO
 				}
 			}
 			return result;

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

@@ -321,9 +321,12 @@ public void run() {
 
 	public static List<String> getPublicSearchFields(GateKeeper gateKeeper, String simpleName) throws IcatException {
 		if (gateKeeper.getPublicSearchFieldsStale() || publicSearchFields.size() == 0) {
-			publicSearchFields.put("Datafile", buildPublicSteps(gateKeeper, Datafile.getDocumentFields()));
-			publicSearchFields.put("Dataset", buildPublicSteps(gateKeeper, Dataset.getDocumentFields()));
-			publicSearchFields.put("Investigation", buildPublicSteps(gateKeeper, Investigation.getDocumentFields()));
+			logger.info("Building public search fields from public tables and steps");
+			publicSearchFields.put("Datafile", buildPublicSearchFields(gateKeeper, Datafile.getDocumentFields()));
+			publicSearchFields.put("Dataset", buildPublicSearchFields(gateKeeper, Dataset.getDocumentFields()));
+			publicSearchFields.put("Investigation",
+					buildPublicSearchFields(gateKeeper, Investigation.getDocumentFields()));
+			gateKeeper.markPublicSearchFieldsFresh();
 		}
 		return publicSearchFields.get(simpleName);
 	}
@@ -397,14 +400,17 @@ public void deleteDocument(EntityBaseBean bean) throws IcatException {
 		}
 	}
 
-	private static List<String> buildPublicSteps(GateKeeper gateKeeper, Map<String, Relationship[]> map) {
+	private static List<String> buildPublicSearchFields(GateKeeper gateKeeper, Map<String, Relationship[]> map) {
 		List<String> fields = new ArrayList<>();
-		for (Entry<String, Relationship[]> entry: map.entrySet()) {
+		for (Entry<String, Relationship[]> entry : map.entrySet()) {
 			Boolean includeField = true;
 			if (entry.getValue() != null) {
-				for (Relationship relationship: entry.getValue()) {
+				for (Relationship relationship : entry.getValue()) {
 					if (!gateKeeper.allowed(relationship)) {
 						includeField = false;
+						logger.debug("Access to {} blocked by disallowed relationship between {} and {}", entry.getKey(),
+								relationship.getOriginBean().getSimpleName(),
+								relationship.getDestinationBean().getSimpleName());
 						break;
 					}
 				}
@@ -416,9 +422,9 @@ private static List<String> buildPublicSteps(GateKeeper gateKeeper, Map<String,
 		return fields;
 	}
 
-    public String buildSearchAfter(ScoredEntityBaseBean lastBean, String sort) throws IcatException {
-        return searchApi.buildSearchAfter(lastBean, sort);
-    }
+	public String buildSearchAfter(ScoredEntityBaseBean lastBean, String sort) throws IcatException {
+		return searchApi.buildSearchAfter(lastBean, sort);
+	}
 
 	private void pushPendingCalls() {
 		timer.schedule(new EnqueuedSearchRequestHandler(), 0L);
@@ -469,7 +475,8 @@ public SearchResult freeTextSearch(JsonObject jo, int blockSize, String sort) th
 		return searchApi.getResults(jo, blockSize, sort);
 	}
 
-	public SearchResult freeTextSearch(JsonObject jo, String searchAfter, int blockSize, String sort, List<String> fields) throws IcatException {
+	public SearchResult freeTextSearch(JsonObject jo, String searchAfter, int blockSize, String sort,
+			List<String> fields) throws IcatException {
 		return searchApi.getResults(jo, searchAfter, blockSize, sort, fields);
 	}