From 038bfba62f3d3e04f62b41c817f0f01a451a64d6 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 4 Jul 2022 18:16:32 +0200 Subject: [PATCH] refactor: DependentResourceSpec shouldn't expose setters --- .../AnnotationControllerConfiguration.java | 42 +++++++----------- .../ControllerConfigurationOverrider.java | 10 ++--- .../dependent/DependentResourceSpec.java | 44 +++++++------------ .../workflow/ManagedWorkflowTestUtils.java | 6 +-- 4 files changed, 38 insertions(+), 64 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 560a79e5be..367b53b0b7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -237,9 +237,11 @@ public List getDependentResources() { throw new IllegalArgumentException( "A DependentResource named: " + name + " already exists: " + spec); } - spec = new DependentResourceSpec(dependentType, config, name); - spec.setDependsOn(Set.of(dependent.dependsOn())); - addConditions(spec, dependent); + spec = new DependentResourceSpec(dependentType, config, name, + Set.of(dependent.dependsOn()), + instantiateConditionIfNotVoid(dependent.readyPostcondition()), + instantiateConditionIfNotVoid(dependent.reconcilePrecondition()), + instantiateConditionIfNotVoid(dependent.deletePostcondition())); specsMap.put(name, spec); } @@ -248,28 +250,18 @@ public List getDependentResources() { return specs; } - @SuppressWarnings("unchecked") - private void addConditions(DependentResourceSpec spec, Dependent dependent) { - if (dependent.deletePostcondition() != VoidCondition.class) { - spec.setDeletePostCondition(instantiateCondition(dependent.deletePostcondition())); - } - if (dependent.readyPostcondition() != VoidCondition.class) { - spec.setReadyPostcondition(instantiateCondition(dependent.readyPostcondition())); - } - if (dependent.reconcilePrecondition() != VoidCondition.class) { - spec.setReconcilePrecondition(instantiateCondition(dependent.reconcilePrecondition())); - } - } - - private Condition instantiateCondition(Class condition) { - try { - return condition.getDeclaredConstructor().newInstance(); - } catch (InstantiationException - | IllegalAccessException - | InvocationTargetException - | NoSuchMethodException e) { - throw new OperatorException(e); + private Condition instantiateConditionIfNotVoid(Class condition) { + if (condition != VoidCondition.class) { + try { + return condition.getDeclaredConstructor().newInstance(); + } catch (InstantiationException + | IllegalAccessException + | InvocationTargetException + | NoSuchMethodException e) { + throw new OperatorException(e); + } } + return null; } private String getName(Dependent dependent, Class dependentType) { @@ -280,7 +272,7 @@ private String getName(Dependent dependent, Class d return name; } - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "unchecked"}) private Object createKubernetesResourceConfig(Class dependentType) { Object config; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 440a076428..4936b6bc02 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -211,13 +211,9 @@ public ControllerConfiguration build() { @SuppressWarnings({"rawtypes", "unchecked"}) private DependentResourceSpec updateSpec(String name, DependentResourceSpec spec, KubernetesDependentResourceConfig c) { - var res = new DependentResourceSpec(spec.getDependentResourceClass(), - c.setNamespaces(namespaces), name); - res.setReadyPostcondition(spec.getReadyCondition()); - res.setReconcilePrecondition(spec.getReconcileCondition()); - res.setDeletePostCondition(spec.getDeletePostCondition()); - res.setDependsOn(spec.getDependsOn()); - return res; + return new DependentResourceSpec(spec.getDependentResourceClass(), + c.setNamespaces(namespaces), name, spec.getDependsOn(), spec.getReadyCondition(), + spec.getReconcileCondition(), spec.getDeletePostCondition()); } public static ControllerConfigurationOverrider override( diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java index 2ef9e58de4..2f22dd5a96 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java @@ -1,6 +1,6 @@ package io.javaoperatorsdk.operator.api.config.dependent; -import java.util.HashSet; +import java.util.Collections; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -16,19 +16,30 @@ public class DependentResourceSpec, C> { private final String name; - private Set dependsOn; + private final Set dependsOn; - private Condition readyCondition; + private final Condition readyCondition; - private Condition reconcileCondition; + private final Condition reconcileCondition; - private Condition deletePostCondition; + private final Condition deletePostCondition; public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig, String name) { + this(dependentResourceClass, dependentResourceConfig, name, Collections.emptySet(), null, null, + null); + } + + public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig, + String name, Set dependsOn, Condition readyCondition, + Condition reconcileCondition, Condition deletePostCondition) { this.dependentResourceClass = dependentResourceClass; this.dependentResourceConfig = dependentResourceConfig; this.name = name; + this.dependsOn = dependsOn; + this.readyCondition = readyCondition; + this.reconcileCondition = reconcileCondition; + this.deletePostCondition = deletePostCondition; } public Class getDependentResourceClass() { @@ -68,44 +79,21 @@ public int hashCode() { } public Set getDependsOn() { - if (dependsOn == null) { - dependsOn = new HashSet<>(0); - } return dependsOn; } - public DependentResourceSpec setDependsOn(Set dependsOn) { - this.dependsOn = dependsOn; - return this; - } - @SuppressWarnings("rawtypes") public Condition getReadyCondition() { return readyCondition; } - public DependentResourceSpec setReadyPostcondition(Condition readyCondition) { - this.readyCondition = readyCondition; - return this; - } - @SuppressWarnings("rawtypes") public Condition getReconcileCondition() { return reconcileCondition; } - public DependentResourceSpec setReconcilePrecondition(Condition reconcileCondition) { - this.reconcileCondition = reconcileCondition; - return this; - } - @SuppressWarnings("rawtypes") public Condition getDeletePostCondition() { return deletePostCondition; } - - public DependentResourceSpec setDeletePostCondition(Condition deletePostCondition) { - this.deletePostCondition = deletePostCondition; - return this; - } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java index 85d57db7a0..2332778e2e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java @@ -10,10 +10,8 @@ public class ManagedWorkflowTestUtils { @SuppressWarnings("unchecked") public static DependentResourceSpec createDRS(String name, String... dependOns) { - final var spec = new DependentResourceSpec(EmptyTestDependentResource.class, - null, name); - spec.setDependsOn(Set.of(dependOns)); - return spec; + return new DependentResourceSpec(EmptyTestDependentResource.class, + null, name, Set.of(dependOns), null, null, null); } }