From 54b081e70053ddacd250804403d49ec52a510f3b Mon Sep 17 00:00:00 2001 From: Takiguchi Date: Sun, 4 Aug 2019 21:47:58 +0200 Subject: [PATCH] Refactor some code in image service. --- .../core/services/IllegalAccessException.java | 21 ++++ .../org/codiki/images/ImageController.java | 24 ++--- .../java/org/codiki/images/ImageService.java | 54 +++++----- src/main/resources/application.yml | 2 + .../org/codiki/images/ImageServiceTest.java | 100 ++++++++++++++++++ 5 files changed, 159 insertions(+), 42 deletions(-) create mode 100644 src/main/java/org/codiki/core/services/IllegalAccessException.java create mode 100644 src/test/java/org/codiki/images/ImageServiceTest.java diff --git a/src/main/java/org/codiki/core/services/IllegalAccessException.java b/src/main/java/org/codiki/core/services/IllegalAccessException.java new file mode 100644 index 0000000..8e6a3c9 --- /dev/null +++ b/src/main/java/org/codiki/core/services/IllegalAccessException.java @@ -0,0 +1,21 @@ +package org.codiki.core.services; + +/** + * Exception thrown when a user attempts to access to a resource that is not his own or he hasn't access rights. + */ +public class IllegalAccessException extends RuntimeException { + public IllegalAccessException() { + } + + public IllegalAccessException(String message) { + super(message); + } + + public IllegalAccessException(String message, Throwable cause) { + super(message, cause); + } + + public IllegalAccessException(Throwable cause) { + super(cause); + } +} diff --git a/src/main/java/org/codiki/images/ImageController.java b/src/main/java/org/codiki/images/ImageController.java index a9071de..ceb441a 100755 --- a/src/main/java/org/codiki/images/ImageController.java +++ b/src/main/java/org/codiki/images/ImageController.java @@ -1,6 +1,7 @@ package org.codiki.images; import org.codiki.core.entities.dto.ImageDTO; +import org.codiki.core.services.IllegalAccessException; import org.codiki.core.utils.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -11,7 +12,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; -import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.security.Principal; @@ -36,17 +36,17 @@ public class ImageController { @PostMapping("/uploadAvatar") public ResponseEntity uploadAvatar(@RequestParam("file") MultipartFile pFile, - final HttpServletRequest pRequest, - final HttpServletResponse pResponse, final Principal pPrincipal) { LOG.debug("Upload avatar."); ResponseEntity result; try { - result = ResponseEntity.ok(imageService.uploadAvatar(pFile, pResponse, pPrincipal)); - } catch(final Exception pEx) { - LOG.error("Error during avatar upload.", pEx); - result = ResponseEntity.status(HttpStatus.EXPECTATION_FAILED) - .body(StringUtils.concat("Fail to upload ", pFile.getOriginalFilename() + ".")); + result = ResponseEntity.ok(imageService.uploadAvatar(pFile, pPrincipal)); + } catch(IllegalAccessException ex) { + result = ResponseEntity.status(HttpServletResponse.SC_UNAUTHORIZED).build(); + } catch(Exception ex) { + LOG.error("Error during avatar upload.", ex); + result = ResponseEntity.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR) + .body(StringUtils.concat("Fail to upload ", pFile.getOriginalFilename() + ".")); } return result; } @@ -55,13 +55,13 @@ public class ImageController { public ResponseEntity uploadImage(@RequestParam("file") MultipartFile pFile, final HttpServletResponse pResponse, final Principal pPrincipal) throws IOException { - ResponseEntity result = null; + ResponseEntity result; try { result = ResponseEntity.status(HttpStatus.OK) .body(imageService.uploadImage(pFile, pPrincipal)); - } catch(NoSuchElementException pEx) { - pResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED); - } catch(final Exception pEx) { + } catch(IllegalAccessException pEx) { + result = ResponseEntity.status(HttpServletResponse.SC_UNAUTHORIZED).build(); + } catch(Exception ex) { result = ResponseEntity.status(HttpStatus.EXPECTATION_FAILED) .body(StringUtils.concat("Fail to upload ", pFile.getOriginalFilename() + ".")); } diff --git a/src/main/java/org/codiki/images/ImageService.java b/src/main/java/org/codiki/images/ImageService.java index 55547f2..39e7d81 100755 --- a/src/main/java/org/codiki/images/ImageService.java +++ b/src/main/java/org/codiki/images/ImageService.java @@ -6,13 +6,12 @@ import org.codiki.core.entities.persistence.User; import org.codiki.core.repositories.ImageRepository; import org.codiki.core.repositories.UserRepository; import org.codiki.core.services.FileUploadService; +import org.codiki.core.services.IllegalAccessException; import org.codiki.core.services.UserService; import org.springframework.core.io.Resource; import org.springframework.stereotype.Service; import org.springframework.web.multipart.MultipartFile; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; import java.security.Principal; import java.util.Date; import java.util.List; @@ -48,24 +47,23 @@ public class ImageService { this.imageRepository = imageRepository; } + /** + * Save the uploaded file in persistence folder and set it as profil picture to uploader-user. + * @param pFile Uploaded file + * @param pPrincipal Uploader-user. + * @return Name of the uploaded file. + * @throws IllegalAccessException If the user isn't allowed to upload file + */ public String uploadAvatar(final MultipartFile pFile, - final HttpServletResponse pResponse, - final Principal pPrincipal) throws IOException { + final Principal pPrincipal) throws IllegalAccessException { final String avatarFileName = fileUploadService.uploadProfileImage(pFile); - - final Optional connectedUser = userService.getUserByPrincipal(pPrincipal); - if(connectedUser.isPresent()) { - final Optional userFromDb = userRepository.findById(connectedUser.get().getId()); - if(userFromDb.isPresent()) { - userFromDb.get().setImage(avatarFileName); - userRepository.save(userFromDb.get()); - } else { - pResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED); - } - } else { - pResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED); - } - + + userService.getUserByPrincipal(pPrincipal) + .ifPresentOrElse(user -> { + user.setImage(avatarFileName); + userRepository.save(user); + }, IllegalAccessException::new); + return avatarFileName; } @@ -73,16 +71,13 @@ public class ImageService { final String imageFileName = fileUploadService.uploadImage(pFile); userService.getUserByPrincipal(pPrincipal) - .map(User::getId) - .map(userRepository::findById) - .orElseThrow(NoSuchElementException::new) - .ifPresent(user -> { + .ifPresentOrElse(user -> { final Image image = new Image(); image.setLink(imageFileName); image.setDate(new Date()); image.setUser(user); imageRepository.save(image); - }); + }, IllegalAccessException::new); return imageFileName; } @@ -96,13 +91,12 @@ public class ImageService { } public List getUserImages(final Principal pPrincipal) { - return userService.getUserByPrincipal(pPrincipal) - .map(User::getId) - .map(imageRepository::getByUserId) - .orElseThrow(NoSuchElementException::new) - .stream() - .map(ImageDTO::new) - .collect(Collectors.toList()); + return imageRepository.getByUserId(userService.getUserByPrincipal(pPrincipal) + .map(User::getId) + .orElseThrow(NoSuchElementException::new)) + .stream() + .map(ImageDTO::new) + .collect(Collectors.toList()); } public Optional getImageDetails(final String pImageLink) { diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 25418f2..5202365 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -37,6 +37,8 @@ spring: url: jdbc:postgresql://localhost:5432/codiki username: codiki password: P@ssword + # TODO: Delete all Lazy relationships and set following property to false +# open-in-view: false # Because detection is disabled you have to set correct dialect by hand. database-platform: org.hibernate.dialect.PostgreSQL9Dialect # Disable feature detection by this undocumented parameter. diff --git a/src/test/java/org/codiki/images/ImageServiceTest.java b/src/test/java/org/codiki/images/ImageServiceTest.java new file mode 100644 index 0000000..5c6f2eb --- /dev/null +++ b/src/test/java/org/codiki/images/ImageServiceTest.java @@ -0,0 +1,100 @@ +package org.codiki.images; + +import org.codiki.core.entities.dto.ImageDTO; +import org.codiki.core.entities.persistence.Image; +import org.codiki.core.entities.persistence.User; +import org.codiki.core.repositories.ImageRepository; +import org.codiki.core.repositories.UserRepository; +import org.codiki.core.services.FileUploadService; +import org.codiki.core.services.UserService; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.security.Principal; +import java.util.Collections; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +@RunWith(MockitoJUnitRunner.class) +public class ImageServiceTest { + + private ImageService imageService; + + @Mock + private UserService userService; + @Mock + private UserRepository userRepository; + @Mock + private FileUploadService fileUploadService; + @Mock + private ImageRepository imageRepository; + + @Before + public void setUp() { + imageService = new ImageService(userService, + userRepository, + fileUploadService, + imageRepository); + } + + @Test + public void getUserImages_should_return_ImageDTO_list() { + // given + Principal principal = mock(Principal.class); + User user = new User(); + user.setId(1L); + given(userService.getUserByPrincipal(principal)).willReturn(Optional.of(user)); + + Image image = new Image(); + image.setId(2L); + image.setLink("linkValue"); + given(imageRepository.getByUserId(1L)).willReturn(Collections.singletonList(image)); + + // when + List result = imageService.getUserImages(principal); + + // then + assertThat(result).isNotNull().hasSize(1); + assertThat(result.get(0).getId()).isEqualTo(2L); + assertThat(result.get(0).getLink()).isEqualTo("linkValue"); + } + + @Test + public void getUserImages_should_throw_a_NoSuchElementException_if_user_doesnt_exist() { + // given + Principal principal = mock(Principal.class); + given(userService.getUserByPrincipal(principal)).willReturn(Optional.empty()); + + // when + Throwable throwable = catchThrowable(() -> imageService.getUserImages(principal)); + + // then + assertThat(throwable).isNotNull().isExactlyInstanceOf(NoSuchElementException.class); + } + + @Test + public void getUserImages_should_return_an_empty_list_if_user_doesnt_have_any_image() { + // given + Principal principal = mock(Principal.class); + User user = new User(); + user.setId(1L); + given(userService.getUserByPrincipal(principal)).willReturn(Optional.of(user)); + + given(imageRepository.getByUserId(1L)).willReturn(Collections.emptyList()); + + // when + List result = imageService.getUserImages(principal); + + // then + assertThat(result).isNotNull().isEmpty(); + } +}