feat: move name is directly to dependent resource

- use this name when throwing aggregate exception

Signed-off-by: Attila Mészáros <[email protected]>

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java

@@ -68,4 +68,8 @@ static String defaultNameFor(Class<? extends DependentResource> dependentResourc
   default boolean isDeletable() {
     return this instanceof Deleter;
   }
+
+  String getName();
+
+  void setName(String name);
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java

@@ -29,6 +29,8 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
   private ResourceDiscriminator<R, P> resourceDiscriminator;
   private final DependentResourceReconciler<R, P> dependentResourceReconciler;
 
+  protected String name = DependentResource.defaultNameFor(this.getClass());
+
   @SuppressWarnings({"unchecked"})
   protected AbstractDependentResource() {
     creator = creatable ? (Creator<R, P>) this : null;
@@ -176,4 +178,13 @@ protected boolean isUpdatable() {
   public boolean isDeletable() {
     return deletable;
   }
+
+  @Override
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java

@@ -139,11 +139,13 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
                 .eventSourceContextForDynamicRegistration());
         var es = eventSource.orElseThrow();
         context.eventSourceRetriever()
-            .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
+            .dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().getName(),
+                es);
 
       } else {
         context.eventSourceRetriever()
-            .dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
+            .dynamicallyDeRegisterEventSource(
+                dependentResourceNode.getDependentResource().getName());
       }
     }
   }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java

@@ -14,6 +14,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
 
+import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
 import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;
 
 @SuppressWarnings("rawtypes")
@@ -77,13 +78,13 @@ public Workflow<P> resolve(KubernetesClient client,
       ControllerConfiguration<P> configuration) {
     final var alreadyResolved = new HashMap<String, DependentResourceNode>(orderedSpecs.size());
     for (DependentResourceSpec spec : orderedSpecs) {
-      final var node = new DependentResourceNode(spec.getName(),
+      final var node = new DependentResourceNode(
           spec.getReconcileCondition(),
           spec.getDeletePostCondition(),
           spec.getReadyCondition(),
           spec.getActivationCondition(),
           resolve(spec, client, configuration));
-      alreadyResolved.put(node.getName(), node);
+      alreadyResolved.put(node.getDependentResource().getName(), node);
       spec.getDependsOn()
           .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend)));
     }
@@ -104,6 +105,10 @@ private <R> DependentResource<R, P> resolve(DependentResourceSpec<R, P> spec,
         configuration.getConfigurationService().dependentResourceFactory()
             .createFrom(spec, configuration);
 
+    if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET)) {
+      dependentResource.setName(spec.getName());
+    }
+
     if (dependentResource instanceof KubernetesClientAware) {
       ((KubernetesClientAware) dependentResource).setKubernetesClient(client);
     }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java

@@ -77,7 +77,7 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
           bottomLevelResource.remove(dependsOn);
         }
       }
-      map.put(node.getName(), node);
+      map.put(node.getDependentResource().getName(), node);
     }
     if (topLevelResources.size() == 0) {
       throw new IllegalStateException(

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java

@@ -12,25 +12,20 @@ public class DependentResourceNode<R, P extends HasMetadata> {
 
   private final List<DependentResourceNode> dependsOn = new LinkedList<>();
   private final List<DependentResourceNode> parents = new LinkedList<>();
-  private final String name;
+
   private Condition<R, P> reconcilePrecondition;
   private Condition<R, P> deletePostcondition;
   private Condition<R, P> readyPostcondition;
   private Condition<R, P> activationCondition;
   private final DependentResource<R, P> dependentResource;
 
-  DependentResourceNode(String name, DependentResource<R, P> dependentResource) {
-    this(name, null, null, null, null, dependentResource);
-  }
-
   DependentResourceNode(DependentResource<R, P> dependentResource) {
-    this(getNameFor(dependentResource), null, null, null, null, dependentResource);
+    this(null, null, null, null, dependentResource);
   }
 
-  public DependentResourceNode(String name, Condition<R, P> reconcilePrecondition,
+  public DependentResourceNode(Condition<R, P> reconcilePrecondition,
       Condition<R, P> deletePostcondition, Condition<R, P> readyPostcondition,
       Condition<R, P> activationCondition, DependentResource<R, P> dependentResource) {
-    this.name = name;
     this.reconcilePrecondition = reconcilePrecondition;
     this.deletePostcondition = deletePostcondition;
     this.readyPostcondition = readyPostcondition;
@@ -55,11 +50,6 @@ public List<DependentResourceNode> getParents() {
     return parents;
   }
 
-  public String getName() {
-    return name;
-  }
-
-
   public Optional<Condition<R, P>> getReconcilePrecondition() {
     return Optional.ofNullable(reconcilePrecondition);
   }
@@ -106,12 +96,12 @@ public boolean equals(Object o) {
       return false;
     }
     DependentResourceNode<?, ?> that = (DependentResourceNode<?, ?>) o;
-    return name.equals(that.name);
+    return this.getDependentResource().getName().equals(that.getDependentResource().getName());
   }
 
   @Override
   public int hashCode() {
-    return name.hashCode();
+    return this.getDependentResource().getName().hashCode();
   }
 
   @SuppressWarnings("rawtypes")

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java

@@ -21,14 +21,9 @@ public class WorkflowBuilder<P extends HasMetadata> {
   private boolean isCleaner = false;
 
   public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource) {
-    return addDependentResource(dependentResource, null);
-  }
-
-  public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource, String name) {
-    currentNode = name == null ? new DependentResourceNode<>(dependentResource)
-        : new DependentResourceNode<>(name, dependentResource);
+    currentNode = new DependentResourceNode<>(dependentResource);
     isCleaner = isCleaner || dependentResource.isDeletable();
-    final var actualName = currentNode.getName();
+    final var actualName = dependentResource.getName();
     dependentResourceNodes.put(actualName, currentNode);
     return this;
   }
@@ -71,7 +66,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)
   DependentResourceNode getNodeByDependentResource(DependentResource<?, ?> dependentResource) {
     // first check by name
     final var node =
-        dependentResourceNodes.get(DependentResourceNode.getNameFor(dependentResource));
+        dependentResourceNodes.get(dependentResource.getName());
     if (node != null) {
       return node;
     } else {

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java

@@ -38,7 +38,7 @@ public void throwAggregateExceptionIfErrorsPresent() {
     if (erroredDependentsExist()) {
       throw new AggregatedOperatorException("Exception(s) during workflow execution.",
           erroredDependents.entrySet().stream()
-              .collect(Collectors.toMap(e -> e.getKey().getClass().getName(), Entry::getValue)));
+              .collect(Collectors.toMap(e -> e.getKey().getName(), Entry::getValue)));
     }
   }
 }

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

@@ -103,6 +103,14 @@ public Class<Object> resourceType() {
         return Object.class;
       }
 
+      @Override
+      public String getName() {
+        return null;
+      }
+
+      @Override
+      public void setName(String name) {}
+
       @Override
       public void configureWith(String config) {
         this.config = config;

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java

@@ -175,6 +175,14 @@ public Class<ConfigMap> resourceType() {
       return ConfigMap.class;
     }
 
+    @Override
+    public String getName() {
+      return null;
+    }
+
+    @Override
+    public void setName(String name) {}
+
     @Override
     public void configureWith(CustomConfig config) {
       this.config = config;

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java

@@ -9,6 +9,8 @@
 public class EmptyTestDependentResource
     implements DependentResource<Deployment, TestCustomResource> {
 
+  private String name;
+
   @Override
   public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
       Context<TestCustomResource> context) {
@@ -19,5 +21,15 @@ public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
   public Class<Deployment> resourceType() {
     return Deployment.class;
   }
+
+  @Override
+  public String getName() {
+    return name;
+  }
+
+  @Override
+  public void setName(String name) {
+    this.name = name;
+  }
 }
 

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java

@@ -35,11 +35,10 @@ public class AbstractWorkflowExecutorTest {
 
   public class TestDependent extends KubernetesDependentResource<ConfigMap, TestCustomResource> {
 
-    private final String name;
-
     public TestDependent(String name) {
       super(ConfigMap.class);
-      this.name = name;
+      setName(name);
+
     }
 
     @Override
@@ -92,7 +91,7 @@ public void delete(TestCustomResource primary, Context<TestCustomResource> conte
   }
 
   public class TestErrorDependent implements DependentResource<String, TestCustomResource> {
-    private final String name;
+    private String name;
 
     public TestErrorDependent(String name) {
       this.name = name;
@@ -110,6 +109,16 @@ public Class<String> resourceType() {
       return String.class;
     }
 
+    @Override
+    public String getName() {
+      return name;
+    }
+
+    @Override
+    public void setName(String name) {
+      this.name = name;
+    }
+
     @Override
     public String toString() {
       return name;

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilderTest.java

@@ -13,8 +13,10 @@ class WorkflowBuilderTest {
   @Test
   void workflowIsCleanerIfAtLeastOneDRIsCleaner() {
     var dr = mock(DependentResource.class);
+    when(dr.getName()).thenReturn("dr");
     var deleter = mock(DependentResource.class);
     when(deleter.isDeletable()).thenReturn(true);
+    when(deleter.getName()).thenReturn("deleter");
 
     var workflow = new WorkflowBuilder<TestCustomResource>()
         .addDependentResource(deleter)

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java

@@ -538,7 +538,7 @@ void reconcilePreconditionNotCheckedOnNonActiveDependent() {
   @Test
   void deletesDependentsOfNonActiveDependentButNotTheNonActive() {
     TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2");
-    TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_2");
+    TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3");
 
     var workflow = new WorkflowBuilder<TestCustomResource>()
         .addDependentResource(dr1).withActivationCondition(notMetCondition)

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java

@@ -17,8 +17,7 @@
 import static org.junit.Assert.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.withSettings;
+import static org.mockito.Mockito.*;
 
 @SuppressWarnings("rawtypes")
 class WorkflowTest {
@@ -27,9 +26,9 @@ class WorkflowTest {
 
   @Test
   void zeroTopLevelDRShouldThrowException() {
-    var dr1 = mock(DependentResource.class);
-    var dr2 = mock(DependentResource.class);
-    var dr3 = mock(DependentResource.class);
+    var dr1 = mockDependent("dr1");
+    var dr2 = mockDependent("dr2");
+    var dr3 = mockDependent("dr3");
 
     var cyclicWorkflowBuilderSetup = new WorkflowBuilder<TestCustomResource>()
         .addDependentResource(dr1).dependsOn()
@@ -43,9 +42,9 @@ void zeroTopLevelDRShouldThrowException() {
 
   @Test
   void calculatesTopLevelResources() {
-    var dr1 = mock(DependentResource.class);
-    var dr2 = mock(DependentResource.class);
-    var independentDR = mock(DependentResource.class);
+    var dr1 = mockDependent("dr1");
+    var dr2 = mockDependent("dr2");
+    var independentDR = mockDependent("independentDR");
 
     var workflow = new WorkflowBuilder<TestCustomResource>()
         .addDependentResource(independentDR)
@@ -63,9 +62,9 @@ void calculatesTopLevelResources() {
 
   @Test
   void calculatesBottomLevelResources() {
-    var dr1 = mock(DependentResource.class);
-    var dr2 = mock(DependentResource.class);
-    var independentDR = mock(DependentResource.class);
+    var dr1 = mockDependent("dr1");
+    var dr2 = mockDependent("dr2");
+    var independentDR = mockDependent("independentDR");
 
     Workflow<TestCustomResource> workflow = new WorkflowBuilder<TestCustomResource>()
         .addDependentResource(independentDR)
@@ -100,4 +99,11 @@ void isDeletableShouldWork() {
         GarbageCollected.class));
     assertFalse(DefaultWorkflow.isDeletable(dr.getClass()));
   }
+
+  static DependentResource mockDependent(String name) {
+    var res = mock(DependentResource.class);
+    when(res.getName()).thenReturn(name);
+    return res;
+  }
+
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java

@@ -470,6 +470,14 @@ public Class<ConfigMap> resourceType() {
       return ConfigMap.class;
     }
 
+    @Override
+    public String getName() {
+      return null;
+    }
+
+    @Override
+    public void setName(String name) {}
+
     @Override
     public void configureWith(CustomConfig config) {
       this.config = config;