feat: update mapping component regex for HTTP destination #1820
Conversation
sandeepdsvs
requested review from
lvrach,
cisse21,
debanjan97,
am6010,
ssbeefeater,
ruchiramoitra,
ItsSudip,
shrouti1507,
krishna2020 and
1abhishekpandey
as code owners
WalkthroughThe changes in this pull request enhance the validation logic across multiple configuration files related to HTTP settings. Modifications include updates to regex patterns in Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1820 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 53 53
Branches 7 7
=========================================
Hits 53 53 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/configurations/destinations/http/ui-config.json(3 hunks)
🔇 Additional comments (1)
src/configurations/destinations/http/ui-config.json (1)
Line range hint 534-599: Overall regex pattern implementation looks good
The regex patterns for Request Body, Query Parameters, and Headers mappings are well-structured and consistent. They properly validate:
- RudderStack path expressions (e.g.,
$.properties.key,$['key'],$["key"]) - Simple identifiers (e.g.,
basic_auth,content_type) - Array indices (e.g.,
$[0])
Once the trailing newlines are removed, these patterns will provide robust validation for HTTP destination mappings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/configurations/destinations/http/schema.json (1)
51-57: Consider maintaining consistent field order across sectionsWhile the validation patterns are correct, the field order in queryParams ("to" before "from") differs from propertiesMapping ("from" before "to"). Consider maintaining consistent ordering for better maintainability.
Apply this diff to maintain consistent ordering:
"queryParams": { "type": "array", "items": { "type": "object", "properties": { - "to": { - "type": "string", - "pattern": "^(?!\\$).{0,100}$" - }, - "from": { + "from": { "type": "string", "pattern": "^\\$(\\.(\\w+|\\*)|\\[\\d+\\]|\\[('\\w+'|\"\\w+\")\\]|\\[\\*\\]|\\.\\w+\\(\\))*$|^(?!\\$).{0,100}$" + }, + "to": { + "type": "string", + "pattern": "^(?!\\$).{0,100}$" } } } },test/data/validation/destinations/http.json (1)
20-35: Consider adding edge case testsWhile the current test cases cover basic scenarios, consider adding tests for:
- Maximum length values (100 characters)
- Invalid JSON paths
- Special characters in literal values
Here's an example of additional test cases to add:
{ "headers": [ { "from": "$.very.long.path.that.should.work", "to": "a".repeat(100) }, { "from": "$invalid.path.", "to": "should-fail" }, { "from": "$.valid.path", "to": "special@#$chars" } ] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/configurations/destinations/http/schema.json(3 hunks)src/configurations/destinations/http/ui-config.json(3 hunks)test/data/validation/destinations/http.json(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/configurations/destinations/http/ui-config.json
🔇 Additional comments (3)
src/configurations/destinations/http/schema.json (2)
67-73: Field ordering inconsistency in headers section
The headers section has the same field ordering inconsistency as noted in the queryParams section.
37-41: LGTM: Improved validation patterns for properties mapping
The updated regex patterns provide clearer validation rules:
- Source field ("from") must be a valid JSON path
- Destination field ("to") supports both JSON paths and literal values
Let's verify the regex patterns against some example mappings:
✅ Verification successful
Regex patterns for properties mapping validated successfully
The validation tests confirm that the regex patterns work as intended:
- "from" pattern correctly accepts JSON paths starting with $ (e.g.,
$.properties.event,$.array[0].field) - "from" pattern correctly rejects invalid paths without $ prefix
- "to" pattern correctly accepts both JSON paths and literal values (up to 100 chars)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Test the new regex patterns with example values
echo 'Testing "from" pattern...'
echo '$.properties.event' | grep -P '^\\$(\\.(\\w+|\\*)|\\[\\d+\\]|\\[('\''\\w+''\'|"\\w+")\\]|\\[\\*\\]|\\.\\w+\\(\\))*$'
echo 'Testing "to" pattern...'
echo '$.events.name' | grep -P '^\\$(\\.(\\w+|\\*)|\\[\\d+\\]|\\[('\''\\w+''\'|"\\w+")\\]|\\[\\*\\]|\\.\\w+\\(\\))*$|^(?!\\$).{0,100}$'
echo 'literal_value' | grep -P '^\\$(\\.(\\w+|\\*)|\\[\\d+\\]|\\[('\''\\w+''\'|"\\w+")\\]|\\[\\*\\]|\\.\\w+\\(\\))*$|^(?!\\$).{0,100}$'
Length of output: 366
Script:
#!/bin/bash
# Let's try a simpler approach to test the regex patterns
# Testing "from" pattern with valid and invalid cases
echo "Testing 'from' pattern with valid cases:"
echo '$.properties.event' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$'
echo '$.array[0].field' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$'
echo '$.array[*].field' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$'
echo -e "\nTesting 'from' pattern with invalid cases:"
echo 'invalid_path' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$' || echo "Rejected as expected"
echo 'properties.event' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$' || echo "Rejected as expected"
echo -e "\nTesting 'to' pattern with valid cases:"
echo '$.properties.event' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$|^(?!\$).{0,100}$'
echo 'literal_value' | grep -E '^\$((\.[a-zA-Z0-9_]+|\.\*|\[[0-9]+\]|\['"'"'[a-zA-Z0-9_]+'"'"'\]|\[\*\]|\.[a-zA-Z0-9_]+\(\))*$|^(?!\$).{0,100}$'
Length of output: 1514
Script:
#!/bin/bash
# Testing with simplified regex patterns to validate the core functionality
echo "Testing 'from' pattern with valid cases:"
echo '$.properties.event' | grep -E '^\$(\.[a-zA-Z0-9_]+|\[.+\])*$'
echo '$.array[0].field' | grep -E '^\$(\.[a-zA-Z0-9_]+|\[.+\])*$'
echo -e "\nTesting 'from' pattern with invalid cases:"
echo 'invalid_path' | grep -E '^\$(\.[a-zA-Z0-9_]+|\[.+\])*$' || echo "Invalid path rejected"
echo 'properties.event' | grep -E '^\$(\.[a-zA-Z0-9_]+|\[.+\])*$' || echo "Missing $ rejected"
echo -e "\nTesting 'to' pattern with valid cases:"
# Testing both JSON path and literal values
echo '$.properties.event' | grep -E '(^\$(\.[a-zA-Z0-9_]+|\[.+\])*$)|(^[^$].{0,99}$)'
echo 'literal_value' | grep -E '(^\$(\.[a-zA-Z0-9_]+|\[.+\])*$)|(^[^$].{0,99}$)'
Length of output: 906
test/data/validation/destinations/http.json (1)
257-272: LGTM: Comprehensive error test cases
The test cases effectively validate:
- Batch size constraints
- URL format requirements
- Consent management configurations
What are the changes introduced in this PR?
Update mapping component regex for HTTP destination
What is the related Linear task?
Resolves INT-2961
Please explain the objectives of your changes below
Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A
Any new dependencies introduced with this change?
N/A
Any new checks got introduced or modified in test suites. Please explain the changes.
N/A
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
I have executed schemaGenerator tests and updated schema if needed
Are sensitive fields marked as secret in definition config?
My test cases and placeholders use only masked/sample values for sensitive fields
Is the PR limited to 10 file changes & one task?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
These changes ensure that user inputs conform to expected formats, resulting in a more robust configuration interface.