dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.08k stars 2.03k forks source link

Feature - Allow grain type name to be set at startup #8846

Closed michaelmcneilnet closed 8 months ago

michaelmcneilnet commented 8 months ago

https://learn.microsoft.com/en-us/dotnet/orleans/grains/grain-identity#grain-type-names

The grain type name can only be set via an attribute, which requires a constant string. As the author of a library that includes grains, I would like to allow my users to set the grain type name of the library grains so they don't clash with my customers' grains.

ReubenBond commented 8 months ago

This is possible today, but it's not ergonomic. The way you do it is by creating an implementation of Orleans.Metadata.IGrainTypeProvider and registering that in the container, like this:

public interface IMyGrain : IGrainWithStringKey {}

public class MyGrain : IMyGrain { }

public class MyLibraryOptions
{
    public string GrainTypeName { get; set; } = "my-default-grain-type";
}

internal class MyGrainTypeProvider(IOptions<MyLibraryOptions> options) : IGrainTypeProvider
{
    public bool TryGetGrainType(Type type, out GrainType grainType)
    {
        if (type == typeof(MyGrain))
        {
            grainType = GrainType.Create(options.Value.GrainTypeName);
            return true;
        }

        grainType = default;
        return false;
    }
}

Register it in DI:

services.AddSingleton<IGrainTypeProvider, MyGrainTypeProvider>();

Verify that it worked:

var grain = grainFactory.GetGrain<IMyGrain>("test");
var grainTypeName = grain.GetGrainId().Type.ToString();
Assert.Equal("my-default-grain-type", grainTypeName);

I have not tested this code, so please let us know if that works. I'm interested to learn more about your use case, too.

In the source, the responsible code is here: https://github.com/dotnet/orleans/blob/4166e7804d82b974b5be32797af27bb68ebe9895/src/Orleans.Core/Manifest/GrainTypeResolver.cs#L50

michaelmcneilnet commented 8 months ago

Ah nice, thanks Reuben! I asked in the Discord yesterday and got an answer about manipulating source generated code, so I assumed it wasn't possible out of the box 😅

Use Case I wrote a SignalR backplane library a while back with a UserGrain to track each connected user. I am writing a game that uses this backplane, which also has a UserGrain, for each game player.

When I run the game server, I get

System.InvalidOperationException: 'An entry with the key user is already present.
Existing: ["type-name": "UserGrain", "interface.0": "X.Grains.IUserGrain", "full-type-name": "X.Grains.UserGrain", "interface.1": "Orleans.IGrainWithStringKey", "diag.asm": "X", "diag.type": "X.Grains.UserGrain,X"]
Trying to add: ["interface.3": "TheLibrary.Abstractions.IUserGrain", "type-name": "UserGrain", "interface.0": "TheLibrary.Grains.ISignalrGrain", "full-type-name": "TheLibrary.Grains.UserGrain", "interface.1": "Orleans.IGrainWithStringKey", "interface.2": "Orleans.IRemindable", "diag.asm": "TheLibrary", "diag.type": "TheLibrary.Grains.UserGrain,TheLibrary"]
Consider using the [GrainType("name")] attribute to give these classes unique names.'

(X is my game and TheLibrary is the signal backplane library, I redacted yesterday in Discord out of habit)

I want the library to expose a way to set the grain names of its internal grains, so I can override it to signalrusergrain from my game server.

Does this make sense or am I misunderstanding the error/going about the fix the wrong way?

ReubenBond commented 8 months ago

@michaelmcneilnet in this case, you likely also want to make sure to override the interface type name (eg, IUserGrain), so you'd do something very similar to the above for a IGrainInterfaceTypeProvider implementation.

You could also consider namespacing your type names using the attributes, eg, [GrainType("my-lib.user")] & [GrainInterfaceType("my-lib.IUserGrain")]

michaelmcneilnet commented 8 months ago

That's a good point about the interface, too.

I thought about setting them in the library directly with attributes, but I was worried about any possible breaking changes for existing library consumers. Can you think of any issues doing this?

ReubenBond commented 8 months ago

You're right, this would be a breaking change if the references are persisted or if a rolling upgrade is performed (i.e, the usual upgrade strategy).

michaelmcneilnet commented 8 months ago

I think what I'll do is use your code above, and set the default going forward to be the fully namespaced name. That way, existing users can override this to keep the original name to avoid the breaking change.

I will update this thread when it's done to let you know if the code above worked as expected. If it does, might it be worth adding it to the docs in my original post as an alternative to the attribute?

michaelmcneilnet commented 8 months ago

@ReubenBond Your code worked perfectly (you can see PR here if you are interested https://github.com/unifiedfx/UFX.Orleans.SignalRBackplane/pull/13)

I think it would be good to have IGrainTypeProvider and IGrainInterfaceTypeProvider documented somewhere, as they are really useful!