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

Feedback and issues around transactions and the command line #173

Closed
agross opened this issue Feb 6, 2023 · 9 comments
Closed

Feedback and issues around transactions and the command line #173

agross opened this issue Feb 6, 2023 · 9 comments

Comments

@agross
Copy link

agross commented Feb 6, 2023

Hello!

I recently started a new project and was interested in trying out Wolverine after following @jeremydmiller's blog posts. He was asking for feedback so here are my experiences.

The project is a Web API with hexagonal architecture. I'll be using Marten for document storage and right now am interested in the mediator bits of Wolverine. Perhaps async messaging will be added at some point in time when the need arises. The setup relevant Marten / Wolverine looks like this:

services.AddScoped<IBuildingRepository, BuildingRepository>();

var marten = services.AddMarten(options =>
                     {
                       options.Connection(configuration.GetConnectionString("Marten") ??
                                          throw new
                                            InvalidOperationException("Please set the connection string \"Marten\" in appsettings."));

                       options.DatabaseSchemaName = "wpm";

                       // If we're running in development mode, let Marten just take care
                       // of all necessary schema building and patching behind the scenes.
                       if (environment.IsDevelopment())
                       {
                         options.AutoCreateSchemaObjects = AutoCreate.All;
                       }
                     })

                     // This is the wolverine integration for the outbox/inbox,
                     // transactional middleware, saga persistence we don't care about
                     // yet.
                     .IntegrateWithWolverine("wolverine_messages")

                     // Just letting Marten build out known database schema elements upfront
                     // Helps with Wolverine integration in development.
                     .ApplyAllDatabaseChangesOnStartup();

if (environment.IsDevelopment())
{
  marten.InitializeWith<Buildings>();
}

host.UseWolverine(options =>
{
  // Automatic transactions around handlers.
  options.Handlers
         .AutoApplyTransactions();

  options.Handlers
         .Discovery(d => d.IncludeAssembly(typeof(Application.DependencyInjection).Assembly));

  // Enrolling all local queues into the durable inbox/outbox processing.
  options.Policies.UseDurableLocalQueues();
});

Automatic transactions around handlers

These are the handler bits I'll be referring to.

public record RegisterBuildingCommand(BuildingDto Building);

public class RegisterBuildingCommandHandler
{
  public static void Handle(RegisterBuildingCommand command,
                            IBuildingRepository repo)
  {
    repo.Add(command.Building);
  }
}

public class BuildingRepository : IBuildingRepository
{
  readonly IDocumentSession _session;

  public BuildingRepository(IDocumentSession session)
  {
    _session = session;
  }

  public void Add(BuildingDto building)
  {
    _session.Store(building);
  }

  public BuildingDto[] GetAll()
    => _querySession.Query<BuildingDto>().ToArray();
}

The documentation states

With this enabled, Wolverine will automatically use the Marten transactional middleware for handlers that have a dependency on IDocumentSession (meaning the method takes in IDocumentSession or has some dependency that itself depends on IDocumentSession)

My testing found that the "or has some dependency that itself depends on IDocumentSession" bit is currently not true. The BuildingRepository stores the BuildingDto, but that is never committed. Only after injecting the session into the handler and moving the session.Store call right into it the document is inserted into the database.

Handlers with return values log FailureAcknowledgement

There is another handler to retrieve all stored buildings.

public static BuildingDto[] Handle(AllBuildingsQuery _,
                                     IBuildingRepository repo)
    => repo.GetAll();

Whenever this runs the following is logged:

dbug: Marten.IDocumentStore[0]
      Marten executed in 0 ms, SQL: select d.data from wpm.mt_doc_buildingdto as d
      
dbug: Wolverine.Runtime.WolverineRuntime[100]
      Enqueued for sending FailureAcknowledgement#018625c6-1315-4f01-9793-9bcf2fd1d4fa to local://replies/
dbug: Wolverine.Runtime.WolverineRuntime[102]
      Started processing FailureAcknowledgement#018625c6-1315-4f01-9793-9bcf2fd1d4fa
dbug: Wolverine.Runtime.WolverineRuntime[103]
      Finished processing FailureAcknowledgement#018625c6-1315-4f01-9793-9bcf2fd1d4fa
dbug: Wolverine.Runtime.WolverineRuntime[103]
      Finished processing AllBuildingsQuery#018625c6-118c-44e8-93d8-5d3a9763cd9b
info: Wolverine.Runtime.WolverineRuntime[104]
      Successfully processed message FailureAcknowledgement#018625c6-1315-4f01-9793-9bcf2fd1d4fa from local://replies/

I do not know whether this is to be expected, but FailureAcknowledgement does not sound good to me.

Every time this happens a "Handled" row appears in wolverine_incoming_envelopes with data that looks like this:

source�Infrastructure
message-type Wolverine.Runtime.RemoteInvocation.FailureAcknowledgement	
reply-uri�local://replies/
content-type�application/json�
correlation-id da4e87378b6d0937490d8b14a5b10cc0�
conversation-id$018625c6-118c-44e8-93d8-5d3a9763cd9b�
destination�local://replies/�
accepted-content-types�application/json�
id$018625c6-1315-4f01-9793-9bcf2fd1d4fa

���{"$id":"1","RequestId":"018625c6-118c-44e8-93d8-5d3a9763cd9b","Message":"No response was created for expected response 'Application.UseCases.Buildings.Models.BuildingDto[]'"}

Command line failures

While researching why automatic transactions are not applied I was interested in seeing the code generated for the handler, similar to what @jeremydmiller uses in his blog posts (e.g. last code snippet here).

$ dotnet run --project src/Web/Web.csproj -- codegen preview
Building...
Searching 'JasperFx.CodeGeneration.Commands, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Weasel.CommandLine, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Marten.CommandLine, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Wolverine, Version=0.9.8.0, Culture=neutral, PublicKeyToken=null' for commands

No Wolverine extensions are detected

# => just hangs
$ dotnet run --project src/Web/Web.csproj -- codegen preview -v
Building...
Searching 'JasperFx.CodeGeneration.Commands, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Weasel.CommandLine, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Marten.CommandLine, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Wolverine, Version=0.9.8.0, Culture=neutral, PublicKeyToken=null' for commands

No Wolverine extensions are detected

Verbose flag is on.
[red]ERROR:[/]System.NotSupportedException: The IHost (Microsoft.AspNetCore.Builder.WebApplication) is already constructed. See https://jasperfx.github.io/oakton for
alternative bootstrapping to enable this feature.
  at IHostBuilder Oakton.PreBuiltHostBuilder.ConfigureServices(Action<HostBuilderContext, IServiceCollection> configureDelegate)
  at void Oakton.NetCoreInput.ApplyHostBuilderInput()
  at IHost Oakton.NetCoreInput.BuildHost()
  at bool JasperFx.CodeGeneration.Commands.GenerateCodeCommand.Execute(GenerateCodeInput input)
  at Task<bool> Oakton.OaktonCommand`1.Oakton.IOaktonCommand.Execute(object input)
  at Task<bool> Oakton.CommandRun.Execute()
  at async Task<int> Oakton.CommandExecutor.execute(CommandRun run)

This is probably related to JasperFx/oakton#75, so I tried the --launch-profile workaround.

$ dotnet run --project src/Web/Web.csproj --launch-profile Web -- resources check
Building...
Searching 'JasperFx.CodeGeneration.Commands, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Weasel.CommandLine, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Marten.CommandLine, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' for commands
Searching 'Wolverine, Version=0.9.8.0, Culture=neutral, PublicKeyToken=null' for commands

No Wolverine extensions are detected

[red]ERROR:[/]System.Security.VerificationException: Method Spectre.Console.AlignableExtensions.LeftAligned: type argument 'Spectre.Console.FigletText' violates the
constraint of type parameter 'T'.
  at async Task<bool> Oakton.Resources.ResourcesCommand.Execute(ResourceInput input)
  at void System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<TStateMachine>(ref TStateMachine stateMachine)
  at Task<bool> Oakton.Resources.ResourcesCommand.Execute(ResourceInput input)
  at Task<bool> Oakton.OaktonAsyncCommand`1.Oakton.IOaktonCommand.Execute(object input)
  at Task<bool> Oakton.CommandRun.Execute()
  at async Task<int> Oakton.CommandExecutor.execute(CommandRun run)

Using Paket

I prefer to use Paket for NuGet dependency management. There might be some issues with open version constraints, for example Spectre.Console >= 0.45 required by Oakton 6.0. Paket will always use the newest version that satisfies >= 0.45, as opposed to NuGet which preferred the oldest version that matches the version constraint the last time I used it.

The paket.lock file below does not contain packages starting with System. or Microsoft. (I removed them for brevity).

Spectre.Console 0.45 causes the same System.Security.VerificationException above.

RESTRICTION: == net7.0
NUGET
  remote: https://api.nuget.org/v3/index.json
    FastExpressionCompiler (3.3.4)
    Humanizer.Core (2.14.1)
    JasperFx.CodeGeneration (1.0.1)
      FastExpressionCompiler (>= 3.3.3)
      JasperFx.Core (>= 1.1)
    JasperFx.CodeGeneration.Commands (1.0.1)
      JasperFx.CodeGeneration (>= 1.0.1)
      JasperFx.RuntimeCompiler (>= 1.0.1)
      Oakton (>= 6.0)
    JasperFx.Core (1.1)
    JasperFx.RuntimeCompiler (1.0.1)
      JasperFx.CodeGeneration (>= 1.0.1)
    JasperFx.TypeDiscovery (1.0)
    Lamar (10.0.1)
      JasperFx.CodeGeneration (>= 1.0)
      JasperFx.Core (>= 1.1)
      JasperFx.TypeDiscovery (>= 1.0)
    Lamar.Microsoft.DependencyInjection (10.0.1)
      Lamar (>= 10.0.1)
    Marten (6.0.0-alpha.7)
      JasperFx.CodeGeneration (>= 1.0.1)
      JasperFx.Core (>= 1.1)
      JasperFx.RuntimeCompiler (>= 1.0.1)
      JasperFx.TypeDiscovery (>= 1.0)
      Newtonsoft.Json (>= 13.0.2)
      Npgsql.Json.NET (>= 7.0.1)
      Remotion.Linq (>= 2.2)
      Weasel.Postgresql (>= 6.0.0-alpha.8)
    Marten.CommandLine (6.0.0-alpha.7)
      JasperFx.CodeGeneration.Commands (>= 1.0.1)
      Marten (>= 6.0.0-alpha.7)
      Oakton (>= 6.0)
      Weasel.CommandLine (>= 6.0.0-alpha.8)
      Humanizer.Core (>= 2.14.1)
      Humanizer.Core (>= 2.14.1)
    Newtonsoft.Json (13.0.2)
    Npgsql (7.0.1)
    Npgsql.Json.NET (7.0.1)
      Newtonsoft.Json (>= 13.0.1)
      Npgsql (>= 7.0.1)
    Oakton (6.0)
      JasperFx.Core (>= 1.0)
      JasperFx.TypeDiscovery (>= 1.0)
      Spectre.Console (>= 0.45)
    Remotion.Linq (2.2)
    runtime.native.System (4.3.1)
    Spectre.Console (0.46)
    Swashbuckle.AspNetCore (6.5)
      Swashbuckle.AspNetCore.Swagger (>= 6.5)
      Swashbuckle.AspNetCore.SwaggerGen (>= 6.5)
      Swashbuckle.AspNetCore.SwaggerUI (>= 6.5)
    Swashbuckle.AspNetCore.Swagger (6.5)
    Swashbuckle.AspNetCore.SwaggerGen (6.5)
      Swashbuckle.AspNetCore.Swagger (>= 6.5)
    Swashbuckle.AspNetCore.SwaggerUI (6.5)
      runtime.native.System (>= 4.3)
    Weasel.CommandLine (6.0.0-alpha.8)
      Oakton (>= 6.0)
      Weasel.Core (>= 6.0.0-alpha.8)
    Weasel.Core (6.0.0-alpha.8)
      JasperFx.Core (>= 1.1)
    Weasel.Postgresql (6.0.0-alpha.8)
      Npgsql (>= 7.0.1)
      Weasel.Core (>= 6.0.0-alpha.8)
    WolverineFx (0.9.8)
      FastExpressionCompiler (>= 3.3.3)
      JasperFx.CodeGeneration.Commands (>= 1.0.1)
      JasperFx.Core (>= 1.1)
      JasperFx.RuntimeCompiler (>= 1.0.1)
      Lamar.Microsoft.DependencyInjection (>= 10.0.1)
      Newtonsoft.Json (>= 13.0.1)
      Oakton (>= 6.0)
    WolverineFx.Marten (0.9.8)
      Marten.CommandLine (>= 6.0.0-alpha.6)
      WolverineFx.Postgresql (>= 0.9.8)
    WolverineFx.Postgresql (0.9.8)
      Weasel.Postgresql (>= 6.0.0-alpha.6)
      WolverineFx.RDBMS (>= 0.9.8)
    WolverineFx.RDBMS (0.9.8)
      Weasel.CommandLine (>= 6.0.0-alpha.6)
      Weasel.Core (>= 6.0.0-alpha.6)
      WolverineFx (>= 0.9.8)
@agross
Copy link
Author

agross commented Feb 6, 2023

Another one came up. I'm using Pact to verify the contract between my API and an Angular client. I cannot use Alba because Pact requires a real TCP socket to make requests.

The fixture to spin up the website looks like this:

public class WebFixture : IAsyncLifetime
{
  readonly ITestOutputHelper _testOutputHelper;
  public readonly Uri BackendUri = new("http://localhost:9001");
  WebApplication? _app;

  public WebFixture(ITestOutputHelper testOutputHelper)
  {
    _testOutputHelper = testOutputHelper;
  }

  public Task InitializeAsync()
  {
    var builder = WebApplication.CreateBuilder(new WebApplicationOptions
    {
      EnvironmentName = "ProviderTests",
    });

    builder.Logging
           .Services
           .AddSingleton<ILoggerProvider>(_ => new XUnitLoggerProvider(_testOutputHelper));

    var startup = new StartupWrapper();
    startup.ConfigureServices(builder);

    _app = builder.Build();
    _app.Urls.Add(BackendUri.ToString());

    startup.ConfigureRequestPipeline(_app);

    return _app.StartAsync();
  }

  public Task DisposeAsync()
    => _app?.DisposeAsync().AsTask() ?? Task.CompletedTask;
}

What happens here is nothing special, just the normal bootstrapping and small additional services and middleware (ConfigureServices, ConfigureRequestPipeline) to allow Pact to inject "provider states" into the application.

The test is successful apart from this being logged:

Verifying a pact between WPM Admin and WPM API

  A request for all buildings
     Given There is 1 building
    returns a response which
      has status code 200 (OK)
      includes headers
        "Content-Type" with value "application/json" (OK)
      has a matching body (OK)


fail: Wolverine.Runtime.WolverineRuntime[0]
      Error while trying to stop DurabilityAgent
      System.ObjectDisposedException: Cannot access a disposed object.
      Object name: 'NpgsqlConnection'.
         at Npgsql.NpgsqlConnection.CreateCommand()
         at Npgsql.NpgsqlConnection.CreateDbCommand()
         at Weasel.Core.CommandExtensions.CreateCommand(DbConnection conn, String sql)
         at Wolverine.Postgresql.PostgresqlSettings.ReleaseGlobalLockAsync(DbConnection conn, Int32 lockId, CancellationToken cancellation, DbTransaction tx)
         at Wolverine.RDBMS.DurableStorageSession.ReleaseNodeLockAsync(Int32 lockId)
         at Wolverine.RDBMS.Durability.DurabilityAgent.StopAsync(CancellationToken cancellationToken)

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: < 1 ms - Web.ProviderTests.dll (net7.0)

@jeremydmiller
Copy link
Member

@agross Oh man, thank you so much for taking all this time. Might take a bit for me to get to think on this enough to respond, but I wanted you to know that I at least saw this come in.

@agross
Copy link
Author

agross commented Feb 7, 2023

Hi @jeremydmiller,

at the risk of flooding you with feedback (take your time!), I today tried to Wolverine-ify RBAC on the granularity of a command (still talking about IMessageBus).

I use KeyCloak for authentication and there is an excellent NuGet with an example to implement RBAC with a Mediatr behavior..

The code has a comment // TODO: consider reflection performance impact which I'm also after.

From my understanding, the Wolverine equivalent of IPipelineBehavior would be a middleware that would incur the same reflection cost per processed message. My idea was to make use of the code generation capability around the handler and pay that cost only once during setup.

using System.Runtime.CompilerServices;

using Application.Authorization;

using JasperFx.CodeGeneration;
using JasperFx.CodeGeneration.Frames;

using Lamar;

using Microsoft.Extensions.Logging;

using Wolverine.Configuration;
using Wolverine.Runtime.Handlers;

namespace Infrastructure.Authorization;

public class AuthorizationPolicy : IHandlerPolicy
{
  static readonly Dictionary<Type, string[]> MessageTypeToPolicies = new();

  public void Apply(HandlerGraph graph, GenerationRules rules, IContainer container)
  {
    foreach (var chain in graph.Chains)
    {
      Apply(chain);
    }
  }

  void Apply(HandlerChain chain)
  {
    var policies = chain.MessageType
                        .GetCustomAttributes(typeof(AuthorizeAttribute), true)
                        .Cast<AuthorizeAttribute>()
                        .Select(x => x.Policy)
                        .Where(x => !string.IsNullOrWhiteSpace(x))
                        .ToArray();

    if (!policies.Any())
    {
      return;
    }

    MessageTypeToPolicies.Add(chain.MessageType, policies!);

    var method = GetType()
                 .GetMethod(nameof(EnforcePolicy))
                 .MakeGenericMethod(chain.MessageType);

    var methodCall = new MethodCall(GetType(), method);

    chain.Middleware.Add(methodCall);
  }

  [MethodImpl(MethodImplOptions.AggressiveInlining)]
  public static async Task EnforcePolicy<T>(IIdentityService identityService, ILogger logger, T message)
  {
    var policies = MessageTypeToPolicies[message.GetType()];

    foreach (var policy in policies)
    {
      if (await identityService.AuthorizeAsync(policy))
      {
        continue;
      }

      logger.LogWarning("Failed {Policy} policy authorization with user {User} for {Message}",
                        policy,
                        identityService.UserId,
                        message);

      throw new UnauthorizedAccessException();
    }
  }
}

The Apply does the reflection bits and then injects a method call, like a middleware's Before(). But the Before() variant would always need to reflect, right?

As you can see the handler policy uses a dictionary to cache the roles per message, but my ultimate goal would be to pass the list of roles directly to EnforcePolicy, perhaps as the first argument.

I tried that for a couple of hours, even cloned JasperFx.CodeGeneration, but I could not find a way to make the first argument a constant expression. Perhaps another way is to register "required roles per message" in the container, but that seems a bit too overboard. It would be great if you could share your ideas!

@jeremydmiller
Copy link
Member

jeremydmiller commented Mar 2, 2023

Spawning #218 from this for the transactions

@jeremydmiller
Copy link
Member

I fixed the "Handlers with return values log FailureAcknowledgement" today, that came in from Discord as well.

@jeremydmiller
Copy link
Member

Spawning JasperFx/oakton#78 for the Oakton issue.

@jeremydmiller
Copy link
Member

Spawning #219 for the release global lock issue

@jeremydmiller
Copy link
Member

@agross,

I'm technically closing this, but only after spawning a bunch of smaller issues:-)

Thanks for all the feedback!

@agross
Copy link
Author

agross commented Mar 3, 2023

Thank you for all the work you put into your OSS! 👍

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

No branches or pull requests

2 participants