k8s.contrib.crd - Fix class name collision for colliding nested properties.

[ichekrygin]

Fix colliding class names when rendering CRD with duplicated nested properties.
With this PR, the generated *pkl file changes the file naming convention for "spec.compute.field":

• from: classes ["Spec", "Compute", "Field"]
• to: classes ["Spec", "SpecCompute", "SpecComputeField"]

This change was suggested in #40 (comment)

Closes: #40

Currently, cards.yaml does not handle CRDs containing duplicated nested objects/lists resulting in the pkl file/module with duplicate class names. For example, if input CRD contains duplicated nested properties as in snipped below:

          properties:
            spec:
              description: Represents the configuration of a Restate Cluster
              properties:
                compute:
                  description: Compute configuration
                  properties:
                    ...
                    redObject:
                      description: Test nested objects field collision.
                      properties:
                        nestedField:
                          description: Nested field.
                          type: string
                        nestedObject:
                          description: Nested child object.
                          properties:
                            nestedRed:
                              description: Nested field.
                              type: string
                          type: object
                        nestedList:
                          description: Nested list object red.
                          items:
                            description: Red nested object test items.
                            properties:
                              nestedListItemField:
                                description: Red nested list field.
                                type: string
                            type: object
                          nullable: true
                          type: array
                      required:
                        - nestedField
                      type: object
                    blueObject:
                      description: Test nested objects field collision.
                      properties:
                        nestedField:
                          description: Nested field.
                          type: string
                        nestedObject:
                          description: Nested child object.
                          properties:
                            nestedBlue:
                              description: Nested field.
                              type: string
                          type: object
                        nestedList:
                          description: Nested list object blue.
                          items:
                            description: Blue nested object test items.
                            properties:
                              nestedListItemField:
                                description: Blue nested list field.
                                type: string
                            type: object
                          nullable: true
                          type: array
                      required:
                        - nestedField
                      type: object
                  required:
                    - image
                  type: object

We can observe colliding class definitions results in generated pkl file, such as:

...
    /// Nested child object.
    class NestedObject {
      /// Nested field.
      nestedRed: String?
    }
...
    /// Nested child object.
    class NestedObject {
      /// Nested field.
      nestedBlue: String?
    }

Similarly, with list types:

...
    /// Red nested object test items.
    class NestedList {
      /// Red nested list field.
      nestedListItemField: String?
    }
...
    /// Blue nested object test items.
    class NestedList {
      /// Blue nested list field.
      nestedListItemField: String?
    }

[bioball]

Thanks for looking into this!

The issue here is really that there's a bug in the conflict detection logic; the fix is:

diff --git a/packages/k8s.contrib.crd/internal/ModuleGenerator.pkl b/packages/k8s.contrib.crd/internal/ModuleGenerator.pkl
index 4662bfb..87da6b7 100644
--- a/packages/k8s.contrib.crd/internal/ModuleGenerator.pkl
+++ b/packages/k8s.contrib.crd/internal/ModuleGenerator.pkl
@@ -281,18 +281,19 @@ function isClassLike(schema: JsonSchema.Schema): Boolean =
 /// Try to use the parent property's name as part of the class name in case of conflict.
 /// If already at the root, add a number at the end.
 local function determineTypeName(path: List<String>, candidateName: String, existingTypeNames: Set<Type>, index: Int): Type =
-  if (existingTypeNames.contains(utils.pascalCase(candidateName)))
-    if (path.isEmpty)
-      determineTypeName(path, candidateName + index.toString(), existingTypeNames, index + 1)
+  let (candidate = utils.pascalCase(candidateName))
+    if (existingTypeNames.findOrNull((it) -> it.name == candidate) != null)
+      if (path.isEmpty)
+        determineTypeName(path, candidateName + index.toString(), existingTypeNames, index + 1)
+      else
+        determineTypeName(
+          path.dropLast(1),
+          utils.pascalCase(path.last.capitalize()) + candidate,
+          existingTypeNames,
+          index
+        )
     else
-      determineTypeName(
-        path.dropLast(1),
-        utils.pascalCase(path.last.capitalize()) + utils.pascalCase(candidateName),
-        existingTypeNames,
-        index
-      )
-  else
-    new { name = utils.pascalCase(candidateName); moduleName = module.moduleName }
+      new { name = candidate; moduleName = module.moduleName }
 
 /// The schemas that should be rendered as classes.
 ///

Can you apply this diff, undo your current change? It'd be good to not incur a breaking change when we don't really need it.

[ichekrygin]

@bioball, (edited, after re-reading your comment).
What is the expected outcome of this diff?

[ichekrygin]

With the diff, i see the following:

...
    /// Nested child object.
    class NestedObject {
      /// Nested field.
      nestedRed: String?
    }
...
    /// Nested child object.
    class NestedObjectNestedObject { <-- is that what expected?
      /// Nested field.
      nestedBlue: String?
    }

[ichekrygin]

I suspect the intent was:

determineTypeName(path, utils.pascalCase(path.dropLast(1).last.capitalize()) + candidate, existingTypeNames, index)

Instead of:

determineTypeName(path.dropLast(1), utils.pascalCase(path.last.capitalize()) + candidate, existingTypeNames, index)

Which results in:

 /// Nested child object.
    class BlueObjectNestedObject {
      /// Nested field.
      nestedBlue: String?
    }

[ichekrygin]

@bioball , any updates on this?

[bioball]

Yeah, your suggestion looks right. Happy to accept that change!

Apologies on the late response here!