pact-foundation / pact-net

.NET version of Pact. Enables consumer driven contract testing, providing a mock service and DSL for the consumer project, and interaction playback and verification for the service provider project.
https://pact.io
MIT License
823 stars 225 forks source link

RFC: Support System.Text.Json default polymorphic type discriminator when using .WithJsonBody() #499

Closed andyfurniss4 closed 1 month ago

andyfurniss4 commented 1 month ago

Is your feature request related to a problem? Please describe. When using polymorphism with System.Text.Json, the default type discriminator key will be $type (see https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/polymorphism?pivots=dotnet-8-0#polymorphic-type-discriminators). Since we must pass a dynamic object to .WithJsonBody(dynamic body) to provide the ability to use that various IMatcher implementations, there is no way to provide this $type field, as $type is not a valid C# property name. This would also be a problem for any other metadata-like properties you might have which are prefixed with a non-valid C# character.

[JsonDerivedType(typeof(Orange), "orange")]
public interface IFruit
{
  public string Name { get; }
  public FruitShape Shape { get; }
}

public record Orange(string Name, FruitShape Shape) : IFruit
{
  public void Zest()
  {
    // Do something
  }
}
{
  "$type": "orange",
  "name": "orange",
  "shape": "round"
}

This leaves us with two rather poor options:

  1. Change the type discriminator key and thus our public API schema, simply for the purposes of some Pact tests.
  2. Pass pure JSON to WithJsonBody, but this prevents us from using the matching functionality.

Describe the solution you'd like Considerations for an alternative way of building an expected JSON body, which does not restrict you to C# syntax when defining property names. We are dealing with JSON here so it doesn't really make sense to have the restriction imposed.

Breaking Changes I would hope that any new method would simply be an additional overload of the .WithJsonBody() method, so hopefuly not a breaking change.

Describe alternatives you've considered Two remaning options outlined above.

Additional context N/A

adamrodger commented 1 month ago

Would using an ExpandoObject work in this situation? It can behave like a Dictionary<string, object> so the property names may not be limited to only valid C# syntax.

On Mon, 20 May 2024, 15:39 Andy Furniss, @.***> wrote:

Is your feature request related to a problem? Please describe. When using polymorphism with System.Text.Json, the default type discriminator key will be $type (see https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/polymorphism?pivots=dotnet-8-0#polymorphic-type-discriminators). Since we must pass a dynamic object to .WithJsonBody(dynamic body) to provide the ability to use that various IMatcher implementations, there is no way to provide this $type field, as $type is not a valid C# property name. This would also be a problem for any other metadata-type properties you might have which are prefixed with a non-valid C# character.

[JsonDerivedType(typeof(Orange), "orange")]public interface IFruit{ public string Name { get; } public FruitShape Shape { get; }} public record Orange(string Name, FruitShape Shape){ public void Zest() { // Do something }}

{ "$type": "orange", "name": "orange", "shape": "round" }

This leaves us with two rather poor options:

  1. Change the type discriminator key and thus our public API schema, simply for the purposes of some Pact tests.
  2. Pass pure JSON to WithJsonBody, but this prevents us from using the matching functionality.

Describe the solution you'd like Considerations for an alternative way of building an expected JSON body, which does not restrict you to C# syntax when defining property names. We are dealing with JSON here so it doesn't really make sense to have the restriction imposed.

Breaking Changes I would hope that any new method would simply be an additional overload of the .WithJsonBody() method, so hopefuly not a breaking change.

Describe alternatives you've considered Two remaning options outlined above.

Additional context N/A

— Reply to this email directly, view it on GitHub https://github.com/pact-foundation/pact-net/issues/499, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD4FKQKP75WQP4V2ML6BVTZDIDLNAVCNFSM6AAAAABH73CDCKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMYDMMJUG42TSNA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

andyfurniss4 commented 1 month ago

Thanks @adamrodger, I hadn't thought of using an ExpandoObject (I can't remember the last time I did 😄). That's resolved our current issues.