Adding EF core, database project, and some usage.

[dhoehna]

Summary of the pull request

Adds the DevHome.Database project that houses the dbcontext for interacting with the database.
Adds more logic to the RepositoryManagement page.
Adds some database usage.

1. Reading from the database
2. Writing to the database from the clone repo task.

References and relevant issues

Detailed description of the pull request / Additional comments

More documentation added in the future.

Validation steps performed

Manually ran.

PR checklist

• Closes #xxx
• Tests added/passed
• Documentation updated

Movies!
Cloning repos and showing them in the Repository Management Screen.

Removing a repository in FE and showing the change in the Repository Management page.

[dkbennett]

I have a few concerns here. The "Changes Requested" refers only to item 1 below, which is about the tests. The rest could be considered general feedback for future iterations.

Requested Changes

1. This really needs tests around the database functionality and the data objects within. No one should be able to cause a problem here that breaks the db or data model without a test failing and blocking the checkin. If we accidentally ship with such a db problem, that could create a cascade of data issues across many customers and we will forever have to deal with that possible state and recover from it. This absolutely needs tests that run as part of the CI validation to prevent this.

Things to consider not blocking

2. There is no schema control over the data, so it would be easy for someone to add columns or data and create a migration problem without realizing we have a migration problem. At the very least the schema needs to be versioned and I strongly recommend explicitly setting the schema and not letting Entity's Magic do it to where someone can easily and unknowingly change the schema and create a migration problem without knowing it.
    

3. There's very little logging about what Entity Framework is doing. I'm not a fan of Entity, as it does a little too much "magic" and not enough visibility. To resolve this while still using Entity, I think there needs to be a lot more try-catch around various things with appropriate logging added in so we can help diagnose a problem. We may get random exception and have a malformed db and no idea why or how it got that way.

4. In general I'm expecting that nearly anything with working with the DB could throw for filesystem reasons, database locked reasons, unique constraint violation, or any number of things could go wrong and we could get corrupt data outside of using it in a sqlite transaction and rolling back the change. Without the transactions corruption seems very likely, and without try-catches around many of these things this could be a random crash factory.

[dkbennett]

Not familiar with what magic EntityFramework might be doing, but SQLite has transaction support where you can ensure data integrity should something go wrong with any of this.

I don't see much in the way of try-catch blocks for things going wrong and I'm particularly nervous about not having fine control over the schema, transactions, etc to ensure data integrity.

[dhoehna]

but SQLite has transaction support

By default Entity Framework uses Transactions any SaveChange

try-catch blocks for things going wrong is put inside a transaction.

They will be added later. The purpose of this PR is to get the database structure and some usage examples into the feature branch. More try/catches will be added after this.

[dhoehna]

fine control over the schema, transactions, etc to ensure data integrity.

The schema is defined by the C# classes. We will have finer control over the schema. EF generates SQL scripts from migrations. When the schema is updated human eyes will review the generated script.

The migration strategy is as follows
During Dev-Inner loop. Use Migrations.
During a version change with a new schema: Generate a SQL script based on the migrations and get sign-off that the script is correct. Database migrations will use the script.

As I said in another comment, EF uses transactions by default.

What other concerns around data integrity do you have?

[dhoehna]

@dkbennett

This really needs tests around the database functionality

I will add tests for the current usage. I will add more in future PRs when I add implementations.

There is no schema control over the data

In this PR. Correct. This PR is for getting the database and some usage into the feature branch. I will add logic to check the schema version in a future PR. I promise. Further, Entity Frame does not have a method for detecting schema changes for packages.

There's very little logging about what Entity Framework is doing.

I will add more logging in future PRs (Before the feature gets into main).

working with the DB could throw

I did run into exceptions as well. I will add more try/catches in future PRs. This includes telemetry.

Another reason to add the database code in a PR with minimum try/catch and logging is to enable multiple branches to work on the database. Specifically when one PR is in review. I can work on another Repository Management feature while the other PR is in review.

[jsidewhite]