feat(QueryPageOptions): SearchModel support serialize#7327
Conversation
Reviewer's GuideImplements typed, round‑trippable serialization for QueryPageOptions.SearchModel and hardens type resolution in JSON converters by emitting assembly‑qualified type names and using a safe type loader, with corresponding unit tests added. Sequence diagram for QueryPageOptions.SearchModel JSON round-tripsequenceDiagram
participant Caller
participant JsonSerializer
participant JsonQueryPageOptionsConverter
participant Utf8JsonWriter
participant Utf8JsonReader
participant TypeExtensions
Caller->>JsonSerializer: Serialize(QueryPageOptions)
JsonSerializer->>JsonQueryPageOptionsConverter: Write(writer, options)
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WriteStartObject
alt SearchModel is not null
JsonQueryPageOptionsConverter->>JsonQueryPageOptionsConverter: WriteSearchModel(writer, SearchModel, options)
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WriteStartObject(searchModel)
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WriteString(type, SearchModel.GetType().AssemblyQualifiedName)
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WritePropertyName(value)
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WriteRawValue(Serialize(SearchModel, options))
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WriteEndObject
end
JsonQueryPageOptionsConverter->>Utf8JsonWriter: WriteEndObject
JsonSerializer-->>Caller: JsonPayload
Caller->>JsonSerializer: Deserialize(JsonPayload, QueryPageOptions)
JsonSerializer->>JsonQueryPageOptionsConverter: Read(ref reader, options)
JsonQueryPageOptionsConverter->>Utf8JsonReader: Read searchModel property
JsonQueryPageOptionsConverter->>JsonQueryPageOptionsConverter: ReadSearchModel(ref reader, options)
JsonQueryPageOptionsConverter->>Utf8JsonReader: Read type
JsonQueryPageOptionsConverter->>TypeExtensions: GetSafeType(typeName)
TypeExtensions-->>JsonQueryPageOptionsConverter: ResolvedType or null
alt ResolvedType is not null
JsonQueryPageOptionsConverter->>JsonSerializer: Deserialize(ref reader, ResolvedType, options)
JsonSerializer-->>JsonQueryPageOptionsConverter: StrongTypedInstance
JsonQueryPageOptionsConverter->>JsonQueryPageOptionsConverter: Set SearchModel = StrongTypedInstance
else ResolvedType is null
JsonQueryPageOptionsConverter->>JsonSerializer: Deserialize<object>(ref reader, options)
JsonSerializer-->>JsonQueryPageOptionsConverter: FallbackObject
JsonQueryPageOptionsConverter->>JsonQueryPageOptionsConverter: Set SearchModel = FallbackObject
end
JsonQueryPageOptionsConverter-->>JsonSerializer: QueryPageOptions
JsonSerializer-->>Caller: QueryPageOptions instance
Sequence diagram for FilterKeyValueAction fieldValueType handlingsequenceDiagram
participant Caller
participant JsonSerializer
participant JsonFilterKeyValueActionConverter
participant Utf8JsonWriter
participant Utf8JsonReader
participant TypeExtensions
Caller->>JsonSerializer: Serialize(FilterKeyValueAction)
JsonSerializer->>JsonFilterKeyValueActionConverter: Write(writer, options)
JsonFilterKeyValueActionConverter->>Utf8JsonWriter: WriteStartObject
JsonFilterKeyValueActionConverter->>Utf8JsonWriter: WriteString(fieldKey, value.FieldKey)
JsonFilterKeyValueActionConverter->>JsonFilterKeyValueActionConverter: WriteFieldValueType(writer, value, options)
alt FieldValue is not null
JsonFilterKeyValueActionConverter->>Utf8JsonWriter: WriteString(fieldValueType, FieldValue.GetType().AssemblyQualifiedName)
end
JsonFilterKeyValueActionConverter->>Utf8JsonWriter: WritePropertyName(fieldValue)
JsonFilterKeyValueActionConverter->>Utf8JsonWriter: WriteRawValue(Serialize(FieldValue, options))
JsonFilterKeyValueActionConverter->>Utf8JsonWriter: WriteEndObject
JsonSerializer-->>Caller: JsonPayload
Caller->>JsonSerializer: Deserialize(JsonPayload, FilterKeyValueAction)
JsonSerializer->>JsonFilterKeyValueActionConverter: Read(ref reader, options)
JsonFilterKeyValueActionConverter->>Utf8JsonReader: Read fieldKey
JsonFilterKeyValueActionConverter->>Utf8JsonReader: Read fieldValueType
JsonFilterKeyValueActionConverter->>TypeExtensions: GetSafeType(typeName)
TypeExtensions-->>JsonFilterKeyValueActionConverter: ResolvedType or null
JsonFilterKeyValueActionConverter->>Utf8JsonReader: Read fieldValue
alt ResolvedType is not null
JsonFilterKeyValueActionConverter->>JsonSerializer: Deserialize(ref reader, ResolvedType, options)
JsonSerializer-->>JsonFilterKeyValueActionConverter: StrongTypedFieldValue
else ResolvedType is null
JsonFilterKeyValueActionConverter->>JsonSerializer: Deserialize<object>(ref reader, options)
JsonSerializer-->>JsonFilterKeyValueActionConverter: FallbackFieldValue
end
JsonFilterKeyValueActionConverter-->>JsonSerializer: FilterKeyValueAction instance
JsonSerializer-->>Caller: FilterKeyValueAction
Class diagram for updated JSON converters and type resolutionclassDiagram
class QueryPageOptions {
object SearchModel
int PageIndex
int PageItems
bool IsPage
bool IsPageable
bool IsAdvanceSearch
bool IsEnableSorting
bool IsFilter
bool IsEnd
bool IsFirstPage
bool IsLastPage
bool IsMobile
bool IsDialog
bool IsVirtualScroll
bool IsFixedHeader
bool IsFixedFooter
bool IsMultipleSelect
bool IsTree
bool IsExcel
bool IsExport
bool IsSearch
bool IsAdvanceSearchDialog
bool IsFixedColumns
bool IsShowFooter
bool IsShowTopToolbar
bool IsShowExport
bool IsShowColumnList
bool IsShowSearch
bool IsShowToolbar
bool IsShowColumnVisible
bool IsShowColumnExtend
bool IsShowDefaultButtons
bool IsShowPagination
bool IsTreeLoaded
bool IsToolbarTemplate
bool IsShowAdd
bool IsShowEdit
bool IsShowDelete
bool IsShowRefresh
bool IsShowToggle
bool IsShowLock
bool IsShowDetail
bool IsShowExtendButtons
bool IsShowExportButtons
bool IsShowPrint
bool IsShowColumnReset
bool IsShowExcel
bool IsShowPdf
bool IsShowCsv
bool IsShowCopy
bool IsShowAggregates
bool IsShowEmpty
bool IsShowCheckbox
bool IsShowRadio
bool IsShowLineNumber
bool IsShowAdvanceSearch
bool IsShowSearchText
bool IsShowSearchButton
bool IsShowColumn
bool IsShowDefaultGroup
bool IsShowTree
bool IsShowColumnWidth
bool IsShowExtendQuery
bool IsShowToast
bool IsShowColumnOptions
bool IsShowLoading
bool IsShowBooleanFilter
bool IsStriped
bool IsBordered
bool IsHover
bool IsShowToolbarTop
bool IsShowToolbarBottom
bool IsCardView
bool IsFixedToolbar
bool IsShowDetailRow
bool IsShowColumnListCheckbox
bool IsShowColumnListRadio
bool IsResizable
bool IsResizableColumn
bool IsResizableRow
bool IsResizableColumnWidth
bool IsResizableRowHeight
bool IsVirtualize
bool IsFixedVirtualize
bool IsFixedVirtualizeHeader
bool IsFixedVirtualizeFooter
bool IsFixedVirtualizeColumn
bool IsFixedVirtualizeRow
bool IsFixedVirtualizeColumnWidth
bool IsFixedVirtualizeRowHeight
bool IsFixedVirtualizeColumnTemplate
bool IsFixedVirtualizeRowTemplate
bool IsFixedVirtualizeDetailRow
bool IsFixedVirtualizeDetailRowTemplate
bool IsFixedVirtualizeToolbar
bool IsFixedVirtualizeToolbarTemplate
bool IsFixedVirtualizeFooterTemplate
bool IsFixedVirtualizeHeaderTemplate
bool IsFixedVirtualizeEmptyTemplate
bool IsFixedVirtualizeLoadingTemplate
bool IsFixedVirtualizeColumnOptions
bool IsFixedVirtualizeColumnOptionsTemplate
bool IsFixedVirtualizeColumnOptionsToolbar
bool IsFixedVirtualizeColumnOptionsToolbarTemplate
bool IsFixedVirtualizeColumnOptionsFooter
bool IsFixedVirtualizeColumnOptionsFooterTemplate
bool IsFixedVirtualizeColumnOptionsHeader
bool IsFixedVirtualizeColumnOptionsHeaderTemplate
bool IsFixedVirtualizeColumnOptionsEmptyTemplate
bool IsFixedVirtualizeColumnOptionsLoadingTemplate
}
class JsonQueryPageOptionsConverter {
+QueryPageOptions Read(Utf8JsonReader reader, JsonSerializerOptions options)
+void Write(Utf8JsonWriter writer, QueryPageOptions value, JsonSerializerOptions options)
-static void WriteSearchModel(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
-static void ReadSearchModel(Utf8JsonReader reader, QueryPageOptions value, JsonSerializerOptions options)
}
class FilterKeyValueAction {
string FieldKey
object FieldValue
}
class JsonFilterKeyValueActionConverter {
+FilterKeyValueAction Read(Utf8JsonReader reader, JsonSerializerOptions options)
+void Write(Utf8JsonWriter writer, FilterKeyValueAction value, JsonSerializerOptions options)
-static void WriteFieldValueType(Utf8JsonWriter writer, FilterKeyValueAction value, JsonSerializerOptions options)
}
class TypeExtensions {
+string GetUniqueTypeName(Type type)
+Type GetSafeType(string typeName)
}
class Utf8JsonWriter
class Utf8JsonReader
class JsonSerializerOptions
JsonQueryPageOptionsConverter --> QueryPageOptions : converts
JsonQueryPageOptionsConverter --> Utf8JsonWriter : writes
JsonQueryPageOptionsConverter --> Utf8JsonReader : reads
JsonQueryPageOptionsConverter --> TypeExtensions : uses GetSafeType
JsonFilterKeyValueActionConverter --> FilterKeyValueAction : converts
JsonFilterKeyValueActionConverter --> Utf8JsonWriter : writes
JsonFilterKeyValueActionConverter --> Utf8JsonReader : reads
JsonFilterKeyValueActionConverter --> TypeExtensions : uses GetSafeType
TypeExtensions ..> Type : uses
QueryPageOptions o-- object : SearchModel
FilterKeyValueAction o-- object : FieldValue
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In both
WriteSearchModeland thefieldValuewrite path, consider usingJsonSerializer.Serialize(writer, value, options)instead ofJsonSerializer.Serialize(...); writer.WriteRawValue(...)to avoid double serialization and rely on the serializer’s configured converters consistently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both `WriteSearchModel` and the `fieldValue` write path, consider using `JsonSerializer.Serialize(writer, value, options)` instead of `JsonSerializer.Serialize(...); writer.WriteRawValue(...)` to avoid double serialization and rely on the serializer’s configured converters consistently.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Converter/JsonQueryPageOptionConverter.cs:301-298` </location>
<code_context>
writer.WriteEndObject();
}
+
+ private static void WriteSearchModel(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
+ {
+ writer.WriteStartObject("searchModel");
+ writer.WriteString("type", value.GetType().AssemblyQualifiedName);
+ writer.WritePropertyName("value");
+ writer.WriteRawValue(JsonSerializer.Serialize(value, options));
+ writer.WriteEndObject();
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The new searchModel shape and embedded type information may break backward compatibility with existing JSON payloads.
Previously `searchModel` was serialized directly as its value (object/primitive/array). It’s now wrapped with type metadata:
```json
"searchModel": {
"type": "...AssemblyQualifiedName...",
"value": { ... }
}
```
`ReadSearchModel` assumes this new object shape; older payloads without the wrapper will be ignored (non-object token → no search model), causing silent data loss.
If compatibility with existing payloads is required, please either:
* Support both formats when reading (e.g., if `TokenType != StartObject`, fall back to `JsonSerializer.Deserialize<object>`), or
* Introduce a versioned field (e.g., `searchModelV2`) and keep the old behavior for `searchModel`.
Spelling this out will avoid subtle issues in mixed-version clients/servers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7327 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 747 747
Lines 32714 32760 +46
Branches 4534 4540 +6
=========================================
+ Hits 32714 32760 +46
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request adds serialization support for the SearchModel property in QueryPageOptions to enable proper round-trip serialization/deserialization. The implementation stores type information alongside the SearchModel value to preserve the concrete type during deserialization.
Key changes:
- Implements type-aware serialization for SearchModel by storing AssemblyQualifiedName alongside the value
- Refactors type deserialization to use a new shared
GetSafeTypeutility method - Adds comprehensive unit tests for SearchModel serialization scenarios including anonymous types, ITableSearchModel implementations, and invalid type handling
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTest/Extensions/QueryPageOptionsExtensionsTest.cs | Updated existing test to use concrete Foo type instead of anonymous object, added three new test methods for SearchModel serialization scenarios, and added FooSearchModel test helper class |
| src/BootstrapBlazor/Extensions/TypeExtensions.cs | Added new GetSafeType utility method to safely retrieve Type instances from type names with null/error handling |
| src/BootstrapBlazor/Converter/JsonQueryPageOptionConverter.cs | Refactored SearchModel serialization to include type metadata, extracted WriteSearchModel and ReadSearchModel helper methods |
| src/BootstrapBlazor/Converter/JsonFilterKeyValueActionConverter.cs | Refactored to use new GetSafeType utility and extracted WriteFieldValueType helper method for consistency |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bumped to 10.1.4-beta07 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ? $"{type.FullName}-{type.TypeHandle.Value}" | ||
| : $"{type.FullName}"; | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line should be removed to follow consistent code formatting conventions within this file.
| var foo = expected.SearchModel as Foo; | ||
| Assert.NotNull(foo); | ||
| Assert.Equal("Test1", foo.Name); |
There was a problem hiding this comment.
The test verifies that the Name property is correctly deserialized, but it does not verify that the Count property (which is also set to 2 on line 185) is correctly deserialized. Consider adding an assertion like Assert.Equal(2, foo.Count); after line 202 to ensure complete test coverage of the SearchModel deserialization.
| } | ||
| writer.WriteEndObject(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The WriteSearchModel method lacks XML documentation comments. Consider adding a summary that describes the method's purpose and documents its parameters (writer, value, options) to maintain consistency with the public API documentation style in this codebase.
| /// <summary> | |
| /// Serializes the specified search model object into a JSON structure with type information. | |
| /// </summary> | |
| /// <param name="writer">The <see cref="Utf8JsonWriter"/> to write the JSON to.</param> | |
| /// <param name="value">The search model object to serialize.</param> | |
| /// <param name="options">The serializer options to use when serializing the object.</param> |
| if (value.FieldValue != null) | ||
| { | ||
| var type = value.FieldValue.GetType(); | ||
| writer.WriteString("fieldValueType", type.AssemblyQualifiedName); |
There was a problem hiding this comment.
Using AssemblyQualifiedName for type serialization can potentially allow arbitrary type instantiation during deserialization if the JSON is received from untrusted sources. While the code uses GetSafeType with throwOnError: false, which prevents exceptions, it still allows any type that can be loaded to be instantiated. Consider implementing a whitelist of allowed types or adding validation to ensure only expected types can be deserialized through this mechanism.
| } | ||
|
|
||
| [Fact] | ||
| public void SearchModel_Serialize_NullType() |
There was a problem hiding this comment.
The test method name "SearchModel_Serialize_NullType" is misleading. The test does not check null type handling but rather checks handling of an invalid/non-existent type name (by replacing "FooSearchModel" with "FooSearchModel1"). A more accurate name would be "SearchModel_Serialize_InvalidType" or "SearchModel_Serialize_UnknownType".
| public void SearchModel_Serialize_NullType() | |
| public void SearchModel_Serialize_InvalidType() |
| writer.WriteRawValue(JsonSerializer.Serialize(value, options)); | ||
| writer.WriteEndObject(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The ReadSearchModel method lacks XML documentation comments. Consider adding a summary that describes the method's purpose and documents its parameters (reader, value, options) to maintain consistency with the public API documentation style in this codebase.
| /// <summary> | |
| /// Reads and deserializes the <c>searchModel</c> property from the JSON input and assigns it to the <see cref="QueryPageOptions.SearchModel"/> property of the specified <paramref name="value"/> instance. | |
| /// </summary> | |
| /// <param name="reader">The <see cref="Utf8JsonReader"/> to read from. Passed by reference.</param> | |
| /// <param name="value">The <see cref="QueryPageOptions"/> instance to assign the deserialized search model to.</param> | |
| /// <param name="options">The <see cref="JsonSerializerOptions"/> to use for deserialization.</param> |
|
|
||
| writer.WriteEndObject(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The WriteFieldValueType method lacks XML documentation comments. Consider adding a summary that describes the method's purpose and documents its parameters (writer, value, options) to maintain consistency with the public API documentation style in this codebase.
| /// <summary> | |
| /// Writes the type information of the <c>FieldValue</c> property to the JSON output if it is not null. | |
| /// </summary> | |
| /// <param name="writer">The <see cref="Utf8JsonWriter"/> to write to.</param> | |
| /// <param name="value">The <see cref="FilterKeyValueAction"/> instance containing the field value.</param> | |
| /// <param name="options">The <see cref="JsonSerializerOptions"/> to use for serialization.</param> |
| private static void WriteSearchModel(Utf8JsonWriter writer, object value, JsonSerializerOptions options) | ||
| { | ||
| writer.WriteStartObject("searchModel"); | ||
| writer.WriteString("type", value.GetType().AssemblyQualifiedName); |
There was a problem hiding this comment.
Using AssemblyQualifiedName for type serialization can potentially allow arbitrary type instantiation during deserialization if the JSON is received from untrusted sources. While the code uses GetSafeType with throwOnError: false, which prevents exceptions, it still allows any type that can be loaded to be instantiated. Consider implementing a whitelist of allowed types or adding validation to ensure only expected types can be deserialized through this mechanism.
| reader.Read(); | ||
| propertyName = reader.GetString(); | ||
| if (propertyName == "value") | ||
| { | ||
| reader.Read(); | ||
| if (type != null) | ||
| { | ||
| value.SearchModel = JsonSerializer.Deserialize(ref reader, type, options); | ||
| } | ||
| else | ||
| { | ||
| value.SearchModel = JsonSerializer.Deserialize<object>(ref reader, options); | ||
| } | ||
| } |
There was a problem hiding this comment.
The deserialization logic assumes the JSON properties "type" and "value" appear in a specific order, but JSON object property order is not guaranteed by the specification. If the "value" property appears before "type" in the JSON, the type variable will be null and deserialization will fall back to using JsonElement. Consider reading both properties first into local variables before attempting deserialization, or handle the case where properties might appear in any order.
Link issues
fixes #7326
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add typed serialization support for QueryPageOptions.SearchModel and strengthen type handling in JSON converters.
New Features:
Bug Fixes:
Enhancements:
Tests: