From a3b11854944900519d26f0d4203c4f4021d00f1e Mon Sep 17 00:00:00 2001 From: Ivaylo Ivanov Date: Sat, 21 Mar 2020 16:04:53 +0100 Subject: [PATCH] US04, TS20: Make it possible to delete a horse through the API, add better exception handling --- .../individual/endpoint/HorseEndpoint.java | 30 +++++++++++-- .../individual/persistence/FileDao.java | 8 ++++ .../individual/persistence/HorseDao.java | 15 ++++++- .../persistence/impl/HorseFileDao.java | 14 ++++++ .../persistence/impl/HorseJdbcDao.java | 45 +++++++++++++++---- .../individual/service/HorseService.java | 19 +++++--- .../service/impl/SimpleHorseService.java | 9 +++- .../integration/HorseEndpointTest.java | 30 ++++++++++++- .../unit/persistence/HorseDaoTestBase.java | 24 +++++++++- .../unit/persistence/HorseFileDaoTest.java | 19 ++++++++ .../unit/service/HorseServiceTest.java | 3 +- 11 files changed, 192 insertions(+), 24 deletions(-) diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/endpoint/HorseEndpoint.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/endpoint/HorseEndpoint.java index 87e97fe..49e6aea 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/endpoint/HorseEndpoint.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/endpoint/HorseEndpoint.java @@ -57,7 +57,7 @@ public class HorseEndpoint { } catch (DataAccessException e) { LOGGER.error(e.getMessage()); throw new ResponseStatusException(HttpStatus.UNPROCESSABLE_ENTITY, - "Something went wrong during the communication with the database", e); + "Something went wrong during the communication with the database"); } } @@ -72,11 +72,35 @@ public class HorseEndpoint { } catch (ValidationException e) { LOGGER.error(e.getMessage()); throw new ResponseStatusException(HttpStatus.BAD_REQUEST, - "Error during updating horse with id " + id + ": " + e.getMessage(), e); + "Error during updating horse with id " + id + ": " + e.getMessage()); } catch (DataAccessException e) { LOGGER.error(e.getMessage()); throw new ResponseStatusException(HttpStatus.UNPROCESSABLE_ENTITY, - "Something went wrong during the communication with the database", e); + "Something went wrong during the communication with the database"); + } catch (IOException e) { + LOGGER.error(e.getMessage()); + throw new ResponseStatusException(HttpStatus.PARTIAL_CONTENT, + "Operation completed with errors: image could not be saved"); + } + } + + @DeleteMapping(value = "/{id}") + @ResponseStatus(HttpStatus.NO_CONTENT) + public void deleteHorse(@PathVariable("id") Long id) { + LOGGER.info("DELETE " + BASE_URL + "/{}", id); + try { + horseService.deleteHorse(id); + } catch (DataAccessException e) { + LOGGER.error(e.getMessage()); + throw new ResponseStatusException(HttpStatus.UNPROCESSABLE_ENTITY, + "Something went wrong during the communication with the database"); + } catch (NotFoundException e) { + LOGGER.error(e.getMessage()); + throw new ResponseStatusException(HttpStatus.NOT_FOUND, + "The requested horse has not been found"); + } catch (IOException e) { + // Log the error and return as it does not concern the user + LOGGER.error(e.getMessage()); } } diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/FileDao.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/FileDao.java index d32bbc1..af44628 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/FileDao.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/FileDao.java @@ -8,6 +8,14 @@ public interface FileDao { /** * Used for saving files on the local file system * @param file file to save + * @throws IOException if something goes wrong with saving the file */ void save(MultipartFile file) throws IOException; + + /** + * Used for deleting file from the local filesystem + * @param fileName file to delete + * @throws IOException if something goes wrong with deleting the file + */ + void delete(String fileName) throws IOException; } diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/HorseDao.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/HorseDao.java index eeb4e28..ffffc35 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/HorseDao.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/HorseDao.java @@ -4,6 +4,8 @@ import at.ac.tuwien.sepm.assignment.individual.entity.Horse; import at.ac.tuwien.sepm.assignment.individual.exception.NotFoundException; import org.springframework.dao.DataAccessException; +import java.io.IOException; + public interface HorseDao { /** @@ -24,7 +26,16 @@ public interface HorseDao { /** * @param horse that specifies the new horse values alongside with the id of the horse to update * @return the updated horse - * @throws DataAccessException will be thrown if something goes wrong during the database access. + * @throws DataAccessException will be thrown if something goes wrong during the database access. + * @throws IOException will be thrown if the old horse image could not be deleted */ - Horse updateHorse(Horse horse); + Horse updateHorse(Horse horse) throws DataAccessException, IOException; + + /** + * @param id of the horse to delete + * @throws DataAccessException will be thrown if something goes wrong during the database access. + * @throws NotFoundException will be thrown if the horse could not be found in the database. + * @throws IOException will be thrown if the horse image could not be deleted + */ + void deleteHorse(Long id) throws DataAccessException, NotFoundException, IOException; } diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseFileDao.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseFileDao.java index dae30de..0e45e33 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseFileDao.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseFileDao.java @@ -49,4 +49,18 @@ public class HorseFileDao implements FileDao { throw new IOException("Saving the file failed"); } } + + @Override + public void delete(String fileName) throws IOException { + if(fileName == null || fileName.equals("")) + throw new IOException("Cannot delete an unexisting file"); + + LOGGER.trace("Deleting file from " + FILE_BASE_PATH + fileName); + try { + new File(FILE_BASE_PATH + fileName).delete(); + } catch(Exception e) { + LOGGER.error(e.getMessage()); + throw new IOException("Deleting the file specified failed"); + } + } } diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseJdbcDao.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseJdbcDao.java index c374f40..7fccd3b 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseJdbcDao.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/persistence/impl/HorseJdbcDao.java @@ -3,6 +3,7 @@ package at.ac.tuwien.sepm.assignment.individual.persistence.impl; import at.ac.tuwien.sepm.assignment.individual.entity.Horse; import at.ac.tuwien.sepm.assignment.individual.enums.ERace; import at.ac.tuwien.sepm.assignment.individual.exception.NotFoundException; +import at.ac.tuwien.sepm.assignment.individual.persistence.FileDao; import at.ac.tuwien.sepm.assignment.individual.persistence.HorseDao; import at.ac.tuwien.sepm.assignment.individual.util.ValidationException; import org.slf4j.Logger; @@ -16,6 +17,8 @@ import org.springframework.jdbc.support.GeneratedKeyHolder; import org.springframework.jdbc.support.KeyHolder; import org.springframework.stereotype.Repository; +import javax.xml.crypto.Data; +import java.io.IOException; import java.lang.invoke.MethodHandles; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -30,6 +33,7 @@ public class HorseJdbcDao implements HorseDao { private static final Logger LOGGER = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private final JdbcTemplate jdbcTemplate; private final NamedParameterJdbcTemplate namedParameterJdbcTemplate; + private final FileDao fileDao = new HorseFileDao(); public HorseJdbcDao(JdbcTemplate jdbcTemplate, NamedParameterJdbcTemplate namedParameterJdbcTemplate) { this.jdbcTemplate = jdbcTemplate; @@ -87,21 +91,20 @@ public class HorseJdbcDao implements HorseDao { }, keyHolder); if (changes == 0) - throw new NotFoundException("Creating horse failed, no rows affected"); + throw new DataAccessException("Creating horse failed, no rows affected") {}; horse.setId(((Number)keyHolder.getKeys().get("id")).longValue()); return horse; } catch (DataAccessException e) { // We are doing this in order to not change the exception type - throw new DataAccessException("Adding new records failed", e) {}; - } catch(NotFoundException e){ - throw new DataRetrievalFailureException("No new records added", e); + throw new DataAccessException("Adding new records failed", e) { + }; } } @Override - public Horse updateHorse(Horse horse) { + public Horse updateHorse(Horse horse) throws DataAccessException, IOException { LOGGER.trace("Update horse {}", horse.toString()); String sql = "UPDATE " + TABLE_NAME + " SET name=?, description=?, score=?, birthday=?, race=?, image_path=?, owner_id=?, updated_at=? WHERE id=?"; @@ -137,16 +140,40 @@ public class HorseJdbcDao implements HorseDao { }); if (changes == 0) - throw new NotFoundException("Updating horse failed, no rows affected"); + throw new DataAccessException("Updating horse failed, no rows affected") {}; + + horse.setUpdatedAt(oldHorse.getUpdatedAt()); + + fileDao.delete(oldHorse.getImagePath()); - horse.setCreatedAt(oldHorse.getCreatedAt()); return horse; } catch (DataAccessException e) { // We are doing this in order to not change the exception type throw new DataAccessException("Updating records failed", e) {}; - } catch(NotFoundException e){ - throw new DataRetrievalFailureException("No new records updated", e); + } + } + + @Override + public void deleteHorse(Long id) throws DataAccessException, NotFoundException, IOException { + Horse horseToDelete = this.findOneById(id); + LOGGER.trace("Delete horse with id {}", id); + final String sql = "DELETE FROM " + TABLE_NAME + " WHERE id=?"; + + try { + int changes = jdbcTemplate.update(connection -> { + PreparedStatement ps = connection.prepareStatement(sql); + ps.setLong(1, id); + return ps; + }); + + if (changes == 0) + throw new DataAccessException("Deleting horse failed, no rows affected") {}; + + fileDao.delete(horseToDelete.getImagePath()); + } catch(DataAccessException e){ + // We are doing this in order to not change the exception type + throw new DataAccessException("Deleting records failed", e) {}; } } diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/HorseService.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/HorseService.java index c0be2be..3b108d5 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/HorseService.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/HorseService.java @@ -20,7 +20,7 @@ public interface HorseService { /** * @param horse to add. * @return the new horse. - * @throws ValidationException will be thrown if something goes wrong during verification. + * @throws ValidationException will be thrown if something goes wrong during verification. * @throws DataAccessException will be thrown if the horse could not be saved in the database. */ Horse addHorse(Horse horse); @@ -28,15 +28,24 @@ public interface HorseService { /** * @param horse that specifies the new horse values alongside with the id of the horse to update * @return the updated horse - * @throws ValidationException will be thrown if something goes wrong during verification. + * @throws ValidationException will be thrown if something goes wrong during verification. * @throws DataAccessException will be thrown if the horse could not be saved in the database. + * @throws IOException will be thrown if the old horse image could not be deleted */ - Horse updateHorse(Horse horse); + Horse updateHorse(Horse horse) throws ValidationException, DataAccessException, IOException; + + /** + * @param id of the horse to delete + * @throws NotFoundException will be thrown if the horse could not be found in the system + * @throws DataAccessException will be thrown if the horse could not be deleted from the database + * @throws IOException will be thrown if the horse image could not be deleted + */ + void deleteHorse(Long id) throws NotFoundException, DataAccessException, IOException; /** * @param img image to upload - * @throws IOException will be thrown if something goes wrong with saving the file + * @throws IOException will be thrown if something goes wrong with saving the file * @throws ValidationException will be thrown if the file is in the incorrect format */ - void saveImage(MultipartFile img) throws IOException; + void saveImage(MultipartFile img) throws IOException, ValidationException; } diff --git a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/impl/SimpleHorseService.java b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/impl/SimpleHorseService.java index c4ee63e..677fd11 100644 --- a/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/impl/SimpleHorseService.java +++ b/backend/src/main/java/at/ac/tuwien/sepm/assignment/individual/service/impl/SimpleHorseService.java @@ -1,6 +1,7 @@ package at.ac.tuwien.sepm.assignment.individual.service.impl; import at.ac.tuwien.sepm.assignment.individual.entity.Horse; +import at.ac.tuwien.sepm.assignment.individual.exception.NotFoundException; import at.ac.tuwien.sepm.assignment.individual.persistence.FileDao; import at.ac.tuwien.sepm.assignment.individual.persistence.HorseDao; import at.ac.tuwien.sepm.assignment.individual.service.HorseService; @@ -44,12 +45,18 @@ public class SimpleHorseService implements HorseService { } @Override - public Horse updateHorse(Horse horse) { + public Horse updateHorse(Horse horse) throws ValidationException, DataAccessException, IOException { this.validator.validateUpdateHorse(horse); LOGGER.trace("updateHorse({})", horse.toString()); return horseJdbcDao.updateHorse(horse); } + @Override + public void deleteHorse(Long id) throws NotFoundException, DataAccessException, IOException { + LOGGER.trace("deleteHorse({})", id); + horseJdbcDao.deleteHorse(id); + } + @Override public void saveImage(MultipartFile img) throws ValidationException, IOException { this.validator.validateHorseImage(img); diff --git a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/integration/HorseEndpointTest.java b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/integration/HorseEndpointTest.java index c93c89c..3d4125d 100644 --- a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/integration/HorseEndpointTest.java +++ b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/integration/HorseEndpointTest.java @@ -9,11 +9,11 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.web.server.LocalServerPort; +import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.io.ClassPathResource; import org.springframework.http.*; import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.context.event.annotation.AfterTestMethod; -import org.springframework.test.context.jdbc.Sql; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.client.RestTemplate; @@ -111,4 +111,30 @@ public class HorseEndpointTest { new File(FILE_BASE_PATH + "horse.jpg").delete(); } } + + @Test + @DisplayName("Deleting an existing horse will return HTTP 204") + public void deletingHorse_existing_shouldReturnStatus204() { + // Create the horse + HorseDto newHorse = new HorseDto("Zephyr", "Nice horse", (short) 4, Date.valueOf("2020-01-01"), ERace.APPALOOSA, "files/test.png", null); + + HttpEntity request = new HttpEntity<>(newHorse); + ResponseEntity response = REST_TEMPLATE + .exchange(BASE_URL + port + HORSE_URL, HttpMethod.POST, request, HorseDto.class); + + // Delete and test if deleted + ResponseEntity res = REST_TEMPLATE + .exchange(BASE_URL + port + HORSE_URL + '/' + response.getBody().getId(), HttpMethod.DELETE, null, new ParameterizedTypeReference() {}); + + assertEquals(res.getStatusCode(), HttpStatus.NO_CONTENT); + } + + @Test + @DisplayName("Deleting an nonexistent horse will return HTTP 404") + public void deletingHorse_nonexistent_shouldReturnStatus404() { + assertThrows(HttpClientErrorException.NotFound.class, () -> + REST_TEMPLATE + .exchange(BASE_URL + port + HORSE_URL + "/0", HttpMethod.DELETE, null, new ParameterizedTypeReference() {})); + } } + diff --git a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseDaoTestBase.java b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseDaoTestBase.java index d7553c0..6d8e5c0 100644 --- a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseDaoTestBase.java +++ b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseDaoTestBase.java @@ -4,12 +4,14 @@ import static org.junit.jupiter.api.Assertions.*; import at.ac.tuwien.sepm.assignment.individual.entity.Horse; import at.ac.tuwien.sepm.assignment.individual.enums.ERace; +import at.ac.tuwien.sepm.assignment.individual.exception.NotFoundException; import at.ac.tuwien.sepm.assignment.individual.persistence.HorseDao; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataAccessException; +import java.io.IOException; import java.sql.Date; public abstract class HorseDaoTestBase { @@ -34,7 +36,7 @@ public abstract class HorseDaoTestBase { @Test @DisplayName("Updating a horse with the correct parameters should return the horse") - public void updatingHorse_correctParameters_shouldReturnHorse() { + public void updatingHorse_correctParameters_shouldReturnHorse() throws IOException { // Create horse Horse newHorse = new Horse("Zephyr", "Nice horse", (short) 4, Date.valueOf("2020-01-01"), ERace.APPALOOSA, "files/test.png", null); Horse savedHorse = horseDao.addHorse(newHorse); @@ -80,4 +82,24 @@ public abstract class HorseDaoTestBase { assertThrows(DataAccessException.class, () -> horseDao.addHorse(newHorse)); } + @Test + @DisplayName("Deleting an existing horse should delete the horse") + public void deletingHorse_existing_shouldDeleteHorse() throws IOException { + // Create the horse + Horse newHorse = new Horse("Zephyr", "Nice horse", (short) 4, Date.valueOf("2020-01-01"), ERace.APPALOOSA, "files/test.png", null); + Horse savedHorse = horseDao.addHorse(newHorse); + + // Delete the horse + horseDao.deleteHorse(savedHorse.getId()); + + // Check if deleted + assertThrows(NotFoundException.class, () -> horseDao.findOneById(savedHorse.getId())); + } + + @Test + @DisplayName("Deleting an nonexistent horse should throw NotFoundException") + public void deletingHorse_nonexistent_shouldThrowNotFound() throws IOException { + assertThrows(NotFoundException.class, () -> horseDao.deleteHorse(null)); + } + } diff --git a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseFileDaoTest.java b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseFileDaoTest.java index f07b9eb..60bc8aa 100644 --- a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseFileDaoTest.java +++ b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/persistence/HorseFileDaoTest.java @@ -43,6 +43,25 @@ public class HorseFileDaoTest { assertThrows(IOException.class, () -> horseFileDao.save(new MockMultipartFile("file", image.getName(), MediaType.TEXT_HTML_VALUE, new FileInputStream(image)))); } + @Test + @DisplayName("Deleting a file with a correct name should result in a deleted file") + public void deletingFile_correctName_shouldDeleteFile() throws FileNotFoundException, IOException { + // Save the file + File image = new File("src/test/resources/at/ac/tuwien/sepm/assignment/individual/integration/horse.jpg"); + horseFileDao.save(new MockMultipartFile("file", image.getName(), MediaType.IMAGE_JPEG_VALUE, new FileInputStream(image))); + + // Then delete it + horseFileDao.delete("horse.jpg"); + + assertFalse(new File(FILE_BASE_PATH + "horse.jpg").exists()); + } + + @Test + @DisplayName("Deleting a file with an empty name should throw an Exception") + public void deletingFile_emptyName_shouldThrowIO() { + assertThrows(IOException.class, () -> horseFileDao.delete("")); + } + @AfterEach public void cleanup() { new File(FILE_BASE_PATH + "horse.jpg").delete(); diff --git a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/service/HorseServiceTest.java b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/service/HorseServiceTest.java index ea71dd0..4e1dca7 100644 --- a/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/service/HorseServiceTest.java +++ b/backend/src/test/java/at/ac/tuwien/sepm/assignment/individual/unit/service/HorseServiceTest.java @@ -14,6 +14,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit.jupiter.SpringExtension; +import java.io.IOException; import java.sql.Date; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -42,7 +43,7 @@ public class HorseServiceTest { @Test @DisplayName("Updating a horse with the correct parameters will return the new horse") - public void updatingHorse_correctParameters_shouldReturnHorse() { + public void updatingHorse_correctParameters_shouldReturnHorse() throws IOException { // Create horse Horse newHorse = new Horse("Zephyr", "Nice horse", (short) 4, Date.valueOf("2020-01-01"), ERace.APPALOOSA, "files/test.png", null); Horse savedHorse = horseService.addHorse(newHorse);