fix(Table): support QueryPageOptions serialization#7310
Merged
Conversation
|
Thanks for your PR, @kimdiego2098. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Contributor
Reviewer's GuideRefactors QueryPageOptions JSON (de)serialization to use a single FilterKeyValueAction representation and a polymorphic object converter, ensuring filters and search models serialize/deserialize correctly and adds a small sample test usage. Sequence diagram for QueryPageOptions round-trip JSON serialization with FilterKeyValueActionsequenceDiagram
actor SampleComponent
participant JsonSerializer as SystemTextJson_JsonSerializer
participant Converter as JsonQueryPageOptionsConverter
participant Options as QueryPageOptions
participant Ext as QueryPageOptionsExtensions
participant Fkva as FilterKeyValueAction
SampleComponent->>Options: Configure filters and searches
SampleComponent->>JsonSerializer: Serialize(Options)
JsonSerializer->>Converter: Write(writer, Options, options)
Converter->>Options: Check Searches, CustomerSearches, AdvanceSearches, Filters
alt Any_filter_list_non_empty
Converter->>Ext: ToFilter(Options)
alt Existing_FilterKeyValueAction
Ext-->>Converter: return Options.FilterKeyValueAction
else Build_new_FilterKeyValueAction
Ext->>Fkva: new FilterKeyValueAction()
Ext->>Fkva: Populate from Searches and Filters
Ext-->>Converter: return Fkva
end
Converter->>JsonSerializer: Write filterKeyValueAction property
end
Converter-->>JsonSerializer: Finish writing QueryPageOptions JSON
JsonSerializer-->>SampleComponent: JSON string
SampleComponent->>JsonSerializer: Deserialize(json, QueryPageOptions)
JsonSerializer->>Converter: Read(ref reader, typeToConvert, options)
Converter->>Options: Create new QueryPageOptions
Converter->>Options: Set IsVirtualScroll, IsFirstQuery, etc.
Converter->>Options: Read filterKeyValueAction and assign FilterKeyValueAction
Converter-->>JsonSerializer: return Options
JsonSerializer-->>SampleComponent: QueryPageOptions instance with FilterKeyValueAction
SampleComponent->>Ext: ToFilter(Options)
Ext-->>SampleComponent: returns existing FilterKeyValueAction from Options.FilterKeyValueAction
Class diagram for updated QueryPageOptions JSON serialization modelclassDiagram
class QueryPageOptions {
object SearchModel
List~IFilterAction~ Searches
List~IFilterAction~ CustomerSearches
List~IFilterAction~ AdvanceSearches
List~IFilterAction~ Filters
internal FilterKeyValueAction FilterKeyValueAction
bool IsVirtualScroll
bool IsFirstQuery
bool IsAdvanceSearch
bool IsSimpleSearch
string? SearchText
int PageIndex
int PageItems
}
class FilterKeyValueAction {
string? FieldName
object? FieldValue
string? Operator
bool IsAnd
bool IsOr
}
class ObjectWithTypeConverter {
+object Read(Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
}
class JsonQueryPageOptionsConverter {
+QueryPageOptions Read(Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+void Write(Utf8JsonWriter writer, QueryPageOptions value, JsonSerializerOptions options)
}
class QueryPageOptionsExtensions {
+FilterKeyValueAction ToFilter(QueryPageOptions option)
}
interface IFilterAction
QueryPageOptions --> "*" IFilterAction : uses
QueryPageOptions --> FilterKeyValueAction : aggregates
QueryPageOptionsExtensions ..> QueryPageOptions : extension
QueryPageOptionsExtensions ..> FilterKeyValueAction : creates
JsonQueryPageOptionsConverter ..> QueryPageOptions : converts
JsonQueryPageOptionsConverter ..> FilterKeyValueAction : serializes
ObjectWithTypeConverter <.. QueryPageOptions : applied_to_SearchModel
ObjectWithTypeConverter <.. FilterKeyValueAction : applied_to_FieldValue
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- The new ObjectWithTypeConverter deserializes arbitrary types via Type.GetType and assembly-qualified names, which is risky and brittle; consider restricting allowed types/namespaces and handling Type.GetType returning null or invalid payloads more defensively.
- When no $type is present, ObjectWithTypeConverter returns a JsonElement clone, which changes the runtime type compared to previously using a concrete model; ensure this is compatible with existing usages of SearchModel/FieldValue or consider a fallback strategy that preserves expected types.
- The JSON round-trip in TablesFilter.OnQueryAsync appears to be test/debug-only and adds overhead on each query; consider removing it or wrapping it in a DEBUG conditional.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new ObjectWithTypeConverter deserializes arbitrary types via Type.GetType and assembly-qualified names, which is risky and brittle; consider restricting allowed types/namespaces and handling Type.GetType returning null or invalid payloads more defensively.
- When no $type is present, ObjectWithTypeConverter returns a JsonElement clone, which changes the runtime type compared to previously using a concrete model; ensure this is compatible with existing usages of SearchModel/FieldValue or consider a fallback strategy that preserves expected types.
- The JSON round-trip in TablesFilter.OnQueryAsync appears to be test/debug-only and adds overhead on each query; consider removing it or wrapping it in a DEBUG conditional.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Converter/JsonQueryPageOptionConverter.cs:190` </location>
<code_context>
- }
- if (value.Filters.Count != 0)
+
+ if (value.Searches.Count != 0||value.CustomerSearches.Count != 0||value.AdvanceSearches.Count != 0|| value.Filters.Count != 0)
{
- writer.WriteStartArray("filters");
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against the case where `FilterKeyValueAction` is already set but filter lists are empty.
Because deserialization sets `FilterKeyValueAction` directly and leaves `Searches` / `CustomerSearches` / `AdvanceSearches` / `Filters` empty, this `if` will skip writing filters when all lists are empty even if `FilterKeyValueAction` is populated (e.g., after a round-trip). To avoid losing existing filter state on re‑serialization, also include `value.FilterKeyValueAction != null` in this condition.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/Filters/FilterKeyValueAction.cs:23` </location>
<code_context>
/// <summary>
/// 获得/设置 Filter 项字段值
/// </summary>
+ [JsonConverter(typeof(ObjectWithTypeConverter))]
public object? FieldValue { get; set; }
</code_context>
<issue_to_address>
**🚨 issue (security):** Using type information from JSON for polymorphic deserialization can be a security risk.
Annotating `FieldValue` with `ObjectWithTypeConverter` allows clients to control the `$type` used during deserialization. Because `Type.GetType` with assembly-qualified names can resolve arbitrary types, this is unsafe for untrusted input. Use a restricted type resolution strategy (e.g., a whitelist or custom resolver) instead of calling `Type.GetType` directly on user data.
</issue_to_address>
### Comment 3
<location> `src/BootstrapBlazor/Converter/ObjectWithTypeConverter.cs:20-23` </location>
<code_context>
+ if (!doc.RootElement.TryGetProperty("$type", out var typeProp))
+ return doc.RootElement.Clone(); // 无类型信息
+
+ var type = Type.GetType(typeProp.GetString()!)!;
+
+ var valueElement = doc.RootElement.GetProperty("value");
+ return JsonSerializer.Deserialize(valueElement.GetRawText(), type, options);
+ }
+
</code_context>
<issue_to_address>
**issue:** Handle cases where the specified type cannot be resolved or the `value` property is missing.
`Type.GetType` can return null (e.g., type moved, different assembly, trimming), and the null-forgiving operator will hide this until a later NRE. Also, if `$type` is present but `value` is missing, `GetProperty("value")` will throw. Please handle these cases explicitly (e.g., detect and throw a `JsonException`, fall back to `JsonElement`, or apply a migration strategy) instead of relying on implicit null/KeyNotFound exceptions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| if (value.Filters.Count != 0) | ||
|
|
||
| if (value.Searches.Count != 0||value.CustomerSearches.Count != 0||value.AdvanceSearches.Count != 0|| value.Filters.Count != 0) |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Guard against the case where FilterKeyValueAction is already set but filter lists are empty.
Because deserialization sets FilterKeyValueAction directly and leaves Searches / CustomerSearches / AdvanceSearches / Filters empty, this if will skip writing filters when all lists are empty even if FilterKeyValueAction is populated (e.g., after a round-trip). To avoid losing existing filter state on re‑serialization, also include value.FilterKeyValueAction != null in this condition.
| /// <summary> | ||
| /// 获得/设置 Filter 项字段值 | ||
| /// </summary> | ||
| [JsonConverter(typeof(ObjectWithTypeConverter))] |
Contributor
There was a problem hiding this comment.
🚨 issue (security): Using type information from JSON for polymorphic deserialization can be a security risk.
Annotating FieldValue with ObjectWithTypeConverter allows clients to control the $type used during deserialization. Because Type.GetType with assembly-qualified names can resolve arbitrary types, this is unsafe for untrusted input. Use a restricted type resolution strategy (e.g., a whitelist or custom resolver) instead of calling Type.GetType directly on user data.
Comment on lines
+20
to
+23
| var type = Type.GetType(typeProp.GetString()!)!; | ||
|
|
||
| var valueElement = doc.RootElement.GetProperty("value"); | ||
| return JsonSerializer.Deserialize(valueElement.GetRawText(), type, options); |
Contributor
There was a problem hiding this comment.
issue: Handle cases where the specified type cannot be resolved or the value property is missing.
Type.GetType can return null (e.g., type moved, different assembly, trimming), and the null-forgiving operator will hide this until a later NRE. Also, if $type is present but value is missing, GetProperty("value") will throw. Please handle these cases explicitly (e.g., detect and throw a JsonException, fall back to JsonElement, or apply a migration strategy) instead of relying on implicit null/KeyNotFound exceptions.
ArgoZhang
previously approved these changes
Dec 12, 2025
ArgoZhang
changed the title
This was referenced Dec 22, 2025
This was referenced Jan 7, 2026
This was referenced Jan 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues
fixed #7320
Summary by Sourcery
Fix JSON serialization and deserialization for QueryPageOptions filter data by introducing a reusable typed-object converter and consolidating filter-related fields into FilterKeyValueAction.
Bug Fixes:
Enhancements: