davsclaus commented on code in PR #26054:
URL: https://github.com/apache/camel/pull/26054#discussion_r3921297663
##########
components/camel-platform-http/src/main/java/org/apache/camel/component/platform/http/HttpEndpointModel.java:
##########
@@ -25,7 +25,7 @@
/**
* Model of available http endpoints.
*/
-public class HttpEndpointModel implements Comparable<HttpEndpointModel> {
+public class HttpEndpointModel {
Review Comment:
Removing `implements Comparable<HttpEndpointModel>` (and the public
`compareTo`) is a **public API break** that isn't required by the fix — the
`equals`/`hashCode` change plus the `TreeSet`→`LinkedHashSet` switch is
sufficient on its own. `HttpEndpointModel` is public, and downstream consumers
(e.g. `camel-spring-boot`'s `CamelRequestHandlerMapping`, `camel-quarkus`)
receive these models via `PlatformHttpListener` and may sort/store them.
Per the project standard *"Do NOT change public API signatures without
justification / Maintain backwards compatibility for public APIs"*, could you
either keep `Comparable` (it's harmless now that `LinkedHashSet` is used), or
justify the removal in the PR description and add a `camel-4x-upgrade-guide`
entry? I confirmed nothing inside `apache/camel` relies on it, but the
spring-boot/quarkus side is out of this repo.
##########
components/camel-platform-http/src/main/java/org/apache/camel/component/platform/http/PlatformHttpComponent.java:
##########
@@ -167,13 +167,48 @@ public void removeHttpEndpoint(String uri) {
this.removeHttpEndpoint(this.httpEndpoints, uri);
}
+ /**
+ * Removes the http endpoint registered for the given consumer.
+ */
+ public void removeHttpEndpoint(Consumer consumer) {
+ if (consumer == null) {
+ return;
+ }
+ this.removeHttpEndpoint(this.httpEndpoints, consumer);
+ }
+
/**
* Removes a known http endpoint managed by this component.
*/
public void removeHttpManagementEndpoint(String uri) {
this.removeHttpEndpoint(this.httpManagementEndpoints, uri);
}
+ /**
+ * Removes the http management endpoint registered for the given consumer.
+ */
+ public void removeHttpManagementEndpoint(Consumer consumer) {
Review Comment:
`removeHttpManagementEndpoint(Consumer)` looks unused —
`DefaultPlatformHttpConsumer.doStop()` only calls the non-management
`removeHttpEndpoint(Consumer)` overload, and I don't see another caller. Fine
to keep for symmetry with the existing `String` overloads, but is that the
intent, or is it dead code that can be dropped?
--
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]
