[ODS-6551] Support feature for optimized use of serialized aggregate data #1184

gmcelhanon · 2024-11-09T04:12:13Z

See also:

[ODS-6551] Support feature for optimized use of serialized aggregate data Ed-Fi-ODS-Implementation#1171

gmcelhanon

Comments to help with code review.

gmcelhanon · 2024-11-20T04:35:45Z

Application/EdFi.Ods.Api/Container/Modules/ApplicationModule.cs

@@ -292,11 +292,6 @@ protected override void Load(ContainerBuilder builder)
                .As<IResourceValidator>()
                .SingleInstance();

-            builder.RegisterType<NoEntityExtensionsFactory>()


The use of this interface is now protected by feature checks -- no need for a placeholder component/registration.

gmcelhanon · 2024-11-20T04:36:37Z

Application/EdFi.Ods.Api/Container/Modules/ExtensionsModule.cs

@@ -47,6 +48,13 @@ public override void ApplyConfigurationSpecificRegistrations(ContainerBuilder bu
                .As<IEntityExtensionRegistrar>()
                .SingleInstance();

+            // Set feature-specific static resolvers


Rather than setting the statics for these components in OdsStartupBase, they are now done here in the feature-specific module.

gmcelhanon · 2024-11-20T04:40:05Z

Application/EdFi.Ods.Api/Container/Modules/PersistenceModule.cs

+                .As<IEntityDeserializer>()
+                .SingleInstance();
+
+            builder.RegisterType<PersonSurrogateIdMutator>()


These components handle lookup of USIs and DescriptorIds on Person types and Descriptor entities (respectively). They use the resolver/cache facilities when the values are needed after subsequent deserialization after initial creation (where they are not available to be serialized into the AggregateData column until after the entity is already INSERTed).

gmcelhanon · 2024-11-20T04:40:41Z

Application/EdFi.Ods.Api/Container/Modules/PersistenceModule.cs

@@ -197,6 +211,11 @@ protected override void Load(ContainerBuilder builder)
                .As<ISessionFactory>()
                .SingleInstance();

+            builder.Register(


This component is needed in new locations to enable deserialized entities to be reassociated with the current NHibernate session.

gmcelhanon · 2024-11-20T04:42:00Z

Application/EdFi.Ods.Common/Database/ColumnNames.cs

+
+namespace EdFi.Ods.Common.Database;
+
+public static class ColumnNames


Providing a location for commonly used column names to avoid expansion of embedded string values throughout the code.

gmcelhanon · 2024-11-20T06:05:45Z

Utilities/CodeGeneration/EdFi.Ods.CodeGen/Program.cs

@@ -90,7 +92,10 @@ await _container.Resolve<IApplicationRunner>()
            {
                var baseException = e.GetBaseException() ?? e;

-                _logger.Error(baseException.Message);
+                // Save the last exception in a static property to make it available for subsequent inspection.
+                LastException = e;


Make sure we make the exception available to the calling code when we complete the call (from unit tests).

gmcelhanon · 2024-11-20T06:08:45Z

...ests.Verify.Extensions.Sample.1.0.0_Std_5.2.0_Models_Entities_Entities.generated.approved.cs

@@ -206,23 +211,27 @@ namespace EdFi.Ods.Entities.NHibernate.BusAggregate.Sample
    /// <summary>
    /// Represents a read-only reference to the <see cref="Bus"/> entity.
    /// </summary>
+    [MessagePackObject]


Reference Data is serialized so that we don't have to look up the Id/Discriminator values separately. Full support of links is coming with ODS-6557.

gmcelhanon · 2024-11-20T06:09:34Z

...ests.Verify.Extensions.Sample.1.0.0_Std_5.2.0_Models_Entities_Entities.generated.approved.cs

@@ -536,8 +553,10 @@ public BusRoute()
        //                         Primary Key
        // -------------------------------------------------------------
        [DomainSignature]
+        [Key(6)]


Aggregate root entities start their key values at 6 due to inherited properties that are already marked for serialization.

gmcelhanon · 2024-11-20T06:10:13Z

...ests.Verify.Extensions.Sample.1.0.0_Std_5.2.0_Models_Entities_Entities.generated.approved.cs

@@ -629,6 +660,7 @@ public virtual int? StaffClassificationDescriptorId
        private int? _staffClassificationDescriptorId;
        private string _staffClassificationDescriptor;

+        [IgnoreMember]


We serialize descriptor Ids, not the full URI. It's definitive and more compact.

gmcelhanon · 2024-11-20T06:10:50Z

...ests.Verify.Extensions.Sample.1.0.0_Std_5.2.0_Models_Entities_Entities.generated.approved.cs

@@ -673,6 +706,7 @@ public virtual int? StaffUSI
        private int? _staffUSI;
        private string _staffUniqueId;

+        [IgnoreMember]


We serialize USIs not UniqueIds. It's definitive and more compact.

# Conflicts: # Utilities/CodeGeneration/EdFi.Ods.CodeGen.Tests/Approval/5.2.0/DataStandard_520_ApprovalTests.Verify.Standard_Std_5.2.0_Models_Entities_Entities.generated.approved.cs

…roblems with single process-based code generation of multiple standards. Attach test reports from CodeGen processing for better error reporting. Exceptions occurring during code generation are now exposed to tests via a static LastException property.

…h derived resources.

…olumns are renamed.

…e key is defined on the base type so that query based on AK values can be properly constructed. Updated approval tests.

… during GetByKey where item has been reloaded through deserialization and the serialized data doesn't include the CreateDate value.

…logic.

… serialization.

…logic.

… "role" name for obtaining the NHibernate persister.

…(even before they are assigned by NHibernate), Student/Descriptor surrogate key resolution, and other edge case scenarios (like correct support for implicit extensions).

…ed approval tests.

…sisted USIs to be cleared, causing authorization failures.

…iately, and failing to resolve deserialized USIs into UniqueIds correctly.

…erived types when collections are inherited from a base type.

…persisters for implicit extensions.

…g surrogate ids after deserialization when the entity doesn't have it serialized yet.

…rence re-associated.

…setup steps.

…ink` objects.

…Hibernate in case of deserialization failure.

… to allow them to surface during Postman test execution.

…ing data created without the extensions.

…serializing data created without the extensions.

… behind Extensions feature.

…feature being disabled.

…timized around serialized data feature setting.

…mns.

This reverts commit af78a16.

axelmarquezh · 2024-11-21T21:10:53Z

Application/EdFi.Ods.Features/SerializedData/SerializedDataNHibernateConfigurationActivity.cs

+            // Maps the CreatedByOwnershipTokenID column dynamically
+            // Requires there be a property on the base entity already
+            // nHibernate wraps property getter exception in PropertyAccessException if any
+            // underlying mapped properties are set to access "none", due to an invoke exception being triggered.
+            // generated = "always" to avoid nHibernate trying to set values for it
+            // <property name="CreatedByOwnershipTokenID" column="ChangeVersion" type="long" not-null="true" generated="always" />


Optional: This comment doesn't reflect the reality, seems to have been copy/pasted.

gmcelhanon force-pushed the ODS-6551 branch 2 times, most recently from 41e95cf to 5e8ad8a Compare November 12, 2024 20:34

gmcelhanon force-pushed the ODS-6551 branch from bccf954 to bf1df56 Compare November 18, 2024 23:48

gmcelhanon marked this pull request as ready for review November 20, 2024 04:32

gmcelhanon requested a review from a team as a code owner November 20, 2024 04:32

gmcelhanon commented Nov 20, 2024

View reviewed changes

gmcelhanon added 24 commits November 22, 2024 13:07

Initial implementation of prepared serialized data feature.

5e223a2

Updated approval tests.

c776fbd

# Conflicts: # Utilities/CodeGeneration/EdFi.Ods.CodeGen.Tests/Approval/5.2.0/DataStandard_520_ApprovalTests.Verify.Standard_Std_5.2.0_Models_Entities_Entities.generated.approved.cs

Fixed ambiguous column issue in SQL generated in GetByKey queries wit…

25b9893

…h derived resources.

Refined GetByKey query approach after encountering issues where key c…

81904ae

…olumns are renamed.

Enhanced IHasAlternateKeyValues to include indicator as to whether th…

c41aab6

…e key is defined on the base type so that query based on AK values can be properly constructed. Updated approval tests.

Attempted fix to an issue with possible erroneous transient detection…

7d96951

… during GetByKey where item has been reloaded through deserialization and the serialized data doesn't include the CreateDate value.

Fixed Polly retry logic for deadlocks.

4816ab9

Adjustments after rebase.

fcb7635

Refinement of the Polly policy for the deadlocked transactions retry …

737a906

…logic.

Refinement of the Polly policy for the deadlocked transactions retry …

16e366a

…logic.

Refinement of the Polly policy for the deadlocked transactions retry …

0b4c49b

…logic.

Added logic to catch and log any exception that may occur during data…

66df31b

… serialization.

Refinement of the Polly policy for the deadlocked transactions retry …

700aa60

…logic.

Refinements to NHibernate persistence components for data serialization.

4c422f4

Fix code gen to use correct property name for deriving the collection…

58e6950

… "role" name for obtaining the NHibernate persister.

Adding support for serializing child entities with CreateDate values …

6f492a6

…(even before they are assigned by NHibernate), Student/Descriptor surrogate key resolution, and other edge case scenarios (like correct support for implicit extensions).

Updated approval tests.

f58992b

Fixed issue with deserializing AggregateExtensions collections. Updat…

322285e

…ed approval tests.

Fixed CodeGen issue with serializing UniqueIds on usages, causing per…

69c8d91

…sisted USIs to be cleared, causing authorization failures.

Fix issue with Get Many requests not deserializing the entities immed…

2d9bc45

…iately, and failing to resolve deserialized USIs into UniqueIds correctly.

Fixed role name construction for obtaining collection persisters on d…

482c54f

…erived types when collections are inherited from a base type.

Fixed issue just introduced for failed attempts to obtain collection …

57e0168

…persisters for implicit extensions.

Reworked the deserialization and simplified the approach to populatin…

c38dfe7

…g surrogate ids after deserialization when the entity doesn't have it serialized yet.

gmcelhanon added 20 commits November 22, 2024 13:07

Fixed issues with identifying child items correctly after parent refe…

054d516

…rence re-associated.

Fixed issues with updating primary keys with serialized data enabled.

382cd45

Modified Postman setup step to ensure descriptors are created during …

91e6d3f

…setup steps.

Added Polly retry policy for establishing a database connection.

2c2e61c

Updated response tests to be (temporarily) tolerant of issues with `l…

7deb75f

…ink` objects.

Some minor cleanup and enhancements to tests.

963530d

Update all deserialization usages to allow fallback to loading with N…

1394416

…Hibernate in case of deserialization failure.

Temporarily disable fallback behavior from deserialization exceptions…

262d609

… to allow them to surface during Postman test execution.

Change commit for template database data.

af78a16

Fixed Postman test.

61ddfbb

Added support for proper initialization of extensions when deserializ…

683d755

…ing data created without the extensions.

Fix the added support for proper initialization of extensions when de…

012477a

…serializing data created without the extensions.

Fixed deserialization issue.

f091699

Added INFO log statement before each conditional module invocation.

d0f22d9

Updated code for better isolation of component usage and registration…

819d713

… behind Extensions feature.

Fixed extensions logic during deserialization tolerant of extensions …

e53dc05

…feature being disabled.

Change implementation around GetEntitiesBySpecification to be more op…

6c93d15

…timized around serialized data feature setting.

Changes after self-code review of the PR.

fa53ca3

Updated approval tests.

00fcc4e

Removed manually generated script files for adding AggregateData colu…

1a0f89d

…mns.

gmcelhanon force-pushed the ODS-6551 branch from b18146e to 1a0f89d Compare November 22, 2024 20:55

gmcelhanon added 2 commits November 22, 2024 15:03

Revert "Change commit for template database data."

d9517ec

This reverts commit af78a16.

Updated approval tests.

acfc1f2

mjaksn approved these changes Nov 22, 2024

View reviewed changes

axelmarquezh approved these changes Nov 22, 2024

View reviewed changes

vimayya approved these changes Nov 22, 2024

View reviewed changes

mjaksn merged commit aa35e6e into main Nov 22, 2024
41 checks passed

mjaksn deleted the ODS-6551 branch November 22, 2024 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ODS-6551] Support feature for optimized use of serialized aggregate data #1184

[ODS-6551] Support feature for optimized use of serialized aggregate data #1184

gmcelhanon commented Nov 9, 2024 •

edited

Loading

gmcelhanon left a comment

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

gmcelhanon Nov 20, 2024

axelmarquezh Nov 21, 2024 •

edited

Loading


		namespace EdFi.Ods.Common.Database;

		public static class ColumnNames

[ODS-6551] Support feature for optimized use of serialized aggregate data #1184

[ODS-6551] Support feature for optimized use of serialized aggregate data #1184

Conversation

gmcelhanon commented Nov 9, 2024 • edited Loading

gmcelhanon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axelmarquezh Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

gmcelhanon commented Nov 9, 2024 •

edited

Loading

axelmarquezh Nov 21, 2024 •

edited

Loading