diff --git a/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java b/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java index f78950f8..e16bb169 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java @@ -50,9 +50,10 @@ public ResponseEntity receiveSectionFile(@RequestParam("file") MultipartFile @PostMapping(value = "/register/block", produces = MediaType.APPLICATION_JSON_VALUE) public ResponseEntity receiveBlockFile(@RequestParam("file") MultipartFile file, - @RequestParam(value="existingExternalNames", required=false) List existingExternalNames) + @RequestParam(value="existingExternalNames", required=false) List existingExternalNames, + @RequestParam(value="ignoreExternalNames", required=false) List ignoreExternalNames) throws URISyntaxException { - BiFunction biFunction = (user, multipartFile) -> fileRegisterService.registerBlocks(user, multipartFile, existingExternalNames); + BiFunction biFunction = (user, multipartFile) -> fileRegisterService.registerBlocks(user, multipartFile, existingExternalNames, ignoreExternalNames); return receiveFile("block", file, biFunction); } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/UnreleaseServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/UnreleaseServiceImp.java index 0f05d998..21a757a5 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/UnreleaseServiceImp.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/UnreleaseServiceImp.java @@ -12,10 +12,8 @@ import uk.ac.sanger.sccp.utils.UCMap; import java.util.*; -import java.util.stream.Collectors; import static java.util.Objects.requireNonNull; -import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; /** diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterService.java index 887f4819..cf287225 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterService.java @@ -32,7 +32,7 @@ public interface FileRegisterService { * @exception UncheckedIOException the file cannot be read */ default RegisterResult registerBlocks(User user, MultipartFile multipartFile) throws ValidationException, UncheckedIOException { - return registerBlocks(user, multipartFile, null); + return registerBlocks(user, multipartFile, null, null); } /** @@ -40,11 +40,12 @@ default RegisterResult registerBlocks(User user, MultipartFile multipartFile) th * @param user the user responsible * @param multipartFile the file data * @param existingExternalNames known existing tissue external names to reregister + * @param ignoreExternalNames external names of rows to exclude from the request * @return the result of the registration * @exception ValidationException the data received is invalid * @exception UncheckedIOException the file cannot be read */ - RegisterResult registerBlocks(User user, MultipartFile multipartFile, List existingExternalNames) + RegisterResult registerBlocks(User user, MultipartFile multipartFile, List existingExternalNames, List ignoreExternalNames) throws ValidationException, UncheckedIOException; /** diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterServiceImp.java index 5b5ab497..9dd34f9b 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterServiceImp.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterServiceImp.java @@ -55,6 +55,7 @@ public FileRegisterServiceImp(IRegisterService sectionRe * @param service service method to validate and perform the request * @param existingExternalNames null or a list of tissue external names referenced in the request * that are known to already exist + * @param ignoreExternalNames null or a list of tissue external names referenced in the request that should be ignored * @return the result of the registration request * @param the type of request to read from the file * @param the type of result expected from the request @@ -62,7 +63,7 @@ public FileRegisterServiceImp(IRegisterService sectionRe * @exception UncheckedIOException there was an IOException reading the file */ protected Res register(User user, MultipartFile multipartFile, MultipartFileReader fileReader, - BiFunction service, List existingExternalNames) + BiFunction service, List existingExternalNames, List ignoreExternalNames) throws ValidationException { Req req; try { @@ -70,6 +71,9 @@ protected Res register(User user, MultipartFile multipartFile, Multip } catch (IOException e) { throw new UncheckedIOException(e); } + if (!nullOrEmpty(ignoreExternalNames) && req instanceof RegisterRequest) { + updateToRemove((RegisterRequest) req, ignoreExternalNames); + } if (!nullOrEmpty(existingExternalNames) && req instanceof RegisterRequest) { updateWithExisting((RegisterRequest) req, existingExternalNames); } @@ -77,7 +81,7 @@ protected Res register(User user, MultipartFile multipartFile, Multip } /** - * Update the blocks in the given request to specify that they are existing tissue if they + * Updates the blocks in the given request to specify that they are existing tissue if they * are found in the given list of existing external names. * @param request a block register request * @param existingExternalNames list of known existing external names @@ -98,19 +102,38 @@ public void updateWithExisting(RegisterRequest request, List existingExt } } + /** + * Updates the blocks in the given request to remove those matching the given list of external names. + * @param request block register request + * @param externalNames external names to remove + */ + public void updateToRemove(RegisterRequest request, List externalNames) { + if (nullOrEmpty(externalNames) || nullOrEmpty(request.getBlocks())) { + return; + } + Set ignoreUC = externalNames.stream() + .filter(Objects::nonNull) + .map(String::toUpperCase) + .collect(toSet()); + List blocks = request.getBlocks().stream() + .filter(block -> block==null || block.getExternalIdentifier()==null || !ignoreUC.contains(block.getExternalIdentifier().toUpperCase())) + .toList(); + request.setBlocks(blocks); + } + @Override public RegisterResult registerSections(User user, MultipartFile multipartFile) throws ValidationException { - return register(user, multipartFile, sectionFileReader, sectionRegisterService::register, null); + return register(user, multipartFile, sectionFileReader, sectionRegisterService::register, null, null); } @Override - public RegisterResult registerBlocks(User user, MultipartFile multipartFile, List existingExternalNames) - throws ValidationException, UncheckedIOException { - return register(user, multipartFile, blockFileReader, blockRegisterService::register, existingExternalNames); + public RegisterResult registerBlocks(User user, MultipartFile multipartFile, List existingExternalNames, + List ignoreExternalNames) throws ValidationException, UncheckedIOException { + return register(user, multipartFile, blockFileReader, blockRegisterService::register, existingExternalNames, ignoreExternalNames); } @Override public RegisterResult registerOriginal(User user, MultipartFile multipartFile) throws ValidationException, UncheckedIOException { - return register(user, multipartFile, originalSampleFileReader, originalSampleRegisterService::register, null); + return register(user, multipartFile, originalSampleFileReader, originalSampleRegisterService::register, null, null); } } diff --git a/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestFileBlockRegister.java b/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestFileBlockRegister.java index 2ea9cf93..e0db4dd1 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestFileBlockRegister.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestFileBlockRegister.java @@ -88,7 +88,7 @@ public void testClashes() throws Exception { when(mockRegService.register(any(), any())).thenReturn(RegisterResult.clashes( List.of(new RegisterClash(tissue1, List.of(lw1)), new RegisterClash(tissue2, List.of(lw2))) )); - var response = upload("testdata/block_reg_existing.xlsx", null, true); + var response = upload("testdata/block_reg_existing.xlsx", null, null, true); var map = objectMapper.readValue(response.getContentAsString(), Map.class); Object clashesObj = map.get("clashes"); assertNotNull(clashesObj); @@ -115,7 +115,7 @@ public void testExistingExtNames() throws Exception { User user = creator.createUser("user1"); tester.setUser(user); when(mockRegService.register(any(), any())).thenThrow(new ValidationException(List.of("Bad reg"))); - var response = upload("testdata/block_reg_existing.xlsx", List.of("Ext17"), false); + var response = upload("testdata/block_reg_existing.xlsx", List.of("Ext17"), null, false); var map = objectMapper.readValue(response.getContentAsString(), Map.class); ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(RegisterRequest.class); verify(mockRegService).register(eq(user), requestCaptor.capture()); @@ -126,11 +126,32 @@ public void testExistingExtNames() throws Exception { assertEquals("Bad reg", getProblem(map)); } + /** + * This test only goes as far as the service level, but checks that external names to ignore + * are received in the post request and used to filter the register request. + */ + @Test + @Transactional + public void testIgnoreExtNames() throws Exception { + User user = creator.createUser("user1"); + tester.setUser(user); + when(mockRegService.register(any(), any())).thenThrow(new ValidationException(List.of("Bad reg"))); + var response = upload("testdata/block_reg_existing.xlsx", null, List.of("Ext17"), false); + var map = objectMapper.readValue(response.getContentAsString(), Map.class); + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(RegisterRequest.class); + verify(mockRegService).register(eq(user), requestCaptor.capture()); + RegisterRequest request = requestCaptor.getValue(); + assertThat(request.getBlocks()).hasSize(1); + BlockRegisterRequest br = request.getBlocks().getFirst(); + assertEquals("EXT18", br.getExternalIdentifier()); + assertEquals("Bad reg", getProblem(map)); + } + private MockHttpServletResponse upload(String filename) throws Exception { - return upload(filename, null, false); + return upload(filename, null, null, false); } - private MockHttpServletResponse upload(String filename, List existing, boolean success) throws Exception { + private MockHttpServletResponse upload(String filename, List existing, List ignore, boolean success) throws Exception { URL url = Resources.getResource(filename); byte[] bytes = Resources.toByteArray(url); MockMultipartFile file = new MockMultipartFile("file", bytes); @@ -138,6 +159,9 @@ private MockHttpServletResponse upload(String filename, List existing, b if (existing!=null) { rb.param("existingExternalNames", existing.toArray(String[]::new)); } + if (ignore!=null) { + rb.param("ignoreExternalNames", ignore.toArray(String[]::new)); + } MvcResult mvcr = tester.getMockMvc() .perform(rb) .andExpect(success ? status().is2xxSuccessful() : status().is4xxClientError()) diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/register/TestFileRegisterService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/register/TestFileRegisterService.java index 9b409440..87d369a3 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/register/TestFileRegisterService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/register/TestFileRegisterService.java @@ -63,8 +63,8 @@ void cleanup() throws Exception { } @ParameterizedTest - @MethodSource("regAndExtNamesArgs") - void testSectionRegister_success(Object request, List existingExt) throws IOException { + @MethodSource("regExtNamesIgnoreArgs") + void testSectionRegister_success(Object request, List existingExt, List ignoreExt) throws IOException { Matchers.mockTransactor(mockTransactor); IRegisterService regService; MultipartFileReader fileReader; @@ -80,10 +80,10 @@ void testSectionRegister_success(Object request, List existingExt) throw } else { regService = mockBlockRegisterService; fileReader = mockBlockFileReader; - if (existingExt==null) { + if (existingExt==null && ignoreExt==null) { functionUnderTest = service::registerBlocks; } else { - functionUnderTest = (user, mpFile) -> service.registerBlocks(user, mpFile, existingExt); + functionUnderTest = (user, mpFile) -> service.registerBlocks(user, mpFile, existingExt, ignoreExt); } } doReturn(request).when(fileReader).read(any(MultipartFile.class)); @@ -98,17 +98,25 @@ void testSectionRegister_success(Object request, List existingExt) throw } else { verify(service, never()).updateWithExisting(any(), any()); } + if (request instanceof RegisterRequest && ignoreExt!=null) { + verify(service).updateToRemove((RegisterRequest) request, ignoreExt); + } else { + verify(service, never()).updateToRemove(any(), any()); + } verify(mockTransactor).transact(any(), any()); } - static Stream regAndExtNamesArgs() { + static Stream regExtNamesIgnoreArgs() { return Arrays.stream(new Object[][] { - {new SectionRegisterRequest(), null}, - {new RegisterRequest(), null}, - {new OriginalSampleRegisterRequest(), null}, - {new OriginalSampleRegisterRequest(), List.of("Alpha1")}, - }).map(Arguments::of); - } + {new SectionRegisterRequest()}, + {new OriginalSampleRegisterRequest()}, + {new RegisterRequest()}, + {new RegisterRequest(), List.of("Alpha1"), null}, + {new RegisterRequest(), null, List.of("Beta")}, + {new RegisterRequest(), List.of("Alpha1"), List.of("Beta")}, + }).map(arr -> arr.length < 3 ? Arrays.copyOf(arr, 3) : arr) + .map(Arguments::of); +} @ParameterizedTest @MethodSource("regArgs") @@ -151,11 +159,8 @@ static Stream regArgs() { public void testUpdateWithExisting(boolean any) { String[] extNames = { null, "Alpha1", "Beta", "Alpha2" }; List blocks = Arrays.stream(extNames) - .map(xn -> { - BlockRegisterRequest b = new BlockRegisterRequest(); - b.setExternalIdentifier(xn); - return b; - }).toList(); + .map(TestFileRegisterService::blockRegWithExternalName) + .toList(); RegisterRequest request = new RegisterRequest(blocks); List existing = any ? List.of("ALPHA1", "alpha2") : List.of(); @@ -164,4 +169,23 @@ public void testUpdateWithExisting(boolean any) { assertEquals(any && (i==1 || i==3), blocks.get(i).isExistingTissue()) ); } + + @ParameterizedTest + @ValueSource(booleans={false,true}) + public void testUpdateToRemove(boolean anyToRemove) { + String[] extNames = { null, "Alpha1", "Beta", "Alpha2" }; + RegisterRequest request = new RegisterRequest(Arrays.stream(extNames) + .map(TestFileRegisterService::blockRegWithExternalName) + .toList()); + List ignore = anyToRemove ? List.of("ALPHA1", "alpha2") : List.of(); + service.updateToRemove(request, ignore); + String[] remaining = (anyToRemove ? new String[]{null, "Beta"} : extNames); + assertThat(request.getBlocks().stream().map(BlockRegisterRequest::getExternalIdentifier)).containsExactly(remaining); + } + + private static BlockRegisterRequest blockRegWithExternalName(String xn) { + BlockRegisterRequest br = new BlockRegisterRequest(); + br.setExternalIdentifier(xn); + return br; + } } \ No newline at end of file