From c8da4fa76af2f8b37bd061630febc62864b96947 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 26 May 2026 17:22:04 +0100 Subject: [PATCH 01/40] Use BooleanInstruction approach for getting fasttrack concepts --- .../cl/dtg/isaac/api/managers/FastTrackManger.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java index 141df37786..8a4aec3a48 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java @@ -11,9 +11,10 @@ import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import java.util.Arrays; @@ -68,11 +69,10 @@ public final boolean isValidFasTrackGameboardId(final String gameboardId) { * @throws ContentManagerException if content cant be found which matches the question ID. */ public final String getConceptFromQuestionId(final String questionId) throws ContentManagerException { - Map> fieldsToMatch = Maps.newHashMap(); - fieldsToMatch.put(TYPE_FIELDNAME, Arrays.asList(FAST_TRACK_QUESTION_TYPE)); - fieldsToMatch.put(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, Arrays.asList(questionId)); - ResultsWrapper resultsList = contentManager.findByFieldNames( - ContentService.generateDefaultFieldToMatch(fieldsToMatch), 0, DEFAULT_RESULTS_LIMIT); + BooleanInstruction fieldsToMatch = new BooleanInstruction(); + fieldsToMatch.must(new MatchInstruction(TYPE_FIELDNAME, FAST_TRACK_QUESTION_TYPE)); + fieldsToMatch.must(new MatchInstruction(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, questionId)); + ResultsWrapper resultsList = this.contentManager.nestedMatchSearch(fieldsToMatch, 0, DEFAULT_RESULTS_LIMIT, null, null); String upperConceptTitle = ""; if (resultsList.getTotalResults() == 1) { From edfe46c986069bb464670da38e462d3a039b66a0 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 27 May 2026 14:24:01 +0100 Subject: [PATCH 02/40] Use BooleanInstruction approach for getting fasttrack questions --- .../isaac/api/managers/FastTrackManger.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java index 8a4aec3a48..b0f77ec080 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java @@ -1,6 +1,5 @@ package uk.ac.cam.cl.dtg.isaac.api.managers; -import com.google.api.client.util.Lists; import com.google.api.client.util.Maps; import com.google.inject.Inject; import org.slf4j.Logger; @@ -18,12 +17,10 @@ import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; @@ -113,24 +110,25 @@ public final List getConceptProgress( private List getFastTrackConceptQuestions( final String boardTag, final List levelFilters, final String conceptTitle ) throws ContentManagerException { - List stringLevelFilters = levelFilters.stream().map(FASTTRACK_LEVEL::name).collect(Collectors.toList()); - - List fieldsToMap = Lists.newArrayList(); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, Constants.BooleanOperator.OR, QUESTION_PAGE_TYPES)); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, Constants.BooleanOperator.AND, - Collections.singletonList(conceptTitle))); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TAGS_FIELDNAME, Constants.BooleanOperator.AND, Collections.singletonList(boardTag))); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TAGS_FIELDNAME, Constants.BooleanOperator.OR, stringLevelFilters)); + List stringLevelFilters = levelFilters.stream().map(FASTTRACK_LEVEL::name).toList(); + BooleanInstruction searchInstruction = new BooleanInstruction(); + + for (String type : QUESTION_PAGE_TYPES) { + searchInstruction.should(new MatchInstruction(TYPE_FIELDNAME, type)); + } + for (String levelFilter : stringLevelFilters) { + searchInstruction.should(new MatchInstruction(TAGS_FIELDNAME, levelFilter)); + } + searchInstruction.must(new MatchInstruction(TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, conceptTitle)); + searchInstruction.must(new MatchInstruction(TAGS_FIELDNAME, boardTag)); + Map sortInstructions = Maps.newHashMap(); sortInstructions.put(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, Constants.SortOrder.ASC); - return this.contentManager - .findByFieldNames(fieldsToMap, 0, SEARCH_MAX_WINDOW_SIZE, sortInstructions) - .getResults(); + ResultsWrapper results = this.contentManager + .nestedMatchSearch(searchInstruction, 0, SEARCH_MAX_WINDOW_SIZE, null, sortInstructions); + + return results.getResults(); } } From 94dd09d52bb9ce30e50f9c9f77d249a03146d614 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 27 May 2026 14:34:28 +0100 Subject: [PATCH 03/40] Use BooleanInstruction approach for finding available quizzes --- .../dtg/isaac/api/managers/QuizManager.java | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java index 828ef02ef2..2521de2dfc 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java @@ -15,7 +15,6 @@ */ package uk.ac.cam.cl.dtg.isaac.api.managers; -import com.google.api.client.util.Lists; import com.google.inject.Inject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,10 +36,11 @@ import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import jakarta.annotation.Nullable; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -58,7 +58,6 @@ public class QuizManager { private static final Logger log = LoggerFactory.getLogger(QuizManager.class); private final AbstractConfigLoader properties; - private final ContentService contentService; private final GitContentManager contentManager; private final ContentSummarizerService contentSummarizerService; @@ -79,29 +78,25 @@ public QuizManager(final AbstractConfigLoader properties, final ContentService c final GitContentManager contentManager, final ContentSummarizerService contentSummarizerService) { this.properties = properties; - this.contentService = contentService; this.contentManager = contentManager; this.contentSummarizerService = contentSummarizerService; } - public ResultsWrapper getAvailableQuizzes(String visibleToRole, @Nullable Integer startIndex, @Nullable Integer limit, boolean showNoFilterQuizzes) throws ContentManagerException { + public ResultsWrapper getAvailableQuizzes(final String visibleToRole, @Nullable final Integer startIndex, + @Nullable final Integer limit, final boolean showNoFilterQuizzes) throws ContentManagerException { - List fieldsToMatch = Lists.newArrayList(); - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, Constants.BooleanOperator.AND, Collections.singletonList(QUIZ_TYPE))); + BooleanInstruction searchInstruction = new BooleanInstruction(); + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, QUIZ_TYPE)); if (!showNoFilterQuizzes) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(TAGS_FIELDNAME, BooleanOperator.NOT, - Collections.singletonList(HIDE_FROM_FILTER_TAG))); + searchInstruction.mustNot(new MatchInstruction(TAGS_FIELDNAME, HIDE_FROM_FILTER_TAG)); } if (null != visibleToRole) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(HIDDEN_FROM_ROLES_FIELDNAME, - BooleanOperator.NOT, Collections.singletonList(visibleToRole))); + searchInstruction.mustNot(new MatchInstruction(HIDDEN_FROM_ROLES_FIELDNAME, visibleToRole)); } - ResultsWrapper content = this.contentService.findMatchingContent(fieldsToMatch, startIndex, limit); - + ResultsWrapper content = this.contentManager.nestedMatchSearch(searchInstruction, startIndex, limit, null, null); return this.contentSummarizerService.extractContentSummaryFromResultsWrapper(content, QuizSummaryDTO.class); } From 73c5b091abf58617939c312b4f5f737ea00f4a6d Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 27 May 2026 14:41:02 +0100 Subject: [PATCH 04/40] Use BooleanInstruction approach for finding glossary terms --- .../uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java index 524091c581..db1ede24a2 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java @@ -29,6 +29,8 @@ import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import uk.ac.cam.cl.dtg.segue.dao.ILogManager; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import jakarta.ws.rs.GET; @@ -41,8 +43,6 @@ import jakarta.ws.rs.core.Request; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; -import java.util.Collections; -import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -135,12 +135,9 @@ public final Response getTerms(@Context final Request request, @QueryParam("star // Get from server cache, else load and cache: try { ResultsWrapper c = termCache.get(cacheKey, () -> { - List fieldsToMatch = Collections.singletonList( - new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, BooleanOperator.AND, Collections.singletonList("glossaryTerm")) - ); - - return this.contentManager.findByFieldNames(fieldsToMatch, startIndexOfResults, resultsLimit); + BooleanInstruction searchInstruction = new BooleanInstruction();; + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, "glossaryTerm")); + return this.contentManager.nestedMatchSearch(searchInstruction, startIndexOfResults, resultsLimit, null, null); }); return Response.ok(c).tag(etag).cacheControl(getCacheControl(NUMBER_SECONDS_IN_ONE_HOUR, true)).build(); From d211f00b9fc0530d86b40def70cc423709818cd3 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 27 May 2026 14:45:56 +0100 Subject: [PATCH 05/40] Use BooleanInstruction approach for getting notifications --- .../cl/dtg/segue/api/managers/NotificationPicker.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java index 7fc61e78a3..52b7390edf 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java @@ -30,6 +30,8 @@ import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import java.util.Calendar; import java.util.Collections; @@ -73,12 +75,11 @@ public NotificationPicker(final GitContentManager contentManager, final PgUserNo public List getAvailableNotificationsForUser(final RegisteredUserDTO user) throws ContentManagerException, SegueDatabaseException { // get users notification record - List fieldsToMatch = Lists.newArrayList(); - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, BooleanOperator.AND, Collections.singletonList("notification"))); + BooleanInstruction searchInstruction = new BooleanInstruction(); + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, "notification")); ResultsWrapper allContentNotifications = this.contentManager - .findByFieldNames(fieldsToMatch, 0, -1); + .nestedMatchSearch(searchInstruction, 0, -1, null, null); Map listOfRecordedNotifications = getMapOfRecordedNotifications(user); From 00dbc6c829db1fde404af1fe032c48a6726864fa Mon Sep 17 00:00:00 2001 From: James Sharkey Date: Wed, 27 May 2026 15:50:07 +0100 Subject: [PATCH 06/40] Remove deprecated gameboard endpoint and methods This endpoint has been unsupported for over a year, and for two months has been inaccessible in production (since 3d839bfdc81b314f33f3e9f906af054bdeecba3a). Several deprecated methods were required for it to work, and without it these too can be removed. This gets rid of at least one more deprecated ES method. Since we no longer generate random gameboards, we no longer have any need to store temporary boards. The cache can be removed too, but for now I have just removed the "store new temporary board" method. --- .../cl/dtg/isaac/api/GameboardsFacade.java | 90 ----- .../dtg/isaac/api/managers/GameManager.java | 327 +----------------- .../dao/GameboardPersistenceManager.java | 18 +- .../segue/dao/content/GitContentManager.java | 31 -- .../api/AbstractIsaacIntegrationTest.java | 2 +- .../isaac/api/managers/GameManagerTest.java | 100 ------ .../dtg/isaac/app/GameboardsFacadeTest.java | 125 ------- 7 files changed, 4 insertions(+), 689 deletions(-) delete mode 100644 src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java delete mode 100644 src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/GameboardsFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/GameboardsFacade.java index 281dcfa3c5..2137651bc9 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/GameboardsFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/GameboardsFacade.java @@ -124,96 +124,6 @@ public GameboardsFacade(final AbstractConfigLoader properties, final ILogManager this.fastTrackManger = fastTrackManger; } - /** - * REST end point to provide a Temporary Gameboard stored in volatile storage. - * - * @param request - * - this allows us to check to see if a user is currently loggedin. - * @param title - * - the title of the generated board - * @param subjects - * - a comma separated list of subjects - * @param fields - * - a comma separated list of fields - * @param topics - * - a comma separated list of topics - * @param levels - * - a comma separated list of levels - * @param concepts - * - a comma separated list of conceptIds - * @param questionCategories - * - a comma separated list of question categories - * @return a Response containing a gameboard object or containing a SegueErrorResponse. - */ - @Deprecated - @GET - @Path("gameboards") - @Produces(MediaType.APPLICATION_JSON) - @GZIP - @Operation(summary = "Get a temporary board of questions matching provided constraints.") - public final Response generateTemporaryGameboard(@Context final HttpServletRequest request, - @QueryParam("title") final String title, @QueryParam("subjects") final String subjects, - @QueryParam("fields") final String fields, @QueryParam("topics") final String topics, - @QueryParam("stages") final String stages, @QueryParam("difficulties") final String difficulties, - @QueryParam("examBoards") final String examBoards, @QueryParam("levels") final String levels, - @QueryParam("concepts") final String concepts, @QueryParam("questionCategories") final String questionCategories) { - List subjectsList = splitCsvStringQueryParam(subjects); - List fieldsList = splitCsvStringQueryParam(fields); - List topicsList = splitCsvStringQueryParam(topics); - List levelsList = null; - List stagesList = splitCsvStringQueryParam(stages); - List difficultiesList = splitCsvStringQueryParam(difficulties); - List examBoardsList = splitCsvStringQueryParam(examBoards); - List conceptsList = splitCsvStringQueryParam(concepts); - List questionCategoriesList = splitCsvStringQueryParam(questionCategories); - - if (null != levels && !levels.isEmpty()) { - String[] levelsAsString = levels.split(","); - - levelsList = Lists.newArrayList(); - for (String s : levelsAsString) { - try { - levelsList.add(Integer.parseInt(s)); - } catch (NumberFormatException e) { - return new SegueErrorResponse(Status.BAD_REQUEST, "Levels must be numbers if specified.", e) - .toResponse(); - } - } - } - - try { - log.warn("Method generateTemporaryGameboard was called by an API request!"); - // FIXME: remove endpoint after 2026-05. brownout below, in a revertible way, until removal. - if (!getProperties().getProperty(SEGUE_APP_ENVIRONMENT).equals(EnvironmentType.DEV.name())) { - return SegueErrorResponse.getGoneResponse(); - } - - AbstractSegueUserDTO boardOwner = this.userManager.getCurrentUser(request); - GameboardDTO gameboard; - - gameboard = gameManager.generateRandomGameboard(title, subjectsList, fieldsList, topicsList, levelsList, - conceptsList, questionCategoriesList, stagesList, difficultiesList, examBoardsList,boardOwner); - - if (null == gameboard) { - return new SegueErrorResponse(Status.NO_CONTENT, - "We cannot find any questions based on your filter criteria.").toResponse(); - } - - return Response.ok(gameboard).cacheControl(getCacheControl(NEVER_CACHE_WITHOUT_ETAG_CHECK, false)).build(); - } catch (IllegalArgumentException e) { - return new SegueErrorResponse(Status.BAD_REQUEST, "Your gameboard filter request is invalid.").toResponse(); - } catch (SegueDatabaseException e) { - String message = "SegueDatabaseException whilst generating a gameboard"; - log.error(message, e); - return new SegueErrorResponse(Status.INTERNAL_SERVER_ERROR, message).toResponse(); - } catch (ContentManagerException e1) { - SegueErrorResponse error = new SegueErrorResponse(Status.NOT_FOUND, "Error locating the version requested", - e1); - log.error(error.getErrorMessage(), e1); - return error.toResponse(); - } - } - /** * REST end point to retrieve a specific gameboard by Id. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java index a87c12a6e1..55955cfbf4 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java @@ -17,7 +17,6 @@ import com.google.api.client.util.Lists; import com.google.api.client.util.Maps; -import com.google.common.collect.Sets; import com.google.inject.Inject; import org.apache.commons.collections4.comparators.ComparatorChain; import org.apache.commons.lang3.Validate; @@ -27,7 +26,6 @@ import uk.ac.cam.cl.dtg.isaac.dao.GameboardPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dos.AudienceContext; import uk.ac.cam.cl.dtg.isaac.dos.GameboardContentDescriptor; -import uk.ac.cam.cl.dtg.isaac.dos.GameboardCreationMethod; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuestionPage; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; import uk.ac.cam.cl.dtg.isaac.dos.LightweightQuestionValidationResponse; @@ -40,7 +38,6 @@ import uk.ac.cam.cl.dtg.isaac.dto.GameboardDTO; import uk.ac.cam.cl.dtg.isaac.dto.GameboardItem; import uk.ac.cam.cl.dtg.isaac.dto.GameboardListDTO; -import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuestionPageDTO; import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuickQuestionDTO; import uk.ac.cam.cl.dtg.isaac.dto.IsaacWildcardDTO; import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; @@ -56,7 +53,6 @@ import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; -import uk.ac.cam.cl.dtg.util.mappers.MainMapper; import jakarta.annotation.Nullable; import jakarta.validation.constraints.NotNull; @@ -67,7 +63,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Random; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -84,13 +79,10 @@ public class GameManager { private static final float DEFAULT_QUESTION_PASS_MARK = 75; - private static final int MAX_QUESTIONS_TO_SEARCH = 20; private static final int GAMEBOARD_QUESTIONS_DEFAULT = 10; private static int gameboardQuestionsLimit; private final GameboardPersistenceManager gameboardPersistenceManager; - private final Random randomGenerator; - private final MainMapper mapper; private final GitContentManager contentManager; private final QuestionManager questionManager; @@ -103,98 +95,22 @@ public class GameManager { * - so we can augment game objects with actual detailed content * @param gameboardPersistenceManager * - a persistence manager that deals with storing and retrieving gameboards. - * @param mapper - * - allows mapping between DO and DTO object types. */ @Inject public GameManager(final GitContentManager contentManager, - final GameboardPersistenceManager gameboardPersistenceManager, final MainMapper mapper, + final GameboardPersistenceManager gameboardPersistenceManager, final QuestionManager questionManager, final AbstractConfigLoader properties) { this.contentManager = contentManager; this.gameboardPersistenceManager = gameboardPersistenceManager; this.questionManager = questionManager; - this.randomGenerator = new Random(); - - this.mapper = mapper; - try { GameManager.gameboardQuestionsLimit = Integer.parseInt(properties.getProperty(GAMEBOARD_QUESTION_LIMIT)); } catch (NumberFormatException e) { GameManager.gameboardQuestionsLimit = GAMEBOARD_QUESTIONS_DEFAULT; } } - - /** - * This method expects only one of its 3 subject tag filter parameters to have more than one element due to - * restrictions on the question filter interface. - * - * @param title - * title of the board - * @param subjects - * list of subjects to include in filtered results - * @param fields - * list of fields to include in filtered results - * @param topics - * list of topics to include in filtered results - * @param levels - * list of levels to include in filtered results - * @param concepts - * list of concepts (relatedContent) to include in filtered results - * @param questionCategories - * list of question categories (i.e. problem_solving, book) to include in filtered results - * @param boardOwner - * The user that should be marked as the creator of the gameBoard. - * @return a gameboard if possible that satisfies the conditions provided by the parameters. Will return null if no - * questions can be provided. - * @throws SegueDatabaseException - * - if there is an error contacting the database. - * @throws ContentManagerException - * - if there is an error retrieving the content requested. - */ - @Deprecated - public GameboardDTO generateRandomGameboard( - final String title, final List subjects, final List fields, final List topics, - final List levels, final List concepts, final List questionCategories, - final List stages, final List difficulties, final List examBoards, - final AbstractSegueUserDTO boardOwner) - throws SegueDatabaseException, ContentManagerException { - - Long boardOwnerId; - if (boardOwner instanceof RegisteredUserDTO registeredUser) { - boardOwnerId = registeredUser.getId(); - } else { - // anonymous users do not get to own a board so just mark it as unowned. - boardOwnerId = null; - } - - Map>> usersQuestionAttempts = questionManager - .getQuestionAttemptsByUser(boardOwner); - - GameFilter gameFilter = new GameFilter( - subjects, fields, topics, levels, concepts, questionCategories, stages, difficulties, examBoards); - - List selectionOfGameboardQuestions = - this.getSelectedGameboardQuestions(gameFilter, usersQuestionAttempts); - - if (!selectionOfGameboardQuestions.isEmpty()) { - String uuid = UUID.randomUUID().toString(); - - // filter game board ready questions to make up a decent gameboard. - log.debug("Created gameboard: '{}'.", uuid); - - GameboardDTO gameboardDTO = new GameboardDTO(uuid, title, selectionOfGameboardQuestions, - null, null, new Date(), gameFilter, - boardOwnerId, GameboardCreationMethod.FILTER, Sets.newHashSet()); - - this.gameboardPersistenceManager.temporarilyStoreGameboard(gameboardDTO); - - return augmentGameboardWithQuestionAttemptInformation(gameboardDTO, usersQuestionAttempts); - } else { - return null; - } - } /** * linkUserToGameboard. Persist the gameboard in permanent storage and also link it to the users account. @@ -698,7 +614,6 @@ public List>> gatherGamePro return result; } - /** * Find all wildcards. * @@ -977,139 +892,6 @@ private void permanentlyStoreGameboard(final GameboardDTO gameboardToStore) thro this.gameboardPersistenceManager.saveGameboardToPermanentStorage(gameboardToStore); } - /** - * This method aims to (somewhat) intelligently select some useful gameboard questions. - * - * @param gameFilter - * - the filter query that should be used to make up the gameboard. - * @param usersQuestionAttempts - * - the users question attempt information if available. - * @return Gameboard questions - * @throws ContentManagerException - * - if there is an error retrieving the content requested. - */ - @Deprecated - private List getSelectedGameboardQuestions(final GameFilter gameFilter, - final Map>> usersQuestionAttempts) - throws ContentManagerException { - - Long seed = new Random().nextLong(); - int searchIndex = 0; - List selectionOfGameboardQuestions = this.getNextQuestionsForFilter(gameFilter, searchIndex, - seed); - - if (selectionOfGameboardQuestions.isEmpty()) { - // no questions found just return an empty list. - return Lists.newArrayList(); - } - - Set gameboardReadyQuestions = Sets.newHashSet(); - List completedQuestions = Lists.newArrayList(); - // choose the gameboard questions to include. - while (gameboardReadyQuestions.size() < GAMEBOARD_QUESTIONS_DEFAULT && !selectionOfGameboardQuestions.isEmpty()) { - for (GameboardItem gameboardItem : selectionOfGameboardQuestions) { - CompletionState questionState; - try { - this.augmentGameItemWithAttemptInformation(gameboardItem, usersQuestionAttempts); - questionState = gameboardItem.getState(); - } catch (ResourceNotFoundException e) { - throw new ContentManagerException( - "Resource not found exception, this shouldn't happen as the selectionOfGameboardQuestions " - + "should only show available content."); - } - - if (questionState.equals(CompletionState.ALL_ATTEMPTED) - || questionState.equals(CompletionState.ALL_CORRECT)) { - completedQuestions.add(gameboardItem); - } else { - gameboardReadyQuestions.add(gameboardItem); - } - - // stop inner loop if we have reached our target - if (gameboardReadyQuestions.size() == GAMEBOARD_QUESTIONS_DEFAULT) { - break; - } - } - - if (gameboardReadyQuestions.size() == GAMEBOARD_QUESTIONS_DEFAULT) { - break; - } - - // increment search start index to see if we can get more questions for the given criteria. - searchIndex = searchIndex + selectionOfGameboardQuestions.size(); - - // we couldn't fill it up on the last round of questions so lets get more if no more this will be empty - selectionOfGameboardQuestions = this.getNextQuestionsForFilter(gameFilter, searchIndex, seed); - } - - // Try and make up the difference with completed ones if we haven't reached our target size - if (gameboardReadyQuestions.size() < GAMEBOARD_QUESTIONS_DEFAULT && !completedQuestions.isEmpty()) { - for (GameboardItem completedQuestion : completedQuestions) { - if (gameboardReadyQuestions.size() < GAMEBOARD_QUESTIONS_DEFAULT) { - gameboardReadyQuestions.add(completedQuestion); - } else if (gameboardReadyQuestions.size() == GAMEBOARD_QUESTIONS_DEFAULT) { - break; - } - } - } - - // Convert to List and randomise the questions again, as we may have injected some completed questions. - List gameboardQuestionList = Lists.newArrayList(gameboardReadyQuestions); - Collections.shuffle(gameboardQuestionList); - - return gameboardQuestionList; - } - - /** - * Gets you the next set of questions that match the given filter. - * - * @param gameFilter - * - to enable search - * @param index - * - the starting index of the query. - * @param randomSeed - * - so that we can use search pagination and not repeat ourselves. - * @return a list of gameboard items. - * @throws ContentManagerException - * - if there is a problem accessing the content repository. - */ - @Deprecated - public List getNextQuestionsForFilter(final GameFilter gameFilter, final int index, - final Long randomSeed) throws ContentManagerException { - // get some questions - List fieldsToMap = Lists.newArrayList(); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, BooleanOperator.AND, Collections.singletonList(QUESTION_TYPE))); - fieldsToMap.addAll(generateFieldToMatchForQuestionFilter(gameFilter)); - - // Search for questions that match the fields to map variable. - - ResultsWrapper results = this.contentManager.findByFieldNamesRandomOrder( - fieldsToMap, index, MAX_QUESTIONS_TO_SEARCH, randomSeed); - - List questionsForGameboard = results.getResults(); - - List selectionOfGameboardQuestions = Lists.newArrayList(); - - // Map each Content object into an GameboardItem object - for (ContentDTO c : questionsForGameboard) { - // Only keep questions that have not been superseded. - // Yes, this should probably be done in the fieldsToMap filter above, but this is simpler. - if (c instanceof IsaacQuestionPageDTO qp) { - if (qp.getSupersededBy() != null && !qp.getSupersededBy().isEmpty()) { - // This question has been superseded. Don't include it. - continue; - } - } - - GameboardItem questionInfo = this.gameboardPersistenceManager.convertToGameboardItem( - c, new GameboardContentDescriptor(c.getId(), QUESTION_TYPE, AudienceContext.fromFilter(gameFilter))); - selectionOfGameboardQuestions.add(questionInfo); - } - - return selectionOfGameboardQuestions; - } - /** * AugmentGameItemWithAttemptInformation * @@ -1195,113 +977,6 @@ private void augmentGameItemWithAttemptInformation( gameItem.setState(state); } - /** - * Helper method to generate field to match requirements for search queries (specialised for isaac-filtering rules) - * - * This method will decide what should be AND and what should be OR based on the field names used. - * - * @param gameFilter - * - filter object containing all the filter information used to make this board. - * @return A map ready to be passed to a content provider - */ - private static List generateFieldToMatchForQuestionFilter( - final GameFilter gameFilter) { - - // Validate that the field sizes are as we expect for tags - // Check that the query provided adheres to the rules we expect - if (!validateFilterQuery(gameFilter)) { - throw new IllegalArgumentException("Error validating filter query."); - } - - List fieldsToMatch = Lists.newArrayList(); - - // handle question categories - if (null != gameFilter.getQuestionCategories()) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - TAGS_FIELDNAME, BooleanOperator.OR, gameFilter.getQuestionCategories())); - } - - // Filter on content tags - List tagAnds = Lists.newArrayList(); - List tagOrs = Lists.newArrayList(); - - // deal with tags which represent subjects, fields and topics - if (null != gameFilter.getSubjects()) { - if (gameFilter.getSubjects().size() > 1) { - tagOrs.addAll(gameFilter.getSubjects()); - } else { // should be exactly 1 - tagAnds.addAll(gameFilter.getSubjects()); - - // ok now we are allowed to look at the fields - if (null != gameFilter.getFields()) { - // If multiple fields are chosen, don't filter by field at all, unless there are no topics - // /!\ This was changed for the CS question finder, and doesn't break the PHY question finder - if (gameFilter.getFields().size() == 1) { - tagAnds.addAll(gameFilter.getFields()); - } else if (null == gameFilter.getTopics()) { - tagOrs.addAll(gameFilter.getFields()); - } - // Now we look at topics - if (null != gameFilter.getTopics()) { - if (gameFilter.getTopics().size() > 1) { - tagOrs.addAll(gameFilter.getTopics()); - } else { - tagAnds.addAll(gameFilter.getTopics()); - } - } - } - } - } - - // deal with adding overloaded tags field for subjects, fields and topics - if (tagAnds.size() > 0) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(TAGS_FIELDNAME, BooleanOperator.AND, tagAnds)); - } - if (tagOrs.size() > 0) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(TAGS_FIELDNAME, BooleanOperator.OR, tagOrs)); - } - - // now deal with levels - if (null != gameFilter.getLevels()) { - List levelsAsStrings = Lists.newArrayList(); - for (Integer levelInt : gameFilter.getLevels()) { - levelsAsStrings.add(levelInt.toString()); - } - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(LEVEL_FIELDNAME, BooleanOperator.OR, levelsAsStrings)); - } - - // Handle the nested audience fields: stage, difficulty and examBoard - if (null != gameFilter.getStages()) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - STAGE_FIELDNAME, BooleanOperator.OR, gameFilter.getStages())); - } - if (null != gameFilter.getDifficulties()) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - DIFFICULTY_FIELDNAME, BooleanOperator.OR, gameFilter.getDifficulties())); - } - if (null != gameFilter.getExamBoards()) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - EXAM_BOARD_FIELDNAME, BooleanOperator.OR, gameFilter.getExamBoards())); - } - - // handle concepts - if (null != gameFilter.getConcepts()) { - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - RELATED_CONTENT_FIELDNAME, BooleanOperator.OR, gameFilter.getConcepts())); - } - - // handle exclusions - // exclude questions with no-filter tag - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(TAGS_FIELDNAME, BooleanOperator.NOT, - Collections.singletonList(HIDE_FROM_FILTER_TAG))); - - // exclude questions marked deprecated - fieldsToMatch.add(new GitContentManager.BooleanSearchClause(DEPRECATED_FIELDNAME, BooleanOperator.NOT, - Collections.singletonList("true"))); - - return fieldsToMatch; - } - /** * Currently only validates subjects, fields and topics. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java index 33fed3a785..2dd77c8552 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java @@ -106,7 +106,8 @@ public GameboardPersistenceManager(final PostgresSqlDb database, final GitConten this.database = database; this.mapper = mapper; this.contentManager = contentManager; - this.objectMapper = objectMapper.getSharedContentObjectMapper();; + this.objectMapper = objectMapper.getSharedContentObjectMapper(); + // FIXME: since removal of generateRandomGameboard, no more temporary boards to store, so this can go too: this.gameboardNonPersistentStorage = CacheBuilder.newBuilder() .expireAfterAccess(GAMEBOARD_TTL_MINUTES, TimeUnit.MINUTES). build(); } @@ -165,21 +166,6 @@ public List getLiteGameboardsByIds(final Collection gamebo return this.getGameboardsByIds(gameboardIds, false); } - /** - * Keep generated gameboard in non-persistent storage. - * - * This will be removed if the gameboard is saved to persistent storage. - * - * @param gameboard - * to temporarily store. - * @return gameboard id - */ - public String temporarilyStoreGameboard(final GameboardDTO gameboard) { - this.gameboardNonPersistentStorage.put(gameboard.getId(), this.convertToGameboardDO(gameboard)); - - return gameboard.getId(); - } - /** * Save a gameboard to persistent storage. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 2e0ab3d0c7..4fc662ec5f 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -587,37 +587,6 @@ public final ResultsWrapper findByFieldNames( return finalResults; } - @Deprecated - public final ResultsWrapper findByFieldNamesRandomOrder( - final List fieldsToMatch, final Integer startIndex, - final Integer limit - ) throws ContentManagerException { - return this.findByFieldNamesRandomOrder(fieldsToMatch, startIndex, limit, null); - } - - @Deprecated - public ResultsWrapper findByFieldNamesRandomOrder( - final List fieldsToMatch, final Integer startIndex, - final Integer limit, @Nullable final Long randomSeed - ) throws ContentManagerException { - ResultsWrapper finalResults; - - ResultsWrapper searchHits; - searchHits = searchProvider.randomisedMatchSearch( - contentIndex, CONTENT_INDEX_TYPE.CONTENT.toString(), fieldsToMatch, startIndex, limit, - randomSeed, this.getBaseFilters()); - - // setup object mapper to use pre-configured deserializer module. - // Required to deal with type polymorphism - List result = contentSubclassMapper.mapFromStringListToContentList(searchHits.getResults()); - - List contentDTOResults = contentSubclassMapper.getDTOByDOList(result); - - finalResults = new ResultsWrapper<>(contentDTOResults, searchHits.getTotalResults()); - - return finalResults; - } - public final ByteArrayOutputStream getFileBytes(final String filename) throws IOException { return database.getFileByCommitSHA(getCurrentContentSHA(), filename); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java index 2ed31299e3..f5e5c98826 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java @@ -297,7 +297,7 @@ public static void setUpClass() throws Exception { IAssignmentPersistenceManager assignmentPersistenceManager = new PgAssignmentPersistenceManager(postgresSqlDb, mainMapper); GameboardPersistenceManager gameboardPersistenceManager = new GameboardPersistenceManager(postgresSqlDb, contentManager, mainMapper, contentMapper); - gameManager = new GameManager(contentManager, gameboardPersistenceManager, mainMapper, questionManager, properties); + gameManager = new GameManager(contentManager, gameboardPersistenceManager, questionManager, properties); groupManager = new GroupManager(pgUserGroupPersistenceManager, userAccountManager, gameManager, mainMapper); userAssociationManager = new UserAssociationManager(pgAssociationDataManager, userAccountManager, groupManager); PgTransactionManager pgTransactionManager = new PgTransactionManager(postgresSqlDb); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java deleted file mode 100644 index f46d63e483..0000000000 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2022 Matthew Trew - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package uk.ac.cam.cl.dtg.isaac.api.managers; - -import org.easymock.Capture; -import org.easymock.EasyMock; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import uk.ac.cam.cl.dtg.isaac.dao.GameboardPersistenceManager; -import uk.ac.cam.cl.dtg.isaac.dto.GameFilter; -import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; -import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.api.managers.QuestionManager; -import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; -import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; -import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager.BooleanSearchClause; -import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; -import uk.ac.cam.cl.dtg.util.YamlLoader; -import uk.ac.cam.cl.dtg.util.mappers.MainMapper; - -import java.util.Collections; -import java.util.List; -import java.util.Objects; - -import static org.easymock.EasyMock.replay; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static uk.ac.cam.cl.dtg.segue.api.Constants.*; - -public class GameManagerTest { - - private GitContentManager dummyContentManager; - private GameboardPersistenceManager dummyGameboardPersistenceManager; - private MainMapper dummyMapper; - private QuestionManager dummyQuestionManager; - private AbstractConfigLoader dummyConfigLoader; - - @BeforeEach - public void setUp() { - this.dummyContentManager = EasyMock.createMock(GitContentManager.class); - this.dummyGameboardPersistenceManager = EasyMock.createMock(GameboardPersistenceManager.class); - this.dummyMapper = EasyMock.createMock(MainMapper.class); - this.dummyQuestionManager = EasyMock.createMock(QuestionManager.class); - this.dummyConfigLoader = EasyMock.createMock(YamlLoader.class); - - EasyMock.expect(dummyConfigLoader.getProperty(GAMEBOARD_QUESTION_LIMIT)).andStubReturn("30"); - replay(dummyConfigLoader); - } - - @Test - public void getNextQuestionsForFilter_appliesExclusionFilterForDeprecatedQuestions() throws - ContentManagerException { - - // Arrange - GameManager gameManager = new GameManager( - this.dummyContentManager, - this.dummyGameboardPersistenceManager, - this.dummyMapper, - this.dummyQuestionManager, - this.dummyConfigLoader - ); - - // configure the mock GitContentManager to record the filters that are sent to it by getNextQuestionsForFilter() - Capture> capturedFilters = Capture.newInstance(); - EasyMock.expect(dummyContentManager.findByFieldNamesRandomOrder( - EasyMock.capture(capturedFilters), - EasyMock.anyInt(), - EasyMock.anyInt(), - EasyMock.anyLong()) - ).andStubReturn(new ResultsWrapper<>()); - replay(dummyContentManager); - - // Act - gameManager.getNextQuestionsForFilter(new GameFilter(), 1, 1L); - - // Assert - // check that one of the filters sent to GitContentManager was the deprecated question exclusion filter - List filters = capturedFilters.getValues().getFirst(); - BooleanSearchClause deprecatedFilter = filters.stream() - .filter(f -> Objects.equals(f.field(), "deprecated")).toList().getFirst(); - - assertNotNull(deprecatedFilter); - assertEquals(deprecatedFilter.operator(), Constants.BooleanOperator.NOT); - assertEquals(deprecatedFilter.values(), Collections.singletonList("true")); - } -} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java deleted file mode 100644 index 55d2439ee2..0000000000 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java +++ /dev/null @@ -1,125 +0,0 @@ -/* - * Copyright 2014 Stephen Cummins - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.isaac.app; - -import org.easymock.EasyMock; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import uk.ac.cam.cl.dtg.isaac.api.Constants; -import uk.ac.cam.cl.dtg.isaac.api.GameboardsFacade; -import uk.ac.cam.cl.dtg.isaac.api.managers.FastTrackManger; -import uk.ac.cam.cl.dtg.isaac.api.managers.GameManager; -import uk.ac.cam.cl.dtg.isaac.dto.users.AbstractSegueUserDTO; -import uk.ac.cam.cl.dtg.isaac.dto.users.AnonymousUserDTO; -import uk.ac.cam.cl.dtg.segue.api.managers.QuestionManager; -import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager; -import uk.ac.cam.cl.dtg.segue.dao.ILogManager; -import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; -import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; -import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.Status; -import java.util.List; - -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.verify; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static uk.ac.cam.cl.dtg.segue.api.Constants.*; - -/** - * Test class for the user manager class. - * - */ -public class GameboardsFacadeTest { - - private AbstractConfigLoader dummyPropertiesLoader = null; - private GameManager dummyGameManager = null; - private ILogManager dummyLogManager = null; - private UserAccountManager userManager; - private QuestionManager questionManager; - private FastTrackManger fastTrackManager; - - /** - * Initial configuration of tests. - * - * @throws Exception - * - test exception - */ - @BeforeEach - public final void setUp() throws Exception { - this.dummyPropertiesLoader = createMock(AbstractConfigLoader.class); - this.dummyGameManager = createMock(GameManager.class); - this.dummyLogManager = createMock(ILogManager.class); - this.userManager = createMock(UserAccountManager.class); - this.questionManager = createMock(QuestionManager.class); - this.fastTrackManager = createMock(FastTrackManger.class); - expect(this.dummyPropertiesLoader.getProperty(Constants.FASTTRACK_GAMEBOARD_WHITELIST)) - .andReturn("ft_board_1,ft_board_2").anyTimes(); - expect(dummyPropertiesLoader.getProperty(SEGUE_APP_ENVIRONMENT)) - .andReturn(EnvironmentType.DEV.name()).anyTimes(); - replay(this.dummyPropertiesLoader); - } - - /** - * Verify that when an empty gameboard is noticed a 204 is returned. - * - * @throws ContentManagerException - */ - @Test - public final void isaacEndPoint_checkEmptyGameboardCausesErrorNoUser_SegueErrorResponseShouldBeReturned() - throws SegueDatabaseException, ContentManagerException { - GameboardsFacade gameboardFacade = new GameboardsFacade( - dummyPropertiesLoader, dummyLogManager, dummyGameManager, questionManager, - userManager, fastTrackManager); - - HttpServletRequest dummyRequest = createMock(HttpServletRequest.class); - String subjects = "physics"; - String fields = "mechanics"; - String topics = "dynamics"; - String levels = "2,3,4"; - String concepts = "newtoni"; - String title = "Newton"; - String questionCategory = "problem_solving"; - String stages = "a_level"; - String difficulties = "practice_1"; - String examBoards = "wjec"; - - expect( - dummyGameManager.generateRandomGameboard( - EasyMock. anyObject(), EasyMock.> anyObject(), - EasyMock.> anyObject(), EasyMock.> anyObject(), - EasyMock.> anyObject(), EasyMock.> anyObject(), - EasyMock.> anyObject(), EasyMock.> anyObject(), - EasyMock.> anyObject(), EasyMock.> anyObject(), - EasyMock. anyObject())) - .andReturn(null).atLeastOnce(); - - expect(userManager.getCurrentUser(dummyRequest)).andReturn(new AnonymousUserDTO("testID")) - .atLeastOnce(); - - replay(dummyGameManager); - - Response r = gameboardFacade.generateTemporaryGameboard(dummyRequest, title, subjects, fields, topics, - stages, difficulties, examBoards,levels, concepts, questionCategory); - - assertTrue(r.getStatus() == Status.NO_CONTENT.getStatusCode()); - verify(dummyGameManager); - } -} From b78587237e82c1a60f0ff4a77d27e64c16c1e8b9 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 27 May 2026 15:57:39 +0100 Subject: [PATCH 07/40] Use BooleanInstruction approach for getting wildcards --- .../ac/cam/cl/dtg/isaac/api/managers/GameManager.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java index 55955cfbf4..c1aba87378 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java @@ -52,6 +52,8 @@ import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import jakarta.annotation.Nullable; @@ -624,16 +626,13 @@ public List>> gatherGamePro * - if we cannot access the content requested. */ public List getWildcards() throws NoWildcardException, ContentManagerException { - List fieldsToMap = Lists.newArrayList(); - - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, BooleanOperator.OR, Collections.singletonList(WILDCARD_TYPE))); + BooleanInstruction searchInstruction = new BooleanInstruction(); + searchInstruction.should(new MatchInstruction(TYPE_FIELDNAME, WILDCARD_TYPE)); Map sortInstructions = Maps.newHashMap(); sortInstructions.put(TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, SortOrder.ASC); - ResultsWrapper wildcardResults = this.contentManager.findByFieldNames( - fieldsToMap, 0, -1, sortInstructions); + ResultsWrapper wildcardResults = this.contentManager.nestedMatchSearch(searchInstruction, 0, -1, null, sortInstructions); if (wildcardResults.getTotalResults() == 0) { throw new NoWildcardException(); From 490872abdaa3b2a9f48133d003bc194092580cf9 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 29 May 2026 09:52:39 +0100 Subject: [PATCH 08/40] Use BooleanInstruction approach for getting subject pods --- .../uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index 0e4d355946..56d02fe382 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java @@ -59,6 +59,8 @@ import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import uk.ac.cam.cl.dtg.util.mappers.MainMapper; @@ -1159,17 +1161,17 @@ public final Response getPodList(@Context final Request request, } try { - Map> fieldsToMatch = Maps.newHashMap(); - fieldsToMatch.put(TYPE_FIELDNAME, List.of(POD_FRAGMENT_TYPE)); - fieldsToMatch.put(TAGS_FIELDNAME, List.of(subject)); + BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, POD_FRAGMENT_TYPE)); + searchInstruction.must(new MatchInstruction(TAGS_FIELDNAME, subject)); Map sortInstructions = new HashMap<>(); sortInstructions.put("id.raw", SortOrder.DESC); // Sort by ID (i.e. most recent; all pod ids start yyyymmdd) // We would ideally also sort by presence of 'featured' tag, tricky with current implementation - ResultsWrapper pods = api.findMatchingContent( - ContentService.generateDefaultFieldToMatch(fieldsToMatch), startIndex, MAX_PODS_TO_RETURN, - sortInstructions); + ResultsWrapper pods = this.contentManager.nestedMatchSearch( + searchInstruction, startIndex, MAX_PODS_TO_RETURN, null, sortInstructions); return Response.ok(pods).cacheControl(getCacheControl(NUMBER_SECONDS_IN_TEN_MINUTES, true)) .tag(etag) From 2d72a001a7ff836a8d5739615d9ffcec01ff43ef Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 29 May 2026 10:17:32 +0100 Subject: [PATCH 09/40] Use BooleanInstruction approach for finding events to delete PII for --- ...eEventAdditionalBookingInformationJob.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java b/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java index a196008b42..18524cb7f5 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java @@ -11,23 +11,20 @@ import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.configuration.SegueGuiceConfigurationModule; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; -import uk.ac.cam.cl.dtg.segue.search.AbstractFilterInstruction; -import uk.ac.cam.cl.dtg.segue.search.DateRangeFilterInstruction; -import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; +import uk.ac.cam.cl.dtg.segue.search.RangeInstruction; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Timestamp; import java.time.ZonedDateTime; -import java.util.Collections; import java.util.Date; -import java.util.List; import java.util.Map; import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; @@ -36,7 +33,6 @@ public class DeleteEventAdditionalBookingInformationJob implements Job { private static final Logger log = LoggerFactory.getLogger(DeleteEventAdditionalBookingInformationJob.class); - private final AbstractConfigLoader properties; private final PostgresSqlDb database; private final GitContentManager contentManager; @@ -46,7 +42,6 @@ public class DeleteEventAdditionalBookingInformationJob implements Job { */ public DeleteEventAdditionalBookingInformationJob() { Injector injector = SegueGuiceConfigurationModule.getGuiceInjector(); - properties = injector.getInstance(AbstractConfigLoader.class); contentManager = injector.getInstance(GitContentManager.class); database = injector.getInstance(PostgresSqlDb.class); @@ -57,20 +52,19 @@ public void execute(final JobExecutionContext context) throws JobExecutionExcept // Magic number Integer limit = 10000; Integer startIndex = 0; - Map> fieldsToMatch = Maps.newHashMap(); Map sortInstructions = Maps.newHashMap(); - Map filterInstructions = Maps.newHashMap(); - fieldsToMatch.put(TYPE_FIELDNAME, Collections.singletonList(EVENT_TYPE)); sortInstructions.put(DATE_FIELDNAME, SortOrder.DESC); - DateRangeFilterInstruction anyEventsToNow = new DateRangeFilterInstruction(null, new Date()); - filterInstructions.put(ENDDATE_FIELDNAME, anyEventsToNow); + + BooleanInstruction instruction = new BooleanInstruction(); + instruction.must(new MatchInstruction(TYPE_FIELDNAME, EVENT_TYPE)); + instruction.must(new RangeInstruction(ENDDATE_FIELDNAME).lessThanOrEqual(new Date().getTime())); + ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime thirtyDaysAgo = now.plusDays(-30); try { - ResultsWrapper findByFieldNames = this.contentManager.findByFieldNames( - ContentService.generateDefaultFieldToMatch(fieldsToMatch), - startIndex, limit, sortInstructions, filterInstructions); - for (ContentDTO contentResult : findByFieldNames.getResults()) { + ResultsWrapper results = this.contentManager.nestedMatchSearch( + instruction, startIndex, limit, null, sortInstructions); + for (ContentDTO contentResult : results.getResults()) { if (contentResult instanceof IsaacEventPageDTO page) { // Event end date (if present) > 30 days ago, else event date > 30 days ago boolean endDate30DaysAgo = page.getEndDate() != null && page.getEndDate().toInstant().isBefore(thirtyDaysAgo.toInstant()); From ea12d1a73d32c120aa873ef7aa46f3b5157321a7 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 29 May 2026 10:35:00 +0100 Subject: [PATCH 10/40] Use BooleanInstruction approach for finding events for notification emails --- .../EventNotificationEmailManager.java | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java index 357a213eac..d7bad6c3f5 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java @@ -16,20 +16,19 @@ import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryDTO; import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserException; import uk.ac.cam.cl.dtg.segue.comm.EmailManager; import uk.ac.cam.cl.dtg.segue.comm.EmailType; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; -import uk.ac.cam.cl.dtg.segue.search.AbstractFilterInstruction; -import uk.ac.cam.cl.dtg.segue.search.DateRangeFilterInstruction; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; +import uk.ac.cam.cl.dtg.segue.search.RangeInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import java.time.ZonedDateTime; import java.util.Arrays; -import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; @@ -97,22 +96,22 @@ public void sendReminderEmails() { // Magic number Integer limit = 10000; Integer startIndex = 0; - Map> fieldsToMatch = Maps.newHashMap(); Map sortInstructions = Maps.newHashMap(); - Map filterInstructions = Maps.newHashMap(); - fieldsToMatch.put(TYPE_FIELDNAME, Collections.singletonList(EVENT_TYPE)); sortInstructions.put(DATE_FIELDNAME, Constants.SortOrder.DESC); ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime threeDaysAhead = now.plusDays(3); - DateRangeFilterInstruction - eventsWithinThreeDays = new DateRangeFilterInstruction(new Date(), Date.from(threeDaysAhead.toInstant())); - filterInstructions.put(DATE_FIELDNAME, eventsWithinThreeDays); + + BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, EVENT_TYPE)); + searchInstruction.must(new RangeInstruction(DATE_FIELDNAME) + .greaterThanOrEqual(new Date().getTime()) + .lessThanOrEqual(Date.from(threeDaysAhead.toInstant()).getTime())); try { - ResultsWrapper findByFieldNames = this.contentManager.findByFieldNames( - ContentService.generateDefaultFieldToMatch(fieldsToMatch), startIndex, limit, sortInstructions, - filterInstructions); - for (ContentDTO contentResult : findByFieldNames.getResults()) { + ResultsWrapper results = this.contentManager.nestedMatchSearch( + searchInstruction, startIndex, limit, null, sortInstructions); + for (ContentDTO contentResult : results.getResults()) { if (contentResult instanceof IsaacEventPageDTO event) { // Skip sending emails for cancelled events if (EventStatus.CANCELLED.equals(event.getEventStatus())) { @@ -138,23 +137,22 @@ public void sendFeedbackEmails() { // Magic number Integer limit = 10000; Integer startIndex = 0; - Map> fieldsToMatch = Maps.newHashMap(); Map sortInstructions = Maps.newHashMap(); - Map filterInstructions = Maps.newHashMap(); - fieldsToMatch.put(TYPE_FIELDNAME, Collections.singletonList(EVENT_TYPE)); sortInstructions.put(DATE_FIELDNAME, Constants.SortOrder.DESC); ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime sixtyDaysAgo = now.plusDays(-60); - DateRangeFilterInstruction eventsInLastSixtyDays = new DateRangeFilterInstruction( - Date.from(sixtyDaysAgo.toInstant()), new Date()); - filterInstructions.put(DATE_FIELDNAME, eventsInLastSixtyDays); + BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, EVENT_TYPE)); + searchInstruction.must(new RangeInstruction(DATE_FIELDNAME) + .greaterThanOrEqual(Date.from(sixtyDaysAgo.toInstant()).getTime()) + .lessThanOrEqual(new Date().getTime())); try { - ResultsWrapper findByFieldNames = this.contentManager.findByFieldNames( - ContentService.generateDefaultFieldToMatch(fieldsToMatch), startIndex, limit, sortInstructions, - filterInstructions); - for (ContentDTO contentResult : findByFieldNames.getResults()) { + ResultsWrapper results = this.contentManager.nestedMatchSearch( + searchInstruction, startIndex, limit, null, sortInstructions); + for (ContentDTO contentResult : results.getResults()) { if (contentResult instanceof IsaacEventPageDTO event) { // Skip sending emails for cancelled events if (EventStatus.CANCELLED.equals(event.getEventStatus())) { From 7deb79d2f79710c5ecf7cf05e1bde1f19d85646f Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 29 May 2026 14:12:38 +0100 Subject: [PATCH 11/40] Use BooleanInstruction approach for listing content objects --- .../ac/cam/cl/dtg/isaac/api/PagesFacade.java | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index 56d02fe382..c3d3f10b21 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java @@ -182,31 +182,25 @@ public final Response getConceptList(@Context final Request request, @QueryParam @QueryParam("tags") final String tags, @DefaultValue(DEFAULT_START_INDEX_AS_STRING) @QueryParam("start_index") final Integer startIndex, @DefaultValue(DEFAULT_RESULTS_LIMIT_AS_STRING) @QueryParam("limit") final Integer limit) { - Map> fieldsToMatch = Maps.newHashMap(); - fieldsToMatch.put(TYPE_FIELDNAME, List.of(CONCEPT_TYPE)); - StringBuilder etagCodeBuilder = new StringBuilder(); - - Integer newLimit = null; + Integer newLimit = limit; if (limit != null) { - newLimit = limit; etagCodeBuilder.append(limit); } - // options + List idsList = null; if (ids != null) { - List idsList = Arrays.asList(ids.split(",")); - fieldsToMatch.put(ID_FIELDNAME, idsList); + idsList = Arrays.asList(ids.split(",")); newLimit = idsList.size(); etagCodeBuilder.append(ids); } + List tagList = null; if (tags != null) { - fieldsToMatch.put(TAGS_FIELDNAME, Arrays.asList(tags.split(","))); + tagList = Arrays.asList(tags.split(",")); etagCodeBuilder.append(tags); } - Map booleanOperatorOverrideMap = ImmutableMap.of(TAGS_FIELDNAME, BooleanOperator.OR); // Calculate the ETag on last modified date of tags list // NOTE: Assumes that the latest version of the content is being used. @@ -220,10 +214,37 @@ public final Response getConceptList(@Context final Request request, @QueryParam } try { - return listContentObjects(fieldsToMatch, booleanOperatorOverrideMap, startIndex, newLimit).tag(etag) + BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); + searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, CONCEPT_TYPE)); + + if (idsList != null && !idsList.isEmpty()) { + BooleanInstruction idsInstruction = new BooleanInstruction(); + for (String id : idsList) { + idsInstruction.should(new MatchInstruction(ID_FIELDNAME, id)); + } + searchInstruction.must(idsInstruction); + } + + if (tagList != null && !tagList.isEmpty()) { + BooleanInstruction tagsInstruction = new BooleanInstruction(); + for (String tag : tagList) { + tagsInstruction.should(new MatchInstruction(TAGS_FIELDNAME, tag)); + } + searchInstruction.must(tagsInstruction); + } + + ResultsWrapper c = this.contentManager.nestedMatchSearch( + searchInstruction, startIndex, newLimit, null, null); + + ResultsWrapper summarizedContent = new ResultsWrapper<>( + this.extractContentSummaryFromList(c.getResults()), + c.getTotalResults()); + + return Response.ok(summarizedContent).tag(etag) .cacheControl(getCacheControl(NUMBER_SECONDS_IN_ONE_HOUR, true)) .build(); - } catch (ContentManagerException e1) { + } catch (final ContentManagerException e1) { SegueErrorResponse error = new SegueErrorResponse(Status.NOT_FOUND, "Error locating the content requested", e1); log.error(error.getErrorMessage(), e1); From 6311da228d981a742dc7008d69619ed4a452cb49 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 29 May 2026 14:20:23 +0100 Subject: [PATCH 12/40] Remove ContentService No longer needed since all of its methods are superseded by AbstractInstruction-based equivalents --- .../ac/cam/cl/dtg/isaac/api/PagesFacade.java | 45 +----- .../dtg/isaac/api/managers/QuizManager.java | 6 +- .../segue/api/services/ContentService.java | 137 ------------------ .../api/AbstractIsaacIntegrationTest.java | 3 +- .../cam/cl/dtg/isaac/api/PagesFacadeIT.java | 3 +- .../isaac/api/managers/QuizManagerTest.java | 6 +- 6 files changed, 6 insertions(+), 194 deletions(-) delete mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/api/services/ContentService.java diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index c3d3f10b21..565f2424b9 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java @@ -53,7 +53,6 @@ import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; import uk.ac.cam.cl.dtg.segue.api.managers.QuestionManager; import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserLoggedInException; import uk.ac.cam.cl.dtg.segue.dao.ILogManager; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; @@ -104,7 +103,6 @@ public class PagesFacade extends AbstractIsaacFacade { private static final Logger log = LoggerFactory.getLogger(PagesFacade.class); - private final ContentService api; private final MainMapper mapper; private final UserAccountManager userManager; private final URIManager uriManager; @@ -117,8 +115,6 @@ public class PagesFacade extends AbstractIsaacFacade { /** * Creates an instance of the pages controller which provides the REST endpoints for accessing page content. * - * @param api - * - Instance of ContentService * @param propertiesLoader * - Instance of properties Loader * @param logManager @@ -139,13 +135,12 @@ public class PagesFacade extends AbstractIsaacFacade { * - For looking up bookmark information. */ @Inject - public PagesFacade(final ContentService api, final AbstractConfigLoader propertiesLoader, + public PagesFacade(final AbstractConfigLoader propertiesLoader, final ILogManager logManager, final MainMapper mapper, final GitContentManager contentManager, final UserAccountManager userManager, final URIManager uriManager, final QuestionManager questionManager, final GameManager gameManager, final UserAttemptManager userAttemptManager, final BookmarksManager bookmarksManager) { super(propertiesLoader, logManager); - this.api = api; this.mapper = mapper; this.contentManager = contentManager; this.userManager = userManager; @@ -1302,44 +1297,6 @@ private List extractContentSummaryFromList(final List list of queries to match - * @param booleanOperatorOverrideMap - * - an optional map of the form fieldname -> one of 'AND', 'OR' or 'NOT', to specify the - * type of matching needed for that field. Overrides any other default matching behaviour - * for the given fields - * @param startIndex - * - the initial index for the first result. - * @param limit - * - the maximums number of results to return - * @return Response builder containing a list of content summary objects or containing a SegueErrorResponse - */ - private Response.ResponseBuilder listContentObjects(final Map> fieldsToMatch, - @Nullable final Map booleanOperatorOverrideMap, - final Integer startIndex, - final Integer limit) - throws ContentManagerException { - ResultsWrapper c; - - c = api.findMatchingContent( - ContentService.generateDefaultFieldToMatch(fieldsToMatch, booleanOperatorOverrideMap), - startIndex, - limit - ); - - ResultsWrapper summarizedContent = new ResultsWrapper<>( - this.extractContentSummaryFromList(c.getResults()), - c.getTotalResults()); - - return Response.ok(summarizedContent); - } - /** * Convert an optional comma-separated URL parameter into a value suitable for logging. * @param urlParam - the URL parameter value, potentially null or empty. diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java index 2521de2dfc..be678e61d4 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java @@ -33,7 +33,6 @@ import uk.ac.cam.cl.dtg.isaac.dto.content.DetailedQuizSummaryDTO; import uk.ac.cam.cl.dtg.isaac.dto.content.QuizSummaryDTO; import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; @@ -66,16 +65,13 @@ public class QuizManager { * * @param properties * - global properties map - * @param contentService - * - so we can look up content * @param contentManager * - so we can fetch specific content. * @param contentSummarizerService * - so we can summarize content with links */ @Inject - public QuizManager(final AbstractConfigLoader properties, final ContentService contentService, - final GitContentManager contentManager, + public QuizManager(final AbstractConfigLoader properties, final GitContentManager contentManager, final ContentSummarizerService contentSummarizerService) { this.properties = properties; this.contentManager = contentManager; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/services/ContentService.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/services/ContentService.java deleted file mode 100644 index 67577eecfa..0000000000 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/services/ContentService.java +++ /dev/null @@ -1,137 +0,0 @@ -/* - * Copyright 2021 Raspberry Pi Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.api.services; - -import com.google.api.client.util.Lists; -import com.google.inject.Inject; -import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; -import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; -import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; -import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; - -import jakarta.annotation.Nullable; -import java.util.List; -import java.util.Map; - -import static uk.ac.cam.cl.dtg.segue.api.Constants.*; - -public class ContentService { - private final GitContentManager contentManager; - - @Inject - public ContentService(final GitContentManager contentManager) { - this.contentManager = contentManager; - } - - /** - * This method will return a ResultsWrapper based on the parameters supplied. - * - * @param fieldsToMatch - List of Boolean search clauses that must be true for the returned content. - * @param startIndex - the start index for the search results. - * @param limit - the max number of results to return. - * @return Response containing a ResultsWrapper or a Response containing null if none found. - */ - public final ResultsWrapper findMatchingContent( - final List fieldsToMatch, - @Nullable final Integer startIndex, @Nullable final Integer limit - ) throws ContentManagerException { - return findMatchingContent(fieldsToMatch, startIndex, limit, null); - } - - /** - * This method will return a ResultsWrapper based on the parameters supplied. - * - * @param fieldsToMatch - List of Boolean search clauses that must be true for the returned content. - * @param startIndex - the start index for the search results. - * @param limit - the max number of results to return. - * @param sortInstructions - Map of sorting functions to use in ElasticSearch query - * @return Response containing a ResultsWrapper or a Response containing null if none found. - */ - public final ResultsWrapper findMatchingContent( - final List fieldsToMatch, - @Nullable final Integer startIndex, @Nullable final Integer limit, - @Nullable Map sortInstructions - ) throws ContentManagerException { - - Integer newLimit = Constants.DEFAULT_RESULTS_LIMIT; - Integer newStartIndex = 0; - - if (limit != null) { - newLimit = limit; - } - if (startIndex != null) { - newStartIndex = startIndex; - } - - return this.contentManager.findByFieldNames(fieldsToMatch, newStartIndex, newLimit, sortInstructions); - } - - /** - * Helper method to generate field to match requirements for search queries. - * - * An overloaded version of the static method also exists which allows overloading default boolean operator values. - * - * @param fieldsToMatch - * - expects a map of the form fieldname -> list of queries to match - * @return A list of boolean search clauses to be passed to a content provider - */ - public static List generateDefaultFieldToMatch(final Map> fieldsToMatch) { - return ContentService.generateDefaultFieldToMatch(fieldsToMatch, null); - } - - /** - * Helper method to generate field to match requirements for search queries. - * - * Assumes whether to filter by 'any' or 'all' on a field by field basis, with the default being 'all'. - * You can pass an optional map specifying particular kinds of matching for specific fields - * - * @param fieldsToMatch - * - expects a map of the form fieldname -> list of queries to match - * @param booleanOperatorOverrideMap - * - an optional map of the form fieldname -> one of 'AND', 'OR' or 'NOT', to specify the - * type of matching needed for that field. Overrides any other default matching behaviour - * for the given fields - * @return A list of boolean search clauses to be passed to a content provider - */ - public static List generateDefaultFieldToMatch(final Map> fieldsToMatch, - @Nullable final Map booleanOperatorOverrideMap) { - List fieldsToMatchOutput = Lists.newArrayList(); - - for (Map.Entry> pair : fieldsToMatch.entrySet()) { - // First check if the field needs to be forced to a particular kind of matching - if (null != booleanOperatorOverrideMap && booleanOperatorOverrideMap.containsKey(pair.getKey())) { - fieldsToMatchOutput.add(new GitContentManager.BooleanSearchClause( - pair.getKey(), booleanOperatorOverrideMap.get(pair.getKey()), pair.getValue())); - } else if (pair.getKey().equals(ID_FIELDNAME)) { - fieldsToMatchOutput.add(new GitContentManager.BooleanSearchClause( - pair.getKey(), Constants.BooleanOperator.OR, pair.getValue())); - } else if (pair.getKey().equals(TYPE_FIELDNAME) && pair.getValue().size() > 1) { - // special case of when you want to allow more than one - fieldsToMatchOutput.add(new GitContentManager.BooleanSearchClause( - pair.getKey(), Constants.BooleanOperator.OR, pair.getValue())); - } else if (NESTED_QUERY_FIELDS.contains(pair.getKey())) { // these should match if either are true - fieldsToMatchOutput.add(new GitContentManager.BooleanSearchClause( - pair.getKey(), Constants.BooleanOperator.OR, pair.getValue())); - } else { - fieldsToMatchOutput.add(new GitContentManager.BooleanSearchClause( - pair.getKey(), Constants.BooleanOperator.AND, pair.getValue())); - } - } - - return fieldsToMatchOutput; - } -} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java index f5e5c98826..32ceafa650 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java @@ -61,7 +61,6 @@ import uk.ac.cam.cl.dtg.segue.api.monitors.RegistrationMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.TeacherPasswordResetMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.TokenOwnerLookupMisuseHandler; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.auth.AuthenticationProvider; import uk.ac.cam.cl.dtg.segue.auth.IAuthenticator; import uk.ac.cam.cl.dtg.segue.auth.ISecondFactorAuthenticator; @@ -306,7 +305,7 @@ public static void setUpClass() throws Exception { assignmentManager = new AssignmentManager(assignmentPersistenceManager, groupManager, new EmailService(properties, emailManager, groupManager, userAccountManager, mailGunEmailManager), gameManager, properties); schoolListReader = createNiceMock(SchoolListReader.class); - quizManager = new QuizManager(properties, new ContentService(contentManager), contentManager, new ContentSummarizerService(mainMapper, new URIManager(properties))); + quizManager = new QuizManager(properties, contentManager, new ContentSummarizerService(mainMapper, new URIManager(properties))); quizAssignmentPersistenceManager = new PgQuizAssignmentPersistenceManager(postgresSqlDb, mainMapper); quizAssignmentManager = new QuizAssignmentManager(quizAssignmentPersistenceManager, new EmailService(properties, emailManager, groupManager, userAccountManager, mailGunEmailManager), quizManager, groupManager, properties); assignmentService = new AssignmentService(userAccountManager); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java index 7d6875f30b..718016a9a7 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java @@ -21,7 +21,6 @@ import uk.ac.cam.cl.dtg.isaac.api.managers.URIManager; import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentSummaryDTO; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; @@ -41,7 +40,7 @@ public class PagesFacadeIT extends IsaacIntegrationTest{ @BeforeEach public void setUp() { - this.pagesFacade = new PagesFacade(new ContentService(contentManager), properties, logManager, + this.pagesFacade = new PagesFacade(properties, logManager, mainMapper, contentManager, userAccountManager, new URIManager(properties), questionManager, gameManager, userAttemptManager, bookmarksManager); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java index ae47e7b790..9b6b034f00 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java @@ -8,7 +8,6 @@ import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuizSectionDTO; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.api.services.ContentService; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; @@ -31,15 +30,14 @@ public class QuizManagerTest extends AbstractManagerTest { public void setUp() { properties = createMock(AbstractConfigLoader.class); - ContentService contentService = createMock(ContentService.class); GitContentManager contentManager = createMock(GitContentManager.class); ContentSummarizerService contentSummarizerService = createMock(ContentSummarizerService.class); - quizManager = new QuizManager(properties, contentService, contentManager, contentSummarizerService); + quizManager = new QuizManager(properties, contentManager, contentSummarizerService); brokenQuiz = new IsaacQuizDTO(); brokenQuiz.setChildren(ImmutableList.of(quizSection1, new ContentDTO(), quizSection2)); - replay(properties, contentService, contentManager, contentSummarizerService); + replay(properties, contentManager, contentSummarizerService); } @Test From 085ff84b961aef7622af0c858c8f646d89abf996 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 10:54:45 +0100 Subject: [PATCH 13/40] Use BooleanInstruction approach for finding invalid q ids in gameboards --- .../dao/GameboardPersistenceManager.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java index 2dd77c8552..3234480fdb 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java @@ -37,12 +37,13 @@ import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; -import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.util.mappers.MainMapper; import jakarta.annotation.Nullable; @@ -431,21 +432,28 @@ public List getGameboardsByUserId(final RegisteredUserDTO user) th public List getInvalidQuestionIdsFromGameboard(final GameboardDTO gameboardDTO) { GameboardDO gameboardDO = this.convertToGameboardDO(gameboardDTO); + List questionIds = gameboardDO.getContents().stream().map(GameboardContentDescriptor::getId).toList(); + // build query the db to get full question information - List fieldsToMap = Lists.newArrayList(); + BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - Constants.ID_FIELDNAME + '.' + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Constants.BooleanOperator.OR, - gameboardDO.getContents().stream().map(GameboardContentDescriptor::getId).collect(Collectors.toList()))); + BooleanInstruction idsInstruction = new BooleanInstruction(); + for (String questionId : questionIds) { + idsInstruction.should(new MatchInstruction(ID_FIELDNAME + '.' + UNPROCESSED_SEARCH_FIELD_SUFFIX, questionId)); + } + searchInstruction.must(idsInstruction); - fieldsToMap.add(new GitContentManager.BooleanSearchClause( - TYPE_FIELDNAME, Constants.BooleanOperator.OR, QUESTION_PAGE_TYPES)); + BooleanInstruction questionTypesInstruction = new BooleanInstruction(); + for (String questionPageType : QUESTION_PAGE_TYPES) { + questionTypesInstruction.should(new MatchInstruction(TYPE_FIELDNAME, questionPageType)); + } + searchInstruction.must(questionTypesInstruction); - // Search for questions that match the ids. + // Search for questions that match the ids. ResultsWrapper results; try { - results = this.contentManager.findByFieldNames( - fieldsToMap, 0, gameboardDO.getContents().size()); + results = this.contentManager.nestedMatchSearch(searchInstruction, 0, gameboardDO.getContents().size(), null, null); } catch (ContentManagerException e) { results = new ResultsWrapper<>(); log.error("Unable to select questions for gameboard.", e); From 83745fec3bd25d28f4b5e82c598c6aaf92dc16d6 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 11:03:03 +0100 Subject: [PATCH 14/40] Use BooleanInstruction approach for populating related content --- .../segue/dao/content/GitContentManager.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 4fc662ec5f..d77ca6afdf 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -47,6 +47,7 @@ import uk.ac.cam.cl.dtg.segue.search.IsaacSearchInstructionBuilder; import uk.ac.cam.cl.dtg.segue.search.IsaacSearchInstructionBuilder.Priority; import uk.ac.cam.cl.dtg.segue.search.IsaacSearchInstructionBuilder.Strategy; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.segue.search.SearchInField; import uk.ac.cam.cl.dtg.segue.search.SegueSearchException; import uk.ac.cam.cl.dtg.segue.search.SimpleExclusionInstruction; @@ -692,19 +693,23 @@ public ContentDTO populateRelatedContent(final ContentDTO contentDTO) return contentDTO; } - // build query the db to get full content information - List fieldsToMap = Lists.newArrayList(); - List relatedContentIds = Lists.newArrayList(); for (ContentSummaryDTO summary : contentDTO.getRelatedContent()) { relatedContentIds.add(summary.getId()); } - fieldsToMap.add(new BooleanSearchClause( - Constants.ID_FIELDNAME + '.' + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, - Constants.BooleanOperator.OR, relatedContentIds)); + // build query the db to get full content information + BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); + + BooleanInstruction idsInstruction = new BooleanInstruction(); + for (String relatedContentId : relatedContentIds) { + idsInstruction.should(new MatchInstruction( + Constants.ID_FIELDNAME + '.' + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, relatedContentId)); + } + searchInstruction.must(idsInstruction); - ResultsWrapper results = this.findByFieldNames(fieldsToMap, 0, relatedContentIds.size()); + ResultsWrapper results = this.nestedMatchSearch(searchInstruction, 0, relatedContentIds.size(), null, null); List relatedContentDTOs = Lists.newArrayList(); From c5f480c89370e275221e2199c8cdf3ab02388bf0 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 11:06:19 +0100 Subject: [PATCH 15/40] Remove deprecated find by field name methods --- .../segue/dao/content/GitContentManager.java | 56 ------------------- 1 file changed, 56 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index d77ca6afdf..3cd6d90f32 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -532,62 +532,6 @@ public final ResultsWrapper questionSearch( return new ResultsWrapper<>(contentSubclassMapper.getDTOByDOList(searchResults), searchHits.getTotalResults()); } - @Deprecated - public final ResultsWrapper findByFieldNames( - final List fieldsToMatch, final Integer startIndex, final Integer limit - ) throws ContentManagerException { - return this.findByFieldNames(fieldsToMatch, startIndex, limit, null); - } - - @Deprecated - public final ResultsWrapper findByFieldNames( - final List fieldsToMatch, final Integer startIndex, - final Integer limit, @Nullable final Map sortInstructions - ) throws ContentManagerException { - return this.findByFieldNames(fieldsToMatch, startIndex, limit, sortInstructions, null); - } - - @Deprecated - public final ResultsWrapper findByFieldNames( - final List fieldsToMatch, final Integer startIndex, final Integer limit, - @Nullable final Map sortInstructions, - @Nullable final Map filterInstructions - ) throws ContentManagerException { - ResultsWrapper finalResults; - - final Map newSortInstructions; - if (null == sortInstructions || sortInstructions.isEmpty()) { - newSortInstructions = Maps.newHashMap(); - newSortInstructions.put(Constants.TITLE_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, - Constants.SortOrder.ASC); - } else { - newSortInstructions = sortInstructions; - } - - // add base filters to filter instructions - Map newFilterInstructions = filterInstructions; - if (this.getBaseFilters() != null) { - if (null == newFilterInstructions) { - newFilterInstructions = Maps.newHashMap(); - } - newFilterInstructions.putAll(this.getBaseFilters()); - } - - ResultsWrapper searchHits = searchProvider.matchSearch(contentIndex, - CONTENT_INDEX_TYPE.CONTENT.toString(), fieldsToMatch, startIndex, limit, - newSortInstructions, newFilterInstructions); - - // setup object mapper to use pre-configured deserializer module. - // Required to deal with type polymorphism - List result = contentSubclassMapper.mapFromStringListToContentList(searchHits.getResults()); - - List contentDTOResults = contentSubclassMapper.getDTOByDOList(result); - - finalResults = new ResultsWrapper<>(contentDTOResults, searchHits.getTotalResults()); - - return finalResults; - } - public final ByteArrayOutputStream getFileBytes(final String filename) throws IOException { return database.getFileByCommitSHA(getCurrentContentSHA(), filename); } From 53e41d096b174c2758d8555ad74e9401fc92c248 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 11:29:45 +0100 Subject: [PATCH 16/40] Use BooleanInstruction approach for school lookup --- .../segue/dao/schools/SchoolListReader.java | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java index 4b09db0ae1..649867d912 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java @@ -25,13 +25,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import uk.ac.cam.cl.dtg.isaac.dos.users.School; +import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; import uk.ac.cam.cl.dtg.segue.search.ISearchProvider; +import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.segue.search.SegueSearchException; import jakarta.annotation.Nullable; import java.io.IOException; import java.util.List; -import java.util.Map; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; @@ -93,10 +94,19 @@ public List findSchoolByNameOrPostCode(final String searchQuery, @Nullab Integer queryLimit = limit == null ? DEFAULT_RESULTS_LIMIT : limit; - List schoolSearchResults = searchProvider.fuzzySearch(SCHOOLS_INDEX_BASE, SCHOOLS_INDEX_TYPE.SCHOOL_SEARCH.toString(), - searchQuery, 0, queryLimit, Map.of(SCHOOL_CLOSED_FIELDNAME_POJO, List.of("false")), null, SCHOOL_URN_FIELDNAME_POJO, - SCHOOL_ESTABLISHMENT_NAME_FIELDNAME_POJO, SCHOOL_POSTCODE_FIELDNAME_POJO) - .getResults(); + BooleanInstruction schoolSearchInstruction = new BooleanInstruction(); + schoolSearchInstruction.must(new MatchInstruction(SCHOOL_CLOSED_FIELDNAME_POJO, "false")); + + // At least one of these fields must match, with some fuzziness + BooleanInstruction textSearchInstruction = new BooleanInstruction(); + textSearchInstruction.should(new MatchInstruction(SCHOOL_URN_FIELDNAME_POJO, searchQuery, 2L, true)); + textSearchInstruction.should(new MatchInstruction(SCHOOL_ESTABLISHMENT_NAME_FIELDNAME_POJO, searchQuery, 2L, true)); + textSearchInstruction.should(new MatchInstruction(SCHOOL_POSTCODE_FIELDNAME_POJO, searchQuery, 2L, true)); + schoolSearchInstruction.must(textSearchInstruction); + + List schoolSearchResults = searchProvider.nestedMatchSearch(SCHOOLS_INDEX_BASE, + SCHOOLS_INDEX_TYPE.SCHOOL_SEARCH.toString(), 0, queryLimit, schoolSearchInstruction, null, null + ).getResults(); List resultList = Lists.newArrayList(); for (String schoolString : schoolSearchResults) { From a6dd00bd4732e03c3f13dde53e51a95fb09ca257 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 11:33:30 +0100 Subject: [PATCH 17/40] Remove deprecated fuzzySearch and convertToBoolMap helper method --- .../segue/search/ElasticSearchProvider.java | 98 ------------------- .../cl/dtg/segue/search/ISearchProvider.java | 32 ------ 2 files changed, 130 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java index 41dfc70dda..aca1884de0 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java @@ -217,76 +217,6 @@ public ResultsWrapper nestedMatchSearch(final String indexBase, final St return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit, sortOrder); } - @Override - @Deprecated - public ResultsWrapper fuzzySearch(final String indexBase, final String indexType, final String searchString, - final Integer startIndex, final Integer limit, - @Nullable final Map> fieldsThatMustMatch, - @Nullable final Map filterInstructions, - final String... fields) throws SegueSearchException { - if (null == indexBase || null == indexType || null == searchString || null == fields) { - log.warn("A required field is missing. Unable to execute search."); - return null; - } - - BoolQuery.Builder masterQuery; - if (null != fieldsThatMustMatch) { - masterQuery = new BoolQuery.Builder().must(this.generateBoolMatchQuery(this.convertToBoolMap(fieldsThatMustMatch))._toQuery()); - } else { - masterQuery = new BoolQuery.Builder(); - } - - BoolQuery.Builder query = new BoolQuery.Builder(); - Set boostFields = ImmutableSet.of("id", "title", "tags"); - - List searchTerms = Lists.newArrayList(); - searchTerms.addAll(Arrays.asList(searchString.split(" "))); - if (searchTerms.size() > 1) { - searchTerms.add(searchString); - } - - for (String f : fields) { - float boost = boostFields.contains(f) ? 2f : 1f; - - for (String searchTerm : searchTerms) { - Query initialFuzzySearch = MatchQuery.of(m -> m - .field(f) - .query(searchTerm) - .fuzziness("AUTO") - .prefixLength(0) - .boost(boost) - )._toQuery(); - query.should(initialFuzzySearch); - - Query regexSearch = WildcardQuery.of(r -> r - .field(f) - .value("*" + searchTerm + "*") - .boost(boost) - )._toQuery(); - query.should(regexSearch); - } - } - - // this query is just a bit smarter than the regex search above. - Query multiMatchPrefixQuery = MultiMatchQuery.of(mm -> mm - .query(searchString) - .fields(Arrays.asList(fields)) - .type(TextQueryType.PhrasePrefix) - .prefixLength(2) - .boost(2.0f) - )._toQuery(); - query.should(multiMatchPrefixQuery); - - masterQuery.must(query.build()._toQuery()); - - if (filterInstructions != null) { - masterQuery.filter(generateFilterQuery(filterInstructions)); - } - - Query finalQuery = masterQuery.build()._toQuery(); - return this.executeBasicQuery(indexBase, indexType, finalQuery, startIndex, limit); - } - @Override public ResultsWrapper termSearch(final String indexBase, final String indexType, final String searchTerm, final String field, final int startIndex, final int limit, @@ -724,34 +654,6 @@ private ResultsWrapper executeQuery(final SearchRequest searchRequest) t } } - - /** - * Utility function to support conversion between simple field maps and bool maps. - * - * @param fieldsThatMustMatch - * - the map that should be converted into a suitable map for querying. - * @return Map where each field is using the OR boolean operator. - * - * @deprecated as {@code AbstractInstruction}-based instructions should be preferred over - * {@code fieldsToMatch}-style instructions. - */ - @Deprecated - private List convertToBoolMap(final Map> fieldsThatMustMatch) { - if (null == fieldsThatMustMatch) { - return null; - } - - List result = Lists.newArrayList(); - - for (Map.Entry> pair : fieldsThatMustMatch.entrySet()) { - result.add(new GitContentManager.BooleanSearchClause( - pair.getKey(), Constants.BooleanOperator.OR, pair.getValue())); - } - - return result; - } - - /** * Based on the relatively abstract {@code matchInstruction}, generates a {@code Query} which is usable by * Elasticsearch. diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java index 68c13a9d28..14c587c051 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java @@ -77,38 +77,6 @@ ResultsWrapper matchSearch( @Nullable final Map filterInstructions ) throws SegueSearchException; - /** - * Executes a fuzzy search on an array of fields and will consider the fieldsThatMustMatchMap. - * - * This method should prioritise exact prefix matches and then fill it with fuzzy ones. - * - * @param indexBase - * - the base string for the name of the index - * @param indexType - * - the name of the type of document being searched for - * @param searchString - * - the string to use for fuzzy matching - * @param startIndex - * - e.g. 0 for the first set of results - * @param limit - * - the maximum number of results to return -1 will attempt to return all results. - * @param fieldsThatMustMatch - * - Map of Must match field -> value - * @param filterInstructions - * - post search filter instructions e.g. remove content of a certain type. - * @param fields - * - array (var args) of fields to search using the searchString - * @return results - * @deprecated in favour of {@code BooleanInstruction}-based searches. - */ - @Deprecated - ResultsWrapper fuzzySearch( - final String indexBase, final String indexType, final String searchString, - final Integer startIndex, final Integer limit, final Map> fieldsThatMustMatch, - @Nullable final Map filterInstructions, - final String... fields - ) throws SegueSearchException; - ResultsWrapper nestedMatchSearch( final String indexBase, final String indexType, final Integer startIndex, final Integer limit, @NotNull final BooleanInstruction matchInstruction, @Nullable Long randomSeed, From 49a294270df0a7d575e86c94fb82662274ce3c86 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 12:34:53 +0100 Subject: [PATCH 18/40] Remove findByExactMatch Deprecated; AbstractFilterInstruction-based searching should use AbstractInstructions instead --- .../segue/dao/schools/SchoolListReader.java | 11 ++++---- .../segue/search/ElasticSearchProvider.java | 26 ------------------- .../cl/dtg/segue/search/ISearchProvider.java | 24 ----------------- 3 files changed, 6 insertions(+), 55 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java index 649867d912..045fce7461 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java @@ -144,11 +144,12 @@ public School findSchoolById(final String schoolURN) throws UnableToIndexSchools throw new UnableToIndexSchoolsException("unable to ensure the cache has been populated"); } - List matchingSchoolList; - - matchingSchoolList = searchProvider.findByExactMatch(SCHOOLS_INDEX_BASE, SCHOOLS_INDEX_TYPE.SCHOOL_SEARCH.toString(), - SCHOOL_URN_FIELDNAME.toLowerCase() + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, - schoolURN, 0, DEFAULT_RESULTS_LIMIT, null).getResults(); + BooleanInstruction searchInstruction = new BooleanInstruction(); + searchInstruction.must(new MatchInstruction(SCHOOL_URN_FIELDNAME.toLowerCase() + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, schoolURN)); + + List matchingSchoolList = searchProvider.nestedMatchSearch(SCHOOLS_INDEX_BASE, + SCHOOLS_INDEX_TYPE.SCHOOL_SEARCH.toString(), 0, DEFAULT_RESULTS_LIMIT, searchInstruction, null, null + ).getResults(); if (matchingSchoolList.isEmpty()) { return null; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java index aca1884de0..6a70c8aada 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java @@ -301,32 +301,6 @@ public Collection getAllIndices() { } } - @Override - public ResultsWrapper findByExactMatch(final String indexBase, final String indexType, - final String fieldname, final String needle, final int startIndex, - final int limit, - final Map filterInstructions) - throws SegueSearchException { - ResultsWrapper resultList; - - Query query = MatchQuery.of(m -> m - .field(fieldname) - .query(needle) - )._toQuery(); - - if (filterInstructions != null) { - Query matchQuery = query; - query = BoolQuery.of(bq -> bq - .must(matchQuery) - .filter(generateFilterQuery(filterInstructions)) - )._toQuery(); - } - - resultList = this.executeBasicQuery(indexBase, indexType, query, startIndex, limit); - - return resultList; - } - @Override public ResultsWrapper findByPrefix(final String indexBase, final String indexType, final String fieldname, final String prefix, final int startIndex, final int limit, diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java index 14c587c051..3f7d9d0bce 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java @@ -135,30 +135,6 @@ ResultsWrapper randomisedMatchSearch( int startIndex, int limit, Long randomSeed, Map filterInstructions ) throws SegueSearchException; - /** - * Query for a list of Results that exactly match a given id. - * - * @param indexBase - * base string for the index that the content is stored in - * @param indexType - * - type of index as registered with search provider. - * @param fieldname - * - fieldName to search within. - * @param needle - * - needle to search for. - * @param startIndex - * - start index for results - * @param limit - * - the maximum number of results to return -1 will attempt to return all results. - * @param filterInstructions - * - post search filter instructions e.g. remove content of a certain type. - * @return A list of results that match the id prefix. - */ - ResultsWrapper findByExactMatch( - String indexBase, String indexType, String fieldname, String needle, int startIndex, int limit, - @Nullable Map filterInstructions - ) throws SegueSearchException; - /** * Query for a list of Results that match a given id prefix. * From 32351da69464d5a31289d286056f2d5072df1a52 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 14:09:51 +0100 Subject: [PATCH 19/40] Remove term search Deprecated; AbstractFilterInstruction-based searching should use AbstractInstructions instead --- .../segue/dao/content/GitContentManager.java | 37 ++++++++----------- .../segue/search/ElasticSearchProvider.java | 28 -------------- .../cl/dtg/segue/search/ISearchProvider.java | 27 -------------- 3 files changed, 15 insertions(+), 77 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 3cd6d90f32..1c015a0731 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -52,7 +52,6 @@ import uk.ac.cam.cl.dtg.segue.search.SegueSearchException; import uk.ac.cam.cl.dtg.segue.search.SimpleExclusionInstruction; import uk.ac.cam.cl.dtg.segue.search.SimpleFilterInstruction; -import uk.ac.cam.cl.dtg.segue.search.TermsFilterInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import uk.ac.cam.cl.dtg.util.mappers.ContentMapper; @@ -253,12 +252,13 @@ public Content getContentDOById(final String id, final boolean failQuietly) thro try { ResultsWrapper result = contentDOcache.get(k, () -> { - ResultsWrapper rawResults = searchProvider.termSearch( - contentIndex, - CONTENT_INDEX_TYPE.CONTENT.toString(), - id, - Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, 0, 1, - getBaseFilters()); + BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); + searchInstruction.must(new MatchInstruction( + Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, id)); + + ResultsWrapper rawResults = searchProvider.nestedMatchSearch(contentIndex, + CONTENT_INDEX_TYPE.CONTENT.toString(), 0, 1, searchInstruction, null, null); List searchResults = contentSubclassMapper .mapFromStringListToContentList(rawResults.getResults()); @@ -301,24 +301,17 @@ public ResultsWrapper getUnsafeCachedContentDTOsMatchingIds(final Co try { return contentDTOcache.get(k, () -> { - Map finalFilter = Maps.newHashMap(); - finalFilter.putAll(new ImmutableMap.Builder() - .put(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, - new TermsFilterInstruction(ids)) - .build()); + BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder() + .buildBaseInstructions(new BooleanInstruction()); - if (getBaseFilters() != null) { - finalFilter.putAll(getBaseFilters()); + BooleanInstruction idsInstruction = new BooleanInstruction(1); + for (String id : ids) { + idsInstruction.should(new MatchInstruction(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, id)); } + searchInstruction.must(idsInstruction); - ResultsWrapper searchHits = this.searchProvider.termSearch( - contentIndex, - CONTENT_INDEX_TYPE.CONTENT.toString(), - null, - null, - startIndex, - limit, - finalFilter + ResultsWrapper searchHits = this.searchProvider.nestedMatchSearch(contentIndex, + CONTENT_INDEX_TYPE.CONTENT.toString(), startIndex, limit, searchInstruction, null, null ); List searchResults = contentSubclassMapper.mapFromStringListToContentList(searchHits.getResults()); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java index 6a70c8aada..7c439beba3 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java @@ -217,34 +217,6 @@ public ResultsWrapper nestedMatchSearch(final String indexBase, final St return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit, sortOrder); } - @Override - public ResultsWrapper termSearch(final String indexBase, final String indexType, - final String searchTerm, final String field, final int startIndex, final int limit, - @Nullable final Map filterInstructions) - throws SegueSearchException { - if (null == indexBase || null == indexType || (null == searchTerm && null != field)) { - log.error("A required field or field combination is missing. Unable to execute search."); - return null; - } - - Query query = BoolQuery.of(bq -> { - if (searchTerm != null) { - Query termsQuery = TermsQuery.of(t -> t.field(field).terms(ts -> ts.value(List.of(FieldValue.of(searchTerm)))))._toQuery(); - bq.must(termsQuery); - } - if (filterInstructions != null) { - bq.filter(generateFilterQuery(filterInstructions)); - } - return bq; - })._toQuery(); - - if (null == searchTerm && null == filterInstructions) { - throw new SegueSearchException("This method requires either searchTerm or filter instructions."); - } - - return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit); - } - /** * This method will create a threadsafe client that can be used to talk to an Elastic Search cluster. * @param address diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java index 3f7d9d0bce..3755400dc6 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java @@ -83,33 +83,6 @@ ResultsWrapper nestedMatchSearch( @Nullable final Map sortOrder ) throws SegueSearchException; - /** - * Executes a terms search using an array of terms on a single field. - * - * Useful for tag searches - Current setting is that results will only be returned if they match all search terms. - * - * note: null searches are allowed providing a filter is specified. - * - * @param indexBase - * - the base string for the name of the index - * @param indexType - * - the name of the type of document being searched for - * @param searchterms - * - e.g. tags can be null - * @param field - * - to match against - cannot be null if searchterm is not null. - * @param startIndex - * - start index for results - * @param limit - * - the maximum number of results to return -1 will attempt to return all results. - * @param filterInstructions - instructions for filtering the results - * @return results - */ - ResultsWrapper termSearch( - final String indexBase, final String indexType, final String searchterms, final String field, - final int startIndex, final int limit, final Map filterInstructions - ) throws SegueSearchException; - /** * RandomisedPaginatedMatchSearch The same as paginatedMatchSearch but the results are returned in a random order. * From 345c52da0bd8cf749e2e6e120cb640d7d6e6dcf1 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 14:11:51 +0100 Subject: [PATCH 20/40] Remove duplicated base filters logic This is superseded by buildBaseInstructions, which is easier to work with for AbstractInstructions (as opposed to AbstractFilterInstructions). --- .../segue/dao/content/GitContentManager.java | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 1c015a0731..ae618cc4c7 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -747,27 +747,6 @@ public ResultsWrapper nestedMatchSearch(final BooleanInstruction ins return new ResultsWrapper<>(dtoResults, searchHits.getTotalResults()); } - /** - * Returns the basic filter configuration. - * - * @return either null or a map setup with filter/exclusion instructions, based on environment properties. - */ - private Map getBaseFilters() { - if (!this.hideRegressionTestContent && !this.showOnlyPublishedContent) { - return null; - } - - HashMap filters = new HashMap<>(); - - if (this.hideRegressionTestContent) { - filters.put("tags", new SimpleExclusionInstruction(REGRESSION_TEST_TAG)); - } - if (this.showOnlyPublishedContent) { - filters.put("published", new SimpleFilterInstruction("true")); - } - return ImmutableMap.copyOf(filters); - } - /** * A method which adds information to the contentSummaryDTO, summary, from values evaluated from the content. * @param content the original content object which was used to create the summary. From 1ecfdfb8c84c811b2f13ce2945adbd0c6d52cb69 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 14:23:51 +0100 Subject: [PATCH 21/40] Remove deprecated Elasticsearch query builders & helpers Also remove the tests for the removed methods. These all used AbstractFilterInstructions. Going forward, AbstractInstruction-based searching should be used instead. --- .../segue/search/ElasticSearchProvider.java | 265 ------------------ .../cl/dtg/segue/search/ISearchProvider.java | 102 ------- .../search/ElasticSearchProviderTest.java | 124 +------- 3 files changed, 4 insertions(+), 487 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java index 7c439beba3..e4cac06313 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java @@ -125,66 +125,6 @@ public String getNestedFieldConnector() { return ES_FIELD_CONNECTOR; } - @Override - public ResultsWrapper matchSearch(final String indexBase, final String indexType, - final List fieldsToMatch, final int startIndex, - final int limit, final Map sortInstructions, - @Nullable final Map filterInstructions) throws SegueSearchException { - Query query = generateBoolMatchQuery(fieldsToMatch)._toQuery(); - - if (filterInstructions != null) { - Query finalQuery = query; - query = BoolQuery.of(bq -> bq - .must(finalQuery) - .filter(generateFilterQuery(filterInstructions)) - )._toQuery(); - } - - return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit, sortInstructions); - } - - @Override - public final ResultsWrapper randomisedMatchSearch(final String indexBase, final String indexType, - final List fieldsToMatch, final int startIndex, final int limit, - final Long randomSeed, final Map filterInstructions) - throws SegueSearchException { - // build up the query from the fieldsToMatch map - Query query = ConstantScoreQuery.of(cs -> cs - .filter(generateBoolMatchQuery(fieldsToMatch)._toQuery()) - )._toQuery(); - - RandomScoreFunction randomScoreFunction; - - if (null != randomSeed) { - randomScoreFunction = RandomScoreFunction.of(r -> r - .seed(String.valueOf(randomSeed)) - .field("_seq_no") - ); - } else { - randomScoreFunction = RandomScoreFunction.of(r -> r); - } - - Query constantScoreQuery = query; - query = FunctionScoreQuery.of(fsq -> fsq - .query(constantScoreQuery) - .functions(fn -> fn - .randomScore(randomScoreFunction) - ) - )._toQuery(); - - if (filterInstructions != null) { - Query functionScoreQuery = query; - query = BoolQuery.of(bq -> bq - .must(functionScoreQuery) - .filter(generateFilterQuery(filterInstructions)) - )._toQuery(); - } - - log.debug("Randomised Query, to be sent to elasticsearch is : {}", query); - - return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit); - } - @Override public ResultsWrapper nestedMatchSearch(final String indexBase, final String indexType, final Integer startIndex, final Integer limit, @@ -273,56 +213,6 @@ public Collection getAllIndices() { } } - @Override - public ResultsWrapper findByPrefix(final String indexBase, final String indexType, final String fieldname, - final String prefix, final int startIndex, final int limit, - final Map filterInstructions) - throws SegueSearchException { - ResultsWrapper resultList; - - Query query = WildcardQuery.of(w -> w - .field(fieldname) - .value(prefix + "*") - )._toQuery(); - - if (filterInstructions != null) { - Query prefixQuery = query; - query = BoolQuery.of(bq -> bq - .must(prefixQuery) - .filter(generateFilterQuery(filterInstructions)) - )._toQuery(); - } - - resultList = this.executeBasicQuery(indexBase, indexType, query, startIndex, limit); - - return resultList; - } - - @Override - public ResultsWrapper findByRegEx(final String indexBase, final String indexType, final String fieldname, - final String regex, final int startIndex, final int limit, - final Map filterInstructions) - throws SegueSearchException { - ResultsWrapper resultList; - - Query query = RegexpQuery.of(r -> r - .field(fieldname) - .value(regex) - )._toQuery(); - - if (filterInstructions != null) { - Query regExpQuery = query; - query = BoolQuery.of(bq -> bq - .must(regExpQuery) - .filter(generateFilterQuery(filterInstructions)) - )._toQuery(); - } - - resultList = this.executeBasicQuery(indexBase, indexType, query, startIndex, limit); - - return resultList; - } - /** * Utility method to convert sort instructions from external classes into something Elastic search can use. * @@ -351,161 +241,6 @@ private void addSortInstructions(final SearchRequest.Builder requestBuilder, } } - /** - * Helper method to create elastic search understandable filter instructions. - * - * @param filterInstructions - * - in the form "fieldName --> instruction key --> instruction value" - * @return filter query - */ - public Query generateFilterQuery(final Map filterInstructions) { - BoolQuery.Builder filter = new BoolQuery.Builder(); - for (Entry fieldToFilterInstruction : filterInstructions.entrySet()) { - // date filter logic - if (fieldToFilterInstruction.getValue() instanceof DateRangeFilterInstruction dateRangeInstruction) { - // Note: assumption that dates are stored in long format. - filter.must(RangeQuery.of(r -> - r.date(d -> { - d.field(fieldToFilterInstruction.getKey()); - if (dateRangeInstruction.getFromDate() != null) { - d.gte(String.valueOf(dateRangeInstruction.getFromDate().getTime())); - } - if (dateRangeInstruction.getToDate() != null) { - d.lte(String.valueOf(dateRangeInstruction.getToDate().getTime())); - } - return d; - }) - )._toQuery()); - } - - if (fieldToFilterInstruction.getValue() instanceof SimpleFilterInstruction sfi) { - - List fieldsToMatch = Lists.newArrayList(); - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - fieldToFilterInstruction.getKey(), Constants.BooleanOperator.AND, - Collections.singletonList(sfi.getMustMatchValue()))); - - filter.must(this.generateBoolMatchQuery(fieldsToMatch)._toQuery()); - } - - if (fieldToFilterInstruction.getValue() instanceof TermsFilterInstruction sfi) { - filter.must( - TermsQuery.of(t -> t - .field(fieldToFilterInstruction.getKey()) - .terms(ts -> ts - .value( - sfi.getMatchValues().stream() - .map(FieldValue::of) - .collect(Collectors.toList()) - ) - ) - )._toQuery() - ); - } - - if (fieldToFilterInstruction.getValue() instanceof SimpleExclusionInstruction sfi) { - - List fieldsToMatch = Lists.newArrayList(); - fieldsToMatch.add(new GitContentManager.BooleanSearchClause( - fieldToFilterInstruction.getKey(), Constants.BooleanOperator.AND, - Collections.singletonList(sfi.getMustNotMatchValue()))); - - filter.mustNot(this.generateBoolMatchQuery(fieldsToMatch)._toQuery()); - } - } - - return filter.build()._toQuery(); - } - - /** - * Utility method to generate a BoolMatchQuery based on the parameters provided. - * - * @param fieldsToMatch - * - the fields that the bool query should match. - * @return a bool query configured to match the fields to match. - * @deprecated as {@code AbstractInstruction}-based instructions should be preferred over - * {@code BooleanSearchClause}, which are instead processed by {@code processMatchInstructions()}. - */ - @Deprecated - private BoolQuery generateBoolMatchQuery(final List fieldsToMatch) { - BoolQuery.Builder masterQuery = new BoolQuery.Builder(); - Map nestedQueriesByPath = Maps.newHashMap(); - - for (GitContentManager.BooleanSearchClause searchClause : fieldsToMatch) { - // Each search clause is its own boolean query that gets added to the master query as a must match clause - BoolQuery.Builder query = new BoolQuery.Builder(); - - // Add the clause to the query value by value - for (String value : searchClause.values()) { - Query matchQuery = MatchQuery.of(m -> m - .field(searchClause.field()) - .query(value) - )._toQuery(); - - if (Constants.BooleanOperator.OR.equals(searchClause.operator())) { - query.should(matchQuery); - } else if (Constants.BooleanOperator.AND.equals(searchClause.operator())) { - query.must(matchQuery); - } else if (Constants.BooleanOperator.NOT.equals(searchClause.operator())) { - query.mustNot(matchQuery); - } else { - log.warn("Null argument received in paginated match search... " - + "This is not usually expected. Ignoring it and continuing anyway."); - } - } - - // The way we're using this query, if we have a "should" the document needs to match at least one of the options. - if (Constants.BooleanOperator.OR.equals(searchClause.operator())) { - query.minimumShouldMatch("1"); - } - - if (!Constants.NESTED_QUERY_FIELDS.contains(searchClause.field())) { - masterQuery.must(query.build()._toQuery()); - } else { - // Nested fields need to use a nested query which specifies the path of the nested field. - String nestedPath = searchClause.field().split("\\.")[0]; - nestedQueriesByPath.putIfAbsent(nestedPath, new BoolQuery.Builder()); - nestedQueriesByPath.get(nestedPath).must(query.build()._toQuery()); - } - } - - // Nested queries are grouped so that queries on the same nested path are not queried independently. - for (Entry entry : nestedQueriesByPath.entrySet()) { - Query nestedQuery = NestedQuery.of(nq -> nq - .path(entry.getKey()) - .query(entry.getValue().build()._toQuery()) - .scoreMode(ChildScoreMode.Sum) - )._toQuery(); - masterQuery.must(nestedQuery); - } - - return masterQuery.build(); - } - - /** - * Provides default search execution using the fields specified. - * - * This method does not provide any way of controlling sort order or limiting information returned. It is most - * useful for doing simple searches with fewer results e.g. by id. - * - * @param indexBase - * - search index base string to execute the query against. - * @param indexType - * - index type to execute the query against. - * @param query - * - the query to run. - * @param startIndex - * - start index for results - * @param limit - * - the maximum number of results to return -1 will attempt to return all results. - * @return list of the search results - */ - private ResultsWrapper executeBasicQuery(final String indexBase, final String indexType, - final Query query, final int startIndex, final int limit) - throws SegueSearchException { - return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit, null); - } - /** * Provides default search execution using the fields specified. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java index 3755400dc6..cf18356cad 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java @@ -50,114 +50,12 @@ public interface ISearchProvider { */ Collection getAllIndices(); - /** - * Paginated Match search for one field. - * - * @param indexBase - * - ElasticSearch index base string - * @param indexType - * - Index type - * @param fieldsToMatch - * - the field name to use - and the field name search term - * @param startIndex - * - e.g. 0 for the first set of results - * @param limit - * - e.g. 10 for 10 results per page - * @param sortInstructions - * - the map of how to sort each field of interest. - * @param filterInstructions - * - the map of how to sort each field of interest. - * @return Results - */ - @Deprecated - ResultsWrapper matchSearch( - final String indexBase, final String indexType, - final List fieldsToMatch, final int startIndex, final int limit, - final Map sortInstructions, - @Nullable final Map filterInstructions - ) throws SegueSearchException; - ResultsWrapper nestedMatchSearch( final String indexBase, final String indexType, final Integer startIndex, final Integer limit, @NotNull final BooleanInstruction matchInstruction, @Nullable Long randomSeed, @Nullable final Map sortOrder ) throws SegueSearchException; - /** - * RandomisedPaginatedMatchSearch The same as paginatedMatchSearch but the results are returned in a random order. - * - * @param indexBase - * base string for the index that the content is stored in - * @param indexType - * - type of index as registered with search provider. - * @param fieldsToMatch - * - List of boolean clauses used for field matching. - * @param startIndex - * - start index for results - * @param limit - * - the maximum number of results to return. - * @param randomSeed - * - random seed. - * @param filterInstructions - * - post search filter instructions e.g. remove content of a certain type. - * @return results in a random order for a given match search. - */ - @Deprecated - ResultsWrapper randomisedMatchSearch( - String indexBase, String indexType, List fieldsToMatch, - int startIndex, int limit, Long randomSeed, Map filterInstructions - ) throws SegueSearchException; - - /** - * Query for a list of Results that match a given id prefix. - * - * This is useful if you use un-analysed fields for ids and use the dot separator as a way of nesting fields. - * - * @param indexBase - * base string for the index that the content is stored in - * @param indexType - * - type of index as registered with search provider. - * @param fieldname - * - fieldName to search within. - * @param prefix - * - idPrefix to search for. - * @param startIndex - * - start index for results - * @param limit - * - the maximum number of results to return -1 will attempt to return all results. - * @param filterInstructions - * - post search filter instructions e.g. remove content of a certain type. - * @return A list of results that match the id prefix. - */ - ResultsWrapper findByPrefix( - String indexBase, String indexType, String fieldname, String prefix, int startIndex, int limit, - @Nullable Map filterInstructions - ) throws SegueSearchException; - - /** - * Find content by a regex. - * - * @param indexBase - * base string for the index that the content is stored in - * @param indexType - * - type of index as registered with search provider. - * @param fieldname - * - fieldName to search within. - * @param regex - * - regex to search for. - * @param startIndex - * - start index for results - * @param limit - * - the maximum number of results to return -1 will attempt to return all results. - * @param filterInstructions - * - post search filter instructions e.g. remove content of a certain type. - * @return A list of results that match the id prefix. - */ - ResultsWrapper findByRegEx( - String indexBase, String indexType, String fieldname, String regex, int startIndex, int limit, - @Nullable Map filterInstructions - ) throws SegueSearchException; - /* * TODO: We need to change the return type of these two methods to avoid having ES specific things */ diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java index c9357ed3a8..5e14bc5fd8 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java @@ -15,125 +15,9 @@ */ package uk.ac.cam.cl.dtg.segue.search; -import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; -import co.elastic.clients.elasticsearch._types.query_dsl.Query; -import org.junit.jupiter.api.Test; - -import java.util.HashMap; -import java.util.Map; - -import static org.junit.jupiter.api.Assertions.assertEquals; - public class ElasticSearchProviderTest { - - @Test - public void generateFilterQuery_simpleFilterInstruction_returnsFilteredQuery() { - // Arrange - ElasticSearchProvider provider = new ElasticSearchProvider(null); - - Map filters = new HashMap<>(); - filters.put("published", new SimpleFilterInstruction("true")); - - Query expectedQuery = BoolQuery.of(bq -> bq - .must(Query.of(q -> q - .bool(b -> b - .must(Query.of(q2 -> q2 - .bool(b2 -> b2 - .must(Query.of(q3 -> q3 - .match(m -> m - .field("published") - .query(p -> p.stringValue("true")) - ) - )) - ) - )) - ) - )) - )._toQuery(); - - // Act - Query actualQuery = provider.generateFilterQuery(filters); - - // Assert - assertEquals(expectedQuery.toString(), actualQuery.toString()); - } - - @Test - public void generateFilterQuery_simpleExclusionInstruction_returnsExclusionQuery() { - // Arrange - ElasticSearchProvider provider = new ElasticSearchProvider(null); - - Map filters = new HashMap<>(); - filters.put("published", new SimpleExclusionInstruction("true")); - - Query expectedQuery = BoolQuery.of(bq -> bq - .mustNot(Query.of(q -> q - .bool(b -> b - .must(Query.of(q2 -> q2 - .bool(b2 -> b2 - .must(Query.of(q3 -> q3 - .match(m -> m - .field("published") - .query(p -> p.stringValue("true")) - ) - )) - ) - )) - ) - )) - )._toQuery(); - - // Act - Query actualQuery = provider.generateFilterQuery(filters); - - // Assert - assertEquals(expectedQuery.toString(), actualQuery.toString()); - } - - @Test - public void generateFilterQuery_simpleFilterInstructionAndExclusionInstruction_returnsFilteredAndExcludedQuery() { - // Arrange - ElasticSearchProvider provider = new ElasticSearchProvider(null); - - Map filters = new HashMap<>(); - filters.put("published", new SimpleFilterInstruction("true")); - filters.put("tags", new SimpleExclusionInstruction("regression_test")); - - Query expectedMustQuery = BoolQuery.of(bq -> bq - .must(Query.of(q -> q - .bool(b -> b - .must(Query.of(q2 -> q2 - .match(m -> m - .field("published") - .query(p -> p.stringValue("true")) - ) - )) - ) - )) - )._toQuery(); - - Query expectedMustNotQuery = BoolQuery.of(bq -> bq - .must(Query.of(q -> q - .bool(b -> b - .must(Query.of(q2 -> q2 - .match(m -> m - .field("tags") - .query(p -> p.stringValue("regression_test")) - ) - )) - ) - )) - )._toQuery(); - - Query expectedQuery = BoolQuery.of(bq -> bq - .must(expectedMustQuery) - .mustNot(expectedMustNotQuery) - )._toQuery(); - - // Act - Query actualQuery = provider.generateFilterQuery(filters); - - // Assert - assertEquals(expectedQuery.toString(), actualQuery.toString()); - } + /* + TODO This used to contain tests for the old-style ES query builder methods. It should be updated to test + nestedMatchSearch and related logic. + */ } From e00e259ac2195682058579479b6215b55052a921 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 14:27:34 +0100 Subject: [PATCH 22/40] Remove AbstractFilterInstruction & inheritors --- .../segue/dao/content/GitContentManager.java | 4 -- .../search/AbstractFilterInstruction.java | 26 ------- .../search/DateRangeFilterInstruction.java | 71 ------------------- .../segue/search/ElasticSearchProvider.java | 11 --- .../cl/dtg/segue/search/ISearchProvider.java | 2 - .../search/SimpleExclusionInstruction.java | 38 ---------- .../segue/search/SimpleFilterInstruction.java | 39 ---------- .../segue/search/TermsFilterInstruction.java | 36 ---------- 8 files changed, 227 deletions(-) delete mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/search/AbstractFilterInstruction.java delete mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/search/DateRangeFilterInstruction.java delete mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleExclusionInstruction.java delete mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleFilterInstruction.java delete mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/search/TermsFilterInstruction.java diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index ae618cc4c7..c93a286b5a 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -23,7 +23,6 @@ import com.google.api.client.util.Sets; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -41,7 +40,6 @@ import uk.ac.cam.cl.dtg.isaac.dto.content.SidebarDTO; import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.database.GitDb; -import uk.ac.cam.cl.dtg.segue.search.AbstractFilterInstruction; import uk.ac.cam.cl.dtg.segue.search.BooleanInstruction; import uk.ac.cam.cl.dtg.segue.search.ISearchProvider; import uk.ac.cam.cl.dtg.segue.search.IsaacSearchInstructionBuilder; @@ -50,8 +48,6 @@ import uk.ac.cam.cl.dtg.segue.search.MatchInstruction; import uk.ac.cam.cl.dtg.segue.search.SearchInField; import uk.ac.cam.cl.dtg.segue.search.SegueSearchException; -import uk.ac.cam.cl.dtg.segue.search.SimpleExclusionInstruction; -import uk.ac.cam.cl.dtg.segue.search.SimpleFilterInstruction; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import uk.ac.cam.cl.dtg.util.mappers.ContentMapper; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/AbstractFilterInstruction.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/AbstractFilterInstruction.java deleted file mode 100644 index 8102b22442..0000000000 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/AbstractFilterInstruction.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2015 Stephen Cummins - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.search; - -/** - * FilterInstruction - * - * Class to help abstract away the search filter format within Segue. - * - */ -public abstract class AbstractFilterInstruction { - -} diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/DateRangeFilterInstruction.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/DateRangeFilterInstruction.java deleted file mode 100644 index 801e95a1dd..0000000000 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/DateRangeFilterInstruction.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright 2015 Stephen Cummins - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.search; - -import java.util.Date; - -import jakarta.annotation.Nullable; - -/** - * DateRangeFilterInstruction. - * - * To be used to filter results based on a date range. - * - * @author sac92 - */ -public class DateRangeFilterInstruction extends AbstractFilterInstruction { - private Date fromDate; - private Date toDate; - - /** - * Create a new date range filter instruction. - * - * At least one of the fields should be populated. Otherwise you will get an illegal arguments exception; - * - * @param fromDate - * the start date results should match. - * @param toDate - * the end date that results can match. - */ - public DateRangeFilterInstruction(@Nullable final Date fromDate, @Nullable final Date toDate) { - this.fromDate = fromDate; - this.toDate = toDate; - - if (null == fromDate && null == toDate) { - throw new IllegalArgumentException( - "You must provide either a from date or a to date for this filter to work. " - + "Both are currently null"); - } - } - - /** - * Gets the fromDate. - * - * @return the fromDate - */ - public final Date getFromDate() { - return fromDate; - } - - /** - * Gets the toDate. - * - * @return the toDate - */ - public final Date getToDate() { - return toDate; - } -} diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java index e4cac06313..a54694162c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java @@ -17,11 +17,9 @@ import co.elastic.clients.elasticsearch.ElasticsearchClient; import co.elastic.clients.elasticsearch._types.ElasticsearchException; -import co.elastic.clients.elasticsearch._types.FieldValue; import co.elastic.clients.elasticsearch._types.SortOptions; import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; import co.elastic.clients.elasticsearch._types.query_dsl.ChildScoreMode; -import co.elastic.clients.elasticsearch._types.query_dsl.ConstantScoreQuery; import co.elastic.clients.elasticsearch._types.query_dsl.ExistsQuery; import co.elastic.clients.elasticsearch._types.query_dsl.FunctionBoostMode; import co.elastic.clients.elasticsearch._types.query_dsl.FunctionScoreQuery; @@ -31,8 +29,6 @@ import co.elastic.clients.elasticsearch._types.query_dsl.Query; import co.elastic.clients.elasticsearch._types.query_dsl.RandomScoreFunction; import co.elastic.clients.elasticsearch._types.query_dsl.RangeQuery; -import co.elastic.clients.elasticsearch._types.query_dsl.RegexpQuery; -import co.elastic.clients.elasticsearch._types.query_dsl.TermsQuery; import co.elastic.clients.elasticsearch._types.query_dsl.TextQueryType; import co.elastic.clients.elasticsearch._types.query_dsl.WildcardQuery; import co.elastic.clients.elasticsearch.core.GetResponse; @@ -47,12 +43,9 @@ import co.elastic.clients.transport.rest5_client.Rest5ClientTransport; import co.elastic.clients.transport.rest5_client.low_level.Rest5Client; import com.fasterxml.jackson.databind.node.ObjectNode; -import com.google.api.client.util.Lists; -import com.google.api.client.util.Maps; import com.google.common.base.CaseFormat; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHost; @@ -62,7 +55,6 @@ import org.slf4j.LoggerFactory; import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import jakarta.annotation.Nullable; import jakarta.validation.constraints.NotNull; @@ -75,11 +67,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; -import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java index cf18356cad..5a518259aa 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java @@ -19,12 +19,10 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.segue.api.Constants; -import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import jakarta.annotation.Nullable; import jakarta.validation.constraints.NotNull; import java.util.Collection; -import java.util.List; import java.util.Map; /** diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleExclusionInstruction.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleExclusionInstruction.java deleted file mode 100644 index a7ffa7a528..0000000000 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleExclusionInstruction.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2022 Matthew Trew - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.search; - -/** - * SimpleExclusionInstruction - * - * SimpleExclusionInstruction expect a single value which must not appear in the results. - */ -public class SimpleExclusionInstruction extends AbstractFilterInstruction { - private final String mustNotMatchValue; - - public SimpleExclusionInstruction(String mustNotMatchValue) { - - this.mustNotMatchValue = mustNotMatchValue; - } - - /** - * Get the value which must not be matched. - * @return the value to exclude. - */ - public String getMustNotMatchValue() { - return mustNotMatchValue; - } -} diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleFilterInstruction.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleFilterInstruction.java deleted file mode 100644 index 9e825a1881..0000000000 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/SimpleFilterInstruction.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2017 Stephen Cummins - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.search; - -/** - * SimpleFilterInstruction - * A class to help encapsulate filter instructions. - * - * SimpleFilterInstructions expect a single value which must be matched in the results. - */ -public class SimpleFilterInstruction extends AbstractFilterInstruction { - private final String mustMatchValue; - - public SimpleFilterInstruction(String mustMatchValue) { - - this.mustMatchValue = mustMatchValue; - } - - /** - * Get the value which must be matched. - * @return valueForFiltering. - */ - public String getMustMatchValue() { - return mustMatchValue; - } -} diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/TermsFilterInstruction.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/TermsFilterInstruction.java deleted file mode 100644 index f62fd021d5..0000000000 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/TermsFilterInstruction.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2017 Stephen Cummins - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.search; - -import java.util.Collection; - -/** - * TermsFilterInstruction. - * A class to help encapsulate filter instructions. - * - * This instruction will expect to match at least one of the terms in the list provided. - */ -public class TermsFilterInstruction extends AbstractFilterInstruction { - private final Collection matchValues; - - public TermsFilterInstruction(Collection matchValues) { - this.matchValues = matchValues; - } - - public Collection getMatchValues() { - return matchValues; - } -} From 2125d57e1ab6966345a0d182438cb9d46925a8fc Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 14:28:56 +0100 Subject: [PATCH 23/40] Remove BooleanSearchClause Deprecated in favour of BooleanInstructions --- .../cam/cl/dtg/segue/dao/content/GitContentManager.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index c93a286b5a..27bd652c64 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -755,13 +755,4 @@ private static void generateDerivedSummaryValues(final ContentDTO content, final List questionPartIds = questionParts.stream().map(QuestionDTO::getId).collect(Collectors.toList()); summary.setQuestionPartIds(questionPartIds); } - - /** - * An abstract representation of a search clause that can be interpreted as desired by different search providers. - * - * @deprecated in favour of {@code BooleanInstruction}, as an attempt to unify approaches to searching. - */ - @Deprecated - public record BooleanSearchClause(String field, BooleanOperator operator, List values) { - } } From 905ff45fd0bc5a9f1973d284691373118342b4f2 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 15:24:25 +0100 Subject: [PATCH 24/40] Show nofilter content by default in content manager To allow e.g. nofilter quizzes to be viewed. If nofilter content needs to be excluded based on role, this should be handled outside of this manager (e.g. in QuizManager). --- .../uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 27bd652c64..245669bb36 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -718,7 +718,7 @@ public String getCurrentContentSHA() { */ public IsaacSearchInstructionBuilder getBaseSearchInstructionBuilder() { return new IsaacSearchInstructionBuilder( - searchProvider, this.showOnlyPublishedContent, this.hideRegressionTestContent, true); + searchProvider, this.showOnlyPublishedContent, this.hideRegressionTestContent, false); } /** From 87ca6e08915bdc7443830d42c5cb5fbcf574b032 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Mon, 1 Jun 2026 15:38:51 +0100 Subject: [PATCH 25/40] Fix nested boolean query structure The "should" clauses should be grouped together and nested in a parent "must" to ensure at least one clause per group is satisfied. --- .../cam/cl/dtg/isaac/api/managers/FastTrackManger.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java index b0f77ec080..cc70921a05 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java @@ -113,12 +113,18 @@ private List getFastTrackConceptQuestions( List stringLevelFilters = levelFilters.stream().map(FASTTRACK_LEVEL::name).toList(); BooleanInstruction searchInstruction = new BooleanInstruction(); + BooleanInstruction pageTypeInstruction = new BooleanInstruction(); for (String type : QUESTION_PAGE_TYPES) { - searchInstruction.should(new MatchInstruction(TYPE_FIELDNAME, type)); + pageTypeInstruction.should(new MatchInstruction(TYPE_FIELDNAME, type)); } + searchInstruction.must(pageTypeInstruction); + + BooleanInstruction levelInstruction = new BooleanInstruction(); for (String levelFilter : stringLevelFilters) { - searchInstruction.should(new MatchInstruction(TAGS_FIELDNAME, levelFilter)); + levelInstruction.should(new MatchInstruction(TAGS_FIELDNAME, levelFilter)); } + searchInstruction.must(levelInstruction); + searchInstruction.must(new MatchInstruction(TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, conceptTitle)); searchInstruction.must(new MatchInstruction(TAGS_FIELDNAME, boardTag)); From 6fc7a824180205a70e0910ba36de7c8f831104c5 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 3 Jun 2026 10:18:27 +0100 Subject: [PATCH 26/40] Remove ElasticSearchProviderTest ElasticSearchProvider no longer contains any public helper functions for building queries that can be tested in isolation. It only contains functions that directly interact with the Elasticsearch client which can't easily be tested with unit tests. --- .../search/ElasticSearchProviderTest.java | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java deleted file mode 100644 index 5e14bc5fd8..0000000000 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2022 Matthew Trew - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package uk.ac.cam.cl.dtg.segue.search; - -public class ElasticSearchProviderTest { - /* - TODO This used to contain tests for the old-style ES query builder methods. It should be updated to test - nestedMatchSearch and related logic. - */ -} From 9a6708cbd52ff5298dd9f2f959e7cae1333e3201 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 3 Jun 2026 14:10:25 +0100 Subject: [PATCH 27/40] Don't unnecessarily nest match instructions inside boolean instructions If a boolean instruction contains just one match clause, this can be simplified to just a match instruction. Overloading nestedMatchSearch allows us to remove the confusing nesting at the call site whilst still re-using all the existing boolean search functionality. --- .../dtg/isaac/api/managers/GameManager.java | 3 +-- .../cam/cl/dtg/segue/api/GlossaryFacade.java | 3 +-- .../api/managers/NotificationPicker.java | 3 +-- .../segue/dao/content/GitContentManager.java | 25 ++++++++++++++++--- .../segue/dao/schools/SchoolListReader.java | 3 +-- .../segue/search/ElasticSearchProvider.java | 12 +++++++++ .../cl/dtg/segue/search/ISearchProvider.java | 6 +++++ 7 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java index c1aba87378..23d1f19617 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java @@ -626,8 +626,7 @@ public List>> gatherGamePro * - if we cannot access the content requested. */ public List getWildcards() throws NoWildcardException, ContentManagerException { - BooleanInstruction searchInstruction = new BooleanInstruction(); - searchInstruction.should(new MatchInstruction(TYPE_FIELDNAME, WILDCARD_TYPE)); + MatchInstruction searchInstruction = new MatchInstruction(TYPE_FIELDNAME, WILDCARD_TYPE); Map sortInstructions = Maps.newHashMap(); sortInstructions.put(TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, SortOrder.ASC); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java index db1ede24a2..761c1832b7 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java @@ -135,8 +135,7 @@ public final Response getTerms(@Context final Request request, @QueryParam("star // Get from server cache, else load and cache: try { ResultsWrapper c = termCache.get(cacheKey, () -> { - BooleanInstruction searchInstruction = new BooleanInstruction();; - searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, "glossaryTerm")); + MatchInstruction searchInstruction = new MatchInstruction(TYPE_FIELDNAME, "glossaryTerm"); return this.contentManager.nestedMatchSearch(searchInstruction, startIndexOfResults, resultsLimit, null, null); }); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java index 52b7390edf..ce3fab402c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/NotificationPicker.java @@ -75,8 +75,7 @@ public NotificationPicker(final GitContentManager contentManager, final PgUserNo public List getAvailableNotificationsForUser(final RegisteredUserDTO user) throws ContentManagerException, SegueDatabaseException { // get users notification record - BooleanInstruction searchInstruction = new BooleanInstruction(); - searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, "notification")); + MatchInstruction searchInstruction = new MatchInstruction(TYPE_FIELDNAME, "notification"); ResultsWrapper allContentNotifications = this.contentManager .nestedMatchSearch(searchInstruction, 0, -1, null, null); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 245669bb36..f7663c6613 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -248,10 +248,7 @@ public Content getContentDOById(final String id, final boolean failQuietly) thro try { ResultsWrapper result = contentDOcache.get(k, () -> { - BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); - searchInstruction.must(new MatchInstruction( - Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, id)); + MatchInstruction searchInstruction = new MatchInstruction(Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, id); ResultsWrapper rawResults = searchProvider.nestedMatchSearch(contentIndex, CONTENT_INDEX_TYPE.CONTENT.toString(), 0, 1, searchInstruction, null, null); @@ -743,6 +740,26 @@ public ResultsWrapper nestedMatchSearch(final BooleanInstruction ins return new ResultsWrapper<>(dtoResults, searchHits.getTotalResults()); } + /** + * Search for content that matches a given instruction and map the hits to DTOs. + * + * @param instruction - the {@link MatchInstruction} to search with. + * @param startIndex - the initial index for the first result. + * @param limit - the maximum number of results to return. + * @param randomSeed - the random seed to use for the search. + * @param sortInstructions - map of sorting functions to use in ElasticSearch query. + * @return a ResultsWrapper containing the matching content as DTOs and the total number of results. + * @throws ContentManagerException + */ + public ResultsWrapper nestedMatchSearch(final MatchInstruction instruction, final Integer startIndex, + final Integer limit, final Long randomSeed, + final Map sortInstructions) + throws ContentManagerException { + BooleanInstruction booleanInstruction = new BooleanInstruction(); + booleanInstruction.must(instruction); + return nestedMatchSearch(booleanInstruction, startIndex, limit, randomSeed, sortInstructions); + } + /** * A method which adds information to the contentSummaryDTO, summary, from values evaluated from the content. * @param content the original content object which was used to create the summary. diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java index 045fce7461..522b931eea 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java @@ -144,8 +144,7 @@ public School findSchoolById(final String schoolURN) throws UnableToIndexSchools throw new UnableToIndexSchoolsException("unable to ensure the cache has been populated"); } - BooleanInstruction searchInstruction = new BooleanInstruction(); - searchInstruction.must(new MatchInstruction(SCHOOL_URN_FIELDNAME.toLowerCase() + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, schoolURN)); + MatchInstruction searchInstruction = new MatchInstruction(SCHOOL_URN_FIELDNAME.toLowerCase() + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, schoolURN); List matchingSchoolList = searchProvider.nestedMatchSearch(SCHOOLS_INDEX_BASE, SCHOOLS_INDEX_TYPE.SCHOOL_SEARCH.toString(), 0, DEFAULT_RESULTS_LIMIT, searchInstruction, null, null diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java index a54694162c..1a7e4bc845 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProvider.java @@ -146,6 +146,18 @@ public ResultsWrapper nestedMatchSearch(final String indexBase, final St return this.executeBasicQuery(indexBase, indexType, query, startIndex, limit, sortOrder); } + @Override + public ResultsWrapper nestedMatchSearch(final String indexBase, final String indexType, + final Integer startIndex, final Integer limit, + @NotNull final MatchInstruction matchInstruction, + @Nullable final Long randomSeed, + @Nullable final Map sortOrder + ) throws SegueSearchException { + BooleanInstruction booleanInstruction = new BooleanInstruction(); + booleanInstruction.must(matchInstruction); + return nestedMatchSearch(indexBase, indexType, startIndex, limit, booleanInstruction, randomSeed, sortOrder); + } + /** * This method will create a threadsafe client that can be used to talk to an Elastic Search cluster. * @param address diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java index 5a518259aa..c07ae39a57 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/ISearchProvider.java @@ -54,6 +54,12 @@ ResultsWrapper nestedMatchSearch( @Nullable final Map sortOrder ) throws SegueSearchException; + ResultsWrapper nestedMatchSearch( + final String indexBase, final String indexType, final Integer startIndex, final Integer limit, + @NotNull final MatchInstruction matchInstruction, @Nullable Long randomSeed, + @Nullable final Map sortOrder + ) throws SegueSearchException; + /* * TODO: We need to change the return type of these two methods to avoid having ES specific things */ From 681ab6dd29b0d4e24e98ae2eddd23f974ef13949 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 3 Jun 2026 15:54:38 +0100 Subject: [PATCH 28/40] Don't unnecessarily exclude unpublished/nofilter (etc) content --- .../java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java | 6 ++---- .../api/managers/EventNotificationEmailManager.java | 6 ++---- .../cl/dtg/isaac/dao/GameboardPersistenceManager.java | 3 +-- .../cam/cl/dtg/segue/dao/content/GitContentManager.java | 9 +++------ 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index 565f2424b9..8e39173e3a 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java @@ -209,8 +209,7 @@ public final Response getConceptList(@Context final Request request, @QueryParam } try { - BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, CONCEPT_TYPE)); if (idsList != null && !idsList.isEmpty()) { @@ -1177,8 +1176,7 @@ public final Response getPodList(@Context final Request request, } try { - BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, POD_FRAGMENT_TYPE)); searchInstruction.must(new MatchInstruction(TAGS_FIELDNAME, subject)); diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java index d7bad6c3f5..ee237e8d8c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java @@ -101,8 +101,7 @@ public void sendReminderEmails() { ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime threeDaysAhead = now.plusDays(3); - BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, EVENT_TYPE)); searchInstruction.must(new RangeInstruction(DATE_FIELDNAME) .greaterThanOrEqual(new Date().getTime()) @@ -142,8 +141,7 @@ public void sendFeedbackEmails() { ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime sixtyDaysAgo = now.plusDays(-60); - BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); searchInstruction.must(new MatchInstruction(TYPE_FIELDNAME, EVENT_TYPE)); searchInstruction.must(new RangeInstruction(DATE_FIELDNAME) .greaterThanOrEqual(Date.from(sixtyDaysAgo.toInstant()).getTime()) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java index 3234480fdb..5cb56e1c37 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/GameboardPersistenceManager.java @@ -435,8 +435,7 @@ public List getInvalidQuestionIdsFromGameboard(final GameboardDTO gamebo List questionIds = gameboardDO.getContents().stream().map(GameboardContentDescriptor::getId).toList(); // build query the db to get full question information - BooleanInstruction searchInstruction = this.contentManager.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); BooleanInstruction idsInstruction = new BooleanInstruction(); for (String questionId : questionIds) { diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index f7663c6613..e8b1e35c4c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -294,8 +294,7 @@ public ResultsWrapper getUnsafeCachedContentDTOsMatchingIds(final Co try { return contentDTOcache.get(k, () -> { - BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); BooleanInstruction idsInstruction = new BooleanInstruction(1); for (String id : ids) { @@ -629,13 +628,11 @@ public ContentDTO populateRelatedContent(final ContentDTO contentDTO) } // build query the db to get full content information - BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder() - .buildBaseInstructions(new BooleanInstruction()); + BooleanInstruction searchInstruction = new BooleanInstruction(); BooleanInstruction idsInstruction = new BooleanInstruction(); for (String relatedContentId : relatedContentIds) { - idsInstruction.should(new MatchInstruction( - Constants.ID_FIELDNAME + '.' + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, relatedContentId)); + idsInstruction.should(new MatchInstruction(Constants.ID_FIELDNAME + '.' + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, relatedContentId)); } searchInstruction.must(idsInstruction); From c0bf8dfbecc10b8eff25ea16180a8f86b63635e1 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 5 Jun 2026 11:06:47 +0100 Subject: [PATCH 29/40] Remove unnecessary "minimum should match" argument This nested instruction has no must/filter clauses, so "minimum should match" defaults to 1 anyway. --- .../uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index e8b1e35c4c..4e07e201f7 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -296,7 +296,7 @@ public ResultsWrapper getUnsafeCachedContentDTOsMatchingIds(final Co BooleanInstruction searchInstruction = new BooleanInstruction(); - BooleanInstruction idsInstruction = new BooleanInstruction(1); + BooleanInstruction idsInstruction = new BooleanInstruction(); for (String id : ids) { idsInstruction.should(new MatchInstruction(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, id)); } From 42df78a25b245ddea73b5af40b7967ad4cef1e44 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Thu, 11 Jun 2026 16:37:24 +0100 Subject: [PATCH 30/40] Add documentation link to BooleanInstruction class --- .../uk/ac/cam/cl/dtg/segue/search/BooleanInstruction.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/BooleanInstruction.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/BooleanInstruction.java index d7d44dda4c..7c36026704 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/BooleanInstruction.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/BooleanInstruction.java @@ -4,6 +4,11 @@ import java.util.List; +/** + * Class to represent an Elasticsearch boolean query. + * musts/shoulds/shouldNots work as described in the Elasticsearch documentation: + * https://www.elastic.co/docs/reference/query-languages/query-dsl/query-dsl-bool-query + */ public class BooleanInstruction extends AbstractInstruction { private List musts = Lists.newArrayList(); private List shoulds = Lists.newArrayList(); From b7284de26d27198e948d9fe3224e9ed54a920e26 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Thu, 11 Jun 2026 16:59:15 +0100 Subject: [PATCH 31/40] Sort concepts listing alphabetically --- src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index 8e39173e3a..cff2e45a91 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java @@ -227,9 +227,12 @@ public final Response getConceptList(@Context final Request request, @QueryParam } searchInstruction.must(tagsInstruction); } + + Map sortInstructions = Maps.newHashMap(); + sortInstructions.put(TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, SortOrder.ASC); ResultsWrapper c = this.contentManager.nestedMatchSearch( - searchInstruction, startIndex, newLimit, null, null); + searchInstruction, startIndex, newLimit, null, sortInstructions); ResultsWrapper summarizedContent = new ResultsWrapper<>( this.extractContentSummaryFromList(c.getResults()), From c118c770e3bb89dfa2622360fe1a77ba6bb837bc Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 16 Jun 2026 11:38:34 +0100 Subject: [PATCH 32/40] Revert change to include nofilter content by default Go back to excluding nofilter content by default, and explicitly set the `excludeNofilterContent` property on the base builder when required. --- .../uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java | 3 +++ .../uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java index a7c7520749..462eac6c10 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java @@ -77,6 +77,7 @@ public ResultsWrapper getEvents(final String tags, final Integer sta throws ContentManagerException, SegueDatabaseException { IsaacSearchInstructionBuilder searchInstructionBuilder = this.contentManager.getBaseSearchInstructionBuilder() + .includeHiddenContent(includeHiddenContent) .includeContentTypes(Collections.singleton(EVENT_TYPE)); if (tags != null) { @@ -128,7 +129,9 @@ public ResultsWrapper> getEventOverviews(final Integer start final String filter, final RegisteredUserDTO currentUser) throws ContentManagerException, SegueDatabaseException { + // This is only used for a staff-only endpoint, so always include nofilter content IsaacSearchInstructionBuilder searchInstructionBuilder = this.contentManager.getBaseSearchInstructionBuilder() + .includeHiddenContent(true) .includeContentTypes(Collections.singleton(EVENT_TYPE)); final Map sortInstructions = Maps.newHashMap(); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 4e07e201f7..5a320279be 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -712,7 +712,7 @@ public String getCurrentContentSHA() { */ public IsaacSearchInstructionBuilder getBaseSearchInstructionBuilder() { return new IsaacSearchInstructionBuilder( - searchProvider, this.showOnlyPublishedContent, this.hideRegressionTestContent, false); + searchProvider, this.showOnlyPublishedContent, this.hideRegressionTestContent, true); } /** From 08c44d8f189c38fc426516a9aee3f8b75eca14d0 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 16 Jun 2026 11:45:23 +0100 Subject: [PATCH 33/40] Update Javadoc --- src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java index 7a89602d32..2179304cf8 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java @@ -1288,6 +1288,7 @@ public final Response getEventOverviews(@Context final HttpServletRequest reques /** * REST end point to provide a summary of events suitable for mapping. + * Excludes nofilter events. * * @param request - this allows us to check to see if a user is currently logged in. * @param startIndex - the initial index for the first result. From c39bf1fccdf70b098f4d5f1c1fbfe998aa3689a8 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 16 Jun 2026 12:00:18 +0100 Subject: [PATCH 34/40] Remove duplicated nofilter filter --- .../java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java index 462eac6c10..5d356a3bbe 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventsManager.java @@ -77,7 +77,6 @@ public ResultsWrapper getEvents(final String tags, final Integer sta throws ContentManagerException, SegueDatabaseException { IsaacSearchInstructionBuilder searchInstructionBuilder = this.contentManager.getBaseSearchInstructionBuilder() - .includeHiddenContent(includeHiddenContent) .includeContentTypes(Collections.singleton(EVENT_TYPE)); if (tags != null) { From b8d99b5044b9adfc1a932fe374547c19c0b99d3a Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 16 Jun 2026 13:37:20 +0100 Subject: [PATCH 35/40] Exclude unpublished & regression test content where necessary --- .../ac/cam/cl/dtg/segue/dao/content/GitContentManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 5a320279be..13db8c4276 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -247,8 +247,8 @@ public Content getContentDOById(final String id, final boolean failQuietly) thro try { ResultsWrapper result = contentDOcache.get(k, () -> { - - MatchInstruction searchInstruction = new MatchInstruction(Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, id); + BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder().includeHiddenContent(true).build(); + searchInstruction.must(new MatchInstruction(Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, id)); ResultsWrapper rawResults = searchProvider.nestedMatchSearch(contentIndex, CONTENT_INDEX_TYPE.CONTENT.toString(), 0, 1, searchInstruction, null, null); @@ -294,7 +294,7 @@ public ResultsWrapper getUnsafeCachedContentDTOsMatchingIds(final Co try { return contentDTOcache.get(k, () -> { - BooleanInstruction searchInstruction = new BooleanInstruction(); + BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder().includeHiddenContent(true).build(); BooleanInstruction idsInstruction = new BooleanInstruction(); for (String id : ids) { From 2fe8151d859ab86fbbfed11d5bade5b8c41b9feb Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 17 Jun 2026 14:13:33 +0100 Subject: [PATCH 36/40] In search instruction builder, don't filter by content type by default If a search instruction is built without explicit type filters being passed in, we were defaulting to only returning pages that would be returned by sitewide search. This meant that the search instruction builder - and its more generally useful functionality such as setting base filters - couldn't easily be used in places where we want to return content of _any_ type, unless every possible content type was passed in to the builder. Instead, if no content types are provided, assume all content types should be returned. The existing places that need to filter by content type, such as sitewide search, already pass in the appropriate content types anyway so we weren't relying on the previous default behaviour. This also requires not explicitly setting minimumShouldMatch, since in the case where no content types are provided we no longer require at least one content type to match, so we should let it default to zero. In the case where content types _are_ provided, minimumShouldMatch will default to one anyway, since all of its nested clauses are shoulds. --- .../cl/dtg/segue/search/IsaacSearchInstructionBuilder.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/search/IsaacSearchInstructionBuilder.java b/src/main/java/uk/ac/cam/cl/dtg/segue/search/IsaacSearchInstructionBuilder.java index df5d5c585d..68371e155e 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/search/IsaacSearchInstructionBuilder.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/search/IsaacSearchInstructionBuilder.java @@ -236,18 +236,12 @@ public IsaacSearchInstructionBuilder includeHiddenContent(final boolean includeH public BooleanInstruction build() { BooleanInstruction masterInstruction = this.buildBaseInstructions(new BooleanInstruction()); - masterInstruction.setMinimumShouldMatch(1); - List contentTypes = Optional.ofNullable(this.includedContentTypes) .orElse(Collections.emptySet()) .stream() .filter(SEARCHABLE_DOC_TYPES::contains) .collect(Collectors.toList()); - if (contentTypes.isEmpty()) { - contentTypes = Lists.newArrayList(SITE_WIDE_SEARCH_VALID_DOC_TYPES); - } - for (String contentType : contentTypes) { BooleanInstruction contentInstruction = new BooleanInstruction(); From 0bbf0ca8a065aa870eba6f9ae830dd72254b8a32 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 17 Jun 2026 14:15:06 +0100 Subject: [PATCH 37/40] Fix Checkstyle warnings --- .../segue/dao/content/GitContentManager.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index 13db8c4276..b13e61db78 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -346,14 +346,14 @@ public final ResultsWrapper siteWideSearch( .includeContentTypes(contentTypes) // High priority matches on untokenised search string - .searchFor(new SearchInField(Constants.ID_FIELDNAME + "." + - Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) + .searchFor(new SearchInField(Constants.ID_FIELDNAME + "." + + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) .priority(Priority.HIGH).strategy(Strategy.SIMPLE)) - .searchFor(new SearchInField(Constants.TITLE_FIELDNAME + "." + - Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) + .searchFor(new SearchInField(Constants.TITLE_FIELDNAME + "." + + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) .priority(Priority.HIGH).strategy(Strategy.SIMPLE)) - .searchFor(new SearchInField(Constants.SUBTITLE_FIELDNAME + "." + - Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) + .searchFor(new SearchInField(Constants.SUBTITLE_FIELDNAME + "." + + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) .priority(Priority.HIGH).strategy(Strategy.SIMPLE)) // Fuzzy search term matches @@ -461,14 +461,14 @@ public final ResultsWrapper questionSearch( if (searchString != null && !searchString.isBlank()) { // High priority matches on untokenised search string searchInstructionBuilder - .searchFor(new SearchInField(Constants.ID_FIELDNAME + "." + - Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) + .searchFor(new SearchInField(Constants.ID_FIELDNAME + "." + + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) .priority(Priority.HIGH).strategy(Strategy.SIMPLE)) - .searchFor(new SearchInField(Constants.TITLE_FIELDNAME + "." + - Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) + .searchFor(new SearchInField(Constants.TITLE_FIELDNAME + "." + + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) .priority(Priority.HIGH).strategy(Strategy.SIMPLE)) - .searchFor(new SearchInField(Constants.SUBTITLE_FIELDNAME + "." + - Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) + .searchFor(new SearchInField(Constants.SUBTITLE_FIELDNAME + "." + + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, Collections.singleton(searchString)) .priority(Priority.HIGH).strategy(Strategy.SIMPLE)); } @@ -712,7 +712,7 @@ public String getCurrentContentSHA() { */ public IsaacSearchInstructionBuilder getBaseSearchInstructionBuilder() { return new IsaacSearchInstructionBuilder( - searchProvider, this.showOnlyPublishedContent, this.hideRegressionTestContent, true); + searchProvider, false, false, false); } /** From 8faa2173b20e024a655cb283ae4ce446af1eade7 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 17 Jun 2026 14:27:24 +0100 Subject: [PATCH 38/40] Allow for events without end dates when finding past events All events should now have end dates, but this wasn't always the case. To be safe, the initial list of events considered here should include events with a date in the past, in case the end date isn't set. The logic further down still disregards the date if an end date _is_ set. --- .../jobs/DeleteEventAdditionalBookingInformationJob.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java b/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java index 18524cb7f5..c34cad04e2 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/DeleteEventAdditionalBookingInformationJob.java @@ -57,7 +57,12 @@ public void execute(final JobExecutionContext context) throws JobExecutionExcept BooleanInstruction instruction = new BooleanInstruction(); instruction.must(new MatchInstruction(TYPE_FIELDNAME, EVENT_TYPE)); - instruction.must(new RangeInstruction(ENDDATE_FIELDNAME).lessThanOrEqual(new Date().getTime())); + + // Start with all events with end dates in the past OR dates in the past, in case end date isn't set + BooleanInstruction pastEventsInstruction = new BooleanInstruction(); + pastEventsInstruction.should(new RangeInstruction(ENDDATE_FIELDNAME).lessThanOrEqual(new Date().getTime())); + pastEventsInstruction.should(new RangeInstruction(DATE_FIELDNAME).lessThanOrEqual(new Date().getTime())); + instruction.must(pastEventsInstruction); ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime thirtyDaysAgo = now.plusDays(-30); From 88c5d6e45430cff7acfd729ad5a480a12b798c6f Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 17 Jun 2026 14:45:44 +0100 Subject: [PATCH 39/40] Remove duplicated logic The search logic here was identical to that in getUnsafeCachedContentDTOsMatchingIds, so just call the method. --- .../cl/dtg/segue/dao/content/GitContentManager.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index b13e61db78..b2abb7463b 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -627,16 +627,7 @@ public ContentDTO populateRelatedContent(final ContentDTO contentDTO) relatedContentIds.add(summary.getId()); } - // build query the db to get full content information - BooleanInstruction searchInstruction = new BooleanInstruction(); - - BooleanInstruction idsInstruction = new BooleanInstruction(); - for (String relatedContentId : relatedContentIds) { - idsInstruction.should(new MatchInstruction(Constants.ID_FIELDNAME + '.' + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, relatedContentId)); - } - searchInstruction.must(idsInstruction); - - ResultsWrapper results = this.nestedMatchSearch(searchInstruction, 0, relatedContentIds.size(), null, null); + ResultsWrapper results = getUnsafeCachedContentDTOsMatchingIds(relatedContentIds, 0, relatedContentIds.size()); List relatedContentDTOs = Lists.newArrayList(); From 5516d1bcb18989d62add1fec51d6cb755e6943ed Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 19 Jun 2026 09:56:21 +0100 Subject: [PATCH 40/40] Revert accidental change --- .../uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index b2abb7463b..5fbc9e23f3 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -703,7 +703,7 @@ public String getCurrentContentSHA() { */ public IsaacSearchInstructionBuilder getBaseSearchInstructionBuilder() { return new IsaacSearchInstructionBuilder( - searchProvider, false, false, false); + searchProvider, this.showOnlyPublishedContent, this.hideRegressionTestContent, true); } /**