Skip to content

Commit f1ead67

Browse files
metacosmmetacosm
authored
refactor: improve configuration utilities (#1519)
* refactor: improve configuration utilities * fix: account for parameterized argument
1 parent efd47da commit f1ead67

File tree

3 files changed

+145
-78
lines changed

3 files changed

+145
-78
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java‎

Lines changed: 32 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package io.javaoperatorsdk.operator.api.config;
22

33
import java.lang.annotation.Annotation;
4-
import java.lang.reflect.Constructor;
5-
import java.lang.reflect.InvocationTargetException;
64
import java.time.Duration;
75
import java.util.Arrays;
86
import java.util.Collections;
@@ -41,10 +39,6 @@
4139
public class AnnotationControllerConfiguration<P extends HasMetadata>
4240
implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration<P> {
4341

44-
private static final String CONTROLLER_CONFIG_ANNOTATION =
45-
ControllerConfiguration.class.getSimpleName();
46-
private static final String KUBE_DEPENDENT_NAME = KubernetesDependent.class.getSimpleName();
47-
4842
protected final Reconciler<P> reconciler;
4943
private final ControllerConfiguration annotation;
5044
private List<DependentResourceSpec> specs;
@@ -152,71 +146,54 @@ public Optional<Duration> maxReconciliationInterval() {
152146
@Override
153147
public RateLimiter getRateLimiter() {
154148
final Class<? extends RateLimiter> rateLimiterClass = annotation.rateLimiter();
155-
return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class,
156-
CONTROLLER_CONFIG_ANNOTATION);
149+
return Utils.instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class,
150+
Utils.contextFor(this, null, null), this::configureFromAnnotatedReconciler);
157151
}
158152

159153
@Override
160154
public Retry getRetry() {
161155
final Class<? extends Retry> retryClass = annotation.retry();
162-
return instantiateAndConfigureIfNeeded(retryClass, Retry.class, CONTROLLER_CONFIG_ANNOTATION);
156+
return Utils.instantiateAndConfigureIfNeeded(retryClass, Retry.class,
157+
Utils.contextFor(this, null, null), this::configureFromAnnotatedReconciler);
163158
}
164159

160+
165161
@SuppressWarnings("unchecked")
166-
protected <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
167-
Class<T> expectedType, String context) {
168-
try {
169-
final Constructor<? extends T> constructor = targetClass.getDeclaredConstructor();
170-
constructor.setAccessible(true);
171-
final var instance = constructor.newInstance();
172-
if (instance instanceof AnnotationConfigurable) {
173-
AnnotationConfigurable configurable = (AnnotationConfigurable) instance;
174-
final Class<? extends Annotation> configurationClass =
175-
(Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
176-
targetClass, AnnotationConfigurable.class);
177-
final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
178-
if (configAnnotation != null) {
179-
configurable.initFrom(configAnnotation);
180-
}
162+
private <T> void configureFromAnnotatedReconciler(T instance) {
163+
if (instance instanceof AnnotationConfigurable) {
164+
AnnotationConfigurable configurable = (AnnotationConfigurable) instance;
165+
final Class<? extends Annotation> configurationClass =
166+
(Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
167+
instance.getClass(), AnnotationConfigurable.class);
168+
final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
169+
if (configAnnotation != null) {
170+
configurable.initFrom(configAnnotation);
181171
}
182-
return instance;
183-
} catch (InstantiationException | IllegalAccessException | InvocationTargetException
184-
| NoSuchMethodException e) {
185-
throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '"
186-
+ targetClass.getName() + "' for '" + getName()
187-
+ "' reconciler in " + context
188-
+ ". You need to provide an accessible no-arg constructor.", e);
189172
}
190173
}
191174

192175
@Override
193176
@SuppressWarnings("unchecked")
194177
public Optional<OnAddFilter<P>> onAddFilter() {
195-
return (Optional<OnAddFilter<P>>) createFilter(annotation.onAddFilter(), OnAddFilter.class,
196-
CONTROLLER_CONFIG_ANNOTATION);
197-
}
198-
199-
protected <T> Optional<? extends T> createFilter(Class<? extends T> filter, Class<T> defaultValue,
200-
String origin) {
201-
if (defaultValue.equals(filter)) {
202-
return Optional.empty();
203-
} else {
204-
return Optional.of(instantiateAndConfigureIfNeeded(filter, defaultValue, origin));
205-
}
178+
return Optional.ofNullable(
179+
Utils.instantiate(annotation.onAddFilter(), OnAddFilter.class,
180+
Utils.contextFor(this, null, null)));
206181
}
207182

208183
@SuppressWarnings("unchecked")
209184
@Override
210185
public Optional<OnUpdateFilter<P>> onUpdateFilter() {
211-
return (Optional<OnUpdateFilter<P>>) createFilter(annotation.onUpdateFilter(),
212-
OnUpdateFilter.class, CONTROLLER_CONFIG_ANNOTATION);
186+
return Optional.ofNullable(
187+
Utils.instantiate(annotation.onUpdateFilter(), OnUpdateFilter.class,
188+
Utils.contextFor(this, null, null)));
213189
}
214190

215191
@SuppressWarnings("unchecked")
216192
@Override
217193
public Optional<GenericFilter<P>> genericFilter() {
218-
return (Optional<GenericFilter<P>>) createFilter(annotation.genericFilter(),
219-
GenericFilter.class, CONTROLLER_CONFIG_ANNOTATION);
194+
return Optional.ofNullable(
195+
Utils.instantiate(annotation.genericFilter(), GenericFilter.class,
196+
Utils.contextFor(this, null, null)));
220197
}
221198

222199
@SuppressWarnings({"rawtypes", "unchecked"})
@@ -244,12 +221,12 @@ public List<DependentResourceSpec> getDependentResources() {
244221
throw new IllegalArgumentException(
245222
"A DependentResource named '" + name + "' already exists: " + spec);
246223
}
247-
final var context = "DependentResource of type '" + dependentType.getName() + "'";
224+
final var context = Utils.contextFor(this, dependentType, null);
248225
spec = new DependentResourceSpec(dependentType, config, name,
249226
Set.of(dependent.dependsOn()),
250-
instantiateConditionIfNotDefault(dependent.readyPostcondition(), context),
251-
instantiateConditionIfNotDefault(dependent.reconcilePrecondition(), context),
252-
instantiateConditionIfNotDefault(dependent.deletePostcondition(), context));
227+
Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
228+
Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
229+
Utils.instantiate(dependent.deletePostcondition(), Condition.class, context));
253230
specsMap.put(name, spec);
254231
}
255232

@@ -258,14 +235,6 @@ public List<DependentResourceSpec> getDependentResources() {
258235
return specs;
259236
}
260237

261-
protected Condition<?, ?> instantiateConditionIfNotDefault(Class<? extends Condition> condition,
262-
String context) {
263-
if (condition != Condition.class) {
264-
return instantiateAndConfigureIfNeeded(condition, Condition.class, context);
265-
}
266-
return null;
267-
}
268-
269238
private String getName(Dependent dependent, Class<? extends DependentResource> dependentType) {
270239
var name = dependent.name();
271240
if (name.isBlank()) {
@@ -299,18 +268,14 @@ private Object createKubernetesResourceConfig(Class<? extends DependentResource>
299268

300269

301270
final var context =
302-
KUBE_DEPENDENT_NAME + " annotation on " + dependentType.getName() + " DependentResource";
303-
onAddFilter = createFilter(kubeDependent.onAddFilter(), OnAddFilter.class, context)
304-
.orElse(null);
271+
Utils.contextFor(this, dependentType, null);
272+
onAddFilter = Utils.instantiate(kubeDependent.onAddFilter(), OnAddFilter.class, context);
305273
onUpdateFilter =
306-
createFilter(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context)
307-
.orElse(null);
274+
Utils.instantiate(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context);
308275
onDeleteFilter =
309-
createFilter(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context)
310-
.orElse(null);
276+
Utils.instantiate(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context);
311277
genericFilter =
312-
createFilter(kubeDependent.genericFilter(), GenericFilter.class, context)
313-
.orElse(null);
278+
Utils.instantiate(kubeDependent.genericFilter(), GenericFilter.class, context);
314279
}
315280

316281
config =

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java‎

Lines changed: 102 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
11
package io.javaoperatorsdk.operator.api.config;
22

33
import java.io.IOException;
4+
import java.lang.annotation.Annotation;
5+
import java.lang.reflect.Constructor;
6+
import java.lang.reflect.InvocationTargetException;
47
import java.lang.reflect.ParameterizedType;
58
import java.lang.reflect.Type;
69
import java.text.SimpleDateFormat;
710
import java.time.Instant;
811
import java.util.Arrays;
912
import java.util.Date;
13+
import java.util.Optional;
1014
import java.util.Properties;
1115

1216
import org.slf4j.Logger;
1317
import org.slf4j.LoggerFactory;
1418

1519
import io.javaoperatorsdk.operator.OperatorException;
20+
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
1621

1722
public class Utils {
1823

@@ -106,17 +111,49 @@ public static Class<?> getFirstTypeArgumentFromExtendedClass(Class<?> clazz) {
106111

107112
public static Class<?> getFirstTypeArgumentFromInterface(Class<?> clazz,
108113
Class<?> expectedImplementedInterface) {
109-
return Arrays.stream(clazz.getGenericInterfaces())
110-
.filter(type -> type.getTypeName().startsWith(expectedImplementedInterface.getName())
111-
&& type instanceof ParameterizedType)
112-
.map(ParameterizedType.class::cast)
113-
.findFirst()
114-
.map(t -> (Class<?>) t.getActualTypeArguments()[0])
115-
.orElseThrow(() -> new RuntimeException(
116-
"Couldn't retrieve generic parameter type from " + clazz.getSimpleName()
117-
+ " because it doesn't implement "
118-
+ expectedImplementedInterface.getSimpleName()
119-
+ " directly"));
114+
if (expectedImplementedInterface.isAssignableFrom(clazz)) {
115+
final var genericInterfaces = clazz.getGenericInterfaces();
116+
Optional<? extends Class<?>> target = Optional.empty();
117+
if (genericInterfaces.length > 0) {
118+
// try to find the target interface among them
119+
target = Arrays.stream(genericInterfaces)
120+
.filter(type -> type.getTypeName().startsWith(expectedImplementedInterface.getName())
121+
&& type instanceof ParameterizedType)
122+
.map(ParameterizedType.class::cast)
123+
.findFirst()
124+
.map(t -> {
125+
final Type argument = t.getActualTypeArguments()[0];
126+
if (argument instanceof Class) {
127+
return (Class<?>) argument;
128+
}
129+
// account for the case where the argument itself has parameters, which we will ignore
130+
// and just return the raw type
131+
if (argument instanceof ParameterizedType) {
132+
final var rawType = ((ParameterizedType) argument).getRawType();
133+
if (rawType instanceof Class) {
134+
return (Class<?>) rawType;
135+
}
136+
}
137+
throw new IllegalArgumentException(clazz.getSimpleName() + " implements "
138+
+ expectedImplementedInterface.getSimpleName()
139+
+ " but indirectly. Java type erasure doesn't allow to retrieve the generic type from it. Retrieved type was: "
140+
+ argument);
141+
});
142+
}
143+
144+
if (target.isPresent()) {
145+
return target.get();
146+
}
147+
148+
// try the parent
149+
var parent = clazz.getSuperclass();
150+
if (!Object.class.equals(parent)) {
151+
return getFirstTypeArgumentFromInterface(parent, expectedImplementedInterface);
152+
}
153+
}
154+
throw new IllegalArgumentException("Couldn't retrieve generic parameter type from "
155+
+ clazz.getSimpleName() + " because it or its superclasses don't implement "
156+
+ expectedImplementedInterface.getSimpleName());
120157
}
121158

122159
public static Class<?> getFirstTypeArgumentFromSuperClassOrInterface(Class<?> clazz,
@@ -144,4 +181,58 @@ public static Class<?> getFirstTypeArgumentFromSuperClassOrInterface(Class<?> cl
144181
"Couldn't retrieve generic parameter type from " + clazz.getSimpleName(), e);
145182
}
146183
}
184+
185+
public static <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
186+
Class<T> expectedType, String context, Configurator<T> configurator) {
187+
// if class to instantiate equals the expected interface, we cannot instantiate it so just
188+
// return null as it means we passed on void-type default value
189+
if (expectedType.equals(targetClass)) {
190+
return null;
191+
}
192+
193+
try {
194+
final Constructor<? extends T> constructor = targetClass.getDeclaredConstructor();
195+
constructor.setAccessible(true);
196+
final var instance = constructor.newInstance();
197+
198+
if (configurator != null) {
199+
configurator.configure(instance);
200+
}
201+
202+
return instance;
203+
} catch (InstantiationException | IllegalAccessException | InvocationTargetException
204+
| NoSuchMethodException e) {
205+
throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '"
206+
+ targetClass.getName() + "': you need to provide an accessible no-arg constructor."
207+
+ (context != null ? " Context: " + context : ""), e);
208+
}
209+
}
210+
211+
public static <T> T instantiate(Class<? extends T> toInstantiate, Class<T> expectedType,
212+
String context) {
213+
return instantiateAndConfigureIfNeeded(toInstantiate, expectedType, context, null);
214+
}
215+
216+
@FunctionalInterface
217+
public interface Configurator<T> {
218+
void configure(T instance);
219+
}
220+
221+
@SuppressWarnings("rawtypes")
222+
public static String contextFor(ControllerConfiguration<?> controllerConfiguration,
223+
Class<? extends DependentResource> dependentType,
224+
Class<? extends Annotation> configurationAnnotation) {
225+
final var annotationName =
226+
configurationAnnotation != null ? configurationAnnotation.getSimpleName()
227+
: io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class
228+
.getSimpleName();
229+
var context = "annotation: " + annotationName + ", ";
230+
if (dependentType != null) {
231+
context += "DependentResource: " + dependentType.getName() + ", ";
232+
}
233+
context += "reconciler: " + controllerConfiguration.getName();
234+
235+
236+
return context;
237+
}
147238
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@
99
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
1010
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
1111
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
12+
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
1213
import io.javaoperatorsdk.operator.processing.dependent.EmptyTestDependentResource;
1314
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
15+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
1416
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
1517

1618
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
1720
import static org.junit.jupiter.api.Assertions.assertFalse;
1821
import static org.junit.jupiter.api.Assertions.assertNotNull;
1922
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -89,6 +92,14 @@ void getsFirstTypeArgumentFromInterface() {
8992
assertThat(Utils.getFirstTypeArgumentFromInterface(EmptyTestDependentResource.class,
9093
DependentResource.class))
9194
.isEqualTo(Deployment.class);
95+
96+
assertThatIllegalArgumentException().isThrownBy(
97+
() -> Utils.getFirstTypeArgumentFromInterface(TestKubernetesDependentResource.class,
98+
DependentResource.class));
99+
100+
assertThat(Utils.getFirstTypeArgumentFromInterface(TestKubernetesDependentResource.class,
101+
DependentResourceConfigurator.class))
102+
.isEqualTo(KubernetesDependentResourceConfig.class);
92103
}
93104

94105
@Test

0 commit comments

Comments
 (0)