From fb263b7122cac37de3a8435d1a0e3f349892a833 Mon Sep 17 00:00:00 2001 From: David Robinson <14000840+khelwood@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:39:25 +0100 Subject: [PATCH 1/3] x1158 Support specifying existing external names in file block reg The manual block reg request has an "existing" boolean field to specify that the tissue external name is already in use and that's OK. The file block reg now accepts an additional "existingExternalNames" parameter listing external names that are known and accepted to be existing tissues. --- .../sccp/stan/RegistrationFileController.java | 9 ++- .../service/register/FileRegisterService.java | 17 ++++- .../register/FileRegisterServiceImp.java | 43 ++++++++++-- .../filereader/BlockRegisterFileReader.java | 4 +- .../TestFileBlockRegister.java | 63 +++++++++++++++--- .../register/TestFileRegisterService.java | 55 ++++++++++++--- .../testdata/block_reg_existing.xlsx | Bin 0 -> 10992 bytes 7 files changed, 159 insertions(+), 32 deletions(-) create mode 100644 src/test/resources/testdata/block_reg_existing.xlsx 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 db401e2d..e80fdb6e 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java @@ -50,8 +50,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 existingExternalNames) + throws URISyntaxException { + BiFunction biFunction = (user, multipartFile) -> fileRegisterService.registerBlocks(user, multipartFile, existingExternalNames); + return receiveFile("block", file, biFunction); } @PostMapping(value="/register/original", produces = MediaType.APPLICATION_JSON_VALUE) @@ -68,7 +71,7 @@ public ResponseEntity receiveFile(String desc, MultipartFile file, result = serviceFunction.apply(user, file); } catch (ValidationException e) { log.error("File "+desc+" registration failed.", e); - List problems = e.getProblems().stream().map(Object::toString).collect(toList()); + List problems = e.getProblems().stream().map(Object::toString).toList(); Map> output = Map.of("problems", problems); return ResponseEntity.badRequest().body(output); } 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 29dda8d9..d51d79d1 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 @@ -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 existingExternalNames) + throws ValidationException, UncheckedIOException; /** * Registers original samples as described in the given file. 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 0e1d7284..0f8dd828 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 @@ -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 sectionRe } protected Res register(User user, MultipartFile multipartFile, MultipartFileReader fileReader, - BiFunction service) + BiFunction service, List 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 existingExternalNames) { + if (nullOrEmpty(existingExternalNames) || nullOrEmpty(request.getBlocks())) { + return; + } + Set 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 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); } } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/register/filereader/BlockRegisterFileReader.java b/src/main/java/uk/ac/sanger/sccp/stan/service/register/filereader/BlockRegisterFileReader.java index 02a90eb6..1995624b 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/register/filereader/BlockRegisterFileReader.java +++ b/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; } - - } /** 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 98540d65..42f10d54 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 @@ -3,16 +3,22 @@ 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.request.register.RegisterRequest; +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; @@ -20,6 +26,10 @@ 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,6 +49,9 @@ public class TestFileBlockRegister { @Autowired ObjectMapper objectMapper; + @MockBean + IRegisterService mockRegService; + @Test @Transactional public void testValidationError() throws Exception { @@ -46,9 +59,8 @@ public void testValidationError() throws Exception { 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,51 @@ public void testNoRows() throws Exception { tester.setUser(user); var response = upload("testdata/reg_empty.xlsx"); var map = objectMapper.readValue(response.getContentAsString(), Map.class); - //noinspection unchecked - List problems = (List) map.get("problems"); - assertThat(problems).containsOnly("No registrations requested."); + assertEquals("No registrations requested.", getProblem(map)); + verifyNoInteractions(mockRegService); + } + + /** + * 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")); + 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(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); + } + + private MockHttpServletResponse upload(String filename, List existing) 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(status().is4xxClientError()).andReturn(); return mvcr.getResponse(); } + + @SuppressWarnings("unchecked") + private static String getProblem(Map map) { + List problems = (List) map.get("problems"); + assertThat(problems).hasSize(1); + return problems.getFirst(); + } } 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 dfa3ed27..9b409440 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 @@ -3,8 +3,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.*; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.web.multipart.MultipartFile; @@ -16,12 +15,13 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.Arrays; +import java.util.List; import java.util.function.BiFunction; +import java.util.stream.IntStream; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -52,8 +52,8 @@ class TestFileRegisterService { @BeforeEach void setup() { mocking = MockitoAnnotations.openMocks(this); - service = new FileRegisterServiceImp(mockSectionRegisterService, mockBlockRegisterService,mockOriginalRegisterService, - mockSectionFileReader, mockBlockFileReader, mockOriginalFileReader, mockTransactor); + service = spy(new FileRegisterServiceImp(mockSectionRegisterService, mockBlockRegisterService,mockOriginalRegisterService, + mockSectionFileReader, mockBlockFileReader, mockOriginalFileReader, mockTransactor)); user = EntityFactory.getUser(); } @@ -63,8 +63,8 @@ void cleanup() throws Exception { } @ParameterizedTest - @MethodSource("regArgs") - void testSectionRegister_success(Object request) throws IOException { + @MethodSource("regAndExtNamesArgs") + void testSectionRegister_success(Object request, List existingExt) throws IOException { Matchers.mockTransactor(mockTransactor); IRegisterService regService; MultipartFileReader fileReader; @@ -80,7 +80,11 @@ void testSectionRegister_success(Object request) throws IOException { } else { regService = mockBlockRegisterService; fileReader = mockBlockFileReader; - functionUnderTest = service::registerBlocks; + if (existingExt==null) { + functionUnderTest = service::registerBlocks; + } else { + functionUnderTest = (user, mpFile) -> service.registerBlocks(user, mpFile, existingExt); + } } doReturn(request).when(fileReader).read(any(MultipartFile.class)); RegisterResult result = new RegisterResult(); @@ -89,9 +93,23 @@ void testSectionRegister_success(Object request) throws IOException { verify(fileReader).read(file); //noinspection unchecked,rawtypes verify((IRegisterService) regService).register(user, request); + if (request instanceof RegisterRequest && existingExt!=null) { + verify(service).updateWithExisting((RegisterRequest) request, existingExt); + } else { + verify(service, never()).updateWithExisting(any(), any()); + } verify(mockTransactor).transact(any(), any()); } + static Stream regAndExtNamesArgs() { + return Arrays.stream(new Object[][] { + {new SectionRegisterRequest(), null}, + {new RegisterRequest(), null}, + {new OriginalSampleRegisterRequest(), null}, + {new OriginalSampleRegisterRequest(), List.of("Alpha1")}, + }).map(Arguments::of); + } + @ParameterizedTest @MethodSource("regArgs") void testSectionRegister_IOException(Object request) throws IOException { @@ -127,4 +145,23 @@ static Stream regArgs() { {new OriginalSampleRegisterRequest()}, }).map(Arguments::of); } + + @ParameterizedTest + @ValueSource(booleans={false,true}) + 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(); + RegisterRequest request = new RegisterRequest(blocks); + List existing = any ? List.of("ALPHA1", "alpha2") : List.of(); + + service.updateWithExisting(request, existing); + IntStream.range(0, blocks.size()).forEach(i -> + assertEquals(any && (i==1 || i==3), blocks.get(i).isExistingTissue()) + ); + } } \ No newline at end of file diff --git a/src/test/resources/testdata/block_reg_existing.xlsx b/src/test/resources/testdata/block_reg_existing.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..2bd9ef77ed72e112301d873787f1f8484e00b9ca GIT binary patch literal 10992 zcmeI2Wl$XZw&(}<;K3b&26uP2;32pq_#lI8kijK`1`ol4ySo$I-AQmufZ)7I?m2hw zoxN|pPp{sGdwQz6XS#c>s-FHW|Env(!Q%lC0muLVfC}Jam-5sa1^~!L003|S$gr=Z zfDRx_2avIrr=z8dA*+YI9c2zYEJHQ`7JC1Gw*STdz;MEvO&2>}-wBd;C}e3unks*(*u$BfHLIlq9jNY~ZO{q~XFvv(A0O`wT3-*>U;p(V)EdpF?D2 zy)d{oECrIeu!4+EWD`bZD>!0H5lJv8vLR@=)&2Ost8wt`168*pU9-g|linYdh{Cm4 zN^dqGKr(*qd>%d9Sl!@G=>jRC@6DZc$07Q-HGy{2Qa(u-reTTL6s*H*8EN0|smT(` zia;oz6C=Tb9TP}#sI~#~gqgCd^7ih_-gi@Gc-FgFM7j6vv_7=`>sgKlkJxg#-!Z90 z)Yc%hE+%m7Wf<#?v8Fg8+Xh2`eI7)`h-x^Wxz&D&A67W&a*7Fnes@y>B74_k1!T$T zQ+#5>Qfcm}TA}sw*Mc|j0KnrT96wJaS)uGf2-f!=cN_l==}kz%T4Z*NE|#NnmYGVaPp0lD-zQ)=OkIDl8qj0x0#EX z%lC4Mo(yiSu}sAcg&&j#*Jxy?&ZH}`##nWUQE>A}!B0irzt!(kdAVwKRR%L7sd-ci zuBhkB-j5$k^Pfw~-@_0K6Hq>!e2+iiV)kaC%xA!k>f)MMThmIwy232eS&-V(*ut*k zyL4JR?!C`*)${=kYHpk>&M~=uy6g*o{TlAGp$yL+K9ug_fz!djNQ%N$Xt4Ubk*uQJ zGr&TxOv3>H!~kR%4?DL1vJ-cpv#l8rX!|pe{o7_>pwSF^@Bi+tC4S5X%1qx$P;1bH zuM@<4#rjJOSNyG_!azNV+YQ*q3-r2X+HyghEp!n@VhEjRHOO z@xh_2U_Vs!=nKgGR_Z0G8VcL7Ib*;3Vrcq`*%{hD5%j`cVU7Vx&r2i#027J^l%PMe zQ=+}3y2Xy=Yi#{U_08Kc{p)O4fFf>_kXaq47KUz(YM2)-`G~s*iXbm(+j+Tl;-cf5QOl}5fEksIDLJD0HaCk%qkUdZ zBypEf{P;`eE7?;Ta`cC*XY5BUMcB#;&y!zcW}ku2RquMbgIg3jz4(wQq);n1e{^MT zBqc_O4l*s8$*`I9+;S4bSYzlbP;-LFFTA_Fy6{ zR!R_#NA-LfVwtY8B%geKTN*s>S_V?1aj?@n0=Wc?>`=sprxG`7MI2kUH5lli+_u9U$zG=LfuEY;8I;+Ii@tSs#<*?#Rg zemdpDmrAiDd^l}L1OAjZtOqlt{T)>w3g|DR>^&^loVVo@Qpw#kDL)aRy?xsu@CAob z+^sSh;j%82Qa%7)k0$w!5RSKMfud5zO-UuD(8Dg4K$V@VBP#W#NUwy+uY}d8{hHQv zHvw6}a}lLzjtPc(FIdf5@B|*S|Dy)8xW)!3f=Eq)UJsjSx#jFkZtg5ILO~L1puic^ zJy7pcuTdilJ$uv_o;FHGAHFT>PE_*V7GeA=^z&yW2E|&$U*M;>#Tiw_k6IU^RFR9c zi(It={cDMdZ7({bFQlaqV?RN>OuBmBWr7bAc%7%>i)4oq54ql|avZ?3KPF^F58~oe zg7X5|1SNX!;J=W}4SBE;nAwDu(D8AKH5#z8i+p-7(0k{>Wlbn+`muQe2vD})0*{!Q z&(H620fgUo6mh*W$QaQV8?JoHaJxGpV)bNS8wt2q4qRA{J}(dA%U1Sp2%(6+S(iZq ziW7-0cFDE60oV8WveDP(yZD$_B<4ZqB_t1(?Ks*{-6 z)Z;MHl&3U@J&IT%?kIl4y~rKb*}+k5RcO z%H#|PL~%O!Mb}HenD@_q)MLTF81|u8C0(c5Dd`WHgKKx9BE{I_ERS1I^jxZ5~gQ-A@AR>hJ03Xtf@%WPCgmF4jIF{Nys73H%gA$ ztBjWq85dQdWM$5Ji)GW#7aFS`%e7X7v^_1;Ro5|WewV`DA6J=TU27}k;lxKiI{X00n$yleV z(?#W}`942tHA)$drm;j!zkwC9oGhv*{3}GqatZt!ej?by@Xoa=OgPg1)p(hDtJKRY zvCO>PJ^h@9@AwZF@u$H1j&MX+R>{CLt!tZ#H7FDp;XShp*DP3DsDaHt>H#3li5sYJ)R4e{mb8-AZHpTYr# z?Ut*j+zf8Vn%F7Ae&zM$^>P05E`SU~^Msb$*Z$0<>@fU6%rdq;pfNbGzx>KjF~cyS z%D=2*A>?1zv`0 zpsYyXs}vL?P#_T~cEh|XVENq*270xC!IW~s0Po!r`;eqOPY35Hse?Sdc~CGDHP%N3 z`thMP3pgc=P{O4m9?D?M*#_S6&!MkL;a4y?-;WGPFPPTL))Z9BnOsFNx2zGL)g##| zv;{j-^iFYshLbj7z<80V+D9FXiH_mwBP{}Xj#710Rn(88WXb|Zi@I9RLaUsHpK^?< z;iV08HBe1N3w*WSf;(U|=Hdq}h4j1lcC>x}bbIqr-+PpLWOEp#CzF-x$LZ@I+G45g z6eg%+Ic>jMZrSO2#4{(6yfKj8UL!b%0}J|h4*QZcU2tA2IsoA1kJm0c*wievi8$u_1@7M&8uXIT@vNm zHa|#OC$lGjPh^Aqrfp&~5qGBpx*b|6c)_I_9~V!OyJTh9@@80hy7fy*c$-U2=Y3O? zxz|IY6rI}xild|X9-txj@4+}}oPfm`+G4=R0ssjA$Z%b(EGz>&qJhGXMDI5`XUpcG zR-}>{QmNt2(`6llGvTx{;PC2WmY7aFkz$FtVYmKDIB_d-qytdYPmi?Lvr34{05*7Z z)@o!pn4C3(#|;&ijGdlgxZrI!JELpn{#0GaU3@1gbu7_aKpf1o5s(bL0G zz^+k)Q4WCAjDyj2URxTJZ+u6AL*-RW=6A|ouO77t4PMI%D~WZWMYOQd35!$@An3H< z%8s$Gv2uwGILRb66g}&OB*f2D<;D|bZVWnm`K#@by__I=2z7WG9gK2YghRv2 za}%`_tC_IE66oir;8at7*Hd}nK7o1+-_yZx_RYZde%sh5AdfVRw`Dy%O6Z#9SbAx8 zcL)#ffxZVo0Bf;>Tz*ezj}L!u1>vdQRIP8f+;YRhd7?_gdpO<%yt&dII2!F_3h8Qf z{yH+Gq4xw5qNTCf?YIJq>xWBsKLOWUshQ;+OH12ZGtBrFey*!2@mxCLG{{xIw*jsw zv|}6={;)DMf%`csgunwHAs?T?1PPTRSPNkIszY`fk}J5(;10c+S?ey#xXOiX_Panu5U{9XcpF%wSw^A2(xtRM_2F6`|w z>8;n9$=V04dH~}L7G)kWOsZxno$cyanK8L+u6)egj7#-cBu z-!S>6(zRqIkO4GyUP!t}QTP-QiPn3a?Zzi6?BYZLAY&O`Md*tIAlNvce88U4KoiEo zXN%h1VmpK!6K22ys`6z|1YduI(cN-HFzZ_G_#o0CNs0O^bbFsTfW|3`xTrik6Gg5A z@s0DhL7NHPK_E(6H75$bp|)H~6i_9=$5wyK|Bm9`h5-TvC=X?AGqm!fx^o zfhED4-29~ZpBwSeZIoT4;ittsUOD{B%fB+V*Oj z;#dz3oBk>1@yU8pcU2E_iNFa8;eAU21k=0i()0L zksylOs%a|s??|W~tPCQ2YbDt5*;6-z3#q6Y~F0CcKeR_QvD5UD8=`;{2?%aa9~IB3|aD-YWK^ zeG3a&2T`OYvPCj=(B(3})>C}1Z!9I-dRoM^aTp3{9-U9x&>8lfm2~4iCmvzOl~za2 zGMvS7I~yJ5Q;51wW$?H~T$)~aSxYQ#)~D3ie$>{7E}y8Sq`>*o1kKP7&&@C^V8d(4 z7h}uSQ=2a{qxNRC<8_{+>sDbdOgYJt7|o(|vg`L;EhV;HPv&&xlP)*Hu`aijTa$B= zH%~LVn`{+dO$BN4Z43PPw&2jTBPb}V4`U>vtiMk`7B%cqiNC{-C<%I~&r(#q_o7Pl4#Np9C>1KSFz${G3VsR}-2}#|l zLs5KAvVWC|c6=%FRJO`s8JNHedLCdypkfu?WMyME{oWk1dP^OXkP_^=uh25T`l0iY)id+=>u9%C$Vb|a4!>?vrnl6*)kO;@|wh-94`Eu{1=rpUv zCG4~2E!42#F7`l5IWd%HnZt9|oZhy04>#7muSb4isa zqI7vH*~y$0osh#U+5<69;lfm(K-bCLDB0FHobfI^4It;uWfLkngHz@~;GC?KZ0!2( z*7<`&6`$Jy6F@C4+fbJIBrSo1o=JWALK%*jCa*HWoBIgd*ayqjBvi7+N*Q)3F3 zs^s@PV86;Y0Dxm?Ppkd`WRh)2!k~+Eey#I+}IL@Be$LKGf z(Y_QpFa+BsPQ4AcJKPzMO!1H|;x~$GQk+6FJ}Z7*IDNe#pp{3J6sryvpU-Xq?S)tV zbTZv&%nuODCP(fIAo|ECzmShF8%RU9Oft`tJI#1^krbl7I;PNepZRbYNq+|l zX5it_FT2UGBoJlDB&g75c&ySLu4Hjp*QStE+r!xoW7p{}%&ce^^uAGYJ~lDk$9#E( zDifQ^mc8A)*q(BaJD~DqLZ~Zs=|e%U6N+l~BGz&w_C56V{3~gyVjPuMdG4gZqA*<# zJ^RP$|JTu*Jh;60|AGqVuaW|?X4}P%)xU;xOCh;xySSLBU$P0RYK^X>7~^PNG>K23 zRkb|yU#ra_xuOoDrA;2%Id)z<1Cf}%ogMn|%TDUyIg_pP9oH>sSs z#G8W7Ag;9+dy6!)>-;iOUJ=zc{nH&_@G#}O5~tcr8=gSMfsuU z8AoVYTZTmo;U_A6tv^v&&Jx+HY2bW4WhiVKRpDYu&^BZgIhIt?5Y%WbYO!aFCt}kU z904SGG;@okoU4_yjSz5G85`<`O&71F^1!0G`Be}80^|Q1Dvd<1PSc=XObz_-1hFlO72lib^`0 zO^is+S#&-9o}cyXDzTI=e)>Qt5!fWR+g}eyqEH;eM@IaF zgXWeC!;!A)4dr1c>V!nix&X{&9S4!w9FE*QO%Fj4_WJ~7a3z1R%ZYGZ*0~pZ?)v0B zAE5-qG2El#Fs@BkDC~}4^CI2DgwRti-FK`${kuwTP|Zy}H)IsZgU)nzFI$75g7%Oe z%y{$q9!;b!)DbBPsMNqxcqMaa4oQSNhrmjpy#}Pg;c>NQ#U_+``W zD5c5e^BdyQNK19m9-n&lX$)HD(QfYo5Jw`0BBOYM88OuKd9q6ZTLKX7U><=*{Uxj@ zj3r4Gm37j|@VrT$+T$X=h7X7F4uF1Bk7(P91WhF;r1P|QVKMTfvKHU-ipe{Oz^FEH zv+E3Dr?GQ^Wg}S#t(;D?UF5`kQyj+$BMX!<$F0~OEA5xERc;7b{e#z%Fxl=4+u~Nr z2(ziTqS!H}nG9JFral)=dc|5Y5_;Ie33lVk)sI9FH-jSC5Kfz=I0}S1Ti;x4q$s|= zs_(Adr6;}t{4kK^R0e4n#ou1ryp7d!alDk`#uC5dCp2#@P1XOv1oEhgQ!2yq@phui)Rue^}k8uJl(0e_d|$Z^56( z9H{61)8eDw1^>DPu7m(34FEv@`GLYZO8@iM{{e=RBl7?N literal 0 HcmV?d00001 From 1f6ba2004d4e0cf28921fb4fbbe9b2416a0e6be7 Mon Sep 17 00:00:00 2001 From: David Robinson <14000840+khelwood@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:49:10 +0100 Subject: [PATCH 2/3] x1158 Include clashes in the file registration response Data has to be explicitly put into the response from file registration, because it doesn't go through the graphql layer. --- .../sccp/stan/RegistrationFileController.java | 21 ++++++---- .../TestFileBlockRegister.java | 41 ++++++++++++++++--- 2 files changed, 49 insertions(+), 13 deletions(-) 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 e80fdb6e..73ff2768 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java +++ b/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; @@ -77,15 +76,23 @@ public ResponseEntity receiveFile(String desc, MultipartFile file, } log.info("{} {} file registration: {}", user.getUsername(), desc, result); List barcodes = result.getLabware().stream().map(Labware::getBarcode).collect(toList()); - final Map> output; - if (nullOrEmpty(result.getLabwareSolutions())) { - output = Map.of("barcodes", barcodes); - } else { - output = Map.of("barcodes", barcodes, "labwareSolutions", barcodeSolutions(result.getLabwareSolutions())); + final Map> 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> serialiseClashes(List clashes) { + return clashes.stream() + .>map(clash -> Map.of("tissue", Map.of("externalName", clash.getTissue().getExternalName()))) + .toList(); + } + protected List> barcodeSolutions(Collection labwareSolutions) { return labwareSolutions.stream() .map(lsn -> Map.of("barcode", lsn.getBarcode(), "solution", lsn.getSolutionName())) 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 42f10d54..f12e6ac3 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 @@ -15,8 +15,8 @@ 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.request.register.RegisterRequest; +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; @@ -74,6 +74,32 @@ public void testNoRows() throws Exception { 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>> clashes = (List>>) clashesObj; + assertThat(clashes).hasSize(2); + assertThat(clashes.stream() + .map(clash -> (String) clash.get("tissue").get("externalName")) + ).containsExactlyInAnyOrder("EXT1", "EXT2"); + } + /** * 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. @@ -84,7 +110,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")); + var response = upload("testdata/block_reg_existing.xlsx", 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()); @@ -96,10 +122,10 @@ public void testExistingExtNames() throws Exception { } private MockHttpServletResponse upload(String filename) throws Exception { - return upload(filename, null); + return upload(filename, null, false); } - private MockHttpServletResponse upload(String filename, List existing) throws Exception { + private MockHttpServletResponse upload(String filename, List existing, boolean success) throws Exception { URL url = Resources.getResource(filename); byte[] bytes = Resources.toByteArray(url); MockMultipartFile file = new MockMultipartFile("file", bytes); @@ -107,7 +133,10 @@ private MockHttpServletResponse upload(String filename, List existing) t if (existing!=null) { rb.param("existingExternalNames", existing.toArray(String[]::new)); } - MvcResult mvcr = tester.getMockMvc().perform(rb).andExpect(status().is4xxClientError()).andReturn(); + MvcResult mvcr = tester.getMockMvc() + .perform(rb) + .andExpect(success ? status().is2xxSuccessful() : status().is4xxClientError()) + .andReturn(); return mvcr.getResponse(); } From 90f11e23f86d81d6b8a63b473d2ad2e897e64597 Mon Sep 17 00:00:00 2001 From: David Robinson <14000840+khelwood@users.noreply.github.com> Date: Thu, 4 Apr 2024 09:46:57 +0100 Subject: [PATCH 3/3] x1158 Add labware barcode and type to clash info --- .../sanger/sccp/stan/RegistrationFileController.java | 11 ++++++++++- .../stan/integrationtest/TestFileBlockRegister.java | 9 +++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) 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 73ff2768..f78950f8 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/RegistrationFileController.java @@ -89,7 +89,16 @@ public ResponseEntity receiveFile(String desc, MultipartFile file, protected List> serialiseClashes(List clashes) { return clashes.stream() - .>map(clash -> Map.of("tissue", Map.of("externalName", clash.getTissue().getExternalName()))) + .>map(clash -> Map.of("tissue", Map.of("externalName", clash.getTissue().getExternalName()), + "labware", serialiseClashLabware(clash) + )) + .toList(); + } + + protected List> serialiseClashLabware(RegisterClash clash) { + return clash.getLabware().stream() + .>map(lw -> Map.of("barcode", lw.getBarcode(), + "labwareType", Map.of("name", lw.getLabwareType().getName()))) .toList(); } 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 f12e6ac3..2ea9cf93 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 @@ -96,8 +96,13 @@ public void testClashes() throws Exception { List>> clashes = (List>>) clashesObj; assertThat(clashes).hasSize(2); assertThat(clashes.stream() - .map(clash -> (String) clash.get("tissue").get("externalName")) - ).containsExactlyInAnyOrder("EXT1", "EXT2"); + .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")); + } } /**