Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: adding virtual object serialization method to entities #642

Closed
wants to merge 2 commits into from

Conversation

nozzlegear
Copy link
Owner

@nozzlegear nozzlegear commented Jun 6, 2021

I'm experimenting with adding custom object serialization to all of our entity models. Open to feedback here, but what I'm envisioning is a virtual ToJsonDictionary method on the base ShopifyObject class that all of our models inherit. The base implementation will use the current ObjectExtensions.ToDictionary method to serialize itself, and if developers need custom serialization for a certain object they can override the method.

One thing to consider for this implementation is that it still does not give the developer control of the entire object that gets sent during requests to Shopify. For example, when creating an order we need to send {"order": { ... }}. The developer can customize what's inside the order itself, but they have no control of the outer body. Is that okay? Technically if they need to control that part they can just create their own service implementation and override whatever method they're trying to customize.

Things that would be nice to have but not necessary for this particular pull request:

  1. Add some kind of custom deserialization method for each object. I'm not sure how that could/should be done with Json.Net, since you typically just pass the class type to the deserializer and let Json.Net figure out the rest.
  2. I'd still like to implement something like Update Product by explicit fields #477 where developers can explicitly choose which fields get serialized, maybe with some kind of extra customization that also tells the serializer if it should be included when null. Psuedo code:
myObject
    .SetNullSerialization(NullSerialization.Exclude)
    .ConfigureNullSerialization(x => x.Property1 =  NullSerialization.Include; x.Property2 = NullSerialization.Exclude)

Something to consider when implementing the goal of this pull request. Open to feedback here!

@nozzlegear nozzlegear added enhancement object updating Bugs and strange behavior around updating objects, primarly related to #284 experimental Deals with experimental features or changes that may not be published in ShopifySharp itself RFC Request for comments labels Jun 6, 2021
@clement911
Copy link
Collaborator

clement911 commented Jun 7, 2021

I feel like it's a bit too loose and complicated. This would be easy to shoot oneself in the foot and might lead to some confusion.

I have recently seen an interesting pattern that is very easy to implement, and that I would like to propose as a potential alternative solution.

The idea is to simply add a bool SparseUpdate { get; set;} property to all entity objects.
When serializing the object, ShopifySharp would check the value of this property.
If false, then all properties are serialized.
If true, then only non-null properties are serialized.

It seems to that it would strike a nice balance between flexibility and simplicity.
What do you think?

@nozzlegear
Copy link
Owner Author

nozzlegear commented Jun 9, 2021 via email

@clement911
Copy link
Collaborator

clement911 commented Jun 9, 2021

The QuickBooks API uses this pattern, but the difference is that it's handled by the API itself.
Every entity have a Sparse property. If set to true, the API will ignore null properties.
See https://developer.intuit.com/app/developer/qbo/docs/api/accounting/all-entities/customer#sparse-update-a-customer

Here is an example to update just the MiddleName:
image

For ShopifySharp, we'll have to handle it in the library and change the serialization accordingly.

Here is what I'm thinking.

Add the following to ShopifyObject.cs

[JsonIgnore]
public bool? SparseUpdate { get; set; }

I guess it could be nullable, although a default value of false seems appropriate.

Create a new ShopifyObjectJsonConverter class, which has custom logic. It inspects the value of SparseUpdate, and according to the value it will either ignores null (don't serialize them) or serializes them.

Add this converter to the ShopifyObject class.

@nozzlegear
Copy link
Owner Author

What would we do in cases where you specifically want to send null with a sparse update, for whatever reason? Something like this:

{ "property1": 123456, "property2": null }

@clement911
Copy link
Collaborator

That would not work and that is one downside. If you want to send a null, you must first load the object via the api first and send a full update.

@nozzlegear nozzlegear closed this Aug 5, 2022
@nozzlegear nozzlegear deleted the virtual-object-serialization branch December 11, 2023 19:43
@nozzlegear nozzlegear restored the virtual-object-serialization branch December 11, 2023 19:43
@nozzlegear nozzlegear deleted the virtual-object-serialization branch May 16, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement experimental Deals with experimental features or changes that may not be published in ShopifySharp itself object updating Bugs and strange behavior around updating objects, primarly related to #284 RFC Request for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants