msawczyn / EFDesigner2022

Entity Framework visual design surface and code-first code generation for EF6, Core and beyond
MIT License
119 stars 21 forks source link

The navigation properties were removed after updating to EFDesigner2022 from EFDesigner2019 #41

Closed adar2 closed 1 year ago

adar2 commented 1 year ago

Hi, First of all you have great extension!. Since I switched to VS2022 instead of VS2019, I had to use the new version of this extension. When I first added a model using the extension, the DbContext that was generated did not have all the navigation properties that the old version did, which caused me some problems. Is this a known issue, or are those properties no longer needed? Is there something I'm missing?

msawczyn commented 1 year ago

No, the code should be pretty much the same, and the navigation properties should certainly be there. Which version of the 2022 designer are you using (current version on Marketplace is 4.2.1.3).

adar2 commented 1 year ago

currently 4.1.2.0

msawczyn commented 1 year ago

Hmm. Haven't seen that happen. Could you please post a small sample .efmodel file that shows the problem?

adar2 commented 1 year ago

just to be clear i'm attaching here this photo that shows what happens when the code is generated.. image

msawczyn commented 1 year ago

Thanks for that. In order to determine what's going on, I'd need an .efmodel file. I understand not wanting to share the .efmodel that produced the code above, but if you can get the same behavior to occur with a simpler file, say with 2 entities and a bidirectional association between them (with the same cardinality on both sides as your production model), I'd be able to see why that's happening.

Note that the navigation properties are indeed being specified. I see TenantInfo.ResourceDefs and ResourceDef.TenantInfo in a bidirectional, one-to-many association (TenantInfo 1 <--> * ResourceDef). The delete behavior isn't generated, but it shouldn't be - specifying NoAction will cause tons of hard-to-find SQL exceptions when the necessary objects aren't in memory. Since it's required, when the TenantInfo is deleted, you most definitely need to have the associated ResourceDefs deleted. If this isn't what you want done, you'll need to change the cardinality to 0..1 - the database definitely complain otherwise.

It looks like, in VS2019, the relationship was 0/1..N rather than 1..N. Did that get changed? The IsRequired call gets generated when there's a required association; if 0/1, it's optional, so it wouldn't get generated.

Unfortunately, that's the most I can see without an .efmodel file to go by.

adar2 commented 1 year ago

Hi, i'm attaching zip with .efmodel and the generated .cs file. EFModel1.zip

msawczyn commented 1 year ago

Thanks for that. I opened that file and ran code generation on it. Things look right.

The DbContext-derived class created

namespace iBI.CCR.Orchestrator.Benchmark
{
   /// <inheritdoc/>
   public partial class EFModel1 : DbContext
   {
      #region DbSets
      public virtual Microsoft.EntityFrameworkCore.DbSet<global::iBI.CCR.Orchestrator.Benchmark.Entity1> Entity1 { get; set; }
      public virtual Microsoft.EntityFrameworkCore.DbSet<global::iBI.CCR.Orchestrator.Benchmark.Entity2> Entity2 { get; set; }

      #endregion DbSets

      /// <summary>
      ///     <para>
      ///         Initializes a new instance of the <see cref="T:Microsoft.EntityFrameworkCore.DbContext" /> class using the specified options.
      ///         The <see cref="M:Microsoft.EntityFrameworkCore.DbContext.OnConfiguring(Microsoft.EntityFrameworkCore.DbContextOptionsBuilder)" /> method will still be called to allow further
      ///         configuration of the options.
      ///     </para>
      /// </summary>
      /// <param name="options">The options for this context.</param>
      public EFModel1(DbContextOptions<EFModel1> options) : base(options)
      {
      }

      partial void CustomInit(DbContextOptionsBuilder optionsBuilder);

      /// <inheritdoc />
      protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
      {
         optionsBuilder.UseLazyLoadingProxies();

         CustomInit(optionsBuilder);
      }

      partial void OnModelCreatingImpl(ModelBuilder modelBuilder);
      partial void OnModelCreatedImpl(ModelBuilder modelBuilder);

      /// <summary>
      ///     Override this method to further configure the model that was discovered by convention from the entity types
      ///     exposed in <see cref="T:Microsoft.EntityFrameworkCore.DbSet`1" /> properties on your derived context. The resulting model may be cached
      ///     and re-used for subsequent instances of your derived context.
      /// </summary>
      /// <remarks>
      ///     If a model is explicitly set on the options for this context (via <see cref="M:Microsoft.EntityFrameworkCore.DbContextOptionsBuilder.UseModel(Microsoft.EntityFrameworkCore.Metadata.IModel)" />)
      ///     then this method will not be run.
      /// </remarks>
      /// <param name="modelBuilder">
      ///     The builder being used to construct the model for this context. Databases (and other extensions) typically
      ///     define extension methods on this object that allow you to configure aspects of the model that are specific
      ///     to a given database.
      /// </param>
      protected override void OnModelCreating(ModelBuilder modelBuilder)
      {
         base.OnModelCreating(modelBuilder);
         OnModelCreatingImpl(modelBuilder);

         modelBuilder.HasDefaultSchema("dbo");

         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.Entity1>()
                     .ToTable("Entity1")
                     .HasKey(t => t.Id);
         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.Entity1>()
                     .Property(t => t.Id)
                     .ValueGeneratedOnAdd()
                     .IsRequired();
         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.Entity1>()
                     .HasMany<global::iBI.CCR.Orchestrator.Benchmark.Entity2>(p => p.Entity2)
                     .WithMany(p => p.Entity1)
                     .UsingEntity(x => x.ToTable("Entity2_Entity1_x_Entity1_Entity2"));

         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.Entity2>()
                     .ToTable("Entity2")
                     .HasKey(t => t.Id);
         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.Entity2>()
                     .Property(t => t.Id)
                     .ValueGeneratedOnAdd()
                     .IsRequired();

         OnModelCreatedImpl(modelBuilder);
      }
   }
}

and the two entities are

namespace iBI.CCR.Orchestrator.Benchmark
{
   public partial class Entity1
   {
      partial void Init();

      /// <summary>
      /// Default constructor
      /// </summary>
      public Entity1()
      {
         Entity2 = new System.Collections.Generic.HashSet<global::iBI.CCR.Orchestrator.Benchmark.Entity2>();

         Init();
      }

      /*************************************************************************
       * Properties
       *************************************************************************/

      /// <summary>
      /// Identity, Indexed, Required
      /// Unique identifier
      /// </summary>
      [Key]
      [Required]
      [System.ComponentModel.Description("Unique identifier")]
      public long Id { get; set; }

      /*************************************************************************
       * Navigation properties
       *************************************************************************/

      public virtual ICollection<global::iBI.CCR.Orchestrator.Benchmark.Entity2> Entity2 { get; private set; }

   }
}

and

namespace iBI.CCR.Orchestrator.Benchmark
{
   public partial class Entity2
   {
      partial void Init();

      /// <summary>
      /// Default constructor
      /// </summary>
      public Entity2()
      {
         Entity1 = new System.Collections.Generic.HashSet<global::iBI.CCR.Orchestrator.Benchmark.Entity1>();

         Init();
      }

      /*************************************************************************
       * Properties
       *************************************************************************/

      /// <summary>
      /// Identity, Indexed, Required
      /// Unique identifier
      /// </summary>
      [Key]
      [Required]
      [System.ComponentModel.Description("Unique identifier")]
      public long Id { get; set; }

      /*************************************************************************
       * Navigation properties
       *************************************************************************/

      public virtual ICollection<global::iBI.CCR.Orchestrator.Benchmark.Entity1> Entity1 { get; private set; }

   }
}

Are you getting something different?

Note that the model you forwarded doesn't have the same characteristics as the code you posted above. The original is a 0.1..N association, whereas this one is an N..N association.

If I change it to be image

then I get an OnModelCreating of

      protected override void OnModelCreating(ModelBuilder modelBuilder)
      {
         base.OnModelCreating(modelBuilder);
         OnModelCreatingImpl(modelBuilder);

         modelBuilder.HasDefaultSchema("dbo");

         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.ResourceDef>()
                     .ToTable("ResourceDefs")
                     .HasKey(t => t.Id);
         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.ResourceDef>()
                     .Property(t => t.Id)
                     .ValueGeneratedOnAdd()
                     .IsRequired();

         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.TenantInfo>()
                     .ToTable("TenantInfoes")
                     .HasKey(t => t.Id);
         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.TenantInfo>()
                     .Property(t => t.Id)
                     .ValueGeneratedOnAdd()
                     .IsRequired();
         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.TenantInfo>()
                     .HasMany<global::iBI.CCR.Orchestrator.Benchmark.ResourceDef>(p => p.ResourceDefs)
                     .WithOne(p => p.TenantInfo)
                     .HasForeignKey("TenantInfoId")
                     .IsRequired();

         OnModelCreatedImpl(modelBuilder);
      }

and the entities become

namespace iBI.CCR.Orchestrator.Benchmark
{
   public partial class TenantInfo
   {
      partial void Init();

      /// <summary>
      /// Default constructor
      /// </summary>
      public TenantInfo()
      {
         ResourceDefs = new System.Collections.Generic.HashSet<global::iBI.CCR.Orchestrator.Benchmark.ResourceDef>();

         Init();
      }

      /*************************************************************************
       * Properties
       *************************************************************************/

      /// <summary>
      /// Identity, Indexed, Required
      /// Unique identifier
      /// </summary>
      [Key]
      [Required]
      [System.ComponentModel.Description("Unique identifier")]
      public long Id { get; set; }

      /*************************************************************************
       * Navigation properties
       *************************************************************************/

      public virtual ICollection<global::iBI.CCR.Orchestrator.Benchmark.ResourceDef> ResourceDefs { get; private set; }

   }
}

and

namespace iBI.CCR.Orchestrator.Benchmark
{
   public partial class ResourceDef
   {
      partial void Init();

      /// <summary>
      /// Default constructor. Protected due to required properties, but present because EF needs it.
      /// </summary>
      protected ResourceDef()
      {
         Init();
      }

      /// <summary>
      /// Replaces default constructor, since it's protected. Caller assumes responsibility for setting all required values before saving.
      /// </summary>
      public static ResourceDef CreateResourceDefUnsafe()
      {
         return new ResourceDef();
      }

      /// <summary>
      /// Public constructor with required data
      /// </summary>
      /// <param name="tenantinfo"></param>
      public ResourceDef(global::iBI.CCR.Orchestrator.Benchmark.TenantInfo tenantinfo) : this()
      {
         if (tenantinfo == null) throw new ArgumentNullException(nameof(tenantinfo));
         this.TenantInfo = tenantinfo;
         tenantinfo.ResourceDefs.Add(this);

      }

      /// <summary>
      /// Static create function (for use in LINQ queries, etc.)
      /// </summary>
      /// <param name="tenantinfo"></param>
      public static ResourceDef Create(global::iBI.CCR.Orchestrator.Benchmark.TenantInfo tenantinfo)
      {
         return new ResourceDef(tenantinfo);
      }

      /*************************************************************************
       * Properties
       *************************************************************************/

      /// <summary>
      /// Identity, Indexed, Required
      /// Unique identifier
      /// </summary>
      [Key]
      [Required]
      [System.ComponentModel.Description("Unique identifier")]
      public long Id { get; set; }

      /*************************************************************************
       * Navigation properties
       *************************************************************************/

      /// <summary>
      /// Required
      /// </summary>
      public virtual global::iBI.CCR.Orchestrator.Benchmark.TenantInfo TenantInfo { get; set; }

   }
}

which is also the proper code for that design.

adar2 commented 1 year ago

I do get the same results. To conclude I understand that the navigation properties in the entities themselves are enough and the modelBuilder.Entity<global::iBI.DaaS.Concurrent.Metadata.Models.ResourceDef>().Navigation(e => e.TenantInfo).IsRequired(); and likewise that had been generated in the previous versions of the extension are redundant. Am i right?

MichaelSawczyn-AWH commented 1 year ago

For the most part, yes.

Having navigation properties in the entities lets EFCore know they need to be handled, and they really don't need to be specified in the DbContext configuration. If they're not specified there, convention takes over and EFCore will give them its default behavior.

The designer emits a lot of code that truly isn't needed, but rather than assume that conventions won't change, that emitted code explicitly states what behavior should occur. That's helpful for both the programmer, and to stabilize things should conventions change in a later EFCore version.

The line of code you list above, though, is needed because something special is going on ... the IsRequired() call. Without it, EFCore would assume the association was 0/1..N rather than 1..N. At the time that specific syntax was emitted, there was a bug in EFCore that made the IsRequired call be the last thing in a fluent call chain, so we decided to emit it on a line by itself. It has since been reintegrated into the call chain, as you can see above with the code

         modelBuilder.Entity<global::iBI.CCR.Orchestrator.Benchmark.TenantInfo>()
                     .HasMany<global::iBI.CCR.Orchestrator.Benchmark.ResourceDef>(p => p.ResourceDefs)
                     .WithOne(p => p.TenantInfo)
                     .HasForeignKey("TenantInfoId")
                     .IsRequired();

The upshot is that the code generated using VS2019's designer was valid, as is the code generated by VS2022. There are an uncountable number of ways to specify the DbContext configuration, and they'll evolve both as the designer evolves and as the EFCore libraries do.

adar2 commented 1 year ago

@msawczyn Thank you very much for the elaborated answer.