Define bulletin write fields explicitly - #4292
Conversation
|
The underlying problem is real: addBulletin previously called bulletinDao.save(bulletin) with a client-supplied id, which makes JPA merge into an existing row — so POST /api/bulletin could overwrite an arbitrary bulletin. Copying an explicit field set is the right shape of fix. One blocking issue though. The add path can now create duplicate bulletin names validate() still consults the request's id when checking for duplicates (BulletinServiceImpl.java:81-84): Bulletin existBulletin = bulletinDao.findByName(bulletin.getName()); BulletinController.java:65-66 runs validate() before addBulletin(). So a POST carrying an existing bulletin's id and its name passes validation (the ids match), and addBulletin then discards the id and inserts a second row with the same name. Before this PR the same request merged into the existing row, so no duplicate was possible. Bulletin.name has no unique constraint, and BulletinDao.java:38 declares Bulletin findByName(String name) returning a single entity — once duplicates exist, every subsequent add/edit touching that name throws IncorrectResultSizeDataAccessException, and deleteByNameIn (line 33) removes both rows. Repro: Suggestion: ignore the request id when validating the create path (compare against null, or split validation into create/update variants), and consider adding a unique constraint on name so the invariant is enforced by the database rather than by a read-then-write check that is racy anyway. Note on the @Schema changes Switching id / creator / modifier / gmtCreate / gmtUpdate to accessMode = READ_ONLY only changes the generated OpenAPI document — Jackson still binds those fields from the request body at runtime. The actual enforcement here is the explicit field copy in addBulletin/editBulletin, which is fine, but the annotations shouldn't be read as a control. Might be worth saying so in the PR description so reviewers don't assume the schema is doing the work. Optional Since Bulletin is used as both the request body and the response body, the explicit field list has to be kept in sync by hand — adding a business field later and forgetting to add it to both methods fails silently. A small request DTO would make that a compile-time concern instead. |
Summary
Validation
./mvnw -pl hertzbeat-manager -am -Dtest=BulletinServiceTest test -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=falsegit diff --checkAI assistance: used for draft implementation and test iteration.
Human validation: all 7 focused bulletin service tests passed across the 22-module source reactor, with Checkstyle and diff hygiene passing.
Risk notes: create and edit now accept only the existing business fields; persistence-managed id and audit values are no longer copied from request entities.