fix(dataproducer): Populate context default values before resolving

[klausi]

Fix for #1233.

It is a bit unfortunate that Drupal core does not provide a good API function to get context values with default values populated, but not a big problem. We can implement that ourselves.

Now we could also update a lot of resolve() function signatures to remove the ? null type parameter, because now default values will always be passed.

Should we do it as part of this change or do we consider this an API break?

[klausi]

That is a good point - if you have defined your default values wrong and they don't match the types on your resolver then there can be fatal errors.

We use quite a few dataproducers in Jobiqo and I did not see any problem in our automated testing, so probably this is a rare problem.

So I would risk it and push this to 4.x, with a warning in the release notes.

What do you think about changing the function signature of various dataproducer's resolve() methods? With this change we can remove a couple of ? operators of some types. I did not do that yet in this PR. I would consider the resolve() methods not part of the API surface of the graphql module, so we could risk that as well.

[Kingdutch]

I think in general I'm a bit more conservative with these kinds of changes. With all the modules we update in Open Social these kinds of subtle changes can add significant overhead which we tend to pay attention to with major updates, but with minor updates we generally start from the "I expect this still works the same and update mostly blindly" principle.

What do you think would be the impact of making this an opt-in flag in a minor version with a deprecation notice? Then we remove the flag in the next major version and default to the new behaviour?

if (Settings::get('graphql_context_defaults')) {
  $context = $this->getContextValuesWithDefaults();
}
else {
  @trigger_error("Not using defaults from context is deprecated in GraphQL 4.x and this behaviour will change in 5.0. Opt-in to the new behaviour by adding `$settings['graphql_context_defaults'] = TRUE;` to your settings.php file");
  $context = $this->getContextValues();
}

(above code is illustrative and not tested)

[klausi]

I'm not a fan of the setting as it makes it more complicated and harder to maintain, but it could work. We could have an update function for old installations that sets the config to false, then at least all new installations will get the new default.

I will check this approach.

[klausi]

Alright, this seems to work now. Can you review again and let me know if this is what you had in mind? Thanks!