SeriesService.addImageToSeries(): remove uploaded file when exception occurs.

Addressed to #301

Author: php-coder

src/main/java/ru/mystamps/web/service/DatabaseImagePersistenceStrategy.java

@@ -98,4 +98,11 @@ public ImageDto getPreview(ImageInfoDto image) {
 		return imageDto;
 	}
 
+	@Override
+	public void removeIfPossible(ImageInfoDto image) {
+		// It's supposed that this method will be used for removing a file when exception occurs.
+		// In such case it's impossible to modify database because a whole transaction will be
+		// rolled back.
+	}
+	
 }

src/main/java/ru/mystamps/web/service/FilesystemImagePersistenceStrategy.java

@@ -117,6 +117,16 @@ public ImageDto getPreview(ImageInfoDto image) {
 		return get(previewDir, image, false);
 	}
 
+	@Override
+	public void removeIfPossible(ImageInfoDto image) {
+		Path dest = generateFilePath(storageDir, image);
+		try {
+			Files.deleteIfExists(dest);
+		} catch (Exception ex) { // NOPMD: AvoidCatchingGenericException
+			LOG.warn("Couldn't delete file {}: {}", dest, ex.getMessage());
+		}
+	}
+	
 	// protected to allow spying
 	protected Path generateFilePath(File dir, ImageInfoDto image) {
 		return new File(dir, generateFileName(image)).toPath();

src/main/java/ru/mystamps/web/service/ImagePersistenceStrategy.java

@@ -27,4 +27,5 @@ public interface ImagePersistenceStrategy {
 	void savePreview(byte[] data, ImageInfoDto image);
 	ImageDto get(ImageInfoDto image);
 	ImageDto getPreview(ImageInfoDto image);
+	void removeIfPossible(ImageInfoDto image);
 }

src/main/java/ru/mystamps/web/service/ImageService.java

@@ -30,4 +30,5 @@ public interface ImageService {
 	ImageDto getOrCreatePreview(Integer imageId);
 	void addToSeries(Integer seriesId, Integer imageId);
 	List<Integer> findBySeriesId(Integer seriesId);
+	void removeIfPossible(ImageInfoDto imageInfo);
 }

src/main/java/ru/mystamps/web/service/ImageServiceImpl.java

@@ -145,6 +145,13 @@ public List<Integer> findBySeriesId(Integer seriesId) {
 		return imageDao.findBySeriesId(seriesId);
 	}
 
+	@Override
+	public void removeIfPossible(ImageInfoDto imageInfo) {
+		Validate.isTrue(imageInfo != null, "Image info must be non null");
+		
+		imagePersistenceStrategy.removeIfPossible(imageInfo);
+	}
+	
 	private ImageDto createPreview(ImageInfoDto previewInfo, byte[] image) {
 		try {
 			byte[] preview = imagePreviewStrategy.createPreview(image);

src/main/java/ru/mystamps/web/service/SeriesServiceImpl.java

@@ -171,7 +171,14 @@ public void addImageToSeries(AddImageDto dto, Integer seriesId, Integer userId)
 
 		ImageInfoDto imageInfo = imageService.save(dto.getImage());
 		Integer imageId = imageInfo.getId();
-		imageService.addToSeries(seriesId, imageId);
+		
+		try {
+			imageService.addToSeries(seriesId, imageId);
+			
+		} catch (RuntimeException ex) { // NOPMD: AvoidCatchingGenericException
+			imageService.removeIfPossible(imageInfo);
+			throw ex;
+		}
 
 		LOG.info(
 			"Image #{} has been added to series #{} by user #{}",

src/test/groovy/ru/mystamps/web/service/ImageServiceImplTest.groovy

@@ -318,4 +318,28 @@ class ImageServiceImplTest extends Specification {
 			result == expectedResult
 	}
 
+	//
+	// Tests for removeIfPossible()
+	//
+	
+	def "removeIfPossible() should throw exception when image info is null"() {
+		when:
+			service.removeIfPossible(null)
+		then:
+			thrown IllegalArgumentException
+	}
+	
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
+	def "removeIfPossible() should pass argument to strategy"() {
+		given:
+			ImageInfoDto expectedImageInfo = TestObjects.createImageInfoDto()
+		when:
+			service.removeIfPossible(expectedImageInfo)
+		then:
+			1 * imagePersistenceStrategy.removeIfPossible({ ImageInfoDto imageInfo ->
+				assert imageInfo == expectedImageInfo
+				return true
+			})
+	}
+	
 }

src/test/groovy/ru/mystamps/web/service/SeriesServiceImplTest.groovy

@@ -601,6 +601,25 @@ class SeriesServiceImplTest extends Specification {
 			})
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
+	def "addImageToSeries() should remove image when exception occurs"() {
+		given:
+			ImageInfoDto expectedImageInfo = new ImageInfoDto(456, 'JPEG')
+		and:
+			imageService.addToSeries(_ as Integer, _ as Integer) >> { throw new IllegalStateException() }
+		when:
+			service.addImageToSeries(imageForm, 111, 222)
+		then:
+			imageService.save(_) >> expectedImageInfo
+		and:
+			1 * imageService.removeIfPossible({ ImageInfoDto imageInfo ->
+				assert imageInfo == expectedImageInfo
+				return true
+			})
+		and:
+			thrown IllegalStateException
+	}
+	
 	//
 	// Tests for countAll()
 	//