Add support for non-async operations to bit Besql (#9811)#9812
Add support for non-async operations to bit Besql (#9811)#9812msynk merged 1 commit intobitfoundation:developfrom yasmoradi:9811
Conversation
WalkthroughThe changes adjust how non-asynchronous operations are handled in the Bit Besql framework. In the database context interceptor, command execution methods now check for specific keywords and, when detected, invoke an asynchronous throttling routine instead of throwing an exception. The custom BesqlNonAsyncOperationException has been removed. Additionally, the Weather component demo now uses a synchronous method (SyncVersionTest) that adds two forecasts and commits them synchronously with SaveChanges, reflecting support for non-async operations. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Interceptor as BesqlDbContextInterceptor
participant Throttle as ThrottledSync
App->>Interceptor: Execute command (e.g., ReaderExecuted)
Interceptor->>Interceptor: Check if command text has target keywords
alt Command is targeted
Interceptor->>Throttle: Invoke ThrottledSync asynchronously
Throttle-->>Interceptor: Return processed result
else Command not targeted
Interceptor-->>App: Return original result
end
Interceptor-->>App: Return final result
sequenceDiagram
participant User as User
participant Weather as Weather Component
participant DB as Database Context
User->>Weather: Trigger SyncVersionTest
Weather->>Weather: Add two forecast objects
Weather->>DB: Call SaveChanges synchronously
DB-->>Weather: Acknowledge save
Weather-->>User: Update forecasts count
Assessment against linked issues
Poem
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/Besql/Bit.Besql/BesqlDbContextInterceptor.cs (2)
10-10: Ensure true immutability of keyword list.
Marking the array reference as readonly prevents reassignment, but the array elements can still be modified at runtime. If true immutability is desired, consider using an immutable data structure or returning an unmodifiable view.
17-22: Handle potential unobserved exceptions from the background task.
Currently, the returned Task from ThrottledSync is ignored (fire-and-forget). If exceptions occur during execution, they may go unobserved. Consider a retry mechanism, error logging, or awaiting the task (based on requirements) to ensure failures are properly handled.src/Besql/Demo/Bit.Besql.Demo.Client/Pages/Weather.razor (1)
25-25: Consider enhancing the sync operation warning.While the button text includes a warning, consider making it more prominent by:
- Adding a tooltip with detailed explanation
- Using a different button style/color to indicate caution
- <button class="btn btn-primary" @onclick="SyncVersionTest">Sync version (Not recommended at all)</button> + <button class="btn btn-warning" + @onclick="SyncVersionTest" + title="Warning: Synchronous operations can block the UI thread and degrade user experience. Use async operations whenever possible."> + Sync version (Not recommended) + </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Besql/Bit.Besql/BesqlDbContextInterceptor.cs(3 hunks)src/Besql/Bit.Besql/BesqlNonAsyncOperationException.cs(0 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Pages/Weather.razor(3 hunks)
💤 Files with no reviewable changes (1)
- src/Besql/Bit.Besql/BesqlNonAsyncOperationException.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (3)
src/Besql/Bit.Besql/BesqlDbContextInterceptor.cs (2)
44-49: Same caution regarding ignoring ThrottledSync’s returned Task.
This is effectively the same concurrency pattern described in lines 17-22.
71-75: Same caution regarding ignoring ThrottledSync’s returned Task.
This is effectively the same concurrency pattern described in lines 17-22.src/Besql/Demo/Bit.Besql.Demo.Client/Pages/Weather.razor (1)
83-100:⚠️ Potential issueImprove sync implementation and reduce code duplication.
Several issues need attention:
- Code is duplicated from AddWeatherForecast
- Missing error handling
- UI could freeze during sync operations
- Same issue with long summary string
Consider these improvements:
private void SyncVersionTest() { - using var dbContext = DbContextFactory.CreateDbContext(); - dbContext.WeatherForecasts.Add(new() - { - Date = new DateTimeOffset(2024, 1, 4, 10, 10, 10, TimeSpan.Zero), - Summary = "A B C D E F G H I J K L M N O P Q R S T U V W X Y Z...", - TemperatureC = Random.Shared.Next(1, 30) - }); - dbContext.WeatherForecasts.Add(new() - { - Date = new DateTimeOffset(2024, 1, 4, 10, 10, 10, TimeSpan.Zero), - Summary = "Test", - TemperatureC = Random.Shared.Next(1, 30) - }); - dbContext.SaveChanges(); - forecastsCount += 2; + try + { + using var dbContext = DbContextFactory.CreateDbContext(); + // Extract common forecast creation logic to reduce duplication + var forecasts = CreateTestForecasts(); + dbContext.WeatherForecasts.AddRange(forecasts); + dbContext.SaveChanges(); + forecastsCount += forecasts.Count; + } + catch (Exception ex) + { + // Handle errors appropriately + Console.Error.WriteLine($"Error in sync operation: {ex.Message}"); + throw; + } } + + // Helper method to reduce duplication + private static List<WeatherForecast> CreateTestForecasts() + { + return new List<WeatherForecast> + { + new() + { + Date = DateTimeOffset.Now, + Summary = new string('A', 100), + TemperatureC = Random.Shared.Next(1, 30) + }, + new() + { + Date = DateTimeOffset.Now, + Summary = "Test", + TemperatureC = Random.Shared.Next(1, 30) + } + }; + }Also, verify the impact of sync operations on UI responsiveness:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other sync operations that might affect UI ast-grep --pattern 'SaveChanges()' # Look for sync SaveChanges calls rg 'CreateDbContext\(' # Look for sync context creationLength of output: 300
Reduce Synchronous Operations and Code Duplication in Weather Forecast Handling
The shell script searches confirm that synchronous operations (e.g., the CreateDbContext call and subsequent SaveChanges) are used in the Weather.razor file. This supports the review comment’s concerns regarding potential UI freezes, code duplication, and the lack of error handling.
Key points:
- The sync DbContext creation and SaveChanges() in Weather.razor were identified.
- No additional instances elsewhere were found that could mitigate UI impact.
- The proposed improvements—using a try/catch block and extracting shared code to a helper method—are appropriate.
2 participants
closes #9811
Summary by CodeRabbit