Skip to content

Commit 6fcb730

Browse files
php-coderphp-coder
committed
feat: series images can be replaced.
Fix #1303
1 parent 1512041 commit 6fcb730

26 files changed

+378
-5
lines changed

‎NEWS.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
0.x (upcoming release)
22
- (feature) a series can be marked as a similar to another one
3+
- (feature) series images can be replaced
34

45
0.4.3
56
- (feature) add support for Ukrainian hryvnia

‎src/main/config/spotbugs-filter.xml

+8
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@
4444
</Or>
4545
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />
4646
</Match>
47+
<Match>
48+
<Class name="ru.mystamps.web.feature.image.ReplaceImageDataDbDto" />
49+
<Or>
50+
<Method name="getContent" />
51+
<Method name="setContent" />
52+
</Or>
53+
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />
54+
</Match>
4755
<Match>
4856
<Class name="ru.mystamps.web.feature.image.ImageDto" />
4957
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />

‎src/main/java/ru/mystamps/web/feature/image/DatabaseImagePersistenceStrategy.java

+24
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,30 @@ public void savePreview(byte[] data, ImageInfoDto image) {
6464
log.info("Image #{}: preview has been saved", image.getId());
6565
}
6666

67+
// @todo #1303 DatabaseImagePersistenceStrategy.replace(): add unit tests
68+
@Override
69+
public void replace(byte[] data, ImageInfoDto oldImage, ImageInfoDto newImage) {
70+
ReplaceImageDataDbDto newData = new ReplaceImageDataDbDto();
71+
newData.setImageId(oldImage.getId());
72+
newData.setContent(data);
73+
newData.setPreview(false);
74+
75+
imageDataDao.replace(newData);
76+
log.info("Image #{}: image has been replaced", oldImage.getId());
77+
}
78+
79+
// @todo #1303 DatabaseImagePersistenceStrategy.replacePreview(): add unit tests
80+
@Override
81+
public void replacePreview(byte[] data, ImageInfoDto image) {
82+
ReplaceImageDataDbDto newData = new ReplaceImageDataDbDto();
83+
newData.setImageId(image.getId());
84+
newData.setContent(data);
85+
newData.setPreview(true);
86+
87+
imageDataDao.replace(newData);
88+
log.info("Image #{}: preview has been replaced", image.getId());
89+
}
90+
6791
@Override
6892
public ImageDto get(ImageInfoDto image) {
6993
ImageDto imageDto = imageDataDao.findByImageId(image.getId(), false);

‎src/main/java/ru/mystamps/web/feature/image/FilesystemImagePersistenceStrategy.java

+50
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package ru.mystamps.web.feature.image;
1919

20+
import org.apache.commons.lang3.Validate;
2021
import org.slf4j.Logger;
2122
import org.springframework.web.multipart.MultipartFile;
2223

@@ -27,7 +28,9 @@
2728
import java.nio.file.Path;
2829
import java.nio.file.StandardOpenOption;
2930
import java.util.Locale;
31+
import java.util.Objects;
3032

33+
@SuppressWarnings("PMD.TooManyMethods")
3134
public class FilesystemImagePersistenceStrategy implements ImagePersistenceStrategy {
3235

3336
private final Logger log;
@@ -100,6 +103,48 @@ public void savePreview(byte[] data, ImageInfoDto image) {
100103
}
101104
}
102105

106+
// @todo #1303 FilesystemImagePersistenceStrategy.replace(): add unit tests
107+
@Override
108+
public void replace(byte[] data, ImageInfoDto oldImage, ImageInfoDto newImage) {
109+
Validate.validState(
110+
Objects.equals(oldImage.getId(), newImage.getId()),
111+
"Old and new image must have the same id but they aren't: %d != %d",
112+
oldImage.getId(),
113+
newImage.getId()
114+
);
115+
116+
try {
117+
Path dest = generateFilePath(storageDir, newImage);
118+
rewriteFile(data, dest);
119+
log.info("Image #{}: data ({}) has been replaced", oldImage.getId(), dest);
120+
121+
// when we replace file.jpeg by file.png we have to remove the old one
122+
if (!oldImage.getType().equals(newImage.getType())) {
123+
removeIfPossible(oldImage);
124+
}
125+
126+
} catch (IOException ex) {
127+
throw new ImagePersistenceException(ex);
128+
}
129+
}
130+
131+
// @todo #1303 FilesystemImagePersistenceStrategy.replacePreview(): add unit tests
132+
@Override
133+
public void replacePreview(byte[] data, ImageInfoDto image) {
134+
// we assume that a preview is always generated as JPEG so that
135+
// the new and old files have the same filename
136+
137+
try {
138+
Path dest = generateFilePath(previewDir, image);
139+
rewriteFile(data, dest);
140+
141+
log.info("Image #{}: preview ({}) has been replaced", image.getId(), dest);
142+
143+
} catch (IOException ex) {
144+
throw new ImagePersistenceException(ex);
145+
}
146+
}
147+
103148
@Override
104149
public ImageDto get(ImageInfoDto image) {
105150
return get(storageDir, image, true);
@@ -142,6 +187,11 @@ protected void writeToFile(byte[] data, Path dest) throws IOException {
142187
Files.write(dest, data, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE);
143188
}
144189

190+
// protected to allow spying
191+
protected void rewriteFile(byte[] data, Path dest) throws IOException {
192+
Files.write(dest, data);
193+
}
194+
145195
// protected to allow spying
146196
protected boolean exists(Path path) {
147197
return Files.exists(path);

‎src/main/java/ru/mystamps/web/feature/image/ImageDao.java

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
public interface ImageDao {
2323
Integer add(String type, String filename);
24+
void replace(Integer id, String type, String filename);
2425
void addToSeries(Integer seriesId, Integer imageId);
2526
ImageInfoDto findById(Integer imageId);
2627
List<Integer> findBySeriesId(Integer seriesId);

‎src/main/java/ru/mystamps/web/feature/image/ImageDataDao.java

+1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@
2020
public interface ImageDataDao {
2121
ImageDto findByImageId(Integer imageId, boolean preview);
2222
Integer add(AddImageDataDbDto imageData);
23+
void replace(ReplaceImageDataDbDto imageData);
2324
}

‎src/main/java/ru/mystamps/web/feature/image/ImageInfoDto.java

+8
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@
2727
@EqualsAndHashCode
2828
@ToString
2929
public class ImageInfoDto {
30+
31+
private static final String PREVIEW_TYPE = "jpeg";
32+
3033
private final Integer id;
3134
private final String type;
35+
36+
public static ImageInfoDto newPreview(Integer imageId) {
37+
return new ImageInfoDto(imageId, PREVIEW_TYPE);
38+
}
39+
3240
}

‎src/main/java/ru/mystamps/web/feature/image/ImagePersistenceStrategy.java

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
public interface ImagePersistenceStrategy {
2323
void save(MultipartFile file, ImageInfoDto image);
2424
void savePreview(byte[] data, ImageInfoDto image);
25+
void replace(byte[] data, ImageInfoDto oldImage, ImageInfoDto newImage);
26+
void replacePreview(byte[] data, ImageInfoDto image);
2527
ImageDto get(ImageInfoDto image);
2628
ImageDto getPreview(ImageInfoDto image);
2729
void removeIfPossible(ImageInfoDto image);

‎src/main/java/ru/mystamps/web/feature/image/ImageService.java

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
public interface ImageService {
2525
ImageInfoDto save(MultipartFile file);
26+
void replace(Integer imageId, MultipartFile file);
2627
ImageDto get(Integer imageId);
2728
ImageDto getOrCreatePreview(Integer imageId);
2829
void addToSeries(Integer seriesId, Integer imageId);

‎src/main/java/ru/mystamps/web/feature/image/ImageServiceImpl.java

+65-1
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,15 @@
2727
import ru.mystamps.web.feature.image.ImageDb.Images;
2828
import ru.mystamps.web.support.spring.security.HasAuthority;
2929

30+
import java.io.IOException;
3031
import java.util.List;
3132
import java.util.Locale;
3233

3334
import static org.apache.commons.lang3.StringUtils.substringAfter;
3435
import static org.apache.commons.lang3.StringUtils.substringBefore;
3536

37+
// Complains on duplicated String literal "Image id must be non null"
38+
@SuppressWarnings("PMD.AvoidDuplicateLiterals")
3639
@RequiredArgsConstructor
3740
public class ImageServiceImpl implements ImageService {
3841

@@ -76,6 +79,59 @@ public ImageInfoDto save(MultipartFile file) {
7679

7780
return imageInfo;
7881
}
82+
83+
// @todo #1303 ImageServiceImpl.replace(): add unit tests
84+
// @todo #1303 ImageServiceImpl: reduce duplication between add() and replace()
85+
// @todo #1303 ImageServiceImpl.replace(): ensure that method cleanups file after exception
86+
@Override
87+
@Transactional
88+
@PreAuthorize(HasAuthority.REPLACE_IMAGE)
89+
public void replace(Integer imageId, MultipartFile file) {
90+
Validate.isTrue(imageId != null, "Image id must be non null");
91+
Validate.isTrue(imageId > 0, "Image id must be greater than zero");
92+
93+
Validate.isTrue(file != null, "File must be non null");
94+
Validate.isTrue(file.getSize() > 0, "Image size must be greater than zero");
95+
96+
String contentType = file.getContentType();
97+
Validate.isTrue(contentType != null, "File type must be non null");
98+
99+
String extension = extractExtensionFromContentType(contentType);
100+
Validate.validState(
101+
StringUtils.equalsAny(extension, "png", "jpeg"),
102+
"File type must be PNG or JPEG image, but '%s' (%s) were passed",
103+
contentType, extension
104+
);
105+
106+
String imageType = extension.toUpperCase(Locale.ENGLISH);
107+
108+
// Trim and abbreviate a filename. It shouldn't fail a process because the field is optional
109+
String filename = StringUtils.trimToNull(file.getOriginalFilename());
110+
filename = abbreviateIfLengthGreaterThan(filename, Images.FILENAME_LENGTH);
111+
112+
ImageInfoDto oldImageInfo = imageDao.findById(imageId);
113+
114+
imageDao.replace(imageId, imageType, filename);
115+
log.info(
116+
"Image #{}: meta data has been replaced by '{}', type={}",
117+
imageId,
118+
filename,
119+
imageType
120+
);
121+
122+
byte[] image = getBytes(file);
123+
ImageInfoDto newImageInfo = new ImageInfoDto(imageId, imageType);
124+
imagePersistenceStrategy.replace(image, oldImageInfo, newImageInfo);
125+
126+
// It was also possible to replace an image, remove a preview and let it to be generated
127+
// later, on demand. But this process will be split in time and if a preview generation
128+
// fails, we'll end up with inconsistency between an image and its preview. As such issues
129+
// visible to users and might go unnoticed by admins, we decided to generate both images
130+
// at the same time and have more guarantees regarding consistency.
131+
byte[] preview = imagePreviewStrategy.createPreview(image);
132+
ImageInfoDto previewInfo = ImageInfoDto.newPreview(imageId);
133+
imagePersistenceStrategy.replacePreview(preview, previewInfo);
134+
}
79135

80136
@Override
81137
@Transactional(readOnly = true)
@@ -97,7 +153,7 @@ public ImageDto getOrCreatePreview(Integer imageId) {
97153
Validate.isTrue(imageId != null, "Image id must be non null");
98154
Validate.isTrue(imageId > 0, "Image id must be greater than zero");
99155

100-
ImageInfoDto previewInfo = new ImageInfoDto(imageId, "jpeg");
156+
ImageInfoDto previewInfo = ImageInfoDto.newPreview(imageId);
101157

102158
// NB: the race between getPreview() and createReview() is possible.
103159
// If this happens, the last request will overwrite the first.
@@ -176,4 +232,12 @@ private String abbreviateIfLengthGreaterThan(String text, int maxLength) {
176232
return StringUtils.abbreviate(text, maxLength);
177233
}
178234

235+
private static byte[] getBytes(MultipartFile file) {
236+
try {
237+
return file.getBytes();
238+
} catch (IOException e) {
239+
throw new IllegalStateException(e);
240+
}
241+
}
242+
179243
}

‎src/main/java/ru/mystamps/web/feature/image/JdbcImageDao.java

+20
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ public class JdbcImageDao implements ImageDao {
4040
@Value("${image.add}")
4141
private String addImageSql;
4242

43+
@Value("${image.replace}")
44+
private String replaceImageSql;
45+
4346
@Value("${series_image.add}")
4447
private String addImageToSeriesSql;
4548

@@ -72,6 +75,23 @@ public Integer add(String type, String filename) {
7275
return Integer.valueOf(holder.getKey().intValue());
7376
}
7477

78+
@Override
79+
public void replace(Integer id, String type, String filename) {
80+
Map<String, Object> params = new HashMap<>();
81+
params.put("id", id);
82+
params.put("type", type);
83+
params.put("filename", filename);
84+
85+
int affected = jdbcTemplate.update(replaceImageSql, params);
86+
87+
Validate.validState(
88+
affected == 1,
89+
"Unexpected number of affected rows after replacing image #%d: %d",
90+
id,
91+
affected
92+
);
93+
}
94+
7595
@Override
7696
public void addToSeries(Integer seriesId, Integer imageId) {
7797
Map<String, Object> params = new HashMap<>();

‎src/main/java/ru/mystamps/web/feature/image/JdbcImageDataDao.java

+20
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ public class JdbcImageDataDao implements ImageDataDao {
4141
@Value("${image_data.add}")
4242
private String addImageDataSql;
4343

44+
@Value("${image_data.replace}")
45+
private String replaceImageDataSql;
46+
4447
@Override
4548
public ImageDto findByImageId(Integer imageId, boolean preview) {
4649
Map<String, Object> params = new HashMap<>();
@@ -83,4 +86,21 @@ public Integer add(AddImageDataDbDto imageData) {
8386
return Integer.valueOf(holder.getKey().intValue());
8487
}
8588

89+
@Override
90+
public void replace(ReplaceImageDataDbDto imageData) {
91+
Map<String, Object> params = new HashMap<>();
92+
params.put("image_id", imageData.getImageId());
93+
params.put("content", imageData.getContent());
94+
params.put("preview", imageData.isPreview());
95+
96+
int affected = jdbcTemplate.update(replaceImageDataSql, params);
97+
98+
Validate.validState(
99+
affected == 1,
100+
"Unexpected number of affected rows after replacing data of image #%d: %d",
101+
imageData.getImageId(),
102+
affected
103+
);
104+
}
105+
86106
}

‎src/main/java/ru/mystamps/web/feature/image/ReplaceImageDataDbDto.java

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright (C) 2009-2020 Slava Semushin <[email protected]>
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License as published by
6+
* the Free Software Foundation; either version 2 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program; if not, write to the Free Software
16+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17+
*/
18+
package ru.mystamps.web.feature.image;
19+
20+
import lombok.Getter;
21+
import lombok.Setter;
22+
23+
@Getter
24+
@Setter
25+
public class ReplaceImageDataDbDto {
26+
private Integer imageId;
27+
private byte[] content;
28+
private boolean preview;
29+
}

0 commit comments

Comments
 (0)