-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: fix element parsing and add proper namespace support in wsdl import #6610
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
base: main
Are you sure you want to change the base?
Conversation
…port - Fix elements being lost during WSDL parsing - Add support for both xsd: and xs: namespace prefixes in XSD schemas - Generate namespace-qualified XML elements (e.g., tns:ElementName) - Add xmlns declarations to root elements in SOAP envelopes - Fix handling of both element and type references in WSDL messages - Prevent element duplication in complex content generation - Ensure SOAP-compliant XML output matching industry standards
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR refactors WSDL-to-Bruno conversion by introducing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bruno-converters/src/wsdl/wsdl-to-bruno.js (1)
319-385: Consider extracting shared parsing logic.
parseElementInlineduplicates much ofparseElement's inline type handling. A private helper could reduce this duplication.🔎 Suggested approach
Extract the common complexType/sequence/choice/all/attribute parsing into a private helper method like
_parseInlineContent(element, parsedElement, namespace)and call it from both methods.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-converters/src/wsdl/wsdl-to-bruno.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-converters/src/wsdl/wsdl-to-bruno.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-converters/src/wsdl/wsdl-to-bruno.js
⏰ 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: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
🔇 Additional comments (9)
packages/bruno-converters/src/wsdl/wsdl-to-bruno.js (9)
680-686: Solid helper for dual-prefix XSD handling.Clean abstraction that eliminates repetitive prefix checks throughout the parser. The fallback to unprefixed
nodeNameis a good defensive measure for edge cases.
700-724: Clean namespace prefix resolution.Good caching strategy and sensible fallback to
tns/tnsNprefixes. The lookup order (cached → existing WSDL prefixes → generate new) is appropriate.
769-809: Well-structured root element handling.The
isRootflag cleanly separates root-specific behavior (xmlns declarations, no optional comments). Namespace-qualified element names will produce SOAP-compliant output.
917-945: Good guard against duplicate element generation.The early return when
element.elementsis populated prevents re-processing already-resolved inline types. The self-reference check at line 938 adds protection against infinite recursion.
1020-1064: Robust handling of element vs type references.The branching logic correctly handles document-style (
part.element) and RPC-style (part.type) WSDL patterns. Namespace resolution from prefixes is properly implemented.
207-235: Consistent use ofgetXsdNodethroughout parsing.The refactoring properly leverages the new helper. Parsing complex types before elements ensures type references resolve correctly.
428-497: Extension handling properly traverses inheritance.The recursive call to
parseComplexTypeContentforcomplexContent/extensionensures base type elements and attributes are inherited correctly.
553-565: Clean simple type facet extraction.Properly handles restriction facets including enumerations, patterns, and length constraints.
692-698: Proper state initialization for namespace tracking.The
namespaceCounterandnamespacePrefixesMap provide necessary state for the new prefix generation logic.
| let count = 0; | ||
| for (const [key] of this.wsdlData.elements) { | ||
| console.log(` ${key}`); | ||
| count++; | ||
| if (count >= 3) break; | ||
| } |
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.
Remove debug logging before merge.
These console.log statements leak schema details and add noise. They appear to be leftover debug artifacts.
🔎 Proposed fix
- let count = 0;
- for (const [key] of this.wsdlData.elements) {
- console.log(` ${key}`);
- count++;
- if (count >= 3) break;
- }
return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let count = 0; | |
| for (const [key] of this.wsdlData.elements) { | |
| console.log(` ${key}`); | |
| count++; | |
| if (count >= 3) break; | |
| } |
🤖 Prompt for AI Agents
In packages/bruno-converters/src/wsdl/wsdl-to-bruno.js around lines 757 to 762,
the for-loop that prints schema keys (`console.log`) is leftover debug output;
remove the console.log and associated count/break logic so schema details are
not logged (either delete the loop entirely or replace it with a no-op or a
conditional debug logger controlled by a verbose flag if runtime inspection is
required).
fix: #6049
Description
Contribution Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.