Open
Conversation
…http client interceptor
…URL handling and modify target configurations - Introduced `computeDestinationExpressions` to streamline target URL modification logic. - Improved `DispatchingInterceptor` handling of base path resolution with illegal characters. - Enhanced `urlEncode` and `pathSeg` utility functions for safety and clarity. - Refactored `HTTPClientInterceptor` to adjust the target flow and error messages. - Added extensive URI tests, including custom-parsed and relative path resolution scenarios. - Updated `applyTargetModifications` for streamlined behavior during request processing.
…t URLs - Introduced `TemplateUtil` with a method to check for template markers. - Enhanced `TemplateExchangeExpression` to support URL encoding for dynamic templates. - Updated `Target` to skip unnecessary evaluation for URLs without template markers. - Added extensive tests to verify correct URL encoding across various languages (Groovy, SpEL, XPath, JSONPath).
… target URLs
- Documented the change to URL encode expressions inside `${}` in target URLs.
- Updated tests to improve validation coverage and logging for clearer output.
- Introduced a `templateExpressionCache` in `Target` to avoid redundant parsing and compilation of frequently used expressions. - Improved token parsing and evaluation logic in `TemplateExchangeExpression`, enhancing clarity and performance. - Renamed test helper method `extracted` to `testExpression` for better readability. - Updated test cases across multiple classes to cover new enhancements and ensure consistent behavior.
…simplify logic - Eliminated `templateExpressionCache` to reduce complexity and improve maintainability. - Updated `computeDestinationExpressions` to use `Collectors.toList()` for a mutable list. - Simplified `evaluateTemplate` by removing cache-related logic and comments on minor performance differences.
…n `Target` - Added `Escaping` enum with options `NONE`, `URL`, and `SEGMENT` to enhance placeholder escaping in URLs. - Updated `Target` to include configurable escaping behavior and a helper method `getEscapingFunction()`. - Extended test cases to validate new escaping options in various scenarios.
- Updated `URI` to leverage `var` for better readability and added JavaDoc for `resolve` method. - Improved `ResolverMap` handling of URI schemes, including `classpath:` and `internal:`. - Added new tests in `HTTPClientInterceptorTest` for complete path computation, including URL encoding.
…e template evaluation - Added `UriIllegalCharacterDetector` for validating URI components with RFC 3986 compliance and optional extensions. - Updated `Target` to skip template evaluation for URLs without placeholders and log warnings for insecure configurations. - Enhanced `TemplateUtil.containsTemplateMarker` with null safety. - Improved `RewriteInterceptor` to handle query parsing errors with more informative logging. - Refactored tests and added new ones to validate changes.
- Refactored `removeLastSegment` method for concise logic using `Math.max()` to avoid redundant checks. - Replaced `assertEquals(false, ...)` with `assertFalse(...)` for improved readability in tests. - Cleaned up unused imports in `ResolverMapCombineTest`.
- Replaced `pathSeg` method with `pathEncode` for improved naming clarity and functionality in line with RFC 3986. - Introduced `IPv6Util` for robust validation of IPv6 address text, ensuring only allowed characters are accepted. - Refactored `Target` to include URL encoding strategy through a dedicated `escapingFunction` and removed redundant logic. - Streamlined URI combination logic in `ResolverMap` with utility methods for handling slashes and file URIs. - Added extensive tests covering URL path encoding, IPv6 address validation, and robustification of URI logic for edge cases.
… and utilities - Adjusted visibility modifiers of constants to `final` for better immutability (`URITest`). - Reordered equality assertion arguments for better readability (`TargetTest`). - Replaced redundant comments with concise `<p/>` tags in JavaDocs for uniformity. - Refactored tests to leverage `assertNull` for null comparisons (`FileUtilTest`). - Removed unused method `stripIpv6BracketsIfNeeded` and redundant imports for cleaner code. - Enhanced URI combination logic and streamlined utility imports (`DispatchingInterceptor`).
…comprehensive RFC 3986 compliance tests - Added `URIFactory` to support flexible URI creation with options for illegal character handling and backslash escaping. - Implemented extensive
… parsing - Added edge case assertions for parsing IPv6 with empty ports. - Introduced additional checks for IPv4 host and port parsing scenarios. - Renamed test methods to reflect broader case coverage.
…Target` and `CallInterceptor` - Replaced `evaluateExpressions` with `urlIsTemplate` for better semantics and clarity. - Improved `Target` and `CallInterceptor` to skip URL evaluation when no template markers are present. - Introduced stricter validation for configurations involving template markers and illegal characters. - Updated relevant tests to validate changes and added new test cases for URL template evaluation.
…evaluation - Introduced JSON escaping support for `setBody` based on `contentType`. - Enhanced `ExchangeExpression` to centralize router retrieval logic. - Updated tests to include validation logging for transformation scenarios. - Fixed minor formatting inconsistencies and improved code readability.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSetBodyInterceptor now constructs TemplateExchangeExpression via a new protected getExchangeExpression() that selects an escaping function from the request/response Content-Type (with fallback); EscapingUtil gains JSON/XML escaping and MIME lookup; ExchangeExpression centralizes nullable router resolution; tests add null-value cases and RestAssured failure logging. Changes
Sequence DiagramsequenceDiagram
participant SetBodyInterceptor as SetBodyInterceptor
participant EscapingUtil as EscapingUtil
participant CommonFuncs as CommonBuiltInFunctions
participant StringEscaper as StringEscapeUtils
participant ExchangeExpr as ExchangeExpression
participant TemplateExpr as TemplateExchangeExpression
SetBodyInterceptor->>EscapingUtil: getEscapingFunction(contentType)
alt mime is JSON
EscapingUtil->>CommonFuncs: resolve toJSON
CommonFuncs-->>EscapingUtil: jsonEscaper(func)
else mime is XML
EscapingUtil->>StringEscaper: resolve escapeXml11
StringEscaper-->>EscapingUtil: xmlEscaper(func)
else no mapping
EscapingUtil-->>SetBodyInterceptor: Optional.empty()
end
SetBodyInterceptor->>ExchangeExpr: getRouter(interceptor)
ExchangeExpr-->>SetBodyInterceptor: router (nullable)
SetBodyInterceptor->>TemplateExpr: create(expression, language, router, escapingFunc)
TemplateExpr-->>SetBodyInterceptor: TemplateExchangeExpression instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
This pull request needs "/ok-to-test" from an authorized committer. |
![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: 1
🧹 Nitpick comments (1)
core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java (1)
49-53:getEscapingFunction(String mimeType)only handles JSON — consider documenting the extension pattern.Currently this method returns
Optional.empty()for all non-JSON MIME types (including XML, HTML, etc.). This meanssetBodywithcontentType="application/xml"will fall back toNONEescaping, which could lead to injection of special XML characters. This is acceptable as a first step, but worth noting in the Javadoc that additional MIME types (e.g., XML) may need escaping support in the future.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java` around lines 49 - 53, Update the Javadoc for getEscapingFunction(String mimeType) to explicitly state that it currently only returns an escaping function for JSON (via the JSON constant) and returns Optional.empty() for all other MIME types (so callers fall back to NONE), and note that additional MIME types such as XML and HTML may require escaping in the future; also mention the intended extension pattern (add isXml/isHtml predicates and corresponding escaping functions and return them from getEscapingFunction) and reference the getEscapingFunction method and the JSON/ NONE escaping constants so maintainers know where to extend behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptor.java`:
- Around line 94-100: The warning in getExchangeExpression() is logged even when
contentType is null; change the behavior so the warn() call inside the
getEscapingFunction().orElseGet(...) is only executed when contentType is
non-null (i.e., log only if a user explicitly configured a contentType but no
escaping strategy exists). Update the
TemplateExchangeExpression.newInstance(...) call path in getExchangeExpression()
to check contentType != null before calling log.warn and otherwise return
getEscapingFunction(NONE) silently; reference getExchangeExpression(),
TemplateExchangeExpression.newInstance, getEscapingFunction(contentType),
contentType and NONE to locate the change.
---
Nitpick comments:
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 49-53: Update the Javadoc for getEscapingFunction(String mimeType)
to explicitly state that it currently only returns an escaping function for JSON
(via the JSON constant) and returns Optional.empty() for all other MIME types
(so callers fall back to NONE), and note that additional MIME types such as XML
and HTML may require escaping in the future; also mention the intended extension
pattern (add isXml/isHtml predicates and corresponding escaping functions and
return them from getEscapingFunction) and reference the getEscapingFunction
method and the JSON/ NONE escaping constants so maintainers know where to extend
behavior.
core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptor.java
Show resolved
Hide resolved
- Introduced XML escaping using `StringEscapeUtils::escapeXml11`. - Updated `getEscapingFunction` to handle XML content type. - Added `XML` escaping type to `Escaping` enum. - Enhanced utility imports and updated dependencies for XML handling.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java (1)
36-52: Javadoc doesn't document the newJSONandXMLenum members.The Javadoc block at lines 36–45 describes only
NONE,URL, andSEGMENT. Consider adding descriptions forJSONandXMLto keep the documentation consistent.📝 Suggested Javadoc addition
* - {`@code` SEGMENT}: Encodes strings as safe URI path segments, ensuring they do not introduce * path separators, query delimiters, or other unsafe characters, as per RFC 3986. + * - {`@code` JSON}: Escapes strings for safe inclusion in a JSON context. + * - {`@code` XML}: Escapes strings for safe inclusion in an XML context using XML 1.1 rules. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java` around lines 36 - 52, The Javadoc for the Escaping enum currently documents NONE, URL and SEGMENT but omits the new JSON and XML constants; update the Javadoc comment for the Escaping enum to add brief descriptions for JSON (escape strings for safe inclusion in JSON values, e.g., quote/backslash/control-char escaping) and XML (escape strings for safe inclusion in XML content/attributes, e.g., entity-encoding for <, >, &, quotes) so each enum member (Escaping.NONE, Escaping.URL, Escaping.SEGMENT, Escaping.JSON, Escaping.XML) is documented consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 69-70: EscapingUtil currently maps JSON ->
CommonBuiltInFunctions::toJSON which quotes string inputs and can produce
double-quoted JSON or a literal "null" when serialization fails; inspect
TemplateExchangeExpression usage in SetBodyInterceptor to determine whether
templates expect expr to produce a fully quoted JSON value (e.g., "field":
${expr}) or a raw unquoted string (e.g., "field": "${expr}"). If templates
expect raw escaped strings, change the JSON mapping to use
StringEscapeUtils::escapeJson (or implement an unquoting wrapper) instead of
toJSON; otherwise keep toJSON but modify CommonBuiltInFunctions.toJSON to (a)
avoid returning null on failure (throw or return a safe value) and (b) only call
objectMapper.writeValueAsString for non-String objects while for String inputs
either return the quoted form intentionally or provide an alternative API;
update EscapingUtil mapping accordingly and add tests that cover both string and
non-string inputs to TemplateExchangeExpression to prevent double-quoting.
---
Nitpick comments:
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 36-52: The Javadoc for the Escaping enum currently documents NONE,
URL and SEGMENT but omits the new JSON and XML constants; update the Javadoc
comment for the Escaping enum to add brief descriptions for JSON (escape strings
for safe inclusion in JSON values, e.g., quote/backslash/control-char escaping)
and XML (escape strings for safe inclusion in XML content/attributes, e.g.,
entity-encoding for <, >, &, quotes) so each enum member (Escaping.NONE,
Escaping.URL, Escaping.SEGMENT, Escaping.JSON, Escaping.XML) is documented
consistently.
Collaborator
|
/ok-to-test |
…InterceptorTest` - Added tests to verify behavior for null values in JSONPath and Groovy expressions. - Enhanced `SetBodyInterceptorTest` with additional scenarios for content escaping. - Updated imports for improved code readability and consistency.
…lient-interceptor' into compute-url-expression-at-http-client-interceptor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java (1)
56-63: Inconsistent spacing insideOptional.of(…)calls.Both calls on lines 58 and 61 have a spurious space after the opening parenthesis:
Optional.of( getEscapingFunction(...)). Standard Java style keeps these together.📝 Proposed fix
- if (isJson(mimeType)) { - return Optional.of( getEscapingFunction(JSON)); - } - if (isXML(mimeType)) { - return Optional.of( getEscapingFunction(XML)); - } + if (isJson(mimeType)) { + return Optional.of(getEscapingFunction(JSON)); + } + if (isXML(mimeType)) { + return Optional.of(getEscapingFunction(XML)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java` around lines 56 - 63, The Optional.of calls in getEscapingFunction(String mimeType) have inconsistent spacing (e.g., Optional.of( getEscapingFunction(...))). In the getEscapingFunction method, remove the spurious space after the opening parenthesis so both calls are formatted as Optional.of(getEscapingFunction(JSON)) and Optional.of(getEscapingFunction(XML)); keep other logic (isJson, isXML) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 40-47: The Javadoc for EscapingUtil has the URL bullet broken by
misplaced JSON and XML bullet points; edit the comment in EscapingUtil (the
enum/constant descriptions for NONE, URL, JSON, XML, SEGMENT) so that the URL
entry is a contiguous sentence: move the {`@code` JSON} and {`@code` XML} bullet
lines out of the middle of the URL description and place them after the {`@code`
SEGMENT} entry, ensuring the URL description continues "replacing spaces and
other special characters with their percent-encoded counterparts (e.g., SPACE ->
+)." and that the {`@code` JSON} and {`@code` XML} bullets follow the {`@code`
SEGMENT} bullet.
In
`@core/src/test/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptorTest.java`:
- Around line 65-95: The two tests escapeNull and escapeNulGroovy mix
differences in expression languages with escaping behavior; fix by making their
intent explicit and independent: for escapeNull (method escapeNull) either set
sbi.setContentType(APPLICATION_JSON_UTF8) so it actually exercises JSON escaping
path in SetBodyInterceptor (and then update the assertion to the correct
JSON-escaped value), or rename the test to something like
nullWithNoContentTypeBecomesEmptyAndBreaksJson and update the comment to
document this is a deliberate regression anchor; for escapeNulGroovy, remove the
hedging comment, decide and assert the intended canonical behavior (update the
assertion to the confirmed correct JSON representation for a null value when
using GROOVY and APPLICATION_JSON_UTF8), and ensure both tests call
sbi.init(...) and sbi.handleRequest(exc) as they already do so the only
differences are language and sbi.setContentType(...) to isolate the escaping
pipeline from the expression language.
- Line 83: Rename the test method escapeNulGroovy in class
SetBodyInterceptorTest to escapeNullGroovy (fixing the missing "l"); update the
method declaration and any references or annotations (e.g., `@Test` usage or
calls) to the new name so the test runner and any reflective lookups continue to
work; ensure imports/compilation remain intact after the rename.
---
Duplicate comments:
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Line 71: The JSON escaper mapping in EscapingUtil currently uses
CommonBuiltInFunctions::toJSON which serialises values (adding surrounding
quotes for plain strings) causing double-quoting in templates; change the JSON
mapping to use StringEscapeUtils.escapeJson (or an equivalent non-serialising
JSON-string-escaping function) instead of CommonBuiltInFunctions::toJSON, update
the inline comment to reflect the chosen approach, and add/adjust unit tests or
template examples to assert that plain string inputs are escaped but not wrapped
in extra quotes; reference CommonBuiltInFunctions::toJSON,
StringEscapeUtils.escapeJson and the JSON case in EscapingUtil when making the
change.
---
Nitpick comments:
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 56-63: The Optional.of calls in getEscapingFunction(String
mimeType) have inconsistent spacing (e.g., Optional.of(
getEscapingFunction(...))). In the getEscapingFunction method, remove the
spurious space after the opening parenthesis so both calls are formatted as
Optional.of(getEscapingFunction(JSON)) and
Optional.of(getEscapingFunction(XML)); keep other logic (isJson, isXML)
unchanged.
core/src/test/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptorTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptorTest.java
Outdated
Show resolved
Hide resolved
… in `SetBodyInterceptorTest` - Refactored tests to consolidate null value handling for JSONPath and Groovy. - Enhanced `SetBodyInterceptor` to handle `Object` evaluation results, ensuring compatibility and proper type handling. - Updated helper methods to restructure and reuse test setup across scenarios.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java (1)
43-54: Align enum member order with the Javadoc bullet order.The Javadoc lists
NONE → URL → JSON → XML → SEGMENT, but the enum declaresNONE → URL → SEGMENT → JSON → XML. No behavioral impact sinceordinal()is never used, but the mismatch makes the doc harder to cross-reference.♻️ Proposed fix
public enum Escaping { NONE, URL, + JSON, + XML, SEGMENT, - JSON, - XML }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java` around lines 43 - 54, The enum declaration order in EscapingUtil.Escaping does not match the Javadoc bullet order; update the Escaping enum so its members appear in the same sequence as the Javadoc (NONE, URL, JSON, XML, SEGMENT) by reordering the constants in the Escaping enum declaration in EscapingUtil.java to match the documentation.core/src/test/java/com/predic8/membrane/core/interceptor/templating/TemplateInterceptorTest.java (1)
276-280:setJsonSample()is duplicated verbatim inSetBodyInterceptorTest.NullandTemplateInterceptorTest.Null.Consider extracting it to a shared test-fixture utility (e.g.
TestExchanges.jsonBodyWithNullField()) to avoid having two copies drift independently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/test/java/com/predic8/membrane/core/interceptor/templating/TemplateInterceptorTest.java` around lines 276 - 280, The method setJsonSample() duplicated in TemplateInterceptorTest.Null and SetBodyInterceptorTest.Null should be extracted to a shared test fixture to avoid drift: create a utility class (e.g. TestExchanges) with a method jsonBodyWithNullField() that returns the same Exchange currently built by setJsonSample() and update both TemplateInterceptorTest and SetBodyInterceptorTest to call TestExchanges.jsonBodyWithNullField() instead of their local setJsonSample() implementations; ensure the new utility references the same builder chain (post("/foo").json("{\"a\":null}").buildExchange()) and update imports/tests accordingly.core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptor.java (1)
72-77:result.toString()will produce broken JSON/XML for structured expression results.The comment
// Hope that all possibles return types are covered(note: "possibles" typo) acknowledges this branch is under-specified. If a Groovy expression returns aMaporList,toString()produces JVM output like{a=1, b=2}, not valid JSON. Given thatcontentTypecan beapplication/json, this path can silently corrupt the body. Consider using Jackson serialization or at minimum documenting that the caller is responsible for ensuring the expression returns a String.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptor.java` around lines 72 - 77, The branch in SetBodyInterceptor that falls back to result.toString() corrupts structured results (Map/List) for JSON/XML content types; change it so that when result is not a String you detect the message content type (e.g., application/json or application/xml) and serialize structured objects properly (use a shared Jackson ObjectMapper to writeValueAsBytes(result) for JSON, or an appropriate XML serializer for XML), and only use result.toString().getBytes(UTF_8) as a last resort (or document caller responsibility); update the code paths around the local variable result and msg.setBodyContent(...) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptor.java`:
- Around line 97-103: The warning in SetBodyInterceptor.getExchangeExpression
fires for null contentType because getEscapingFunction(contentType) returns
Optional.empty(); change the logic to only log when a non-null contentType is
provided but unrecognized: call getEscapingFunction(contentType) into a
variable, if contentType != null and the Optional is empty then log.warn
mentioning contentType, then pass the Optional.orElseGet(() ->
getEscapingFunction(NONE).get()) (or equivalent) into
TemplateExchangeExpression.newInstance; refer to
SetBodyInterceptor.getExchangeExpression, getEscapingFunction, contentType, NONE
and TemplateExchangeExpression.newInstance to locate and update the code.
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 71-72: The JSON branch in EscapingUtil currently references
CommonBuiltInFunctions::toJSON which uses objectMapper.writeValueAsString and
therefore wraps string inputs (e.g., the literal "null") in quotes; switch the
JSON escaping to avoid quoting string values by either (a) using
StringEscapeUtils::escapeJson for string inputs and keeping
CommonBuiltInFunctions::toJSON for non-strings, or (b) replace the JSON mapper
with a function that detects an input of type String and returns an unquoted
escaped value (and only uses objectMapper for real objects), so that
EscapingUtil (the JSON case) preserves the literal "null" expected by
SetBodyInterceptorTest.Null and TemplateInterceptorTest.Null.
---
Nitpick comments:
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetBodyInterceptor.java`:
- Around line 72-77: The branch in SetBodyInterceptor that falls back to
result.toString() corrupts structured results (Map/List) for JSON/XML content
types; change it so that when result is not a String you detect the message
content type (e.g., application/json or application/xml) and serialize
structured objects properly (use a shared Jackson ObjectMapper to
writeValueAsBytes(result) for JSON, or an appropriate XML serializer for XML),
and only use result.toString().getBytes(UTF_8) as a last resort (or document
caller responsibility); update the code paths around the local variable result
and msg.setBodyContent(...) accordingly.
In `@core/src/main/java/com/predic8/membrane/core/util/uri/EscapingUtil.java`:
- Around line 43-54: The enum declaration order in EscapingUtil.Escaping does
not match the Javadoc bullet order; update the Escaping enum so its members
appear in the same sequence as the Javadoc (NONE, URL, JSON, XML, SEGMENT) by
reordering the constants in the Escaping enum declaration in EscapingUtil.java
to match the documentation.
In
`@core/src/test/java/com/predic8/membrane/core/interceptor/templating/TemplateInterceptorTest.java`:
- Around line 276-280: The method setJsonSample() duplicated in
TemplateInterceptorTest.Null and SetBodyInterceptorTest.Null should be extracted
to a shared test fixture to avoid drift: create a utility class (e.g.
TestExchanges) with a method jsonBodyWithNullField() that returns the same
Exchange currently built by setJsonSample() and update both
TemplateInterceptorTest and SetBodyInterceptorTest to call
TestExchanges.jsonBodyWithNullField() instead of their local setJsonSample()
implementations; ensure the new utility references the same builder chain
(post("/foo").json("{\"a\":null}").buildExchange()) and update imports/tests
accordingly.
rrayst
previously approved these changes
Feb 20, 2026
- Added `.log().ifValidationFails()` to improve debugging of failing test scenarios.
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.
Summary by CodeRabbit
New Features
Tests
Documentation