fix/reviews

src/main/java/goodroad/api/ApiErrors.java

@@ -1,10 +1,23 @@
 package goodroad.api;
 
+import jakarta.validation.ConstraintViolationException;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
+import org.springframework.http.converter.HttpMessageNotReadableException;
+import org.springframework.security.access.AccessDeniedException;
+import org.springframework.web.HttpMediaTypeNotSupportedException;
+import org.springframework.web.bind.MethodArgumentNotValidException;
+import org.springframework.web.bind.MissingServletRequestParameterException;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RestControllerAdvice;
-import lombok.extern.slf4j.Slf4j;
+import org.springframework.web.method.annotation.HandlerMethodValidationException;
+import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;
+import org.springframework.web.multipart.MaxUploadSizeExceededException;
+import org.springframework.web.multipart.support.MissingServletRequestPartException;
+import org.springframework.web.servlet.resource.NoResourceFoundException;
+
 import java.time.Instant;
 
 @Slf4j
@@ -23,8 +36,8 @@ public static class ApiException extends RuntimeException {
         private final HttpStatus status;
         private final String code;
 
-        public ApiException(HttpStatus status, String code, String msg) {
-            super(msg);
+        public ApiException(HttpStatus status, String code, String message) {
+            super(message);
             this.status = status;
             this.code = code;
         }
@@ -42,16 +55,78 @@ public String code() {
     public static class GlobalHandler {
 
         @ExceptionHandler(ApiException.class)
-        public ResponseEntity<ApiError> handleApiException(ApiException e) {
-            log.error("API exception: code={}, msg={}", e.code(), e.getMessage(), e);
-            return ResponseEntity.status(e.status()).body(ApiError.of(e.code(), e.getMessage()));
+        public ResponseEntity<ApiError> handleApiException(ApiException exception) {
+            if (exception.status().is5xxServerError()) {
+                log.error("API exception: code={}, msg={}", exception.code(), exception.getMessage(), exception);
+            }
+            return ResponseEntity.status(exception.status())
+                    .body(ApiError.of(exception.code(), exception.getMessage()));
+        }
+
+        @ExceptionHandler(MethodArgumentNotValidException.class)
+        public ResponseEntity<ApiError> handleValidation(MethodArgumentNotValidException exception) {
+            return badRequest("REQUEST_VALIDATION_FAILED", "Request fields are invalid");
+        }
+
+        @ExceptionHandler(HttpMessageNotReadableException.class)
+        public ResponseEntity<ApiError> handleUnreadableBody(HttpMessageNotReadableException exception) {
+            return badRequest("REQUEST_BODY_INVALID", "Request body is missing or contains invalid JSON");
+        }
+
+        @ExceptionHandler({
+                MethodArgumentTypeMismatchException.class,
+                MissingServletRequestParameterException.class,
+                ConstraintViolationException.class,
+                HandlerMethodValidationException.class
+        })
+        public ResponseEntity<ApiError> handleInvalidParameters(Exception exception) {
+            return badRequest("REQUEST_VALIDATION_FAILED", "Request parameters are invalid");
+        }
+
+        @ExceptionHandler(MissingServletRequestPartException.class)
+        public ResponseEntity<ApiError> handleMissingPart(MissingServletRequestPartException exception) {
+            return badRequest("REQUEST_PART_MISSING", "Required request part is missing");
+        }
+
+        @ExceptionHandler(MaxUploadSizeExceededException.class)
+        public ResponseEntity<ApiError> handleUploadTooLarge(MaxUploadSizeExceededException exception) {
+            return ResponseEntity.status(HttpStatus.PAYLOAD_TOO_LARGE)
+                    .body(ApiError.of("FILE_TOO_LARGE", "Uploaded file is too large"));
+        }
+
+        @ExceptionHandler(HttpMediaTypeNotSupportedException.class)
+        public ResponseEntity<ApiError> handleUnsupportedMediaType(HttpMediaTypeNotSupportedException exception) {
+            return ResponseEntity.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE)
+                    .body(ApiError.of("CONTENT_TYPE_UNSUPPORTED", "Content type is not supported"));
+        }
+
+        @ExceptionHandler(DataIntegrityViolationException.class)
+        public ResponseEntity<ApiError> handleConflict(DataIntegrityViolationException exception) {
+            return ResponseEntity.status(HttpStatus.CONFLICT)
+                    .body(ApiError.of("DATA_CONFLICT", "Operation conflicts with current data"));
+        }
+
+        @ExceptionHandler(AccessDeniedException.class)
+        public ResponseEntity<ApiError> handleAccessDenied(AccessDeniedException exception) {
+            return ResponseEntity.status(HttpStatus.FORBIDDEN)
+                    .body(ApiError.of("ACCESS_DENIED", "Access denied"));
+        }
+
+        @ExceptionHandler(NoResourceFoundException.class)
+        public ResponseEntity<ApiError> handleNotFound(NoResourceFoundException exception) {
+            return ResponseEntity.status(HttpStatus.NOT_FOUND)
+                    .body(ApiError.of("ENDPOINT_NOT_FOUND", "Endpoint not found"));
         }
 
         @ExceptionHandler(Exception.class)
-        public ResponseEntity<ApiError> handleServerError(Exception e) {
-            log.error("Unexpected exception", e);
+        public ResponseEntity<ApiError> handleServerError(Exception exception) {
+            log.error("Unexpected exception", exception);
             return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
                     .body(ApiError.of("SERVER_INTERNAL_ERROR", "Server internal error"));
         }
+
+        private ResponseEntity<ApiError> badRequest(String code, String message) {
+            return ResponseEntity.badRequest().body(ApiError.of(code, message));
+        }
     }
-}
+}

src/main/java/goodroad/reviews/ReviewModerationService.java

@@ -181,7 +181,6 @@ public void reject(String phoneFromAuth, String reviewId, String reason) {
         }
 
         review.setStatus(STATUS_REJECTED);
-        review.setAwardedPoints(0);
         review.setTakenByModeratorId(null);
         review.setTakenAt(null);
         review.setModeratedBy(moderator.getId());

src/main/java/goodroad/reviews/ReviewValidationService.java

@@ -2,17 +2,25 @@
 
 import goodroad.api.ApiErrors.ApiException;
 import goodroad.model.ObstacleType;
+import goodroad.validation.GeoUtils;
 import goodroad.validation.InputRules;
+import goodroad.validation.TrustedUrlService;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
 
 import java.util.*;
 
 @Service
 public class ReviewValidationService {
+    private final TrustedUrlService trustedUrls;
+
+    public ReviewValidationService(TrustedUrlService trustedUrls) {
+        this.trustedUrls = trustedUrls;
+    }
 
     public ValidatedReviewInput validate(
-            UserReviewService.UpsertReviewReq req
+            UserReviewService.UpsertReviewReq req,
+            Long userId
     ) {
 
         if (req == null) {
@@ -31,15 +39,7 @@ public ValidatedReviewInput validate(
             );
         }
 
-        if (Double.isNaN(req.latitude())
-                || Double.isNaN(req.longitude())) {
-
-            throw new ApiException(
-                    HttpStatus.BAD_REQUEST,
-                    "REVIEW_COORDS_INVALID",
-                    "Coordinates are invalid"
-            );
-        }
+        GeoUtils.requireCoordinates(req.latitude(), req.longitude(), "REVIEW_COORDS_INVALID");
 
         UserReviewService.AddressReq address =
                 validateAddress(req.address());
@@ -50,13 +50,14 @@ public ValidatedReviewInput validate(
 
         List<String> photoUrls =
                 normalizePhotoUrls(
-                        req.photoUrls()
+                        req.photoUrls(),
+                        userId
                 );
 
-        String comment =
-                blankToNull(
-                        req.comment()
-                );
+        String comment = blankToNull(req.comment());
+        if (comment != null && comment.length() > 1000) {
+            throw new ApiException(HttpStatus.BAD_REQUEST, "REVIEW_COMMENT_TOO_LONG", "Comment is too long");
+        }
 
         String primaryObstacleType =
                 choosePrimaryObstacleType(
@@ -90,42 +91,42 @@ public ValidatedReviewInput validate(
         }
 
         String country =
-                InputRules.requireAddressText(
+                InputRules.requireCyrillicText(
                         raw.country(),
                         "ADDRESS_COUNTRY_INVALID",
                         "Country"
                 );
 
         String region =
-                InputRules.requireAddressText(
+                InputRules.requireCyrillicText(
                         raw.region(),
                         "ADDRESS_REGION_INVALID",
                         "Region"
                 );
 
         String localityType =
-                InputRules.requireAddressText(
+                InputRules.requireCyrillicText(
                         raw.localityType(),
                         "ADDRESS_LOCALITY_TYPE_INVALID",
                         "Locality type"
                 );
 
         String city =
-                InputRules.requireAddressText(
+                InputRules.requireCyrillicText(
                         raw.city(),
                         "ADDRESS_CITY_INVALID",
                         "City"
                 );
 
         String street =
-                InputRules.requireAddressText(
+                InputRules.requireCyrillicText(
                         raw.street(),
                         "ADDRESS_STREET_INVALID",
                         "Street"
                 );
 
         String house =
-                InputRules.requireAddressText(
+                InputRules.requireDigits(
                         raw.house(),
                         "ADDRESS_HOUSE_INVALID",
                         "House"
@@ -135,6 +136,9 @@ public ValidatedReviewInput validate(
                 blankToNull(
                         raw.placeName()
                 );
+        if (placeName != null && placeName.length() > 180) {
+            throw new ApiException(HttpStatus.BAD_REQUEST, "ADDRESS_PLACE_NAME_INVALID", "Place name is too long");
+        }
 
         return new UserReviewService.AddressReq(
                 country,
@@ -243,14 +247,17 @@ public ValidatedReviewInput validate(
     }
 
     private List<String> normalizePhotoUrls(
-            Collection<String> rawUrls
+            Collection<String> rawUrls,
+            Long userId
     ) {
 
-        List<String> out =
-                new ArrayList<>();
+        List<String> out = new ArrayList<>();
 
         if (rawUrls == null) {
-            return out;
+            return List.of();
+        }
+        if (rawUrls.size() > 10) {
+            throw new ApiException(HttpStatus.BAD_REQUEST, "REVIEW_PHOTO_LIMIT_EXCEEDED", "Too many review photos");
         }
 
         for (String raw : rawUrls) {
@@ -259,7 +266,12 @@ private List<String> normalizePhotoUrls(
                     blankToNull(raw);
 
             if (value != null) {
-                out.add(value);
+                out.add(trustedUrls.requireOwnedStorageUrl(
+                        value,
+                        "reviews",
+                        userId,
+                        "REVIEW_PHOTO_URL_INVALID"
+                ));
             }
         }
 

src/main/java/goodroad/reviews/UserReviewService.java

@@ -139,7 +139,7 @@ public ReviewCardResp createReview(
         UserEntity user = findCurrent(phoneFromAuth);
 
         ReviewValidationService.ValidatedReviewInput input =
-                validator.validate(req);
+                validator.validate(req, user.getId());
 
         ObstacleFeatureEntity feature =
                 featureService.resolveOrCreateFeature(
@@ -217,23 +217,32 @@ public ReviewCardResp updateOwnReview(
         Long oldFeatureId = review.getFeatureId();
 
         ReviewValidationService.ValidatedReviewInput input =
-                validator.validate(req);
+                validator.validate(req, user.getId());
 
         ObstacleFeatureEntity feature =
                 featureService.resolveOrCreateFeature(input);
 
+        reviews.findByFeatureIdAndAuthorId(feature.getId(), user.getId())
+                .filter(existing -> !existing.getId().equals(review.getId()))
+                .ifPresent(existing -> {
+                    throw new ApiException(HttpStatus.CONFLICT, "REVIEW_ALREADY_EXISTS", "Review already exists");
+                });
+
         review.setFeatureId(feature.getId());
         review.setSeverity(input.rating());
         review.setText(input.comment());
         review.setStatus(STATUS_PENDING);
-        review.setAwardedPoints(0);
         review.setModeratorComment(null);
 
         reviews.save(review);
 
         mapper.saveReviewObstacles(review.getId(), input.obstacles());
         mapper.savePhotos(review.getId(), input.photoUrls());
 
+        if (STATUS_APPROVED.equals(oldStatus)) {
+            reviewSupport.recomputeFeatureAggregate(oldFeatureId);
+        }
+
         user.setLastActiveAt(Instant.now());
         users.save(user);
 
@@ -260,7 +269,12 @@ public void deleteOwnReview(
                         )
                 );
 
+        Long featureId = review.getFeatureId();
+        boolean wasApproved = STATUS_APPROVED.equals(review.getStatus());
         reviews.delete(review);
+        if (wasApproved) {
+            reviewSupport.recomputeFeatureAggregate(featureId);
+        }
 
         user.setLastActiveAt(Instant.now());
         users.save(user);