Deprecate most GraphService methods and converge on two standard methods

[nozzlegear]

This change deprecates most GraphService.PostAsync and GraphService.SendAsync methods, of which there were at least six total. It introduces two new GraphService.PostAsync methods to replace them all, which standardize the usage of the GraphRequest class and serializing all requests to json.

Previously, some of the PostAsync/SendAsync methods would serialize the request to json, some wouldn't. Some would send the variables, some wouldn't. Some would let you set your own json wherein you could apply the variables yourself, but then they'd send the application/graphql content-type, which made Shopify ignore any variables sent.

That one has personally bit me each time I've tried to use the GraphService recently; it's been difficult to figure out which method should be used on the service when I want to send variables, without looking at the source code for ShopifySharp.

After this change, the new method for sending Graph requests using the GraphService going forward would look like this:

var query = 
"""
query GetStuff($productId: String!) {
  product(id: $productId) {
    id
    title
  }
}
""";
var variables = new
{
  productId = "gid://shopify/products/123456"
};
var graphRequest = new GraphRequest
{
  Query = query,
  Variables = variables,
  EstimatedGraphCost = 3
};

// Get a JsonElement back
var result = await graphService.PostAsync(graphRequest, cancellationToken);
// Or deserialize into whatever you want
var result = await graphResult.PostAsync<ShopifySharp.GraphQL.Product>(graphRequest, cancellationToken);

I don't want anyone to lose the ability to control how variables are serialized or deserialized though, so I've added an IGraphSerializer interface that can be passed to the GraphService constructor.

public class MyCustomGraphSerializer : IGraphSerializer
{
  public string SerializeToJson(IDictionary<string, object> variables)
  {
    throw new NotImplementedException();
  }
  
  public T DeserializeFromJson<T>(string json)
  {
    throw new NotImplementedException();
  }
}

// elsewhere
var serializer = new MyCustomGraphSerializer()
var graphService = new GraphService(domain, accessToken, graphSerializer: serializer);

I'm not 100% sure this is the exact interface I'll use for this, but it's something along the lines of what I'm thinking. Whether we use this interface or do something else for serialization/deserialization in the GraphService, I want it to play nicely with the factory and DI package.

[clement911]

Should it be also be possible to provide variables via a dictionary?

[nozzlegear]

Oh yeah, I like that more @clement911! So maybe we change the GraphRequest class to look like this:

public class GraphRequest
{
  public string Query { get; set; }
  public Dictionary<string, object> Variables { get; set; }
  
  // Convenience methods for adding/setting variables?
  public GraphRequest AddVariable(string key, object value)
  {
    Variables.Add(key, value);
    return this;
  }
  
  public GraphRequest SetVariable(string key, object value)
  {
    Variables.Set(key, value);
    return this;
  }
}

Then you could just pass in an entire dictionary if you've already got one, use the convenience methods, or access the dictionary object directly.

[clement911]

I'm not sure that the AddVariable / SetVariable methods really are needed.
The user can simply use the Variables dictionary to manage variables, right?

[nozzlegear]

Agreed, I was thinking about that too and it doesn’t sound that useful. I’m going to drop that idea, but I did make the change to switch the Variables type to a dictionary.—Joshua HarmsOn Apr 7, 2024, at 17:19, Clement Gutel ***@***.***> wrote: I'm not sure that the AddVariable / SetVariable methods really are needed. The user can simply use the Variables dictionary to manage variables, right? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[nozzlegear]

Remaining tasks:

• Unit tests to ensure that obsoleted methods did not break with this PR.
• Unit tests to ensure proper serialization of the GraphRequest, GraphExtensions and GraphUserError objects.
• Unit tests for disabling error handling in the GraphService
• Unit tests for enabling error handling in the GraphService
• Make PostAsync and PostAsync<T> return a GraphResult<T>.
• Documentation for this release
• Make it work with .NET 472? (Remove all #IF NET8_0_OR_GREATER checks.) (Not possible at the moment, the generated GraphQL schema uses default interfaces which aren't supported in .NET 472.)