Husqvik
Generating mutation models with ICollection instead of QueryBuilderParamter<ICollection<>>
Is there a way to configure the generation process so that it will not add the QueryBuilderParameter code?
So instead of this:
[JsonConverter(typeof (QueryBuilderParameterConverter<ICollection<BorrowerInput>>))]
public QueryBuilderParameter<ICollection<BorrowerInput>?>? Borrowers
{
get => (QueryBuilderParameter<ICollection<BorrowerInput>>) this._borrowers.Value;
set
{
this._borrowers = new InputPropertyInfo()
{
Name = "borrowers",
Value = (object) value
};
}
}
It would simply generate this:
ICollection<BorrowerInput>?>? Borrowers
For more context:
The requirement in my organization is to provide consumers of our API a GraphQL api client that has c# models included and allows them to pass in a string query and a variable collection and get a strongly typed response.
for instance
var query = "....query text here...."
var response = await client.QueryGraph(query, variables);
I'm using this library to generate the models included in the api client, and graphql-client library for the actual query/mutation logic.
It works great, but one of our consumers is trying to map their models to the mutation models generated by this library and they're having issues because on their end they have a ICollection
Hopefully that makes sense...
Thanks,
Interesting. I expected more issue if some one wants/needs to use System.Text.Json serializer as the input objects don't have support for it.
Is the issue related only to ICollection<>? Or it's something else. And even if only ICollection<> is still don't under what is the issue exactly. Implicit conversion?
So the use case is this:
We're generating a GraphQL Api client in our ci/cd pipeline for others in the org to use to consume our graph. I'm using this lib to generate the models.
The consumers of the api client in their code call the query logic in the api client and get back the models generated from this lib that are included in the api client. They use Automapper to map those models to their own domain's models. For queries, this works great because the generated query models look like this as an example:
public class MilitaryIncome
{
public Decimal? Amount { get; set; }
public string? Id { get; set; }
public bool? IsTaxable { get; set; }
public ICollection<MilitaryIncomeType>? MilitaryIncomeType { get; set; }
public IncomePayPeriodType? MilitaryPayPeriod { get; set; }
public Decimal? MonthlyAmount { get; set; }
public bool? OmitFromTotals { get; set; }
}
etc....
}
But when they try to map their models to the api client's mutation models, automapper blows up because the generated input models have this shape:
public class MilitaryIncomeInput : IGraphQlInputObject
{
private InputPropertyInfo _amount;
private InputPropertyInfo _id;
private InputPropertyInfo _isTaxable;
private InputPropertyInfo _militaryIncomeType;
private InputPropertyInfo _militaryPayPeriod;
private InputPropertyInfo _monthlyAmount;
private InputPropertyInfo _omitFromTotals;
[JsonConverter(typeof (QueryBuilderParameterConverter<Decimal?>))]
public QueryBuilderParameter<Decimal?>? Amount
{
get => (QueryBuilderParameter<Decimal?>) this._amount.Value;
set
{
this._amount = new InputPropertyInfo()
{
Name = "amount",
Value = (object) value
};
}
}
[JsonConverter(typeof (QueryBuilderParameterConverter<string>))]
public QueryBuilderParameter<string?>? Id
{
get => (QueryBuilderParameter<string>) this._id.Value;
set
{
this._id = new InputPropertyInfo()
{
Name = "id",
Value = (object) value
};
}
}
.....
Does that make sense now?
I'm trying to come up with some automapper extension methods that will handle this mapping that can be included in the api client. Then consumer code could just use these extension methods for mapping and problem solved.
But really the cleanest solution would be to have query/mutation models that are simple POCOs and don't have any of the QueryBuilderParameter logic included, since we don't need it for our use case and it's causing these automapper issues.
I was digging through the docs and the tests in the repo to see if this is configurable--nothing jumped out at me though.
UPDATE:
I might have come up with a simple workaround. I will test it out and post back here. If it works I'll add some details in case other people in the future have this issue.
You have a lot more experience with GraphQL model generation than me, so I'm still curious on what your thoughts are though either way 😁
I'm not against adding the option you requested. Indeed there are many trivial cases where simple POCO is more than enough. The QueryBuilderParameter wrapping is there for binding variables. So it's possible to have a GraphQL query builder instance with parameters and then the same instance can be used over and over because the parameter is bound as $parameter. And the implicit conversion is there to be able to also use a literal. Using pure POCOs makes parameter binding impossible but as you mentioned for many use cases that's perfectly fine. Also in case some one has to use System.Text.Json, POCO will work, current implementation will not because there is no System.Text.Json converter to handle the QueryBuilderParameter wrapping.
That would be amazing because the workaround that I mentioned failed. I wasn't able to get those automapping extensions working either, so I'm kind of stuck.
Do you have a rough estimate on when this feature would be ready? I'm deciding if I need to start coming up with a work around or just wait for the enhancement
This is my pet project and I have regular job apart of it. I usually fix things during weekends.
I totally get that and your work is much appreciated 🙌 , I was just asking because if it was going to be weeks or months I would start implementing a post process step that would transform the mutation models into POCOs. Then swap out that temp solution with the new version of this when it's ready
Hi. I've been playing around with the idea. It's very simple to generate input objects without the parameter wrappings but they still aren't perfect POCOs due to internal specific input object handling.
So the class looks like this:
public partial class Collision3 : IGraphQlInputObject
{
public string? Collision { get; set; }
public string? Collision2 { get; set; }
public string? CollisionInput { get; set; }
public string? CollisionInputObject { get; set; }
public string? CollisionDataRecord { get; set; }
public string? CollisionRecord { get; set; }
IEnumerable<InputPropertyInfo> IGraphQlInputObject.GetPropertyValues()
{
if (Collision != null) yield return new() { Name = "collision", Value = Collision };
if (Collision2 != null) yield return new() { Name = "collision2", Value = Collision2 };
if (CollisionInput != null) yield return new() { Name = "collisionInput", Value = CollisionInput };
if (CollisionInputObject != null) yield return new() { Name = "collisionInputObject", Value = CollisionInputObject };
if (CollisionDataRecord != null) yield return new() { Name = "collisionDataRecord", Value = CollisionDataRecord };
if (CollisionRecord != null) yield return new() { Name = "collisionRecord", Value = CollisionRecord };
}
}
but I think this could be ok because the interface implementation is explicit and invisible when working with class. One issue here is that it's impossible to distinguish default null from explicit null. In this implementation nulls will be ignored and not generated in the input object. Which may be a problem if something should be nullified by sending explicit null. There are ways to achieve that using other extension points. But if the intention is to use POCOs the schema should be designed the way that ignoring nulls is fine.
I think to add a new configuration option:
public enum InputObjectMode
{
/// <summary>
/// Supports GraphQL parameter references
/// </summary>
Rich,
Poco
}
What do you think?
I love the configuration option--simple and explicit.
Seems like that class shape will work fine too. In the example there's just string?, but if the property is a ICollection<string> or int or whatever it would work the same I imagine? The fact that the class inherits from IGraphQlInputObject is fine, we can just ignore that.
As far as the nullability issue, that's something I just started looking into at work Fri. What I'm thinking of doing is borrowing the Optional concept from Apollo Kotlin.
I have the class already implemented with unit tests and a json serializer included and it's working well. It basically just provides a way to wrap a property so null and unspecified can be distinguished from each other. The implicit operators are implemented so it can be used naturally like regular primitives. When an Optional<T> property is not set, it's considered Unspecified. When a property needs to be explicitly set to null, it's done like this:
mutationInput.FooBar = Optional<string>.Null
The json serializer ensures that the Unspecified properties are left out of the request payload when using the graphql-dotnet/graphql-client
I built a dotnet tool that leverages your framework to do the heavy lifting for model generation. There's a post processing step at the end that applies some transformations needed for our organization's use case. I'm going to add some logic there to wrap nullable properties with this Option<T> class.
I'll be proofing all of this out early next week.
the QueryBuilderParameter supplies the same effect as Optional, it's also one of the reasons I implemented it in the first place. Since with the new option the property is as simple as { get; set; } I will try to provide an extension point so it can be customized the same way as normal data objects when POCO option is enabled.
It's possibly user error, but my custom mapping that implements: IScalarFieldTypeMappingProvider
doesn't seemed to be getting invoked for certain fields. For example it's never invoked for fields of type int. Here's my config:
var generator = new GraphQlGenerator(new GraphQlGeneratorConfiguration
{
TargetNamespace = options.Namespace,
FileScopedNamespaces = true,
InputObjectMode = InputObjectMode.Poco,
EnableNullableReferences = true,
DataClassMemberNullability = DataClassMemberNullability.DefinedBySchema,
ScalarFieldTypeMappingProvider = new FieldTypeMappingProvider(),
});
Is this known behavior or am I just screwing something up on my end?
Yeah, that's by design. Well known GraphQL scalars have default mapping. There is a configuration property IntegerTypeMapping (and analogical for other built-in scalars). It needs to be set to Custom, then the scalar field type provider should be called.
I'm about done testing everything. So far everything is working great. The only thing I'm trying to figure is why the custom provider is not being called for certain types, like Address in this example. Here's what it looks like in the SDL
type Address {
street1: String
street2: String
city: String
state: String
zipCode: String
country: Country
}
Here's what it looks like in the schema object:
Here's the current state of the config:
var schema = await schemaProcessor.LoadAndParseSchema(options.SchemaPath);
var generator = new GraphQlGenerator(new GraphQlGeneratorConfiguration
{
TargetNamespace = options.Namespace,
FileScopedNamespaces = true,
InputObjectMode = InputObjectMode.Poco,
EnableNullableReferences = true,
DataClassMemberNullability = DataClassMemberNullability.DefinedBySchema,
IntegerTypeMapping = IntegerTypeMapping.Custom,
//PropertyGeneration = PropertyGenerationOption.BackingField,
ScalarFieldTypeMappingProvider = new FieldTypeMappingProvider(),
});
var csharpCode = generator.GenerateFullClientCSharpFile(schema);
Here's an example of how Address is used:
type Borrower {
id: String!
personalInfo: PersonalInfo
employment: [Employment!]!
assets: [Asset!]!
liabilities: [Liability]
creditScores: [CreditScore!]!
mailingAddress: Address
currentAddress: Address
}
I want the Address properties to be wrapped in Optional since it's not required (same issue with PersonalInfo as well). But since the mapping provider where I do the Optional wrapping is not being called, it ends up in the generated models as public Address? MailingAddress as an example.
As I take a step back and think about it though, maybe it's a moot point for these types of scenarios....I'll test further and post back
Right now, as the name IScalarFieldTypeMappingProvider suggests, the provider is only for scalar types. There is another way of achieving what you would like and that is by writing own GenerationContext. But as of now the type cannot be overriden there. I was thinking to extend that option some time ago but I haven't implemented it yet.
Ok good to know, that makes sense based on the naming.
Also, one more thing to point out you might be interested in. Using the mapping provider extension point to wrap nullable scalars in Optional<T> works, but the field name available in the context seem to be pre-model generation naming.
So for instance.
Enum defined in the schema
enum LPResultCode {
ACCEPT
CAUTION
REFER
UNKNOWN
}
This is what is created from the mapping provider:
public Optional<LPResultCode> GetLpResult { get; set; }
Here's what the generated model looks like:
public enum LpResultCode
{
[EnumMember(Value = "ACCEPT")] Accept,
[EnumMember(Value = "CAUTION")] Caution,
[EnumMember(Value = "REFER")] Refer,
[EnumMember(Value = "UNKNOWN")] Unknown
}
I was playing around w/different config options to workaround this but didn't come up with anything.
As far as the original ask for this issue goes though--everything is working great. So feel free to close this issue. Many thanks!!
public Optional<LPResultCode> GetLpResult { get; set; }
I assume just use $"Optional<{context.FieldType.UnwrapIfNonNull().Name}>" to construct the full type name? That's not enough because the properties' and classes' names are Pascalized along the way. You can get .NET enum name using
var csharpEnumName = DefaultScalarFieldTypeMappingProvider.GetFallbackFieldType(context).NetTypeName;
You might need to truncate ? because that method returns nullable type based on configuration and type of object being generated.
in the newest commit it's possible to do this
class MyCustomGenerationContext(GraphQlSchema schema, TextWriter writer, GeneratedObjectType objectTypes = GeneratedObjectType.All)
: SingleFileGenerationContext(schema, writer, objectTypes)
{
public override void OnDataPropertyGeneration(PropertyGenerationContext context)
{
/*
if (<GraphQL object and/or member filter>)
{
WriteMyOwnPropertyNameTypeGetterAndSetter();
return;
}
*/
base.OnDataPropertyGeneration(context);
}
}
and
var generator = new GraphQlGenerator(configuration);
var builder = new StringBuilder();
using var writer = new StringWriter(builder);
var generationContext = new MyCustomGenerationContext(schema, writer, GeneratedObjectType.All);
generator.Generate(generationContext);
Console.WriteLine(builder.ToString());
This extension point has existed for long time but it hasn't included the property type and name until the latest commit, only the property getter and setter.