Merge pull request #375 from sanger/register_clash

x1158 Support specifying existing external names in file block reg

src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java

@@ -14,8 +14,7 @@
 import org.springframework.web.multipart.MultipartFile;
 import uk.ac.sanger.sccp.stan.model.Labware;
 import uk.ac.sanger.sccp.stan.model.User;
-import uk.ac.sanger.sccp.stan.request.register.LabwareSolutionName;
-import uk.ac.sanger.sccp.stan.request.register.RegisterResult;
+import uk.ac.sanger.sccp.stan.request.register.*;
 import uk.ac.sanger.sccp.stan.service.ValidationException;
 import uk.ac.sanger.sccp.stan.service.register.FileRegisterService;
 
@@ -50,8 +49,11 @@ public ResponseEntity<?> receiveSectionFile(@RequestParam("file") MultipartFile
     }
 
     @PostMapping(value = "/register/block", produces = MediaType.APPLICATION_JSON_VALUE)
-    public ResponseEntity<?> receiveBlockFile(@RequestParam("file") MultipartFile file) throws URISyntaxException {
-        return receiveFile("block", file, fileRegisterService::registerBlocks);
+    public ResponseEntity<?> receiveBlockFile(@RequestParam("file") MultipartFile file,
+                                              @RequestParam(value="existingExternalNames", required=false) List<String> existingExternalNames)
+            throws URISyntaxException {
+        BiFunction<User, MultipartFile, RegisterResult> biFunction = (user, multipartFile) -> fileRegisterService.registerBlocks(user, multipartFile, existingExternalNames);
+        return receiveFile("block", file, biFunction);
     }
 
     @PostMapping(value="/register/original", produces = MediaType.APPLICATION_JSON_VALUE)
@@ -68,21 +70,38 @@ public ResponseEntity<?> receiveFile(String desc, MultipartFile file,
             result = serviceFunction.apply(user, file);
         } catch (ValidationException e) {
             log.error("File "+desc+" registration failed.", e);
-            List<String> problems = e.getProblems().stream().map(Object::toString).collect(toList());
+            List<String> problems = e.getProblems().stream().map(Object::toString).toList();
             Map<String, List<String>> output = Map.of("problems", problems);
             return ResponseEntity.badRequest().body(output);
         }
         log.info("{} {} file registration: {}", user.getUsername(), desc, result);
         List<String> barcodes = result.getLabware().stream().map(Labware::getBarcode).collect(toList());
-        final Map<String, List<?>> output;
-        if (nullOrEmpty(result.getLabwareSolutions())) {
-            output = Map.of("barcodes", barcodes);
-        } else {
-            output = Map.of("barcodes", barcodes, "labwareSolutions", barcodeSolutions(result.getLabwareSolutions()));
+        final Map<String, List<?>> output = new HashMap<>();
+        output.put("barcodes", barcodes);
+        if (!nullOrEmpty(result.getLabwareSolutions())) {
+            output.put("labwareSolutions", barcodeSolutions(result.getLabwareSolutions()));
+        }
+        if (!nullOrEmpty(result.getClashes())) {
+            output.put("clashes", serialiseClashes(result.getClashes()));
         }
         return ResponseEntity.ok().body(output);
     }
 
+    protected List<Map<String, ?>> serialiseClashes(List<RegisterClash> clashes) {
+        return clashes.stream()
+                .<Map<String,?>>map(clash -> Map.of("tissue", Map.of("externalName", clash.getTissue().getExternalName()),
+                        "labware", serialiseClashLabware(clash)
+                ))
+                .toList();
+    }
+
+    protected List<Map<String, ?>> serialiseClashLabware(RegisterClash clash) {
+        return clash.getLabware().stream()
+                .<Map<String,?>>map(lw -> Map.of("barcode", lw.getBarcode(),
+                        "labwareType", Map.of("name", lw.getLabwareType().getName())))
+                .toList();
+    }
+
     protected List<Map<String, String>> barcodeSolutions(Collection<LabwareSolutionName> labwareSolutions) {
         return labwareSolutions.stream()
                 .map(lsn -> Map.of("barcode", lsn.getBarcode(), "solution", lsn.getSolutionName()))

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

@@ -6,6 +6,7 @@
 import uk.ac.sanger.sccp.stan.service.ValidationException;
 
 import java.io.UncheckedIOException;
+import java.util.List;
 
 public interface FileRegisterService {
 
@@ -27,7 +28,21 @@ public interface FileRegisterService {
      * @exception ValidationException the data received is invalid
      * @exception UncheckedIOException the file cannot be read
      */
-    RegisterResult registerBlocks(User user, MultipartFile multipartFile) throws ValidationException, UncheckedIOException;
+    default RegisterResult registerBlocks(User user, MultipartFile multipartFile) throws ValidationException, UncheckedIOException {
+        return registerBlocks(user, multipartFile, 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
+     * @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)
+            throws ValidationException, UncheckedIOException;
 
     /**
      * Registers original samples as described in the given file.

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

@@ -11,8 +11,12 @@
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.util.*;
 import java.util.function.BiFunction;
 
+import static java.util.stream.Collectors.toSet;
+import static uk.ac.sanger.sccp.utils.BasicUtils.nullOrEmpty;
+
 /**
  * @author dr6
  */
@@ -44,30 +48,55 @@ public FileRegisterServiceImp(IRegisterService<SectionRegisterRequest> sectionRe
     }
 
     protected <Req, Res> Res register(User user, MultipartFile multipartFile, MultipartFileReader<Req> fileReader,
-                                      BiFunction<User, Req, Res> service)
+                                      BiFunction<User, Req, Res> service, List<String> existingExternalNames)
             throws ValidationException {
-        Req req ;
+        Req req;
         try {
             req = fileReader.read(multipartFile);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
-
+        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
+     * are found in the given list of existing external names.
+     * @param request a block register request
+     * @param existingExternalNames list of known existing external names
+     */
+    public void updateWithExisting(RegisterRequest request, List<String> existingExternalNames) {
+        if (nullOrEmpty(existingExternalNames) || nullOrEmpty(request.getBlocks())) {
+            return;
+        }
+        Set<String> externalNamesUC = existingExternalNames.stream()
+                .filter(Objects::nonNull)
+                .map(String::toUpperCase)
+                .collect(toSet());
+        for (BlockRegisterRequest block : request.getBlocks()) {
+            if (block != null && !nullOrEmpty(block.getExternalIdentifier())
+                    && externalNamesUC.contains(block.getExternalIdentifier().toUpperCase())) {
+                block.setExistingTissue(true);
+            }
+        }
+    }
+
     @Override
     public RegisterResult registerSections(User user, MultipartFile multipartFile) throws ValidationException {
-        return register(user, multipartFile, sectionFileReader, sectionRegisterService::register);
+        return register(user, multipartFile, sectionFileReader, sectionRegisterService::register, null);
     }
 
     @Override
-    public RegisterResult registerBlocks(User user, MultipartFile multipartFile) throws ValidationException, UncheckedIOException {
-        return register(user, multipartFile, blockFileReader, blockRegisterService::register);
+    public RegisterResult registerBlocks(User user, MultipartFile multipartFile, List<String> existingExternalNames)
+            throws ValidationException, UncheckedIOException {
+        return register(user, multipartFile, blockFileReader, blockRegisterService::register, existingExternalNames);
     }
 
     @Override
     public RegisterResult registerOriginal(User user, MultipartFile multipartFile) throws ValidationException, UncheckedIOException {
-        return register(user, multipartFile, originalSampleFileReader, originalSampleRegisterService::register);
+        return register(user, multipartFile, originalSampleFileReader, originalSampleRegisterService::register, null);
     }
 }

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

@@ -22,7 +22,7 @@ enum Column implements IColumn {
         Work_number(Pattern.compile("(work|sgp)\\s*number", Pattern.CASE_INSENSITIVE)),
         Donor_identifier(Pattern.compile("donor\\s*id.*", Pattern.CASE_INSENSITIVE)),
         Life_stage,
-        Collection_date(LocalDate.class, Pattern.compile("(if.*)date.*collection.*", Pattern.CASE_INSENSITIVE)),
+        Collection_date(LocalDate.class, Pattern.compile("(if.*)?(date.*collection|collection.*date).*", Pattern.CASE_INSENSITIVE)),
         Species,
         HuMFre(Pattern.compile("humfre\\s*(number)?", Pattern.CASE_INSENSITIVE)),
         Tissue_type,
@@ -73,8 +73,6 @@ public Pattern getPattern() {
         public boolean isRequired() {
             return this.dataType != Void.class;
         }
-
-
     }
 
     /**

src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestFileBlockRegister.java

@@ -3,23 +3,33 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.io.Resources;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
 import org.springframework.context.annotation.Import;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockMultipartFile;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.request.MockMultipartHttpServletRequestBuilder;
 import uk.ac.sanger.sccp.stan.*;
-import uk.ac.sanger.sccp.stan.model.User;
+import uk.ac.sanger.sccp.stan.model.*;
+import uk.ac.sanger.sccp.stan.request.register.*;
+import uk.ac.sanger.sccp.stan.service.ValidationException;
+import uk.ac.sanger.sccp.stan.service.register.IRegisterService;
 
 import javax.transaction.Transactional;
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.*;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
@@ -39,16 +49,18 @@ public class TestFileBlockRegister {
     @Autowired
     ObjectMapper objectMapper;
 
+    @MockBean
+    IRegisterService<RegisterRequest> mockRegService;
+
     @Test
     @Transactional
     public void testValidationError() throws Exception {
         User user = creator.createUser("user1");
         tester.setUser(user);
         var response = upload("testdata/block_reg.xlsx");
         var map = objectMapper.readValue(response.getContentAsString(), Map.class);
-        List<?> problems = (List<?>) map.get("problems");
-        assertThat(problems).hasSize(1);
-        assertThat((String) problems.get(0)).matches(".*work number.*");
+        assertThat(getProblem(map)).matches(".*work number.*");
+        verifyNoInteractions(mockRegService);
     }
 
     @Test
@@ -58,18 +70,85 @@ public void testNoRows() throws Exception {
         tester.setUser(user);
         var response = upload("testdata/reg_empty.xlsx");
         var map = objectMapper.readValue(response.getContentAsString(), Map.class);
+        assertEquals("No registrations requested.", getProblem(map));
+        verifyNoInteractions(mockRegService);
+    }
+
+    @Test
+    @Transactional
+    public void testClashes() throws Exception {
+        User user = creator.createUser("user1");
+        tester.setUser(user);
+        Tissue tissue1 = creator.createTissue(null, "EXT1");
+        Tissue tissue2 = creator.createTissue(tissue1.getDonor(), "EXT2");
+        Sample sample1 = creator.createSample(tissue1, null);
+        Sample sample2 = creator.createSample(tissue2, null);
+        Labware lw1 = creator.createBlock("STAN-X", sample1);
+        Labware lw2 = creator.createBlock("STAN-Y", sample2);
+        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 map = objectMapper.readValue(response.getContentAsString(), Map.class);
+        Object clashesObj = map.get("clashes");
+        assertNotNull(clashesObj);
         //noinspection unchecked
-        List<String> problems = (List<String>) map.get("problems");
-        assertThat(problems).containsOnly("No registrations requested.");
+        List<Map<String, Map<String, ?>>> clashes = (List<Map<String, Map<String, ?>>>) clashesObj;
+        assertThat(clashes).hasSize(2);
+        assertThat(clashes.stream()
+                .map(clash -> (String) clash.get("tissue").get("externalName")))
+                .containsExactlyInAnyOrder("EXT1", "EXT2");
+        for (var clash : clashes) {
+            String bc = clash.get("tissue").get("externalName").equals("EXT1") ? "STAN-X" : "STAN-Y";
+            assertEquals(List.of(Map.of("barcode", bc,"labwareType", Map.of("name", "Proviasette"))),
+                    clash.get("labware"));
+        }
+    }
+
+    /**
+     * This test only goes as far as the service level, but checks that existing external names
+     * are received in the post request and incorporated into the register request.
+     */
+    @Test
+    @Transactional
+    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 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(2);
+        assertTrue(request.getBlocks().get(0).isExistingTissue());
+        assertFalse(request.getBlocks().get(1).isExistingTissue());
+        assertEquals("Bad reg", getProblem(map));
     }
 
     private MockHttpServletResponse upload(String filename) throws Exception {
+        return upload(filename, null, false);
+    }
+
+    private MockHttpServletResponse upload(String filename, List<String> existing, boolean success) throws Exception {
         URL url = Resources.getResource(filename);
         byte[] bytes = Resources.toByteArray(url);
         MockMultipartFile file = new MockMultipartFile("file", bytes);
-        MvcResult mvcr = tester.getMockMvc().perform(
-                multipart("/register/block").file(file)
-        ).andExpect(status().is4xxClientError()).andReturn();
+        final MockMultipartHttpServletRequestBuilder rb = multipart("/register/block").file(file);
+        if (existing!=null) {
+            rb.param("existingExternalNames", existing.toArray(String[]::new));
+        }
+        MvcResult mvcr = tester.getMockMvc()
+                .perform(rb)
+                .andExpect(success ? status().is2xxSuccessful() : status().is4xxClientError())
+                .andReturn();
         return mvcr.getResponse();
     }
+
+    @SuppressWarnings("unchecked")
+    private static String getProblem(Map<?, ?> map) {
+        List<String> problems = (List<String>) map.get("problems");
+        assertThat(problems).hasSize(1);
+        return problems.getFirst();
+    }
 }