gnodet-bot commented on code in PR #27835:
URL: https://github.com/apache/camel/pull/27835#discussion_r4291530952
##########
core/camel-api/src/main/java/org/apache/camel/Route.java:
##########
@@ -380,16 +380,18 @@ default Resource getTemplatedRouteResource() {
/**
* Get the route type.
*
- * Important: is null after the route has been created.
- *
- * @return the route type during creation of the route, is null after the
route has been created.
+ * @return the route type, no longer null after the route has been created.
*/
@Nullable
NamedNode getRoute();
/**
* Clears the route model when its no longer needed.
+ *
+ * @deprecated since 4.24.0 - the route model is retained at runtime; this
method is a no-op and will be removed in
+ * a later release.
*/
+ @Deprecated(since = "4.24.0", forRemoval = true)
void clearRouteModel();
Review Comment:
⚠️ **Still open (davsclaus thread).** `clearRouteModel()` is still
`abstract` — the upgrade guide at line 26 tells custom `Route` implementations
they "can remove the override", but removing an override of an abstract method
is a compile error. The upgrade guide becomes true only if this is changed to a
`default` no-op.
Changing `abstract` → `default` with an empty body is source-compatible and
binary-compatible: existing implementations that still override it continue to
compile and run, and those that drop the override inherit the no-op default.
```suggestion
@Deprecated(since = "4.24.0", forRemoval = true)
default void clearRouteModel() {
// no-op: the route model is retained at runtime since 4.24.0
}
```
With this change, the `DefaultRoute.clearRouteModel()` override (which is
also now a no-op) can be removed entirely.
##########
core/camel-api/src/main/java/org/apache/camel/Route.java:
##########
@@ -380,16 +380,18 @@ default Resource getTemplatedRouteResource() {
/**
* Get the route type.
*
- * Important: is null after the route has been created.
- *
- * @return the route type during creation of the route, is null after the
route has been created.
+ * @return the route type, no longer null after the route has been created.
Review Comment:
⚠️ **Still open from previous review.** The `@return` text says "no longer
null after the route has been created" while the `@Nullable` annotation on line
385 tells IDEs and null-analysis tools that `null` is a valid return — these
send contradictory signals.
davsclaus correctly noted that `@Nullable` must stay (two tests create
`DefaultRoute` with a null route: `ProcessorReifierTest` and
`DefaultExceptionPolicyStrategyTest`). The `@return` text is what needs fixing
to match reality.
```suggestion
* @return the route model this route was created from, or {@code null}
if this route was not created from a model
```
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -13,6 +13,18 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.23 to 4.24
+=== camel-core - Route.clearRouteModel() is deprecated
+
+The `Route.clearRouteModel()` method is deprecated and is now a no-op. It will
be removed in a later release.
+
+This method dropped the references a runtime route held to its model (the
route definition, the error handler factory
+and the error handlers) once the route was created at startup. The
`CamelContext` already keeps the route definitions
+in its model, so clearing them on the route saved little memory at the cost of
making `Route.getRoute()` and
+`Route.getErrorHandlerFactory()` return `null` for routes created at startup
(but not for routes added after startup).
+
+Applications that called `Route.clearRouteModel()` directly can remove the
call. Implementations of the `Route` interface
+that override `clearRouteModel()` can remove the override.
Review Comment:
⚠️ **Still open (davsclaus thread).** This sentence is incorrect as long as
`clearRouteModel()` remains `abstract` in the `Route` interface. An
implementation that removes its override of an abstract method will not
compile. See the inline suggestion on `Route.java` — if `clearRouteModel()` is
made `default`, then this sentence becomes accurate.
--
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]
