mayurbm commented on PR #25555:
URL: https://github.com/apache/camel/pull/25555#issuecomment-5424494119

   Hi @davsclaus, thank you for the detailed review — you are absolutely right 
on all four points. Updated the PR:
   
   **Comment 1 (core concern — expensive DOM construction):**
   Restored the `looksLikeXml(byte[])` byte-sniff prolog guard on 
`toDOMDocument(byte[], Exchange)` and `toDOMDocument(StreamCache, Exchange)`. 
The check runs *before* `DocumentBuilder.parse()` is ever called, so no DOM 
allocation occurs for obviously non-XML content (empty body, JSON, HTML, plain 
text). This directly addresses the production pain of 5M occurrences causing GC 
pressure.
   
   **Comment 2 (return null / allowNull):**
   Both `byte[]` and `StreamCache` overloads now use `@Converter(allowNull = 
true)` and `return null` when `looksLikeXml` returns false — exactly the 
behaviour the JIRA specified, letting the framework fall through gracefully.
   
   **Comment 3 (fromType diagnostic):**
   Fixed. `toDOMDocument(byte[])` and `toDOMDocument(StreamCache)` each call 
`DocumentBuilder.parse()` directly with a fresh `ByteArrayInputStream` — the 
`InputStream` overload is no longer in the call chain for these paths, so 
`getFromType()` will correctly report `byte[]` or `StreamCache`.
   
   **Comment 4 (test method naming):**
   All test methods renamed to camelCase (`testLooksLikeXmlNullReturnsFalse`, 
`testToDOMDocumentJsonBodyReturnsNull`, etc.) to match project convention.
   
   Local validation:
   - `mvn formatter:format impsort:sort` — no changes needed
   - `Tests run: 21, Failures: 0, Errors: 0` (13 `looksLikeXml` unit tests + 4 
integration tests + 3 existing tests)
   
   All 4 review threads resolved.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to