From 4cc38037d800b7730719b6d533f34e2492741a10 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 18 May 2026 15:04:37 -0700 Subject: [PATCH 1/2] Fix Extensible Entity Extensions --- .../ExtensibleEntity.cs | 89 ++++++++++++- .../ExtensibleEntityExtensions.cs | 8 +- .../docs/changelog/v1.0.0.md | 1 + .../docs/core/extensible-entity.md | 2 + .../Pages/Admin/Settings/Index.razor | 13 +- .../Admin/Controllers/SettingsController.cs | 2 +- .../Areas/Admin/Views/Settings/Index.cshtml | 12 ++ .../Models/ExtensibleEntityExtensionsTests.cs | 122 ++++++++++++++++++ .../Framework/Mvc/AIProfileViewModelTests.cs | 30 +++-- 9 files changed, 258 insertions(+), 21 deletions(-) diff --git a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs index d039c775..52326be6 100644 --- a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs +++ b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs @@ -1,3 +1,5 @@ +using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; namespace CrestApps.Core; @@ -5,13 +7,94 @@ namespace CrestApps.Core; /// /// Base class for entities that support dynamic extensible properties. /// -public abstract class ExtensibleEntity +public abstract class ExtensibleEntity : IJsonOnDeserialized, IJsonOnSerialized, IJsonOnSerializing { + private IDictionary _serializedProperties; + /// /// Gets or sets the dictionary of additional properties that are not explicitly - /// declared on the entity. Values are captured from JSON extension data during - /// deserialization and are round-tripped back on serialization. + /// declared on the entity. /// [JsonExtensionData] public IDictionary Properties { get; set; } = new Dictionary(); + + void IJsonOnDeserialized.OnDeserialized() + { + var properties = new Dictionary(StringComparer.OrdinalIgnoreCase); + + if (Properties.TryGetValue(nameof(Properties), out var propertyValue)) + { + MergeNestedProperties(properties, propertyValue); + } + + Properties = properties; + } + + void IJsonOnSerializing.OnSerializing() + { + _serializedProperties = Properties ?? new Dictionary(StringComparer.OrdinalIgnoreCase); + Properties = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [nameof(Properties)] = JsonSerializer.SerializeToNode(_serializedProperties, ExtensibleEntityExtensions.JsonSerializerOptions)?.DeepClone() as JsonObject ?? [], + }; + } + + void IJsonOnSerialized.OnSerialized() + { + Properties = _serializedProperties ?? new Dictionary(StringComparer.OrdinalIgnoreCase); + _serializedProperties = null; + } + + private static void MergeNestedProperties( + Dictionary properties, + object propertyValue) + { + if (propertyValue is null) + { + return; + } + + if (propertyValue is JsonObject propertyObject) + { + foreach (var property in propertyObject) + { + properties[property.Key] = property.Value?.DeepClone(); + } + + return; + } + + if (propertyValue is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) + { + foreach (var property in jsonElement.EnumerateObject()) + { + properties[property.Name] = property.Value.Clone(); + } + + return; + } + + if (propertyValue is IDictionary propertyDictionary) + { + foreach (var property in propertyDictionary) + { + properties[property.Key] = CloneExtensionValue(property.Value); + } + + return; + } + + throw new JsonException($"'{nameof(Properties)}' must be a JSON object."); + } + + private static object CloneExtensionValue(object value) + { + return value switch + { + null => null, + JsonNode jsonNode => jsonNode.DeepClone(), + JsonElement jsonElement => jsonElement.Clone(), + _ => value, + }; + } } diff --git a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityExtensions.cs b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityExtensions.cs index 44fa5c32..cfa27abf 100644 --- a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityExtensions.cs +++ b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityExtensions.cs @@ -44,8 +44,8 @@ public static T GetOrCreate(this ExtensibleEntity entity, JsonSerializerOptio var key = typeof(T).Name; return entity.Properties.TryGetValue(key, out var value) - ? DeserializeValue(value, jsonSerializerOptions ?? _jsonOptions) ?? new T() - : new T(); + ? DeserializeValue(value, jsonSerializerOptions ?? _jsonOptions) ?? new T() + : new T(); } /// @@ -57,8 +57,8 @@ public static T Get(this ExtensibleEntity entity, string name, JsonSerializer ArgumentException.ThrowIfNullOrEmpty(name); return entity.Properties.TryGetValue(name, out var value) - ? DeserializeValue(value, jsonSerializerOptions ?? _jsonOptions) - : default; + ? DeserializeValue(value, jsonSerializerOptions ?? _jsonOptions) + : default; } /// diff --git a/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md b/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md index 4c17ca40..1148db26 100644 --- a/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md +++ b/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md @@ -67,3 +67,4 @@ description: Initial standalone release notes for the CrestApps.Core repository. - treats valid post-session JSON with an empty `tasks` array as an explicit structured-result failure, persists that clearer error in `PostSessionResults`, and strengthens the shared post-session prompts so every configured task must still return a result even when no tool call is needed - stops serializing redundant top-level post-session error fields on `PostSessionResult`, keeps attempt-specific failures in `AttemptHistory`, retries tool-enabled runs through structured recovery when the model returns invalid task entries such as blank names or blank values, and falls back to a no-tools structured retry when the tool path never actually invoked a tool - refreshes site-settings-backed options through the standard `IOptionsMonitor<>` pipeline by documenting the minimal `IOptionsChangeTokenSource<>` pattern for custom hosts, and moves uploaded AI document vector indexing into a shared `DefaultAIDocumentIndexingService` so MVC and Blazor no longer carry duplicate sample-only indexer implementations +- writes and reads `ExtensibleEntity.Properties` only through the nested `Properties` JSON object instead of flattening typed metadata onto the document root diff --git a/src/CrestApps.Core.Docs/docs/core/extensible-entity.md b/src/CrestApps.Core.Docs/docs/core/extensible-entity.md index 992abab2..2209b83b 100644 --- a/src/CrestApps.Core.Docs/docs/core/extensible-entity.md +++ b/src/CrestApps.Core.Docs/docs/core/extensible-entity.md @@ -13,6 +13,8 @@ description: Learn how to use ExtensibleEntity for dynamic property storage and `ExtensibleEntity` provides a `Properties` dictionary that allows you to attach arbitrary strongly-typed data to any entity without changing its schema. This powers features like AI profile metadata, chat session annotations, and custom application data. +Serialized entities keep extensible values inside the nested `Properties` object. Root-level flattened extension data is not treated as valid extensible metadata. + ## Quick Reference | Method | When to Use | diff --git a/src/Startup/CrestApps.Core.Blazor.Web/Components/Pages/Admin/Settings/Index.razor b/src/Startup/CrestApps.Core.Blazor.Web/Components/Pages/Admin/Settings/Index.razor index 7a16a549..a18a548e 100644 --- a/src/Startup/CrestApps.Core.Blazor.Web/Components/Pages/Admin/Settings/Index.razor +++ b/src/Startup/CrestApps.Core.Blazor.Web/Components/Pages/Admin/Settings/Index.razor @@ -347,6 +347,17 @@ else
The default number of matched chunks or documents to include when a profile does not override it.
+
+ + + @foreach (var retrievalMode in Enum.GetValues()) + { + + } + +
Controls whether document retrieval returns matching chunks directly or expands results hierarchically by parent document.
+
+ @@ -948,7 +959,7 @@ else { IndexProfileName = _model.DocumentIndexProfileName?.Trim(), TopN = _model.DocumentTopN, - RetrievalMode = SiteSettings.Get().RetrievalMode, + RetrievalMode = _model.DocumentRetrievalMode, }); SiteSettings.Set(new AIDataSourceSettings diff --git a/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Controllers/SettingsController.cs b/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Controllers/SettingsController.cs index 3ec12ba2..2388d497 100644 --- a/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Controllers/SettingsController.cs +++ b/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Controllers/SettingsController.cs @@ -274,7 +274,7 @@ public async Task Save(SettingsViewModel model) { IndexProfileName = model.DocumentIndexProfileName?.Trim(), TopN = model.DocumentTopN, - RetrievalMode = _siteSettings.Get().RetrievalMode, + RetrievalMode = model.DocumentRetrievalMode, }); _siteSettings.Set(new AIDataSourceSettings diff --git a/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Views/Settings/Index.cshtml b/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Views/Settings/Index.cshtml index a532f125..189e2bf3 100644 --- a/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Views/Settings/Index.cshtml +++ b/src/Startup/CrestApps.Core.Mvc.Web/Areas/Admin/Views/Settings/Index.cshtml @@ -1,4 +1,5 @@ @using CrestApps.Core.AI.Claude.Models +@using CrestApps.Core.AI.Documents.Models @model CrestApps.Core.Mvc.Web.Areas.Admin.ViewModels.SettingsViewModel @{ ViewData["Title"] = "AI Settings"; @@ -257,6 +258,17 @@
The default number of matched chunks or documents to include when a profile does not override it.
+
+ + +
Controls whether document retrieval returns matching chunks directly or expands results hierarchically by parent document.
+
+ diff --git a/tests/CrestApps.Core.Tests/Core/Models/ExtensibleEntityExtensionsTests.cs b/tests/CrestApps.Core.Tests/Core/Models/ExtensibleEntityExtensionsTests.cs index 9a59f354..7961e982 100644 --- a/tests/CrestApps.Core.Tests/Core/Models/ExtensibleEntityExtensionsTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Models/ExtensibleEntityExtensionsTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; namespace CrestApps.Core.Tests.Core.Models; @@ -22,6 +23,126 @@ public void Get_UsesProvidedSerializerOptions() Assert.Equal(TestMode.SecondValue, settings.Mode); } + [Fact] + public void Serialize_WritesTypedMetadataInsidePropertiesObject() + { + var entity = new TestExtensibleEntity + { + Id = "entity-1", + }; + + entity.Put(new TestSettings + { + Mode = TestMode.SecondValue, + }); + + var json = JsonSerializer.Serialize(entity); + var node = JsonNode.Parse(json)?.AsObject(); + + Assert.NotNull(node); + Assert.Equal("entity-1", node[nameof(TestExtensibleEntity.Id)]?.GetValue()); + Assert.Null(node[nameof(TestSettings)]); + Assert.Equal( + "SecondValue", + node[nameof(ExtensibleEntity.Properties)]?[nameof(TestSettings)]?[nameof(TestSettings.Mode)]?.GetValue()); + } + + [Fact] + public void Deserialize_ReadsTypedMetadataFromNestedPropertiesObject() + { + const string json = """ + { + "Id": "entity-1", + "Properties": { + "TestSettings": { + "Mode": "SecondValue" + } + } + } + """; + + var entity = JsonSerializer.Deserialize(json); + + Assert.NotNull(entity); + Assert.True(entity.TryGet(out var settings)); + Assert.NotNull(settings); + Assert.Equal(TestMode.SecondValue, settings.Mode); + } + + [Fact] + public void Deserialize_IgnoresFlattenedPropertiesOutsideNestedPropertiesObject() + { + const string json = """ + { + "Id": "entity-1", + "TestSettings": { + "Mode": "SecondValue" + } + } + """; + + var entity = JsonSerializer.Deserialize(json); + + Assert.NotNull(entity); + Assert.False(entity.TryGet(out _)); + } + + [Fact] + public void Put_RoundTripsTypedMetadataOnlyThroughPropertiesObject() + { + var entity = new TestExtensibleEntity + { + Id = "entity-1", + }; + + entity.Put(new TestSettings + { + Mode = TestMode.SecondValue, + }); + + var serializedJson = JsonSerializer.Serialize(entity); + var serializedNode = JsonNode.Parse(serializedJson)?.AsObject(); + + Assert.NotNull(serializedNode); + Assert.Null(serializedNode[nameof(TestSettings)]); + Assert.Equal("entity-1", serializedNode[nameof(TestExtensibleEntity.Id)]?.GetValue()); + + var propertiesNode = serializedNode[nameof(ExtensibleEntity.Properties)]?.AsObject(); + + Assert.NotNull(propertiesNode); + Assert.Single(propertiesNode); + Assert.Equal( + "SecondValue", + propertiesNode[nameof(TestSettings)]?[nameof(TestSettings.Mode)]?.GetValue()); + Assert.Null(propertiesNode[nameof(TestSettings.Mode)]); + + const string roundTripJson = """ + { + "Id": "entity-1", + "TestSettings": { + "Mode": "FirstValue" + }, + "Properties": { + "TestSettings": { + "Mode": "SecondValue" + } + } + } + """; + + var roundTrippedEntity = JsonSerializer.Deserialize(roundTripJson); + + Assert.NotNull(roundTrippedEntity); + Assert.True(roundTrippedEntity.TryGet(out var settings)); + Assert.NotNull(settings); + Assert.Equal(TestMode.SecondValue, settings.Mode); + + var property = Assert.Single(roundTrippedEntity.Properties); + Assert.Equal(nameof(TestSettings), property.Key); + Assert.False(roundTrippedEntity.Properties.ContainsKey(nameof(ExtensibleEntity.Properties))); + Assert.False(roundTrippedEntity.Properties.ContainsKey(nameof(TestSettings.Mode))); + } + private static JsonSerializerOptions CreateCamelCaseEnumOptions() { var options = new JsonSerializerOptions(ExtensibleEntityJsonOptions.CreateDefaultSerializerOptions()); @@ -34,6 +155,7 @@ private static JsonSerializerOptions CreateCamelCaseEnumOptions() private sealed class TestExtensibleEntity : ExtensibleEntity { + public string Id { get; set; } } private sealed class TestSettings diff --git a/tests/CrestApps.Core.Tests/Framework/Mvc/AIProfileViewModelTests.cs b/tests/CrestApps.Core.Tests/Framework/Mvc/AIProfileViewModelTests.cs index ec1ce53f..60dd988f 100644 --- a/tests/CrestApps.Core.Tests/Framework/Mvc/AIProfileViewModelTests.cs +++ b/tests/CrestApps.Core.Tests/Framework/Mvc/AIProfileViewModelTests.cs @@ -277,7 +277,7 @@ public void FromProfile_WhenMemoryMetadataExists_ShouldReadValue() } [Fact] - public void FromProfile_ReadsFlattenedExtensionDataAfterReload() + public void FromProfile_ReadsNestedExtensionDataAfterReload() { var json = """ { @@ -293,15 +293,17 @@ public void FromProfile_ReadsFlattenedExtensionDataAfterReload() } }, "ItemId":"3e169d42eee44ccb956d61cd046daf43", - "DataSourceMetadata":{ - "DataSourceId":"4kwqvhkg2p1jx30rrjn7vqv6da" - }, - "AIDataSourceRagMetadata":{ - "IsInScope":true - }, - "DocumentsMetadata":{ - "Documents":[], - "DocumentTopN":5 + "Properties":{ + "DataSourceMetadata":{ + "DataSourceId":"4kwqvhkg2p1jx30rrjn7vqv6da" + }, + "AIDataSourceRagMetadata":{ + "IsInScope":true + }, + "DocumentsMetadata":{ + "Documents":[], + "DocumentTopN":5 + } } } """; @@ -338,7 +340,7 @@ public void As_ShouldReadDictionaryBackedExtensionData() } [Fact] - public void JsonSerialization_ShouldKeepSettingsNestedAndMetadataFlattened() + public void JsonSerialization_ShouldKeepSettingsAndMetadataInsideNestedPropertiesObject() { var profile = new AIProfile { @@ -353,10 +355,14 @@ public void JsonSerialization_ShouldKeepSettingsNestedAndMetadataFlattened() profile.Alter(metadata => metadata.DataSourceId = "serialized-source"); var json = JsonSerializer.Serialize(profile); + var node = JsonNode.Parse(json)?.AsObject(); Assert.Contains("\"Settings\":", json, StringComparison.Ordinal); Assert.Contains("\"AIProfileSettings\":", json, StringComparison.Ordinal); - Assert.Contains("\"DataSourceMetadata\":", json, StringComparison.Ordinal); + Assert.Contains("\"Properties\":", json, StringComparison.Ordinal); + Assert.NotNull(node); + Assert.Null(node[nameof(DataSourceMetadata)]); + Assert.NotNull(node[nameof(ExtensibleEntity.Properties)]?[nameof(DataSourceMetadata)]); var reloaded = JsonSerializer.Deserialize(json); From f6ee5e50952110d4b14f05d196d7006a2bd62f7c Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 18 May 2026 15:29:32 -0700 Subject: [PATCH 2/2] simplify --- .../ExtensibleEntity.cs | 96 ++----------------- ...ExtensibleEntityPropertiesJsonConverter.cs | 95 ++++++++++++++++++ 2 files changed, 102 insertions(+), 89 deletions(-) create mode 100644 src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityPropertiesJsonConverter.cs diff --git a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs index 52326be6..7fa245e6 100644 --- a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs +++ b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntity.cs @@ -1,5 +1,5 @@ -using System.Text.Json; -using System.Text.Json.Nodes; +using System; +using System.Collections.Generic; using System.Text.Json.Serialization; namespace CrestApps.Core; @@ -7,94 +7,12 @@ namespace CrestApps.Core; /// /// Base class for entities that support dynamic extensible properties. /// -public abstract class ExtensibleEntity : IJsonOnDeserialized, IJsonOnSerialized, IJsonOnSerializing +public abstract class ExtensibleEntity { - private IDictionary _serializedProperties; - /// - /// Gets or sets the dictionary of additional properties that are not explicitly - /// declared on the entity. + /// Gets or sets the dictionary of additional properties stored under the + /// Properties JSON object. /// - [JsonExtensionData] - public IDictionary Properties { get; set; } = new Dictionary(); - - void IJsonOnDeserialized.OnDeserialized() - { - var properties = new Dictionary(StringComparer.OrdinalIgnoreCase); - - if (Properties.TryGetValue(nameof(Properties), out var propertyValue)) - { - MergeNestedProperties(properties, propertyValue); - } - - Properties = properties; - } - - void IJsonOnSerializing.OnSerializing() - { - _serializedProperties = Properties ?? new Dictionary(StringComparer.OrdinalIgnoreCase); - Properties = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - [nameof(Properties)] = JsonSerializer.SerializeToNode(_serializedProperties, ExtensibleEntityExtensions.JsonSerializerOptions)?.DeepClone() as JsonObject ?? [], - }; - } - - void IJsonOnSerialized.OnSerialized() - { - Properties = _serializedProperties ?? new Dictionary(StringComparer.OrdinalIgnoreCase); - _serializedProperties = null; - } - - private static void MergeNestedProperties( - Dictionary properties, - object propertyValue) - { - if (propertyValue is null) - { - return; - } - - if (propertyValue is JsonObject propertyObject) - { - foreach (var property in propertyObject) - { - properties[property.Key] = property.Value?.DeepClone(); - } - - return; - } - - if (propertyValue is JsonElement jsonElement && jsonElement.ValueKind == JsonValueKind.Object) - { - foreach (var property in jsonElement.EnumerateObject()) - { - properties[property.Name] = property.Value.Clone(); - } - - return; - } - - if (propertyValue is IDictionary propertyDictionary) - { - foreach (var property in propertyDictionary) - { - properties[property.Key] = CloneExtensionValue(property.Value); - } - - return; - } - - throw new JsonException($"'{nameof(Properties)}' must be a JSON object."); - } - - private static object CloneExtensionValue(object value) - { - return value switch - { - null => null, - JsonNode jsonNode => jsonNode.DeepClone(), - JsonElement jsonElement => jsonElement.Clone(), - _ => value, - }; - } + [JsonConverter(typeof(ExtensibleEntityPropertiesJsonConverter))] + public IDictionary Properties { get; set; } = new Dictionary(StringComparer.OrdinalIgnoreCase); } diff --git a/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityPropertiesJsonConverter.cs b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityPropertiesJsonConverter.cs new file mode 100644 index 00000000..597d772c --- /dev/null +++ b/src/Abstractions/CrestApps.Core.Abstractions/ExtensibleEntityPropertiesJsonConverter.cs @@ -0,0 +1,95 @@ +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Text.Json.Serialization; + +namespace CrestApps.Core; + +/// +/// Serializes and deserializes as a normal nested +/// JSON object while keeping its values inside the property bag. +/// +internal sealed class ExtensibleEntityPropertiesJsonConverter : JsonConverter> +{ + /// + /// Reads the property bag from a nested JSON object. + /// + /// The JSON reader. + /// The type to convert. + /// The serializer options. + /// The deserialized property bag. + public override IDictionary Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + return new Dictionary(StringComparer.OrdinalIgnoreCase); + } + + var node = JsonNode.Parse(ref reader)?.AsObject(); + + if (node is null) + { + return new Dictionary(StringComparer.OrdinalIgnoreCase); + } + + var properties = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var property in node) + { + properties[property.Key] = property.Value?.DeepClone(); + } + + return properties; + } + + /// + /// Writes the property bag as a nested JSON object. + /// + /// The JSON writer. + /// The property bag value. + /// The serializer options. + public override void Write(Utf8JsonWriter writer, IDictionary value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + + foreach (var property in value ?? new Dictionary(StringComparer.OrdinalIgnoreCase)) + { + writer.WritePropertyName(property.Key); + WritePropertyValue(writer, property.Value); + } + + writer.WriteEndObject(); + } + + /// + /// Writes a single property bag value using the extensible entity serializer options. + /// + /// The JSON writer. + /// The property value. + private static void WritePropertyValue(Utf8JsonWriter writer, object value) + { + if (value is null) + { + writer.WriteNullValue(); + + return; + } + + if (value is JsonNode jsonNode) + { + jsonNode.WriteTo(writer, ExtensibleEntityExtensions.JsonSerializerOptions); + + return; + } + + if (value is JsonElement jsonElement) + { + jsonElement.WriteTo(writer); + + return; + } + + JsonSerializer.Serialize(writer, value, value.GetType(), ExtensibleEntityExtensions.JsonSerializerOptions); + } +}