refactor(core): move StringUtil to text package, extend XSLT tran…#2636
Merged
Conversation
…sformation support, and add tutorial examples
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughRelocates StringUtil to package com.predic8.membrane.core.util.text and updates static imports; refactors XSLTInterceptor to a unified handleInternal flow with standardized error handling; adds XML→JSON XSLT, tutorials, and tests; minor docs and tutorial YAML/name updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant XSLTInterceptor as XSLT
participant Transformer
participant Exchange
participant Backend
Client->>XSLT: send request
XSLT->>Exchange: getMessage(REQUEST)
XSLT->>Transformer: transform(input)
alt TransformerException (prolog)
Transformer-->>XSLT: throws TransformerException
XSLT->>Exchange: createErrorResponse(exception, REQUEST) / abort
XSLT-->>Client: ABORT response (error message, truncated input)
else success
Transformer-->>XSLT: transformed payload
XSLT->>Exchange: set transformed message (REQUEST)
XSLT->>Backend: forward exchange
Backend-->>XSLT: response
XSLT->>Exchange: getMessage(RESPONSE)
XSLT->>Transformer: transform(response)
XSLT-->>Client: return transformed response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptor.java`:
- Around line 70-78: The catch block handling TransformerException should guard
against a null message before calling e.getMessage().contains(...); change the
conditional to check e.getMessage() != null (e.g. e.getMessage() != null &&
e.getMessage().contains("not allowed in prolog")) so the
user(...).title(...).detail(...).internal(...).buildAndSetResponse(exc) branch
still runs for prolog errors without risking a NullPointerException; leave the
existing log.debug("", e) and the ABORT return intact.
- Around line 80-93: The createErrorResponse currently always sets the detail
message to "Error transforming request!" which is misleading for response
transformations; update XSLTInterceptor.createErrorResponse to produce a
context-aware message (e.g. "Error transforming request!" vs "Error transforming
response!") by either adding a boolean/direction parameter (e.g. isRequest) or
by inspecting the Exchange (exc) to determine whether the transformation was for
the request or response, then update all call sites in XSLTInterceptor that call
createErrorResponse to pass the correct direction so the user(...).detail(...)
call contains the appropriate, flow-aware text.
In `@distribution/tutorials/xml/to-json.xsl`:
- Around line 7-18: The templates produce raw XML nodes via the identity
template (match="@*|node()") and use <xsl:apply-templates/> so the output is not
valid JSON (missing commas and leaking XML); change the stylesheet to emit text
JSON explicitly (set output method="text" or use xsl:text), update the "books"
template to open an array and join book items with commas, update the "book"
template to render JSON object fields (use xsl:text and apply-templates only for
values) and remove or restrict the identity template so it does not copy XML
nodes into the output, and ensure proper escaping of text content when rendering
values (refer to the templates matching "books", "book", and the identity
template match="@*|node()" to locate and modify).
🧹 Nitpick comments (1)
distribution/src/test/java/com/predic8/membrane/tutorials/xml/XsltJson2XMLTransformationTutorialTest.java (1)
11-15: Align class name with XML→JSON transformationThe test posts XML and expects JSON, but the class name reads JSON→XML. Consider renaming the class (and file) to match the actual direction.
🔧 Proposed rename
-public class XsltJson2XMLTransformationTutorialTest extends AbstractXmlTutorialTest{ +public class XsltXml2JsonTransformationTutorialTest extends AbstractXmlTutorialTest{
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@distribution/src/test/java/com/predic8/membrane/tutorials/xml/XsltXML2JSONTransformationTutorialTest.java`:
- Around line 12-15: In XsltXML2JSONTransformationTutorialTest, the
getTutorialYaml() method returns a hardcoded filename with incorrect casing;
update the return value in getTutorialYaml() to
"35-XSLT-transformation-to-json.yaml" (lowercase 't') so it matches the actual
resource name and avoids failures on case-sensitive filesystems.
🧹 Nitpick comments (1)
core/src/test/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptorTest.java (1)
84-88: Remove stdout debug output from tests.Line 87 prints the response body to stdout, which can clutter CI logs. Consider removing it or switching to a logger with an appropriate level.
♻️ Proposed cleanup
- System.out.println(body); assertTrue(body.contains("rubbish")); assertTrue(body.contains("not allowed in prolog"));
…e unused test code
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/src/test/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptorTest.java`:
- Around line 77-89: Rename the test method noConentInProlog to
noContentInProlog to correct the typo; locate the JUnit test method in class
XSLTInterceptorTest and update the method name signature (the method annotated
with `@Test` that builds the exchange and asserts ABORT and 400) so it uses the
correct spelling, ensuring any references (e.g., test runner or IDE) reflect the
new name.
In
`@distribution/src/test/java/com/predic8/membrane/tutorials/xml/XsltTransformationTutorialTest.java`:
- Around line 12-15: Update the getTutorialYaml() override to reference the
actual YAML filename used in the repo: replace the returned string
"40-XSLT-Transformation.yaml" with the correct
"40-XSLT-Transformation-group.yaml" inside the getTutorialYaml() method so the
XsltTransformationTutorialTest points to the existing tutorial file.
🧹 Nitpick comments (1)
distribution/tutorials/xml/35-XSLT-Transformation-to-json.yaml (1)
20-24: Consider adjusting comment indentation for clarity.The comment on line 23 explains the
beautifierbut appears at the same indentation level assetHeader, which may cause confusion. Consider moving it to align withbeautifier.📝 Suggested fix
- setHeader: name: Content-Type value: application/json - # The beautifier needs the right Content-Type to format the output of the transform - - beautifier: {} + # The beautifier needs the right Content-Type to format the output of the transform + - beautifier: {}
… and clarify beautifier comment
|
This pull request needs "/ok-to-test" from an authorized committer. |
Collaborator
|
/ok-to-test |
…est to string for consistency
… into xslt-transform-prolog-error-msg
rrayst
previously approved these changes
Jan 21, 2026
christiangoerdes
approved these changes
Jan 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…sformation support, and add tutorial examples
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.