Skip to content

Commit f8dd42e

Browse files
committed
fix: avoid emitting incorrect metadata for separate-key one-to-one.
1 parent ba72f98 commit f8dd42e

File tree

9 files changed

+103
-20
lines changed

9 files changed

+103
-20
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- `c-input` no longer attempts to delegate shared-key one-to-one properties to `c-select`, a scenario that is not possible to select values for.
88
- DTOs no longer generate MapToNew methods with unsatisfied `required` constraints.
99
- Fix results from `useResponseCaching` in `ViewModel`/`ListViewModel` `$load` results would get treated as dirty data.
10+
- Separate-key one-to-one relationships, which already weren't supported by Coalesce, will no longer emit incorrect metadata for the navigation prop on the principal side of the relationship. These properties now emit as "value" properties instead of as "referenceNavigation" properties.
1011

1112
# 6.0.1
1213
- `createAspNetCoreHmrPlugin` now displays NPM package mismatches with the standard vite overlay instead of a custom overlay.

docs/modeling/model-types/entities.md

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ On entities that are the "one" side of a one-to-many relationship, collection na
2828

2929
### One-to-one Relationships
3030

31-
One-to-one relationships can be represented in Coalesce, but require fairly specific configuration to satisfy both EF and Coalesce's needs. Specifically, the dependent/child side of the one-to-one (the entity whose PK is also a FK), must explicitly annotate its PK with `[ForeignKey]` pointing at the parent navigation property. For example:
31+
One-to-one relationships can be represented in Coalesce, but require fairly specific configuration to satisfy both EF and Coalesce's needs. Coalesce handles two types of one-to-one relationships:
32+
33+
#### Shared-Key One-to-One (Recommended)
34+
35+
In a shared-key one-to-one relationship, the dependent/child entity's primary key is also its foreign key to the principal/parent entity. The dependent side must explicitly annotate its PK with `[ForeignKey]` pointing at the parent navigation property:
3236

3337
```c#
3438
public class OneToOneParent
@@ -47,7 +51,26 @@ public class OneToOneChild
4751
}
4852
```
4953

50-
Alternatively, you could also implement a one-to-one like a normal one-to-many relationship, where both sides of the relationship have their own distinct PK, and just never add more than one child to any particular principal entity. A unique constraint on the foreign key column can enforce the singularity of this relationship.
54+
The `[InverseProperty]` attribute can optionally be added to the parent's navigation property, but is not required for Coalesce to correctly identify this relationship.
55+
56+
#### Separate-Key One-to-One (Limited Support)
57+
58+
In a separate-key one-to-one relationship, the dependent entity has its own distinct primary key and a separate foreign key property with a unique constraint:
59+
60+
```c#
61+
[Index(nameof(ParentId), IsUnique = true)]
62+
public class OneToOneSeparateKeyChild
63+
{
64+
public int Id { get; set; }
65+
66+
[ForeignKey(nameof(Parent))]
67+
public int ParentId { get; set; }
68+
69+
public OneToOneParent? Parent { get; set; }
70+
}
71+
```
72+
73+
**Important Limitation**: Coalesce cannot fully model reference navigation properties of separate-key one-to-one relationships from the principal/parent side (the side that doesn't own the foreign key). This is because Coalesce can only represent a reference navigation that has a corresponding foreign key property on the same type. Such navigation properties will be emitted as regular "value" role properties rather than reference navigation properties, even if annotated with `[InverseProperty]`. This results in a more limited experience in the [admin pages](/stacks/vue/admin-pages.md) and [ViewModels](/stacks/vue/layers/viewmodels.md)
5174

5275
### Inheritance (TPH and TPT)
5376

src/IntelliTect.Coalesce.CodeGeneration.Api/Generators/ClassDto.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ private void WriteParameterDto(CSharpCodeBuilder b)
165165
.OrderBy(c => c.Parameters.Count())
166166
.Where(c => c.DtoMapToNewConstructorUsage.IsAcceptable)
167167
.FirstOrDefault();
168-
168+
169169
if (bestCtor == null)
170170
{
171171
var reasons = Model.Constructors
@@ -196,7 +196,7 @@ private void WriteParameterDto(CSharpCodeBuilder b)
196196
// If the type has no default constructor but also isn't used as an input by Coalesce,
197197
// don't stop code gen - just gen an exception (that will never be hit at runtime)
198198
b.Line("throw new NotSupportedException(" +
199-
$"\"Type {Model.Name} does not have a constructor suitable for use by Coalesce for new object instantiation. " +
199+
$"\"Type '{Model.Name}' does not have a constructor suitable for use by Coalesce for new object instantiation. " +
200200
"Fortunately, this type appears to never be used in an input position in a Coalesce-generated API.\");");
201201
}
202202
else
@@ -406,8 +406,8 @@ void WriteSetters(CSharpCodeBuilder b, IEnumerable<(IEnumerable<string> conditio
406406
/// <param name="modelVar">The variable that holds the entity/model instance.</param>
407407
/// <returns></returns>
408408
private IEnumerable<string> GetPropertySetterConditional(
409-
PropertyViewModel property,
410-
PropertySecurityPermission permission,
409+
PropertyViewModel property,
410+
PropertySecurityPermission permission,
411411
string modelVar)
412412
{
413413
string RoleCheck(string role) => $"context.IsInRoleCached(\"{role.EscapeStringLiteralForCSharp()}\")";
@@ -599,8 +599,8 @@ string mapCall() => property.Object.IsCustomDto
599599
{
600600
// Only check the includes tree for things that are in the database.
601601
// Otherwise, this would break IncludesExternal.
602-
string treeCheck = property.Type.ClassViewModel.HasDbSet
603-
? $"if (tree == null || tree[nameof({dtoVar}.{name})] != null)"
602+
string treeCheck = property.Type.ClassViewModel.HasDbSet
603+
? $"if (tree == null || tree[nameof({dtoVar}.{name})] != null)"
604604
: "";
605605

606606
setter = $@"{treeCheck}

src/IntelliTect.Coalesce.Tests/TargetClasses/TestDbContext/OneToOne.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,17 @@ public class OneToOneParent
1515
[InverseProperty(nameof(SharedKeyChild2.Parent))] // Can be specified, but not required.
1616
public OneToOneSharedKeyChild2 SharedKeyChild2 { get; set; }
1717

18+
// Coalesce can't model reference navigations of separate-key one-to-one relationships
19+
// from the side that doesn't own the FK, since Coalesce can only represent
20+
// a referenceNavigation that has a corresponding foreignKey on the same type.
21+
// This will emit as a "value" role, despite having InverseProperty
22+
// which would normally make this a referenceNavigation.
23+
[InverseProperty(nameof(SeparateKeyChild.Parent))]
1824
public OneToOneSeparateKeyChild SeparateKeyChild { get; set; }
1925

26+
// This should also emit as a "value" role.
27+
public OneToOneSeparateKeyChild SeparateKeyChildNoIp { get; set; }
28+
2029
public List<OneToOneManyChildren> ManyChildren { get; set; } = [];
2130
}
2231

src/IntelliTect.Coalesce.Tests/Tests/TypeDefinition/PropertyViewModelTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,31 @@ public void OneToOne_ChildNavigations_HasCorrectMetadata(PropertyViewModelData d
354354
Assert.Equal(inverse, vm.InverseProperty.Name);
355355
}
356356

357+
[Theory]
358+
[PropertyViewModelData<OneToOneParent>(nameof(OneToOneParent.SeparateKeyChild))]
359+
[PropertyViewModelData<OneToOneParent>(nameof(OneToOneParent.SeparateKeyChildNoIp))]
360+
public void OneToOne_SeparateKey_ParentNavigations_HasCorrectMetadata(PropertyViewModelData data)
361+
{
362+
PropertyViewModel vm = data;
363+
Assert.Equal(PropertyRole.Value, vm.Role);
364+
Assert.Null(vm.ForeignKeyProperty);
365+
}
366+
367+
[Theory]
368+
[PropertyViewModelData<OneToOneSeparateKeyChild>(nameof(OneToOneSeparateKeyChild.Parent))]
369+
public void OneToOne_SeparateKey_ChildNavigations_HasCorrectMetadata(PropertyViewModelData data)
370+
{
371+
// See comments in OneToOne.cs about what this is all about
372+
373+
PropertyViewModel vm = data;
374+
Assert.Equal(PropertyRole.ReferenceNavigation, vm.Role);
375+
Assert.Equal(vm.Parent.PropertyByName("ParentId"), vm.ForeignKeyProperty);
376+
377+
// We can't model the other side as a reference navigation,
378+
// so the inverse property shouldn't be defined.
379+
Assert.Null(vm.InverseProperty);
380+
}
381+
357382
[Theory]
358383
[PropertyViewModelData<OneToOneManyChildren>(nameof(OneToOneManyChildren.OneToOneParent))]
359384
[Description("https://github.com/IntelliTect/Coalesce/commit/513db257dda32b99099355f1a6de0f5fbf367f5a")]

src/IntelliTect.Coalesce/TypeDefinition/PropertyViewModel.cs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -512,24 +512,38 @@ public DatabaseGeneratedOption DatabaseGenerated
512512
if (prop == null)
513513
{
514514
// If the other side of the relationship is also a reference navigation,
515-
// this is a 1-to-1. If there's otherwise no FK specified, assume that the PK
515+
// this is a shared-key 1-to-1. If there's otherwise no FK specified, assume that the PK
516516
// of this type is the FK we're looking for here.
517-
// See test models "OneToOneParent"/"OneToOneChild1" for example.
518-
519-
if (this.HasAttribute<InversePropertyAttribute>() &&
520-
InverseProperty?.Role == PropertyRole.ReferenceNavigation)
517+
// See test models "OneToOneParent"/"SharedKeyChild2" for example.
518+
if (this.HasAttribute<InversePropertyAttribute>())
521519
{
522520
// Only look at `InverseProperty` if explicitly annotated.
523521
// Otherwise, this will cause infinite recursion.
524-
return EffectiveParent.PrimaryKey;
522+
if (
523+
InverseProperty?.Role == PropertyRole.ReferenceNavigation &&
524+
// The reference navigation on the other side points at this prop's owner type
525+
InverseProperty.Object == this.EffectiveParent &&
526+
// The FK on the other side must reference this prop's type's PK.
527+
// This is what allows us to think of our own PK as also being a FK
528+
// into the other type (even if that's the exact opposite of the relationship in practice).
529+
// This is admittedly a bit of a hack.
530+
InverseProperty.ForeignKeyProperty?.IsPrimaryKey == true
531+
)
532+
{
533+
return EffectiveParent.PrimaryKey;
534+
}
525535
}
526-
527-
if (Object!.ClientProperties.Any(p =>
528-
p.ForeignKeyProperty == Object.PrimaryKey &&
529-
p.Type == this.EffectiveParent.Type
530-
))
536+
else
531537
{
532-
return EffectiveParent.PrimaryKey;
538+
// Handle the similar scenario as above, but when InversePropertyAttribute
539+
// is not present. See "OneToOneParent"/"SharedKeyChild1".
540+
if (Object!.ClientProperties.Any(p =>
541+
p.ForeignKeyProperty == Object.PrimaryKey &&
542+
p.Type == this.EffectiveParent.Type
543+
))
544+
{
545+
return EffectiveParent.PrimaryKey;
546+
}
533547
}
534548
}
535549
}

src/test-targets/metadata.g.ts

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/test-targets/models.g.ts

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/test-targets/viewmodels.g.ts

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)