-
Notifications
You must be signed in to change notification settings - Fork 360
refactor(remote-config): replace kPreUpdate with batch handler API (DEBUG-4402) #7121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b704693 to
eec9001
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7121 +/- ##
==========================================
+ Coverage 84.51% 84.52% +0.01%
==========================================
Files 525 526 +1
Lines 22492 22537 +45
==========================================
+ Hits 19008 19049 +41
- Misses 3484 3488 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Overall package sizeSelf size: 4.4 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
eec9001 to
3efdc4c
Compare
3efdc4c to
b49c683
Compare
This comment has been minimized.
This comment has been minimized.
b49c683 to
c18a9fe
Compare
BridgeAR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like a good improvement to separate things by product. Just left a few minor suggestions
c18a9fe to
e65d976
Compare
- Remove the leaky kPreUpdate hook and introduce a first-class batch API: setBatchHandler()/removeBatchHandler() - Introduce explicit product subscription via subscribeProducts()/ unsubscribeProducts(), and have setProductHandler()/ removeProductHandler() subscribe/unsubscribe for clarity - Centralize ASM/WAF RC product names in appsec/rc-products.js - Update AppSec WAF RC integration to use batched tx (ack/error/markHandled) instead of mutating configs - Improve JSDoc/TS inference for batch transaction/descriptors and WAF manager typing
4d55361 to
16ebc4a
Compare
16ebc4a to
0bc9468
Compare
BridgeAR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just left a question

What does this PR do?
kPreUpdatehook fromRemoteConfigManagerand replaces it with a first-class batch handler API (setBatchHandler/removeBatchHandler) that receives an explicit transaction (ack/error/markHandled).subscribeProducts(...products)/unsubscribeProducts(...products)and makessetProductHandler/removeProductHandlersubscribe/unsubscribe for clarity.appsec/rc-products.js.Motivation
kPreUpdateexposed internal mutable state and blurred responsibility between RC internals and consumers. WAF needs to reconcile multiple products in one logical update, so the new batch API provides a safer and clearer surface.