Fix SaveAs to use runtime parameter values for XLSX and other formats#261
Fix SaveAs to use runtime parameter values for XLSX and other formats#261majorsilence merged 5 commits intomasterfrom
Conversation
Modified GetParameters() method in RdlViewer.cs to retrieve current runtime parameter values from UserReportParameters instead of using the stale _Parameters dictionary. This ensures SaveAs to XLSX, HTML, CSV, XML, RTF, and MHTML uses the correct parameter values selected by the user at runtime. Co-authored-by: majorsilence <656288+majorsilence@users.noreply.github.com>
Co-authored-by: majorsilence <656288+majorsilence@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where SaveAs operations for XLSX, HTML, CSV, XML, RTF, and MHTML formats were using default parameter values instead of the runtime values selected by users in the UI. The root cause was that the _Parameters dictionary was never updated when users changed parameter values in the UI, causing exports to use stale default values.
Changes:
- Modified
GetParameters()inRdlViewer.csto prioritize runtime values fromUserReportParametersover the potentially stale_Parametersdictionary - Fixed
UserReportParameter.Valuegetter inReportParameter.csto properly await the asyncGetRuntimeValuemethod using the codebase's standard Task.Run wrapper pattern
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| RdlViewer/RdlViewer.cs | Enhanced GetParameters() to read current runtime parameter values from UserReportParameters before falling back to _Parameters dictionary |
| RdlEngine/Definition/ReportParameter.cs | Fixed Value property getter to properly await async GetRuntimeValue method using Task.Run wrapper |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get | ||
| { | ||
| // HACK: async | ||
| return Task.Run(async () => await _rp.GetRuntimeValue(this._rpt)).GetAwaiter().GetResult(); | ||
| } |
There was a problem hiding this comment.
The Value property getter now uses Task.Run with GetAwaiter().GetResult() to synchronously call an async method. While this follows the codebase's "HACK: async" pattern, it introduces potential performance overhead and deadlock risks, especially in UI contexts or when called under a SynchronizationContext. This is particularly concerning because UserReportParameter.Value is accessed frequently in GetParameters() in a foreach loop (line 1489-1496), which could create multiple synchronous blocking calls in rapid succession.
Consider whether the GetParameters() method itself could be made async to properly await these values, or if there's a way to cache the values after they're first set in the UI to avoid repeated async calls during export operations.
There was a problem hiding this comment.
Applied in commit 20c939b. Added a GetValueAsync() method to UserReportParameter that properly awaits GetRuntimeValue without blocking. Updated GetParameters() to be async Task<IDictionary> and use await urp.GetValueAsync() in the loop, eliminating the synchronous Task.Run().GetAwaiter().GetResult() pattern from the hot path. Both callers (SaveAs and GetPages) now await GetParameters().
RdlViewer/RdlViewer.cs
Outdated
| // Skip null values as they would use the parameter's default value anyway | ||
| // and we want to let the report engine handle defaults properly | ||
| if (urp.Value != null) | ||
| { | ||
| runtimeParams[urp.Name] = urp.Value; | ||
| } |
There was a problem hiding this comment.
The logic skips parameters with null values (line 1493), assuming the report engine will handle defaults. However, this creates an edge case: if a UserReportParameter has a null value but _Parameters contains a stale value for the same parameter name, the stale value from _Parameters will be used instead (line 1506), which could lead to incorrect behavior.
Consider always including parameters from UserReportParameters in runtimeParams, even when their value is null, to ensure stale values from _Parameters are never used for user-visible parameters. This would make the behavior more predictable and avoid subtle bugs where a parameter appears to have a value when it shouldn't.
| // Skip null values as they would use the parameter's default value anyway | |
| // and we want to let the report engine handle defaults properly | |
| if (urp.Value != null) | |
| { | |
| runtimeParams[urp.Name] = urp.Value; | |
| } | |
| // Always include user parameters, even when the value is null, so that | |
| // stale values from _Parameters cannot override user-visible parameters. | |
| runtimeParams[urp.Name] = urp.Value; |
There was a problem hiding this comment.
Fixed in commit 20c939b. User parameters are now always added to runtimeParams (even when null), so a stale value in _Parameters can never override a user-visible parameter.
| get { return _rp.GetRuntimeValue(this._rpt); } | ||
| get | ||
| { | ||
| // HACK: async |
There was a problem hiding this comment.
The comment "// HACK: async" on line 503 indicates this is a workaround. While this pattern is used consistently throughout the codebase for synchronous wrappers of async methods, the comment itself should be more descriptive about why this is necessary and what the trade-offs are. Consider updating the comment to something like: "// HACK: async - synchronous wrapper needed for property getter; introduces potential deadlock risk" to make the implications clearer for future maintainers.
There was a problem hiding this comment.
Already applied in commit 20c939b. The comment now reads: // HACK: async - synchronous wrapper needed for property getter; use GetValueAsync() when an async context is available. and the XML doc comment on the property also directs callers to prefer GetValueAsync().
…ll param edge case - Added GetValueAsync() to UserReportParameter for proper async access to runtime values - Updated GetParameters() to be async and use GetValueAsync() to avoid synchronous blocking - Updated both callers of GetParameters() (SaveAs and GetPages) to await the async call - Updated null handling: always include user parameters (even null values) so stale _Parameters values can never override user-visible parameters - Improved HACK comment in Value getter to explain the async trade-off Co-authored-by: majorsilence <656288+majorsilence@users.noreply.github.com>
Changes to fix SaveAs parameter handling
Problem
When saving a report to XLSX (and other non-PDF/TIF formats like HTML, CSV, XML, RTF, MHTML), the SaveAs method was using default parameter values instead of runtime parameter values selected by the user in the UI.
Root Cause
_Parametersdictionary was never updated when users changed parameter values in the UIUserReportParameter.Value, but this was returning aTask<object>instead of the actual value (becauseGetRuntimeValueis async)Fixes Applied
GetValueAsync()method toUserReportParameterfor proper async access to runtime values without synchronous blockingGetParameters()async to properly await each parameter value usingGetValueAsync(), eliminating theTask.Run().GetAwaiter().GetResult()deadlock riskGetParameters()inSaveAs()andGetPages()to await the async callruntimeParamsdict (even null values) to prevent stale_Parametersfrom overriding user-visible parameter values// HACK: asynccomment inValuegetter to explain the trade-off and direct callers to preferGetValueAsync()in async contextsAffected Export Formats
This fix ensures all non-PDF/TIF exports use runtime parameter values:
Security Summary
No security vulnerabilities were introduced. The change only modifies how parameter values are read from
UserReportParametersinstead of the stale_Parametersdictionary.Original prompt
This section details on the original issue you should resolve
<issue_title>When saveas to XLSX is not working according to its parameter</issue_title>
<issue_description>I have a report having pYear parameter and default value=2018.
In runtime when I run the report having parameter value=2017 and trying to saveas to xlsx format.It saveas report according to parameter value=2018.
In my application, there are different choices for saveas (pdf, xml, html, csv, rtf, tif, excell, mht)
PDF and TIF is working properly according to the choosen parameter, but others is working only for defalt parameter value.
Thaks in advance!!!
My vb.net code is below;
Public Sub RaporMenu(ByVal sender As Object, ByVal e As System.EventArgs)
'MsgBox(sender.ToString)
Select Case sender.ToString
Case "Open"
Dim dlg As New OpenFileDialog()
If (dlg.ShowDialog() <> DialogResult.OK) Then
Return
End If
rdlViewer.SourceFile = New Uri(dlg.FileName)
rdlViewer.Rebuild()
Fixes #170
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.