fix: add DbSet properties for new entities and configure relationship

src/Stackworx.EfCoreGraphQL/DataLoader.cs

@@ -20,6 +20,12 @@
 
     public bool Nullable { get; init; }
 
+    /// <summary>
+    /// True when the key property is a nullable value type (<c>int?</c>), so its value is reached through
+    /// <c>.Value</c>. A nullable reference type (<c>string?</c>) is dereferenced directly.
+    /// </summary>
+    public bool KeyIsNullableValueType { get; init; }
+
     public DataLoaderType Type { get; init; }
 
     public string? Notes { get; set; }
@@ -49,6 +55,7 @@
         {
             LoaderName = LoaderNames.BatchLoaderName(entityType, pkProp),
             Nullable = false,
+            KeyIsNullableValueType = TypeUtils.TryUnwrapNullable(pkProp.ClrType, out _),
             Type = DataLoader.DataLoaderType.OneToOne,
             KeyType = keyType,
             ReferenceField = keyPropName,
@@ -75,7 +82,8 @@
         }
 
         var keyType = prop.ClrType;
-        if (TypeUtils.TryUnwrapNullable(keyType, out var inner))
+        var keyIsNullableValueType = TypeUtils.TryUnwrapNullable(keyType, out var inner);
+        if (keyIsNullableValueType)
         {
             keyType = inner;
         }
@@ -95,8 +103,9 @@
                 ? LoaderNames.GroupLoaderName(nav.TargetEntityType, prop)
                 : LoaderNames.BatchLoaderName(nav.TargetEntityType, prop),
             Nullable = nullable,
+            KeyIsNullableValueType = keyIsNullableValueType,
             Type = type,
             KeyType = keyType,
             ReferenceField = prop.Name,
             IsShadowProperty = prop.IsShadowProperty(),
             EntityType = TypeUtils.GetNestedQualifiedName(entityType.ClrType),
@@ -114,10 +123,30 @@
         return sb.ToString();
     }
 
+    /// <summary>
+    /// How the key is read off an entity. A nullable value type needs <c>.Value</c>; a nullable reference
+    /// type only needs the null-forgiving operator, since it has no <c>.Value</c> member.
+    /// </summary>
+    private string KeyAccess
+    {
+        get
+        {
+            if (!this.Nullable)
+            {
+                return $"e.{this.ReferenceField}";
+            }
+
+            return this.KeyIsNullableValueType
+                ? $"e.{this.ReferenceField}!.Value"
+                : $"e.{this.ReferenceField}!";
+        }
+    }
+
     public string Emit(int version)
     {
         var sb = new StringBuilder();
         var keyType = TypeUtils.GetNestedQualifiedName(this.KeyType);
+        var keyAccess = this.KeyAccess;
 
         sb.AppendLine($"    [DataLoader]");
 
@@ -142,26 +171,10 @@
                 sb.AppendLine($"        var items = await context.Set<{this.EntityType}>()");
                 sb.AppendLine($"            .AsNoTracking()");
 
-                if (this.Nullable)
-                {
-                    sb.AppendLine($"            .Where(e => keys.Contains(e.{this.ReferenceField}!.Value))");
-                }
-                else
-                {
-                    sb.AppendLine($"            .Where(e => keys.Contains(e.{this.ReferenceField}))");
-                }
-
+                sb.AppendLine($"            .Where(e => keys.Contains({keyAccess}))");
                 sb.AppendLine($"            .ToListAsync(ct);");
                 sb.AppendLine();
-
-                if (this.Nullable)
-                {
-                    sb.AppendLine($"        return items.ToLookup(e => e.{this.ReferenceField}!.Value);");
-                }
-                else
-                {
-                    sb.AppendLine($"        return items.ToLookup(e => e.{this.ReferenceField});");
-                }
+                sb.AppendLine($"        return items.ToLookup(e => {keyAccess});");
 
                 if (this.IsShadowProperty)
                 {
@@ -196,16 +209,8 @@
                 sb.AppendLine($"        return await context.Set<{this.EntityType}>()");
                 sb.AppendLine($"            .AsNoTracking()");
 
-                if (this.Nullable)
-                {
-                    sb.AppendLine($"            .Where(e => keys.Contains(e.{this.ReferenceField}!.Value))");
-                    sb.AppendLine($"            .ToDictionaryAsync(e => e.{this.ReferenceField}!.Value, ct);");
-                }
-                else
-                {
-                    sb.AppendLine($"            .Where(e => keys.Contains(e.{this.ReferenceField}))");
-                    sb.AppendLine($"            .ToDictionaryAsync(e => e.{this.ReferenceField}, ct);");
-                }
+                sb.AppendLine($"            .Where(e => keys.Contains({keyAccess}))");
+                sb.AppendLine($"            .ToDictionaryAsync(e => {keyAccess}, ct);");
 
                 sb.AppendLine("    }");
                 break;

src/Stackworx.EfCoreGraphQL/FieldExtension.cs

@@ -18,6 +18,12 @@ public record FieldExtension
 
     public required bool ReferenceFieldNullable { get; init; }
 
+    /// <summary>
+    /// True when the reference field is a nullable value type (<c>int?</c>), so its value is reached
+    /// through <c>.Value</c>. A nullable reference type (<c>string?</c>) is passed directly.
+    /// </summary>
+    public bool ReferenceFieldIsNullableValueType { get; init; }
+
     public required Type DbContextType { get; init; }
 
     // TODO: Data Loader?
@@ -84,6 +90,7 @@ public static FieldExtension FromNavigation(Type dbContextClass, INavigation nav
             NavigationName = nav.Name,
             ReferenceField = prop.Name,
             ReferenceFieldNullable = prop.IsNullable,
+            ReferenceFieldIsNullableValueType = TypeUtils.TryUnwrapNullable(prop.ClrType, out _),
             IsShadowProperty = prop.IsShadowProperty(),
             DbContextType = dbContextClass,
             LoaderName = loaderName,
@@ -140,10 +147,16 @@ public string Emit()
 
             if (this.ReferenceFieldNullable)
             {
+                // A nullable value type is unwrapped with .Value; a nullable reference type has no such
+                // member and is passed straight through once the null check has narrowed it.
+                var keyAccess = this.ReferenceFieldIsNullableValueType
+                    ? $"parent.{this.ReferenceField}.Value"
+                    : $"parent.{this.ReferenceField}";
+
                 sb.AppendLine($"        if (parent.{this.ReferenceField} is not null)");
                 sb.AppendLine($"        {{");
                 // TODO: this wont always be appropriate
-                sb.AppendLine($"            return await loader.LoadAsync(parent.{this.ReferenceField}.Value, ct);");
+                sb.AppendLine($"            return await loader.LoadAsync({keyAccess}, ct);");
                 sb.AppendLine($"        }}");
                 sb.AppendLine();
                 sb.AppendLine($"        return null;");

src/Stackworx.EfCoreGraphQL/Generate.cs

@@ -40,59 +40,9 @@ public static async Task Generate(
     {
         options ??= new GenerateOptions();
 
-        var version = GetHotchocolateVersion();
-
-        var mode = options.Mode;
-        var filter = options.Filter;
-        var ns = options.Namespace;
         var ci = options.CI;
 
-        var model = dbContext.Model;
-
-        var sb = new StringBuilder();
-        sb.AppendLine("// <auto-generated>");
-        sb.AppendLine("#nullable enable");
-        sb.AppendLine("#pragma warning disable");
-        sb.AppendLine("//  Source: EF Core model navigations");
-        sb.AppendLine("//  Style: HotChocolate [DataLoader] decorator methods");
-        sb.AppendLine("// </auto-generated>");
-        sb.AppendLine();
-        sb.AppendLine($"namespace {ns};");
-        sb.AppendLine("using System;");
-        sb.AppendLine("using System.Linq;");
-        sb.AppendLine("using System.Threading;");
-        sb.AppendLine("using System.Threading.Tasks;");
-        sb.AppendLine("using System.Collections.Generic;");
-        sb.AppendLine("using Microsoft.EntityFrameworkCore;");
-        sb.AppendLine("using HotChocolate;");
-        sb.AppendLine("using GreenDonut;");
-        sb.AppendLine("using HotChocolate.Types;");
-        sb.AppendLine();
-
-        foreach (var entity in model.GetEntityTypes()
-                     .Where(e => !e.IsOwned())
-                     .OrderBy(e => e.Name))
-        {
-            if (entity.ShouldIgnore())
-            {
-                continue;
-            }
-
-            if (mode == Mode.OptIn && !entity.ShouldInclude())
-            {
-                continue;
-            }
-
-            // Allow user to manually exclude specific entities
-            if (filter is not null && filter(entity))
-            {
-                continue;
-            }
-
-            Generate(dbContext.GetType(), entity, sb, version);
-        }
-
-        await File.WriteAllTextAsync(outPath, sb.ToString());
+        await File.WriteAllTextAsync(outPath, BuildSource(dbContext.Model, dbContext.GetType(), options));
 
         if (ci)
         {
@@ -122,9 +72,13 @@ public static string GenerateString(
         IModel model,
         Type contextType,
         GenerateOptions? options = null)
-    {
-        options ??= new GenerateOptions();
+        => BuildSource(model, contextType, options ?? new GenerateOptions());
 
+    private static string BuildSource(
+        IModel model,
+        Type contextType,
+        GenerateOptions options)
+    {
         var version = GetHotchocolateVersion();
 
         var mode = options.Mode;
@@ -151,6 +105,11 @@ public static string GenerateString(
         sb.AppendLine("using HotChocolate.Types;");
         sb.AppendLine();
 
+        // HotChocolate emits one DataLoader class per [DataLoader] method name, and every method we
+        // generate lands in the same namespace. The same loader can be reachable from two entities, so
+        // names are tracked across the whole run to keep each one emitted exactly once.
+        var emittedLoaders = new Dictionary<string, string>(StringComparer.Ordinal);
+
         foreach (var entity in model.GetEntityTypes()
                      .Where(e => !e.IsOwned())
                      .OrderBy(e => e.Name))
@@ -171,12 +130,47 @@ public static string GenerateString(
                 continue;
             }
 
-            Generate(contextType, entity, sb, version);
+            Generate(contextType, entity, sb, version, emittedLoaders, options.IgnoreForeignKeyFields);
         }
 
         return sb.ToString();
     }
 
+    /// <summary>
+    /// Emits a data loader unless one with the same name has already been emitted.
+    /// </summary>
+    /// <remarks>
+    /// A loader name is reachable from more than one entity when a dependent's primary key is also its
+    /// foreign key to the principal: the principal's navigation loader and the dependent's primary-key
+    /// loader are keyed on the same property, so both resolve to the same name and the same query.
+    /// Field extensions reference the loader interface by name, so skipping the repeat is safe.
+    /// </remarks>
+    private static void AppendLoader(
+        StringBuilder sb,
+        Dictionary<string, string> emittedLoaders,
+        string loaderName,
+        string comment,
+        string code)
+    {
+        if (emittedLoaders.TryGetValue(loaderName, out var existing))
+        {
+            if (string.Equals(existing, code, StringComparison.Ordinal))
+            {
+                return;
+            }
+
+            // Two different queries want the same name. Emitting both cannot compile, and silently
+            // dropping one would leave field extensions bound to the wrong loader.
+            throw new InvalidOperationException(
+                $"Two different data loaders were generated with the name '{loaderName}'. " +
+                $"Exclude one of the navigations with [{nameof(EFCoreGraphQLIgnoreAttribute)}] or rename it.");
+        }
+
+        emittedLoaders[loaderName] = code;
+        sb.Append(comment);
+        sb.AppendLine(code);
+    }
+
     private static int GetHotchocolateVersion()
     {
         const int defaultMajor = 16;
@@ -268,7 +262,13 @@ private static bool RunCommand(string fileName, string args)
         return process.ExitCode == 0;
     }
 
-    private static void Generate(Type dbContextClass, IEntityType entity, StringBuilder sb, int version)
+    private static void Generate(
+        Type dbContextClass,
+        IEntityType entity,
+        StringBuilder sb,
+        int version,
+        Dictionary<string, string> emittedLoaders,
+        bool ignoreForeignKeyFields)
     {
         // Skip join tables
         var pk = entity.FindPrimaryKey();
@@ -284,7 +284,9 @@ private static void Generate(Type dbContextClass, IEntityType entity, StringBuil
 
         sb.AppendLine($"// {entity.DisplayName()}");
 
-        var ignoreFields = ForeignKeyIgnoreFields.Get(entity);
+        var ignoreFields = ignoreForeignKeyFields
+            ? ForeignKeyIgnoreFields.Get(entity)
+            : [];
         var entityTypeName = TypeUtils.GetNestedQualifiedName(entity.ClrType);
 
         if (ignoreFields.Count > 0)
@@ -303,8 +305,7 @@ private static void Generate(Type dbContextClass, IEntityType entity, StringBuil
         if (entity.FindPrimaryKey()?.Properties.Count == 1)
         {
             var dataLoader = DataLoader.FromEntity(dbContextClass, entity);
-            sb.Append(dataLoader.EmitComment());
-            sb.AppendLine(dataLoader.Emit(version));
+            AppendLoader(sb, emittedLoaders, dataLoader.LoaderName, dataLoader.EmitComment(), dataLoader.Emit(version));
         }
 
         foreach (var navigation in entity.GetNavigations()
@@ -323,14 +324,26 @@ private static void Generate(Type dbContextClass, IEntityType entity, StringBuil
             }
 
             // Skip dependant navigations
-            if (!navigation.IsOnDependent)
+            var dataLoader = navigation.IsOnDependent
+                ? null
+                : DataLoader.FromNavigation(dbContextClass, navigation);
+
+            var field = FieldExtension.FromNavigation(dbContextClass, navigation);
+
+            // A shadow foreign key has no CLR property to read, so neither the loader nor the field
+            // override can be written against it. Skip the navigation and leave HotChocolate's default
+            // resolution in place: emitting an override that throws turns any query for the field into
+            // an error, which is worse than not overriding it at all.
+            if (field.IsShadowProperty || dataLoader?.IsShadowProperty == true)
             {
-                var dataLoader = DataLoader.FromNavigation(dbContextClass, navigation);
-                sb.Append(dataLoader.EmitComment());
-                sb.AppendLine(dataLoader.Emit(version));
+                continue;
+            }
+
+            if (dataLoader is not null)
+            {
+                AppendLoader(sb, emittedLoaders, dataLoader.LoaderName, dataLoader.EmitComment(), dataLoader.Emit(version));
             }
 
-            var field = FieldExtension.FromNavigation(dbContextClass, navigation);
             sb.Append(field.EmitComment());
             sb.AppendLine(field.Emit());
         }
@@ -383,7 +396,7 @@ private static void Generate(Type dbContextClass, IEntityType entity, StringBuil
             {
                 // TODO: emit extension
                 var manyToMany = ManyToMany.FromNavigation(dbContextClass, navigation);
-                sb.AppendLine(manyToMany.EmitDataLoader());
+                AppendLoader(sb, emittedLoaders, manyToMany.LoaderName, string.Empty, manyToMany.EmitDataLoader());
                 sb.AppendLine(manyToMany.EmitFieldExtension());
             }
         }

src/Stackworx.EfCoreGraphQL/GenerateOptions.cs

@@ -20,4 +20,14 @@ public class GenerateOptions
     /// When enabled, generation will fail the process if generated output differs from git HEAD.
     /// </summary>
     public bool CI { get; set; }
+
+    /// <summary>
+    /// When enabled, foreign-key scalar fields are hidden from the schema via
+    /// <c>ExtendObjectType(IgnoreFields = ...)</c>, on the basis that the navigation replaces them.
+    /// </summary>
+    /// <remarks>
+    /// Disable this for an existing schema: hiding the scalars removes fields that clients may already
+    /// select (e.g. a foreign key that doubles as a business identifier), which breaks their queries.
+    /// </remarks>
+    public bool IgnoreForeignKeyFields { get; set; } = true;
 }

tests/Stackworx.EfCoreGraphQL.Tests/Data/Account.cs

@@ -0,0 +1,11 @@
+namespace Stackworx.EfCoreGraphQL.Tests.Data;
+
+public class Account
+{
+    public int Id { get; set; }
+
+    public required string Name { get; set; }
+
+    // optional navigation to the dependent that shares this entity's primary key
+    public AccountBalance? Balance { get; set; }
+}