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. 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/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index dfd1cd4a97..638c3f5040 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 @@ -54,12 +54,13 @@ 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; 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; @@ -103,7 +104,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; @@ -116,8 +116,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 @@ -138,13 +136,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; @@ -181,31 +178,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. @@ -219,10 +210,39 @@ public final Response getConceptList(@Context final Request request, @QueryParam } try { - return listContentObjects(fieldsToMatch, booleanOperatorOverrideMap, startIndex, newLimit).tag(etag) + BooleanInstruction searchInstruction = 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); + } + + Map sortInstructions = Maps.newHashMap(); + sortInstructions.put(TITLE_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, SortOrder.ASC); + + ResultsWrapper c = this.contentManager.nestedMatchSearch( + searchInstruction, startIndex, newLimit, null, sortInstructions); + + 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); @@ -1172,17 +1192,16 @@ 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 = 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) @@ -1292,44 +1311,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/EventNotificationEmailManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventNotificationEmailManager.java index 357a213eac..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 @@ -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,21 @@ 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 = 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 +136,21 @@ 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 = 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())) { 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..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 @@ -128,7 +128,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/isaac/api/managers/FastTrackManger.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/FastTrackManger.java index 141df37786..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 @@ -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; @@ -11,18 +10,17 @@ 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; -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.*; @@ -68,11 +66,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) { @@ -113,24 +110,31 @@ 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(); + + BooleanInstruction pageTypeInstruction = new BooleanInstruction(); + for (String type : QUESTION_PAGE_TYPES) { + pageTypeInstruction.should(new MatchInstruction(TYPE_FIELDNAME, type)); + } + searchInstruction.must(pageTypeInstruction); + + BooleanInstruction levelInstruction = new BooleanInstruction(); + for (String levelFilter : stringLevelFilters) { + 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)); + 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(); } } 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..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 @@ -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; @@ -55,8 +52,9 @@ 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; import jakarta.annotation.Nullable; import jakarta.validation.constraints.NotNull; @@ -67,7 +65,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 +81,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 +97,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 +616,6 @@ public List>> gatherGamePro return result; } - /** * Find all wildcards. * @@ -709,16 +626,12 @@ 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))); + MatchInstruction searchInstruction = 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(); @@ -977,139 +890,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 +975,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/api/managers/QuizManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManager.java index 828ef02ef2..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 @@ -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; @@ -34,13 +33,13 @@ 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; +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 +57,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; @@ -67,41 +65,34 @@ 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.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); } 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..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 @@ -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; @@ -106,7 +107,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 +167,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. * @@ -445,21 +432,27 @@ 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 = 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); 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..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 @@ -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,8 @@ 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); + MatchInstruction searchInstruction = 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(); 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..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 @@ -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,10 @@ 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"))); + MatchInstruction searchInstruction = new MatchInstruction(TYPE_FIELDNAME, "notification"); ResultsWrapper allContentNotifications = this.contentManager - .findByFieldNames(fieldsToMatch, 0, -1); + .nestedMatchSearch(searchInstruction, 0, -1, null, null); Map listOfRecordedNotifications = getMapOfRecordedNotifications(user); 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/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..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 @@ -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,17 +40,14 @@ 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; 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; -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; @@ -251,13 +247,11 @@ public Content getContentDOById(final String id, final boolean failQuietly) thro try { ResultsWrapper result = contentDOcache.get(k, () -> { + BooleanInstruction searchInstruction = this.getBaseSearchInstructionBuilder().includeHiddenContent(true).build(); + searchInstruction.must(new MatchInstruction(Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, id)); - ResultsWrapper rawResults = searchProvider.termSearch( - contentIndex, - CONTENT_INDEX_TYPE.CONTENT.toString(), - id, - Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, 0, 1, - getBaseFilters()); + ResultsWrapper rawResults = searchProvider.nestedMatchSearch(contentIndex, + CONTENT_INDEX_TYPE.CONTENT.toString(), 0, 1, searchInstruction, null, null); List searchResults = contentSubclassMapper .mapFromStringListToContentList(rawResults.getResults()); @@ -300,24 +294,16 @@ 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().includeHiddenContent(true).build(); - if (getBaseFilters() != null) { - finalFilter.putAll(getBaseFilters()); + BooleanInstruction idsInstruction = new BooleanInstruction(); + 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()); @@ -360,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 @@ -475,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)); } @@ -531,93 +517,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; - } - - @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); } @@ -723,19 +622,12 @@ 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)); - - ResultsWrapper results = this.findByFieldNames(fieldsToMap, 0, relatedContentIds.size()); + ResultsWrapper results = getUnsafeCachedContentDTOsMatchingIds(relatedContentIds, 0, relatedContentIds.size()); List relatedContentDTOs = Lists.newArrayList(); @@ -837,24 +729,23 @@ public ResultsWrapper nestedMatchSearch(final BooleanInstruction ins } /** - * Returns the basic filter configuration. + * Search for content that matches a given instruction and map the hits to DTOs. * - * @return either null or a map setup with filter/exclusion instructions, based on environment properties. + * @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 */ - 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); + 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); } /** @@ -869,13 +760,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) { - } } 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..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 @@ -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) { @@ -134,11 +144,11 @@ 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(); + 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 + ).getResults(); if (matchingSchoolList.isEmpty()) { return null; 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..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 @@ -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,24 @@ 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)); + + // 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); 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()); 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/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(); 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 41dfc70dda..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 @@ -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.*; @@ -125,66 +114,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, @@ -218,101 +147,15 @@ public ResultsWrapper nestedMatchSearch(final String indexBase, final St } @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, - @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); + 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); } /** @@ -371,82 +214,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, - 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. * @@ -475,161 +242,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. * @@ -724,34 +336,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..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 @@ -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; /** @@ -50,195 +48,16 @@ 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; - - /** - * 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, @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. - * - * @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 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. - * - * 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 + 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; /* 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(); 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; - } -} 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..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; @@ -297,7 +296,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); @@ -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 8e085c600b..07dbf59cf6 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 @@ -22,7 +22,6 @@ import uk.ac.cam.cl.dtg.isaac.dos.BookmarkDO; 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; @@ -44,7 +43,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/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/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 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); - } -} 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 c9357ed3a8..0000000000 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java +++ /dev/null @@ -1,139 +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; - -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()); - } -}