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

Pascal-case the DbContext name when scaffolding #27886

Closed
vanillajonathan opened this issue Apr 25, 2022 · 8 comments · Fixed by #27966
Closed

Pascal-case the DbContext name when scaffolding #27886

vanillajonathan opened this issue Apr 25, 2022 · 8 comments · Fixed by #27966
Assignees
Labels
area-scaffolding area-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone

Comments

@vanillajonathan
Copy link
Contributor

When scaffolding from a SQLite database using the dotnet ef tool it will create the DbContext with the initial lowercase if the database filename is lowercase.

Example:

dotnet ef dbcontext scaffold "DataSource=blogging.db;Cache=Shared" Microsoft.EntityFrameworkCore.Sqlite

It creates the DbContext with the filename bloggingContext.cs and the class name bloggingContext which is rather ugly.

Running dotnet ef dbcontext scaffold --help it states:

-c|--context <NAME>                    The name of the DbContext. Defaults to the database name.

But my expectation was that it would still create the filename and the class name with the initial letter uppercase as is the .NET convention.

EF Core version: 6.0.4
Database provider: (e.g. Microsoft.EntityFrameworkCore.Sqlite)
Target framework: (e.g. .NET 6.0)

@ajcvickers
Copy link
Contributor

@vanillajonathan The filename should not have any impact here. Instead, the database name defined in the SQLite database is being used for the context name. As you state, the context name can be specified if you want something different, including different casing. Beyond that templates (#4038) will allow this to be changed if you choose.

@vanillajonathan
Copy link
Contributor Author

Ah, I see.
Perhaps it should always use uppercase the first letter of the filename and the class name regardless of the name defined in the SQLite database?

@bricelam
Copy link
Contributor

bricelam commented Apr 28, 2022

Nope, it actually does come directly from the file name. Technically, the database name is (almost) always main in SQLite. I agree that we should scrub these to make them more idiomatic to .NET.

@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label Apr 28, 2022
@ilmalte
Copy link
Contributor

ilmalte commented Apr 29, 2022

Hey I have seen the label, may I try to work on this? @bricelam

@bricelam
Copy link
Contributor

bricelam commented Apr 29, 2022

Sure! It’s probably easiest to do it here:

modelBuilder.Model.SetDatabaseName(databaseModel.DatabaseName);

And use _candidateNamingService to clean up the name when _options.UseDatabaseNames is true false.

But you might also be able to do it here: 🤷‍♂️

? _code.Identifier(annotatedName + DbContextSuffix)

@bricelam
Copy link
Contributor

bricelam commented Apr 29, 2022

…or here:

var name = Path.GetFileNameWithoutExtension(connection.DataSource);

@ajcvickers ajcvickers added this to the Backlog milestone May 2, 2022
@ilmalte
Copy link
Contributor

ilmalte commented May 2, 2022

Thanks for your suggestion @bricelam!

Do you think could be a good idea to keep the file name as input or is it better to pick up the database name instead?

In case you prefer the first option, if this problem is only related to SQLite, I would go for the file SqliteDatabaseModelFactory.cs since it's in the project EFCore.Sqlite.Core and not in EFCore.Design:

var name = Path.GetFileNameWithoutExtension(connection.DataSource);

My approach would be to capitalize the first letter (of the value) of the variable called name.

To achieve this, I would do something like:

    private static string GetDatabaseName(string? str)

    {

        var name = Path.GetFileNameWithoutExtension(str);

        if (!string.IsNullOrEmpty(name))

        {

            //name = char.ToUpper(name[0]) + name.Substring(1);

            name = char.ToUpper(name[0]) + name[1..];

        }

 

        if (string.IsNullOrEmpty(name))

        {

            name = "Main";

        }

        return name;

    }

Do you think is better to create a method for capitalization? If so, where?

@bricelam
Copy link
Contributor

bricelam commented May 2, 2022

I don't see this issue being specific to SQLite. If a SQL Server database was named blogging, it would also generate bloggingContext. So it probably shouldn't be fixed just inside SqliteDatabaseModelFactory.

The CandidateNamingService.GenerateCandidateIdentifier method has logic to transform strings into Pascal case--it capitalizes the first letter of each word and remove things like underscores and hyphens. I'd reuse that.

@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 18, 2022
@bricelam bricelam modified the milestones: Backlog, 7.0.0, 7.0.0-preview5 May 18, 2022
@ajcvickers ajcvickers changed the title Scaffolding creates lowercase DbContext Pascal-case the DbContext name when scaffolding Jun 23, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview5, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-scaffolding area-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported good first issue This issue should be relatively straightforward to fix. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants