diff --git a/pom.xml b/pom.xml index fbe35e79e67a7707ae65efc7418272caa791ebbb..ba64e7471da5692c9bd7ae6a0df35f6763e14d7c 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ <parent> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-starter-parent</artifactId> - <version>2.2.6.RELEASE</version> + <version>2.4.5</version> <relativePath/> <!-- lookup parent from repository --> </parent> <groupId>it.inaf.ia2</groupId> diff --git a/src/main/java/it/inaf/ia2/transfer/controller/ArchiveFileController.java b/src/main/java/it/inaf/ia2/transfer/controller/ArchiveFileController.java index 4b85174cf004680f962e3b1baffa135f74481fc2..60f14d5816f78292cae24dae42bed45619263a35 100644 --- a/src/main/java/it/inaf/ia2/transfer/controller/ArchiveFileController.java +++ b/src/main/java/it/inaf/ia2/transfer/controller/ArchiveFileController.java @@ -25,7 +25,7 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RestController; @RestController -public class ArchiveFileController { +public class ArchiveFileController extends FileController { @Autowired private ArchiveService archiveService; @@ -48,7 +48,7 @@ public class ArchiveFileController { job.setVosPaths(archiveRequest.getPaths()); CompletableFuture.runAsync(() -> { - archiveService.createArchive(job); + handleFileJob(() -> archiveService.createArchive(job), job.getJobId()); }); HttpHeaders headers = new HttpHeaders(); @@ -57,12 +57,12 @@ public class ArchiveFileController { } @GetMapping(value = "/archive/{fileName}") - public ResponseEntity<?> getArchiveFile(@PathVariable("fileName") String fileName) { + public void getArchiveFile(@PathVariable("fileName") String fileName) { TokenPrincipal principal = (TokenPrincipal) request.getUserPrincipal(); File file = archiveService.getArchiveParentDir(principal).toPath().resolve(fileName).toFile(); - return FileResponseUtil.getFileResponse(response, file); + FileResponseUtil.getFileResponse(response, file); } } diff --git a/src/main/java/it/inaf/ia2/transfer/controller/FileController.java b/src/main/java/it/inaf/ia2/transfer/controller/FileController.java index 2b9205602bfa556b68dbfbd7dcacdb77536124c8..1ad60cb0ff63fcd66e3ec450c38ac4b682666906 100644 --- a/src/main/java/it/inaf/ia2/transfer/controller/FileController.java +++ b/src/main/java/it/inaf/ia2/transfer/controller/FileController.java @@ -5,18 +5,28 @@ */ package it.inaf.ia2.transfer.controller; +import it.inaf.ia2.transfer.exception.JobException; +import it.inaf.ia2.transfer.persistence.JobDAO; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; +import net.ivoa.xml.uws.v1.ExecutionPhase; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; public abstract class FileController { + private static final Logger LOG = LoggerFactory.getLogger(FileController.class); + @Autowired protected HttpServletRequest request; + @Autowired + private JobDAO jobDAO; + public String getPath() { String uri = request.getRequestURI().substring(request.getContextPath().length()); @@ -26,4 +36,26 @@ public abstract class FileController { .map(p -> URLDecoder.decode(p, StandardCharsets.UTF_8)) .collect(Collectors.toList())); } + + public void handleFileJob(Runnable action, String jobId) { + try { + action.run(); + if (jobId != null) { + jobDAO.updateJobPhase(ExecutionPhase.COMPLETED, jobId); + } + } catch (Throwable t) { + JobException jobException; + if (t instanceof JobException) { + jobException = (JobException) t; + } else { + LOG.error("Unexpected error happened", t); + jobException = new JobException(JobException.Type.FATAL, "Internal Fault") + .setErrorDetail("InternalFault: Unexpected error happened"); + } + if (jobId != null) { + jobDAO.setJobError(jobId, jobException); + } + throw t; + } + } } diff --git a/src/main/java/it/inaf/ia2/transfer/controller/FileResponseUtil.java b/src/main/java/it/inaf/ia2/transfer/controller/FileResponseUtil.java index 225c048c9aef7fb1fa2a85aa342c5867271c4c30..88a09952fee99bf7ef6ef4df61b1f400182308df 100644 --- a/src/main/java/it/inaf/ia2/transfer/controller/FileResponseUtil.java +++ b/src/main/java/it/inaf/ia2/transfer/controller/FileResponseUtil.java @@ -5,6 +5,8 @@ */ package it.inaf.ia2.transfer.controller; +import it.inaf.ia2.transfer.exception.FileNotFoundException; +import it.inaf.ia2.transfer.exception.JobException; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -16,32 +18,32 @@ import java.nio.charset.StandardCharsets; import javax.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; -import static org.springframework.http.HttpStatus.NOT_FOUND; -import org.springframework.http.ResponseEntity; public class FileResponseUtil { private static final Logger LOG = LoggerFactory.getLogger(FileResponseUtil.class); - public static ResponseEntity<?> getFileResponse(HttpServletResponse response, File file) { - return getFileResponse(response, file, null); + public static void getFileResponse(HttpServletResponse response, File file) { + getFileResponse(response, file, null); } - public static ResponseEntity<?> getFileResponse(HttpServletResponse response, File file, String fileName) { + public static void getFileResponse(HttpServletResponse response, File file, String vosPath) { + + String fileName = vosPath == null ? file.getName() : vosPath.substring(vosPath.lastIndexOf("/") + 1); if (!file.exists()) { LOG.error("File not found: " + file.getAbsolutePath()); - return new ResponseEntity<>("File " + file.getName() + " not found", NOT_FOUND); + throw new FileNotFoundException(vosPath == null ? file.getAbsolutePath() : vosPath); } if (!file.canRead()) { LOG.error("File not readable: " + file.getAbsolutePath()); - return new ResponseEntity<>("File " + file.getName() + " is not readable", INTERNAL_SERVER_ERROR); + throw new JobException(JobException.Type.FATAL, "Internal Fault") + .setErrorDetail("InternalFault: File " + file.getName() + " is not readable"); } response.setHeader("Content-Disposition", "attachment; filename=" - + URLEncoder.encode(fileName == null ? file.getName() : fileName, StandardCharsets.UTF_8)); + + URLEncoder.encode(fileName, StandardCharsets.UTF_8)); response.setHeader("Content-Length", String.valueOf(file.length())); response.setCharacterEncoding("UTF-8"); @@ -54,6 +56,5 @@ public class FileResponseUtil { } catch (IOException ex) { throw new UncheckedIOException(ex); } - return null; } } diff --git a/src/main/java/it/inaf/ia2/transfer/controller/GetFileController.java b/src/main/java/it/inaf/ia2/transfer/controller/GetFileController.java index 1d8753b336b00780dcdaca87bf25ec793052fdca..2b2d41dea7278de88a79f45016af6fe6bb94edd9 100644 --- a/src/main/java/it/inaf/ia2/transfer/controller/GetFileController.java +++ b/src/main/java/it/inaf/ia2/transfer/controller/GetFileController.java @@ -7,7 +7,11 @@ package it.inaf.ia2.transfer.controller; import it.inaf.ia2.transfer.persistence.model.FileInfo; import it.inaf.ia2.transfer.auth.TokenPrincipal; +import it.inaf.ia2.transfer.exception.FileNotFoundException; +import it.inaf.ia2.transfer.exception.InvalidArgumentException; +import it.inaf.ia2.transfer.exception.PermissionDeniedException; import it.inaf.ia2.transfer.persistence.FileDAO; +import it.inaf.ia2.transfer.persistence.JobDAO; import it.inaf.ia2.transfer.service.AuthorizationService; import java.io.File; import java.util.Optional; @@ -15,10 +19,8 @@ import javax.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import static org.springframework.http.HttpStatus.NOT_FOUND; -import static org.springframework.http.HttpStatus.UNAUTHORIZED; -import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @RestController @@ -29,6 +31,9 @@ public class GetFileController extends FileController { @Autowired private FileDAO fileDAO; + @Autowired + private JobDAO jobDAO; + @Autowired private AuthorizationService authorizationService; @@ -36,32 +41,38 @@ public class GetFileController extends FileController { private HttpServletResponse response; @GetMapping("/**") - public ResponseEntity<?> getFile() { + public void getFile(@RequestParam(value = "jobId", required = false) String jobId) { String path = getPath(); LOG.debug("getFile called for path {}", path); - Optional<FileInfo> optFileInfo = fileDAO.getFileInfo(path); - - if (optFileInfo.isPresent()) { - - FileInfo fileInfo = optFileInfo.get(); + if (jobId == null) { + LOG.debug("getFile called for path {}", path); + } else { + LOG.debug("getFile called for path {} with jobId {}", path, jobId); - if (!authorizationService.isDownloadable(fileInfo, (TokenPrincipal) request.getUserPrincipal())) { - return new ResponseEntity<>("Unauthorized", UNAUTHORIZED); + if (!jobDAO.isJobExisting(jobId)) { + throw new InvalidArgumentException("Job " + jobId + " not found"); } - - return getFileResponse(fileInfo); - } else { - return new ResponseEntity<>("File " + path + " not found", NOT_FOUND); } - } - private ResponseEntity<?> getFileResponse(FileInfo fileInfo) { + handleFileJob(() -> { + Optional<FileInfo> optFileInfo = fileDAO.getFileInfo(path); + + if (optFileInfo.isPresent()) { - File file = new File(fileInfo.getOsPath()); + FileInfo fileInfo = optFileInfo.get(); - return FileResponseUtil.getFileResponse(response, file, fileInfo.getVirtualName()); + if (!authorizationService.isDownloadable(fileInfo, (TokenPrincipal) request.getUserPrincipal())) { + throw new PermissionDeniedException(path); + } + + File file = new File(fileInfo.getOsPath()); + FileResponseUtil.getFileResponse(response, file, path); + } else { + throw new FileNotFoundException(path); + } + }, jobId); } } diff --git a/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java b/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java index 5bdbae00166c01bccd1ddaa66234b51fd76599da..2ced2a915ec557c8f8ca9d4b888916f034585c16 100644 --- a/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java +++ b/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java @@ -5,6 +5,8 @@ */ package it.inaf.ia2.transfer.controller; +import it.inaf.ia2.transfer.exception.FileNotFoundException; +import it.inaf.ia2.transfer.exception.InvalidArgumentException; import it.inaf.ia2.transfer.persistence.model.FileInfo; import it.inaf.ia2.transfer.persistence.FileDAO; import it.inaf.ia2.transfer.persistence.JobDAO; @@ -19,12 +21,9 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; import javax.xml.bind.DatatypeConverter; -import net.ivoa.xml.uws.v1.ExecutionPhase; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import static org.springframework.http.HttpStatus.NOT_FOUND; -import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @@ -44,9 +43,9 @@ public class PutFileController extends FileController { private JobDAO jobDAO; @PutMapping("/**") - public ResponseEntity<?> putFile(@RequestHeader(value = HttpHeaders.CONTENT_ENCODING, required = false) String contentEncoding, + public void putFile(@RequestHeader(value = HttpHeaders.CONTENT_ENCODING, required = false) String contentEncoding, @RequestParam(value = "file", required = false) MultipartFile file, - @RequestParam(value = "jobid", required = false) String jobId, + @RequestParam(value = "jobId", required = false) String jobId, HttpServletRequest request) throws IOException, NoSuchAlgorithmException { String path = getPath(); @@ -57,26 +56,29 @@ public class PutFileController extends FileController { LOG.debug("putFile called for path {} with jobId {}", path, jobId); if (!jobDAO.isJobExisting(jobId)) { - return new ResponseEntity<>("Job " + jobId + " not found", NOT_FOUND); + throw new InvalidArgumentException("Job " + jobId + " not found"); } } - Optional<FileInfo> optFileInfo = fileDAO.getFileInfo(path); + handleFileJob(() -> { + Optional<FileInfo> optFileInfo = fileDAO.getFileInfo(path); - if (optFileInfo.isPresent()) { - try (InputStream in = file != null ? file.getInputStream() : request.getInputStream()) { - FileInfo fileInfo = optFileInfo.get(); + if (optFileInfo.isPresent()) { + try (InputStream in = file != null ? file.getInputStream() : request.getInputStream()) { + FileInfo fileInfo = optFileInfo.get(); - if (file != null) { - fileInfo.setContentType(file.getContentType()); + if (file != null) { + fileInfo.setContentType(file.getContentType()); + } + fileInfo.setContentEncoding(contentEncoding); + storeGenericFile(fileInfo, in, jobId); + } catch (IOException | NoSuchAlgorithmException ex) { + throw new RuntimeException(ex); } - fileInfo.setContentEncoding(contentEncoding); - storeGenericFile(fileInfo, in, jobId); + } else { + throw new FileNotFoundException(path); } - return ResponseEntity.ok().build(); - } else { - return new ResponseEntity<>("File " + path + " not found", NOT_FOUND); - } + }, jobId); } private void storeGenericFile(FileInfo fileInfo, InputStream is, String jobId) throws IOException, NoSuchAlgorithmException { @@ -117,15 +119,6 @@ public class PutFileController extends FileController { fileInfo.getContentEncoding(), fileSize, md5Checksum); - if (jobId != null) { - jobDAO.updateJobPhase(ExecutionPhase.COMPLETED, jobId); - } - - } catch (IOException | NoSuchAlgorithmException ex) { - if (jobId != null) { - jobDAO.updateJobPhase(ExecutionPhase.ERROR, jobId); - } - throw ex; } finally { fileDAO.setBusy(fileInfo.getNodeId(), null); } diff --git a/src/main/java/it/inaf/ia2/transfer/exception/ErrorController.java b/src/main/java/it/inaf/ia2/transfer/exception/ErrorController.java new file mode 100644 index 0000000000000000000000000000000000000000..ed77cc8a5bf51906ae676c8b85ac4219b9aa8fa4 --- /dev/null +++ b/src/main/java/it/inaf/ia2/transfer/exception/ErrorController.java @@ -0,0 +1,45 @@ +/* + * This file is part of vospace-rest + * Copyright (C) 2021 Istituto Nazionale di Astrofisica + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package it.inaf.ia2.transfer.exception; + +import java.nio.charset.StandardCharsets; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.web.servlet.error.AbstractErrorController; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.servlet.error.ErrorAttributes; +import org.springframework.http.MediaType; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("${server.error.path:${error.path:/error}}") +public class ErrorController extends AbstractErrorController { + + @Autowired + public ErrorController(ErrorAttributes errorAttributes) { + super(errorAttributes); + } + + @RequestMapping(produces = MediaType.TEXT_XML_VALUE) + public void errorText(HttpServletRequest request, HttpServletResponse response) throws Exception { + ErrorAttributeOptions options = ErrorAttributeOptions.of(ErrorAttributeOptions.Include.MESSAGE); + Map<String, Object> errors = super.getErrorAttributes(request, options); + response.setContentType("text/plain;charset=UTF-8"); + response.setCharacterEncoding("UTF-8"); + String errorMessage = (String) errors.get("message"); + if (errorMessage != null) { + response.getOutputStream().write(errorMessage.getBytes(StandardCharsets.UTF_8)); + } + } + + @Override + public String getErrorPath() { + return null; + } +} diff --git a/src/main/java/it/inaf/ia2/transfer/exception/FileNotFoundException.java b/src/main/java/it/inaf/ia2/transfer/exception/FileNotFoundException.java new file mode 100644 index 0000000000000000000000000000000000000000..459a465a7bd6356b278c9bfbce10b423320ac367 --- /dev/null +++ b/src/main/java/it/inaf/ia2/transfer/exception/FileNotFoundException.java @@ -0,0 +1,18 @@ +/* + * This file is part of vospace-file-service + * Copyright (C) 2021 Istituto Nazionale di Astrofisica + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package it.inaf.ia2.transfer.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(HttpStatus.NOT_FOUND) +public class FileNotFoundException extends JobException { + + public FileNotFoundException(String path) { + super(Type.FATAL, "Node Not Found"); + setErrorDetail("NodeNotFound Path: " + path); + } +} diff --git a/src/main/java/it/inaf/ia2/transfer/exception/InvalidArgumentException.java b/src/main/java/it/inaf/ia2/transfer/exception/InvalidArgumentException.java new file mode 100644 index 0000000000000000000000000000000000000000..6a7c97af6c8d00ab44913bf88db1ae0a184c0fb6 --- /dev/null +++ b/src/main/java/it/inaf/ia2/transfer/exception/InvalidArgumentException.java @@ -0,0 +1,18 @@ +/* + * This file is part of vospace-file-service + * Copyright (C) 2021 Istituto Nazionale di Astrofisica + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package it.inaf.ia2.transfer.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(HttpStatus.BAD_REQUEST) +public class InvalidArgumentException extends JobException { + + public InvalidArgumentException(String message) { + super(Type.FATAL, "Invalid Argument"); + setErrorDetail("InvalidArgument: " + message); + } +} diff --git a/src/main/java/it/inaf/ia2/transfer/persistence/model/JobException.java b/src/main/java/it/inaf/ia2/transfer/exception/JobException.java similarity index 66% rename from src/main/java/it/inaf/ia2/transfer/persistence/model/JobException.java rename to src/main/java/it/inaf/ia2/transfer/exception/JobException.java index 80c354e6061358c000b29366f7165b5db55dc7d3..cb8c4252400cbb7dda00c8d97adfff746aeb4eb7 100644 --- a/src/main/java/it/inaf/ia2/transfer/persistence/model/JobException.java +++ b/src/main/java/it/inaf/ia2/transfer/exception/JobException.java @@ -3,8 +3,12 @@ * Copyright (C) 2021 Istituto Nazionale di Astrofisica * SPDX-License-Identifier: GPL-3.0-or-later */ -package it.inaf.ia2.transfer.persistence.model; +package it.inaf.ia2.transfer.exception; +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) public class JobException extends RuntimeException { public static enum Type { @@ -24,10 +28,10 @@ public class JobException extends RuntimeException { } private final Type type; - private String errorMessage; private String errorDetail; - public JobException(Type type) { + public JobException(Type type, String message) { + super(message); this.type = type; } @@ -35,20 +39,11 @@ public class JobException extends RuntimeException { return type; } - public String getErrorMessage() { - return errorMessage; - } - - public JobException setErrorMessage(String errorMessage) { - this.errorMessage = errorMessage; - return this; - } - public String getErrorDetail() { return errorDetail; } - public JobException setErrorDetail(String errorDetail) { + public final JobException setErrorDetail(String errorDetail) { this.errorDetail = errorDetail; return this; } diff --git a/src/main/java/it/inaf/ia2/transfer/exception/PermissionDeniedException.java b/src/main/java/it/inaf/ia2/transfer/exception/PermissionDeniedException.java new file mode 100644 index 0000000000000000000000000000000000000000..357f69b62cf659c1ca9e1570535ae5ade05168a6 --- /dev/null +++ b/src/main/java/it/inaf/ia2/transfer/exception/PermissionDeniedException.java @@ -0,0 +1,18 @@ +/* + * This file is part of vospace-file-service + * Copyright (C) 2021 Istituto Nazionale di Astrofisica + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package it.inaf.ia2.transfer.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.FORBIDDEN) +public class PermissionDeniedException extends JobException { + + public PermissionDeniedException(String path) { + super(Type.FATAL, "Permission Denied"); + setErrorDetail("PermissionDenied Path: " + path); + } +} diff --git a/src/main/java/it/inaf/ia2/transfer/persistence/JobDAO.java b/src/main/java/it/inaf/ia2/transfer/persistence/JobDAO.java index 907563637f308d1354eb1e8194bccce526c99e6a..e964ba38c51522ec64c2d9f0390abf1ee9a04e57 100644 --- a/src/main/java/it/inaf/ia2/transfer/persistence/JobDAO.java +++ b/src/main/java/it/inaf/ia2/transfer/persistence/JobDAO.java @@ -5,7 +5,7 @@ */ package it.inaf.ia2.transfer.persistence; -import it.inaf.ia2.transfer.persistence.model.JobException; +import it.inaf.ia2.transfer.exception.JobException; import java.sql.Types; import javax.sql.DataSource; import net.ivoa.xml.uws.v1.ExecutionPhase; @@ -73,7 +73,7 @@ public class JobDAO { jdbcTemplate.update(sql, ps -> { int i = 0; ps.setObject(++i, ExecutionPhase.ERROR, Types.OTHER); - ps.setString(++i, jobError.getErrorMessage()); + ps.setString(++i, jobError.getMessage()); ps.setObject(++i, jobError.getType().value(), Types.OTHER); ps.setBoolean(++i, jobError.getErrorDetail() != null); ps.setString(++i, jobError.getErrorDetail()); diff --git a/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java b/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java index 706cbdc756f50802eeae1e2fc0aa76f73aafa842..91cda365a7390bb52e26a057523d327050749633 100644 --- a/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java +++ b/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java @@ -6,12 +6,12 @@ package it.inaf.ia2.transfer.service; import it.inaf.ia2.transfer.auth.TokenPrincipal; +import it.inaf.ia2.transfer.exception.JobException; +import it.inaf.ia2.transfer.exception.JobException.Type; import it.inaf.ia2.transfer.persistence.FileDAO; import it.inaf.ia2.transfer.persistence.JobDAO; import it.inaf.ia2.transfer.persistence.LocationDAO; import it.inaf.ia2.transfer.persistence.model.FileInfo; -import it.inaf.ia2.transfer.persistence.model.JobException; -import it.inaf.ia2.transfer.persistence.model.JobException.Type; import java.io.BufferedOutputStream; import java.io.File; import java.io.FileInputStream; @@ -19,6 +19,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.security.Principal; import java.util.List; @@ -120,18 +121,8 @@ public class ArchiveService { FileSystemUtils.deleteRecursively(supportDir); } - jobDAO.updateJobPhase(ExecutionPhase.COMPLETED, job.getJobId()); - - } catch (Throwable t) { - JobException jobException; - if (t instanceof JobException) { - jobException = (JobException) t; - } else { - LOG.error("Unexpected error happened creating archive", t); - jobException = new JobException(Type.FATAL).setErrorMessage("Internal Fault") - .setErrorDetail("InternalFault: Unexpected error happened creating archive"); - } - jobDAO.setJobError(job.getJobId(), jobException); + } catch (IOException ex) { + throw new UncheckedIOException(ex); } } @@ -142,7 +133,7 @@ public class ArchiveService { if (!parentDir.exists()) { if (!parentDir.mkdirs()) { LOG.error("Unable to create directory " + parentDir.getAbsolutePath()); - throw new JobException(Type.FATAL).setErrorMessage("Internal Fault") + throw new JobException(Type.FATAL, "Internal Fault") .setErrorDetail("InternalFault: Unable to create temporary directory for job"); } } @@ -150,7 +141,7 @@ public class ArchiveService { File archiveFile = parentDir.toPath().resolve(job.getJobId() + "." + job.getType().getExtension()).toFile(); if (!archiveFile.createNewFile()) { LOG.error("Unable to create file " + archiveFile.getAbsolutePath()); - throw new JobException(Type.FATAL).setErrorMessage("Internal Fault") + throw new JobException(Type.FATAL, "Internal Fault") .setErrorDetail("InternalFault: Unable to create archive file"); } @@ -203,7 +194,7 @@ public class ArchiveService { } @Override - public void close() throws Exception { + public void close() throws IOException { os.close(); } } @@ -261,7 +252,7 @@ public class ArchiveService { if (baseUrl == null) { LOG.error("Location URL not found for location " + fileInfo.getLocationId()); - throw new JobException(Type.FATAL).setErrorMessage("Internal Fault") + throw new JobException(Type.FATAL, "Internal Fault") .setErrorDetail("InternalFault: Unable to retrieve location of file " + fileInfo.getVirtualPath()); } @@ -291,7 +282,7 @@ public class ArchiveService { private <O extends OutputStream, E> void writeFileIntoArchive(FileInfo fileInfo, String relPath, TokenPrincipal tokenPrincipal, ArchiveHandler<O, E> handler) throws IOException { if (!authorizationService.isDownloadable(fileInfo, tokenPrincipal)) { - throw new JobException(Type.FATAL).setErrorMessage("Permission Denied") + throw new JobException(Type.FATAL, "Permission Denied") .setErrorDetail("PermissionDenied: " + fileInfo.getVirtualPath()); } diff --git a/src/test/java/it/inaf/ia2/transfer/controller/GetFileControllerTest.java b/src/test/java/it/inaf/ia2/transfer/controller/GetFileControllerTest.java index d67b07a17a775feecf7a138f0a4dd5246000e6c4..f15b20d23f0e93f269bb292d589e777051089aab 100644 --- a/src/test/java/it/inaf/ia2/transfer/controller/GetFileControllerTest.java +++ b/src/test/java/it/inaf/ia2/transfer/controller/GetFileControllerTest.java @@ -183,6 +183,6 @@ public class GetFileControllerTest { mockMvc.perform(get("/path/to/myfile")) .andDo(print()) - .andExpect(status().isUnauthorized()); + .andExpect(status().isForbidden()); } } diff --git a/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java b/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java index e2b84b1f358b323fe2cd94727904fdd146327a43..90531f8cb8d536a4c146aef9765663eec7764af0 100644 --- a/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java +++ b/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java @@ -150,7 +150,7 @@ public class PutFileControllerTest { // Try with invalid jobid mockMvc.perform(putMultipart("/path/to/test.txt") - .file(fakeFile).param("jobid", "pippo10")) + .file(fakeFile).param("jobId", "pippo10")) .andDo(print()) .andExpect(status().is4xxClientError()); @@ -158,7 +158,7 @@ public class PutFileControllerTest { // Retry with valid jobid mockMvc.perform(putMultipart("/path/to/test.txt") - .file(fakeFile).param("jobid", "pippo5")) + .file(fakeFile).param("jobId", "pippo5")) .andDo(print()) .andExpect(status().is2xxSuccessful()); diff --git a/src/test/java/it/inaf/ia2/transfer/persistence/JobDAOTest.java b/src/test/java/it/inaf/ia2/transfer/persistence/JobDAOTest.java index 37685e7935f8193ec1cdfd58ecdec5e730862ac0..155cfaa180d9f1ee9b787472119281198a9f1c82 100644 --- a/src/test/java/it/inaf/ia2/transfer/persistence/JobDAOTest.java +++ b/src/test/java/it/inaf/ia2/transfer/persistence/JobDAOTest.java @@ -5,8 +5,8 @@ */ package it.inaf.ia2.transfer.persistence; -import it.inaf.ia2.transfer.persistence.model.JobException; -import it.inaf.ia2.transfer.persistence.model.JobException.Type; +import it.inaf.ia2.transfer.exception.JobException; +import it.inaf.ia2.transfer.exception.JobException.Type; import javax.sql.DataSource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; @@ -52,8 +52,7 @@ public class JobDAOTest { assertEquals(ExecutionPhase.QUEUED, dao.getJobPhase("pippo3")); - JobException jobError = new JobException(Type.FATAL) - .setErrorMessage("Error message") + JobException jobError = new JobException(Type.FATAL, "Error message") .setErrorDetail("Error detail"); dao.setJobError("pippo3", jobError); diff --git a/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java b/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java index 36816cdea32f2d0acb33cf100b79be85ecfc7c6b..56c073f9aaebba1a0032c9182683a28f2cfc9ed2 100644 --- a/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java +++ b/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.function.Function; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; -import net.ivoa.xml.uws.v1.ExecutionPhase; import org.junit.jupiter.api.AfterAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -41,8 +40,6 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.http.HttpMethod; @@ -203,8 +200,6 @@ public class ArchiveServiceTest { archiveService.createArchive(job); - verify(jobDAO, times(1)).updateJobPhase(eq(ExecutionPhase.COMPLETED), eq(job.getJobId())); - File result = tmpDir.toPath().resolve("user123").resolve("abcdef." + extension).toFile(); assertTrue(result.exists());