fix(rest/tests): Ensure backwards compatibility when `rest.circuit-br…

…eaker-enabled` is set
spinnaker · Aug 10, 2023 · ed40d3f · ed40d3f
1 parent 4568a32
commit ed40d3f
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 7 deletions.
diff --git a/echo-rest/src/main/java/com/netflix/spinnaker/echo/events/RestEventListener.java b/echo-rest/src/main/java/com/netflix/spinnaker/echo/events/RestEventListener.java
@@ -59,8 +59,11 @@ class RestEventListener implements EventListener {
   @Value("${rest.default-field-name:payload}")
   private String fieldName;
 
+  @Value("${rest.circuit-breaker-enabled:false}")
+  private boolean circuitBreakerEnabled;
+
   @Autowired
-  public RestEventListener(
+  RestEventListener(
       RestUrls restUrls,
       RestEventTemplateEngine restEventTemplateEngine,
       RestEventService restEventService,
@@ -84,7 +87,7 @@ public void processEvent(Event event) {
         .forEach(
             (service) -> {
               try {
-                if (service.getConfig().getCircuitBreakerEnabled()) {
+                if (service.getConfig().getCircuitBreakerEnabled() || circuitBreakerEnabled) {
                   processEventWithCircuitBreaker(event, service);
                 } else {
                   Map<String, Object> eventMap = transformEventToMap(event, service);

diff --git a/echo-rest/src/main/java/com/netflix/spinnaker/echo/events/RestEventService.java b/echo-rest/src/main/java/com/netflix/spinnaker/echo/events/RestEventService.java
@@ -72,7 +72,7 @@ public void sendEvent(Map<String, Object> event, RestUrls.Service service) {
    */
   public CircuitBreaker getCircuitBreakerInstance(RestUrls.Service service) {
     String circuitBreakerInstance =
-        StringUtils.defaultString(service.getConfig().getEventName(), "default");
+        StringUtils.defaultString(service.getConfig().getEventName(), "sendEvent");
 
     return circuitBreakerRegistry.circuitBreaker(circuitBreakerInstance);
   }

diff --git a/echo-rest/src/test/java/com/netflix/spinnaker/echo/events/RestEventListenerTest.java b/echo-rest/src/test/java/com/netflix/spinnaker/echo/events/RestEventListenerTest.java
@@ -44,6 +44,7 @@ class RestEventListenerTest {
   private RestEventListener listener;
   private Event event;
   private RestService restService;
+  private RestEventService restEventService;
 
   private CircuitBreakerRegistry circuitBreakerRegistry;
 
@@ -63,12 +64,11 @@ void setup() {
     // Get the CircuitBreaker from the CircuitBreakerRegistry with a custom configuration
     circuitBreakerRegistry.circuitBreaker("circuitBreakerTest", circuitBreakerConfig);
 
+    restEventService = new RestEventService(new RetrySupport(), circuitBreakerRegistry);
+
     listener =
         new RestEventListener(
-            new RestUrls(),
-            new SimpleEventTemplateEngine(),
-            new RestEventService(new RetrySupport(), circuitBreakerRegistry),
-            new NoopRegistry());
+            new RestUrls(), new SimpleEventTemplateEngine(), restEventService, new NoopRegistry());
 
     event = new Event();
     event.setContent(Map.of("uno", "dos"));
@@ -248,6 +248,32 @@ void shouldSendEventWhenCircuitBreakerIsEnabled() {
     Mockito.verify(restService, Mockito.times(1)).recordEvent(expectedEvent);
   }
 
+  /**
+   * Testing backwards compatibility. Previously if you enabled the circuit breaker via: {@code
+   * rest.circuit-breaker-enabled=true} all rest events used the circuit breaker with the name
+   * "sendEvent". This test ensures that users with legacy configs can continue using the circuit
+   * breaker
+   */
+  @Test
+  void shouldSendEventWithCircuitBreakerWhenListenerCircuitBreakerFlagIsEnabled() {
+    RestProperties.RestEndpointConfiguration config =
+        new RestProperties.RestEndpointConfiguration(); // not enabling circuit breaker per service
+
+    RestUrls.Service service =
+        RestUrls.Service.builder().client(restService).config(config).build();
+
+    listener.getRestUrls().setServices(List.of(service));
+    listener.setCircuitBreakerEnabled(true); // enabling circuit breaker across all rest services
+
+    Map<String, Object> expectedEvent = listener.getMapper().convertValue(event, Map.class);
+
+    listener.processEvent(event);
+
+    Assertions.assertEquals(
+        "sendEvent", restEventService.getCircuitBreakerInstance(service).getName());
+    Mockito.verify(restService, Mockito.times(1)).recordEvent(expectedEvent);
+  }
+
   @Test
   void shouldNotInvokeObjectMapperWhenCircuitBreakerThrowsCallNotPermittedException() {
     RestProperties.RestEndpointConfiguration config =