feat: Prevent MaxCacheSize validation bypass via object initializers#1939
feat: Prevent MaxCacheSize validation bypass via object initializers#1939DaveSkender merged 3 commits intotest-pruningfrom
Conversation
Changes MaxCacheSize from `init` to `private set` to prevent circumventing size validation through object initializers. The property can now only be set through the QuoteHub constructor which performs proper validation. - Changed StreamHub.MaxCacheSize accessor from `init` to `private set` - Updated BaseProvider to accept maxCacheSize parameter in constructor - Refactored QuoteHub to validate maxCacheSize and pass to BaseProvider - Updated all references using object initializer syntax to use constructor parameter - All tests passing, no build or lint issues Co-authored-by: DaveSkender <[email protected]>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request refactors cache size configuration in the QuoteHub and BaseProvider classes. QuoteHub now validates the maximum cache size parameter through a new private helper method before passing it to the base provider. BaseProvider introduces a constructor parameter to accept and store a configurable maximum cache size, replacing the previous hardcoded behavior. The StreamHub MaxCacheSize property changes from an init-only property to a privately settable property. Test files are updated to pass cache size values through the constructor rather than using object initializer syntax. Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/_common/Quotes/Quote.StreamHub.cs (1)
34-47: Include offending value in the out-of-range message (and align bounds wording).The message omits the actual value and implies a strict “less than” bound even though the code allows the inclusive upper bound.
As per coding guidelines, error messages should include the parameter name and offending value.✏️ Proposed update
- string message - = $"'{nameof(maxCacheSize)}' must be greater than 0 and less than {absoluteMaxCacheSize}."; + string message + = $"'{nameof(maxCacheSize)}' must be between 1 and {absoluteMaxCacheSize}. Actual: {maxCacheSize}.";
Signed-off-by: GitHub <[email protected]>
- Fixed MaxCacheSize setter errors from base PR #1939 (now constructor param) - Increased Hurst cache to 120 (100-period + 20 extra) - Increased PMO cache to 100 (35-period + 65 extra) - NOW PASSING - Increased EPMA cache to 100 (20-period + 80 extra) - Increased Slope cache to 100 (14-period + 86 extra) - Fixed Renko formatting issues (whitespace) - 77/80 tests passing (96.25%), 3 remaining require algorithm investigation Co-authored-by: DaveSkender <[email protected]>
MaxCacheSizecould be set through object initializers, bypassing constructor validation that enforces bounds checking (0 < size < 0.8 * int.MaxValue).Changes
MaxCacheSizefrom{ get; init; }to{ get; private set; }maxCacheSizeBefore/After
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.