Skip to content

Commit 2bbace6

Browse files
csviricsviri
authored
fix: API review, minor changes (#775)
1 parent f40023c commit 2bbace6

File tree

8 files changed

+48
-33
lines changed

8 files changed

+48
-33
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,9 @@ public interface Context {
66

77
Optional<RetryInfo> getRetryInfo();
88

9-
<T> T getSecondaryResource(Class<T> expectedType, String... qualifier);
9+
default <T> T getSecondaryResource(Class<T> expectedType) {
10+
return getSecondaryResource(expectedType, null);
11+
}
12+
13+
<T> T getSecondaryResource(Class<T> expectedType, String eventSourceName);
1014
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ public Optional<RetryInfo> getRetryInfo() {
2323
}
2424

2525
@Override
26-
public <T> T getSecondaryResource(Class<T> expectedType, String... qualifier) {
26+
public <T> T getSecondaryResource(Class<T> expectedType, String eventSourceName) {
2727
final var eventSource =
28-
controller.getEventSourceManager().getResourceEventSourceFor(expectedType, qualifier);
28+
controller.getEventSourceManager().getResourceEventSourceFor(expectedType, eventSourceName);
2929
return eventSource.map(es -> es.getAssociated(primaryResource)).orElse(null);
3030
}
3131
}

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,16 @@ public ControllerResourceEventSource<R> getControllerResourceEventSource() {
140140
}
141141

142142
public <S> Optional<ResourceEventSource<R, S>> getResourceEventSourceFor(
143-
Class<S> dependentType, String... qualifier) {
143+
Class<S> dependentType) {
144+
return getResourceEventSourceFor(dependentType, null);
145+
}
146+
147+
public <S> Optional<ResourceEventSource<R, S>> getResourceEventSourceFor(
148+
Class<S> dependentType, String qualifier) {
144149
if (dependentType == null) {
145150
return Optional.empty();
146151
}
147-
String name = (qualifier != null && qualifier.length >= 1) ? qualifier[0] : "";
152+
String name = qualifier == null ? "" : qualifier;
148153
final var eventSource = eventSources.get(dependentType, name);
149154
return Optional.ofNullable(eventSource);
150155
}
@@ -192,13 +197,13 @@ public boolean contains(EventSource source) {
192197
if (eventSources == null || eventSources.isEmpty()) {
193198
return false;
194199
}
195-
return findMatchingSource(qualifier(source), eventSources).isPresent();
200+
return findMatchingSource(name(source), eventSources).isPresent();
196201
}
197202

198203
public void add(EventSource eventSource) {
199204
if (contains(eventSource)) {
200205
throw new IllegalArgumentException("An event source is already registered for the "
201-
+ keyAsString(getDependentType(eventSource), qualifier(eventSource))
206+
+ keyAsString(getDependentType(eventSource), name(eventSource))
202207
+ " class/name combination");
203208
}
204209
sources.computeIfAbsent(keyFor(eventSource), k -> new ArrayList<>()).add(eventSource);
@@ -210,7 +215,7 @@ private Class getDependentType(EventSource source) {
210215
: source.getClass();
211216
}
212217

213-
private String qualifier(EventSource source) {
218+
private String name(EventSource source) {
214219
return source.name();
215220
}
216221

@@ -233,7 +238,7 @@ private String keyFor(Class<?> dependentType) {
233238
return key;
234239
}
235240

236-
public <S> ResourceEventSource<R, S> get(Class<S> dependentType, String qualifier) {
241+
public <S> ResourceEventSource<R, S> get(Class<S> dependentType, String name) {
237242
final var sourcesForType = sources.get(keyFor(dependentType));
238243
if (sourcesForType == null || sourcesForType.isEmpty()) {
239244
return null;
@@ -244,13 +249,13 @@ public <S> ResourceEventSource<R, S> get(Class<S> dependentType, String qualifie
244249
if (size == 1) {
245250
source = sourcesForType.get(0);
246251
} else {
247-
if (qualifier == null || qualifier.isBlank()) {
252+
if (name == null || name.isBlank()) {
248253
throw new IllegalArgumentException("There are multiple EventSources registered for type "
249254
+ dependentType.getCanonicalName()
250-
+ ", you need to provide a qualifier to specify which EventSource you want to query. Known qualifiers: "
251-
+ sourcesForType.stream().map(this::qualifier).collect(Collectors.joining(",")));
255+
+ ", you need to provide a name to specify which EventSource you want to query. Known names: "
256+
+ sourcesForType.stream().map(this::name).collect(Collectors.joining(",")));
252257
}
253-
source = findMatchingSource(qualifier, sourcesForType).orElse(null);
258+
source = findMatchingSource(name, sourcesForType).orElse(null);
254259

255260
if (source == null) {
256261
return null;
@@ -259,29 +264,29 @@ public <S> ResourceEventSource<R, S> get(Class<S> dependentType, String qualifie
259264

260265
if (!(source instanceof ResourceEventSource)) {
261266
throw new IllegalArgumentException(source + " associated with "
262-
+ keyAsString(dependentType, qualifier) + " is not a "
267+
+ keyAsString(dependentType, name) + " is not a "
263268
+ ResourceEventSource.class.getSimpleName());
264269
}
265270
final var res = (ResourceEventSource<R, S>) source;
266271
final var resourceClass = res.getResourceClass();
267272
if (!resourceClass.isAssignableFrom(dependentType)) {
268273
throw new IllegalArgumentException(source + " associated with "
269-
+ keyAsString(dependentType, qualifier)
274+
+ keyAsString(dependentType, name)
270275
+ " is handling " + resourceClass.getName() + " resources but asked for "
271276
+ dependentType.getName());
272277
}
273278
return res;
274279
}
275280

276-
private Optional<EventSource> findMatchingSource(String qualifier,
281+
private Optional<EventSource> findMatchingSource(String name,
277282
List<EventSource> sourcesForType) {
278-
return sourcesForType.stream().filter(es -> qualifier(es).equals(qualifier)).findAny();
283+
return sourcesForType.stream().filter(es -> name(es).equals(name)).findAny();
279284
}
280285

281286
@SuppressWarnings("rawtypes")
282-
private String keyAsString(Class dependentType, String qualifier) {
283-
return qualifier != null && qualifier.length() > 0
284-
? "(" + dependentType.getName() + ", " + qualifier + ")"
287+
private String keyAsString(Class dependentType, String name) {
288+
return name != null && name.length() > 0
289+
? "(" + dependentType.getName() + ", " + name + ")"
285290
: dependentType.getName();
286291
}
287292
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.javaoperatorsdk.operator;
22

3-
import org.junit.Test;
3+
import org.junit.jupiter.api.Test;
44

55
import io.fabric8.kubernetes.api.model.HasMetadata;
66
import io.fabric8.kubernetes.client.CustomResource;
@@ -37,7 +37,6 @@ public void addingMultipleControllersForCustomResourcesWithDifferentVersionsShou
3737
TestCustomResourceV2.class);
3838

3939
checkException(registered, duplicated);
40-
4140
}
4241

4342
private <T extends HasMetadata, U extends HasMetadata> void checkException(

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
1818
import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource;
1919
import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource;
20+
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
2021

2122
import static org.assertj.core.api.Assertions.assertThat;
2223
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -94,48 +95,50 @@ void retrievingEventSourceForClassShouldWork() {
9495
}
9596

9697
@Test
97-
void shouldNotBePossibleToAddEventSourcesForSameTypeAndQualifier() {
98+
void shouldNotBePossibleToAddEventSourcesForSameTypeAndName() {
9899
EventSourceManager manager = initManager();
99100

100101
CachingEventSource eventSource = mock(CachingEventSource.class);
101-
when(eventSource.getResourceClass()).thenReturn(String.class);
102+
when(eventSource.getResourceClass()).thenReturn(TestCustomResource.class);
102103
when(eventSource.name()).thenReturn("name1");
103104
manager.registerEventSource(eventSource);
104105

105106
eventSource = mock(CachingEventSource.class);
106-
when(eventSource.getResourceClass()).thenReturn(String.class);
107+
when(eventSource.getResourceClass()).thenReturn(TestCustomResource.class);
107108
when(eventSource.name()).thenReturn("name1");
108109
final var source = eventSource;
109110

110111
final var exception = assertThrows(OperatorException.class,
111112
() -> manager.registerEventSource(source));
112113
final var cause = exception.getCause();
113114
assertTrue(cause instanceof IllegalArgumentException);
114-
assertTrue(cause.getMessage().contains(
115-
"An event source is already registered for the (java.lang.String, name1) class/name combination"));
115+
assertThat(cause.getMessage()).contains(
116+
"An event source is already registered for the (io.javaoperatorsdk.operator.sample.simple.TestCustomResource, name1) class/name combination");
116117
}
117118

118119
@Test
119120
void retrievingAnEventSourceWhenMultipleAreRegisteredForATypeShouldRequireAQualifier() {
120121
EventSourceManager manager = initManager();
121122

122123
CachingEventSource eventSource = mock(CachingEventSource.class);
123-
when(eventSource.getResourceClass()).thenReturn(String.class);
124+
when(eventSource.getResourceClass()).thenReturn(TestCustomResource.class);
124125
when(eventSource.name()).thenReturn("name1");
125126
manager.registerEventSource(eventSource);
126127

127128
CachingEventSource eventSource2 = mock(CachingEventSource.class);
128-
when(eventSource2.getResourceClass()).thenReturn(String.class);
129+
when(eventSource2.getResourceClass()).thenReturn(TestCustomResource.class);
129130
when(eventSource2.name()).thenReturn("name2");
130131
manager.registerEventSource(eventSource2);
131132

132133
final var exception = assertThrows(IllegalArgumentException.class,
133-
() -> manager.getResourceEventSourceFor(String.class));
134+
() -> manager.getResourceEventSourceFor(TestCustomResource.class));
134135
assertTrue(exception.getMessage().contains("name1"));
135136
assertTrue(exception.getMessage().contains("name2"));
136137

137-
assertTrue(manager.getResourceEventSourceFor(String.class, "name2").get().equals(eventSource2));
138-
assertTrue(manager.getResourceEventSourceFor(String.class, "name1").get().equals(eventSource));
138+
assertTrue(manager.getResourceEventSourceFor(TestCustomResource.class, "name2").get()
139+
.equals(eventSource2));
140+
assertTrue(manager.getResourceEventSourceFor(TestCustomResource.class, "name1").get()
141+
.equals(eventSource));
139142
}
140143

141144
@Test

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResource.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22

33
import io.fabric8.kubernetes.client.CustomResource;
44
import io.fabric8.kubernetes.model.annotation.Group;
5+
import io.fabric8.kubernetes.model.annotation.Kind;
56
import io.fabric8.kubernetes.model.annotation.Version;
67

78
@Group("sample.javaoperatorsdk.io")
89
@Version("v1")
10+
@Kind("TestCustomResource")
911
public class TestCustomResource
1012
extends CustomResource<TestCustomResourceSpec, TestCustomResourceStatus> {
1113

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceV2.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22

33
import io.fabric8.kubernetes.client.CustomResource;
44
import io.fabric8.kubernetes.model.annotation.Group;
5+
import io.fabric8.kubernetes.model.annotation.Kind;
56
import io.fabric8.kubernetes.model.annotation.Version;
67

78
@Group("sample.javaoperatorsdk.io")
89
@Version("v2")
10+
@Kind("TestCustomResource")
911
public class TestCustomResourceV2
1012
extends CustomResource<TestCustomResourceSpec, TestCustomResourceStatus> {
1113

‎sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public List<EventSource> prepareEventSources(
5252
public UpdateControl<MySQLSchema> reconcile(MySQLSchema schema, Context context) {
5353
var dbSchema = context.getSecondaryResource(Schema.class);
5454
try (Connection connection = getConnection()) {
55-
if (dbSchema != null) {
55+
if (dbSchema == null) {
5656
var schemaName = schema.getMetadata().getName();
5757
String password = RandomStringUtils.randomAlphanumeric(16);
5858
String secretName = String.format(SECRET_FORMAT, schemaName);

0 commit comments

Comments
 (0)