Add new DgsDataFetchingEnvironment.isArgumentSet

[paulbakker]

Utility method to check if an argument is explicitly set, usable for things like sparse updates.

Pull Request type

• Bugfix
• Feature
• Refactoring (no functional changes, no api changes)
• Build related changes
• Other (please describe):

Changes in this PR

Added an instance method isArgumentSet to DgsDataFetchingEnvironment. Given a path to an argument, it returns a boolean indicating if this argument was explicitly set.
This is important mostly for sparse update mutations where an explicitly provided null value needs to be handled differently from an argument that wasn't provided at all.
The following datafetching methods are examples of use:

/**
 * Check if a top level argument was provided explicitly.
 * An explicit null value should also return true.
 */
@DgsMutation
fun inputTester(
    @InputArgument topLevelArg: String?,
    dfe: DgsDataFetchingEnvironment,
): Boolean = dfe.isArgumentSet("topLevelArg")

/**
 * Check properties of complex input type if they were provided explicitly.
 * An explicit null value should also return true.
 * This example uses var args for the property path (e.g. isArgumentSet("personInput", "address", "city"))
 */
@DgsMutation
fun complexInputTester(
    @InputArgument personInput: PersonInput?,
    dfe: DgsDataFetchingEnvironment,
): InputTestResult {
    val nameIsSet = dfe.isArgumentSet("personInput", "name")
    val cityIsSet = dfe.isArgumentSet("personInput", "address", "city")

    return InputTestResult(nameIsSet, cityIsSet)
}

/**
 * Check properties of complex input type if they were provided explicitly.
 * An explicit null value should also return true.
 * This example uses the . and -> delimiters for the property paths (e.g. isArgumentSet(personInput->address->city))
 */
@DgsMutation
fun complexInputTesterWithDelimiter(
    @InputArgument personInput: PersonInput?,
    dfe: DgsDataFetchingEnvironment,
): InputTestResult {
    val nameIsSet = dfe.isArgumentSet("personInput.name")
    val cityIsSet = dfe.isArgumentSet("personInput->address -> city")

    return InputTestResult(nameIsSet, cityIsSet)
}

Alternatives considered

This is an alternative solution to changing generated input types in CodeGen. While this solution requires arguments to be referenced by name (as a String/constant), it doesn't require all generated types to be aware of explicit setting.
This solution also works when codegen is not used, e.g. when Records are used for input types.

Another alternative is to use @ArgumentValue from Spring GraphQL. This doesn't automatically account for nested input arguments automatically, but could be a nice solution when types are manually created, or for top level arguments only. Once we remove the legacy DGS implementation later this year, we'll add support for @ArgumentValue as an additional option.

[srinivasankavitha]

Do we need to support 2 ways of specifying the path?

[paulbakker]

I mentioned earlier that I just couldn't decide which one is nicer, but there's a reason to support both.
When just writing the path manually, the "bla->other" approach looks more concise, and is the nicer opinion. That's what I would personally use.
Some folks don't like strings that aren't compile time checked, and would prefer using constants. Codegen already generates those, so you could write something like:

isArgumentSet(DgsConstants.MUTATION.RECORD_INPUT_ARGUMENT.StartupEvent, DgsConstants.STARTUPEVENTINPUT.ApplicationName)

In that case the varargs are nicer because you don't need to do a string concat just to create the argument.

[paulbakker]

Somewhat related; what I really wanted is some sort of method reference, but I don't think we can get that to work with Java. E.g. something like what you would do with a lambda:

isArgumentSet(personInput::getName)

[srinivasankavitha]

looks great to me! Thanks for adding this.

[luispollo]

Hi @paulbakker and @srinivasankavitha! Any thoughts on whether this might create the wrong incentives from a separation of concerns perspective, since I'd expect this kind of check to be in the data access layer when deciding whether to update a field, as opposed to at the API layer as shown here? In other words, any guidance on how you might pass that context down into lower layers of the application that would have a use for it?

[paulbakker]

@luispollo I think this is the correct layer for checking this. The GraphQL layer (data fetchers) should check input given by the user, this can be part of that.
How that information is then used depends on the application. Some applications will use something like a Spring Data Repository directly in the data fetcher, in other cases there might be a separate layer to forward to.
If forwarded through layers, you definitely shouldn't use the data fetching environment in these layers, because that's a GraphQL only concern. In that case you might need to account for this in your own types defined by your own APIs, which likely shouldn't use the GraphQL generated types directly.

[luispollo]

Thanks @paulbakker!

How that information is then used depends on the application. Some applications will use something like a Spring Data Repository directly in the data fetcher, in other cases there might be a separate layer to forward to.

Let me share some more context. We're using the DGS codegen gradle plugin, which means in the GraphQL layer, those are the types we use for input objects. The issue here is that you can't distinguish between a null value being provided in the input vs. not being provided at all because both cases end up causing a null value to be assigned to a nullable object property. This is true both at this layer and any layer below because that context is lost, regardless of whether you use the generated types or your own types (the latter of which is the approach my team uses to avoid leakage across layers). With this PR, we now have the ability to make that distinction, but only at the GraphQL API layer. Let's say we have a repository-type component wired into the data fetcher, which is a pattern we use. How would you propose we pass the context that a specific field, or set of fields, were explicitly set to null in the request down to that repository object in a way that's (1) not leaking GraphQL concerns into the data layer and (2) not super cumbersome/bespoke? Any sample implementations?

Thanks for engaging! 🙏