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.

Sign up for GitHub

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

Jump to bottom

Provider state parameters are serialised as numbers, causes errors in provider state middleware when using System.Text.Json #449

Closed
DavidJFowler opened this issue Apr 6, 2023 · 6 comments · May be fixed by #506
Closed

Provider state parameters are serialised as numbers, causes errors in provider state middleware when using System.Text.Json #449

DavidJFowler opened this issue Apr 6, 2023 · 6 comments · May be fixed by #506
Labels
bug Indicates an unexpected problem or unintended behavior upstream Indicates that an issue relates to an upstream problem (such as in pact-reference) wontfix Indicates that work won't continue on an issue, pull request, or discussion

Comments

@DavidJFowler
Copy link

If the value of a provider state parameter is a number string, it is serialised in the pact file as a number. This is an issue if the provider state middleware in the provider tests uses System.Text.Json to deserialise a ProviderState value as System.Text.Json does not deserialise non-string values into string properties. https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/migrate-from-newtonsoft?pivots=dotnet-6-0#non-string-values-for-string-properties.

For example:

using System.Net;
using Newtonsoft.Json.Serialization;
using Newtonsoft.Json;
using PactNet;

var config = new PactConfig
{
    PactDir = Path.Join("..", "..", "..", "pacts"),
    DefaultJsonSettings = new JsonSerializerSettings
    {
        ContractResolver = new CamelCasePropertyNamesContractResolver(),

    }
};

var pact = Pact.V3("consumer", "provider", config).WithHttpInteractions();

pact.UponReceiving("A valid request")
    .Given("state with parameter", new Dictionary<string, string>() {["id"] = "10", ["name"] = "Fred"})
    .WithRequest(HttpMethod.Get, "/api/endpoint")
    .WillRespond()
    .WithStatus(HttpStatusCode.OK);

await pact.VerifyAsync(async ctx =>
{
    using var apiClient = new ApiClient(ctx.MockServerUri);

    await apiClient.TestRequestAsync();
});

public class ApiClient: IDisposable
{
    private readonly HttpClient _httpClient;

    public ApiClient(Uri baseUri)
    {
        _httpClient = new HttpClient { BaseAddress = baseUri };
    }

    public async Task TestRequestAsync()
    {
        await _httpClient.GetAsync("api/endpoint");
    }

    public void Dispose()
    {
        _httpClient.Dispose();
    }
}

generates this pact. Note that parameter "id" is serialised as 10 not "10".

{
  "consumer": {
    "name": "consumer"
  },
  "interactions": [
    {
      "description": "A valid request",
      "providerStates": [
        {
          "name": "state with parameter",
          "params": {
            "id": 10,
            "name": "Fred"
          }
        }
      ],
      "request": {
        "method": "GET",
        "path": "/api/endpoint"
      },
      "response": {
        "status": 200
      }
    }
  ],
  "metadata": {
    "pactRust": {
      "ffi": "0.4.0",
      "models": "1.0.4"
    },
    "pactSpecification": {
      "version": "3.0.0"
    }
  },
  "provider": {
    "name": "provider"
  }
}
@adamrodger
Copy link
Contributor

Thanks for the report 👍 I'll check out whether the problem is happening in PactNet itself or whether it's after we've passed to the FFI library.

If it's the latter, and gut feel is that it is, then I'll raise an issue on the other repo and link here.

@adamrodger adamrodger added the bug Indicates an unexpected problem or unintended behavior label Apr 9, 2023
adamrodger added a commit that referenced this issue Apr 9, 2023
@adamrodger
test(449): Add a test to reproduce issue #449
d4b105b
@adamrodger
Copy link
Contributor

I've added a test which reproduces the issue, and confirms that this is an issue in pact-reference, not pact-net. We're passing a string value but a numeric value ends up getting written to the pact file.

I'll raise an upstream issue and link here.

@adamrodger adamrodger mentioned this issue Apr 9, 2023
Closed
@mefellows mefellows added the upstream Indicates that an issue relates to an upstream problem (such as in pact-reference) label Aug 18, 2023
@adamrodger adamrodger closed this as not planned Won't fix, can't repro, duplicate, stale May 27, 2024
@github-project-automation github-project-automation bot moved this from New Issue to Closed in Pact Triage (not yet in use) May 27, 2024
@adamrodger adamrodger added the wontfix Indicates that work won't continue on an issue, pull request, or discussion label May 27, 2024
@adamrodger
Copy link
Contributor

Unfortunately the upstream maintainers closed the linked issue I created to change this as they believe it's working as expected, and so there's nothing I can do to change this issue.

I've added a comment saying I don't agree that's working as expected, but that's where we are unfortunately.

@YOU54F
Copy link
Member

YOU54F commented May 27, 2024

Added a comment here about a resolution from reading the linked issue, which appears to resolve the issue reproduction. (great things to have to hand!)

pact-foundation/pact-reference#263 (comment)

@YOU54F
Copy link
Member

YOU54F commented May 27, 2024

question - how would you expect someone to pass in an integer value here if the signatures are strings for end users?

@YOU54F
Copy link
Member

YOU54F commented May 28, 2024

Unfortunately the upstream maintainers closed the linked issue I created to change this as they believe it's working as expected, and so there's nothing I can do to change this issue.

I've been reflecting on this overnight and I'm sorry you feel that there is nothing you can do to change this issue. I actually thought that when I read that comment that it was going to be something outside the Pact ecosystem, rather than from inside.

We definitely want to find the right, or most pragmatic solution, that works well for as many users, in as many consuming languages as possible!

Maybe the current direction isn't correct, but we are always happy to deliberate, and deliberate some more until we can get to a democratised solution :)

We are all part of one big (hopefully happy) family!

@YOU54F YOU54F mentioned this issue Jun 10, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior upstream Indicates that an issue relates to an upstream problem (such as in pact-reference) wontfix Indicates that work won't continue on an issue, pull request, or discussion
Projects
Pact Triage (not yet in use)
Status: Closed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix/numeric provider state param YOU54F/pact-net
4 participants
@mefellows @adamrodger @DavidJFowler @YOU54F