fix: validate the "comment" field for emptiness and max length

Fix #1411

Author: php-coder

src/main/java/ru/mystamps/web/feature/series/RestSeriesController.java

@@ -43,7 +43,6 @@ class RestSeriesController {
 
 	private final SeriesService seriesService;
 
-	// @todo #785 Update series: add validation for a comment
 	// @todo #1339 Update series: add validation for catalog numbers
 	// @todo #1340 Update series: add validation for a price
 	// @todo #1343 Update series: add validation for a release year
@@ -73,7 +72,7 @@ public ResponseEntity<Void> updateSeries(
 			String path = patch.getPath();
 			switch (path) {
 				case "/comment":
-					seriesService.addComment(seriesId, patch.getValue(), currentUserId);
+					seriesService.addComment(seriesId, currentUserId, patch.getValue());
 					break;
 				case "/release_year":
 					seriesService.addReleaseYear(seriesId, patch.integerValue(), currentUserId);

src/main/java/ru/mystamps/web/feature/series/SeriesService.java

@@ -17,15 +17,28 @@
  */
 package ru.mystamps.web.feature.series;
 
+import javax.validation.constraints.Size;
 import java.math.BigDecimal;
 import java.util.Date;
 import java.util.List;
 
+import static ru.mystamps.web.feature.series.SeriesValidation.MAX_SERIES_COMMENT_LENGTH;
+
 // FIXME: move stamps related methods to separate interface (#88)
 @SuppressWarnings("PMD.TooManyMethods")
 public interface SeriesService {
 	Integer add(AddSeriesDto dto, Integer userId);
-	void addComment(Integer seriesId, String comment, Integer userId);
+
+	// CheckStyle: ignore LineLength for next 6 lines
+	// @todo #1411 Configure MethodValidationPostProcessor to use messages from ValidationMessages.properties
+	void addComment(
+		Integer seriesId,
+		Integer userId,
+		// @NotBlank isn't needed because PatchRequest.value has @NotEmpty and its setter trims blank values to null
+		@Size(max = MAX_SERIES_COMMENT_LENGTH, message = "Value is greater than allowable maximum of {max} characters")
+		String comment
+	);
+	
 	void addReleaseYear(Integer seriesId, Integer year, Integer userId);
 	void addCatalogPrice(StampsCatalog catalog, Integer seriesId, BigDecimal price, Integer userId);
 	void addCatalogNumbers(StampsCatalog catalog, Integer seriesId, String numbers, Integer userId);

src/main/java/ru/mystamps/web/feature/series/SeriesServiceImpl.java

@@ -22,6 +22,7 @@
 import org.slf4j.Logger;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.validation.annotation.Validated;
 import ru.mystamps.web.feature.image.ImageInfoDto;
 import ru.mystamps.web.feature.image.ImageService;
 import ru.mystamps.web.support.spring.security.HasAuthority;
@@ -38,6 +39,7 @@
 // The String literal "Series id must be non null" appears N times in this file
 // and we think that it's OK.
 @SuppressWarnings({ "PMD.TooManyMethods", "PMD.AvoidDuplicateLiterals" })
+@Validated
 @RequiredArgsConstructor
 public class SeriesServiceImpl implements SeriesService {
 
@@ -115,11 +117,11 @@ public Integer add(AddSeriesDto dto, Integer userId) {
 	@Override
 	@Transactional
 	@PreAuthorize(HasAuthority.ADD_COMMENTS_TO_SERIES)
-	public void addComment(Integer seriesId, String comment, Integer userId) {
+	public void addComment(Integer seriesId, Integer userId, String comment) {
 		Validate.isTrue(seriesId != null, "Series id must be non null");
+		Validate.isTrue(userId != null, "User id must be non null");
 		Validate.isTrue(comment != null, "Comment must be non null");
 		Validate.isTrue(!comment.trim().isEmpty(), "Comment must be non empty");
-		Validate.isTrue(userId != null, "User id must be non null");
 
 		// We don't need to modify series.updated_at/updated_by fields because:
 		// - a comment is a meta information that is invisible publicly

src/main/java/ru/mystamps/web/support/spring/mvc/ValidationErrors.java

@@ -60,11 +60,14 @@ private void extractFieldError(ConstraintViolation<?> violation) {
 		// ValidationErrors
 		Node last = getLastOrNull(violation.getPropertyPath());
 		String name;
-		if (last == null || last.getKind() != ElementKind.PROPERTY) {
+		if (last != null && last.getKind() == ElementKind.PROPERTY) {
+			name = last.getName();
+		} else if (last != null && last.getKind() == ElementKind.PARAMETER) {
+			// emulate validation of PatchRequest.value field
+			name = "value";
+		} else {
 			// fallback to a field path for unsupported kinds
 			name = violation.getPropertyPath().toString();
-		} else {
-			name = last.getName();
 		}
 
 		String message = violation.getMessage();

src/test/groovy/ru/mystamps/web/feature/series/SeriesServiceImplTest.groovy

@@ -439,31 +439,31 @@ class SeriesServiceImplTest extends Specification {
 
 	def 'addComment() should throw exception when series id is null'() {
 		when:
-			service.addComment(null, null, Random.userId())
+			service.addComment(null, Random.userId(), 'text')
 		then:
 			IllegalArgumentException ex = thrown()
 			ex.message == 'Series id must be non null'
 	}
 
 	def 'addComment() should throw exception when comment is null'() {
 		when:
-			service.addComment(Random.id(), null, Random.userId())
+			service.addComment(Random.id(), Random.userId(), null)
 		then:
 			IllegalArgumentException ex = thrown()
 			ex.message == 'Comment must be non null'
 	}
 
 	def 'addComment() should throw exception when comment is empty or blank'() {
 		when:
-			service.addComment(Random.id(), sample('', '  '), Random.userId())
+			service.addComment(Random.id(), Random.userId(), sample('', '  '))
 		then:
 			IllegalArgumentException ex = thrown()
 			ex.message == 'Comment must be non empty'
 	}
 
 	def 'addComment() should throw exception when user id is null'() {
 		when:
-			service.addComment(Random.id(), 'a comment', null)
+			service.addComment(Random.id(), null, 'a comment')
 		then:
 			IllegalArgumentException ex = thrown()
 			ex.message == 'User id must be non null'
@@ -476,7 +476,7 @@ class SeriesServiceImplTest extends Specification {
 			Integer expectedUserId = Random.userId()
 			String expectedComment = 'this is important'
 		when:
-			service.addComment(expectedSeriesId, expectedComment, expectedUserId)
+			service.addComment(expectedSeriesId, expectedUserId, expectedComment)
 		then:
 			1 * seriesDao.addComment({ AddCommentDbDto dto ->
 				assert dto?.seriesId == expectedSeriesId

src/test/robotframework/series/add-comment/validation.robot

@@ -0,0 +1,38 @@
+*** Settings ***
+Documentation   Verify validation scenarios for adding a comment to a series
+Library         SeleniumLibrary
+Resource        ../../auth.steps.robot
+Resource        ../../selenium.utils.robot
+Suite Setup     Before Test Suite
+Suite Teardown  Close Browser
+Force Tags      series  add-comment  validation
+
+*** Test Cases ***
+# In essence, here we test @NotEmpty annotation on PatchRequest.value because it validates an incoming value first
+Add comment with empty required field
+	[Setup]                           Disable Client Validation  add-comment-form
+	Submit Form                       id:add-comment-form
+	Wait Until Page Contains Element  id:new-comment.errors
+	Element Text Should Be            id:new-comment.errors  must not be empty
+
+# In essence, here we test @NotEmpty annotation on PatchRequest.value and setValue() that trims blank values to null,
+# because it validates an incoming value first
+Add a blank comment
+	Input Text                        id:new-comment  ${SPACE}${SPACE}
+	Submit Form                       id:add-comment-form
+	Wait Until Page Contains Element  id:new-comment.errors
+	Element Text Should Be            id:new-comment.errors  must not be empty
+
+Add too long comment
+	${letter}=                        Set Variable  x
+	Input Text                        id:new-comment  ${letter * 1025}
+	Submit Form                       id:add-comment-form
+	Wait Until Page Contains Element  id:new-comment.errors
+	Element Text Should Be            id:new-comment.errors  Value is greater than allowable maximum of 1024 characters
+
+*** Keywords ***
+Before Test Suite
+	Open Browser                        ${SITE_URL}/account/auth  ${BROWSER}
+	Register Keyword To Run On Failure  Log Source
+	Log In As                           login=coder  password=test
+	Go To                               ${SITE_URL}/series/5