fix: Consistent multipart form handling and @contentType support in examples #6325
Conversation
sanish-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughThis PR adds support for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (4)
Comment |
![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
🧹 Nitpick comments (1)
packages/bruno-lang/v2/src/bruToJson.js (1)
422-435: Implementation duplicated withattributes.js.The
multilinetextblockandsinglelinevaluehandlers are identical to those inattributes.js. Consider extracting these to a shared utility to reduce duplication, though this can be deferred.// Could extract to a shared module, e.g., in common/semantic-handlers.js: const createMultilinetextblockHandler = () => (_1, content, _2, _3, contentType) => { const multilineString = content.sourceString .split('\n') .map((line) => line.slice(4)) .join('\n'); if (!contentType.sourceString) { return multilineString; } return `${multilineString} ${contentType.sourceString}`; };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/bruno-lang/v2/src/bruToJson.js(5 hunks)packages/bruno-lang/v2/src/common/attributes.js(2 hunks)packages/bruno-lang/v2/src/common/semantic-utils.js(2 hunks)packages/bruno-lang/v2/src/example/bruToJson.js(2 hunks)packages/bruno-lang/v2/src/example/jsonToBru.js(2 hunks)packages/bruno-lang/v2/src/example/request/bruToJson.js(2 hunks)packages/bruno-lang/v2/src/example/response/bruToJson.js(2 hunks)packages/bruno-lang/v2/tests/bruToJson.spec.js(1 hunks)packages/bruno-lang/v2/tests/examples/examples.spec.js(1 hunks)packages/bruno-lang/v2/tests/examples/fixtures/bru/examples-multiline-contenttype.bru(1 hunks)packages/bruno-lang/v2/tests/examples/fixtures/json/examples-multiline-contenttype.json(1 hunks)packages/bruno-tests/collection/echo/echo multipart.bru(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-lang/v2/tests/examples/examples.spec.jspackages/bruno-lang/v2/src/common/semantic-utils.jspackages/bruno-lang/v2/src/example/response/bruToJson.jspackages/bruno-lang/v2/src/example/jsonToBru.jspackages/bruno-lang/v2/src/example/request/bruToJson.jspackages/bruno-lang/v2/tests/bruToJson.spec.jspackages/bruno-lang/v2/src/example/bruToJson.jspackages/bruno-lang/v2/src/common/attributes.jspackages/bruno-lang/v2/src/bruToJson.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
packages/bruno-lang/v2/tests/examples/examples.spec.jspackages/bruno-lang/v2/tests/bruToJson.spec.js
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
packages/bruno-lang/v2/tests/examples/examples.spec.jspackages/bruno-lang/v2/tests/bruToJson.spec.js
🧬 Code graph analysis (2)
packages/bruno-lang/v2/src/example/jsonToBru.js (1)
packages/bruno-lang/v2/src/utils.js (1)
getValueString(35-49)
packages/bruno-lang/v2/tests/bruToJson.spec.js (3)
packages/bruno-lang/v2/tests/dictionary.spec.js (1)
expected(10-18)packages/bruno-lang/v2/src/bruToJson.js (1)
parser(1084-1094)packages/bruno-lang/v2/src/collectionBruToJson.js (1)
parser(561-571)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
🔇 Additional comments (20)
packages/bruno-lang/v2/src/example/jsonToBru.js (2)
1-1: LGTM!The import addition is correct and necessary for the multiline value wrapping functionality.
141-143: LGTM!The implementation correctly wraps multiline values using
getValueString(), which handles empty/null values, preserves single-line values, and wraps multiline values with triple quotes. This fixes the inconsistency between example and normal request handling as described in the PR objectives.The indentation logic is correct: multiline content receives 2-space indentation from
getValueString, then the entire item gets 4 additional spaces fromindentStringCustom(line 134), resulting in properly nested output.packages/bruno-lang/v2/src/common/semantic-utils.js (2)
72-72: LGTM: dotAll flag enables multiline contentType matching.The
sflag addition correctly enables the regex to match@contentTypeannotations across newlines, which is essential for multiline value support.
88-88: LGTM: Consistent contentType extraction across file types.Correctly applies the same dotAll flag to file extraction, maintaining consistency with multipart handling.
packages/bruno-tests/collection/echo/echo multipart.bru (2)
15-17: LGTM: Well-structured test for multiline contentType.The multiline field with
@contentTypeannotation correctly demonstrates the new feature and follows the expected syntax.
27-27: LGTM: Assertion validates end-to-end contentType handling.The assertion correctly validates that the contentType annotation is properly transmitted in the multipart request.
packages/bruno-lang/v2/src/example/request/bruToJson.js (2)
32-33: LGTM: Grammar extension for contentType annotations.The grammar additions correctly implement optional
@contentTypeannotations on multiline blocks while maintaining backward compatibility. Thest*spacing and optional?operator are well-placed.
46-47: LGTM: Value rule refactoring improves clarity.Extracting
singlelinevalueas a separate rule enhances grammar readability and aligns with the semantic handler structure.packages/bruno-lang/v2/tests/bruToJson.spec.js (1)
179-205: LGTM: Comprehensive test for multiline contentType feature.The test case thoroughly validates parsing of multiline body parts with
@contentTypeannotations, covering the expected structure and ensuring proper newline handling. Well-aligned with coding guidelines for testing new functionality.packages/bruno-lang/v2/src/example/response/bruToJson.js (1)
26-27: LGTM: Consistent grammar across request/response parsers.The grammar changes correctly mirror the request parser updates, maintaining consistent multiline contentType handling across both request and response blocks.
Also applies to: 40-41
packages/bruno-lang/v2/src/example/bruToJson.js (1)
28-29: LGTM: Unified grammar implementation.Grammar changes are consistent with request/response parsers, ensuring uniform contentType handling across the entire example parsing infrastructure.
Also applies to: 42-43
packages/bruno-lang/v2/tests/examples/examples.spec.js (1)
312-376: LGTM: Excellent test coverage for multiline contentType.The test suite comprehensively validates the new feature through:
- Parsing verification against fixtures
- ContentType extraction for multiple field types
- Round-trip conversion integrity
Tests cover edge cases (with/without contentType) and follow established patterns. Well-aligned with coding guidelines.
packages/bruno-lang/v2/tests/examples/fixtures/bru/examples-multiline-contenttype.bru (1)
1-65: LGTM: Comprehensive test fixture.The fixture provides excellent coverage of multiline contentType scenarios:
- Multiline JSON with contentType
- Single-line values with contentType
- Multiline content without contentType
- Complex nested structures
Well-structured and supports thorough testing of the feature.
packages/bruno-lang/v2/tests/examples/fixtures/json/examples-multiline-contenttype.json (1)
1-69: Well-structured test fixture with good coverage.Covers key scenarios: multiline JSON, plain text, and empty contentType. This will validate the round-trip parsing correctly.
packages/bruno-lang/v2/src/common/attributes.js (3)
54-64: Hardcoded 4-character slice for indentation removal.The
line.slice(4)assumes exactly 4 spaces of indentation. If the indentation varies, this could truncate content or leave extra whitespace. This matches the pattern inbruToJson.js, so it's consistent, but consider adding a brief comment explaining this expectation.
65-67: LGTM!Clean implementation with proper optional chaining and empty string fallback.
60-63: No issue found; optional grammar elements in Ohm.js always provide a.sourceStringproperty.Ohm.js returns an empty string (
"") when an optional grammar element doesn't match input. The code's conditional checkif (!contentType.sourceString)correctly handles this, as empty strings are falsy in JavaScript. Accessing.sourceStringon optional elements cannot throw—the property is guaranteed to exist.packages/bruno-lang/v2/src/bruToJson.js (3)
55-56: Grammar rules for contentType annotation look correct.The
contenttypeannotationrule properly captures the@contentType(...)syntax, andmultilinetextblockcorrectly makes it optional with?.
69-70: LGTM!Correct ordering:
listandmultilinetextblockare prioritized before falling back tosinglelinevalue.
214-224: Good use ofs(dotAll) flag for multiline content.The
sflag ensures.matches newline characters, which is necessary for multiline values. The non-greedy.*?correctly captures content before the annotation.
sanish-bruno
force-pushed
the
fix/example-multipart
branch
from
934fae1 to
f160e3d
Compare
sanish-bruno
force-pushed
the
fix/example-multipart
branch
from
f160e3d to
6e001ca
Compare
1 participant
Follow up to #6217
Problem
There were inconsistencies in how multipart form bodies were handled between normal requests and examples:
'''triple quotes when converting JSON to BRU format@contentTypeannotations on multiline valuesSolution
This PR fixes the inconsistencies by:
1. Added @ContentType annotation support to example grammars
example/bruToJson.jsgrammar to support@contentTypeon multiline text blocksexample/request/bruToJson.jsgrammar to support@contentTypeon multiline text blocksexample/response/bruToJson.jsgrammar to support@contentTypeon multiline text blockscommon/attributes.jsto process@contentTypeannotations2. Fixed multiline value handling in examples
example/jsonToBru.jsto usegetValueString()which properly wraps multiline values with'''triple quotes3. Standardized value handling across all grammars
value = list | multilinetextblock | singlelinevaluesinglelinevaluehandler tocommon/attributes.jsmatching the pattern inbruToJson.jspairhandler to work consistently with the new value structure4. Updated contentType extraction
common/semantic-utils.jsto use the same regex pattern asbruToJson.jsfor extracting@contentTypefrom valuesmultipartExtractContentTypeandfileExtractContentTypenow use consistent patternsChanges Made
Grammar Updates
packages/bruno-lang/v2/src/example/bruToJson.js: Added@contentTypesupport andsinglelinevaluepatternpackages/bruno-lang/v2/src/example/request/bruToJson.js: Added@contentTypesupport andsinglelinevaluepatternpackages/bruno-lang/v2/src/example/response/bruToJson.js: Added@contentTypesupport andsinglelinevaluepatternSemantic Handlers
packages/bruno-lang/v2/src/common/attributes.js:contenttypeannotationhandlermultilinetextblockhandler to matchbruToJson.jspattern (splits by newlines, removes 4-char indentation)singlelinevaluehandlervaluehandler (now handled by grammar routing)Content Type Extraction
packages/bruno-lang/v2/src/common/semantic-utils.js:multipartExtractContentTypeto match pattern inbruToJson.jsfileExtractContentTypeto match pattern inbruToJson.jsJSON to BRU Conversion
packages/bruno-lang/v2/src/example/jsonToBru.js:getValueStringimportgetValueString()for wrapping multiline values with'''Testing
Added comprehensive test cases for multiline strings with
@contentTypeannotations:New test fixture:
examples-multiline-contenttype.bruwith various scenarios:@contentType(application/json)@contentType(text/plain)@contentTypeTest cases added:
should parse examples with multiline strings and @contentType annotations- Basic parsing verificationshould correctly extract contentType from multiline values- Detailed contentType extraction verificationshould handle round-trip conversion for multiline strings with contentType- Round-trip conversion validationAll tests pass ✅
Example
Before this fix, examples with multiline values and
@contentTypewould not parse correctly:example { request: { body:multipart-form: { test: ''' { "hello" : "there" } ''' @contentType(application/json) } } }Now this correctly:
@contentTypeannotationBreaking Changes
None - this is a bug fix that maintains backward compatibility while fixing inconsistencies.
Related Issues
Fixes inconsistencies in multipart form handling between examples and normal requests.
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
@contentTypeannotations on multiline text blocks, enabling explicit content type declaration within request/response bodies.Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.