Skip to content

Define bulletin write fields explicitly - #4292

Open
zqr10159 wants to merge 1 commit into
apache:masterfrom
zqr10159:maintenance/bulletin-write-boundary
Open

Define bulletin write fields explicitly#4292
zqr10159 wants to merge 1 commit into
apache:masterfrom
zqr10159:maintenance/bulletin-write-boundary

Conversation

@zqr10159

Copy link
Copy Markdown
Member

Summary

  • create bulletins from an explicit set of business fields
  • update an existing bulletin in place while retaining persistence-managed fields
  • mark ids and audit metadata as read-only in the API schema

Validation

  • ./mvnw -pl hertzbeat-manager -am -Dtest=BulletinServiceTest test -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false
  • git diff --check

AI 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.

@zqr10159
zqr10159 marked this pull request as ready for review July 31, 2026 02:51
@Duansg

Duansg commented Aug 3, 2026

Copy link
Copy Markdown
Member

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());
if (existBulletin != null && !existBulletin.getId().equals(bulletin.getId())) {
throw new IllegalArgumentException("Bulletin name duplicated");
}

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:
POST /api/bulletin {"id": , "name": "<that bulletin's name>", "app": "...", "monitorIds": [...], "fields": {...}}
then check select name, count() from hzb_bulletin group by name having count() > 1.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants