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

Add support for persistent state facets #55

Closed
seniorquico opened this issue Mar 31, 2019 · 11 comments · Fixed by #105
Closed

Add support for persistent state facets #55

seniorquico opened this issue Mar 31, 2019 · 11 comments · Fixed by #105
Assignees
Labels

Comments

@seniorquico
Copy link
Collaborator

seniorquico commented Mar 31, 2019

Orleans 2.3.0 introduced a persistent state facet. With the new system, grain classes do not need to extend Grain<TGrainState> to use a built-in storage provider (they still need to extend Grain). This essentially trades the State property for constructor arguments passed via dependency injection. Unlike the single State property of Grain<TGrainState>, the constructor may take multiple state arguments. Additionally, each state argument may be mapped to different storage providers. Unfortunately, official docs on the new system aren't yet published.

Here's a simple example of a grain with a single state object (the "normal" pattern for a Grain<TGrainState> class):

public sealed class ColorGrainWithFacet : Grain, IColorGrain
{
    public ColorGrainWithFacet(
        [PersistentState("State")] IPersistentState<ColorGrainState> state)
    {
        this.state = state;
    }

    ...

Here's an advanced example of a grain with multiple state objects obtained from different storage providers:

public sealed class ColorGrainWithFacet : Grain, IColorGrain
{
    public ColorGrainWithFacet(
        [PersistentState("State")] IPersistentState<ColorGrainState> state, // default storage provider
        [PersistentState("Counters", "DifferentProvider")] IPersistentState<CounterGrainState> counters)
    {
        this.state = state;
        this.counters = counters;
    }

    ...

There are some significant implementation differences, and the new system doesn't currently work with the test kit's CreateGrainAsync method and its StorageManager.

First, Orleans has special logic to create and inject IPersistentState<TGrainState> arguments using a factory associated with the [PersistentState] attribute. It doesn't work to simply add an IPersistentState<TState> to our dependency injection container before calling CreateGrainAsync. Here's a standlone example (based on Moq) that identifies the required services:

// Arrange
TestKitSilo silo = ...;
TGrainState state = ...;

var mockState = new Mock<IPersistentState<TGrainState>>();
mockState.SetupGet(o => o.State).Returns(state);

var mockMapper = new Mock<IAttributeToFactoryMapper<PersistentStateAttribute>>();
mockMapper.Setup(o => o.GetFactory(It.IsAny<ParameterInfo>(), It.IsAny<PersistentStateAttribute>())).Returns(context => mockState.Object);

silo.AddService(mockMapper.Object);

// Act
var grain = await this.silo.CreateGrainAsync<TGrain>(...);
...

// Assert
...

Second, the default Orleans factory associated with the [PersistentState] attribute works with storage providers pulled directly from the dependency injection container. Unlike Grain<TGrainState>, the new system never calls IGrainRuntime.GetStorage<TGrainState>. The new system simply bypasses the test kit's StorageManager in tests.

Finally, as noted above, the new system supports multiple state objects per grain instance. This doesn't work with the the test kit's StorageManager which assumes there is at most one state object per grain instance.

The test kit should provide new utilities for unit tests to work with persistent state facets. I have some WIP tests shaping up in my fork if anyone else is currently looking at testing persistent state facets.

@tomodutch
Copy link

I'm working on a project that uses IPersistentState so I'm curious how far you're along with this implementation.

@seniorquico
Copy link
Collaborator Author

@TFarla I haven't added any helpers to the TestKit. However, you can still test grains that leverage the facet system with a little setup work. I added a unit test sample with #83 that demonstrates using the TestKit to inject the desired IPersistentState. Specifically, look at the new StorageFacetTests.cs class file.

@PepijnK
Copy link

PepijnK commented Jun 19, 2020

For anyone Googling for this issue like me, here is the actual error that you get when assuming the CreateGrainAsync just works:

System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
---- Orleans.Runtime.OrleansException : Attribute mapper Castle.Proxies.IAttributeToFactoryMapper`1Proxy failed to create a factory for grain type FooBarGrain.

@enewnham
Copy link
Contributor

enewnham commented Jan 28, 2021

Here is a helper extension method we are using

        public static T AddPersistentState<T>( this TestKitSilo silo, T state )
        {
            var mockState = new Mock<IPersistentState<T>>( );
            mockState.SetupGet( o => o.State ).Returns( state );

            var mockMapper = new Mock<IAttributeToFactoryMapper<PersistentStateAttribute>>( );
            mockMapper.Setup( o => o.GetFactory( It.IsAny<ParameterInfo>( ), It.IsAny<PersistentStateAttribute>( ) ) ).Returns( _ => mockState.Object );

            silo.AddService( mockMapper.Object );

            return state;
        }

        // And how it is used
        public class MyTest : TestKitBase
        {
            private readonly MyState _state = new MyState( );

            public MyTest( )
            {
                Silo.AddPersistentState( _state )
            }
        }

@Romanx
Copy link
Contributor

Romanx commented Mar 9, 2021

Thanks to @enewnham I adapted their extension to make it more of a wrapper allowing the values to be set during the processing of the grain and updated during processing. If any of TestKit maintainers are interested i'd be happy to submit a PR to make this offical.

using System;
using System.Reflection;
using System.Threading.Tasks;
using Moq;
using Orleans.Core;
using Orleans.Runtime;

namespace Orleans.TestKit
{
    public static class TestKitExtensions
    {
        public static IPersistentState<T> AddPersistentState<T>(this TestKitSilo silo, T? state = default)
            where T : new()
        {
            var stateMock = new PersistentStateMock<T>(silo.StorageManager.GetStorage<T>())
            {
                State = state ?? Activator.CreateInstance<T>(),
            };

            var mockMapper = new Mock<IAttributeToFactoryMapper<PersistentStateAttribute>>();
            mockMapper.Setup(o => o.GetFactory(It.IsAny<ParameterInfo>(), It.IsAny<PersistentStateAttribute>())).Returns(_ => stateMock);

            silo.AddService(mockMapper.Object);

            return stateMock;
        }

        private class PersistentStateMock<TState> : IPersistentState<TState>
        {
            private readonly IStorage<TState> storage;

            public PersistentStateMock(IStorage<TState> storage)
            {
                this.storage = storage;
            }

            public TState State
            {
                get => this.storage.State;
                set => this.storage.State = value;
            }

            public string Etag => this.storage.Etag;

            public bool RecordExists => this.storage.RecordExists;

            public Task ClearStateAsync() => this.storage.ClearStateAsync();

            public Task ReadStateAsync() => this.storage.ReadStateAsync();

            public Task WriteStateAsync() => this.storage.WriteStateAsync();
        }
    }
}

@seniorquico
Copy link
Collaborator Author

A PR would be awesome!

One complexity with the state facets is that you may inject multiple, different instances. It would be great if that were accommodated.

@Romanx
Copy link
Contributor

Romanx commented Mar 11, 2021

@seniorquico I've opened a PR with some suggested changes that should allow multiple state facets to be registered based on state name and storage name. I had to make some other changes around storage which hopefully are clear

@runenilsenoe
Copy link

Any updates on this? @Romanx

@Romanx
Copy link
Contributor

Romanx commented May 12, 2022

Sorry about the delay here, i've not had a chance to take a look. Let me try bring this into a reasonable state today.

@Romanx
Copy link
Contributor

Romanx commented May 12, 2022

Okay brought the branch into line with main and it should be functionally there although there are more external changes than I would have wanted.

@runenilsenoe
Copy link

@seniorquico any updates on this?

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

Successfully merging a pull request may close this issue.

6 participants