From c243d83990492dfa5d201c9d8c3bffc79ab8c57e Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Tue, 23 Jul 2024 14:23:54 +0200 Subject: [PATCH 1/3] [kbss/record-manager-ui#190] Add one import handler for all types of files --- .../study/rest/PatientRecordController.java | 136 ++++++++++-------- 1 file changed, 73 insertions(+), 63 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index 5e8dae63..aa3d2b87 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -6,6 +6,7 @@ import cz.cvut.kbss.study.dto.PatientRecordDto; import cz.cvut.kbss.study.dto.RecordImportResult; import cz.cvut.kbss.study.exception.NotFoundException; +import cz.cvut.kbss.study.exception.ValidationException; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.RecordPhase; import cz.cvut.kbss.study.model.User; @@ -61,7 +62,7 @@ public class PatientRecordController extends BaseController { public PatientRecordController(PatientRecordService recordService, ApplicationEventPublisher eventPublisher, ExcelRecordConverter excelRecordConverter, RestTemplate restTemplate, ConfigReader configReader, ObjectMapper objectMapper, - UserService userService) { + UserService userService) { this.recordService = recordService; this.eventPublisher = eventPublisher; this.excelRecordConverter = excelRecordConverter; @@ -78,7 +79,7 @@ public List getRecords( @RequestParam MultiValueMap params, UriComponentsBuilder uriBuilder, HttpServletResponse response) { final Page result = recordService.findAll(RecordFilterMapper.constructRecordFilter(params), - RestUtils.resolvePaging(params)); + RestUtils.resolvePaging(params)); eventPublisher.publishEvent(new PaginatedResultRetrievedEvent(this, uriBuilder, response, result)); return result.getContent(); } @@ -98,17 +99,17 @@ public ResponseEntity exportRecords( .map(o -> o.filter(l -> !l.isEmpty())) .filter(Optional::isPresent) .map(o -> o.map(l -> l.stream().flatMap(s -> - MediaType.parseMediaTypes(s).stream() - .filter(RestUtils::isSupportedExportType) - ).max(Comparator.comparing(MediaType::getQualityValue)).orElse(null))) + MediaType.parseMediaTypes(s).stream() + .filter(RestUtils::isSupportedExportType) + ).max(Comparator.comparing(MediaType::getQualityValue)).orElse(null))) .filter(Optional::isPresent) .map(o -> o.orElse(null)) .findFirst() .orElse(MediaType.APPLICATION_JSON) .removeQualityValue(); - return switch (exportType.toString()){ - case Constants.MEDIA_TYPE_EXCEL -> exportRecordsExcel(params, uriBuilder, response); + return switch (exportType.toString()) { + case Constants.MEDIA_TYPE_EXCEL -> exportRecordsExcel(params, uriBuilder, response); case MediaType.APPLICATION_JSON_VALUE -> exportRecordsAsJson(params, uriBuilder, response); default -> throw new IllegalArgumentException("Unsupported export type: " + exportType); }; @@ -116,7 +117,7 @@ public ResponseEntity exportRecords( protected ResponseEntity> exportRecordsAsJson( MultiValueMap params, - UriComponentsBuilder uriBuilder, HttpServletResponse response){ + UriComponentsBuilder uriBuilder, HttpServletResponse response) { final Page result = recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), RestUtils.resolvePaging(params)); eventPublisher.publishEvent(new PaginatedResultRetrievedEvent(this, uriBuilder, response, result)); @@ -126,7 +127,7 @@ protected ResponseEntity> exportRecordsAsJson( } public ResponseEntity exportRecordsExcel(MultiValueMap params, - UriComponentsBuilder uriBuilder, HttpServletResponse response){ + UriComponentsBuilder uriBuilder, HttpServletResponse response) { RecordFilterParams filterParams = new RecordFilterParams(); filterParams.setMinModifiedDate(null); filterParams.setMaxModifiedDate(null); @@ -157,13 +158,11 @@ private PatientRecord findInternal(String key) { return record; } - - @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.CREATED) public ResponseEntity createRecord(@RequestBody PatientRecord record) { - if(userService.getCurrentUser().getInstitution() == null) + if (userService.getCurrentUser().getInstitution() == null) return ResponseEntity.status(HttpStatus.CONFLICT).body("User is not assigned to any institution"); recordService.persist(record); @@ -176,22 +175,22 @@ public ResponseEntity createRecord(@RequestBody PatientRecord record) { } @PreAuthorize( - "hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @PostMapping(value = "/publish", produces = {MediaType.APPLICATION_JSON_VALUE}) public RecordImportResult publishRecords( - @RequestParam(name = "institution", required = false) String institutionKey, - @RequestParam(required = false) MultiValueMap params, - HttpServletRequest request) { + @RequestParam(name = "institution", required = false) String institutionKey, + @RequestParam(required = false) MultiValueMap params, + HttpServletRequest request) { String onPublishRecordsServiceUrl = configReader.getConfig(ConfigParam.ON_PUBLISH_RECORDS_SERVICE_URL); - if(onPublishRecordsServiceUrl == null || onPublishRecordsServiceUrl.isBlank()) { + if (onPublishRecordsServiceUrl == null || onPublishRecordsServiceUrl.isBlank()) { LOG.info("No publish service url provided, noop."); RecordImportResult result = new RecordImportResult(0); result.addError("Cannot publish completed records. Publish server not configured."); return result; } - // export + // export final Page result = recordService.findAllFull(RecordFilterMapper.constructRecordFilter(params), RestUtils.resolvePaging(params)); List records = result.getContent(); @@ -227,7 +226,7 @@ public String getFilename() { // Call the import endpoint LOG.debug("Publishing records."); ResponseEntity responseEntity = restTemplate.postForEntity( - onPublishRecordsServiceUrl, requestEntity, RecordImportResult.class); + onPublishRecordsServiceUrl, requestEntity, RecordImportResult.class); // TODO make records published @@ -235,32 +234,58 @@ public String getFilename() { return responseEntity.getBody(); } - @PostMapping(value = "/import/json", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public RecordImportResult importRecordsJson(@RequestPart("file") MultipartFile file, - @RequestParam(name = "phase", required = false) String phase) { + @PostMapping(value = "/import", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) + public RecordImportResult importRecords( + @RequestPart("file") MultipartFile file, + @RequestParam(name = "phase", required = false) String phase) { + + if (file.isEmpty()) { + throw new ValidationException("Cannot import records, missing input file"); + } + + String filename = file.getOriginalFilename(); + + if (filename == null) { + throw new ValidationException("Cannot import records, missing filename"); + } List records; - if(file.isEmpty()) - throw new IllegalArgumentException("Cannot import records, missing input file"); + if (filename.endsWith(".json")) { + records = getRecordsJson(file, phase); + } else if (filename.endsWith(".xls") || filename.endsWith(".xlsx")) { + records = getRecordsExcel(file, phase); + } else if (filename.endsWith(".tsv")) { + records = getRecordsTsv(file, phase) + } else { + throw new IllegalArgumentException("Unsupported file type: " + filename); + } + + final RecordImportResult importResult; + if (phase != null) { + final RecordPhase targetPhase = RecordPhase.fromIriOrName(phase); + importResult = recordService.importRecords(records, targetPhase); + } else { + importResult = recordService.importRecords(records); + } + LOG.trace("Records imported with result: {}.", importResult); + return importResult; + } + + private List getRecordsJson(MultipartFile file, String phase) { + List records; try { - records = objectMapper.readValue(file.getBytes(), new TypeReference>(){}); + return objectMapper.readValue(file.getBytes(), new TypeReference>() { + }); } catch (IOException e) { throw new RuntimeException("Failed to parse JSON content", e); } - return importRecords(records, phase); } - @PostMapping(value = "/import/excel", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public RecordImportResult importRecordsExcel( - @RequestPart("file") MultipartFile file, - @RequestParam(name = "phase", required = false) String phase) { + private List getRecordsExcel(MultipartFile file, String phase) { List records; - if(file.isEmpty()) - throw new IllegalArgumentException("Cannot import records, missing input file"); - String excelImportServiceUrl = configReader.getConfig(ConfigParam.EXCEL_IMPORT_SERVICE_URL); if (excelImportServiceUrl == null) @@ -273,8 +298,8 @@ public RecordImportResult importRecordsExcel( body.add("files", file.getResource()); String request = UriComponentsBuilder.fromHttpUrl(excelImportServiceUrl) - .queryParam("datasetResource", "@%s".formatted(file.getOriginalFilename())) - .toUriString(); + .queryParam("datasetResource", "@%s".formatted(file.getOriginalFilename())) + .toUriString(); HttpEntity> requestEntity = new HttpEntity<>(body, headers); @@ -282,20 +307,17 @@ public RecordImportResult importRecordsExcel( URI.create(request), HttpMethod.POST, requestEntity, - new ParameterizedTypeReference>() {} + new ParameterizedTypeReference>() { + } ); - records = responseEntity.getBody(); - return importRecords(records, phase); + return responseEntity.getBody(); } - @PostMapping(value = "/import/tsv", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public RecordImportResult importRecordsTsv( - @RequestPart("file") MultipartFile file, - @RequestParam(name = "phase", required = false) String phase) { + private RecordImportResult getRecordsTsv(MultipartFile file, String phase) { List records; - if(file.isEmpty()) + if (file.isEmpty()) throw new IllegalArgumentException("Cannot import records, missing input file"); String excelImportServiceUrl = configReader.getConfig(ConfigParam.EXCEL_IMPORT_SERVICE_URL); @@ -310,15 +332,15 @@ public RecordImportResult importRecordsTsv( body.add("files", file.getResource()); String request = UriComponentsBuilder.fromHttpUrl(excelImportServiceUrl) - .queryParam("datasetResource", "@%s".formatted(file.getOriginalFilename())) - .toUriString(); + .queryParam("datasetResource", "@%s".formatted(file.getOriginalFilename())) + .toUriString(); HttpEntity> requestEntity = new HttpEntity<>(body, headers); ResponseEntity responseEntity = restTemplate.postForEntity( - URI.create(request), - requestEntity, - byte[].class + URI.create(request), + requestEntity, + byte[].class ); LOG.info("Import finished with status {}", responseEntity.getStatusCode()); @@ -330,18 +352,6 @@ public RecordImportResult importRecordsTsv( return new RecordImportResult(); } - public RecordImportResult importRecords(List records, String phase) { - final RecordImportResult importResult; - if (phase != null) { - final RecordPhase targetPhase = RecordPhase.fromIriOrName(phase); - importResult = recordService.importRecords(records, targetPhase); - } else { - importResult = recordService.importRecords(records); - } - LOG.trace("Records imported with result: {}.", importResult); - return importResult; - } - @PutMapping(value = "/{key}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) @@ -359,13 +369,13 @@ public void updateRecord(@PathVariable("key") String key, @RequestBody PatientRe onUpdateRecord(record.getUri()); } - private void onUpdateRecord(URI uri){ + private void onUpdateRecord(URI uri) { Objects.nonNull(uri); String onRecordUpdateServiceUrl = Optional.ofNullable(configReader) .map(r -> r.getConfig(ConfigParam.ON_UPDATE_RECORD_SERVICE_URL)) .orElse(null); - if(onRecordUpdateServiceUrl == null || onRecordUpdateServiceUrl.isBlank()) { + if (onRecordUpdateServiceUrl == null || onRecordUpdateServiceUrl.isBlank()) { LOG.debug("No onRecordUpdateServiceUrl service url provided, noop."); return; } @@ -374,7 +384,7 @@ private void onUpdateRecord(URI uri){ String requestUrl = UriComponentsBuilder.fromHttpUrl(onRecordUpdateServiceUrl) .queryParam("record", uri) .toUriString(); - restTemplate.getForObject(requestUrl,String.class); + restTemplate.getForObject(requestUrl, String.class); } @DeleteMapping(value = "/{key}") From 4f2f6e41df995c35635bdb4afe7652e14375ebac Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Tue, 23 Jul 2024 23:16:25 +0200 Subject: [PATCH 2/3] [kbss/record-manager-ui#190] Correct return type in getRecordsFromTsv --- .../kbss/study/rest/PatientRecordController.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index aa3d2b87..892c97f7 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.sql.Array; import java.util.*; import java.util.stream.Stream; @@ -252,11 +253,11 @@ public RecordImportResult importRecords( List records; if (filename.endsWith(".json")) { - records = getRecordsJson(file, phase); + records = getRecordsFromJson(file, phase); } else if (filename.endsWith(".xls") || filename.endsWith(".xlsx")) { - records = getRecordsExcel(file, phase); + records = getRecordsFromExcel(file, phase); } else if (filename.endsWith(".tsv")) { - records = getRecordsTsv(file, phase) + records = getRecordsFromTsv(file, phase); } else { throw new IllegalArgumentException("Unsupported file type: " + filename); } @@ -272,7 +273,7 @@ public RecordImportResult importRecords( return importResult; } - private List getRecordsJson(MultipartFile file, String phase) { + private List getRecordsFromJson(MultipartFile file, String phase) { List records; try { return objectMapper.readValue(file.getBytes(), new TypeReference>() { @@ -282,7 +283,7 @@ private List getRecordsJson(MultipartFile file, String phase) { } } - private List getRecordsExcel(MultipartFile file, String phase) { + private List getRecordsFromExcel(MultipartFile file, String phase) { List records; @@ -313,7 +314,7 @@ private List getRecordsExcel(MultipartFile file, String phase) { return responseEntity.getBody(); } - private RecordImportResult getRecordsTsv(MultipartFile file, String phase) { + private List getRecordsFromTsv(MultipartFile file, String phase) { List records; @@ -349,7 +350,7 @@ private RecordImportResult getRecordsTsv(MultipartFile file, String phase) { LOG.debug("Response body length is {}", responseBody.length); } - return new RecordImportResult(); + return new ArrayList(); } From 7514dc4e789711e2be68c065d5ea8cfa783167c7 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Tue, 23 Jul 2024 23:17:22 +0200 Subject: [PATCH 3/3] [kbss/record-manager-ui#190] Correct request's endpoints in tests for importRecords handler --- .../cvut/kbss/study/rest/PatientRecordControllerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java index 250d0ce7..b36eb72d 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java @@ -340,7 +340,7 @@ void importRecordsJsonImportsSpecifiedRecordsAndReturnsImportResult() throws Exc MediaType.MULTIPART_FORM_DATA_VALUE, toJson(records).getBytes()); final MvcResult mvcResult = mockMvc.perform( - multipart("/records/import/json") + multipart("/records/import") .file(file) .contentType(MediaType.MULTIPART_FORM_DATA_VALUE) ).andReturn(); @@ -370,7 +370,7 @@ void importRecordsJsonImportsSpecifiedRecordsWithSpecifiedPhaseAndReturnsImportR MediaType.MULTIPART_FORM_DATA_VALUE, toJson(records).getBytes()); mockMvc.perform( - multipart("/records/import/json") + multipart("/records/import") .file(file) .contentType(MediaType.MULTIPART_FORM_DATA_VALUE) .param("phase", targetPhase.getIri()) @@ -389,7 +389,7 @@ void importRecordsJsonReturnsConflictWhenServiceThrowsRecordAuthorNotFound() thr MediaType.MULTIPART_FORM_DATA_VALUE, toJson(records).getBytes()); mockMvc.perform( - multipart("/records/import/json") + multipart("/records/import") .file(file) .contentType(MediaType.MULTIPART_FORM_DATA_VALUE) ).andExpect(status().isConflict());