Croway commented on code in PR #125:
URL:
https://github.com/apache/camel-upgrade-recipes/pull/125#discussion_r3852158108
##########
camel-upgrade-recipes/src/main/java/org/apache/camel/upgrade/camel418_3/RenameHeaderInXmlDsl.java:
##########
@@ -87,27 +101,54 @@ public Xml.Tag doVisitTag(Xml.Tag tag, ExecutionContext
ctx) {
// Check if this is a setHeader, header, or removeHeader tag
String tagName = t.getName();
- if ("setHeader".equals(tagName) || "header".equals(tagName) ||
"removeHeader".equals(tagName)) {
- // Look for the "name" attribute with oldHeaderName value
- return t.withAttributes(ListUtils.map(t.getAttributes(), attr
-> {
- if ("name".equals(attr.getKeyAsString()) &&
- oldHeaderName.equals(attr.getValueAsString())) {
- // Replace with new header name
- return attr.withValue(
- new Xml.Attribute.Value(
- attr.getValue().getId(),
- "",
- Markers.EMPTY,
- attr.getValue().getQuote(),
- newHeaderName
- )
- );
- }
+ boolean headerTag = "setHeader".equals(tagName) ||
"header".equals(tagName) || "removeHeader".equals(tagName);
+
+ t = t.withAttributes(ListUtils.map(t.getAttributes(), attr -> {
+ String value = attr.getValueAsString();
+ if (value == null) {
return attr;
- }));
+ }
+
+ // The "name" attribute of a header element holds the header
name itself
+ if (headerTag && "name".equals(attr.getKeyAsString()) &&
oldHeaderName.equals(value)) {
+ return withValue(attr, newHeaderName);
+ }
+
+ // Any other attribute may carry a Simple expression, e.g.
<log message="${header.x}"/>
Review Comment:
Fixed in e047318. Added `RecipesUtil.camelXmlDslPrecondition()` and gated
`RenameHeaderInXmlDsl` on it.
Detection is: the Spring beans namespace rules a document out, a
`camel.apache.org` namespace rules it in, otherwise it falls back to the root
element name — Camel XML DSL files are often written with no namespace at all,
and the existing tests in this repo use bare `<route>`, `<routes>` and
`<camelContext>` roots, so a namespace-only check would have been too strict.
Negative test `doesNotMigrateNonCamelXml` uses exactly your `<property
value="${header.operation}"/>` shape, and `migratesCamelRoutesNestedInBeans`
pins the ambiguous `<beans>` root down the other way.
Your point generalises further than the one recipe, though. `XmlDsl46Recipe`
matches `bean/property` in any XML with no gate, and a Spring bean definition
has precisely that shape — so running the Spring Boot migration over a project
containing a Spring bean file rewrote it into
`<bean><properties><property/></properties></bean>`, silently producing invalid
Spring configuration. I reproduced it on a scratch multi-module project and
gated that recipe too, with a negative test.
That is a pre-existing defect rather than a 4.22 one, so say the word if you
would rather I split it into its own PR. Worth noting the gap is systemic: of
the 13 classes using `AbstractCamelXmlVisitor`, these two are now the only
gated ones. I did not touch the remaining 11 here — happy to raise a follow-up
ticket for them.
--
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]
