Merge pull request #387 from sanger/blockreg_ignore

x1190 Support "ignoreExternalNames" parameter in block reg post req

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<String> existingExternalNames)
+                                              @RequestParam(value="existingExternalNames", required=false) List<String> existingExternalNames,
+                                              @RequestParam(value="ignoreExternalNames", required=false) List<String> ignoreExternalNames)
             throws URISyntaxException {
-        BiFunction<User, MultipartFile, RegisterResult> biFunction = (user, multipartFile) -> fileRegisterService.registerBlocks(user, multipartFile, existingExternalNames);
+        BiFunction<User, MultipartFile, RegisterResult> biFunction = (user, multipartFile) -> fileRegisterService.registerBlocks(user, multipartFile, existingExternalNames, ignoreExternalNames);
         return receiveFile("block", file, biFunction);
     }
 

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;
 
 /**

src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterService.java

@@ -32,19 +32,20 @@ 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);
     }
 
     /**
      * Registers blocks as described in the given file.
      * @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<String> existingExternalNames)
+    RegisterResult registerBlocks(User user, MultipartFile multipartFile, List<String> existingExternalNames, List<String> ignoreExternalNames)
             throws ValidationException, UncheckedIOException;
 
     /**

src/main/java/uk/ac/sanger/sccp/stan/service/register/FileRegisterServiceImp.java

@@ -55,29 +55,33 @@ public FileRegisterServiceImp(IRegisterService<SectionRegisterRequest> 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 <Req> the type of request to read from the file
      * @param <Res> the type of result expected from the request
      * @exception ValidationException the request fails validation
      * @exception UncheckedIOException there was an IOException reading the file
      */
     protected <Req, Res> Res register(User user, MultipartFile multipartFile, MultipartFileReader<Req> fileReader,
-                                      BiFunction<User, Req, Res> service, List<String> existingExternalNames)
+                                      BiFunction<User, Req, Res> service, List<String> existingExternalNames, List<String> ignoreExternalNames)
             throws ValidationException {
         Req req;
         try {
             req = fileReader.read(multipartFile);
         } 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);
         }
         return transactor.transact("register", () -> service.apply(user, req));
     }
 
     /**
-     * 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<String> 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<String> externalNames) {
+        if (nullOrEmpty(externalNames) || nullOrEmpty(request.getBlocks())) {
+            return;
+        }
+        Set<String> ignoreUC = externalNames.stream()
+                .filter(Objects::nonNull)
+                .map(String::toUpperCase)
+                .collect(toSet());
+        List<BlockRegisterRequest> 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<String> existingExternalNames)
-            throws ValidationException, UncheckedIOException {
-        return register(user, multipartFile, blockFileReader, blockRegisterService::register, existingExternalNames);
+    public RegisterResult registerBlocks(User user, MultipartFile multipartFile, List<String> existingExternalNames,
+                                         List<String> 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);
     }
 }

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<RegisterRequest> requestCaptor = ArgumentCaptor.forClass(RegisterRequest.class);
         verify(mockRegService).register(eq(user), requestCaptor.capture());
@@ -126,18 +126,42 @@ 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<RegisterRequest> 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<String> existing, boolean success) throws Exception {
+    private MockHttpServletResponse upload(String filename, List<String> existing, List<String> ignore, boolean success) throws Exception {
         URL url = Resources.getResource(filename);
         byte[] bytes = Resources.toByteArray(url);
         MockMultipartFile file = new MockMultipartFile("file", bytes);
         final MockMultipartHttpServletRequestBuilder rb = multipart("/register/block").file(file);
         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())

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<String> existingExt) throws IOException {
+    @MethodSource("regExtNamesIgnoreArgs")
+    void testSectionRegister_success(Object request, List<String> existingExt, List<String> ignoreExt) throws IOException {
         Matchers.mockTransactor(mockTransactor);
         IRegisterService<?> regService;
         MultipartFileReader<?> fileReader;
@@ -80,10 +80,10 @@ void testSectionRegister_success(Object request, List<String> 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<String> 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<Arguments> regAndExtNamesArgs() {
+    static Stream<Arguments> 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<Arguments> regArgs() {
     public void testUpdateWithExisting(boolean any) {
         String[] extNames = { null, "Alpha1", "Beta", "Alpha2" };
         List<BlockRegisterRequest> 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<String> 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<String> 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;
+    }
 }