[bugfix] the tagValue may be null in the determineNewLabels method (#3606)

Co-authored-by: Calvin <zhengqiwei@apache.org> Co-authored-by: Yang Chen <1597081640@qq.com>
2026-09-17 09:40:58 +00:00 · 2025-07-30 22:00:19 +08:00
parent 9f7b9994b8
commit babbfaea68
2 changed files with 49 additions and 26 deletions
@@ -67,7 +67,7 @@ public class LabelServiceImpl implements LabelService {
    public void modifyLabel(Label label) {
        Optional<Label> optional = labelDao.findById(label.getId());
        if (optional.isPresent()) {
-            
+
            Optional<Label> existOptional = labelDao.findLabelByNameAndTagValue(label.getName(), label.getTagValue());
            if (existOptional.isPresent() && !existOptional.get().getId().equals(label.getId())) {
                throw new IllegalArgumentException("The label with same key and value already exists.");
@@ -116,33 +116,38 @@ public class LabelServiceImpl implements LabelService {

    @Override
    public void deleteLabels(HashSet<Long> ids) {
-        if (CollectionUtils.isEmpty(ids)){
+        if (CollectionUtils.isEmpty(ids)) {
            return;
        }
        labelDao.deleteLabelsByIdIn(ids);
    }

-    public List<Label> determineNewLabels(Set<Map.Entry<String, String>> originLabels){
+    @Override
+    public List<Label> determineNewLabels(Set<Map.Entry<String, String>> originLabels) {

        if (originLabels == null || originLabels.isEmpty()) return List.of();

        // Get all labels from the database
-        Set<Map.Entry<String, String>> allLabels = labelDao.findAll().stream()
-                .map(label -> Map.entry(label.getName(), label.getTagValue()))
+        Set<Label> allLabels = labelDao.findAll().stream()
+                .map(label -> Label.builder()
+                        .name(label.getName())
+                        .tagValue(label.getTagValue())
+                        .build())
                .collect(Collectors.toSet());

        // If the bound label (key:value) does not exist, then add it
-        Set<Map.Entry<String, String>> addLabelsKv = originLabels.stream()
+        return originLabels.stream()
+                .map(entry -> Label.builder()
+                        .name(entry.getKey())
+                        .tagValue(entry.getValue())
+                        .build())
                .filter(label -> !allLabels.contains(label))
-                .collect(Collectors.toCollection(HashSet::new));
-
-        return addLabelsKv.stream().map(kv -> {
-            Label label = new Label();
-            label.setId(null);
-            label.setName(kv.getKey());
-            label.setTagValue(kv.getValue());
-            label.setType((byte) 0);
-            return label;
-        }).toList();
+                .map(label -> Label.builder()
+                        .id(null)
+                        .name(label.getName())
+                        .tagValue(label.getTagValue())
+                        .type((byte) 0)
+                        .build())
+                .toList();
    }
 }
@@ -17,6 +17,7 @@

 package org.apache.hertzbeat.manager.service;

+import org.junit.jupiter.api.Assertions;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -26,7 +27,10 @@ import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;

+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;

 import org.apache.hertzbeat.common.entity.manager.Label;
@@ -46,25 +50,25 @@ import org.springframework.data.jpa.domain.Specification;
 */
@ExtendWith(MockitoExtension.class)
 class LabelServiceTest {
-    
+
    @InjectMocks
    private LabelServiceImpl labelService;
-    
+
    @Mock
    private LabelDao labelDao;
-    
+
    @Test
    void addLabel() {
        // Prepare test data
-        Label label =  Label.builder().id(1L).name("tagname").tagValue("tagvalue").build();
+        Label label = Label.builder().id(1L).name("tagname").tagValue("tagvalue").build();
        when(labelDao.findLabelByNameAndTagValue(anyString(), anyString())).thenReturn(Optional.empty());
-        
+
        labelService.addLabel(label);
-        
+
        verify(labelDao).save(label);
-        
+
    }
-    
+
    @Test
    void modifyLabel() {
        Label tag = Label.builder().id(1L).build();
@@ -75,15 +79,29 @@ class LabelServiceTest {
        when(labelDao.findById(1L)).thenReturn(Optional.empty());
        assertThrows(IllegalArgumentException.class, () -> labelService.modifyLabel(tag));
    }
-    
+
    @Test
    void getLabels() {
        when(labelDao.findAll(any(Specification.class), any(PageRequest.class))).thenReturn(Page.empty());
        assertNotNull(labelService.getLabels(null, null, 1, 10));
    }
-    
+
    @Test
    void deleteLabels() {
        assertDoesNotThrow(() -> labelService.deleteLabels(new HashSet<>(1)));
    }
+
+    @Test
+    void determineNewLabels() {
+        List<Label> allLabelList = labelDao.findAll();
+        allLabelList.add(Label.builder().name("tag1").build());
+        when(labelDao.findAll()).thenReturn(allLabelList);
+
+        Map<String, String> labels = new HashMap<>();
+        labels.put("tag1", null);
+        labels.put("tag2", "2");
+        assertDoesNotThrow(() -> labelService.determineNewLabels(labels.entrySet()));
+        assertNotNull(labelService.determineNewLabels(labels.entrySet()));
+        Assertions.assertEquals(1, labelService.determineNewLabels(labels.entrySet()).size());
+    }
 }