Validation that Multiple Rules with Same Name cannot be Created (#266)

* Validation that Multiple Rules with Same Name cannot be Created * resolved comments
hypertrace · Jan 8, 2025 · fada235 · fada235
1 parent 8760970
commit fada235
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 4 deletions.
diff --git a/...in/java/org/hypertrace/notification/config/service/NotificationRuleConfigServiceImpl.java b/...in/java/org/hypertrace/notification/config/service/NotificationRuleConfigServiceImpl.java
@@ -3,6 +3,7 @@
 import io.grpc.Channel;
 import io.grpc.Status;
 import io.grpc.stub.StreamObserver;
+import java.util.List;
 import java.util.UUID;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
@@ -41,7 +42,12 @@ public void createNotificationRule(
       StreamObserver<CreateNotificationRuleResponse> responseObserver) {
     try {
       RequestContext requestContext = RequestContext.CURRENT.get();
-      validator.validateCreateNotificationRuleRequest(requestContext, request);
+      List<NotificationRule> existingNotificationRules =
+          notificationRuleStore.getAllObjects(requestContext).stream()
+              .map(ConfigObject::getData)
+              .collect(Collectors.toList());
+      validator.validateCreateNotificationRuleRequest(
+          requestContext, request, existingNotificationRules);
       NotificationRule notificationRule =
           NotificationRule.newBuilder()
               .setId(UUID.randomUUID().toString())
@@ -66,7 +72,12 @@ public void updateNotificationRule(
       StreamObserver<UpdateNotificationRuleResponse> responseObserver) {
     try {
       RequestContext requestContext = RequestContext.CURRENT.get();
-      validator.validateUpdateNotificationRuleRequest(requestContext, request);
+      List<NotificationRule> existingNotificationRules =
+          notificationRuleStore.getAllObjects(requestContext).stream()
+              .map(ConfigObject::getData)
+              .collect(Collectors.toList());
+      validator.validateUpdateNotificationRuleRequest(
+          requestContext, request, existingNotificationRules);
       NotificationRule notificationRule =
           NotificationRule.newBuilder()
               .setId(request.getId())

diff --git a/...hypertrace/notification/config/service/NotificationRuleConfigServiceRequestValidator.java b/...hypertrace/notification/config/service/NotificationRuleConfigServiceRequestValidator.java
@@ -4,30 +4,54 @@
 import static org.hypertrace.config.validation.GrpcValidatorUtils.validateRequestContextOrThrow;
 
 import io.grpc.Status;
+import java.util.List;
 import org.hypertrace.core.grpcutils.context.RequestContext;
 import org.hypertrace.notification.config.service.v1.CreateNotificationRuleRequest;
 import org.hypertrace.notification.config.service.v1.DeleteNotificationRuleRequest;
 import org.hypertrace.notification.config.service.v1.GetAllNotificationRulesRequest;
 import org.hypertrace.notification.config.service.v1.GetNotificationRuleRequest;
 import org.hypertrace.notification.config.service.v1.NotificationIntegrationTarget;
+import org.hypertrace.notification.config.service.v1.NotificationRule;
 import org.hypertrace.notification.config.service.v1.NotificationRuleMutableData;
 import org.hypertrace.notification.config.service.v1.UpdateNotificationRuleRequest;
 
 public class NotificationRuleConfigServiceRequestValidator {
 
   public void validateCreateNotificationRuleRequest(
-      RequestContext requestContext, CreateNotificationRuleRequest request) {
+      RequestContext requestContext,
+      CreateNotificationRuleRequest request,
+      List<NotificationRule> existingNotificationRules) {
     validateRequestContextOrThrow(requestContext);
+    validateNonDuplicateNotificationRuleOrThrow(
+        request.getNotificationRuleMutableData().getRuleName(), existingNotificationRules);
     validateNotificationRuleMutableData(request.getNotificationRuleMutableData());
   }
 
   public void validateUpdateNotificationRuleRequest(
-      RequestContext requestContext, UpdateNotificationRuleRequest request) {
+      RequestContext requestContext,
+      UpdateNotificationRuleRequest request,
+      List<NotificationRule> existingNotificationRules) {
     validateRequestContextOrThrow(requestContext);
     validateNonDefaultPresenceOrThrow(request, UpdateNotificationRuleRequest.ID_FIELD_NUMBER);
+    validateNonDuplicateNotificationRuleOrThrow(
+        request.getNotificationRuleMutableData().getRuleName(), existingNotificationRules);
     validateNotificationRuleMutableData(request.getNotificationRuleMutableData());
   }
 
+  private void validateNonDuplicateNotificationRuleOrThrow(
+      String ruleName, List<NotificationRule> existingNotificationRules) {
+    for (NotificationRule existingNotificationRule : existingNotificationRules) {
+      if (existingNotificationRule
+          .getNotificationRuleMutableData()
+          .getRuleName()
+          .equals(ruleName)) {
+        throw Status.ALREADY_EXISTS
+            .withDescription("Notification Rule with the same name already exists.")
+            .asRuntimeException();
+      }
+    }
+  }
+
   private void validateNotificationRuleMutableData(NotificationRuleMutableData data) {
     validateNonDefaultPresenceOrThrow(data, NotificationRuleMutableData.RULE_NAME_FIELD_NUMBER);
     if (data.hasIntegrationTarget()) {

diff --git a/...ava/org/hypertrace/notification/config/service/NotificationRuleConfigServiceImplTest.java b/...ava/org/hypertrace/notification/config/service/NotificationRuleConfigServiceImplTest.java
@@ -1,7 +1,9 @@
 package org.hypertrace.notification.config.service;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.util.List;
 import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator;
@@ -21,11 +23,13 @@ class NotificationRuleConfigServiceImplTest {
 
   MockGenericConfigService mockGenericConfigService;
   NotificationRuleConfigServiceGrpc.NotificationRuleConfigServiceBlockingStub notificationStub;
+  NotificationRuleStore notificationRuleStore;
 
   @BeforeEach
   void beforeEach() {
     mockGenericConfigService =
         new MockGenericConfigService().mockUpsert().mockGet().mockGetAll().mockDelete();
+    notificationRuleStore = mock(NotificationRuleStore.class);
 
     ConfigChangeEventGenerator configChangeEventGenerator = mock(ConfigChangeEventGenerator.class);
     mockGenericConfigService
@@ -44,6 +48,7 @@ void createReadUpdateDeleteNotificationRules() {
         getNotificationRuleMutableData("rule1", "channel1");
     NotificationRuleMutableData notificationRuleMutableData2 =
         getNotificationRuleMutableData("rule2", "channel1");
+    when(notificationRuleStore.getAllObjects(any())).thenReturn(List.of());
 
     NotificationRule notificationRule1 =
         notificationStub