Skip to content

Commit ac93528

Browse files
metacosmmetacosm
authored
refactor: add default implementation for name() (#2255)
 Signed-off-by: Chris Laprun <[email protected]>
1 parent 77c5fc3 commit ac93528

File tree

13 files changed

+42
-65
lines changed

13 files changed

+42
-65
lines changed

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ default boolean isDeletable() {
6969
return this instanceof Deleter;
7070
}
7171

72-
String name();
72+
default String name() {
73+
return defaultNameFor(getClass());
74+
}
7375

7476
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,21 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
3030
private ResourceDiscriminator<R, P> resourceDiscriminator;
3131
private final DependentResourceReconciler<R, P> dependentResourceReconciler;
3232

33-
protected String name = DependentResource.defaultNameFor(this.getClass());
33+
protected String name;
3434

3535
@SuppressWarnings({"unchecked"})
3636
protected AbstractDependentResource() {
37+
this(null);
38+
}
39+
40+
protected AbstractDependentResource(String name) {
3741
creator = creatable ? (Creator<R, P>) this : null;
3842
updater = updatable ? (Updater<R, P>) this : null;
3943

4044
dependentResourceReconciler = this instanceof BulkDependentResource
4145
? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P>) this)
4246
: new SingleDependentResourceReconciler<>(this);
47+
this.name = name == null ? DependentResource.defaultNameFor(this.getClass()) : name;
4348
}
4449

4550
/**

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java

+5
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ public abstract class AbstractEventSourceHolderDependentResource<R, P extends Ha
3131
protected String eventSourceNameToUse;
3232

3333
protected AbstractEventSourceHolderDependentResource(Class<R> resourceType) {
34+
this(resourceType, null);
35+
}
36+
37+
protected AbstractEventSourceHolderDependentResource(Class<R> resourceType, String name) {
38+
super(name);
3439
this.resourceType = resourceType;
3540
}
3641

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
4343

4444
@SuppressWarnings("unchecked")
4545
public KubernetesDependentResource(Class<R> resourceType) {
46-
super(resourceType);
46+
this(resourceType, null);
47+
}
48+
49+
@SuppressWarnings("unchecked")
50+
public KubernetesDependentResource(Class<R> resourceType, String name) {
51+
super(resourceType, name);
4752

4853
usingCustomResourceUpdateMatcher = this instanceof ResourceUpdaterMatcher;
4954
updaterMatcher = usingCustomResourceUpdateMatcher

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java

+6-9
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import io.javaoperatorsdk.operator.processing.event.ResourceID;
1919

2020
@SuppressWarnings("rawtypes")
21-
public abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
21+
abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
2222

2323
protected final Workflow<P> workflow;
2424
protected final P primary;
@@ -133,19 +133,16 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
133133
boolean activationConditionMet,
134134
DependentResourceNode<R, P> dependentResourceNode) {
135135
if (dependentResourceNode.getActivationCondition().isPresent()) {
136+
final var dr = dependentResourceNode.getDependentResource();
137+
final var eventSourceRetriever = context.eventSourceRetriever();
136138
if (activationConditionMet) {
137139
var eventSource =
138-
dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever()
139-
.eventSourceContextForDynamicRegistration());
140+
dr.eventSource(eventSourceRetriever.eventSourceContextForDynamicRegistration());
140141
var es = eventSource.orElseThrow();
141-
context.eventSourceRetriever()
142-
.dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().name(),
143-
es);
142+
eventSourceRetriever.dynamicallyRegisterEventSource(dr.name(), es);
144143

145144
} else {
146-
context.eventSourceRetriever()
147-
.dynamicallyDeRegisterEventSource(
148-
dependentResourceNode.getDependentResource().name());
145+
eventSourceRetriever.dynamicallyDeRegisterEventSource(dr.name());
149146
}
150147
}
151148
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,14 @@ public Workflow<P> resolve(KubernetesClient client,
7979
ControllerConfiguration<P> configuration) {
8080
final var alreadyResolved = new HashMap<String, DependentResourceNode>(orderedSpecs.size());
8181
for (DependentResourceSpec spec : orderedSpecs) {
82+
final var dependentResource = resolve(spec, client, configuration);
8283
final var node = new DependentResourceNode(
8384
spec.getReconcileCondition(),
8485
spec.getDeletePostCondition(),
8586
spec.getReadyCondition(),
8687
spec.getActivationCondition(),
87-
resolve(spec, client, configuration));
88-
alreadyResolved.put(node.getDependentResource().name(), node);
88+
dependentResource);
89+
alreadyResolved.put(dependentResource.name(), node);
8990
spec.getDependsOn()
9091
.forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend)));
9192
}
@@ -106,9 +107,9 @@ private <R> DependentResource<R, P> resolve(DependentResourceSpec<R, P> spec,
106107
configuration.getConfigurationService().dependentResourceFactory()
107108
.createFrom(spec, configuration);
108109

109-
if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET)
110-
&& dependentResource instanceof NameSetter) {
111-
((NameSetter) dependentResource).setName(spec.getName());
110+
final var name = spec.getName();
111+
if (name != null && !NO_VALUE_SET.equals(name) && dependentResource instanceof NameSetter) {
112+
((NameSetter) dependentResource).setName(name);
112113
}
113114

114115
if (dependentResource instanceof KubernetesClientAware) {

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
* @param <P> primary resource
2121
*/
2222
@SuppressWarnings("rawtypes")
23-
public class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
23+
class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
2424

2525
private final Map<String, DependentResourceNode> dependentResourceNodes;
2626
private final Set<DependentResourceNode> topLevelResources;
@@ -79,7 +79,7 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
7979
}
8080
map.put(node.getDependentResource().name(), node);
8181
}
82-
if (topLevelResources.size() == 0) {
82+
if (topLevelResources.isEmpty()) {
8383
throw new IllegalStateException(
8484
"No top-level dependent resources found. This might indicate a cyclic Set of DependentResourceNode has been provided.");
8585
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
99

1010
@SuppressWarnings("rawtypes")
11-
public class DependentResourceNode<R, P extends HasMetadata> {
11+
class DependentResourceNode<R, P extends HasMetadata> {
1212

1313
private final List<DependentResourceNode> dependsOn = new LinkedList<>();
1414
private final List<DependentResourceNode> parents = new LinkedList<>();
@@ -54,7 +54,6 @@ public Optional<Condition<R, P>> getReconcilePrecondition() {
5454
return Optional.ofNullable(reconcilePrecondition);
5555
}
5656

57-
5857
public Optional<Condition<R, P>> getDeletePostcondition() {
5958
return Optional.ofNullable(deletePostcondition);
6059
}
@@ -104,12 +103,6 @@ public int hashCode() {
104103
return this.getDependentResource().name().hashCode();
105104
}
106105

107-
@SuppressWarnings("rawtypes")
108-
static String getNameFor(DependentResource dependentResource) {
109-
return DependentResource.defaultNameFor(dependentResource.getClass()) + "#"
110-
+ dependentResource.hashCode();
111-
}
112-
113106
@Override
114107
public String toString() {
115108
return "DependentResourceNode{" + getDependentResource() + '}';

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)
6565

6666
DependentResourceNode getNodeByDependentResource(DependentResource<?, ?> dependentResource) {
6767
// first check by name
68-
final var node =
69-
dependentResourceNodes.get(dependentResource.name());
68+
final var node = dependentResourceNodes.get(dependentResource.name());
7069
if (node != null) {
7170
return node;
7271
} else {

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java

+1-10
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,7 @@
2626
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder;
2727
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
2828

29-
import static org.junit.jupiter.api.Assertions.assertEquals;
30-
import static org.junit.jupiter.api.Assertions.assertFalse;
31-
import static org.junit.jupiter.api.Assertions.assertNotNull;
32-
import static org.junit.jupiter.api.Assertions.assertNull;
33-
import static org.junit.jupiter.api.Assertions.assertTrue;
29+
import static org.junit.jupiter.api.Assertions.*;
3430

3531
class ControllerConfigurationOverriderTest {
3632
private final BaseConfigurationService configurationService = new BaseConfigurationService();
@@ -102,11 +98,6 @@ public Class<Object> resourceType() {
10298
return Object.class;
10399
}
104100

105-
@Override
106-
public String name() {
107-
return null;
108-
}
109-
110101
@Override
111102
public void configureWith(String config) {
112103
this.config = config;

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java

+1-9
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@
2424
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
2525

2626
import static io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolverTest.CustomAnnotationReconciler.DR_NAME;
27-
import static org.junit.jupiter.api.Assertions.assertEquals;
28-
import static org.junit.jupiter.api.Assertions.assertNotNull;
29-
import static org.junit.jupiter.api.Assertions.assertNull;
30-
import static org.junit.jupiter.api.Assertions.assertTrue;
27+
import static org.junit.jupiter.api.Assertions.*;
3128

3229
class DependentResourceConfigurationResolverTest {
3330

@@ -175,11 +172,6 @@ public Class<ConfigMap> resourceType() {
175172
return ConfigMap.class;
176173
}
177174

178-
@Override
179-
public String name() {
180-
return null;
181-
}
182-
183175
@Override
184176
public void configureWith(CustomConfig config) {
185177
this.config = config;

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ public class AbstractWorkflowExecutorTest {
3636
public class TestDependent extends KubernetesDependentResource<ConfigMap, TestCustomResource> {
3737

3838
public TestDependent(String name) {
39-
super(ConfigMap.class);
40-
setName(name);
41-
39+
super(ConfigMap.class, name);
4240
}
4341

4442
@Override
@@ -51,7 +49,7 @@ public ReconcileResult<ConfigMap> reconcile(TestCustomResource primary,
5149

5250
@Override
5351
public String toString() {
54-
return name;
52+
return name();
5553
}
5654
}
5755

@@ -91,7 +89,7 @@ public void delete(TestCustomResource primary, Context<TestCustomResource> conte
9189
}
9290

9391
public class TestErrorDependent implements DependentResource<String, TestCustomResource> {
94-
private String name;
92+
private final String name;
9593

9694
public TestErrorDependent(String name) {
9795
this.name = name;

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java

+1-12
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,7 @@
4242
import io.javaoperatorsdk.operator.processing.retry.RetryExecution;
4343
import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent;
4444

45-
import static org.junit.jupiter.api.Assertions.assertEquals;
46-
import static org.junit.jupiter.api.Assertions.assertFalse;
47-
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
48-
import static org.junit.jupiter.api.Assertions.assertNotNull;
49-
import static org.junit.jupiter.api.Assertions.assertNull;
50-
import static org.junit.jupiter.api.Assertions.assertThrows;
51-
import static org.junit.jupiter.api.Assertions.assertTrue;
45+
import static org.junit.jupiter.api.Assertions.*;
5246

5347
class BaseConfigurationServiceTest {
5448

@@ -470,11 +464,6 @@ public Class<ConfigMap> resourceType() {
470464
return ConfigMap.class;
471465
}
472466

473-
@Override
474-
public String name() {
475-
return null;
476-
}
477-
478467
@Override
479468
public void configureWith(CustomConfig config) {
480469
this.config = config;

0 commit comments

Comments
 (0)