fix review findings

This commit is contained in:
2026-04-17 15:12:00 -06:00
parent ced651db32
commit f3ed5e0b9a
4 changed files with 30 additions and 54 deletions

View File

@@ -24,6 +24,7 @@ import com.petshop.backend.service.AvatarStorageService;
import com.petshop.backend.service.EmailService; import com.petshop.backend.service.EmailService;
import com.petshop.backend.service.PasswordResetService; import com.petshop.backend.service.PasswordResetService;
import com.petshop.backend.util.AuthenticationHelper; import com.petshop.backend.util.AuthenticationHelper;
import com.petshop.backend.util.ImageValidationUtil;
import com.petshop.backend.util.PhoneUtils; import com.petshop.backend.util.PhoneUtils;
import com.petshop.backend.util.StringUtils; import com.petshop.backend.util.StringUtils;
import jakarta.validation.Valid; import jakarta.validation.Valid;
@@ -296,18 +297,7 @@ public class AuthController {
public ResponseEntity<AvatarUploadResponse> uploadAvatar(@RequestParam("avatar") MultipartFile file) { public ResponseEntity<AvatarUploadResponse> uploadAvatar(@RequestParam("avatar") MultipartFile file) {
User user = getAuthenticatedUser(); User user = getAuthenticatedUser();
if (file.isEmpty()) { ImageValidationUtil.validate(file);
throw new BusinessException("Please select a file to upload");
}
if (file.getSize() > 5 * 1024 * 1024) {
throw new BusinessException("File size must not exceed 5MB");
}
String contentType = file.getContentType();
if (contentType == null || (!contentType.equals("image/jpeg") && !contentType.equals("image/png") && !contentType.equals("image/gif"))) {
throw new BusinessException("Only JPG, PNG, and GIF images are allowed");
}
try { try {
avatarStorageService.deleteAvatar(user); avatarStorageService.deleteAvatar(user);

View File

@@ -1,10 +1,12 @@
package com.petshop.backend.controller; package com.petshop.backend.controller;
import com.petshop.backend.entity.User; import com.petshop.backend.entity.User;
import com.petshop.backend.exception.BusinessException;
import com.petshop.backend.exception.ResourceNotFoundException;
import com.petshop.backend.repository.UserRepository; import com.petshop.backend.repository.UserRepository;
import com.petshop.backend.service.AvatarStorageService; import com.petshop.backend.service.AvatarStorageService;
import com.petshop.backend.util.ImageValidationUtil;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.DeleteMapping;
@@ -15,6 +17,8 @@ import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController; import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartFile;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException; import java.io.IOException;
import java.util.HashMap; import java.util.HashMap;
@@ -24,6 +28,8 @@ import java.util.Map;
@RequestMapping("/api/v1/users") @RequestMapping("/api/v1/users")
public class UserAvatarController { public class UserAvatarController {
private static final Logger log = LoggerFactory.getLogger(UserAvatarController.class);
private final UserRepository userRepository; private final UserRepository userRepository;
private final AvatarStorageService avatarStorageService; private final AvatarStorageService avatarStorageService;
@@ -53,29 +59,10 @@ public class UserAvatarController {
@PostMapping("/{userId}/avatar") @PostMapping("/{userId}/avatar")
@PreAuthorize("hasRole('ADMIN')") @PreAuthorize("hasRole('ADMIN')")
public ResponseEntity<?> uploadUserAvatar(@PathVariable Long userId, @RequestParam("avatar") MultipartFile file) { public ResponseEntity<?> uploadUserAvatar(@PathVariable Long userId, @RequestParam("avatar") MultipartFile file) {
User user = userRepository.findById(userId).orElse(null); User user = userRepository.findById(userId)
if (user == null) { .orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + userId));
return ResponseEntity.notFound().build();
}
if (file.isEmpty()) { ImageValidationUtil.validate(file);
Map<String, String> error = new HashMap<>();
error.put("message", "Please select a file to upload");
return ResponseEntity.badRequest().body(error);
}
if (file.getSize() > 5 * 1024 * 1024) {
Map<String, String> error = new HashMap<>();
error.put("message", "File size must not exceed 5MB");
return ResponseEntity.badRequest().body(error);
}
String contentType = file.getContentType();
if (contentType == null || (!contentType.equals("image/jpeg") && !contentType.equals("image/png") && !contentType.equals("image/gif"))) {
Map<String, String> error = new HashMap<>();
error.put("message", "Only JPG, PNG, and GIF images are allowed");
return ResponseEntity.badRequest().body(error);
}
try { try {
avatarStorageService.deleteAvatar(user); avatarStorageService.deleteAvatar(user);
@@ -88,31 +75,26 @@ public class UserAvatarController {
result.put("message", "Avatar uploaded successfully"); result.put("message", "Avatar uploaded successfully");
return ResponseEntity.ok(result); return ResponseEntity.ok(result);
} catch (IOException e) { } catch (IOException e) {
Map<String, String> error = new HashMap<>(); throw new BusinessException("Failed to upload avatar: " + e.getMessage());
error.put("message", "Failed to upload avatar: " + e.getMessage());
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(error);
} }
} }
@DeleteMapping("/{userId}/avatar") @DeleteMapping("/{userId}/avatar")
@PreAuthorize("hasRole('ADMIN')") @PreAuthorize("hasRole('ADMIN')")
public ResponseEntity<?> deleteUserAvatar(@PathVariable Long userId) { public ResponseEntity<?> deleteUserAvatar(@PathVariable Long userId) {
User user = userRepository.findById(userId).orElse(null); User user = userRepository.findById(userId)
if (user == null) { .orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + userId));
return ResponseEntity.notFound().build();
}
try { try {
avatarStorageService.deleteAvatar(user); avatarStorageService.deleteAvatar(user);
user.setAvatarUrl(null);
userRepository.save(user);
Map<String, String> result = new HashMap<>();
result.put("message", "Avatar removed successfully");
return ResponseEntity.ok(result);
} catch (IOException e) { } catch (IOException e) {
Map<String, String> error = new HashMap<>(); log.warn("Failed to delete avatar for user {}: {}", userId, e.getMessage());
error.put("message", "Failed to remove avatar: " + e.getMessage());
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(error);
} }
user.setAvatarUrl(null);
userRepository.save(user);
Map<String, String> result = new HashMap<>();
result.put("message", "Avatar removed successfully");
return ResponseEntity.ok(result);
} }
} }

View File

@@ -126,8 +126,8 @@ public class CouponService {
BigDecimal discount = BigDecimal.ZERO; BigDecimal discount = BigDecimal.ZERO;
String type = coupon.getDiscountType(); String type = coupon.getDiscountType();
if (isPercentageType(type)) { if (isPercentageType(type)) {
discount = subtotal.multiply(coupon.getDiscountValue()) discount = subtotal.multiply(coupon.getDiscountValue()
.divide(BigDecimal.valueOf(100), 2, RoundingMode.HALF_UP); .divide(new BigDecimal("100"), 4, RoundingMode.HALF_UP));
} else if (isFixedType(type)) { } else if (isFixedType(type)) {
discount = coupon.getDiscountValue(); discount = coupon.getDiscountValue();
} }

View File

@@ -7,15 +7,19 @@ import java.util.Locale;
public final class ImageValidationUtil { public final class ImageValidationUtil {
public static final long MAX_IMAGE_SIZE = 5 * 1024 * 1024; public static final long DEFAULT_MAX_IMAGE_SIZE = 5 * 1024 * 1024;
private ImageValidationUtil() {} private ImageValidationUtil() {}
public static void validate(MultipartFile file) { public static void validate(MultipartFile file) {
validate(file, DEFAULT_MAX_IMAGE_SIZE);
}
public static void validate(MultipartFile file, long maxSizeBytes) {
if (file.isEmpty()) { if (file.isEmpty()) {
throw new BusinessException("Please select an image to upload"); throw new BusinessException("Please select an image to upload");
} }
if (file.getSize() > MAX_IMAGE_SIZE) { if (file.getSize() > maxSizeBytes) {
throw new BusinessException("Image file size must be less than 5MB"); throw new BusinessException("Image file size must be less than 5MB");
} }
String contentType = file.getContentType(); String contentType = file.getContentType();