From 3ffddbaa6a61787f9b7a8758e4bef4b3ac7f7683 Mon Sep 17 00:00:00 2001 From: Logic Date: Thu, 30 Jul 2026 18:05:00 +0800 Subject: [PATCH 1/2] maintenance: define bulletin write fields --- .../common/entity/manager/Bulletin.java | 11 ++-- .../service/impl/BulletinServiceImpl.java | 15 ++++- .../manager/service/BulletinServiceTest.java | 60 +++++++++++++++++++ 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java b/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java index b98082296f5..bdfe0e75c21 100644 --- a/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java +++ b/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java @@ -17,6 +17,7 @@ package org.apache.hertzbeat.common.entity.manager; +import static io.swagger.v3.oas.annotations.media.Schema.AccessMode.READ_ONLY; import static io.swagger.v3.oas.annotations.media.Schema.AccessMode.READ_WRITE; import io.swagger.v3.oas.annotations.media.Schema; import jakarta.persistence.Column; @@ -55,7 +56,7 @@ public class Bulletin { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) - @Schema(description = "Bulletin ID", example = "1") + @Schema(description = "Bulletin ID", example = "1", accessMode = READ_ONLY) private Long id; @Schema(description = "Bulletin Name", example = "Bulletin1", accessMode = READ_WRITE) @@ -74,19 +75,19 @@ public class Bulletin { @Convert(converter = JsonMapListAttributeConverter.class) private Map> fields; - @Schema(title = "The creator of this record", example = "tom", accessMode = READ_WRITE) + @Schema(title = "The creator of this record", example = "tom", accessMode = READ_ONLY) @CreatedBy private String creator; - @Schema(title = "The modifier of this record", example = "tom", accessMode = READ_WRITE) + @Schema(title = "The modifier of this record", example = "tom", accessMode = READ_ONLY) @LastModifiedBy private String modifier; - @Schema(title = "Record create time", example = "2024-07-02T20:09:34.903217", accessMode = READ_WRITE) + @Schema(title = "Record create time", example = "2024-07-02T20:09:34.903217", accessMode = READ_ONLY) @CreatedDate private LocalDateTime gmtCreate; - @Schema(title = "Record modify time", example = "2024-07-02T20:09:34.903217", accessMode = READ_WRITE) + @Schema(title = "Record modify time", example = "2024-07-02T20:09:34.903217", accessMode = READ_ONLY) @LastModifiedDate private LocalDateTime gmtUpdate; } diff --git a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java index f28cb309f90..e7ccb4fd548 100644 --- a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java +++ b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java @@ -94,7 +94,12 @@ public void editBulletin(Bulletin bulletin) { if (optional.isEmpty()) { throw new IllegalArgumentException("Bulletin not found"); } - bulletinDao.save(bulletin); + Bulletin storedBulletin = optional.get(); + storedBulletin.setName(bulletin.getName()); + storedBulletin.setMonitorIds(bulletin.getMonitorIds()); + storedBulletin.setApp(bulletin.getApp()); + storedBulletin.setFields(bulletin.getFields()); + bulletinDao.save(storedBulletin); } /** @@ -103,7 +108,13 @@ public void editBulletin(Bulletin bulletin) { @Override @Transactional(rollbackFor = Exception.class) public void addBulletin(Bulletin bulletin) { - bulletinDao.save(bulletin); + Bulletin newBulletin = Bulletin.builder() + .name(bulletin.getName()) + .monitorIds(bulletin.getMonitorIds()) + .app(bulletin.getApp()) + .fields(bulletin.getFields()) + .build(); + bulletinDao.save(newBulletin); } /** diff --git a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java index dd9768347b0..e0f132ca04a 100644 --- a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java +++ b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java @@ -24,6 +24,7 @@ import org.apache.hertzbeat.warehouse.store.realtime.RealTimeDataReader; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -32,19 +33,24 @@ import org.springframework.data.domain.PageRequest; import org.springframework.data.jpa.domain.Specification; +import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** @@ -119,6 +125,60 @@ public void testAddBulletin() throws Exception { }); } + @Test + void addBulletinIgnoresClientManagedFields() { + Bulletin bulletin = new Bulletin(); + bulletin.setId(7L); + bulletin.setCreator("submitted-creator"); + bulletin.setModifier("submitted-modifier"); + bulletin.setGmtCreate(LocalDateTime.of(2020, 1, 1, 0, 0)); + bulletin.setGmtUpdate(LocalDateTime.of(2020, 1, 2, 0, 0)); + + bulletinService.addBulletin(bulletin); + + ArgumentCaptor saved = ArgumentCaptor.forClass(Bulletin.class); + verify(bulletinDao).save(saved.capture()); + assertNull(saved.getValue().getId()); + assertNull(saved.getValue().getCreator()); + assertNull(saved.getValue().getModifier()); + assertNull(saved.getValue().getGmtCreate()); + assertNull(saved.getValue().getGmtUpdate()); + } + + @Test + void editBulletinPreservesStoredManagedFields() { + LocalDateTime createdAt = LocalDateTime.of(2020, 1, 1, 0, 0); + LocalDateTime updatedAt = LocalDateTime.of(2020, 1, 2, 0, 0); + Bulletin stored = Bulletin.builder() + .id(7L) + .name("old-name") + .creator("stored-creator") + .modifier("stored-modifier") + .gmtCreate(createdAt) + .gmtUpdate(updatedAt) + .build(); + Bulletin submitted = Bulletin.builder() + .id(7L) + .name("new-name") + .creator("submitted-creator") + .modifier("submitted-modifier") + .gmtCreate(createdAt.minusYears(1)) + .gmtUpdate(updatedAt.minusYears(1)) + .build(); + when(bulletinDao.findById(7L)).thenReturn(Optional.of(stored)); + + bulletinService.editBulletin(submitted); + + ArgumentCaptor saved = ArgumentCaptor.forClass(Bulletin.class); + verify(bulletinDao).save(saved.capture()); + assertSame(stored, saved.getValue()); + assertEquals("new-name", saved.getValue().getName()); + assertEquals("stored-creator", saved.getValue().getCreator()); + assertEquals("stored-modifier", saved.getValue().getModifier()); + assertEquals(createdAt, saved.getValue().getGmtCreate()); + assertEquals(updatedAt, saved.getValue().getGmtUpdate()); + } + @Test public void testGetBulletins() throws Exception { Bulletin bulletin = new Bulletin(); From 68b340d4b06f5190b337857232478458e7a1a23e Mon Sep 17 00:00:00 2001 From: Logic Date: Tue, 4 Aug 2026 20:20:44 +0800 Subject: [PATCH 2/2] Enforce bulletin name uniqueness by operation --- .../common/entity/manager/Bulletin.java | 5 +++- .../controller/BulletinController.java | 4 +-- .../manager/service/BulletinService.java | 2 +- .../service/impl/BulletinServiceImpl.java | 7 ++++-- .../controller/BulletinControllerTest.java | 8 +++--- .../manager/service/BulletinServiceTest.java | 25 +++++++++++++++---- 6 files changed, 36 insertions(+), 15 deletions(-) diff --git a/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java b/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java index bdfe0e75c21..f5ba5658b29 100644 --- a/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java +++ b/hertzbeat-common-spring/src/main/java/org/apache/hertzbeat/common/entity/manager/Bulletin.java @@ -28,6 +28,7 @@ import jakarta.persistence.GenerationType; import jakarta.persistence.Id; import jakarta.persistence.Table; +import jakarta.persistence.UniqueConstraint; import java.time.LocalDateTime; import java.util.List; import java.util.Map; @@ -51,7 +52,9 @@ @AllArgsConstructor @NoArgsConstructor @EntityListeners(AuditingEntityListener.class) -@Table(name = "hzb_bulletin") +@Table(name = "hzb_bulletin", uniqueConstraints = { + @UniqueConstraint(name = "uk_bulletin_name", columnNames = "name") +}) public class Bulletin { @Id diff --git a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/BulletinController.java b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/BulletinController.java index 3c8bddf6ca4..ee0bc9fd5ad 100644 --- a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/BulletinController.java +++ b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/BulletinController.java @@ -62,7 +62,7 @@ public class BulletinController { @PostMapping public ResponseEntity> addNewBulletin(@Valid @RequestBody Bulletin bulletin) { try { - bulletinService.validate(bulletin); + bulletinService.validate(bulletin, false); bulletinService.addBulletin(bulletin); } catch (Exception e) { return ResponseEntity.ok(Message.fail(FAIL_CODE, "Add failed! " + e.getMessage())); @@ -74,7 +74,7 @@ public ResponseEntity> addNewBulletin(@Valid @RequestBody Bulletin @PutMapping public ResponseEntity> editBulletin(@Valid @RequestBody Bulletin bulletin) { try { - bulletinService.validate(bulletin); + bulletinService.validate(bulletin, true); bulletinService.editBulletin(bulletin); } catch (Exception e) { return ResponseEntity.ok(Message.fail(FAIL_CODE, "Edit failed! " + e.getMessage())); diff --git a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/BulletinService.java b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/BulletinService.java index f69840e6f27..70d92c7c32c 100644 --- a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/BulletinService.java +++ b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/BulletinService.java @@ -31,7 +31,7 @@ public interface BulletinService { /** * validate Bulletin */ - void validate(Bulletin bulletin) throws IllegalArgumentException; + void validate(Bulletin bulletin, boolean isModify) throws IllegalArgumentException; /** * Get Bulletin by id diff --git a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java index e7ccb4fd548..0100421bf0f 100644 --- a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java +++ b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/BulletinServiceImpl.java @@ -65,7 +65,7 @@ public class BulletinServiceImpl implements BulletinService { * validate Bulletin */ @Override - public void validate(Bulletin bulletin) throws IllegalArgumentException { + public void validate(Bulletin bulletin, boolean isModify) throws IllegalArgumentException { if (bulletin == null) { throw new IllegalArgumentException("Bulletin cannot be null"); } @@ -78,8 +78,11 @@ public void validate(Bulletin bulletin) throws IllegalArgumentException { if (bulletin.getMonitorIds() == null || bulletin.getMonitorIds().isEmpty()) { throw new IllegalArgumentException("Bulletin monitorIds cannot be null or empty"); } + if (isModify && bulletin.getId() == null) { + throw new IllegalArgumentException("Bulletin id cannot be null when editing"); + } Bulletin existBulletin = bulletinDao.findByName(bulletin.getName()); - if (existBulletin != null && !existBulletin.getId().equals(bulletin.getId())) { + if (existBulletin != null && (!isModify || !existBulletin.getId().equals(bulletin.getId()))) { throw new IllegalArgumentException("Bulletin name duplicated"); } } diff --git a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/controller/BulletinControllerTest.java b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/controller/BulletinControllerTest.java index cc383f72294..2a520a3652d 100644 --- a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/controller/BulletinControllerTest.java +++ b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/controller/BulletinControllerTest.java @@ -62,7 +62,7 @@ void testAddNewBulletin() throws Exception { Bulletin bulletinDto = new Bulletin(); doAnswer(invocation -> { throw new IllegalArgumentException("Invalid bulletin"); - }).when(bulletinService).validate(bulletinDto); + }).when(bulletinService).validate(bulletinDto, false); this.mockMvc.perform(MockMvcRequestBuilders.post("/api/bulletin") .contentType("application/json") @@ -72,7 +72,7 @@ void testAddNewBulletin() throws Exception { doAnswer(invocation -> { return null; - }).when(bulletinService).validate(bulletinDto); + }).when(bulletinService).validate(bulletinDto, false); doAnswer(invocation -> { return null; }).when(bulletinService).addBulletin(bulletinDto); @@ -88,7 +88,7 @@ void testEditBulletin() throws Exception { Bulletin bulletinDto = new Bulletin(); doAnswer(invocation -> { throw new IllegalArgumentException("Invalid bulletin"); - }).when(bulletinService).validate(bulletinDto); + }).when(bulletinService).validate(bulletinDto, true); this.mockMvc.perform(MockMvcRequestBuilders.put("/api/bulletin") .contentType("application/json") @@ -98,7 +98,7 @@ void testEditBulletin() throws Exception { doAnswer(invocation -> { return null; - }).when(bulletinService).validate(bulletinDto); + }).when(bulletinService).validate(bulletinDto, true); doAnswer(invocation -> { return null; }).when(bulletinService).editBulletin(bulletinDto); diff --git a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java index e0f132ca04a..529199829e2 100644 --- a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java +++ b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/BulletinServiceTest.java @@ -73,17 +73,17 @@ public class BulletinServiceTest { @Test public void testValidate() throws Exception { assertThrows(IllegalArgumentException.class, () -> { - bulletinService.validate(null); + bulletinService.validate(null, false); }); assertThrows(IllegalArgumentException.class, () -> { - bulletinService.validate(new Bulletin()); + bulletinService.validate(new Bulletin(), false); }); assertThrows(IllegalArgumentException.class, () -> { Bulletin obj = new Bulletin(); obj.setApp("app"); - bulletinService.validate(obj); + bulletinService.validate(obj, false); }); assertThrows(IllegalArgumentException.class, () -> { @@ -93,7 +93,7 @@ public void testValidate() throws Exception { Bulletin obj = new Bulletin(); obj.setApp("app"); obj.setFields(fields); - bulletinService.validate(obj); + bulletinService.validate(obj, false); }); assertDoesNotThrow(() -> { @@ -107,7 +107,7 @@ public void testValidate() throws Exception { obj.setApp("app"); obj.setFields(fields); obj.setMonitorIds(ids); - bulletinService.validate(obj); + bulletinService.validate(obj, false); }); } @@ -125,6 +125,21 @@ public void testAddBulletin() throws Exception { }); } + @Test + void validateCreateRejectsDuplicateNameEvenWhenClientSubmitsExistingId() { + Bulletin stored = Bulletin.builder().id(7L).name("duplicate-name").build(); + Bulletin submitted = Bulletin.builder() + .id(7L) + .name("duplicate-name") + .app("app") + .fields(Map.of("metric", List.of("field"))) + .monitorIds(List.of(1L)) + .build(); + when(bulletinDao.findByName("duplicate-name")).thenReturn(stored); + + assertThrows(IllegalArgumentException.class, () -> bulletinService.validate(submitted, false)); + } + @Test void addBulletinIgnoresClientManagedFields() { Bulletin bulletin = new Bulletin();