Define bulletin write fields explicitly

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;
@@ -27,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;
@@ -50,12 +52,14 @@
 @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
     @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 +78,19 @@ public class Bulletin {
     @Convert(converter = JsonMapListAttributeConverter.class)
     private Map<String, List<String>> 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;
 }

hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/BulletinController.java

@@ -62,7 +62,7 @@ public class BulletinController {
     @PostMapping
     public ResponseEntity<Message<Void>> 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<Message<Void>> addNewBulletin(@Valid @RequestBody Bulletin
     @PutMapping
     public ResponseEntity<Message<Void>> 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()));

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

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");
         }
     }
@@ -94,7 +97,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 +111,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);
     }
 
     /**

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);

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;
 
 /**
@@ -67,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, () -> {
@@ -87,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(() -> {
@@ -101,7 +107,7 @@ public void testValidate() throws Exception {
             obj.setApp("app");
             obj.setFields(fields);
             obj.setMonitorIds(ids);
-            bulletinService.validate(obj);
+            bulletinService.validate(obj, false);
         });
     }
 
@@ -119,6 +125,75 @@ 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();
+        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<Bulletin> 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<Bulletin> 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();