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

ToDoMVC needs fixing and upgrading #76

Open
goblinfactory opened this issue Jan 19, 2020 · 7 comments
Open

ToDoMVC needs fixing and upgrading #76

goblinfactory opened this issue Jan 19, 2020 · 7 comments
Assignees
Labels

Comments

@goblinfactory
Copy link
Collaborator

goblinfactory commented Jan 19, 2020

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.
@rofr
Copy link
Member

rofr commented Jan 19, 2020

@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
Copy link
Collaborator Author

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
Copy link
Member

rofr commented Jan 19, 2020

Looks legit besides the duplicated services.AddSingleton!

@goblinfactory
Copy link
Collaborator Author

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

@rofr
Copy link
Member

rofr commented May 15, 2020

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

@rofr
Copy link
Member

rofr commented May 15, 2020

a709ea5

@goblinfactory
Copy link
Collaborator Author

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

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

No branches or pull requests

2 participants