From 43a157366237870e301e8f7c4e066f0f51e2bd06 Mon Sep 17 00:00:00 2001 From: Bobby Sharp Date: Mon, 11 Mar 2024 16:24:50 -0400 Subject: [PATCH] MODLISTS-90: Temporarily retrieve contents for all fields in entity type definition --- .../folio/list/controller/ListController.java | 4 +- .../org/folio/list/services/ListService.java | 24 ++-- src/main/resources/swagger.api/list.yaml | 9 ++ .../ListControllerListContentsTest.java | 15 ++- .../ListServiceGetListContentsTest.java | 110 +++--------------- 5 files changed, 45 insertions(+), 117 deletions(-) diff --git a/src/main/java/org/folio/list/controller/ListController.java b/src/main/java/org/folio/list/controller/ListController.java index f225bb2d..c90c8e5a 100644 --- a/src/main/java/org/folio/list/controller/ListController.java +++ b/src/main/java/org/folio/list/controller/ListController.java @@ -85,8 +85,8 @@ public ResponseEntity performRefresh(UUID id) { } @Override - public ResponseEntity getListContents(UUID id, Integer offset, Integer size) { - return listService.getListContents(id, offset, size) + public ResponseEntity getListContents(UUID id, List fields, Integer offset, Integer size) { + return listService.getListContents(id, fields, offset, size) .map(ResponseEntity::ok) .orElseThrow(() -> new ListNotFoundException(id, ListActions.READ)); } diff --git a/src/main/java/org/folio/list/services/ListService.java b/src/main/java/org/folio/list/services/ListService.java index 951dc8ed..0a83c8cc 100644 --- a/src/main/java/org/folio/list/services/ListService.java +++ b/src/main/java/org/folio/list/services/ListService.java @@ -4,7 +4,6 @@ import lombok.extern.log4j.Log4j2; import org.apache.commons.collections4.CollectionUtils; -import org.folio.fql.model.field.FqlField; import org.folio.fql.service.FqlService; import org.folio.fql.model.Fql; import org.folio.list.domain.ListContent; @@ -48,7 +47,6 @@ import java.time.OffsetDateTime; import java.util.*; -import java.util.stream.Collectors; import static java.util.stream.Collectors.toMap; import static java.util.Objects.nonNull; @@ -209,13 +207,13 @@ public Optional performRefresh(UUID listId) { }); } - public Optional getListContents(UUID listId, Integer offset, Integer size) { + public Optional getListContents(UUID listId, List fields, Integer offset, Integer size) { log.info("Attempting to get contents for list with listId {}, tenantId {}, offset {}, size {}", listId, executionContext.getTenantId(), offset, size); return listRepository.findByIdAndIsDeletedFalse(listId) .map(list -> { validationService.assertSharedOrOwnedByUser(list, ListActions.READ); - return getListContents(list, offset, size); + return getListContents(list, fields, offset, size); }); } @@ -272,19 +270,13 @@ private void deleteListAndContents(ListEntity list) { listRepository.save(list.withIsDeleted(true)); } - private ResultsetPage getListContents(ListEntity list, Integer offset, Integer limit) { - List> sortedContents = List.of(); - List fields = list.getFields(); - if (CollectionUtils.isEmpty(fields)) { - Fql fql = fqlService.getFql(list.getFqlQuery()); - fields = fqlService.getFqlFields(fql) - .stream() - .map(FqlField::getColumnName) - .collect(Collectors.toList()); - } - if (!fields.contains("id")) { - fields.add("id"); + private ResultsetPage getListContents(ListEntity list, List fields, Integer offset, Integer limit) { + // If fields are not provided, retrieve all fields from the entity type definition + if (isEmpty(fields)) { + EntityType entityType = getEntityType(list.getEntityTypeId()); + fields = getFieldsFromEntityType(entityType); } + List> sortedContents = List.of(); if (list.isRefreshed()) { List> contentIds = listContentsRepository.getContents(list.getId(), list.getSuccessRefresh().getId(), new OffsetRequest(offset, limit)) .stream() diff --git a/src/main/resources/swagger.api/list.yaml b/src/main/resources/swagger.api/list.yaml index de26c227..42cbb289 100644 --- a/src/main/resources/swagger.api/list.yaml +++ b/src/main/resources/swagger.api/list.yaml @@ -219,6 +219,15 @@ paths: description: gets the list contents (if exists). parameters: - $ref: '#/components/parameters/id' + - name: fields + in: query + description: List of fields to retrieve content for + required: false + schema: + type: array + items: + type: + string - name: offset in: query description: Offset to start retrieving items from diff --git a/src/test/java/org/folio/list/controller/ListControllerListContentsTest.java b/src/test/java/org/folio/list/controller/ListControllerListContentsTest.java index 4b270331..a651ef3b 100644 --- a/src/test/java/org/folio/list/controller/ListControllerListContentsTest.java +++ b/src/test/java/org/folio/list/controller/ListControllerListContentsTest.java @@ -37,14 +37,15 @@ void shouldGetListContents() throws Exception { UUID listId = UUID.randomUUID(); Integer offset = 0; Integer size = 2; - var requestBuilder = get("/lists/" + listId + "/contents?size=2&offset=0") + List fields = List.of("key1", "key2", "key3", "key4"); + var requestBuilder = get("/lists/" + listId + "/contents?size=2&offset=0&fields=key1,key2,key3,key4") .contentType(APPLICATION_JSON) .header(XOkapiHeaders.TENANT, listId); List> expectedList = List.of( Map.of("key1", "value1", "key2", "value2"), Map.of("key3", "value3", "key4", "value4")); Optional expectedContent = Optional.of(new ResultsetPage().content(expectedList).totalRecords(expectedList.size())); - when(listService.getListContents(listId, offset, size)).thenReturn(expectedContent); + when(listService.getListContents(listId, fields, offset, size)).thenReturn(expectedContent); mockMvc.perform(requestBuilder) .andExpect(status().isOk()) .andExpect(jsonPath("$.content[0]", is(expectedList.get(0)))) @@ -56,11 +57,12 @@ void getListContentsShouldReturnHttp404WhenListNotFound() throws Exception { UUID listId = UUID.randomUUID(); Integer offset = 0; Integer size = 0; - var requestBuilder = get("/lists/" + listId + "/contents?size=0&offset=0") + List fields = List.of("key1"); + var requestBuilder = get("/lists/" + listId + "/contents?size=0&offset=0&fields=key1") .contentType(APPLICATION_JSON) .header(XOkapiHeaders.TENANT, listId); Optional expectedContent = Optional.empty(); - when(listService.getListContents(listId, offset, size)).thenReturn(expectedContent); + when(listService.getListContents(listId, fields, offset, size)).thenReturn(expectedContent); mockMvc.perform(requestBuilder) .andExpect(status().isNotFound()) .andExpect(jsonPath("$.code", is("read-list.not.found"))); @@ -73,12 +75,13 @@ void shouldReturnHttp401ForAccessingPrivateListContents() throws Exception { listEntity.setId(listId); Integer offset = 0; Integer size = 0; + List fields = List.of("key1"); - var requestBuilder = get("/lists/" + listId + "/contents?size=0&offset=0") + var requestBuilder = get("/lists/" + listId + "/contents?size=0&offset=0&fields=key1") .contentType(APPLICATION_JSON) .header(XOkapiHeaders.TENANT, listId); - when(listService.getListContents(listId, offset, size)) + when(listService.getListContents(listId, fields, offset, size)) .thenThrow(new PrivateListOfAnotherUserException(listEntity, ListActions.READ)); mockMvc.perform(requestBuilder) diff --git a/src/test/java/org/folio/list/service/ListServiceGetListContentsTest.java b/src/test/java/org/folio/list/service/ListServiceGetListContentsTest.java index 8d136bfb..5caa3ec3 100644 --- a/src/test/java/org/folio/list/service/ListServiceGetListContentsTest.java +++ b/src/test/java/org/folio/list/service/ListServiceGetListContentsTest.java @@ -7,16 +7,20 @@ import org.folio.list.exception.PrivateListOfAnotherUserException; import org.folio.list.repository.ListContentsRepository; import org.folio.list.repository.ListRepository; +import org.folio.list.rest.EntityTypeClient; import org.folio.list.rest.QueryClient; import org.folio.list.services.ListActions; import org.folio.list.services.ListService; import org.folio.list.services.ListValidationService; import org.folio.list.utils.TestDataFixture; import org.folio.querytool.domain.dto.ContentsRequest; +import org.folio.querytool.domain.dto.EntityType; +import org.folio.querytool.domain.dto.EntityTypeColumn; import org.folio.querytool.domain.dto.ResultsetPage; import org.folio.spring.FolioExecutionContext; import org.folio.spring.data.OffsetRequest; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -46,6 +50,8 @@ class ListServiceGetListContentsTest { private FqlService fqlService; @Mock private QueryClient queryClient; + @Mock + private EntityTypeClient entityTypeClient; @InjectMocks private ListService listService; @@ -90,12 +96,12 @@ void shouldReturnValidContentPage() { when(listRepository.findByIdAndIsDeletedFalse(listId)).thenReturn(Optional.of(expectedEntity)); when(listContentsRepository.getContents(listId, successRefresh.getId(), new OffsetRequest(offset, size))).thenReturn(listContents); when(queryClient.getContents(contentsRequest)).thenReturn(expectedList); - Optional actualContent = listService.getListContents(listId, offset, size); + Optional actualContent = listService.getListContents(listId, fields, offset, size); assertThat(actualContent).isEqualTo(expectedContent); } @Test - void shouldReturnRequestedFieldsPlusIdsIfIdsNotIncludedInFields() { + void shouldGetFieldsFromEntityTypeIfNotProvided() { String tenantId = "tenant_01"; UUID entityTypeId = UUID.randomUUID(); UUID listId = UUID.randomUUID(); @@ -103,51 +109,10 @@ void shouldReturnRequestedFieldsPlusIdsIfIdsNotIncludedInFields() { List.of(UUID.randomUUID().toString()), List.of(UUID.randomUUID().toString()) ); - int offset = 0; - int size = 2; - int contentVersion = 2; - List fields = new ArrayList<>(List.of("key1", "key2")); - ContentsRequest contentsRequest = new ContentsRequest().entityTypeId(entityTypeId) - .fields(fields) - .ids(contentIds); - List> expectedList = List.of( - Map.of("id", contentIds.get(0), "key1", "value1", "key2", "value2"), - Map.of("id", contentIds.get(1), "key1", "value3", "key2", "value4") - ); - Optional expectedContent = Optional.of(new ResultsetPage().content(expectedList).totalRecords(expectedList.size())); - - List listContents = contentIds.stream().map(id -> { - ListContent content = new ListContent(); - content.setContentId(id); - return content; - }).toList(); - - ListEntity expectedEntity = new ListEntity(); - ListRefreshDetails successRefresh = new ListRefreshDetails(); - successRefresh.setContentVersion(contentVersion); - expectedEntity.setSuccessRefresh(successRefresh); - expectedEntity.setEntityTypeId(entityTypeId); - expectedEntity.setId(listId); - expectedEntity.setFields(fields); - expectedEntity.getSuccessRefresh().setRecordsCount(2); - - when(executionContext.getTenantId()).thenReturn(tenantId); - when(listRepository.findByIdAndIsDeletedFalse(listId)).thenReturn(Optional.of(expectedEntity)); - when(listContentsRepository.getContents(listId, successRefresh.getId(), new OffsetRequest(offset, size))).thenReturn(listContents); - when(queryClient.getContents(contentsRequest)).thenReturn(expectedList); - Optional actualContent = listService.getListContents(listId, offset, size); - assertThat(actualContent).isEqualTo(expectedContent); - } - - @Test - void shouldReturnContentPageWithIdsForEmptyFields() { - String tenantId = "tenant_01"; - UUID entityTypeId = UUID.randomUUID(); - UUID listId = UUID.randomUUID(); - List> contentIds = List.of( - List.of(UUID.randomUUID().toString()), - List.of(UUID.randomUUID().toString()) + List columns = List.of( + new EntityTypeColumn().name("id") ); + EntityType entityType = new EntityType().name("entity-type").columns(columns); int offset = 0; int size = 2; int contentVersion = 2; @@ -177,64 +142,22 @@ void shouldReturnContentPageWithIdsForEmptyFields() { expectedEntity.getSuccessRefresh().setRecordsCount(2); when(executionContext.getTenantId()).thenReturn(tenantId); + when(entityTypeClient.getEntityType(entityTypeId)).thenReturn(entityType); when(listRepository.findByIdAndIsDeletedFalse(listId)).thenReturn(Optional.of(expectedEntity)); when(listContentsRepository.getContents(listId, successRefresh.getId(), new OffsetRequest(offset, size))).thenReturn(listContents); when(queryClient.getContents(contentsRequest)).thenReturn(expectedList); - Optional actualContent = listService.getListContents(listId, offset, size); - assertThat(actualContent).isEqualTo(expectedContent); - } - - @Test - void shouldReturnContentPageWithIdsForNullFields() { - String tenantId = "tenant_01"; - UUID entityTypeId = UUID.randomUUID(); - UUID listId = UUID.randomUUID(); - List> contentIds = List.of( - List.of(UUID.randomUUID().toString()), - List.of(UUID.randomUUID().toString()) - ); - int offset = 0; - int size = 2; - int contentVersion = 2; - List fields = List.of("id"); - ContentsRequest contentsRequest = new ContentsRequest().entityTypeId(entityTypeId) - .fields(fields) - .ids(contentIds); - List> expectedList = List.of( - Map.of("id", contentIds.get(0)), - Map.of("id", contentIds.get(1)) - ); - Optional expectedContent = Optional.of(new ResultsetPage().content(expectedList).totalRecords(expectedList.size())); - - List listContents = contentIds.stream().map(id -> { - ListContent content = new ListContent(); - content.setContentId(id); - return content; - }).toList(); - - ListEntity expectedEntity = new ListEntity(); - ListRefreshDetails successRefresh = new ListRefreshDetails(); - successRefresh.setContentVersion(contentVersion); - expectedEntity.setSuccessRefresh(successRefresh); - expectedEntity.setEntityTypeId(entityTypeId); - expectedEntity.setId(listId); - expectedEntity.getSuccessRefresh().setRecordsCount(2); - - when(executionContext.getTenantId()).thenReturn(tenantId); - when(listRepository.findByIdAndIsDeletedFalse(listId)).thenReturn(Optional.of(expectedEntity)); - when(listContentsRepository.getContents(listId, successRefresh.getId(), new OffsetRequest(offset, size))).thenReturn(listContents); - when(queryClient.getContents(contentsRequest)).thenReturn(expectedList); - Optional actualContent = listService.getListContents(listId, offset, size); + Optional actualContent = listService.getListContents(listId, fields, offset, size); assertThat(actualContent).isEqualTo(expectedContent); } @Test void shouldReturnEmptyContentPageIfNotRefreshed() { UUID listId = UUID.randomUUID(); + List fields = List.of("id", "key1", "key2"); Optional emptyContent = Optional.of(new ResultsetPage().content(List.of()).totalRecords(0)); ListEntity neverRefreshedList = TestDataFixture.getNeverRefreshedListEntity(); when(listRepository.findByIdAndIsDeletedFalse(listId)).thenReturn(Optional.of(neverRefreshedList)); - Optional actualContent = listService.getListContents(listId, 0, 100); + Optional actualContent = listService.getListContents(listId, fields, 0, 100); assertThat(actualContent).isEqualTo(emptyContent); } @@ -242,9 +165,10 @@ void shouldReturnEmptyContentPageIfNotRefreshed() { void shouldThrowExceptionWhenValidationFailed() { UUID listId = UUID.randomUUID(); ListEntity listEntity = TestDataFixture.getNeverRefreshedListEntity(); + List fields = List.of("id", "key1", "key2"); when(listRepository.findByIdAndIsDeletedFalse(listId)).thenReturn(Optional.of(listEntity)); doThrow(new PrivateListOfAnotherUserException(listEntity, ListActions.READ)) .when(listValidationService).assertSharedOrOwnedByUser(listEntity, ListActions.READ); - Assertions.assertThrows(PrivateListOfAnotherUserException.class, () -> listService.getListContents(listId, 0, 100)); + Assertions.assertThrows(PrivateListOfAnotherUserException.class, () -> listService.getListContents(listId, fields, 0, 100)); } }