From 8f5c57481dafd63eb8bee379222477d407508d54 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 24 Jan 2024 15:37:37 +0100 Subject: [PATCH 1/6] [kbss-cvut/record-manager-ui#71] Reduce the size of PatientRecordDto data. Also update code to the new OWL2Java constant naming strategy. --- .../cz/cvut/kbss/study/config/WebAppConfig.java | 2 +- .../cvut/kbss/study/dto/PatientRecordDto.java | 17 +++++++++-------- .../kbss/study/rest/InstitutionController.java | 7 +++---- .../kbss/study/rest/StatisticsController.java | 1 - .../study/environment/generator/Generator.java | 4 ++-- .../java/cz/cvut/kbss/study/model/UserTest.java | 2 +- .../cvut/kbss/study/model/qam/AnswerTest.java | 2 +- .../cvut/kbss/study/model/qam/QuestionTest.java | 2 +- 8 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/config/WebAppConfig.java b/src/main/java/cz/cvut/kbss/study/config/WebAppConfig.java index df53da8b..8034cf95 100644 --- a/src/main/java/cz/cvut/kbss/study/config/WebAppConfig.java +++ b/src/main/java/cz/cvut/kbss/study/config/WebAppConfig.java @@ -44,7 +44,7 @@ public ObjectMapper objectMapper() { */ public static ObjectMapper createJsonObjectMapper() { final ObjectMapper objectMapper = new ObjectMapper(); - objectMapper.setSerializationInclusion(JsonInclude.Include.ALWAYS); + objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); // Ignore UoW references injected into entities objectMapper.addMixIn(UnitOfWorkImpl.class, ManageableIgnoreMixin.class); diff --git a/src/main/java/cz/cvut/kbss/study/dto/PatientRecordDto.java b/src/main/java/cz/cvut/kbss/study/dto/PatientRecordDto.java index 1489461b..8dddf25e 100644 --- a/src/main/java/cz/cvut/kbss/study/dto/PatientRecordDto.java +++ b/src/main/java/cz/cvut/kbss/study/dto/PatientRecordDto.java @@ -4,6 +4,7 @@ import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.model.util.HasOwlKey; +import java.net.URI; import java.util.Date; @OWLClass(iri = Vocabulary.s_c_patient_record) @@ -21,8 +22,8 @@ public class PatientRecordDto extends AbstractEntity implements HasOwlKey { private String localName; @ParticipationConstraints(nonEmpty = true) - @OWLObjectProperty(iri = Vocabulary.s_p_has_author, fetch = FetchType.EAGER) - private User author; + @OWLObjectProperty(iri = Vocabulary.s_p_has_author) + private URI author; @OWLDataProperty(iri = Vocabulary.s_p_created) private Date dateCreated; @@ -30,8 +31,8 @@ public class PatientRecordDto extends AbstractEntity implements HasOwlKey { @OWLDataProperty(iri = Vocabulary.s_p_modified) private Date lastModified; - @OWLObjectProperty(iri = Vocabulary.s_p_has_last_editor, fetch = FetchType.EAGER) - private User lastModifiedBy; + @OWLObjectProperty(iri = Vocabulary.s_p_has_last_editor) + private URI lastModifiedBy; @OWLObjectProperty(iri = Vocabulary.s_p_was_treated_at, fetch = FetchType.EAGER) private Institution institution; @@ -58,11 +59,11 @@ public void setLocalName(String localName) { this.localName = localName; } - public User getAuthor() { + public URI getAuthor() { return author; } - public void setAuthor(User author) { + public void setAuthor(URI author) { this.author = author; } @@ -82,11 +83,11 @@ public void setLastModified(Date lastModified) { this.lastModified = lastModified; } - public User getLastModifiedBy() { + public URI getLastModifiedBy() { return lastModifiedBy; } - public void setLastModifiedBy(User lastModifiedBy) { + public void setLastModifiedBy(URI lastModifiedBy) { this.lastModifiedBy = lastModifiedBy; } diff --git a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java index 2ac7fc0f..54f8f340 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java @@ -8,7 +8,6 @@ import cz.cvut.kbss.study.security.SecurityConstants; import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.PatientRecordService; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -16,7 +15,7 @@ import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.*; -import java.util.Collections; +import java.util.Comparator; import java.util.List; @RestController @@ -38,7 +37,7 @@ public InstitutionController(InstitutionService institutionService, @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getAllInstitutions() { final List institutions = institutionService.findAll(); - Collections.sort(institutions, (a, b) -> a.getName().compareTo(b.getName())); + institutions.sort(Comparator.comparing(Institution::getName)); return institutions; } @@ -85,7 +84,7 @@ public void updateInstitution(@PathVariable("key") String key, @RequestBody Inst throw new BadRequestException("The passed institution's key is different from the specified one."); } final Institution original = findInternal(key); - assert original != null; + institutionService.update(institution); if (LOG.isTraceEnabled()) { LOG.trace("Institution {} successfully updated.", institution); diff --git a/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java b/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java index 86fcf065..ca1b70d5 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java @@ -2,7 +2,6 @@ import cz.cvut.kbss.study.security.SecurityConstants; import cz.cvut.kbss.study.service.StatisticsService; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.GetMapping; diff --git a/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java b/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java index 2a8b5693..93f8a21c 100644 --- a/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java +++ b/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java @@ -26,7 +26,7 @@ private Generator() { * @return Random URI */ public static URI generateUri() { - return URI.create(Vocabulary.ONTOLOGY_IRI_record_manager + "/randomInstance" + randomInt()); + return URI.create(Vocabulary.ONTOLOGY_IRI_RECORD_MANAGER + "/randomInstance" + randomInt()); } /** @@ -183,7 +183,7 @@ public static PatientRecord generatePatientRecord(User author) { public static PatientRecordDto generatePatientRecordDto(User author) { final PatientRecordDto rec = new PatientRecordDto(); rec.setLocalName("RandomRecordDto" + randomInt()); - rec.setAuthor(author); + rec.setAuthor(author.getUri()); rec.setUri(generateUri()); rec.setInstitution(author.getInstitution()); return rec; diff --git a/src/test/java/cz/cvut/kbss/study/model/UserTest.java b/src/test/java/cz/cvut/kbss/study/model/UserTest.java index 759cfce6..3b384e27 100644 --- a/src/test/java/cz/cvut/kbss/study/model/UserTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/UserTest.java @@ -82,7 +82,7 @@ public void generateUriThrowsIllegalStateForEmptyLastName() { @Test public void generateUriDoesNothingIfTheUriIsAlreadySet() { - final String uri = Vocabulary.ONTOLOGY_IRI_record_manager + "/test"; + final String uri = Vocabulary.ONTOLOGY_IRI_RECORD_MANAGER + "/test"; user.setUri(URI.create(uri)); user.generateUri(); assertEquals(uri, user.getUri().toString()); diff --git a/src/test/java/cz/cvut/kbss/study/model/qam/AnswerTest.java b/src/test/java/cz/cvut/kbss/study/model/qam/AnswerTest.java index fff4d364..8659c130 100644 --- a/src/test/java/cz/cvut/kbss/study/model/qam/AnswerTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/qam/AnswerTest.java @@ -14,7 +14,7 @@ public void copyConstructorsCopiesValuesAndTypesNoUri() { final Answer a = new Answer(); a.setTextValue("Cough"); a.setCodeValue(Generator.generateUri()); - a.getTypes().add(Vocabulary.ONTOLOGY_IRI_record_manager + "/infectious-disease/"); + a.getTypes().add(Vocabulary.ONTOLOGY_IRI_RECORD_MANAGER + "/infectious-disease/"); final Answer res = new Answer(a); assertNull(res.getUri()); diff --git a/src/test/java/cz/cvut/kbss/study/model/qam/QuestionTest.java b/src/test/java/cz/cvut/kbss/study/model/qam/QuestionTest.java index 7980c937..3059e75a 100644 --- a/src/test/java/cz/cvut/kbss/study/model/qam/QuestionTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/qam/QuestionTest.java @@ -13,7 +13,7 @@ public class QuestionTest { public void copyConstructorCopiesSubQuestions() { final Question q = new Question(); q.setUri(Generator.generateUri()); - q.getTypes().add(Vocabulary.ONTOLOGY_IRI_record_manager + "/infectious-disease/"); + q.getTypes().add(Vocabulary.ONTOLOGY_IRI_RECORD_MANAGER + "/infectious-disease/"); for (int i = 0; i < Generator.randomInt(10); i++) { final Question child = new Question(); child.setUri(Generator.generateUri()); From 929aecde76229d2bcd97b844696f2f891d0a54f7 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Tue, 30 Jan 2024 16:39:13 +0100 Subject: [PATCH 2/6] [Upd] Update dependencies to work with the latest JOPA 2.0.0-SNAPSHOT build. Required updated jopa-spring-transaction. --- pom.xml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index b0cfd757..a0c35958 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ org.springframework.boot spring-boot-starter-parent - 3.1.4 + 3.2.2 record-manager @@ -48,7 +48,6 @@ ${jdk.version} 2.0.0-SNAPSHOT - 1.9.20 4.11.0 1.5.5.Final @@ -98,7 +97,7 @@ com.github.ledsoft jopa-spring-transaction - 0.2.0 + 0.3.0-SNAPSHOT From a5f22d16303bd20672879218ee563de79b6466ca Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 31 Jan 2024 12:12:55 +0100 Subject: [PATCH 3/6] [kbss-cvut/record-manager-ui#71] Implement filtering, paging and sorting in PatientRecordDao. --- pom.xml | 4 + .../persistence/dao/PatientRecordDao.java | 110 +++++++++++--- .../persistence/dao/util/RecordSort.java | 36 +++++ .../study/service/PatientRecordService.java | 26 +++- .../RepositoryPatientRecordService.java | 16 +- .../persistence/dao/PatientRecordDaoTest.java | 139 +++++++++++++++--- 6 files changed, 283 insertions(+), 48 deletions(-) create mode 100644 src/main/java/cz/cvut/kbss/study/persistence/dao/util/RecordSort.java diff --git a/pom.xml b/pom.xml index a0c35958..5ba25417 100644 --- a/pom.xml +++ b/pom.xml @@ -70,6 +70,10 @@ org.springframework.boot spring-boot-starter-oauth2-resource-server + + org.springframework.data + spring-data-commons + org.apache.httpcomponents.client5 diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDao.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDao.java index e185f217..cb72d921 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDao.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDao.java @@ -16,8 +16,13 @@ import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.dao.util.QuestionSaver; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; +import cz.cvut.kbss.study.persistence.dao.util.RecordSort; import cz.cvut.kbss.study.util.Constants; import cz.cvut.kbss.study.util.IdentificationUtils; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.stereotype.Repository; import java.math.BigInteger; @@ -177,40 +182,85 @@ public void requireUniqueNonEmptyLocalName(PatientRecord entity) { em.clear(); } + /** + * Retrieves DTOs of records matching the specified filtering criteria. + *

+ * Note that since the record modification is tracked by a timestamp and the filter uses dates, this method uses + * beginning of the min date and end of the max date. + *

+ * The returned page contains also information about total number of matching records. + * + * @param filters Record filtering criteria + * @param pageSpec Specification of page and sorting + * @return Page with matching records + * @see #findAllRecordsFull(RecordFilterParams, Pageable) + */ + public Page findAllRecords(RecordFilterParams filters, Pageable pageSpec) { + Objects.requireNonNull(filters); + Objects.requireNonNull(pageSpec); + return findRecords(filters, pageSpec, PatientRecordDto.class); + } + /** * Retrieves records matching the specified filtering criteria. *

* Note that since the record modification is tracked by a timestamp and the filter uses dates, this method uses * beginning of the min date and end of the max date. + *

+ * The returned page contains also information about total number of matching records. * - * @param filterParams Record filtering criteria - * @return List of matching records + * @param filters Record filtering criteria + * @param pageSpec Specification of page and sorting + * @return Page with matching records + * @see #findAllRecords(RecordFilterParams, Pageable) */ - public List findAllFull(RecordFilterParams filterParams) { - Objects.requireNonNull(filterParams); + public Page findAllRecordsFull(RecordFilterParams filters, Pageable pageSpec) { + Objects.requireNonNull(filters); + Objects.requireNonNull(pageSpec); + return findRecords(filters, pageSpec, PatientRecord.class); + } + + private Page findRecords(RecordFilterParams filters, Pageable pageSpec, Class resultClass) { + final Map queryParams = new HashMap<>(); + final String whereClause = constructWhereClause(filters, queryParams); + final String queryString = "SELECT ?r WHERE " + whereClause + resolveOrderBy(pageSpec.getSortOr(RecordSort.defaultSort())); + final TypedQuery query = em.createNativeQuery(queryString, resultClass); + setQueryParameters(query, queryParams); + if (pageSpec.isPaged()) { + query.setFirstResult((int) pageSpec.getOffset()); + query.setMaxResults(pageSpec.getPageSize()); + } + final List records = query.getResultList(); + final TypedQuery countQuery = em.createNativeQuery("SELECT (COUNT(?r) as ?cnt) WHERE " + whereClause, Integer.class); + setQueryParameters(countQuery, queryParams); + final Integer totalCount = countQuery.getSingleResult(); + return new PageImpl<>(records, pageSpec, totalCount); + } + + private void setQueryParameters(TypedQuery query, Map queryParams) { + query.setParameter("type", typeUri) + .setParameter("hasPhase", URI.create(Vocabulary.s_p_has_phase)) + .setParameter("hasInstitution", + URI.create(Vocabulary.s_p_was_treated_at)) + .setParameter("hasKey", URI.create(Vocabulary.s_p_key)) + .setParameter("hasCreatedDate", URI.create(Vocabulary.s_p_created)) + .setParameter("hasLastModified", URI.create(Vocabulary.s_p_modified)); + queryParams.forEach(query::setParameter); + } + + private static String constructWhereClause(RecordFilterParams filters, Map queryParams) { // Could not use Criteria API because it does not support OPTIONAL - String queryString = "SELECT ?r WHERE {" + + String whereClause = "{" + "?r a ?type ; " + "?hasCreatedDate ?created ; " + "?hasInstitution ?institution . " + "?institution ?hasKey ?institutionKey ." + "OPTIONAL { ?r ?hasPhase ?phase . } " + "OPTIONAL { ?r ?hasLastModified ?lastModified . } " + - "BIND (IF (BOUND(?lastModified), ?lastModified, ?created) AS ?edited) "; - final Map queryParams = new HashMap<>(); - queryString += mapParamsToQuery(filterParams, queryParams); - queryString += "} ORDER BY ?edited"; - - final TypedQuery query = em.createNativeQuery(queryString, PatientRecord.class) - .setParameter("type", typeUri) - .setParameter("hasPhase", URI.create(Vocabulary.s_p_has_phase)) - .setParameter("hasInstitution", - URI.create(Vocabulary.s_p_was_treated_at)) - .setParameter("hasKey", URI.create(Vocabulary.s_p_key)) - .setParameter("hasCreatedDate", URI.create(Vocabulary.s_p_created)) - .setParameter("hasLastModified", URI.create(Vocabulary.s_p_modified)); - queryParams.forEach(query::setParameter); - return query.getResultList(); + "BIND (COALESCE(?lastModified, ?created) AS ?date) "; + whereClause += mapParamsToQuery(filters, queryParams); + whereClause += "}"; + return whereClause; } private static String mapParamsToQuery(RecordFilterParams filterParams, Map queryParams) { @@ -218,11 +268,11 @@ private static String mapParamsToQuery(RecordFilterParams filterParams, Map queryParams.put("institutionKey", new LangString(key, Constants.PU_LANGUAGE))); filterParams.getMinModifiedDate().ifPresent(date -> { - filters.add("FILTER (?edited >= ?minDate)"); + filters.add("FILTER (?date >= ?minDate)"); queryParams.put("minDate", date.atStartOfDay(ZoneOffset.UTC).toInstant()); }); filterParams.getMaxModifiedDate().ifPresent(date -> { - filters.add("FILTER (?edited < ?maxDate)"); + filters.add("FILTER (?date < ?maxDate)"); queryParams.put("maxDate", date.plusDays(1).atStartOfDay(ZoneOffset.UTC).toInstant()); }); if (!filterParams.getPhaseIds().isEmpty()) { @@ -232,4 +282,20 @@ private static String mapParamsToQuery(RecordFilterParams filterParams, Map SORTING_PROPERTIES = Set.of(SORT_DATE_PROPERTY); + + private RecordSort() { + throw new AssertionError(); + } + + /** + * Returns the default sort for retrieving records. + *

+ * By default, records are sorted by date of last modification/creation in descending order. + * + * @return Default sort + */ + public static Sort defaultSort() { + return Sort.by(Sort.Order.desc("date")); + } +} diff --git a/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java index 6e57840f..bb02c0e7 100644 --- a/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java @@ -7,6 +7,8 @@ import cz.cvut.kbss.study.model.RecordPhase; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import java.util.List; @@ -43,6 +45,26 @@ public interface PatientRecordService extends BaseService { */ List findAllRecords(); + /** + * Gets records corresponding to the specified filtering, paging, and sorting criteria. + * + * @param filters Record filtering criteria + * @param pageSpec Specification of page and sorting to retrieve + * @return List of matching record DTOs + * @see #findAllFull(RecordFilterParams, Pageable) + */ + Page findAll(RecordFilterParams filters, Pageable pageSpec); + + /** + * Gets records corresponding to the specified filtering, paging, and sorting criteria. + * + * @param filters Record filtering criteria + * @param pageSpec Specification of page and sorting to retrieve + * @return List of matching records + * @see #findAll(RecordFilterParams, Pageable) + */ + Page findAllFull(RecordFilterParams filters, Pageable pageSpec); + /** * Finds all records that match the specified parameters. *

@@ -64,8 +86,8 @@ public interface PatientRecordService extends BaseService { * records. *

* If the current user is an admin, the import procedure retains provenance data of the record. Otherwise, the - * current user is set as the record's author. Also, if the current user is not an admin, the phase of all - * the imported records is set to {@link RecordPhase#open}, for admin, the phase of the records is retained. + * current user is set as the record's author. Also, if the current user is not an admin, the phase of all the + * imported records is set to {@link RecordPhase#open}, for admin, the phase of the records is retained. * * @param records Records to import * @return Instance representing the import result diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java index cc20e7ba..bcf59ece 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java @@ -16,6 +16,8 @@ import cz.cvut.kbss.study.util.Utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -66,10 +68,22 @@ public List findAllRecords() { return recordDao.findAllRecords(); } + @Override + public Page findAll(RecordFilterParams filters, Pageable pageSpec) { + // TODO + return null; + } + + @Transactional(readOnly = true) + @Override + public Page findAllFull(RecordFilterParams filters, Pageable pageSpec) { + return recordDao.findAllRecordsFull(filters, pageSpec); + } + @Transactional(readOnly = true) @Override public List findAllFull(RecordFilterParams filterParams) { - return recordDao.findAllFull(filterParams); + return findAllFull(filterParams, Pageable.unpaged()).getContent(); } @Override diff --git a/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java b/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java index 93ff7cb7..dac6c38f 100644 --- a/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java +++ b/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java @@ -15,17 +15,24 @@ import cz.cvut.kbss.study.persistence.BaseDaoTestRunner; import cz.cvut.kbss.study.persistence.dao.util.QuestionSaver; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; +import cz.cvut.kbss.study.persistence.dao.util.RecordSort; import cz.cvut.kbss.study.util.IdentificationUtils; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import java.net.URI; import java.time.LocalDate; import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.List; +import java.util.ListIterator; import java.util.Set; import java.util.stream.IntStream; @@ -34,6 +41,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; public class PatientRecordDaoTest extends BaseDaoTestRunner { @@ -255,7 +263,10 @@ private void persistRecordWithIdentification(PatientRecord record) { @Test void findAllFullReturnsRecordsMatchingSpecifiedDatePeriod() { final User author = generateAuthorWithInstitution(); - final List allRecords = generateRecordsForAuthor(author); + final List allRecords = generateRecordsForAuthor(author, 5); + for (int i = 0; i < allRecords.size(); i++) { + allRecords.get(i).setDateCreated(new Date(System.currentTimeMillis() - i * Environment.MILLIS_PER_DAY)); + } transactional(() -> allRecords.forEach(this::persistRecordWithIdentification)); final LocalDate minDate = LocalDate.now().minusDays(3); final LocalDate maxDate = LocalDate.now().minusDays(1); @@ -265,23 +276,26 @@ void findAllFullReturnsRecordsMatchingSpecifiedDatePeriod() { return !modifiedDate.isBefore(minDate) && !modifiedDate.isAfter(maxDate); }).toList(); - final List result = - sut.findAllFull(new RecordFilterParams(null, minDate, maxDate, Collections.emptySet())); + final Page result = + sut.findAllRecordsFull(new RecordFilterParams(null, minDate, maxDate, Collections.emptySet()), + Pageable.unpaged()); assertFalse(result.isEmpty()); - assertThat(result, containsSameEntities(expected)); + assertThat(result.getContent(), containsSameEntities(expected)); } - private List generateRecordsForAuthor(User author) { - return IntStream.range(0, 5).mapToObj(i -> { - final PatientRecord r = Generator.generatePatientRecord(author); - if (Generator.randomBoolean()) { - r.setDateCreated(new Date(System.currentTimeMillis() - i * Environment.MILLIS_PER_DAY)); - } else { - r.setDateCreated(new Date(System.currentTimeMillis() - 365 * Environment.MILLIS_PER_DAY)); - r.setLastModified(new Date(System.currentTimeMillis() - i * Environment.MILLIS_PER_DAY)); - } - return r; - }).toList(); + private List generateRecordsForAuthor(User author, int count) { + return IntStream.range(0, count).mapToObj(i -> { + final PatientRecord r = Generator.generatePatientRecord(author); + if (Generator.randomBoolean()) { + r.setDateCreated(new Date(System.currentTimeMillis() - 30 * i * Environment.MILLIS_PER_DAY)); + } else { + r.setDateCreated(new Date(System.currentTimeMillis() - 30 * i * Environment.MILLIS_PER_DAY)); + r.setLastModified(new Date(System.currentTimeMillis() - i * Environment.MILLIS_PER_DAY)); + } + return r; + }).sorted(Comparator.comparing( + (PatientRecord r) -> r.getLastModified() != null ? r.getLastModified() : r.getDateCreated()).reversed()) + .toList(); } @Test @@ -289,10 +303,10 @@ void findAllFullReturnsRecordsMatchingSpecifiedDatePeriodAndInstitution() { final User authorOne = generateAuthorWithInstitution(); final Institution institution = authorOne.getInstitution(); final User authorTwo = generateAuthorWithInstitution(); - final List allRecords = new ArrayList<>(generateRecordsForAuthor(authorOne)); - allRecords.addAll(generateRecordsForAuthor(authorTwo)); + final List allRecords = new ArrayList<>(generateRecordsForAuthor(authorOne, 5)); + allRecords.addAll(generateRecordsForAuthor(authorTwo, 5)); transactional(() -> allRecords.forEach(this::persistRecordWithIdentification)); - final LocalDate minDate = LocalDate.now().minusDays(3); + final LocalDate minDate = LocalDate.now().minusDays(31); final LocalDate maxDate = LocalDate.now().minusDays(1); final List expected = allRecords.stream().filter(r -> { final Date modified = r.getLastModified() != null ? r.getLastModified() : r.getDateCreated(); @@ -301,16 +315,17 @@ void findAllFullReturnsRecordsMatchingSpecifiedDatePeriodAndInstitution() { .equals(institution.getUri()); }).toList(); - final List result = - sut.findAllFull(new RecordFilterParams(institution.getKey(), minDate, maxDate, Collections.emptySet())); + final Page result = + sut.findAllRecordsFull(new RecordFilterParams(institution.getKey(), minDate, maxDate, Collections.emptySet()), + Pageable.unpaged()); assertFalse(result.isEmpty()); - assertThat(result, containsSameEntities(expected)); + assertThat(result.getContent(), containsSameEntities(expected)); } @Test void findAllFullReturnsRecordsMatchingSpecifiedPhase() { final User author = generateAuthorWithInstitution(); - final List allRecords = generateRecordsForAuthor(author); + final List allRecords = generateRecordsForAuthor(author, 5); transactional(() -> allRecords.forEach(r -> { r.setPhase(RecordPhase.values()[Generator.randomInt(RecordPhase.values().length)]); persistRecordWithIdentification(r); @@ -319,11 +334,89 @@ void findAllFullReturnsRecordsMatchingSpecifiedPhase() { final RecordFilterParams filterParams = new RecordFilterParams(); filterParams.setPhaseIds(Set.of(phase.getIri())); - final List result = sut.findAllFull(filterParams); + final Page result = sut.findAllRecordsFull(filterParams, Pageable.unpaged()); assertFalse(result.isEmpty()); result.forEach(res -> assertEquals(phase, res.getPhase())); } + @Test + void findAllFullReturnsRecordsMatchingSpecifiedPage() { + final User author = generateAuthorWithInstitution(); + final List allRecords = generateRecordsForAuthor(author, 10); + transactional(() -> allRecords.forEach(this::persistRecordWithIdentification)); + final int pageSize = 5; + + final Page result = sut.findAllRecordsFull(new RecordFilterParams(), PageRequest.of(1, pageSize)); + assertFalse(result.isEmpty()); + assertEquals(pageSize, result.getNumberOfElements()); + assertThat(result.getContent(), containsSameEntities(allRecords.subList(pageSize, allRecords.size()))); + } + + @Test + void findAllFullReturnsRecordsSortedAccordingToSortSpecification() { + final User author = generateAuthorWithInstitution(); + final List allRecords = generateRecordsForAuthor(author, 5); + transactional(() -> allRecords.forEach(this::persistRecordWithIdentification)); + final Page result = + sut.findAllRecordsFull(new RecordFilterParams(), PageRequest.of(0, allRecords.size(), Sort.Direction.ASC, + RecordSort.SORT_DATE_PROPERTY)); + assertFalse(result.isEmpty()); + assertEquals(allRecords.size(), result.getNumberOfElements()); + final ListIterator itExp = allRecords.listIterator(allRecords.size()); + final ListIterator itAct = result.getContent().listIterator(); + while (itExp.hasPrevious() && itAct.hasNext()) { + assertEquals(itExp.previous().getUri(), itAct.next().getUri()); + } + } + + @Test + void findAllFullThrowsIllegalArgumentExceptionForUnsupportedSortProperty() { + assertThrows(IllegalArgumentException.class, + () -> sut.findAllRecordsFull(new RecordFilterParams(), PageRequest.of(0, 10, Sort.Direction.ASC, + "unknownProperty"))); + } + + @Test + void findAllFullReturnsPageWithTotalNumberOfMatchingRecords() { + final User author = generateAuthorWithInstitution(); + final List allRecords = generateRecordsForAuthor(author, 10); + transactional(() -> allRecords.forEach(this::persistRecordWithIdentification)); + final int pageSize = 5; + + final Page result = sut.findAllRecordsFull(new RecordFilterParams(), PageRequest.of(1, pageSize)); + assertFalse(result.isEmpty()); + assertEquals(pageSize, result.getNumberOfElements()); + assertEquals(allRecords.size(), result.getTotalElements()); + } + + @Test + void findAllRecordsReturnsPageWithMatchingRecords() { + final User author = generateAuthorWithInstitution(); + final int totalCount = 10; + final List allRecords = generateRecordsForAuthor(author, totalCount); + for (int i = 0; i < allRecords.size(); i++) { + allRecords.get(i).setDateCreated(new Date(System.currentTimeMillis() - i * Environment.MILLIS_PER_DAY)); + } + transactional(() -> allRecords.forEach(this::persistRecordWithIdentification)); + final LocalDate minDate = LocalDate.now().minusDays(3); + final LocalDate maxDate = LocalDate.now().minusDays(1); + final List allMatching = allRecords.stream().filter(r -> { + final Date modified = r.getLastModified() != null ? r.getLastModified() : r.getDateCreated(); + final LocalDate modifiedDate = modified.toInstant().atZone(ZoneOffset.UTC).toLocalDate(); + return !modifiedDate.isBefore(minDate) && !modifiedDate.isAfter(maxDate); + }).sorted(Comparator.comparing(r -> r.getLastModified() != null ? r.getLastModified() : r.getDateCreated())).toList(); + final int pageSize = 3; + + final Page result = + sut.findAllRecords(new RecordFilterParams(null, minDate, maxDate, Collections.emptySet()), + PageRequest.of(0, pageSize, Sort.Direction.ASC, RecordSort.SORT_DATE_PROPERTY)); + assertEquals(Math.min(pageSize, allMatching.size()), result.getNumberOfElements()); + assertEquals(allMatching.size(), result.getTotalElements()); + for (int i = 0; i < result.getNumberOfElements(); i++) { + assertEquals(allMatching.get(i).getUri(), result.getContent().get(i).getUri()); + } + } + @Test void persistDoesNotGenerateIdentificationWhenRecordAlreadyHasIt() { final User author = generateAuthorWithInstitution(); From 7f232735b1ec87d68cd7c4541746468454b7a951 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 31 Jan 2024 13:00:58 +0100 Subject: [PATCH 4/6] [kbss-cvut/record-manager-ui#71] Simplify PatientRecordService API, use generic record filtering to get records from institution. --- .../dao/util/RecordFilterParams.java | 4 ++ .../study/rest/InstitutionController.java | 19 +++++-- .../study/rest/PatientRecordController.java | 26 +++------- .../study/rest/util/RecordFilterMapper.java | 15 +++++- .../study/service/PatientRecordService.java | 36 -------------- .../RepositoryPatientRecordService.java | 27 +--------- .../study/rest/InstitutionControllerTest.java | 42 ++++++++++------ .../rest/PatientRecordControllerTest.java | 49 +++++++++---------- src/test/resources/logback-test.xml | 7 +-- 9 files changed, 92 insertions(+), 133 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/util/RecordFilterParams.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/util/RecordFilterParams.java index 33cbc163..337934a2 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/dao/util/RecordFilterParams.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/util/RecordFilterParams.java @@ -22,6 +22,10 @@ public class RecordFilterParams { public RecordFilterParams() { } + public RecordFilterParams(String institutionKey) { + this.institutionKey = institutionKey; + } + // This one mainly is for test data setup public RecordFilterParams(String institutionKey, LocalDate minModifiedDate, LocalDate maxModifiedDate, Set phaseIds) { diff --git a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java index 54f8f340..d96e4bae 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java @@ -8,16 +8,27 @@ import cz.cvut.kbss.study.security.SecurityConstants; import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.PatientRecordService; +import org.springframework.data.domain.Pageable; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; import java.util.Comparator; import java.util.List; +import static cz.cvut.kbss.study.rest.util.RecordFilterMapper.constructRecordFilter; + @RestController @PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") @RequestMapping("/institutions") @@ -59,8 +70,9 @@ private Institution findInternal(String key) { @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isRecordInUsersInstitution(#key)") @GetMapping(value = "/{key}/patients", produces = MediaType.APPLICATION_JSON_VALUE) public List getTreatedPatientRecords(@PathVariable("key") String key) { - final Institution institution = findInternal(key); - return recordService.findByInstitution(institution); + final Institution inst = findInternal(key); + assert inst != null; + return recordService.findAll(constructRecordFilter("institution", key), Pageable.unpaged()).getContent(); } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") @@ -84,6 +96,7 @@ public void updateInstitution(@PathVariable("key") String key, @RequestBody Inst throw new BadRequestException("The passed institution's key is different from the specified one."); } final Institution original = findInternal(key); + assert original != null; institutionService.update(institution); if (LOG.isTraceEnabled()) { diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index fa0c4bbe..3ba7ee48 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -3,15 +3,14 @@ import cz.cvut.kbss.study.dto.PatientRecordDto; import cz.cvut.kbss.study.dto.RecordImportResult; import cz.cvut.kbss.study.exception.NotFoundException; -import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.RecordPhase; import cz.cvut.kbss.study.rest.exception.BadRequestException; import cz.cvut.kbss.study.rest.util.RecordFilterMapper; import cz.cvut.kbss.study.rest.util.RestUtils; import cz.cvut.kbss.study.security.SecurityConstants; -import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.PatientRecordService; +import org.springframework.data.domain.Pageable; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -38,36 +37,25 @@ public class PatientRecordController extends BaseController { private final PatientRecordService recordService; - private final InstitutionService institutionService; - - public PatientRecordController(PatientRecordService recordService, InstitutionService institutionService) { + public PatientRecordController(PatientRecordService recordService) { this.recordService = recordService; - this.institutionService = institutionService; } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getRecords( - @RequestParam(value = "institution", required = false) String institutionKey) { - return institutionKey != null ? recordService.findByInstitution(getInstitution(institutionKey)) : - recordService.findAllRecords(); - } - - private Institution getInstitution(String institutionKey) { - final Institution institution = institutionService.findByKey(institutionKey); - if (institution == null) { - throw NotFoundException.create("Institution", institutionKey); - } - return institution; + @RequestParam(value = "institution", required = false) String institutionKey, + @RequestParam MultiValueMap params) { + return recordService.findAll(RecordFilterMapper.constructRecordFilter(params), Pageable.unpaged()).getContent(); } @PreAuthorize( "hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(value = "/export", produces = MediaType.APPLICATION_JSON_VALUE) public List exportRecords( - @RequestParam(name = "institutionKey", required = false) String institutionKey, + @RequestParam(name = "institution", required = false) String institutionKey, @RequestParam MultiValueMap params) { - return recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params)); + return recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), Pageable.unpaged()).getContent(); } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isRecordInUsersInstitution(#key)") diff --git a/src/main/java/cz/cvut/kbss/study/rest/util/RecordFilterMapper.java b/src/main/java/cz/cvut/kbss/study/rest/util/RecordFilterMapper.java index f6c0abb9..f89e5a75 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/util/RecordFilterMapper.java +++ b/src/main/java/cz/cvut/kbss/study/rest/util/RecordFilterMapper.java @@ -4,6 +4,7 @@ import cz.cvut.kbss.study.rest.exception.BadRequestException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import java.time.LocalDate; @@ -11,6 +12,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -29,11 +31,22 @@ public class RecordFilterMapper { private static final String PHASE_ID_PARAM = "phase"; + /** + * Maps the specified single parameter and value to a new {@link RecordFilterParams} instance. + * + * @param param Parameter name + * @param value Parameter value + * @return New {@code RecordFilterParams} instance + */ + public static RecordFilterParams constructRecordFilter(String param, String value) { + return constructRecordFilter(new LinkedMultiValueMap<>(Map.of(param, List.of(value)))); + } + /** * Maps the specified parameters to a new {@link RecordFilterParams} instance. * * @param params Request parameters to map - * @return New {@code RecordFilter} instance + * @return New {@code RecordFilterParams} instance */ public static RecordFilterParams constructRecordFilter(MultiValueMap params) { Objects.requireNonNull(params); diff --git a/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java index bb02c0e7..91322af1 100644 --- a/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java @@ -2,10 +2,8 @@ import cz.cvut.kbss.study.dto.PatientRecordDto; import cz.cvut.kbss.study.dto.RecordImportResult; -import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.RecordPhase; -import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -22,29 +20,6 @@ public interface PatientRecordService extends BaseService { */ PatientRecord findByKey(String key); - /** - * Gets records of patients treated at the specified institution. - * - * @param institution The institution to filter by - * @return Records of matching patients - */ - List findByInstitution(Institution institution); - - /** - * Gets records of patients created by specified author. - * - * @param author The author to filter by - * @return Records of matching patients - */ - List findByAuthor(User author); - - /** - * Gets records of all patients. - * - * @return Records of matching patients - */ - List findAllRecords(); - /** * Gets records corresponding to the specified filtering, paging, and sorting criteria. * @@ -65,17 +40,6 @@ public interface PatientRecordService extends BaseService { */ Page findAllFull(RecordFilterParams filters, Pageable pageSpec); - /** - * Finds all records that match the specified parameters. - *

- * In contrast to {@link #findAll()}, this method returns full records, not DTOs. - * - * @param filterParams Record filtering criteria - * @return List of matching records - * @see #findAllRecords() - */ - List findAllFull(RecordFilterParams filterParams); - /** * Imports the specified records. *

diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java index bcf59ece..8aaf2c9c 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java @@ -3,7 +3,6 @@ import cz.cvut.kbss.study.dto.PatientRecordDto; import cz.cvut.kbss.study.dto.RecordImportResult; import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException; -import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.RecordPhase; import cz.cvut.kbss.study.model.User; @@ -51,27 +50,9 @@ protected OwlKeySupportingDao getPrimaryDao() { } @Transactional(readOnly = true) - @Override - public List findByInstitution(Institution institution) { - return recordDao.findByInstitution(institution); - } - - @Transactional(readOnly = true) - @Override - public List findByAuthor(User user) { - return recordDao.findByAuthor(user); - } - - @Transactional(readOnly = true) - @Override - public List findAllRecords() { - return recordDao.findAllRecords(); - } - @Override public Page findAll(RecordFilterParams filters, Pageable pageSpec) { - // TODO - return null; + return recordDao.findAllRecords(filters, pageSpec); } @Transactional(readOnly = true) @@ -80,12 +61,6 @@ public Page findAllFull(RecordFilterParams filters, Pageable page return recordDao.findAllRecordsFull(filters, pageSpec); } - @Transactional(readOnly = true) - @Override - public List findAllFull(RecordFilterParams filterParams) { - return findAllFull(filterParams, Pageable.unpaged()).getContent(); - } - @Override protected void prePersist(PatientRecord instance) { final User author = securityUtils.getCurrentUser(); diff --git a/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java index 57db1670..03c5b2be 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java @@ -6,6 +6,7 @@ import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.PatientRecordService; import org.junit.jupiter.api.BeforeEach; @@ -14,6 +15,8 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.Pageable; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MvcResult; @@ -24,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -58,8 +62,8 @@ public void getAllInstitutionsReturnsEmptyListWhenNoInstitutionsAreFound() throw assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); final List body = objectMapper.readValue(result.getResponse().getContentAsString(), - new TypeReference>() { - }); + new TypeReference<>() { + }); assertTrue(body.isEmpty()); } @@ -82,8 +86,8 @@ public void getAllInstitutionsReturnsAlphabeticallySortedInstitutions() throws E assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); final List body = objectMapper.readValue(result.getResponse().getContentAsString(), - new TypeReference>() { - }); + new TypeReference<>() { + }); assertEquals("A", body.get(0).getName()); assertEquals("B", body.get(1).getName()); @@ -102,7 +106,7 @@ public void findByKeyReturnsInstitution() throws Exception { assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); final Institution res = objectMapper.readValue(result.getResponse().getContentAsString(), Institution.class); - assertEquals(res.getUri(),institution.getUri()); + assertEquals(res.getUri(), institution.getUri()); verify(institutionServiceMock).findByKey(key); } @@ -130,16 +134,17 @@ public void getTreatedPatientRecordsReturnsRecords() throws Exception { records.add(record2); when(institutionServiceMock.findByKey(key)).thenReturn(institution); - when(patientRecordServiceMock.findByInstitution(institution)).thenReturn(records); + when(patientRecordServiceMock.findAll(any(RecordFilterParams.class), any(Pageable.class))).thenReturn( + new PageImpl<>(records)); final MvcResult result = mockMvc.perform(get("/institutions/" + key + "/patients/")).andReturn(); assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); final List body = objectMapper.readValue(result.getResponse().getContentAsString(), - new TypeReference>() { - }); + new TypeReference<>() { + }); assertEquals(2, body.size()); verify(institutionServiceMock).findByKey(key); - verify(patientRecordServiceMock).findByInstitution(institution); + verify(patientRecordServiceMock).findAll(new RecordFilterParams(key), Pageable.unpaged()); } @Test @@ -147,7 +152,8 @@ public void createInstitutionReturnsResponseStatusCreated() throws Exception { Institution institution = Generator.generateInstitution(); final MvcResult result = mockMvc.perform(post("/institutions/").content(toJson(institution)) - .contentType(MediaType.APPLICATION_JSON_VALUE)).andReturn(); + .contentType(MediaType.APPLICATION_JSON_VALUE)) + .andReturn(); assertEquals(HttpStatus.CREATED, HttpStatus.valueOf(result.getResponse().getStatus())); } @@ -162,7 +168,9 @@ public void updateInstitutionReturnsResponseStatusNoContent() throws Exception { when(institutionServiceMock.findByKey(key)).thenReturn(institution); final MvcResult result = mockMvc.perform(put("/institutions/" + key).content(toJson(institution)) - .contentType(MediaType.APPLICATION_JSON_VALUE)).andReturn(); + .contentType( + MediaType.APPLICATION_JSON_VALUE)) + .andReturn(); assertEquals(HttpStatus.NO_CONTENT, HttpStatus.valueOf(result.getResponse().getStatus())); verify(institutionServiceMock).findByKey(key); @@ -175,8 +183,10 @@ public void updateInstitutionWithNonMatchingKeyReturnsResponseStatusBadRequest() Institution institution = Generator.generateInstitution(); institution.setKey(key); - final MvcResult result = mockMvc.perform(put("/institutions/123456" ).content(toJson(institution)) - .contentType(MediaType.APPLICATION_JSON_VALUE)).andReturn(); + final MvcResult result = mockMvc.perform(put("/institutions/123456").content(toJson(institution)) + .contentType( + MediaType.APPLICATION_JSON_VALUE)) + .andReturn(); assertEquals(HttpStatus.BAD_REQUEST, HttpStatus.valueOf(result.getResponse().getStatus())); } @@ -191,7 +201,9 @@ public void updateInstitutionReturnsResponseStatusNotFound() throws Exception { when(institutionServiceMock.findByKey(key)).thenReturn(null); final MvcResult result = mockMvc.perform(put("/institutions/" + key).content(toJson(institution)) - .contentType(MediaType.APPLICATION_JSON_VALUE)).andReturn(); + .contentType( + MediaType.APPLICATION_JSON_VALUE)) + .andReturn(); assertEquals(HttpStatus.NOT_FOUND, HttpStatus.valueOf(result.getResponse().getStatus())); verify(institutionServiceMock).findByKey(key); @@ -206,7 +218,7 @@ public void deleteInstitutionReturnsResponseStatusNoContent() throws Exception { when(institutionServiceMock.findByKey(key)).thenReturn(institution); - final MvcResult result = mockMvc.perform(delete("/institutions/12345" )).andReturn(); + final MvcResult result = mockMvc.perform(delete("/institutions/12345")).andReturn(); assertEquals(HttpStatus.NO_CONTENT, HttpStatus.valueOf(result.getResponse().getStatus())); verify(institutionServiceMock).findByKey(key); diff --git a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java index b3ec5d22..7101f264 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java @@ -11,7 +11,6 @@ import cz.cvut.kbss.study.model.RecordPhase; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; -import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.PatientRecordService; import cz.cvut.kbss.study.util.IdentificationUtils; import org.junit.jupiter.api.BeforeEach; @@ -21,6 +20,9 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.Pageable; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MvcResult; @@ -54,9 +56,6 @@ public class PatientRecordControllerTest extends BaseControllerTestRunner { @Mock private PatientRecordService patientRecordServiceMock; - @Mock - private InstitutionService institutionServiceMock; - @InjectMocks private PatientRecordController controller; @@ -97,7 +96,8 @@ public void getRecordReturnsFoundRecord() throws Exception { @Test public void getRecordsReturnsEmptyListWhenNoReportsAreFound() throws Exception { - when(patientRecordServiceMock.findAllRecords()).thenReturn(Collections.emptyList()); + when(patientRecordServiceMock.findAll(any(RecordFilterParams.class), any(Pageable.class))).thenReturn( + Page.empty()); final MvcResult result = mockMvc.perform(get("/records/")).andReturn(); @@ -123,7 +123,8 @@ public void getRecordsReturnsAllRecords() throws Exception { records.add(record2); records.add(record3); - when(patientRecordServiceMock.findAllRecords()).thenReturn(records); + when(patientRecordServiceMock.findAll(any(RecordFilterParams.class), any(Pageable.class))).thenReturn( + new PageImpl<>(records)); final MvcResult result = mockMvc.perform(get("/records/")).andReturn(); @@ -132,11 +133,11 @@ public void getRecordsReturnsAllRecords() throws Exception { new TypeReference<>() { }); assertEquals(3, body.size()); - verify(patientRecordServiceMock).findAllRecords(); + verify(patientRecordServiceMock).findAll(new RecordFilterParams(), Pageable.unpaged()); } @Test - public void finByInstitutionReturnsRecords() throws Exception { + public void findByInstitutionReturnsRecords() throws Exception { final String key = "12345"; Institution institution = Generator.generateInstitution(); @@ -151,8 +152,8 @@ public void finByInstitutionReturnsRecords() throws Exception { records.add(record1); records.add(record2); - when(institutionServiceMock.findByKey(institution.getKey())).thenReturn(institution); - when(patientRecordServiceMock.findByInstitution(institution)).thenReturn(records); + when(patientRecordServiceMock.findAll(any(RecordFilterParams.class), any(Pageable.class))).thenReturn( + new PageImpl<>(records)); System.out.println(institution.getKey()); final MvcResult result = mockMvc.perform(get("/records").param("institution", institution.getKey())).andReturn(); @@ -162,17 +163,7 @@ public void finByInstitutionReturnsRecords() throws Exception { new TypeReference<>() { }); assertEquals(2, body.size()); - verify(institutionServiceMock).findByKey(institution.getKey()); - } - - @Test - public void findByInstitutionReturnsNotFound() throws Exception { - final String key = "12345"; - - when(institutionServiceMock.findByKey(key)).thenReturn(null); - final MvcResult result = mockMvc.perform(get("/records").param("institution", key)).andReturn(); - - assertEquals(HttpStatus.NOT_FOUND, HttpStatus.valueOf(result.getResponse().getStatus())); + verify(patientRecordServiceMock).findAll(new RecordFilterParams(institution.getKey()), Pageable.unpaged()); } @Test @@ -255,7 +246,8 @@ void exportRecordsParsesProvidedDateBoundsAndPassesThemToService() throws Except final LocalDate maxDate = LocalDate.now().minusDays(5); final List records = List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); - when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class))).thenReturn(records); + when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class), any( + Pageable.class))).thenReturn(new PageImpl<>(records)); final MvcResult mvcResult = mockMvc.perform(get("/records/export") .param("minDate", minDate.toString()) @@ -265,20 +257,21 @@ void exportRecordsParsesProvidedDateBoundsAndPassesThemToService() throws Except }); assertThat(result, containsSameEntities(records)); verify(patientRecordServiceMock).findAllFull( - new RecordFilterParams(null, minDate, maxDate, Collections.emptySet())); + new RecordFilterParams(null, minDate, maxDate, Collections.emptySet()), Pageable.unpaged()); } @Test void exportRecordsUsesDefaultValuesForMinAndMaxDateWhenTheyAreNotProvidedByRequest() throws Exception { final List records = List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); - when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class))).thenReturn(records); + when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class), any( + Pageable.class))).thenReturn(new PageImpl<>(records)); final MvcResult mvcResult = mockMvc.perform(get("/records/export")).andReturn(); final List result = readValue(mvcResult, new TypeReference<>() { }); assertThat(result, containsSameEntities(records)); - verify(patientRecordServiceMock).findAllFull(new RecordFilterParams()); + verify(patientRecordServiceMock).findAllFull(new RecordFilterParams(), Pageable.unpaged()); } @Test @@ -287,7 +280,8 @@ void exportRecordsExportsRecordsForProvidedInstitutionForSpecifiedPeriod() throw final LocalDate maxDate = LocalDate.now().minusDays(5); final List records = List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); - when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class))).thenReturn(records); + when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class), any( + Pageable.class))).thenReturn(new PageImpl<>(records)); final MvcResult mvcResult = mockMvc.perform(get("/records/export") .param("minDate", minDate.toString()) @@ -298,7 +292,8 @@ void exportRecordsExportsRecordsForProvidedInstitutionForSpecifiedPeriod() throw }); assertThat(result, containsSameEntities(records)); verify(patientRecordServiceMock).findAllFull( - new RecordFilterParams(user.getInstitution().getKey(), minDate, maxDate, Collections.emptySet())); + new RecordFilterParams(user.getInstitution().getKey(), minDate, maxDate, Collections.emptySet()), + Pageable.unpaged()); } @Test diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml index 93b37394..a0506b13 100644 --- a/src/test/resources/logback-test.xml +++ b/src/test/resources/logback-test.xml @@ -24,12 +24,7 @@ - - - - - - + From bac3a31e6638b61b64c9aada6f142d24a6c5fcdb Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 31 Jan 2024 13:56:23 +0100 Subject: [PATCH 5/6] [kbss-cvut/record-manager-ui#71] Add support for paging and sorting to record REST API. --- .../persistence/dao/ActionHistoryDao.java | 4 +- .../study/rest/PatientRecordController.java | 7 +- .../cvut/kbss/study/rest/util/RestUtils.java | 50 +++++++++- .../cz/cvut/kbss/study/util/Constants.java | 17 +++- .../rest/PatientRecordControllerTest.java | 33 +++++++ .../kbss/study/rest/util/RestUtilsTest.java | 92 ++++++++++++++++++- 6 files changed, 194 insertions(+), 9 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/ActionHistoryDao.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/ActionHistoryDao.java index ad377150..d8d74f16 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/dao/ActionHistoryDao.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/ActionHistoryDao.java @@ -53,8 +53,8 @@ public List findAllWithParams(String typeFilter, User author, int ActionHistory.class) .setParameter("type", typeUri) .setParameter("isCreated", URI.create(Vocabulary.s_p_created)) - .setFirstResult((pageNumber - 1) * Constants.ACTIONS_PER_PAGE) - .setMaxResults(Constants.ACTIONS_PER_PAGE + 1); + .setFirstResult((pageNumber - 1) * Constants.DEFAULT_PAGE_SIZE) + .setMaxResults(Constants.DEFAULT_PAGE_SIZE + 1); if (author != null) { q.setParameter("hasOwner", URI.create(Vocabulary.s_p_has_owner)) diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index 3ba7ee48..07667a99 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -10,7 +10,6 @@ import cz.cvut.kbss.study.rest.util.RestUtils; import cz.cvut.kbss.study.security.SecurityConstants; import cz.cvut.kbss.study.service.PatientRecordService; -import org.springframework.data.domain.Pageable; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -46,7 +45,8 @@ public PatientRecordController(PatientRecordService recordService) { public List getRecords( @RequestParam(value = "institution", required = false) String institutionKey, @RequestParam MultiValueMap params) { - return recordService.findAll(RecordFilterMapper.constructRecordFilter(params), Pageable.unpaged()).getContent(); + return recordService.findAll(RecordFilterMapper.constructRecordFilter(params), RestUtils.resolvePaging(params)) + .getContent(); } @PreAuthorize( @@ -55,7 +55,8 @@ public List getRecords( public List exportRecords( @RequestParam(name = "institution", required = false) String institutionKey, @RequestParam MultiValueMap params) { - return recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), Pageable.unpaged()).getContent(); + return recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), + RestUtils.resolvePaging(params)).getContent(); } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isRecordInUsersInstitution(#key)") diff --git a/src/main/java/cz/cvut/kbss/study/rest/util/RestUtils.java b/src/main/java/cz/cvut/kbss/study/rest/util/RestUtils.java index 0ebc2892..6848f18c 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/util/RestUtils.java +++ b/src/main/java/cz/cvut/kbss/study/rest/util/RestUtils.java @@ -1,9 +1,14 @@ package cz.cvut.kbss.study.rest.util; +import cz.cvut.kbss.study.util.Constants; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; +import org.springframework.util.MultiValueMap; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; @@ -12,9 +17,21 @@ import java.nio.charset.StandardCharsets; import java.time.LocalDate; import java.time.format.DateTimeParseException; +import java.util.Optional; +import java.util.stream.Collectors; public class RestUtils { + /** + * Prefix indicating ascending sort order. + */ + public static final char SORT_ASC = '+'; + + /** + * Prefix indicating descending sort order. + */ + public static final char SORT_DESC = '-'; + private RestUtils() { throw new AssertionError(); } @@ -94,8 +111,39 @@ public static LocalDate parseDate(String dateStr) { try { return LocalDate.parse(dateStr); } catch (DateTimeParseException | NullPointerException e) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Value '" + dateStr + "' is not a valid date in ISO format."); + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, + "Value '" + dateStr + "' is not a valid date in ISO format."); } } + /** + * Resolves paging and sorting configuration from the specified request parameters. + *

+ * If no paging and filtering info is specified, an {@link Pageable#unpaged()} object is returned. + *

+ * Note that for sorting, {@literal +} should be used before sorting property name to specify ascending order, + * {@literal -} for descending order, for example, {@literal -date} indicates sorting by date in descending order. + * + * @param params Request parameters + * @return {@code Pageable} containing values resolved from the params or defaults + */ + public static Pageable resolvePaging(MultiValueMap params) { + if (params.getFirst(Constants.PAGE_PARAM) == null) { + return Pageable.unpaged(); + } + final int page = Integer.parseInt(params.getFirst(Constants.PAGE_PARAM)); + final int size = Optional.ofNullable(params.getFirst(Constants.PAGE_SIZE_PARAM)).map(Integer::parseInt) + .orElse(Constants.DEFAULT_PAGE_SIZE); + if (params.containsKey(Constants.SORT_PARAM)) { + final Sort sort = Sort.by(params.get(Constants.SORT_PARAM).stream().map(sp -> { + if (sp.charAt(0) == SORT_ASC || sp.charAt(0) == SORT_DESC) { + final String property = sp.substring(1); + return sp.charAt(0) == SORT_DESC ? Sort.Order.desc(property) : Sort.Order.asc(property); + } + return Sort.Order.asc(sp); + }).collect(Collectors.toList())); + return PageRequest.of(page, size, sort); + } + return PageRequest.of(page, size); + } } diff --git a/src/main/java/cz/cvut/kbss/study/util/Constants.java b/src/main/java/cz/cvut/kbss/study/util/Constants.java index 0a22c590..e49f7f38 100644 --- a/src/main/java/cz/cvut/kbss/study/util/Constants.java +++ b/src/main/java/cz/cvut/kbss/study/util/Constants.java @@ -28,7 +28,7 @@ private Constants() { /** * Number of history actions fetched from database. Needs to be changes also in front-end. */ - public static final int ACTIONS_PER_PAGE = 25; + public static final int DEFAULT_PAGE_SIZE = 25; /** * Path to directory containing queries used by the system. @@ -37,4 +37,19 @@ private Constants() { * ClassLoader#getResourceAsStream(String)}. */ public static final String QUERY_DIRECTORY = "query"; + + /** + * Name of the request parameter specifying page number. + */ + public static final String PAGE_PARAM = "page"; + + /** + * Name of the request parameter specifying page size. + */ + public static final String PAGE_SIZE_PARAM = "size"; + + /** + * Name of the request parameter specifying sorting. + */ + public static final String SORT_PARAM = "sort"; } diff --git a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java index 7101f264..48a250a2 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java @@ -11,7 +11,10 @@ import cz.cvut.kbss.study.model.RecordPhase; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; +import cz.cvut.kbss.study.persistence.dao.util.RecordSort; +import cz.cvut.kbss.study.rest.util.RestUtils; import cz.cvut.kbss.study.service.PatientRecordService; +import cz.cvut.kbss.study.util.Constants; import cz.cvut.kbss.study.util.IdentificationUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -22,7 +25,9 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MvcResult; @@ -338,4 +343,32 @@ void importRecordsReturnsConflictWhenServiceThrowsRecordAuthorNotFound() throws mockMvc.perform(post("/records/import").content(toJson(records)).contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isConflict()); } + + @Test + void getRecordsResolvesPagingConfigurationFromRequestParameters() throws Exception { + final LocalDate minDate = LocalDate.now().minusDays(35); + final LocalDate maxDate = LocalDate.now().minusDays(5); + final int page = Generator.randomInt(0, 5); + final int pageSize = Generator.randomInt(30, 50); + final List records = + List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); + when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class), any( + Pageable.class))).thenReturn(new PageImpl<>(records)); + + final MvcResult mvcResult = mockMvc.perform(get("/records/export") + .param("minDate", minDate.toString()) + .param("maxDate", maxDate.toString()) + .param(Constants.PAGE_PARAM, Integer.toString(page)) + .param(Constants.PAGE_SIZE_PARAM, + Integer.toString(pageSize)) + .param(Constants.SORT_PARAM, + RestUtils.SORT_DESC + RecordSort.SORT_DATE_PROPERTY)) + .andReturn(); + final List result = readValue(mvcResult, new TypeReference<>() { + }); + assertThat(result, containsSameEntities(records)); + verify(patientRecordServiceMock).findAllFull( + new RecordFilterParams(null, minDate, maxDate, Collections.emptySet()), + PageRequest.of(page, pageSize, Sort.Direction.DESC, RecordSort.SORT_DATE_PROPERTY)); + } } diff --git a/src/test/java/cz/cvut/kbss/study/rest/util/RestUtilsTest.java b/src/test/java/cz/cvut/kbss/study/rest/util/RestUtilsTest.java index a2045f83..ca7df782 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/util/RestUtilsTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/util/RestUtilsTest.java @@ -1,14 +1,30 @@ package cz.cvut.kbss.study.rest.util; +import cz.cvut.kbss.study.environment.generator.Generator; +import cz.cvut.kbss.study.persistence.dao.util.RecordSort; +import cz.cvut.kbss.study.util.Constants; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.http.HttpStatus; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.springframework.web.server.ResponseStatusException; import java.time.LocalDate; import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class RestUtilsTest { @@ -22,14 +38,86 @@ void parseTimestampReturnsLocalDateParsedFromSpecifiedString() { void parseTimestampThrowsResponseStatusExceptionWithStatus400ForUnparseableString() { final Date date = new Date(); final ResponseStatusException ex = assertThrows(ResponseStatusException.class, - () -> RestUtils.parseDate(date.toString())); + () -> RestUtils.parseDate(date.toString())); assertEquals(HttpStatus.BAD_REQUEST, ex.getStatusCode()); } @Test void parseTimestampThrowsResponseStatusExceptionWithStatus400ForNullArgument() { final ResponseStatusException ex = assertThrows(ResponseStatusException.class, - () -> RestUtils.parseDate(null)); + () -> RestUtils.parseDate(null)); assertEquals(HttpStatus.BAD_REQUEST, ex.getStatusCode()); } + + @Test + void resolvePagingCreatesPageableObjectWithSpecifiedPageSizeAndNumber() { + final int page = Generator.randomInt(0, 10); + final int size = Generator.randomInt(20, 50); + final MultiValueMap params = new LinkedMultiValueMap<>(Map.of( + Constants.PAGE_PARAM, List.of(Integer.toString(page)), + Constants.PAGE_SIZE_PARAM, List.of(Integer.toString(size)))); + + final Pageable result = RestUtils.resolvePaging(params); + assertEquals(PageRequest.of(page, size), result); + } + + @Test + void resolvePagingReturnsUnpagedObjectWhenNoPageNumberIsSpecified() { + final MultiValueMap params = new LinkedMultiValueMap<>(); + final Pageable result = RestUtils.resolvePaging(params); + assertTrue(result.isUnpaged()); + } + + @Test + void resolvePagingReturnsPagedObjectWithDefaultPageSizeWhenNoPageSizeIsSpecified() { + final int page = Generator.randomInt(0, 10); + final MultiValueMap params = new LinkedMultiValueMap<>(Map.of( + Constants.PAGE_PARAM, List.of(Integer.toString(page)))); + + final Pageable result = RestUtils.resolvePaging(params); + assertEquals(PageRequest.of(page, Constants.DEFAULT_PAGE_SIZE), result); + } + + @ParameterizedTest + @MethodSource("sortTestArguments") + void resolvePagingAddsSpecifiedSortPropertyAndDirectionToResult(String sortParam, Sort.Order expectedOrder) { + final int page = Generator.randomInt(0, 10); + final int size = Generator.randomInt(20, 50); + final MultiValueMap params = new LinkedMultiValueMap<>(Map.of( + Constants.PAGE_PARAM, List.of(Integer.toString(page)), + Constants.PAGE_SIZE_PARAM, List.of(Integer.toString(size)), + Constants.SORT_PARAM, List.of(sortParam))); + + final Pageable result = RestUtils.resolvePaging(params); + assertNotNull(result.getSort()); + assertEquals(expectedOrder, result.getSort().getOrderFor(expectedOrder.getProperty())); + } + + protected static Stream sortTestArguments() { + return Stream.of( + Arguments.of(RestUtils.SORT_DESC + RecordSort.SORT_DATE_PROPERTY, + Sort.Order.desc(RecordSort.SORT_DATE_PROPERTY)), + Arguments.of(RestUtils.SORT_ASC + RecordSort.SORT_DATE_PROPERTY, + Sort.Order.asc(RecordSort.SORT_DATE_PROPERTY)), + Arguments.of(RecordSort.SORT_DATE_PROPERTY, Sort.Order.asc(RecordSort.SORT_DATE_PROPERTY)) + ); + } + + @Test + void resolvePagingSupportsMultipleSortValues() { + final String anotherSort = "name"; + final int page = Generator.randomInt(0, 10); + final int size = Generator.randomInt(20, 50); + final MultiValueMap params = new LinkedMultiValueMap<>(Map.of( + Constants.PAGE_PARAM, List.of(Integer.toString(page)), + Constants.PAGE_SIZE_PARAM, List.of(Integer.toString(size)), + Constants.SORT_PARAM, List.of(RecordSort.SORT_DATE_PROPERTY, RestUtils.SORT_ASC + anotherSort))); + + final Pageable result = RestUtils.resolvePaging(params); + assertNotNull(result.getSort()); + assertEquals(Sort.Order.asc(RecordSort.SORT_DATE_PROPERTY), + result.getSort().getOrderFor(RecordSort.SORT_DATE_PROPERTY)); + assertEquals(Sort.Order.asc(anotherSort), + result.getSort().getOrderFor(anotherSort)); + } } \ No newline at end of file From 5d62bac1606394aa8b2503ce0b8ce88d230727e0 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 31 Jan 2024 14:59:48 +0100 Subject: [PATCH 6/6] [kbss-cvut/record-manager-ui#71] Return HATEOAS response headers when using paging. --- .../study/rest/PatientRecordController.java | 28 ++- .../event/PaginatedResultRetrievedEvent.java | 37 ++++ .../rest/handler/HateoasPagingListener.java | 80 ++++++++ .../study/rest/util/HttpPaginationLink.java | 18 ++ .../rest/PatientRecordControllerTest.java | 69 ++++++- .../handler/HateoasPagingListenerTest.java | 178 ++++++++++++++++++ .../rest/handler/HttpLinkHeaderUtil.java | 32 ++++ 7 files changed, 427 insertions(+), 15 deletions(-) create mode 100644 src/main/java/cz/cvut/kbss/study/rest/event/PaginatedResultRetrievedEvent.java create mode 100644 src/main/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListener.java create mode 100644 src/main/java/cz/cvut/kbss/study/rest/util/HttpPaginationLink.java create mode 100644 src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java create mode 100644 src/test/java/cz/cvut/kbss/study/rest/handler/HttpLinkHeaderUtil.java diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index 07667a99..2774e90c 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -5,11 +5,15 @@ import cz.cvut.kbss.study.exception.NotFoundException; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.RecordPhase; +import cz.cvut.kbss.study.rest.event.PaginatedResultRetrievedEvent; import cz.cvut.kbss.study.rest.exception.BadRequestException; import cz.cvut.kbss.study.rest.util.RecordFilterMapper; import cz.cvut.kbss.study.rest.util.RestUtils; import cz.cvut.kbss.study.security.SecurityConstants; import cz.cvut.kbss.study.service.PatientRecordService; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.data.domain.Page; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -26,6 +30,7 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.util.UriComponentsBuilder; import java.util.List; @@ -36,17 +41,23 @@ public class PatientRecordController extends BaseController { private final PatientRecordService recordService; - public PatientRecordController(PatientRecordService recordService) { + private final ApplicationEventPublisher eventPublisher; + + public PatientRecordController(PatientRecordService recordService, ApplicationEventPublisher eventPublisher) { this.recordService = recordService; + this.eventPublisher = eventPublisher; } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getRecords( @RequestParam(value = "institution", required = false) String institutionKey, - @RequestParam MultiValueMap params) { - return recordService.findAll(RecordFilterMapper.constructRecordFilter(params), RestUtils.resolvePaging(params)) - .getContent(); + @RequestParam MultiValueMap params, + UriComponentsBuilder uriBuilder, HttpServletResponse response) { + final Page result = recordService.findAll(RecordFilterMapper.constructRecordFilter(params), + RestUtils.resolvePaging(params)); + eventPublisher.publishEvent(new PaginatedResultRetrievedEvent(this, uriBuilder, response, result)); + return result.getContent(); } @PreAuthorize( @@ -54,9 +65,12 @@ public List getRecords( @GetMapping(value = "/export", produces = MediaType.APPLICATION_JSON_VALUE) public List exportRecords( @RequestParam(name = "institution", required = false) String institutionKey, - @RequestParam MultiValueMap params) { - return recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), - RestUtils.resolvePaging(params)).getContent(); + @RequestParam MultiValueMap params, + UriComponentsBuilder uriBuilder, HttpServletResponse response) { + final Page result = recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), + RestUtils.resolvePaging(params)); + eventPublisher.publishEvent(new PaginatedResultRetrievedEvent(this, uriBuilder, response, result)); + return result.getContent(); } @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isRecordInUsersInstitution(#key)") diff --git a/src/main/java/cz/cvut/kbss/study/rest/event/PaginatedResultRetrievedEvent.java b/src/main/java/cz/cvut/kbss/study/rest/event/PaginatedResultRetrievedEvent.java new file mode 100644 index 00000000..8311c834 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/rest/event/PaginatedResultRetrievedEvent.java @@ -0,0 +1,37 @@ +package cz.cvut.kbss.study.rest.event; + +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.context.ApplicationEvent; +import org.springframework.data.domain.Page; +import org.springframework.web.util.UriComponentsBuilder; + +/** + * Fired when a paginated result is retrieved by a REST controller, so that HATEOAS headers can be added to the + * response. + */ +public class PaginatedResultRetrievedEvent extends ApplicationEvent { + + private final UriComponentsBuilder uriBuilder; + private final HttpServletResponse response; + private final Page page; + + public PaginatedResultRetrievedEvent(Object source, UriComponentsBuilder uriBuilder, HttpServletResponse response, + Page page) { + super(source); + this.uriBuilder = uriBuilder; + this.response = response; + this.page = page; + } + + public UriComponentsBuilder getUriBuilder() { + return uriBuilder; + } + + public HttpServletResponse getResponse() { + return response; + } + + public Page getPage() { + return page; + } +} diff --git a/src/main/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListener.java b/src/main/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListener.java new file mode 100644 index 00000000..025a7ecc --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListener.java @@ -0,0 +1,80 @@ +package cz.cvut.kbss.study.rest.handler; + +import cz.cvut.kbss.study.rest.event.PaginatedResultRetrievedEvent; +import cz.cvut.kbss.study.rest.util.HttpPaginationLink; +import cz.cvut.kbss.study.util.Constants; +import org.springframework.context.ApplicationListener; +import org.springframework.data.domain.Page; +import org.springframework.http.HttpHeaders; +import org.springframework.stereotype.Component; +import org.springframework.web.util.UriComponentsBuilder; + +/** + * Generates HATEOAS paging headers based on the paginated result retrieved by a REST controller. + */ +@Component +public class HateoasPagingListener implements ApplicationListener { + + @Override + public void onApplicationEvent(PaginatedResultRetrievedEvent event) { + final Page page = event.getPage(); + final LinkHeader header = new LinkHeader(); + if (page.hasNext()) { + header.addLink(generateNextPageLink(page, event.getUriBuilder()), HttpPaginationLink.NEXT); + header.addLink(generateLastPageLink(page, event.getUriBuilder()), HttpPaginationLink.LAST); + } + if (page.hasPrevious()) { + header.addLink(generatePreviousPageLink(page, event.getUriBuilder()), HttpPaginationLink.PREVIOUS); + header.addLink(generateFirstPageLink(page, event.getUriBuilder()), HttpPaginationLink.FIRST); + } + if (header.hasLinks()) { + event.getResponse().addHeader(HttpHeaders.LINK, header.toString()); + } + } + + private String generateNextPageLink(Page page, UriComponentsBuilder uriBuilder) { + return uriBuilder.replaceQueryParam(Constants.PAGE_PARAM, page.getNumber() + 1) + .replaceQueryParam(Constants.PAGE_SIZE_PARAM, page.getSize()) + .build().encode().toUriString(); + } + + private String generatePreviousPageLink(Page page, UriComponentsBuilder uriBuilder) { + return uriBuilder.replaceQueryParam(Constants.PAGE_PARAM, page.getNumber() - 1) + .replaceQueryParam(Constants.PAGE_SIZE_PARAM, page.getSize()) + .build().encode().toUriString(); + } + + private String generateFirstPageLink(Page page, UriComponentsBuilder uriBuilder) { + return uriBuilder.replaceQueryParam(Constants.PAGE_PARAM, 0) + .replaceQueryParam(Constants.PAGE_SIZE_PARAM, page.getSize()) + .build().encode().toUriString(); + } + + private String generateLastPageLink(Page page, UriComponentsBuilder uriBuilder) { + return uriBuilder.replaceQueryParam(Constants.PAGE_PARAM, page.getTotalPages() - 1) + .replaceQueryParam(Constants.PAGE_SIZE_PARAM, page.getSize()) + .build().encode().toUriString(); + } + + private static class LinkHeader { + + private final StringBuilder linkBuilder = new StringBuilder(); + + private void addLink(String url, HttpPaginationLink type) { + if (!linkBuilder.isEmpty()) { + linkBuilder.append(", "); + } + linkBuilder.append('<').append(url).append('>').append("; ").append("rel=\"").append(type.getName()) + .append('"'); + } + + private boolean hasLinks() { + return !linkBuilder.isEmpty(); + } + + @Override + public String toString() { + return linkBuilder.toString(); + } + } +} diff --git a/src/main/java/cz/cvut/kbss/study/rest/util/HttpPaginationLink.java b/src/main/java/cz/cvut/kbss/study/rest/util/HttpPaginationLink.java new file mode 100644 index 00000000..abdd1a95 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/rest/util/HttpPaginationLink.java @@ -0,0 +1,18 @@ +package cz.cvut.kbss.study.rest.util; + +/** + * Types of HTTP pagination links. + */ +public enum HttpPaginationLink { + NEXT("next"), PREVIOUS("prev"), FIRST("first"), LAST("last"); + + private final String name; + + HttpPaginationLink(String name) { + this.name = name; + } + + public String getName() { + return name; + } +} diff --git a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java index 48a250a2..901120cb 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java @@ -12,6 +12,7 @@ import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import cz.cvut.kbss.study.persistence.dao.util.RecordSort; +import cz.cvut.kbss.study.rest.event.PaginatedResultRetrievedEvent; import cz.cvut.kbss.study.rest.util.RestUtils; import cz.cvut.kbss.study.service.PatientRecordService; import cz.cvut.kbss.study.util.Constants; @@ -23,6 +24,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -61,6 +63,9 @@ public class PatientRecordControllerTest extends BaseControllerTestRunner { @Mock private PatientRecordService patientRecordServiceMock; + @Mock + private ApplicationEventPublisher eventPublisherMock; + @InjectMocks private PatientRecordController controller; @@ -120,18 +125,14 @@ public void getRecordsReturnsAllRecords() throws Exception { User user1 = Generator.generateUser(institution); User user2 = Generator.generateUser(institution); - PatientRecordDto record1 = Generator.generatePatientRecordDto(user1); - PatientRecordDto record2 = Generator.generatePatientRecordDto(user1); - PatientRecordDto record3 = Generator.generatePatientRecordDto(user2); - List records = new ArrayList<>(); - records.add(record1); - records.add(record2); - records.add(record3); + List records = + List.of(Generator.generatePatientRecordDto(user1), Generator.generatePatientRecordDto(user1), + Generator.generatePatientRecordDto(user2)); when(patientRecordServiceMock.findAll(any(RecordFilterParams.class), any(Pageable.class))).thenReturn( new PageImpl<>(records)); - final MvcResult result = mockMvc.perform(get("/records/")).andReturn(); + final MvcResult result = mockMvc.perform(get("/records")).andReturn(); assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); final List body = objectMapper.readValue(result.getResponse().getContentAsString(), @@ -371,4 +372,56 @@ void getRecordsResolvesPagingConfigurationFromRequestParameters() throws Excepti new RecordFilterParams(null, minDate, maxDate, Collections.emptySet()), PageRequest.of(page, pageSize, Sort.Direction.DESC, RecordSort.SORT_DATE_PROPERTY)); } + + @Test + void getRecordsPublishesPagingEvent() throws Exception { + List records = + List.of(Generator.generatePatientRecordDto(user), Generator.generatePatientRecordDto(user), + Generator.generatePatientRecordDto(user)); + + final Page page = new PageImpl<>(records, PageRequest.of(0, 5), 3); + when(patientRecordServiceMock.findAll(any(RecordFilterParams.class), any(Pageable.class))).thenReturn(page); + final MvcResult result = mockMvc.perform(get("/records").queryParam(Constants.PAGE_PARAM, "0") + .queryParam(Constants.PAGE_SIZE_PARAM, "5")) + .andReturn(); + + assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); + final List body = objectMapper.readValue(result.getResponse().getContentAsString(), + new TypeReference<>() { + }); + assertEquals(3, body.size()); + verify(patientRecordServiceMock).findAll(new RecordFilterParams(), PageRequest.of(0, 5)); + final ArgumentCaptor captor = ArgumentCaptor.forClass( + PaginatedResultRetrievedEvent.class); + verify(eventPublisherMock).publishEvent(captor.capture()); + final PaginatedResultRetrievedEvent event = captor.getValue(); + assertEquals(page, event.getPage()); + } + + @Test + void exportRecordsPublishesPagingEvent() throws Exception { + final LocalDate minDate = LocalDate.now().minusDays(35); + final LocalDate maxDate = LocalDate.now().minusDays(5); + final List records = + List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); + final Page page = new PageImpl<>(records, PageRequest.of(0, 50), 100); + when(patientRecordServiceMock.findAllFull(any(RecordFilterParams.class), any(Pageable.class))).thenReturn(page); + + final MvcResult mvcResult = mockMvc.perform(get("/records/export") + .param("minDate", minDate.toString()) + .param("maxDate", maxDate.toString()) + .param(Constants.PAGE_PARAM, "0") + .param(Constants.PAGE_SIZE_PARAM, "50")) + .andReturn(); + final List result = readValue(mvcResult, new TypeReference<>() { + }); + assertThat(result, containsSameEntities(records)); + verify(patientRecordServiceMock).findAllFull( + new RecordFilterParams(null, minDate, maxDate, Collections.emptySet()), PageRequest.of(0, 50)); + final ArgumentCaptor captor = + ArgumentCaptor.forClass(PaginatedResultRetrievedEvent.class); + verify(eventPublisherMock).publishEvent(captor.capture()); + final PaginatedResultRetrievedEvent event = captor.getValue(); + assertEquals(page, event.getPage()); + } } diff --git a/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java b/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java new file mode 100644 index 00000000..ac2a06c2 --- /dev/null +++ b/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java @@ -0,0 +1,178 @@ +package cz.cvut.kbss.study.rest.handler; + +import cz.cvut.kbss.study.dto.PatientRecordDto; +import cz.cvut.kbss.study.environment.generator.Generator; +import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.rest.event.PaginatedResultRetrievedEvent; +import cz.cvut.kbss.study.rest.util.HttpPaginationLink; +import cz.cvut.kbss.study.util.Constants; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; +import org.springframework.http.HttpHeaders; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.web.util.UriComponentsBuilder; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +class HateoasPagingListenerTest { + + private static final String BASE_URL = "http://localhost/rest/records"; + + private UriComponentsBuilder uriBuilder; + private MockHttpServletResponse responseMock; + + private List records; + + private HateoasPagingListener listener; + + @BeforeEach + public void setUp() { + this.listener = new HateoasPagingListener(); + this.uriBuilder = UriComponentsBuilder.newInstance().scheme("http").host("localhost").path("rest/records"); + this.responseMock = new MockHttpServletResponse(); + final User author = Generator.generateUser(null); + this.records = IntStream.range(0, 10).mapToObj(i -> Generator.generatePatientRecordDto(author)) + .collect(Collectors.toList()); + } + + @Test + public void generatesNextRelativeLink() { + final int size = 5; + final Page page = + new PageImpl<>(records.subList(0, size), PageRequest.of(0, size), records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNotNull(linkHeader); + final String nextLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.NEXT.getName()); + assertThat(nextLink, containsString(BASE_URL)); + assertThat(nextLink, containsString(page(1))); + assertThat(nextLink, containsString(pageSize(size))); + } + + private static String page(int pageNo) { + return Constants.PAGE_PARAM + "=" + pageNo; + } + + private static String pageSize(int size) { + return Constants.PAGE_SIZE_PARAM + "=" + size; + } + + private PaginatedResultRetrievedEvent event(Page page) { + return new PaginatedResultRetrievedEvent(this, uriBuilder, responseMock, page); + } + + @Test + public void generatesLastRelativeLink() { + final int size = 5; + final Page page = + new PageImpl<>(records.subList(0, size), PageRequest.of(0, size), records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNotNull(linkHeader); + final String lastLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.LAST.getName()); + assertThat(lastLink, containsString(BASE_URL)); + assertThat(lastLink, containsString(page(1))); + assertThat(lastLink, containsString(pageSize(size))); + } + + @Test + public void generatesPreviousRelativeLink() { + final int size = 5; + final Page page = + new PageImpl<>(records.subList(size, records.size()), PageRequest.of(1, size), + records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNotNull(linkHeader); + final String lastLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.PREVIOUS.getName()); + assertThat(lastLink, containsString(BASE_URL)); + assertThat(lastLink, containsString(page(0))); + assertThat(lastLink, containsString(pageSize(size))); + } + + @Test + public void generatesFirstRelativeLink() { + final int size = 5; + final Page page = + new PageImpl<>(records.subList(size, records.size()), PageRequest.of(1, size), + records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNotNull(linkHeader); + final String lastLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.FIRST.getName()); + assertThat(lastLink, containsString(BASE_URL)); + assertThat(lastLink, containsString(page(0))); + assertThat(lastLink, containsString(pageSize(size))); + } + + @Test + public void generatesAllRelativeLinks() { + final int size = 3; + final int pageNum = 2; + final Page page = new PageImpl<>(records.subList(pageNum * size, pageNum * size + size), + PageRequest.of(pageNum, size), records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNotNull(linkHeader); + final String nextLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.NEXT.getName()); + assertThat(nextLink, containsString(page(pageNum + 1))); + assertThat(nextLink, containsString(pageSize(size))); + final String previousLink = + HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.PREVIOUS.getName()); + assertThat(previousLink, containsString(page(pageNum - 1))); + assertThat(previousLink, containsString(pageSize(size))); + final String firstLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.FIRST.getName()); + assertThat(firstLink, containsString(page(0))); + assertThat(firstLink, containsString(pageSize(size))); + final String lastLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.LAST.getName()); + assertThat(lastLink, containsString(page(3))); + assertThat(lastLink, containsString(pageSize(size))); + } + + @Test + public void generatesNoLinksForEmptyPage() { + final int size = 5; + final Page page = new PageImpl<>(Collections.emptyList(), PageRequest.of(0, size), 0); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNull(linkHeader); + } + + @Test + public void generatesPreviousAndFirstLinkForEmptyPageAfterEnd() { + final int size = 5; + final int pageNum = 4; + final Page page = new PageImpl<>(Collections.emptyList(), PageRequest.of(pageNum, size), + records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNotNull(linkHeader); + final String previousLink = + HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.PREVIOUS.getName()); + assertThat(previousLink, containsString(page(pageNum - 1))); + assertThat(previousLink, containsString(pageSize(size))); + final String firstLink = HttpLinkHeaderUtil.extractURIByRel(linkHeader, HttpPaginationLink.FIRST.getName()); + assertThat(firstLink, containsString(page(0))); + assertThat(firstLink, containsString(pageSize(size))); + } + + @Test + public void generatesNoLinksForOnlyPage() { + final Page page = new PageImpl<>(records, PageRequest.of(0, records.size()), records.size()); + listener.onApplicationEvent(event(page)); + final String linkHeader = responseMock.getHeader(HttpHeaders.LINK); + assertNull(linkHeader); + } + +} \ No newline at end of file diff --git a/src/test/java/cz/cvut/kbss/study/rest/handler/HttpLinkHeaderUtil.java b/src/test/java/cz/cvut/kbss/study/rest/handler/HttpLinkHeaderUtil.java new file mode 100644 index 00000000..75905662 --- /dev/null +++ b/src/test/java/cz/cvut/kbss/study/rest/handler/HttpLinkHeaderUtil.java @@ -0,0 +1,32 @@ +package cz.cvut.kbss.study.rest.handler; + +public class HttpLinkHeaderUtil { + + private HttpLinkHeaderUtil() { + throw new AssertionError(); + } + + public static String extractURIByRel(final String linkHeader, final String rel) { + if (linkHeader == null) { + return null; + } + String uriWithSpecifiedRel = null; + final String[] links = linkHeader.split(", "); + String linkRelation; + for (final String link : links) { + final int positionOfSeparator = link.indexOf(';'); + linkRelation = link.substring(positionOfSeparator + 1).trim(); + if (extractTypeOfRelation(linkRelation).equals(rel)) { + uriWithSpecifiedRel = link.substring(0, positionOfSeparator); + break; + } + } + + return uriWithSpecifiedRel; + } + + private static Object extractTypeOfRelation(final String linkRelation) { + final int positionOfEquals = linkRelation.indexOf('='); + return linkRelation.substring(positionOfEquals + 2, linkRelation.length() - 1).trim(); + } +}