Feature/product datastore#643
Open
F1r3w477 wants to merge 5 commits into
Open
Conversation
Move record maps, cache/LRU, populate-times, and lookup into ProductDatastore. Harden null-provider guards, second-aligned cache lookup, atomic cache limit, and L2 chunks time-populated check. Refs dpaulat#506
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Const-correct locks, deleted ProviderManager moves, explicit lambda captures, named cache limit constant, and const ref Store params.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extracts product record storage/cache responsibilities from RadarProductManager into a dedicated ProductDatastore as part of the multi-provider refactor (Slice B), and moves the internal ProviderManager into its own compilation unit.
Changes:
- Added
ProductDatastorefor record storage, cache/LRU behavior, and product-time population/lookup. - Extracted
ProviderManagerfromradar_product_manager.cppinto standaloneprovider_manager.{hpp,cpp}. - Updated
RadarProductManagerto delegate storage and cache lookups toproductDatastore_and added/updated unit tests and build wiring.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test.cmake | Registers new ProductDatastore unit tests in the test build. |
| test/source/scwx/qt/manager/radar_product_manager.test.cpp | Adds ProviderManager::name() formatting test and adjusts includes. |
| test/source/scwx/qt/manager/product_datastore.test.cpp | New unit tests for datastore dedup, lookup, cache key behavior, and cache limit floor. |
| scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp | Delegates storage/cache/time-population to ProductDatastore and uses extracted ProviderManager. |
| scwx-qt/source/scwx/qt/manager/provider_manager.hpp | New standalone ProviderManager header. |
| scwx-qt/source/scwx/qt/manager/provider_manager.cpp | New standalone ProviderManager implementation (refresh scheduling, shutdown hardening). |
| scwx-qt/source/scwx/qt/manager/product_datastore.hpp | New datastore public API for caching/record maps/LRU and time population. |
| scwx-qt/source/scwx/qt/manager/product_datastore.cpp | Implements record storage/dedup, cache lookups, time population, and iteration helpers. |
| scwx-qt/scwx-qt.cmake | Adds new manager source/header files to the Qt build. |
Avoid relying on transitive includes for std::max/std::find.
Align dpaulat#643 with review standards from dpaulat#642/dpaulat#644: hide implementation details behind Impl, use ProviderManager accessors, fix RPM include order, and add QT_NO_EMIT for wxtest on Linux.
Keep ProductDatastore delegation in RPM; integrate RadarCoordinateTable and merged ProviderManager from develop. Drop duplicate cache/populate logic left in develop's RPM.
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
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.
Summary
Slice B of the multi-provider refactor (#506): extract
ProductDatastorefromRadarProductManager.Record storage, cache/LRU eviction, product-time population, and cache lookup now live in a dedicated class.
RadarProductManagerkeeps orchestration — providers, refresh, coordinate generation, and the L2/L3 load paths — and delegates storage toproductDatastore_.Stacked on #642 (
feature/extract-provider-manager/ Slice A).What moved
ProductDatastore— L2/L3 record maps, recent-record LRU,Store,GetCachedNexradFile,FindLevel2RecordEntries/FindLevel3RecordEntry,PopulateLevel2/3ProductTimes,AreProductTimesPopulatedStoreRadarProductRecord,UpdateRecentRecords, local typedefs)provider_before provider callsGetCachedNexradFilekeys aligned to seconds (matchesStore)cacheLimit_for concurrentSetCacheLimitAreLevel2ProductTimesPopulatedchecks archive and chunks providersTest plan
ProductDatastore.*unit tests (7) — dedup, find, cache lookup, sub-second cache key, cache limit floorRadarProductManager/ProviderManagertests passNotes