Pil0tXia commented on code in PR #4272:
URL: https://github.com/apache/eventmesh/pull/4272#discussion_r1272338529
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/DeleteWebHookConfigHandler.java:
##########
@@ -33,29 +33,69 @@
import lombok.extern.slf4j.Slf4j;
+/**
+ * This class handles the HTTP requests of {@code
/webhook/deleteWebHookConfig} endpoint
+ * and deletes an existing WebHook configuration
+ * according to the given {@linkplain
org.apache.eventmesh.webhook.api.WebHookConfig WebHookConfig}.
+ * <p>
+ * The implementation of
+ * {@linkplain
org.apache.eventmesh.webhook.api.WebHookConfigOperation#deleteWebHookConfig
WebHookConfigOperation}
+ * interface depends on the {@code eventMesh.webHook.operationMode}
configuration in {@code eventmesh.properties}.
+ * <p>
+ * For example, when {@code eventMesh.webHook.operationMode=file}, It calls the
+ * {@linkplain
org.apache.eventmesh.webhook.admin.FileWebHookConfigOperation#deleteWebHookConfig
FileWebHookConfigOperation}
+ * method as implementation to delete the WebHook configuration file;
+ * <p>
+ * When {@code eventMesh.webHook.operationMode=nacos}, It calls the
+ * {@linkplain
org.apache.eventmesh.webhook.admin.NacosWebHookConfigOperation#deleteWebHookConfig
NacosWebHookConfigOperation}
+ * method as implementation to delete the WebHook configuration from Nacos.
+ * <p>
+ * The {@linkplain
org.apache.eventmesh.webhook.receive.storage.HookConfigOperationManager#deleteWebHookConfig
HookConfigOperationManager}
+ * which implements the {@linkplain
org.apache.eventmesh.webhook.api.WebHookConfigOperation WebHookConfigOperation}
+ * interface, does not participate in the implementation of this endpoint.
Review Comment:
The reason I specifically emphasize the difference between
`HookConfigOperationManager` and `FileWebHookConfigOperation` and
`NacosWebHookConfigOperation` is that I spent quite some time understanding the
distinctions among these three.
When you view the implementation of the `WebHookConfigOperation` interface
in IDEA, you will see these three on the screenshot, which might give the
impression that there are three different implementations of this method.
However, that is not the case in reality. In the `/webhook` endpoints,
EventMesh only utilizes the `file` and `nacos` implementations, and
`HookConfigOperationManager` is used differently from both of these and only
one method is being used, which can be somewhat confusing and misleading.
Therefore, I think it's necessary to provide an explanation. Perhaps stating
in the javadoc that "they are different" is not enough; I need to clarify "why
they are different".

--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]