Aias00 commented on PR #6457:
URL: https://github.com/apache/shenyu/pull/6457#issuecomment-5193409786
Good fix — the old null guard was dead code: `Date alertTime =
alert.getDateCreated(); if (Objects.isNull(alert)) { alertTime = new Date(); }`
checked `alert` (the parameter) for null *after* already calling
`alert.getDateCreated()`, so a null `alert` would NPE before reaching the
check, and a null `getDateCreated()` on a non-null `alert` was never replaced.
The replacement `Objects.isNull(alert.getDateCreated()) ? new Date() :
alert.getDateCreated()` correctly guards the right thing.
Moving validation into a dedicated `AlertReportRequest` DTO with `@NotBlank`
on `title`/`content` is the right call — it constrains the HTTP boundary
without affecting internal callers that build `AlarmContent` directly. Tests
cover the 400 paths (blank/null title + content) and the NPE guard for null
`dateCreated`, plus a valid-dispatch assertion via `ArgumentCaptor`.
Two minor notes:
- `shenyu-common/pom.xml` has a cosmetic whitespace-only change (a blank
line removed before `</dependencies>`) that looks unintentional — fine to drop
or keep, just flagging.
- The `/alert/report` endpoint's auth posture is unchanged by this PR
(validation improved, but anyone who could post before still can). Out of scope
here, just noting it's a pre-existing consideration.
--
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]
