Skip to content

Commit

Permalink
[MODCON-148] - Fix "offset" does not behaviour (#149)
Browse files Browse the repository at this point in the history
* [MODCON-148] - Fix "offset" does not behaviour

* [MODCON-148] - Fix "offset" does not behaviour

* [MODCON-148] - Minor improvement

* [MODCON-148] - Fixed test

* [MODCON-148] - replaced all places relate to PageRequest.of
  • Loading branch information
azizbekxm authored Apr 4, 2024
1 parent 9bcb2a6 commit 8e335a3
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 66 deletions.
4 changes: 1 addition & 3 deletions src/main/java/org/folio/consortia/config/AppConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.springframework.web.client.RestTemplate;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
Expand Down Expand Up @@ -46,8 +45,7 @@ public ObjectMapper objectMapper() {
final ObjectMapper objectMapper = new ObjectMapper();
objectMapper.findAndRegisterModules()
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false)
.setSerializationInclusion(JsonInclude.Include.NON_EMPTY);
.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
return objectMapper;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import org.folio.consortia.exception.ResourceAlreadyExistException;
import org.folio.consortia.exception.ResourceNotFoundException;
import org.folio.consortia.service.ConsortiumService;
import org.folio.spring.data.OffsetRequest;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;

import java.util.UUID;
Expand Down Expand Up @@ -58,7 +58,7 @@ public Consortium update(UUID consortiumId, Consortium consortiumDto) {
public ConsortiumCollection getAll() {
var result = new ConsortiumCollection();

Page<ConsortiumEntity> consortiaPage = repository.findAll(PageRequest.of(0, 1));
Page<ConsortiumEntity> consortiaPage = repository.findAll(OffsetRequest.of(0, 1));
result.setConsortia(consortiaPage.stream().map(o -> converter.convert(o, Consortium.class)).toList());
result.setTotalRecords((int) consortiaPage.getTotalElements());
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@
import org.folio.consortia.service.UserTenantService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.FolioModuleMetadata;
import org.folio.spring.data.OffsetRequest;
import org.folio.spring.scope.FolioExecutionContextSetter;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.task.TaskExecutor;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
Expand Down Expand Up @@ -99,7 +99,7 @@ public PublicationDetailsResponse getPublicationDetails(UUID consortiumId, UUID
var publicationStatusEntity = publicationStatusRepository.findById(publicationId)
.orElseThrow(() -> new ResourceNotFoundException(PUBLICATION_ID_FIELD, String.valueOf(publicationId)));

var ptrEntities = publicationTenantRequestRepository.findByPcStateId(publicationId, PageRequest.of(0, Integer.MAX_VALUE));
var ptrEntities = publicationTenantRequestRepository.findByPcStateId(publicationId, OffsetRequest.of(0, Integer.MAX_VALUE));
log.info("getPublicationDetails:: Found {} of {} expected tenant request records", ptrEntities.getTotalElements(), publicationStatusEntity.getTotalRecords());

var errorList = buildErrorListFromPublicationTenantRequestEntities(ptrEntities);
Expand Down Expand Up @@ -305,7 +305,7 @@ public PublicationResultCollection getPublicationResults(UUID consortiumId, UUID
var publicationStatusEntity = publicationStatusRepository.findById(publicationId)
.orElseThrow(() -> new ResourceNotFoundException(PUBLICATION_ID_FIELD, String.valueOf(publicationId)));

var ptrEntities = publicationTenantRequestRepository.findByPcStateId(publicationId, PageRequest.of(0, Integer.MAX_VALUE));
var ptrEntities = publicationTenantRequestRepository.findByPcStateId(publicationId, OffsetRequest.of(0, Integer.MAX_VALUE));
log.info("getPublicationResults:: Found {} of {} expected tenant request records for publication {}", ptrEntities.getTotalElements(), publicationStatusEntity.getTotalRecords(), publicationId);

var resultList = ptrEntities.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
import java.util.Objects;
import java.util.UUID;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.node.TextNode;

import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;
import lombok.SneakyThrows;
import lombok.extern.log4j.Log4j2;
import org.apache.commons.lang3.ObjectUtils;
import org.folio.consortia.client.InventoryClient;
import org.folio.consortia.config.kafka.KafkaService;
Expand All @@ -23,21 +32,11 @@
import org.folio.consortia.service.TenantService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.FolioModuleMetadata;
import org.folio.spring.data.OffsetRequest;
import org.folio.spring.scope.FolioExecutionContextSetter;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.node.TextNode;

import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;
import lombok.SneakyThrows;
import lombok.extern.log4j.Log4j2;

@Service
@Log4j2
@RequiredArgsConstructor
Expand Down Expand Up @@ -145,7 +144,7 @@ public SharingInstanceCollection getSharingInstances(UUID consortiumId, UUID ins
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
var specification = constructSpecification(instanceIdentifier, sourceTenantId, targetTenantId, status);

var sharingInstancePage = sharingInstanceRepository.findAll(specification, PageRequest.of(offset, limit));
var sharingInstancePage = sharingInstanceRepository.findAll(specification, OffsetRequest.of(offset, limit));
var result = new SharingInstanceCollection();
result.setSharingInstances(sharingInstancePage.stream().map(o -> converter.convert(o, SharingInstance.class)).toList());
result.setTotalRecords((int) sharingInstancePage.getTotalElements());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import org.folio.consortia.service.SyncPrimaryAffiliationService;
import org.folio.consortia.service.TenantService;
import org.folio.consortia.service.UserService;
import org.folio.spring.data.OffsetRequest;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand Down Expand Up @@ -118,7 +118,7 @@ private void createPrimaryUserAffiliations(UUID consortiumId, String centralTena
var user = userList.get(idx);
try {
log.info("createPrimaryUserAffiliations:: Processing users: {} of {}", idx + 1, userList.size());
Page<UserTenantEntity> userTenantPage = userTenantRepository.findAnyByUserId(UUID.fromString(user.getId()), PageRequest.of(0, 1));
Page<UserTenantEntity> userTenantPage = userTenantRepository.findAnyByUserId(UUID.fromString(user.getId()), OffsetRequest.of(0, 1));

if (userTenantPage.getTotalElements() > 0) {
log.info("createPrimaryUserAffiliations:: Primary affiliation already exists for tenant/user: {}/{}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import java.util.Optional;
import java.util.UUID;

import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.folio.consortia.client.ConsortiaConfigurationClient;
Expand Down Expand Up @@ -37,18 +39,15 @@
import org.folio.consortia.service.TenantService;
import org.folio.consortia.service.UserService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.data.OffsetRequest;
import org.folio.spring.scope.FolioExecutionContextSetter;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;

import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;

@Service
@Log4j2
@RequiredArgsConstructor
Expand Down Expand Up @@ -81,7 +80,7 @@ public class TenantServiceImpl implements TenantService {
public TenantCollection get(UUID consortiumId, Integer offset, Integer limit) {
TenantCollection result = new TenantCollection();
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
Page<TenantEntity> page = tenantRepository.findByConsortiumId(consortiumId, PageRequest.of(offset, limit));
Page<TenantEntity> page = tenantRepository.findByConsortiumId(consortiumId, OffsetRequest.of(offset, limit));
result.setTenants(page.map(o -> converter.convert(o, Tenant.class)).getContent());
result.setTotalRecords((int) page.getTotalElements());
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.util.Optional;
import java.util.UUID;

import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.apache.commons.collections4.CollectionUtils;
import org.folio.consortia.config.FolioExecutionContextHelper;
import org.folio.consortia.domain.dto.PermissionUser;
Expand All @@ -17,8 +19,8 @@
import org.folio.consortia.domain.dto.UserTenantCollection;
import org.folio.consortia.domain.entity.TenantEntity;
import org.folio.consortia.domain.entity.UserTenantEntity;
import org.folio.consortia.exception.UserAffiliationException;
import org.folio.consortia.exception.ResourceNotFoundException;
import org.folio.consortia.exception.UserAffiliationException;
import org.folio.consortia.repository.UserTenantRepository;
import org.folio.consortia.service.ConsortiumService;
import org.folio.consortia.service.PermissionUserService;
Expand All @@ -27,16 +29,13 @@
import org.folio.consortia.service.UserTenantService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.FolioModuleMetadata;
import org.folio.spring.data.OffsetRequest;
import org.folio.spring.scope.FolioExecutionContextSetter;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;

/**
* Implementation of {@link UserTenantService}.
* <p>
Expand Down Expand Up @@ -68,7 +67,7 @@ public class UserTenantServiceImpl implements UserTenantService {
public UserTenantCollection get(UUID consortiumId, Integer offset, Integer limit) {
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
var result = new UserTenantCollection();
Page<UserTenantEntity> userTenantPage = userTenantRepository.getAll(PageRequest.of(offset, limit));
Page<UserTenantEntity> userTenantPage = userTenantRepository.getAll(OffsetRequest.of(offset, limit));
result.setUserTenants(userTenantPage.map(o -> converter.convert(o, UserTenant.class)).getContent());
result.setTotalRecords((int) userTenantPage.getTotalElements());
return result;
Expand Down Expand Up @@ -99,7 +98,7 @@ public UserTenantCollection getByUsernameAndTenantId(UUID consortiumId, String u
public UserTenantCollection getByUserId(UUID consortiumId, UUID userId, Integer offset, Integer limit) {
consortiumService.checkConsortiumExistsOrThrow(consortiumId);
var result = new UserTenantCollection();
Page<UserTenantEntity> userTenantPage = userTenantRepository.findByUserId(userId, PageRequest.of(offset, limit));
Page<UserTenantEntity> userTenantPage = userTenantRepository.findByUserId(userId, OffsetRequest.of(offset, limit));

if (userTenantPage.getContent().isEmpty()) {
throw new ResourceNotFoundException(USER_ID, String.valueOf(userId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
import org.folio.consortia.exception.ResourceAlreadyExistException;
import org.folio.consortia.repository.ConsortiumRepository;
import org.folio.consortia.support.BaseIT;
import org.folio.spring.data.OffsetRequest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;

class ConsortiumControllerTest extends BaseIT {
private static final String CONSORTIUM_RESOURCE_EXIST_MSG_TEMPLATE = "System can not have more than one consortium record";
Expand Down Expand Up @@ -161,8 +161,8 @@ void shouldGetConsortiumCollection() throws Exception {
List<ConsortiumEntity> consortiumEntityList = new ArrayList<>();
consortiumEntityList.add(consortiumEntity);

when(consortiumRepository.findAll(PageRequest.of(0, 1)))
.thenReturn(new PageImpl<>(consortiumEntityList, PageRequest.of(0, 1), consortiumEntityList.size()));
when(consortiumRepository.findAll(OffsetRequest.of(0, 1)))
.thenReturn(new PageImpl<>(consortiumEntityList, OffsetRequest.of(0, 1), consortiumEntityList.size()));

this.mockMvc.perform(
get("/consortia")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
import java.util.Set;
import java.util.UUID;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.tomakehurst.wiremock.client.WireMock;

import jakarta.validation.ConstraintViolation;
import jakarta.validation.ConstraintViolationException;
import org.folio.consortia.client.ConsortiaConfigurationClient;
import org.folio.consortia.client.PermissionsClient;
import org.folio.consortia.client.SyncPrimaryAffiliationClient;
Expand Down Expand Up @@ -59,6 +64,7 @@
import org.folio.consortia.support.BaseIT;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.FolioModuleMetadata;
import org.folio.spring.data.OffsetRequest;
import org.folio.spring.integration.XOkapiHeaders;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand All @@ -67,15 +73,8 @@
import org.springframework.boot.autoconfigure.domain.EntityScan;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;
import org.springframework.http.MediaType;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.tomakehurst.wiremock.client.WireMock;

import jakarta.validation.ConstraintViolation;
import jakarta.validation.ConstraintViolationException;

@EntityScan(basePackageClasses = TenantEntity.class)
class TenantControllerTest extends BaseIT {
private static final String TENANT_REQUEST_BODY = "{\"id\":\"diku1234\",\"code\":\"TST\",\"name\":\"diku_tenant_name1234\", \"isCentral\":false}";
Expand Down Expand Up @@ -128,12 +127,12 @@ void getTenants() throws Exception {
tenantEntityList.add(tenantEntity1);
tenantEntityList.add(tenantEntity2);

when(tenantRepository.findByConsortiumId(any(), any(PageRequest.of(0, 2)
.getClass()))).thenReturn(new PageImpl<>(tenantEntityList, PageRequest.of(0, 2), tenantEntityList.size()));
when(tenantRepository.findByConsortiumId(any(), any(OffsetRequest.of(1, 2).getClass())))
.thenReturn(new PageImpl<>(tenantEntityList, OffsetRequest.of(2, 2), tenantEntityList.size()));
when(consortiumRepository.existsById(consortiumId)).thenReturn(true);
var headers = defaultHeaders();

this.mockMvc.perform(get("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?limit=2&offset=1").headers(headers))
this.mockMvc.perform(get("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/tenants?limit=2&offset=2").headers(headers))
.andExpectAll(status().isOk(), content().contentType(MediaType.APPLICATION_JSON_VALUE));
}

Expand Down Expand Up @@ -253,7 +252,7 @@ void getBadRequest() throws Exception {
andExpectAll(
status().is4xxClientError(),
content().contentType(MediaType.APPLICATION_JSON_VALUE),
jsonPath("$.errors[0].message", is("Page size must not be less than one")));
jsonPath("$.errors[0].message", is("Limit cannot be negative or zero: 0")));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
import java.util.Optional;
import java.util.UUID;

import feign.FeignException;
import feign.Request;
import feign.RequestTemplate;
import feign.Response;
import org.folio.consortia.client.UsersClient;
import org.folio.consortia.domain.dto.UserTenant;
import org.folio.consortia.domain.dto.UserTenantCollection;
Expand All @@ -31,6 +35,7 @@
import org.folio.consortia.service.TenantService;
import org.folio.consortia.service.UserTenantService;
import org.folio.consortia.support.BaseIT;
import org.folio.spring.data.OffsetRequest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand All @@ -42,16 +47,10 @@
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;

import feign.FeignException;
import feign.Request;
import feign.RequestTemplate;
import feign.Response;

@EntityScan(basePackageClasses = UserTenantEntity.class)
class UserTenantControllerTest extends BaseIT {

Expand Down Expand Up @@ -123,7 +122,7 @@ void shouldGetUserTenantList() throws Exception {
Page<UserTenantEntity> userTenantPage = new PageImpl<>(List.of(createUserTenantEntity(consortiumId)));

when(consortiumRepository.existsById(consortiumId)).thenReturn(true);
when(userTenantRepository.getAll(PageRequest.of(1, 2))).thenReturn(userTenantPage);
when(userTenantRepository.getAll(OffsetRequest.of(1, 2))).thenReturn(userTenantPage);

this.mockMvc.perform(
get("/consortia/7698e46-c3e3-11ed-afa1-0242ac120002/user-tenants?limit=2&offset=1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.folio.consortia.exception.ResourceAlreadyExistException;
import org.folio.consortia.exception.ResourceNotFoundException;
import org.folio.consortia.service.impl.ConsortiumServiceImpl;
import org.folio.spring.data.OffsetRequest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
Expand All @@ -15,7 +16,6 @@
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -44,8 +44,8 @@ void shouldGetConsortiumList() {
List<ConsortiumEntity> consortiumEntityList = new ArrayList<>();
consortiumEntityList.add(consortiumEntity);

when(consortiumRepository.findAll(PageRequest.of(0, 1)))
.thenReturn(new PageImpl<>(consortiumEntityList, PageRequest.of(0, 1), consortiumEntityList.size()));
when(consortiumRepository.findAll(OffsetRequest.of(0, 1)))
.thenReturn(new PageImpl<>(consortiumEntityList, OffsetRequest.of(0, 1), consortiumEntityList.size()));

var consortiumCollection = consortiumService.getAll();
Assertions.assertEquals(1, consortiumCollection.getTotalRecords());
Expand Down
Loading

0 comments on commit 8e335a3

Please sign in to comment.