Skip to content

[ODS-5725] Enforcement of Required/Optional Embedded Objects in Resources (v6.1 for TEA) #4

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.

Sign up for GitHub

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

Open
gmcelhanon wants to merge 5 commits into
base: f-TEA-v6.1
Choose a base branch
from
Open

[ODS-5725] Enforcement of Required/Optional Embedded Objects in Resources (v6.1 for TEA) #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f504fd
Semantic model enhancements
gmcelhanon Oct 6, 2023
a316d41
Open API metadata fix for required/optional rendering.
gmcelhanon Oct 6, 2023
a39e633
CodeGen utility C# changes.
gmcelhanon Oct 6, 2023
4cdfdc3
AssociationView unit tests
gmcelhanon Oct 6, 2023
f9d1652
Required enforcement of required 1-to-1 relationships (for generated …
gmcelhanon Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Application/EdFi.Ods.Common/Models/Domain/Association.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ internal Association(DomainModel domainModel, AssociationDefinition associationD

IsIdentifying = associationDefinition.IsIdentifying;
IsRequired = associationDefinition.IsRequired;
IsRequiredCollection = associationDefinition.Cardinality == Cardinality.OneToOneOrMore;
ConstraintByDatabaseEngine = associationDefinition.ConstraintNames;
PotentiallyLogical = associationDefinition.PotentiallyLogical;
}
Expand All @@ -100,8 +99,6 @@ internal Association(DomainModel domainModel, AssociationDefinition associationD

public bool IsIdentifying { get; }

public bool IsRequiredCollection { get; }

/// <summary>
/// Indicates whether or not the association should be enforced (this allows a reference to be un-resolvable).
/// </summary>
Expand Down Expand Up @@ -146,6 +143,9 @@ public bool IsSelfReferencing
=> PrimaryEntityFullName.Name.EqualsIgnoreCase(SecondaryEntityFullName.Name)
&& PrimaryEntityFullName.Schema.Equals(SecondaryEntityFullName.Schema);

/// <summary>
/// Indicates that the values of the association are required on the receiving entity.
/// </summary>
public bool IsRequired { get; }

/// <summary>
Expand Down
31 changes: 25 additions & 6 deletions Application/EdFi.Ods.Common/Models/Domain/AssociationView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ internal AssociationView(DomainModel domainModel, Association association, bool
.Where(av => av != null)
.Any(av =>
// An optional collection
(av.AssociationType == AssociationViewType.ManyToOne && !av.Association.IsRequiredCollection)
(av.AssociationType == AssociationViewType.ManyToOne && !av.Inverse.IsRequiredCollection)

// An optional incoming one-to-one reference
|| (av.AssociationType == AssociationViewType.OneToOneIncoming && !av.IsRequired));
|| (av.AssociationType == AssociationViewType.OneToOneIncoming && !av.IsRequiredIncomingAssociation));

if (isContainingEntityPresenceOptional)
{
Expand All @@ -121,7 +121,7 @@ internal AssociationView(DomainModel domainModel, Association association, bool
if (AssociationType == AssociationViewType.ManyToOne
|| AssociationType == AssociationViewType.OneToOneIncoming)
{
return !IsRequired;
return !Association.IsRequired;
}

// All other associations do not represent dependencies
Expand Down Expand Up @@ -436,14 +436,33 @@ public string RoleName
/// </summary>
public AssociationView Inverse { get; internal set; }

public bool IsRequired
/// <summary>
/// Indicates that the properties of an incoming association are required on the receiving entity (for outgoing associations,
/// see <see cref="IsRequiredCollection" /> and <see cref="IsRequiredEmbeddedObject" />).
/// </summary>
public bool IsRequiredIncomingAssociation
{
get { return Association.IsRequired; }
get => Association.IsRequired && AssociationType.IsIncoming();
}

/// <summary>
/// Indicates that the association view represents an outgoing one-to-one relationship (single object) with an entity that must exist.
/// </summary>
public bool IsRequiredEmbeddedObject
{
get =>
AssociationType == AssociationViewType.OneToOneOutgoing
&& Association.Cardinality == Cardinality.OneToOne;
}

/// <summary>
/// Indicates that the association view represents an outgoing one-to-many relationship (collection) that must contain at least one item.
/// </summary>
public bool IsRequiredCollection
{
get { return Association.IsRequiredCollection; }
get =>
AssociationType == AssociationViewType.OneToMany
&& Association.Cardinality == Cardinality.OneToOneOrMore;
}

FullName IHasNameContext.ParentFullName
Expand Down
19 changes: 19 additions & 0 deletions Application/EdFi.Ods.Common/Models/Domain/AssociationViewType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,23 @@ public enum AssociationViewType
ToExtension,
FromCore
}

public static class AssociationViewTypeExtensions
{
public static bool IsIncoming(this AssociationViewType associationViewType)
{
return (associationViewType == AssociationViewType.FromBase
|| associationViewType == AssociationViewType.FromCore
|| associationViewType == AssociationViewType.OneToOneIncoming
|| associationViewType == AssociationViewType.ManyToOne);
}

public static bool IsOutgoing(this AssociationViewType associationViewType)
{
return (associationViewType == AssociationViewType.ToDerived
|| associationViewType == AssociationViewType.ToExtension
|| associationViewType == AssociationViewType.OneToMany
|| associationViewType == AssociationViewType.OneToOneOutgoing);
}
}
}
2 changes: 1 addition & 1 deletion Application/EdFi.Ods.Common/Models/Resource/Reference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public Resource ReferencedResource

public bool IsRequired
{
get { return Association.IsRequired; }
get { return Association.IsRequiredIncomingAssociation; }
}

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion Application/EdFi.Ods.Features/OpenApiMetadata/Factories/OpenApiMetadataDefinitionsFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private Schema CreateResourceChildSchema(ResourceChildItem resourceChildItem, Op
resourceChildItem.EmbeddedObjects.Select(
e => new
{
IsRequired = e.Association.IsRequired,
IsRequired = e.Association?.IsRequiredEmbeddedObject ?? false,
Key = e.JsonPropertyName,
Schema = CreateEmbeddedObjectSchema(e, openApiMetadataResource)
})).ToList();
Expand Down Expand Up @@ -324,6 +324,7 @@ private Schema CreateResourceSchema(OpenApiMetadataResource openApiMetadataResou
x => new PropertySchemaInfo
{
PropertyName = x.JsonPropertyName,
IsRequired = x.Association?.IsRequiredEmbeddedObject ?? false,
Sort = SortOrder(x.PropertyName, false),
Schema = CreateEmbeddedObjectSchema(x, openApiMetadataResource)
})).Concat(
Expand Down
22 changes: 11 additions & 11 deletions Application/EdFi.Ods.Tests/EdFi.Ods.Common/Models/Domain/AssociationViewTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2522,25 +2522,25 @@ protected override void Act()
.ToList();

_nonNavigableOptionalManyToOneAssociations = allAssociations.Where(a =>
!a.IsNavigable && !a.IsSelfReferencing && !a.IsRequired
!a.IsNavigable && !a.IsSelfReferencing && !a.Association.IsRequired
&& a.AssociationType == AssociationViewType.ManyToOne)
.ToList();

_nonNavigableOptionalOneToOneIncomingAssociations = allAssociations.Where(a =>
a.AssociationType == AssociationViewType.OneToOneIncoming && !a.IsNavigable && !a.IsRequired)
a.AssociationType == AssociationViewType.OneToOneIncoming && !a.IsNavigable && !a.Association.IsRequired)
.ToList();

_topLevelRequiredManyToOneAssociations = allAssociations.Where(
a => a.AssociationType == AssociationViewType.ManyToOne
&& !a.IsNavigable
&& a.IsRequired
&& a.Association.IsRequired
&& a.ThisEntity.IsAggregateRoot)
.ToList();

var childRequiredManyToOneAssociations = allAssociations.Where(
a => a.AssociationType == AssociationViewType.ManyToOne
&& !a.IsNavigable
&& a.IsRequired
&& a.Association.IsRequired
&& !a.ThisEntity.IsAggregateRoot)
.ToList();

Expand All @@ -2564,37 +2564,37 @@ protected override void Act()
_requiredEmbeddedObjectsWithRequiredAssociations = childRequiredManyToOneAssociations
.Where(a => a.ThisEntity.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming)
.Where(a =>
a.ThisEntity.ParentAssociation.Inverse.IsRequired
a.ThisEntity.ParentAssociation.Inverse.Association.IsRequired

// Check one level up for required child items (if it's in the there)
&& (a.ThisEntity.Parent.Parent == null
|| (a.ThisEntity.Parent.ParentAssociation.AssociationType == AssociationViewType.ManyToOne
&& a.ThisEntity.Parent.ParentAssociation.Inverse.IsRequiredCollection)
|| (a.ThisEntity.Parent.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming
&& a.ThisEntity.Parent.ParentAssociation.Inverse.IsRequired))
&& a.ThisEntity.Parent.ParentAssociation.Inverse.Association.IsRequired))

// Check two levels up for required child items (if it's in the there)
&& (a.ThisEntity.Parent.Parent?.Parent == null
|| (a.ThisEntity.Parent.Parent.ParentAssociation.AssociationType == AssociationViewType.ManyToOne
&& a.ThisEntity.Parent.Parent.ParentAssociation.Inverse.IsRequiredCollection)
|| (a.ThisEntity.Parent.Parent.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming
&& a.ThisEntity.Parent.Parent.ParentAssociation.Inverse.IsRequired)))
&& a.ThisEntity.Parent.Parent.ParentAssociation.Inverse.Association.IsRequired)))
.ToList();

_optionalEmbeddedObjectsWithRequiredAssociations = childRequiredManyToOneAssociations
.Where(a => a.ThisEntity.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming)
.Where(a =>
!a.ThisEntity.ParentAssociation.Inverse.IsRequired
!a.ThisEntity.ParentAssociation.Inverse.Association.IsRequired

// Check one level up for optional child items (if it's in the there)
|| (a.ThisEntity.Parent.Parent != null
&& ((a.ThisEntity.Parent.ParentAssociation.AssociationType == AssociationViewType.ManyToOne && !a.ThisEntity.Parent.ParentAssociation.Inverse.IsRequiredCollection)
|| (a.ThisEntity.Parent.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming && !a.ThisEntity.Parent.ParentAssociation.Inverse.IsRequired)))
|| (a.ThisEntity.Parent.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming && !a.ThisEntity.Parent.ParentAssociation.Inverse.Association.IsRequired)))

// Check two levels up for optional child items (if it's in the there)
|| (a.ThisEntity.Parent.Parent?.Parent != null
&& ((a.ThisEntity.Parent.Parent.ParentAssociation.AssociationType == AssociationViewType.ManyToOne && !a.ThisEntity.Parent.Parent.ParentAssociation.Inverse.IsRequiredCollection)
|| (a.ThisEntity.Parent.Parent.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming && !a.ThisEntity.Parent.Parent.ParentAssociation.Inverse.IsRequired))))
|| (a.ThisEntity.Parent.Parent.ParentAssociation.AssociationType == AssociationViewType.OneToOneIncoming && !a.ThisEntity.Parent.Parent.ParentAssociation.Inverse.Association.IsRequired))))
.ToList();

_untestedAssociations = allAssociations
Expand Down Expand Up @@ -2884,7 +2884,7 @@ public void Should_indicate_that_a_required_reference_on_a_required_embedded_obj
{
var association = _resourceModel.GetAllResources()
.SelectMany(res => res.EmbeddedObjects)
.Where(eo => eo.Association.IsRequired)
.Where(eo => eo.Association.Association.IsRequired)
.SelectMany(eo => eo.ObjectType.References.Where(r => r.IsRequired))
.Select(r => r.Association)
.FirstOrDefault();
Expand Down
3 changes: 2 additions & 1 deletion Utilities/CodeGeneration/EdFi.Ods.CodeGen/Generators/DatabaseMetadataProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ protected override object Build()

bool AssociationIsNotOneToOne(Association a)
=> a.Cardinality != Cardinality.OneToOne
&& a.Cardinality != Cardinality.OneToZeroOrOne
&& a.Cardinality != Cardinality.OneToOneInheritance
&& a.Cardinality != Cardinality.OneToOneExtension;

Expand Down Expand Up @@ -80,7 +81,7 @@ DatabaseMetadata DatabaseMetadataForAssociation(Association a)
return new
{
NamespaceName = EdFiConventions.BuildNamespace(BaseNamespaceName, TemplateContext.SchemaProperCaseName),
IndexMetaData = databaseMetadata.OrderBy(x => x.TableName).ThenBy(x => x.Name)
IndexMetadata = databaseMetadata.OrderBy(x => x.TableName).ThenBy(x => x.Name)
};
}
}
Expand Down
1 change: 1 addition & 0 deletions Utilities/CodeGeneration/EdFi.Ods.CodeGen/Generators/Entities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ private IEnumerable<object> GetClasses(Aggregate aggregate, Entity entity)
ClassName = entity.Name, NamespacePrefix = GetCommonRelativeNamespacePrefix(entity),
OtherClassName = a.OtherEntity.Name,
OtherNamespacePrefix = GetCommonRelativeNamespacePrefix(a.OtherEntity),
IsRequired = a.IsRequiredEmbeddedObject,
}),
RelocatedExtensionOneToOnes = entity.EdFiStandardEntityAssociation?.OtherEntity.NavigableOneToOnes
.Where(a => a.OtherEntity.Schema == entity.Schema)
Expand Down
2 changes: 1 addition & 1 deletion Utilities/CodeGeneration/EdFi.Ods.CodeGen/Generators/Resources/PropertyData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public object Render()
? AssembleOtherUnifiedChild(ExtensionAssociations)
: null,
ImplicitPropertyName = Associations.Any()
? Associations.OrderByDescending(x => x.IsRequired)
? Associations.OrderByDescending(x => x.Association.IsRequired)
.First()
.Name
: null,
Expand Down
2 changes: 1 addition & 1 deletion Utilities/CodeGeneration/EdFi.Ods.CodeGen/Mustache/Entities.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ namespace {{AggregateNamespace}}
// One-to-one relationships
// -------------------------------------------------------------
{{#OneToOnes}}
[ValidateObject]
{{#IsRequired}}[Required]{{/IsRequired}}[ValidateObject]
Copy link
Owner Author

@gmcelhanon gmcelhanon Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this change if you don't want to enforce required on non-extension embedded objects.

public virtual {{AggregateNamespacePrefix}}{{OtherClassName}} {{OtherClassName}}
{
get
Expand Down