wy471x commented on PR #6457: URL: https://github.com/apache/shenyu/pull/6457#issuecomment-5155564980
> I found one regression risk outside the email-template null guard. > > Adding `@NotBlank` to `AlarmContent.title` and `AlarmContent.content` changes runtime behavior because `AlertReportController` already receives `@Valid @RequestBody AlarmContent`. After this change, null/blank `title` or `content` is rejected with a validation error before alert dispatch. > > That can drop existing alerts. For example, the gateway error flow can build an alert from an exception message; an `IllegalArgumentException` can have a null message, and `AlarmSender` copies that value into `AlarmContent.content`. Before this PR the alert could still reach dispatch and the email strategy could render it defensively. With the new shared DTO constraints, the request is rejected at `/alert/report` before any notify strategy sees it. > > Could we keep the email `alert == null` / `dateCreated == null` guard, but remove the new `@NotBlank` constraints and the added `jakarta.validation-api` dependency from `shenyu-common`? If stricter validation is desired for external report requests, it would be safer to put it on a controller-layer request DTO and map into `AlarmContent` explicitly. @Aias00 Hi, I have fixed the issues you mentioned. Please take a look when you have time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
