rshdhere commented on code in PR #6633: URL: https://github.com/apache/fineract/pull/6633#discussion_r4259938114
########## fineract-command/src/main/java/org/apache/fineract/command/core/HookableCommand.java: ########## @@ -0,0 +1,31 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.command.core; + +/** + * Implemented by commands whose successful execution must notify the webhooks registered for its event, as the legacy + * command processing does for every command. The names are the entity and action a hook subscribes to, e.g. + * {@code DOCUMENT} and {@code CREATE}. + */ +public interface HookableCommand { + + String hookEntityName(); Review Comment: Thanks for the review! The new pipeline doesn't know which webhook event a command corresponds to, and this interface is how a command tells it. Webhooks are matched by **entity + action**: `FineractHookListener` calls `retrieveHooksByEvent(entityName, actionName)`, so a hook subscribed to `DOCUMENT` / `CREATE` only fires for a `HookEvent` carrying exactly those names. In the legacy path those names come from the `CommandWrapper` (e.g. `.createDocument()` sets `DOCUMENT` / `CREATE`), and `SynchronousCommandProcessingService.publishHookEvent()` uses them. Since FINERACT-2246, documents go through the new `CommandDispatcher` instead. The new `Command<T>` carries only metadata (id, idempotency key, usernames, timestamps) and the payload; there is no entity or action name anywhere. So even an after-hook that wants to publish a `HookEvent` has nothing to match subscriptions against, which is why DOCUMENT webhooks stopped firing. `HookableCommand` fills that gap: - a command declares its webhook identity: `DocumentCreateCommand` returns `DOCUMENT` / `CREATE`, and so on; - `WebhookCommandHookAfter` publishes a `HookEvent` only for commands implementing it, so it's opt-in: commands that aren't webhook-relevant are untouched, and nothing changes for the legacy path (no double events; CLIENT CREATE still fires once); - the declaration lives next to the command, so migrating another resource to the new pipeline is just a matter of implementing the two methods. Alternatives I considered: deriving the names from the class name (`DocumentCreateCommand` → `DOCUMENT` / `CREATE`) breaks silently on renames and doesn't fit names like `UPDATE_RESCHEDULE`; a central command → (entity, action) mapping works but has to be kept in sync with every new command. If you'd prefer either of those, or a different approach altogether, I'm happy to rework it. -- 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]
