DevrexLabs / memstate

In-memory event-sourced ACID-transactional distributed object graph engine for .NET Standard
https://memstate.io
Other
365 stars 46 forks source link

ToDoMVC needs fixing and upgrading #76

Open goblinfactory opened 4 years ago

goblinfactory commented 4 years ago

Hi @rofr what's the status of the TODO MVC Sample app?

does that need to be upgraded to .net core 3.1? What's it's purpose? I couldnt get it to run without making a few small tweaks

As the code currently stands, when you run, Kestrel starts, but the memstate builder never builds the singleton instance. If you navigate to http://localhost:12202/lists you get the following error

Screen Shot 2020-01-19 at 14 31 35

to fix this I had to change the ConfigureServices as follows, from

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddMvc();

            services.AddSingleton(async provider =>
            {
                return await new EngineBuilder().Build<TodoModel>().ConfigureAwait(false);
            });
        }

change to

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddMvc();
           // I trust you had a good reason for the original async code above, so 
           // I am suspicious that this fix is too simplistic, would appreciate your comments
           // on the original code, and I can capture the rationale in docs if we need to explain to users
            services.AddSingleton(services.AddSingleton(new EngineBuilder().Build<TodoModel>().GetAwaiter().GetResult()));
        }

Also, if you're not running postgres you get an error initialising the serializer, so it's best to add Memstate.JsonNet package

 <ProjectReference Include="..\Memstate.JsonNet\Memstate.JsonNet.csproj" />

so that the only sample project that users see when they clone the project and press F5 will at least start and run. It's not an unreasonable expectation, which brings me back to wanting to know the purpose of the sample project? If we can define the purpose then I can make some changes to help deliver on that goal.

Some idea:

Add this to appsettings.json if you want to use postgres or if you want to use the Memstate docker containers.

    "Memstate": {
        "Journal": {
            "Providers": {
                "Npgsql": {
                    "ConnectionString": "Server=localhost;Username=postgres;Password=postgres;Database=memstate;"
                }
            }
        }
    }
rofr commented 4 years ago

@hagbarddenstore did all of the work on this example project. The purpose was two-fold. One - an example to work on in parallel with library to get the user perspective, but then of course to serve as a general working example without anything specific in mind.

It surely needs some love, feel free to do what you see fit! I really like the idea of integrating some advice/documentation into the views.

Also, related to #79 which suggests adding an ISerializer based on System.Json to the core library and making it the default.

goblinfactory commented 4 years ago

Ah, great. Happy to upgrade this to 3.1 LTS. and put in some docs. quick question, can you confirm this line is ok?

services.AddSingleton(services.AddSingleton(new EngineBuilder().Build<TodoModel>().GetAwaiter().GetResult()));

then I can start looking at a very simple update to the project.

rofr commented 4 years ago

Looks legit besides the duplicated services.AddSingleton!

goblinfactory commented 4 years ago

haha...typo from copy and paste, doh! :D

rofr commented 4 years ago

I threw the todo example away and created a Trello clone instead using .NET Core 3.1

rofr commented 4 years ago

a709ea5f350885955ad8e878f9dc1c11742f6e36

goblinfactory commented 4 years ago

I will be looking at this on Thursday. I have take Thursday and Friday off to do some open source coding.