From be6e5c63a619be836354c6b5da5bba03d1d566fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 30 Apr 2024 10:20:02 +0200 Subject: [PATCH 1/2] improve: ensure unique name on event sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../processing/event/EventSourceManager.java | 2 +- .../processing/event/EventSources.java | 60 +++++++++---------- .../event/EventSourceManagerTest.java | 9 ++- .../processing/event/EventSourcesTest.java | 6 +- 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index a17328e7d7..c65d897734 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -213,7 +213,7 @@ public List> getEventSourcesFor(Class dependentType) { @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 { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java index 79091de0d3..3891da2837 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java @@ -19,10 +19,37 @@ class EventSources

{ private final ConcurrentNavigableMap>> sources = new ConcurrentSkipListMap<>(); - private final TimerEventSource

retryAndRescheduleTimerEventSource = - new TimerEventSource<>("RetryAndRescheduleTimerEventSource"); + private final Map sourceByName = new HashMap<>(); + + private final TimerEventSource

retryAndRescheduleTimerEventSource = + new TimerEventSource<>("RetryAndRescheduleTimerEventSource"); private ControllerEventSource

controllerEventSource; + 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

controller) { controllerEventSource = new ControllerEventSource<>(controller); @@ -54,30 +81,7 @@ Stream> flatMappedSources() { return sources.values().stream().flatMap(c -> c.values().stream()); } - public void clear() { - sources.clear(); - } - - @SuppressWarnings("unchecked") - public EventSource existingEventSourceOfSameNameAndType(EventSource source) { - return (EventSource) existingEventSourcesOfSameType(source).get(source.name()); - } - - private Map> existingEventSourcesOfSameType( - EventSource source) { - return sources.getOrDefault(keyFor(source), Collections.emptyMap()); - } - public void add(EventSource eventSource) { - final var name = eventSource.name(); - final var existing = existingEventSourcesOfSameType(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); - } private String keyFor(EventSource source) { return keyFor(source.resourceType()); @@ -145,10 +149,4 @@ public List> getEventSources(Class dependentType) { return sourcesForType.values().stream() .map(es -> (EventSource) es).toList(); } - - @SuppressWarnings("rawtypes") - 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); - } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index b5ff379dd6..3c27a66e00 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -48,9 +48,9 @@ public void registersEventSource() { @Test public void closeShouldCascadeToEventSources() { EventSource eventSource = mock(EventSource.class); - when(eventSource.resourceType()).thenReturn(EventSource.class); + when(eventSource.name()).thenReturn("name1"); EventSource eventSource2 = mock(TimerEventSource.class); - when(eventSource2.resourceType()).thenReturn(AbstractEventSource.class); + when(eventSource2.name()).thenReturn("name2"); eventSourceManager.registerEventSource(eventSource); eventSourceManager.registerEventSource(eventSource2); @@ -65,11 +65,10 @@ public void closeShouldCascadeToEventSources() { public void startCascadesToEventSources() { EventSource eventSource = mock(EventSource.class); when(eventSource.priority()).thenReturn(EventSourceStartPriority.DEFAULT); - when(eventSource.resourceType()).thenReturn(EventSource.class); + when(eventSource.name()).thenReturn("name1"); EventSource eventSource2 = mock(TimerEventSource.class); when(eventSource2.priority()).thenReturn(EventSourceStartPriority.DEFAULT); - when(eventSource2.resourceType()).thenReturn(AbstractEventSource.class); - + when(eventSource2.name()).thenReturn("name2"); eventSourceManager.registerEventSource(eventSource); eventSourceManager.registerEventSource(eventSource2); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java index 9c2d09bac4..60e702d016 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java @@ -133,8 +133,8 @@ void getShouldWork() { final var mock1 = eventSourceMockWithName(EventSource.class, "name1", HasMetadata.class); final var mock2 = - eventSourceMockWithName(EventSource.class, "name2", HasMetadata.class); - final var mock3 = eventSourceMockWithName(EventSource.class, "name2", ConfigMap.class); + eventSourceMockWithName(ResourceEventSource.class, "name2", HasMetadata.class); + final var mock3 = eventSourceMockWithName(ResourceEventSource.class, "name3", ConfigMap.class); eventSources.add(mock1); eventSources.add(mock2); @@ -142,7 +142,7 @@ void getShouldWork() { 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)); From 8c31ab9fa62b18df646933d9b6f0473302eddce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 May 2024 11:21:33 +0200 Subject: [PATCH 2/2] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/event/EventSources.java | 4 ++-- .../operator/processing/event/EventSourceManagerTest.java | 5 +++++ .../operator/processing/event/EventSourcesTest.java | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java index 3891da2837..e790ae3c32 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java @@ -21,8 +21,8 @@ class EventSources

{ new ConcurrentSkipListMap<>(); private final Map sourceByName = new HashMap<>(); - private final TimerEventSource

retryAndRescheduleTimerEventSource = - new TimerEventSource<>("RetryAndRescheduleTimerEventSource"); + private final TimerEventSource

retryAndRescheduleTimerEventSource = + new TimerEventSource<>("RetryAndRescheduleTimerEventSource"); private ControllerEventSource

controllerEventSource; public void add(EventSource eventSource) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index 3c27a66e00..7e45bda68c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -49,8 +49,11 @@ public void registersEventSource() { public void closeShouldCascadeToEventSources() { EventSource eventSource = mock(EventSource.class); when(eventSource.name()).thenReturn("name1"); + when(eventSource.resourceType()).thenReturn(EventSource.class); + EventSource eventSource2 = mock(TimerEventSource.class); when(eventSource2.name()).thenReturn("name2"); + when(eventSource2.resourceType()).thenReturn(AbstractEventSource.class); eventSourceManager.registerEventSource(eventSource); eventSourceManager.registerEventSource(eventSource2); @@ -66,9 +69,11 @@ public void startCascadesToEventSources() { EventSource eventSource = mock(EventSource.class); when(eventSource.priority()).thenReturn(EventSourceStartPriority.DEFAULT); when(eventSource.name()).thenReturn("name1"); + when(eventSource.resourceType()).thenReturn(EventSource.class); EventSource eventSource2 = mock(TimerEventSource.class); when(eventSource2.priority()).thenReturn(EventSourceStartPriority.DEFAULT); when(eventSource2.name()).thenReturn("name2"); + when(eventSource2.resourceType()).thenReturn(AbstractEventSource.class); eventSourceManager.registerEventSource(eventSource); eventSourceManager.registerEventSource(eventSource2); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java index 60e702d016..3d0d92da4f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java @@ -133,8 +133,8 @@ void getShouldWork() { final var mock1 = eventSourceMockWithName(EventSource.class, "name1", HasMetadata.class); final var mock2 = - eventSourceMockWithName(ResourceEventSource.class, "name2", HasMetadata.class); - final var mock3 = eventSourceMockWithName(ResourceEventSource.class, "name3", ConfigMap.class); + eventSourceMockWithName(EventSource.class, "name2", HasMetadata.class); + final var mock3 = eventSourceMockWithName(EventSource.class, "name3", ConfigMap.class); eventSources.add(mock1); eventSources.add(mock2);