ErikEJ / EFCorePowerTools

Entity Framework Core Power Tools - reverse engineering, migrations and model visualization in Visual Studio & CLI
MIT License
2.19k stars 298 forks source link

Using CLI challenges #2587

Closed dyardy closed 1 month ago

dyardy commented 1 month ago

I am using vs.net with latest extension. I have been using ef power tools for a long time to reverse engineer sql databases. After running the extension it updates the efpt.config.json file within the project > good.

Now when I started using the CLI I had assumed it would take any settings that were done via the extension and re-use them. This is not true.

I then realized after running the CLI it created a efcpt-config.json file.

Now I am trying to modify this efcpt-config.json file to use the identical settings that the GUI was using.

I have a few questions

  1. In the GUI I am able to filter to only pick up a specific schema (how do I set the CLI to use a particular schema filter)

i.e. in the efpt.config.json I see

"FilterSchemas": true, "Schemas": [ { "Name": "ahsip" } ]

How can I do the same in the efcpt?

  1. If I add a new table via SQL am I also required to list the table in the efcpt-config.json file as well?
ErikEJ commented 1 month ago
  1. Yes, have a look at the readme https://github.com/ErikEJ/EFCorePowerTools/blob/master/src%2FCore%2Fefcpt.8%2Freadme.md

    1. No
dyardy commented 1 month ago

1 Thanks! Perfect

 "stored-procedures": [
    {
      "exclusionWildcard": "*"
    }
  ],
  "tables": [
    {
      "exclusionWildcard": "[dbo].*"
    },
    {
      "exclusionWildcard": "[HangFire].*"
    },
...

2 Great thank you!

Awesome tool. It would have been nice to be able to just rely on the GUI created config file (instead of a separate file for the CLI) (Or the option in the GUI to export out a CLI configuration file)

ErikEJ commented 1 month ago

I had to change the file format to make it usable in Tools like VS Code

ErikEJ commented 1 month ago

If you like my free tools, I would be very grateful for a rating/review on Visual Studio Marketplace and/or a one-time or monthly sponsorship

dyardy commented 1 month ago

When the CLI config has "use-nullable-reference-types": true,

you can see the result that it removes the .required() check on the property but it should still require this as the sql field is not null

2024-10-17_13-15-13

after i changed this property it puts in the appropriate IsRequired() to meet database scheme requirements (but it also made any other properties missing the ? nullable type properties

"use-nullable-reference-types": true,

`           

entity.Property(e => e.Name) .IsRequired() .HasMaxLength(200); `

Seems like one is using the new nullable properties when appropriate in c# but it is conflating it with sql schema requirements when you have a non null sql field.

ErikEJ commented 1 month ago

Please provide a text based reproduction, not scattered screen fragments, and I will have a look.

dyardy commented 1 month ago

I will try to simplify what I think is incorrect.

when "use-nullable-reference-types": true like follows

`{
   "CodeGenerationMode": 4,
   "ContextClassName": "DataDbContext",
   "ContextNamespace": null,
   "FilterSchemas": true,
   "IncludeConnectionString": false,
   "ModelNamespace": null,
   "OutputContextPath": "",
   "OutputPath": "Models",
   "PreserveCasingWithRegex": true,
   "ProjectRootNamespace": "DataAccess",
   "Schemas": [
      {
         "Name": "ahsip"
      }
   ],
   "SelectedHandlebarsLanguage": 0,
   "SelectedToBeGenerated": 0,
   "T4TemplatePath": null,
   "Tables": [
        ........removed.........
   ],
   "UiHint": null,
   "UncountableWords": null,
   "UseAsyncStoredProcedureCalls": true,
   "UseBoolPropertiesWithoutDefaultSql": false,
   "UseDatabaseNames": false,
   "UseDateOnlyTimeOnly": false,
   "UseDbContextSplitting": false,
   "UseDecimalDataAnnotationForSprocResult": true,
   "UseFluentApiOnly": true,
   "UseHandleBars": false,
   "UseHierarchyId": false,
   "UseInflector": true,
   "UseLegacyPluralizer": false,
   "UseManyToManyEntity": true,
   "UseNoDefaultConstructor": false,
   "UseNoNavigations": false,
   "UseNoObjectFilter": false,
   "UseNodaTime": false,
   **"UseNullableReferences": true,**
   "UsePrefixNavigationNaming": false,
   "UseSchemaFolders": false,
   "UseSchemaNamespaces": false,
   "UseSpatial": true,
   "UseT4": false
}`

After running cli, it will create generate entity configuration missing the IsRequired()

In SQL Server this field is is marked as not null

i.e. 2024-10-17_16-19-56

I am thinking this is incorrect. SQL Server is still setup to now allow nulls/empty values for this field and so IsRequired is still appropriate.

ErikEJ commented 1 month ago

Please provide a TEXT based CREATE statement for the involved sample table

ErikEJ commented 1 month ago

IsRequired is most likely not needed, as there is a difference now between string and string?

dyardy commented 1 month ago

If the field (nvarchar) in sql server is Not Null would the entity configuration also need IsRequired() ?

ErikEJ commented 1 month ago

@dyardy only the non default values are scaffolded to avoid code bloat.

Please provide a full text based reproduction snd I will have a closer look.

dyardy commented 1 month ago

When the SQL Server field is marked as not null > I would expect the generator to provide code like the following.

modelBuilder.Entity<YourEntity>()
    .Property(e => e.YourFieldName)
    .IsRequired()   // Ensures that the column is not nullable
    .HasColumnType("nvarchar");  // Specifies that the SQL type should be nvarchar

When I use this property
use-nullable-reference-types": true

the generated code looks like (notice the IsRequired() is missing but it should be present

modelBuilder.Entity<YourEntity>()
    .Property(e => e.YourFieldName)   
    .HasColumnType("nvarchar");  // Specifies that the SQL type should be nvarchar
ErikEJ commented 1 month ago

Please provide the CREATE table statement

dyardy commented 4 weeks ago

the create sql exactly, if you are testing it is about the Name column that is setup in the db as not null (required)

`/** Object: Table [ahsip].[FastFieldLookUpLists] Script Date: 10/22/2024 4:20:32 PM **/ SET ANSI_NULLS ON GO

SET QUOTED_IDENTIFIER ON GO

CREATE TABLE [ahsip].[FastFieldLookUpLists]( [ListId] [int] IDENTITY(1,1) NOT NULL, [ProjectId] [int] NOT NULL, [Name] nvarchar NOT NULL, [LookupId] nvarchar NULL, [LastUpdated] [datetime] NULL, [UpdatedBy] [int] NULL, CONSTRAINT [PK_FastFieldLookUpLists] PRIMARY KEY CLUSTERED ( [ListId] ASC )WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY] ) ON [PRIMARY] GO

ALTER TABLE [ahsip].[FastFieldLookUpLists] WITH CHECK ADD CONSTRAINT [FK_FastFieldLookups_Projects] FOREIGN KEY([ProjectId]) REFERENCES [ahsip].[Projects] ([ProjectId]) GO

ALTER TABLE [ahsip].[FastFieldLookUpLists] CHECK CONSTRAINT [FK_FastFieldLookups_Projects] GO

ALTER TABLE [ahsip].[FastFieldLookUpLists] WITH CHECK ADD CONSTRAINT [FK_FastFieldLookups_Users] FOREIGN KEY([UpdatedBy]) REFERENCES [ahsip].[Users] ([UserId]) GO

ALTER TABLE [ahsip].[FastFieldLookUpLists] CHECK CONSTRAINT [FK_FastFieldLookups_Users] GO

`

ErikEJ commented 4 weeks ago

@dyardy IsRequired is not needed in the generated code, as a non-nullable string is always required.