Skip to content

feat!: Invoke async message scenario factories on request #536

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamrodger
Copy link
Contributor

We still have to invoke the factory with sync-over-async at request time and this puts a burden on the user not to use this improperly, but if we want to delay invocation of the factory until after provider states have been configured then this is the first step.

This is also obviously a breaking API change and requires a major version bump. WithAsyncContent makes way more sense because it's the content factory that's async, not the call to the builder as WithContentAsync would imply. This also means the return type changes to void instead of Task.

See #459

We should also investigate making the internal messaging server itself async so that we don't have to do the sync-over-async shenanigans, which could maybe be achieved with Task.Run (or similar) instead of using a dedicated thread. We'd probably then also change the signature of Scenario.Factory to be Func<Task<dynamic>> instead, and the sync versions would wrap in Task.FromResult.

We do get really weird behaviour when using dynamic inside Func<> though, so this has to be carefully investigated, and may not be possible.

We still have to invoke the factory with sync-over-async at request time
and this puts a burden on the user not to use this improperly, but if we
want to delay invocation of the factory until after provider states have
been configured then this is the first step.

This is also obviously a breaking API change and requires a major
version bump. `WithAsyncContent` makes way more sense because it's the
content factory that's async, not the call to the builder as
`WithContentAsync` would imply. This also means the return type changes
to void instead of Task.

See #459
@adamrodger adamrodger added this to the 6.0.0 milestone Apr 1, 2025
@adamrodger
Copy link
Contributor Author

One problem with making the messaging provider async is that you end up with this signature on IMessagenScenarios:

    /// <summary>
    /// Add a message scenario
    /// </summary>
    /// <param name="description">Scenario description</param>
    /// <param name="factory">Message content factory</param>
    IMessageScenarios Add(string description, Func<dynamic> factory);

    /// <summary>
    /// Add a message scenario
    /// </summary>
    /// <param name="description">Scenario description</param>
    /// <param name="factory">Message content factory</param>
    IMessageScenarios Add(string description, Func<Task<dynamic>> factory);

When you invoke that then it causes an issue:

// sync invocation
Scenarios.Add("foo", () => new Foo());

// async invocations
Scenarios.Add("foo", () => Task.FromResult(new Foo())); // hmmmm
Scenarios.Add("foo", async () => await CreateFooAsync()); // also hmmmm

Because of the use of dynamic then technically all of those invocations actually match the sync version, and the API breaks. I haven't tested that out yet but I've had that problem before when the initial redesign happened for 4.x and we wanted to support async with dynamics.

@adamrodger
Copy link
Contributor Author

adamrodger commented Apr 1, 2025

My initial version does get all kinds of weird compiler errors in the tests, and those highlight ways in which this would remove some existing useful functionality. For example, this compiles:

// anonymous type returned from a factory
Func<dynamic> factory = () => new { Foo = 42 };

but this doesn't:

// try so use anonymous type in an async factory
Func<Task<dynamic>> factory = () => Task.FromResult(new { Foo = 42 });

You aren't allowed to use anonymous types in that scenario, and lots of people use anonymous types when defining their pacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant