memstate icon indicating copy to clipboard operation
memstate copied to clipboard

ToDoMVC needs fixing and upgrading

Open goblinfactory opened this issue 6 years ago • 7 comments

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:

  • show how memstate can be used to reduce code bloat and avoid EntityFramework proliferation :D
  • upgrade sample project to .NET core 3.1 LTS
  • add default routing so that when user F5's you get a decent home page as default and dont think that something is wrong
  • remove the boilerplate starter pages and replace with a minimalist TODO MVC pages.
  • add a bit of welcome to memstate TODO example text, in the body of the home page that opens to explain how to modify the appsettings.json to add a Memstate configuration and Npgsql journal provider for postgres, if you want to try that out. The default (without this setting) will be Memstate.JsonNet, which will create a journal file ...etc etc..

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;"
                }
            }
        }
    }
  • Lastly, if we are supplying an MVC application that we need to know works, then we should include some form of end to end web acceptance test to prove that it's running as expected. We can easily do that with a few lines of powershell pester as part of the build, rather than complicating the unit tests? imho. I can help with that.

goblinfactory avatar Jan 19 '20 14:01 goblinfactory

@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.

rofr avatar Jan 19 '20 20:01 rofr

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.

goblinfactory avatar Jan 19 '20 21:01 goblinfactory

Looks legit besides the duplicated services.AddSingleton!

rofr avatar Jan 19 '20 21:01 rofr

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

goblinfactory avatar Jan 19 '20 22:01 goblinfactory

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

rofr avatar May 15 '20 18:05 rofr

a709ea5f350885955ad8e878f9dc1c11742f6e36

rofr avatar May 15 '20 18:05 rofr

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

goblinfactory avatar May 26 '20 21:05 goblinfactory