Better support for POST and PUT requests.

[nozzlegear]

ShopifySharp runs into an issue that many C# libs find themselves facing: non-nullable types and POST/PUT requests don't mesh that well. For example, take the following class:

public class MyObject
{
  public int Id { get; set; }
  public DateTime Created { get; set; }
  public double Price { get; set; }
  public bool Active { get; set; }
}

Suppose that we need to create a "MyObject" through the Shopify API, but the POST option expects just the Price property. We create a new "MyObject" like so:

var myObj = new MyObject() { Price = 123.00 };

And we send that object to our pretend API. However, because of the way C# works with non-nullable types, C# will send the following:

{ "id": 0, "created": "0001-01-01T00:00:00", "price": 123.00, "active": false }

That's not good. The API only wanted the price, but C# sent an empty Id and Created date, and a false Active flag too. This isn't a huge problem when using POST endpoints because Shopify will typically just ignore the extra stuff it isn't looking for.

The big problem is working with updates to objects through PUT requests. Suppose you now want to update just the Active property for an object that was already created. You create your object like so:

var myUpdatedObj = new MyObject() { Active = true };

But when you send it, C# serializes it into this:

{ "id": 0, "created": "0001-01-01T00:00:00", "price": 0, "active": true }

Now that's a big problem. Active got set to true, but the object's price was just set to 0 too. You probably didn't want to do that, but thanks to C#'s non-nullable types it happened anyway without any input from you. The only way to avoid doing this is to pull the object from the API first, then make changes to that object and send the whole object through the update.

That's far from ideal; now updating an object goes from one request to two, and wastes bandwidth too. For ShopifySharp 3.0, I'm looking into ways to prevent sending unnecessary information through POST and PUT requests. I have two solutions right now, but welcome any suggestions:

1. Make all non-string properties nullable. Drawback is that you now have to check if a prop has a value when operating on an object (e.g. Created.Value.ToShortDateString() instead of Created.ToShortDateString()).
2. Create separate classes for creating and updating an object, akin to the way Stripe.net handles create/update requests. Drawback is a lot of duplicated code, and making changes to one object will need to be reflected to its create/update objects too.

[nozzlegear]

Also discussed in #49.

[Infob]

I’m a bit rusty on C# but I have run into that issue before. I ended up using a Dynamic class. That way I just set the relavant stuff.

From: Joshua Harms [mailto:notifications@github.com]
Sent: Wednesday, 20 July 2016 12:50 AM
To: nozzlegear/ShopifySharp ShopifySharp@noreply.github.com
Subject: [nozzlegear/ShopifySharp] Better support for POST and PUT requests. (#62)

ShopifySharp runs into an issue that many C# libs find themselves facing: default property values and POST/PUT requests don't mesh that well. For example, take the following class:

public class MyObject
{
public int Id { get; set; }
public DateTime Created { get; set; }
public double Price { get; set; }
public bool Active { get; set; }
}

Suppose that we need to create a "MyObject" through the Shopify API, but the POST option expects just the Price property. We create a new "MyObject" like so:

var myObj = new MyObject() { Price = 123.00 };

And we send that object to our pretend API. However, because of the way C# works with non-nullable types, C# will send the following:

{ "id": 0, "created": "0001-01-01T00:00:00", "price": 123.00, "active": false }

That's not good. The API only wanted the price, but C# sent an empty Id and Created date, and a false Active flag too. This isn't a huge problem when using POST endpoints because Shopify will typically just ignore the extra stuff it isn't looking for.

The big problem is working with updates to objects through PUT requests. Suppose you now want to update just the Active property for an object that was already created. You create your object like so:

var myUpdatedObj = new MyObject() { Active = true };

But when you send it, C# serializes it into this:

{ "id": 0, "created": "0001-01-01T00:00:00", "price": 0, "active": true }

Now that's a big problem. Active got set to true, but the object's price was just set to 0 too. You probably didn't want to do that, but thanks to C#'s non-nullable types it happened anyway without any input from you. The only way to avoid doing this is to pull the object from the API first, then make changes to that object and send the whole object through the update.

That's far from ideal; now updating an object goes from one request to two, and wastes bandwidth too. For ShopifySharp 3.0, I'm looking into ways to prevent sending unnecessary information through POST and PUT requests. I have two solutions right now, but welcome any suggestions:

1. Make all non-string properties nullable. Drawback is that you now have to check if a prop has a value when operating on an object (e.g. Created.Value.ToShortDateString() instead of Created.ToShortDateString()).
2. Create separate classes for creating and updating an object, akin to the way Stripe.net handles create/update requests https://github.com/jaymedavis/stripe.net/tree/master/src/Stripe/Services/Charges . Drawback is a lot of duplicated code, and making changes to one object will need to be reflected to its create/update objects too.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #62 , or mute the thread https://github.com/notifications/unsubscribe-auth/ABD4kIHzoUeuF7V_H_QTpyLwuLkwZut5ks5qXOQvgaJpZM4JPzy9 . https://github.com/notifications/beacon/ABD4kN_is0vPT4c33MOUBGyBa60KeLu2ks5qXOQvgaJpZM4JPzy9.gif

[nozzlegear]

I've thought about doing dynamic classes, but I don't think they're the right fit for ShopifySharp. With a dynamic class the developer will need to remember the names and casing of the real Shopify properties, not just the ones in a ShopifySharp class. For example, the CreatedAt property in many objects is actually created_at in the Shopify API and just gets serialized to that by this lib. There are a ton of properties that developers would need to know the specific casing and underscores for, so I feel losing intellisense and having to look up property names is too big a tradeoff for supporting PUT/POST requests.

[Infob]

Yes, It made sense in my case as it was a Private App.

I’m looking at building a public app, so will let you know if find a solution.

Those node guys have got it easy!

From: Joshua Harms [mailto:notifications@github.com]
Sent: Thursday, 21 July 2016 11:42 PM
To: nozzlegear/ShopifySharp ShopifySharp@noreply.github.com
Cc: Shaun Latham shaun@infobytesmedia.com.au; Comment comment@noreply.github.com
Subject: Re: [nozzlegear/ShopifySharp] Better support for POST and PUT requests. (#62)

I've thought about doing dynamic classes, but I don't think they're the right fit for ShopifySharp. With a dynamic class the developer will need to remember the names and casing of the real Shopify properties, not just the ones in a ShopifySharp class. For example, the CreatedAt property in many objects is actually created_at in the Shopify API and just gets serialized to that by this lib. There are a ton of properties that developers would need to know the specific casing and underscores for, so I feel losing intellisense and having to look up property names is too big a tradeoff for supporting PUT/POST requests.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub #62 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/ABD4kKSuXBuPSfiq8D_4WmGsOgVYhVChks5qX3cygaJpZM4JPzy9 . https://github.com/notifications/beacon/ABD4kCaQn9f7NNSsCmpLfuGfnFC_7P23ks5qX3cygaJpZM4JPzy9.gif

[clement911]

One idea comes to mind.
For each entity type (Customer, Product, etc...), create a corresponding enum annotated with the flags attribute that enumerates all fields of the entity.

For example

[Flags]
internal enum CustomerFields
{
 None = 0,
 FirstName = 1,
LastName = 2,
OtherField = 4,
etc....
}

Then change each property setter to keep track of what fields were updated (a private property of type CustomerFields would exist on each Customer).
Whenever updating the entity, the code would serialize only the fields that were set...

Another idea would be to keep a copy of each loaded entity and then compare it to the entity being posted to find out what changed...

This is similar to how Entity Framework and other ORMs work...

[cafeasp]

To solve my C# serializes issue, I'm using this in my class like
[JsonProperty("fulfillment_service", NullValueHandling = NullValueHandling.Ignore)] public string FulfillmentService { get; set; }
If I don't assign a value, it won't show up in the json I send.

[nozzlegear]

That looks like a good solution! I'm going to take a closer look at the extra settings for Json.Net this weekend to see what options are available. IIRC, there's another setting that's something like DefaultValueHandling = Ignore that might be useful too.

[clement911]

I don't how see that solves the issue.
String is a reference type. The problem at hand concerns value types (non nullable).

[nozzlegear]

You're right @clement911. The NullValueHandling would work for nullable types, but not for booleans, DateTimes, etc. I've experimented with Json.Net's DefaultValueHandling over the weekend, and that gets closer but doesn't send anything that is a default value; so setting MyClass.MyProperty = false wouldn't send MyProperty at all, because false is the default value of a bool.

It's looking like either setting all properties to nullable types, or using an internal flag system like @clement911 suggested + Json.Net's conditional property serialization are going to be the best solutions. I'll experiment further, other suggestions are still welcome.

[vinhch]

Why not change all non-nullable to nullable , like int?, bool?... Based on my experienceI when working with shopify API, I realize that everything can be null, so all property type in my class is nullable.

And do you have future plan to handling api call limit? I mean that we can have something like if RequestEngine encount a call limit, it will auto-retry request after some timeout, or RequestEngine can set timeout to future request if it see the call limit in response header is going to full (39/40).

Sorry for my poor english.

[nozzlegear]

Thanks for the suggestion @vinhch. Right now we're considering doing exactly what you're suggesting as one of the solutions to this problem.

Regarding the rate limit, I don't have any plans to add auto-retry to the lib as I believe that's better handled by the developer for their specific use-case. You should be able to quickly implement an auto-retry yourself right now by wrapping your API call in a try/catch and then catch the exception when the HttpStatusCode is 429 (too many requests):

try
{
 await orderService.DeleteAsync(orderId);
}
catch (ShopifyException e) when ((int) e.HttpStatusCode == 429 /*too many requests*/)
{
  // You hit the rate limit. Consider waiting for 10 seconds to clear your rate limit and then try again.
}

I've got plans to throw a specific rate limit exception in a future release, but for now the above code is what I use in my own apps. I'd also like to somehow expose the current rate limit after every request, but don't have any ideas as to what that might look like right now.

[mchandschuh]

If we're trying to stay away from duplicate types, then I think making all non-nullable members nullable is the right way to go. JSON.NET does have a DefaultValueHandling enumeration that would prevent serialization of zero values, but sometimes we want to set things to zero as well.

Another option would be for the update methods to accept an argument that specifies which properties to serialize, probably in the form of expression selectors. So, for your example above, it may look like:

var myUpdatedObj = new MyObject() { Active = true };
await service.UpdateAsync(myUpdatedObj, obj => obj.Active);

The signature of the method could accept params Expression<Func<T, object>>[] which would allow the developer to specify each property to be serialized:

public async ShopifyProductVariant UpdateAsync(ShopifyProductVariant variant, params Expression<Func<ShopifyProductVariant>>[] propertiesToUpdate)

The implementation would then inspect each expression to see which properties should be serialized. If no expressions are specified, then all properties would be serialized.

I'm not sure if this fits with your vision of ShopifySharp, but figured I'd offer an alternative to the discussion!

[nozzlegear]

This change has finally landed in the master branch! It will be published on Nuget by Friday evening.