From aa670045ab5fed45e81cc8e92c0bc341d1067d20 Mon Sep 17 00:00:00 2001 From: Marius Date: Fri, 30 Jan 2026 17:38:06 +0800 Subject: [PATCH 1/2] 791 - School field missing for users after school list update --- .../ac/cam/cl/dtg/isaac/api/EventsFacade.java | 21 +- .../SegueGuiceConfigurationModule.java | 7 +- .../dtg/segue/dao/schools/PgSchoolLookup.java | 90 +++++++++ .../segue/dao/schools/SchoolListReader.java | 51 ++++- .../segue/dao/schools/PgSchoolLookupTest.java | 187 +++++++++++++++++ .../dao/schools/SchoolListReaderTest.java | 188 ++++++++++++++++++ 6 files changed, 537 insertions(+), 7 deletions(-) create mode 100644 src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java create mode 100644 src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookupTest.java create mode 100644 src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReaderTest.java 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 82a66504f1..737255317d 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 @@ -88,6 +88,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.jboss.resteasy.annotations.GZIP; @@ -1205,7 +1206,23 @@ private void addTeacherInformation(final Map info, final Registe info.put("teacherEmail", teacher.getEmail()); info.put("teacherId", teacher.getId().toString()); info.put("teacherSchoolId", teacher.getSchoolId() != null ? teacher.getSchoolId() : ""); - info.put("teacherSchoolName", teacher.getSchoolOther() != null ? teacher.getSchoolOther() : ""); + info.put("teacherSchoolName", findUsersSchoolName(teacher)); + } + + private String findUsersSchoolName(RegisteredUserDTO user) { + return Optional.ofNullable(user.getSchoolOther()) + .or(() -> Optional.ofNullable(user.getSchoolId()) + .flatMap(this::findSchoolNameById)) + .orElse(""); + } + + private Optional findSchoolNameById(String schoolId) { + try { + return Optional.ofNullable(schoolListReader.findSchoolById(schoolId).getName()); + } catch (Exception e) { + log.warn("Failed to find school with ID: {}", schoolId, e); + return Optional.empty(); + } } private void addStudentInformation(final Map info, final RegisteredUserDTO student) { @@ -1213,7 +1230,7 @@ private void addStudentInformation(final Map info, final Registe info.put("student_given_name", student.getGivenName() != null ? student.getGivenName() : ""); info.put("student_family_name", student.getFamilyName() != null ? student.getFamilyName() : ""); info.put("student_email", student.getEmail() != null ? student.getEmail() : ""); - info.put("student_school_name", student.getSchoolOther() != null ? student.getSchoolOther() : ""); + info.put("student_school_name", findUsersSchoolName(student)); info.put("student_school_urn", student.getSchoolId() != null ? student.getSchoolId() : ""); addStudentContextInformation(info, student); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java index 25fa9eae8b..b5540478d0 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java @@ -157,6 +157,7 @@ import uk.ac.cam.cl.dtg.segue.dao.associations.PgAssociationDataManager; import uk.ac.cam.cl.dtg.segue.dao.content.ContentMapperUtils; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.dao.schools.PgSchoolLookup; import uk.ac.cam.cl.dtg.segue.dao.schools.SchoolListReader; import uk.ac.cam.cl.dtg.segue.dao.userbadges.IUserBadgePersistenceManager; import uk.ac.cam.cl.dtg.segue.dao.userbadges.PgUserBadgePersistenceManager; @@ -1214,14 +1215,16 @@ private static IsaacSymbolicLogicValidator getSymbolicLogicValidator(final Prope * We want this to be a singleton as otherwise it may not be threadsafe for loading into same SearchProvider. * * @param provider The search provider. + * @param schoolFallbackLookup Fallback lookup for schools not in search index. * @return schoolList reader */ @Inject @Provides @Singleton - private SchoolListReader getSchoolListReader(final ISearchProvider provider) { + private SchoolListReader getSchoolListReader(final ISearchProvider provider, + final PgSchoolLookup schoolFallbackLookup) { if (null == schoolListReader) { - schoolListReader = new SchoolListReader(provider); + schoolListReader = new SchoolListReader(provider, schoolFallbackLookup); log.info("Creating singleton of SchoolListReader"); } return schoolListReader; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java new file mode 100644 index 0000000000..317c1b1da3 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java @@ -0,0 +1,90 @@ +/** + * Copyright 2025 Isaac Computer Science + *
+ * 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.dao.schools; + +import com.google.inject.Inject; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +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.dao.SegueDatabaseException; +import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; + +/** + * Temporary fallback lookup for schools that are no longer in Elasticsearch. + * This queries the schools_2022 table in the database to resolve school names + * for users who registered with schools that have since been removed from the + * current school list. + */ +public class PgSchoolLookup { + private static final Logger log = LoggerFactory.getLogger(PgSchoolLookup.class); + + private final PostgresSqlDb database; + + /** + * Constructor for PgSchoolLookup. + * + * @param database - the postgres datasource to use + */ + @Inject + public PgSchoolLookup(final PostgresSqlDb database) { + this.database = database; + } + + /** + * Look up a school by URN from the schools_2022 database table. + * This is a fallback for schools that no longer exist in the current Elasticsearch index. + * + * @param schoolUrn - the URN of the school to look up + * @return a School object if found, or null if not found + * @throws SegueDatabaseException if there is a database error + */ + public School findSchoolById(final String schoolUrn) throws SegueDatabaseException { + if (schoolUrn == null || schoolUrn.isEmpty()) { + return null; + } + + String query = "SELECT urn, school_name FROM schools_2022 WHERE urn = ?"; + + try (Connection conn = database.getDatabaseConnection(); + PreparedStatement pst = conn.prepareStatement(query)) { + pst.setString(1, schoolUrn); + + try (ResultSet results = pst.executeQuery()) { + if (results.next()) { + School school = new School(); + school.setUrn(results.getString("urn")); + school.setName(results.getString("school_name")); + school.setClosed(true); + school.setDataSource(School.SchoolDataSource.GOVERNMENT_UK); + log.debug("Found school {} in fallback database table schools_2022", schoolUrn); + return school; + } + } + } catch (SQLException e) { + String errorMsg = String.format("Error looking up school with URN %s from schools_2022 table", schoolUrn); + log.error(errorMsg, e); + throw new SegueDatabaseException(errorMsg, e); + } + + log.debug("School with URN {} not found in fallback database table schools_2022", schoolUrn); + return null; + } +} 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 88589b5b17..12cbfbf329 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 @@ -32,12 +32,14 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.api.client.util.Lists; import com.google.inject.Inject; +import jakarta.annotation.Nullable; import java.io.IOException; import java.util.List; import org.elasticsearch.ElasticsearchStatusException; 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.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.search.BasicSearchParameters; import uk.ac.cam.cl.dtg.segue.search.ISearchProvider; import uk.ac.cam.cl.dtg.segue.search.SegueSearchException; @@ -53,19 +55,23 @@ public class SchoolListReader { private static final Logger log = LoggerFactory.getLogger(SchoolListReader.class); private final ISearchProvider searchProvider; + private final PgSchoolLookup schoolFallbackLookup; private final ObjectMapper mapper = getSharedBasicObjectMapper(); private final String dataSourceModificationDate; /** - * SchoolListReader constructor. + * SchoolListReader constructor with database fallback for missing schools. * * @param searchProvider - search provider that can be used to put and retrieve school data. + * @param schoolFallbackLookup - optional fallback for schools not found in search index (can be null). */ @Inject - public SchoolListReader(final ISearchProvider searchProvider) { + public SchoolListReader(final ISearchProvider searchProvider, + @Nullable final PgSchoolLookup schoolFallbackLookup) { this.searchProvider = searchProvider; + this.schoolFallbackLookup = schoolFallbackLookup; String modificationDate; try { @@ -79,6 +85,16 @@ public SchoolListReader(final ISearchProvider searchProvider) { dataSourceModificationDate = modificationDate; } + /** + * SchoolListReader constructor without database fallback. + * Used by ETL module where database access is not available. + * + * @param searchProvider - search provider that can be used to put and retrieve school data. + */ + public SchoolListReader(final ISearchProvider searchProvider) { + this(searchProvider, null); + } + /** * findSchoolByNameOrPostCode. * @@ -146,7 +162,8 @@ public School findSchoolById(final String schoolURN) throws UnableToIndexSchools SCHOOL_URN_FIELDNAME.toLowerCase() + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX, schoolURN, null).getResults(); if (matchingSchoolList.isEmpty()) { - return null; + // Fallback to database lookup for schools that are no longer in the current index + return findSchoolByIdFromFallback(schoolURN); } if (matchingSchoolList.size() > 1) { @@ -157,6 +174,34 @@ public School findSchoolById(final String schoolURN) throws UnableToIndexSchools return mapper.readValue(matchingSchoolList.get(0), School.class); } + /** + * Fallback lookup for schools not found in Elasticsearch. + * This queries the schools_2022 database table for schools that have been + * removed from the current school list but are still referenced by user accounts. + * + * @param schoolURN - the URN of the school to look up + * @return the School if found in the fallback, or null if not found + */ + private School findSchoolByIdFromFallback(final String schoolURN) { + if (schoolFallbackLookup == null) { + log.debug("No fallback lookup configured, returning null for school URN: {}", + sanitiseExternalLogValue(schoolURN)); + return null; + } + + try { + School fallbackSchool = schoolFallbackLookup.findSchoolById(schoolURN); + if (fallbackSchool != null) { + log.info("Found school {} in fallback database table (not in current Elasticsearch index)", + sanitiseExternalLogValue(schoolURN)); + } + return fallbackSchool; + } catch (SegueDatabaseException e) { + log.error("Error looking up school {} from fallback database", sanitiseExternalLogValue(schoolURN), e); + return null; + } + } + /** * Ensure School List has been generated. diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookupTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookupTest.java new file mode 100644 index 0000000000..a8b5edb41a --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookupTest.java @@ -0,0 +1,187 @@ +package uk.ac.cam.cl.dtg.segue.dao.schools; + +import static org.easymock.EasyMock.anyString; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.reset; +import static org.easymock.EasyMock.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import uk.ac.cam.cl.dtg.isaac.dos.users.School; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; +import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; + +class PgSchoolLookupTest { + + private PgSchoolLookup pgSchoolLookup; + private PostgresSqlDb mockDatabase; + private Connection mockConnection; + private PreparedStatement mockPreparedStatement; + private ResultSet mockResultSet; + + @BeforeEach + void setUp() { + mockDatabase = createMock(PostgresSqlDb.class); + mockConnection = createMock(Connection.class); + mockPreparedStatement = createMock(PreparedStatement.class); + mockResultSet = createMock(ResultSet.class); + pgSchoolLookup = new PgSchoolLookup(mockDatabase); + } + + @AfterEach + void tearDown() { + reset(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + } + + @Nested + class FindSchoolByIdTests { + + @Test + void findSchoolById_WithValidUrn_ShouldReturnSchool() throws Exception { + // Arrange + String urn = "102354"; + String schoolName = "Sacred Heart of Mary Girls' School"; + + expect(mockDatabase.getDatabaseConnection()).andReturn(mockConnection); + expect(mockConnection.prepareStatement(anyString())).andReturn(mockPreparedStatement); + mockPreparedStatement.setString(1, urn); + expectLastCall(); + expect(mockPreparedStatement.executeQuery()).andReturn(mockResultSet); + + expect(mockResultSet.next()).andReturn(true); + expect(mockResultSet.getString("urn")).andReturn(urn); + expect(mockResultSet.getString("school_name")).andReturn(schoolName); + + mockResultSet.close(); + expectLastCall(); + mockPreparedStatement.close(); + expectLastCall(); + mockConnection.close(); + expectLastCall(); + + replay(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + + // Act + School result = pgSchoolLookup.findSchoolById(urn); + + // Assert + verify(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + assertNotNull(result); + assertEquals(urn, result.getUrn()); + assertEquals(schoolName, result.getName()); + assertTrue(result.isClosed()); + assertEquals(School.SchoolDataSource.GOVERNMENT_UK, result.getDataSource()); + } + + @Test + void findSchoolById_WithUnknownUrn_ShouldReturnNull() throws Exception { + // Arrange + String urn = "999999"; + + expect(mockDatabase.getDatabaseConnection()).andReturn(mockConnection); + expect(mockConnection.prepareStatement(anyString())).andReturn(mockPreparedStatement); + mockPreparedStatement.setString(1, urn); + expectLastCall(); + expect(mockPreparedStatement.executeQuery()).andReturn(mockResultSet); + + expect(mockResultSet.next()).andReturn(false); + + mockResultSet.close(); + expectLastCall(); + mockPreparedStatement.close(); + expectLastCall(); + mockConnection.close(); + expectLastCall(); + + replay(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + + // Act + School result = pgSchoolLookup.findSchoolById(urn); + + // Assert + verify(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + assertNull(result); + } + + @Test + void findSchoolById_WithNullUrn_ShouldReturnNull() throws Exception { + // Arrange - no database interaction expected + + replay(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + + // Act + School result = pgSchoolLookup.findSchoolById(null); + + // Assert + assertNull(result); + } + + @Test + void findSchoolById_WithEmptyUrn_ShouldReturnNull() throws Exception { + // Arrange - no database interaction expected + + replay(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + + // Act + School result = pgSchoolLookup.findSchoolById(""); + + // Assert + assertNull(result); + } + + @Test + void findSchoolById_WithDatabaseError_ShouldThrowException() throws Exception { + // Arrange + String urn = "102354"; + + expect(mockDatabase.getDatabaseConnection()).andThrow(new SQLException("Connection failed")); + + replay(mockDatabase); + + // Act & Assert + assertThrows(SegueDatabaseException.class, + () -> pgSchoolLookup.findSchoolById(urn)); + + verify(mockDatabase); + } + + @Test + void findSchoolById_WithQueryError_ShouldThrowException() throws Exception { + // Arrange + String urn = "102354"; + + expect(mockDatabase.getDatabaseConnection()).andReturn(mockConnection); + expect(mockConnection.prepareStatement(anyString())).andReturn(mockPreparedStatement); + mockPreparedStatement.setString(1, urn); + expectLastCall(); + expect(mockPreparedStatement.executeQuery()).andThrow(new SQLException("Query failed")); + + mockPreparedStatement.close(); + expectLastCall(); + mockConnection.close(); + expectLastCall(); + + replay(mockDatabase, mockConnection, mockPreparedStatement, mockResultSet); + + // Act & Assert + assertThrows(SegueDatabaseException.class, + () -> pgSchoolLookup.findSchoolById(urn)); + + verify(mockDatabase, mockConnection, mockPreparedStatement); + } + } +} diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReaderTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReaderTest.java new file mode 100644 index 0000000000..8f8016ee9b --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReaderTest.java @@ -0,0 +1,188 @@ +package uk.ac.cam.cl.dtg.segue.dao.schools; + +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.anyString; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.reset; +import static org.easymock.EasyMock.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.elasticsearch.action.get.GetResponse; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import uk.ac.cam.cl.dtg.isaac.dos.users.School; +import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; +import uk.ac.cam.cl.dtg.segue.search.BasicSearchParameters; +import uk.ac.cam.cl.dtg.segue.search.ISearchProvider; +import uk.ac.cam.cl.dtg.segue.search.SegueSearchException; + +class SchoolListReaderTest { + + private SchoolListReader schoolListReader; + private ISearchProvider mockSearchProvider; + private PgSchoolLookup mockFallbackLookup; + private GetResponse mockGetResponse; + + @BeforeEach + void setUp() throws SegueSearchException { + mockSearchProvider = createMock(ISearchProvider.class); + mockFallbackLookup = createMock(PgSchoolLookup.class); + mockGetResponse = createNiceMock(GetResponse.class); + + // Setup default behavior for index metadata lookup (called in constructor) + expect(mockGetResponse.getSource()).andReturn(Map.of("lastModified", "1234567890")).anyTimes(); + replay(mockGetResponse); + + expect(mockSearchProvider.getById(anyString(), anyString(), anyString())) + .andReturn(mockGetResponse).anyTimes(); + expect(mockSearchProvider.hasIndex(anyString(), anyString())).andReturn(true).anyTimes(); + } + + @AfterEach + void tearDown() { + reset(mockSearchProvider, mockFallbackLookup, mockGetResponse); + } + + @Nested + class FindSchoolByIdWithFallbackTests { + + @Test + void findSchoolById_WhenFoundInElasticsearch_ShouldReturnSchool() throws Exception { + // Arrange + String urn = "102354"; + String schoolJson = "{\"urn\":\"102354\",\"name\":\"Test School\",\"postcode\":\"AB1 2CD\",\"closed\":false}"; + + ResultsWrapper resultsWrapper = new ResultsWrapper<>(List.of(schoolJson), 1L); + expect(mockSearchProvider.findByExactMatch(anyObject(BasicSearchParameters.class), anyString(), anyString(), + anyObject())) + .andReturn(resultsWrapper); + + replay(mockSearchProvider, mockFallbackLookup); + + schoolListReader = new SchoolListReader(mockSearchProvider, mockFallbackLookup); + + // Act + School result = schoolListReader.findSchoolById(urn); + + // Assert + verify(mockSearchProvider); + assertNotNull(result); + assertEquals(urn, result.getUrn()); + assertEquals("Test School", result.getName()); + } + + @Test + void findSchoolById_WhenNotInElasticsearchButInFallback_ShouldReturnFromFallback() throws Exception { + // Arrange + String urn = "102354"; + School fallbackSchool = new School(); + fallbackSchool.setUrn(urn); + fallbackSchool.setName("Fallback School"); + fallbackSchool.setClosed(true); + + ResultsWrapper emptyResults = new ResultsWrapper<>(Collections.emptyList(), 0L); + expect(mockSearchProvider.findByExactMatch(anyObject(BasicSearchParameters.class), anyString(), anyString(), + anyObject())) + .andReturn(emptyResults); + + expect(mockFallbackLookup.findSchoolById(urn)).andReturn(fallbackSchool); + + replay(mockSearchProvider, mockFallbackLookup); + + schoolListReader = new SchoolListReader(mockSearchProvider, mockFallbackLookup); + + // Act + School result = schoolListReader.findSchoolById(urn); + + // Assert + verify(mockSearchProvider, mockFallbackLookup); + assertNotNull(result); + assertEquals(urn, result.getUrn()); + assertEquals("Fallback School", result.getName()); + assertTrue(result.isClosed()); + } + + @Test + void findSchoolById_WhenNotInElasticsearchAndNotInFallback_ShouldReturnNull() throws Exception { + // Arrange + String urn = "999999"; + + ResultsWrapper emptyResults = new ResultsWrapper<>(Collections.emptyList(), 0L); + expect(mockSearchProvider.findByExactMatch(anyObject(BasicSearchParameters.class), anyString(), anyString(), + anyObject())) + .andReturn(emptyResults); + + expect(mockFallbackLookup.findSchoolById(urn)).andReturn(null); + + replay(mockSearchProvider, mockFallbackLookup); + + schoolListReader = new SchoolListReader(mockSearchProvider, mockFallbackLookup); + + // Act + School result = schoolListReader.findSchoolById(urn); + + // Assert + verify(mockSearchProvider, mockFallbackLookup); + assertNull(result); + } + + @Test + void findSchoolById_WhenFallbackThrowsException_ShouldReturnNull() throws Exception { + // Arrange + String urn = "102354"; + + ResultsWrapper emptyResults = new ResultsWrapper<>(Collections.emptyList(), 0L); + expect(mockSearchProvider.findByExactMatch(anyObject(BasicSearchParameters.class), anyString(), anyString(), + anyObject())) + .andReturn(emptyResults); + + expect(mockFallbackLookup.findSchoolById(urn)).andThrow(new SegueDatabaseException("DB error")); + + replay(mockSearchProvider, mockFallbackLookup); + + schoolListReader = new SchoolListReader(mockSearchProvider, mockFallbackLookup); + + // Act + School result = schoolListReader.findSchoolById(urn); + + // Assert + verify(mockSearchProvider, mockFallbackLookup); + assertNull(result); + } + + @Test + void findSchoolById_WhenNoFallbackConfigured_ShouldReturnNull() throws Exception { + // Arrange + String urn = "102354"; + + ResultsWrapper emptyResults = new ResultsWrapper<>(Collections.emptyList(), 0L); + expect(mockSearchProvider.findByExactMatch(anyObject(BasicSearchParameters.class), anyString(), anyString(), + anyObject())) + .andReturn(emptyResults); + + replay(mockSearchProvider); + + // Create reader without fallback + schoolListReader = new SchoolListReader(mockSearchProvider); + + // Act + School result = schoolListReader.findSchoolById(urn); + + // Assert + verify(mockSearchProvider); + assertNull(result); + } + } +} From f574ce711c180086e19ee0c3838bc9011bf7ae60 Mon Sep 17 00:00:00 2001 From: Marius Date: Fri, 30 Jan 2026 17:57:39 +0800 Subject: [PATCH 2/2] 791 - School field missing for users after school list update --- .../cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java index 317c1b1da3..1cbcf0af44 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/PgSchoolLookup.java @@ -16,6 +16,8 @@ package uk.ac.cam.cl.dtg.segue.dao.schools; +import static uk.ac.cam.cl.dtg.util.LogUtils.sanitiseExternalLogValue; + import com.google.inject.Inject; import java.sql.Connection; import java.sql.PreparedStatement; @@ -74,17 +76,20 @@ public School findSchoolById(final String schoolUrn) throws SegueDatabaseExcepti school.setName(results.getString("school_name")); school.setClosed(true); school.setDataSource(School.SchoolDataSource.GOVERNMENT_UK); - log.debug("Found school {} in fallback database table schools_2022", schoolUrn); + log.debug("Found school {} in fallback database table schools_2022", + sanitiseExternalLogValue(schoolUrn)); return school; } } } catch (SQLException e) { - String errorMsg = String.format("Error looking up school with URN %s from schools_2022 table", schoolUrn); + String errorMsg = String.format("Error looking up school with URN %s from schools_2022 table", + sanitiseExternalLogValue(schoolUrn)); log.error(errorMsg, e); throw new SegueDatabaseException(errorMsg, e); } - log.debug("School with URN {} not found in fallback database table schools_2022", schoolUrn); + log.debug("School with URN {} not found in fallback database table schools_2022", + sanitiseExternalLogValue(schoolUrn)); return null; } }