Rename NamePolicy and add docs

Signed-off-by: Nelson Arapé <[email protected]>

flytekit-java/src/main/java/org/flyte/flytekit/NodeNamePolicy.java → flytekit-java/src/main/java/org/flyte/flytekit/SdkNodeNamePolicy.java

@@ -17,28 +17,54 @@
 package org.flyte.flytekit;
 
 import java.util.Locale;
+import java.util.Map;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.regex.Pattern;
 
-class NodeNamePolicy {
+/**
+ * Controls the default node id and node name policy when applying {@link SdkTransform} to {@link
+ * SdkWorkflowBuilder}. When using {@link SdkWorkflowBuilder#apply(SdkTransform)} or {@link
+ * SdkWorkflowBuilder#apply(SdkTransform, Map)} then the node id used would be the one returned by
+ * {@link #nextNodeId()}. Also, if a node name haven't been set by the user, then {@link
+ * #toNodeName(String)} would be used.
+ */
+class SdkNodeNamePolicy {
   private static final Pattern UPPER_AFTER_LOWER_PATTERN = Pattern.compile("([a-z])([A-Z]+)");
   private static final int RND_PREFIX_SIZE = 4;
 
   private final String nodeIdPrefix;
   private final AtomicInteger nodeIdSuffix;
 
-  NodeNamePolicy() {
+  SdkNodeNamePolicy() {
     this.nodeIdPrefix = randomPrefix();
     this.nodeIdSuffix = new AtomicInteger();
   }
 
-  // Returns nodes in the format "wqjoz-n1"
+  /**
+   * Returns a unique node ids in the format {@code <prefix>-n<consecutive-number>}, where prefix is
+   * a random, but shared among all ids for this object, set of character in the format {@code
+   * wRRRR} and {@code R} is a random letter in {@code a-z} range.
+   *
+   * @return next unique node id for this policy.
+   */
   String nextNodeId() {
     return nodeIdPrefix + "n" + nodeIdSuffix.getAndIncrement();
   }
 
-  // Translates "FooBar" to "foo-bar"
+  /**
+   * Returns a node appropriate name for a given transformation name. The transformation done are
+   *
+   * <ul>
+   *   <li>Package name is removed
+   *   <li>CamelCase is transformed to kebab-case
+   *   <li>$ is transformed to -
+   * </ul>
+   *
+   * For example {@code com.example.Outer$InnerTask} get translated to {@code outer-inner-task}.
+   *
+   * @return node name.
+   */
   String toNodeName(String name) {
     String lastPart = name.substring(name.lastIndexOf('.') + 1);
     return UPPER_AFTER_LOWER_PATTERN

flytekit-java/src/main/java/org/flyte/flytekit/SdkWorkflowBuilder.java

@@ -40,14 +40,14 @@ public class SdkWorkflowBuilder {
   private final Map<String, SdkBindingData> outputs;
   private final Map<String, String> inputDescriptions;
   private final Map<String, String> outputDescriptions;
-  private final NodeNamePolicy nodeNamePolicy;
+  private final SdkNodeNamePolicy sdkNodeNamePolicy;
 
   public SdkWorkflowBuilder() {
-    this(new NodeNamePolicy());
+    this(new SdkNodeNamePolicy());
   }
 
   // VisibleForTesting
-  SdkWorkflowBuilder(NodeNamePolicy nodeNamePolicy) {
+  SdkWorkflowBuilder(SdkNodeNamePolicy sdkNodeNamePolicy) {
     // Using LinkedHashMap to preserve declaration order
     this.nodes = new LinkedHashMap<>();
     this.inputs = new LinkedHashMap<>();
@@ -56,7 +56,7 @@ public SdkWorkflowBuilder() {
     this.inputDescriptions = new HashMap<>();
     this.outputDescriptions = new HashMap<>();
 
-    this.nodeNamePolicy = nodeNamePolicy;
+    this.sdkNodeNamePolicy = sdkNodeNamePolicy;
   }
 
   public SdkNode apply(String nodeId, SdkTransform transform) {
@@ -81,7 +81,7 @@ SdkNode applyInternal(
       List<String> upstreamNodeIds,
       Map<String, SdkBindingData> inputs) {
 
-    String actualNodeId = Objects.requireNonNullElseGet(nodeId, nodeNamePolicy::nextNodeId);
+    String actualNodeId = Objects.requireNonNullElseGet(nodeId, sdkNodeNamePolicy::nextNodeId);
 
     if (nodes.containsKey(actualNodeId)) {
       CompilerError error =
@@ -94,7 +94,8 @@ SdkNode applyInternal(
     }
 
     String fallbackNodeName =
-        Objects.requireNonNullElseGet(nodeId, () -> nodeNamePolicy.toNodeName(transform.getName()));
+        Objects.requireNonNullElseGet(
+            nodeId, () -> sdkNodeNamePolicy.toNodeName(transform.getName()));
 
     SdkNode sdkNode =
         transform

flytekit-java/src/test/java/org/flyte/flytekit/NodeNamePolicyTest.java → flytekit-java/src/test/java/org/flyte/flytekit/SdkNodeNamePolicyTest.java

@@ -30,16 +30,16 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
-class NodeNamePolicyTest {
+class SdkNodeNamePolicyTest {
 
   @Test
   void nextNodeIdShouldConformToFormat() {
-    assertThat(new NodeNamePolicy().nextNodeId(), matchesRegex("w[a-z]{4}-n\\d+"));
+    assertThat(new SdkNodeNamePolicy().nextNodeId(), matchesRegex("w[a-z]{4}-n\\d+"));
   }
 
   @Test
   void nextNodeIdShouldReturnUniqueIds() {
-    NodeNamePolicy policy = new NodeNamePolicy();
+    SdkNodeNamePolicy policy = new SdkNodeNamePolicy();
 
     var ids = IntStream.range(0, 100).mapToObj(__ -> policy.nextNodeId()).collect(toList());
     var uniqueIds = Set.copyOf(ids);
@@ -49,7 +49,7 @@ void nextNodeIdShouldReturnUniqueIds() {
 
   @Test
   void nextNodeIdShouldReturnSamePrefixForSamePolicy() {
-    NodeNamePolicy policy = new NodeNamePolicy();
+    SdkNodeNamePolicy policy = new SdkNodeNamePolicy();
 
     var ids = IntStream.range(0, 100).mapToObj(__ -> policy.nextNodeId()).collect(toList());
 
@@ -62,8 +62,8 @@ void nextNodeIdShouldReturnSamePrefixForSamePolicy() {
   void nextNodeIdShouldReturnUniquePrefixForDistinctPolicies() {
     var prefixes =
         IntStream.range(0, 100)
-            .mapToObj(__ -> new NodeNamePolicy())
-            .map(NodeNamePolicy::nextNodeId)
+            .mapToObj(__ -> new SdkNodeNamePolicy())
+            .map(SdkNodeNamePolicy::nextNodeId)
             .map(id -> id.substring(0, id.indexOf('-')))
             .collect(toList());
     var uniquePrefixes = Set.copyOf(prefixes);
@@ -80,7 +80,7 @@ void nextNodeIdShouldReturnUniquePrefixForDistinctPolicies() {
     "com.example.FooBar,foo-bar"
   })
   void toNodeName(String taskName, String expectedNodeName) {
-    NodeNamePolicy policy = new NodeNamePolicy();
+    SdkNodeNamePolicy policy = new SdkNodeNamePolicy();
 
     assertThat(policy.toNodeName(taskName), equalTo(expectedNodeName));
   }

flytekit-java/src/test/java/org/flyte/flytekit/SdkWorkflowBuilderTest.java

@@ -62,14 +62,14 @@
 @ExtendWith(MockitoExtension.class)
 class SdkWorkflowBuilderTest {
 
-  @Mock NodeNamePolicy nodeNamePolicy;
+  @Mock SdkNodeNamePolicy sdkNodeNamePolicy;
 
   @Test
   void testTimes4WorkflowIdl() {
-    when(nodeNamePolicy.nextNodeId()).thenReturn("foo-n0", "foo-n1");
-    when(nodeNamePolicy.toNodeName(any())).thenReturn("multiplication-task");
+    when(sdkNodeNamePolicy.nextNodeId()).thenReturn("foo-n0", "foo-n1");
+    when(sdkNodeNamePolicy.toNodeName(any())).thenReturn("multiplication-task");
 
-    SdkWorkflowBuilder builder = new SdkWorkflowBuilder(nodeNamePolicy);
+    SdkWorkflowBuilder builder = new SdkWorkflowBuilder(sdkNodeNamePolicy);
 
     new Times4Workflow().expand(builder);