Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling of field nullability #115

Open
PeterJCLaw opened this issue Jan 15, 2025 · 2 comments
Open

Handling of field nullability #115

PeterJCLaw opened this issue Jan 15, 2025 · 2 comments

Comments

@PeterJCLaw
Copy link

PeterJCLaw commented Jan 15, 2025

Consider the following psuedo-schema:

from typing import TypedDict, NotRequired, Union

class NullabilityTesting(TypedDict):
    optional_non_null_int: NotRequired[int]
    required_non_null_int: int
    optional_nullable_int: NotRequired[Union[int | None]]
    required_nullable_int: Union[int | None]
equivalent formal schema
{
    "name": "nullability-testing",
    "eventType": "track",
    "identitySection": "properties",
    "rules": {
        "$schema": "http://json-schema.org/draft-07/schema#",
        "type": "object",
        "properties": {
            "properties": {
                "type": "object",
                "additionalProperties": false,
                "required": [
                    "required_non_null_int",
                    "required_nullable_int"
                ],
                "properties": {
                    "optional_non_null_int": {
                        "type": ["integer"]
                    },
                    "required_non_null_int": {
                        "type": ["integer"]
                    },
                    "optional_nullable_int": {
                        "type": ["integer", "null"]
                    },
                    "required_nullable_int": {
                        "type": ["integer", "null"]
                    }
                }
            }
        }
    }
}

This expresses the four possible combinations of required-ness (i.e: does the field need to be included in the payload) and null-ness (i.e: can the value be null if provided).

It's really pleasing that RudderStack's type system supports all four combinations here -- both in the schemas and for runtime validation.

However it appears that the code generated by RudderTyper is somewhat less able to correctly represent all four quadrants. Given the intended use of generated code to enforce typing at the client, shifting potential run-time errors to being compile-time errors, it would be great to have consistent support for accurately generating the proper types to match each case.

I appreciate that not all target languages support this fully, however even within those constraints I believe there are errors in the generated code which will lead to one or both of:

  • client (Source) developers missing errors which will fail at runtime (assuming that Tracking Plan validation is enforced)
  • destination developers receiving events which contain nulls which are technically valid but not what was intended by the client developers or represent unexpectedly missing data

Specifics, though I am unfortunately not an expert in any of these languages:

  • Java:
    • optionalNonNullInt is annotated @Nullable (I'm not familiar with the android annotations, however given the presence of a NonNull annotation used elsewhere this appears to be a bug)
    • The presence of required_nullable_int is not checked for in the build method
    • Both of these would lead to the client believe that a payload is fine, yet it being rejected at runtime by the data-plane
  • Swift:
    • optionalNonNullInt appears to be marked as nullable and has a default set in the function call creating the event payload, meaning that nil may be passed to a field which is not nullable (potentially leading to rejection by the data-plane)
    • requiredNullableInt has a default value set in the function call creating the event payload (and the resulting value is always used), removing the required-ness from the generated API (potentially leading to missing data which ought to be being provided)
  • TypeScript appears probably correct from reading the code, though I haven't tested what it actually produces

Ideally here, RudderTyper would preserve both the required-ness and the null-ness of values throughout -- both in terms of the type system (up to the limits of the language) and the runtime checking (at the level expected for the platform). This is needed to ensure that the expected semantics as defined in the schemas are correctly conveyed to implementers.

@contributor-support
Copy link

Thanks for opening this issue! We'll get back to you shortly. If it is a bug, please make sure to add steps to reproduce the issue.

@akashrpo
Copy link

Hi Peter,

For Java, we will release the changes to correctly handle nullability based on the criteria you mentioned. However, since Java follows a builder pattern, the validation for required fields will still happen at runtime.

For Swift, the current behavior is due to how Swift handles optionals—optional properties default to nil when not assigned. The correct approach is to rely on Swift’s default behavior for optionals while ensuring required fields are properly enforced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants