-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat(queue-bundle): bypass unbundle filter for bundles queued through API #259
feat(queue-bundle): bypass unbundle filter for bundles queued through API #259
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
error @permaweb/aoconnect@0.0.57: The engine "yarn" is incompatible with this module. Expected version "please-use-npm". Got "1.22.22" 📝 WalkthroughWalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Caller
participant queueBundle
participant BundleDataImporter
participant Ans104Unbundler
Caller->>queueBundle: Call with item and bypassFilter
queueBundle->>BundleDataImporter: Queue item with bypassFilter
BundleDataImporter->>Ans104Unbundler: Queue item with bypassFilter
Ans104Unbundler->>Ans104Unbundler: Check filter condition
alt bypassFilter is true
Ans104Unbundler->>Ans104Unbundler: Parse bundle regardless of filter
else bypassFilter is false
Ans104Unbundler->>Ans104Unbundler: Apply standard filter logic
end
Possibly related PRs
Suggested reviewers
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/workers/ans104-unbundler.ts (1)
156-156
: Consider adding a debug log for filter bypass.When the filter is bypassed, it would be helpful to log this information for debugging purposes.
- if (bypassFilter || (await this.filter.match(item))) { + if (bypassFilter) { + log.debug('Filter bypassed for bundle unbundling'); + // Continue with unbundling + } else if (await this.filter.match(item)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/system.ts
(1 hunks)src/workers/ans104-unbundler.test.ts
(2 hunks)src/workers/ans104-unbundler.ts
(4 hunks)src/workers/bundle-data-importer.test.ts
(2 hunks)src/workers/bundle-data-importer.ts
(4 hunks)
🔇 Additional comments (7)
src/workers/ans104-unbundler.ts (1)
55-58
: LGTM! Type definition properly updated.The queue type has been correctly updated to include the new
bypassFilter
parameter, maintaining type safety.src/workers/bundle-data-importer.ts (2)
40-40
: LGTM! Interface properly updated.The
bypassFilter
property has been correctly added to theUnbundlingQueueItem
interface.
81-81
: LGTM! Consistent parameter handling.The
bypassFilter
parameter is consistently handled across queue operations with a proper default value.Also applies to: 91-91, 95-95
src/workers/bundle-data-importer.test.ts (1)
153-161
: LGTM! Test coverage properly updated.The test cases have been correctly updated to include and verify the new
bypassFilter
parameter.Also applies to: 175-175
src/workers/ans104-unbundler.test.ts (1)
109-129
: LGTM! Comprehensive test coverage for bypass functionality.The new test case thoroughly verifies that bundles are parsed even when the filter doesn't match if
bypassFilter
is true.src/system.ts (2)
522-522
: LGTM: Parameter correctly passed to bundleDataImporterThe
bypassFilter
parameter is properly forwarded to thebundleDataImporter.queueItem
call, maintaining the chain of responsibility for filter bypass.
522-522
: Verify event listener configurationsThe implementation looks correct, but let's verify a few aspects:
- The
ANS104_BUNDLE_QUEUED
event (API-triggered) correctly bypasses the filter.- The
ANS104_TX_INDEXED
andANS104_NESTED_BUNDLE_INDEXED
events maintain default behavior.- The changes don't affect existing functionality.
Also applies to: 4-5
✅ Verification successful
Event listener configurations are correctly implemented
All event listeners are properly configured with the correct parameters for their respective use cases. The implementation maintains backward compatibility while adding the new filter bypass functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all event listeners and their configurations rg -A 5 "events\.ANS104_(BUNDLE_QUEUED|TX_INDEXED|NESTED_BUNDLE_INDEXED)" --type ts # Search for all calls to queueBundle to ensure proper parameter usage ast-grep --pattern 'queueBundle($$$)'Length of output: 2217
Script:
#!/bin/bash # Search for queueBundle function definition ast-grep --pattern 'async function queueBundle($_) { $$$ }' # Alternative pattern in case it's not a standalone function ast-grep --pattern 'queueBundle = async ($_) => { $$$ }'Length of output: 124
Script:
#!/bin/bash # Search for queueBundle function definition with more context rg -B 2 -A 10 "queueBundle\s*=?\s*(async)?\s*\(.*\)" --type ts # Also search for any references to queueBundle to ensure we don't miss anything rg "queueBundle" --type tsLength of output: 1630
Script:
#!/bin/bash # Get the full function signature with more context rg -B 0 -A 5 "^async function queueBundle\(" --type tsLength of output: 282
No description provided.