improve: ensure unique name on event sources

Signed-off-by: Attila Mészáros <[email protected]>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java

@@ -210,7 +210,7 @@ public <R> List<ResourceEventSource<R, P>> getResourceEventSourcesFor(Class<R> d
   @Override
   public EventSource dynamicallyRegisterEventSource(EventSource eventSource) {
     synchronized (this) {
-      var actual = eventSources.existingEventSourceOfSameNameAndType(eventSource);
+      var actual = eventSources.existingEventSourceByName(eventSource.name());
       if (actual != null) {
         eventSource = actual;
       } else {

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java

@@ -21,10 +21,37 @@ class EventSources<R extends HasMetadata> {
 
   private final ConcurrentNavigableMap<String, Map<String, EventSource>> sources =
       new ConcurrentSkipListMap<>();
+  private final Map<String, EventSource> sourceByName = new HashMap<>();
+
   private final TimerEventSource<R> retryAndRescheduleTimerEventSource =
       new TimerEventSource<>("RetryAndRescheduleTimerEventSource");
   private ControllerResourceEventSource<R> controllerResourceEventSource;
 
+  public void add(EventSource eventSource) {
+    final var name = eventSource.name();
+    var existing = sourceByName.get(name);
+    if (existing != null) {
+      throw new IllegalArgumentException("Event source " + existing
+          + " is already registered with name: " + name);
+    }
+    sourceByName.put(name, eventSource);
+    sources.computeIfAbsent(keyFor(eventSource), k -> new HashMap<>()).put(name, eventSource);
+  }
+
+  public EventSource remove(String name) {
+    var optionalMap = sources.values().stream().filter(m -> m.containsKey(name)).findFirst();
+    sourceByName.remove(name);
+    return optionalMap.map(m -> m.remove(name)).orElse(null);
+  }
+
+  public void clear() {
+    sources.clear();
+    sourceByName.clear();
+  }
+
+  public EventSource existingEventSourceByName(String name) {
+    return sourceByName.get(name);
+  }
 
   void createControllerEventSource(Controller<R> controller) {
     controllerResourceEventSource = new ControllerResourceEventSource<>(controller);
@@ -54,28 +81,7 @@ Stream<EventSource> flatMappedSources() {
     return sources.values().stream().flatMap(c -> c.values().stream());
   }
 
-  public void clear() {
-    sources.clear();
-  }
 
-  public EventSource existingEventSourceOfSameNameAndType(EventSource source) {
-    return existingEventSourceOfSameType(source).get(source.name());
-  }
-
-  public Map<String, EventSource> existingEventSourceOfSameType(EventSource source) {
-    return sources.getOrDefault(keyFor(source), Collections.emptyMap());
-  }
-
-  public void add(EventSource eventSource) {
-    final var name = eventSource.name();
-    final var existing = existingEventSourceOfSameType(eventSource);
-    if (existing.get(name) != null) {
-      throw new IllegalArgumentException("Event source " + existing
-          + " is already registered with name: " + name);
-    }
-
-    sources.computeIfAbsent(keyFor(eventSource), k -> new HashMap<>()).put(name, eventSource);
-  }
 
   @SuppressWarnings("rawtypes")
   private Class<?> getResourceType(EventSource source) {
@@ -158,9 +164,4 @@ public <S> List<ResourceEventSource<S, R>> getEventSources(Class<S> dependentTyp
         .map(es -> (ResourceEventSource<S, R>) es)
         .collect(Collectors.toList());
   }
-
-  public EventSource remove(String name) {
-    var optionalMap = sources.values().stream().filter(m -> m.containsKey(name)).findFirst();
-    return optionalMap.map(m -> m.remove(name)).orElse(null);
-  }
 }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java

@@ -46,7 +46,9 @@ public void registersEventSource() {
   @Test
   public void closeShouldCascadeToEventSources() {
     EventSource eventSource = mock(EventSource.class);
+    when(eventSource.name()).thenReturn("name1");
     EventSource eventSource2 = mock(TimerEventSource.class);
+    when(eventSource2.name()).thenReturn("name2");
 
     eventSourceManager.registerEventSource(eventSource);
     eventSourceManager.registerEventSource(eventSource2);
@@ -61,8 +63,10 @@ public void closeShouldCascadeToEventSources() {
   public void startCascadesToEventSources() {
     EventSource eventSource = mock(EventSource.class);
     when(eventSource.priority()).thenReturn(EventSourceStartPriority.DEFAULT);
+    when(eventSource.name()).thenReturn("name1");
     EventSource eventSource2 = mock(TimerEventSource.class);
     when(eventSource2.priority()).thenReturn(EventSourceStartPriority.DEFAULT);
+    when(eventSource2.name()).thenReturn("name2");
     eventSourceManager.registerEventSource(eventSource);
     eventSourceManager.registerEventSource(eventSource2);
 

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java

@@ -129,15 +129,15 @@ void getShouldWork() {
         eventSourceMockWithName(ResourceEventSource.class, "name1", HasMetadata.class);
     final var mock2 =
         eventSourceMockWithName(ResourceEventSource.class, "name2", HasMetadata.class);
-    final var mock3 = eventSourceMockWithName(ResourceEventSource.class, "name2", ConfigMap.class);
+    final var mock3 = eventSourceMockWithName(ResourceEventSource.class, "name3", ConfigMap.class);
 
     eventSources.add(mock1);
     eventSources.add(mock2);
     eventSources.add(mock3);
 
     assertEquals(mock1, eventSources.get(HasMetadata.class, "name1"));
     assertEquals(mock2, eventSources.get(HasMetadata.class, "name2"));
-    assertEquals(mock3, eventSources.get(ConfigMap.class, "name2"));
+    assertEquals(mock3, eventSources.get(ConfigMap.class, "name3"));
     assertEquals(mock3, eventSources.get(ConfigMap.class, null));