goutamadwant commented on PR #13011:
URL: https://github.com/apache/maven/pull/13011#issuecomment-5565542621
> ## API Design Suggestion: Event/Listener Hierarchy
> The current PR introduces `RepositoryListener` / `RepositoryEvent` as
standalone types with separate registration methods on `Session`. This works,
but it creates an inconsistency with the existing `Listener` / `Event` API
(build lifecycle events) — two parallel, unrelated listener systems on the same
`Session` with different design patterns:
>
> * `Listener` uses `@FunctionalInterface` single-dispatch
(`onEvent(Event)`) — consumers must switch on `EventType`
> * `RepositoryListener` uses typed `default` callbacks
(`artifactDownloading()`, etc.) — much better ergonomics
>
> **Proposed hierarchy** for 4.1.0: introduce `Event` and `Listener` as base
marker types, with `ExecutionEvent`/`ExecutionListener` (build lifecycle) and
`RepositoryEvent`/`RepositoryListener` (repository operations) as typed
specializations.
>
> ### Event hierarchy
> ```java
> @Experimental @Immutable
> public interface Event {
> @Nonnull Session session();
> }
>
> @Experimental @Immutable
> public interface ExecutionEvent extends Event {
> @Nonnull ExecutionEventType type();
> @Nonnull Optional<Project> project();
> @Nonnull Optional<MojoExecution> mojoExecution();
> @Nonnull Optional<Exception> exception();
> }
>
> @Experimental @Immutable
> public interface RepositoryEvent extends Event {
> @Nonnull RepositoryEventType type();
> @Nonnull Optional<Artifact> artifact();
> @Nonnull Optional<RepositoryMetadata> metadata();
> @Nonnull Optional<Path> path();
> @Nonnull Optional<Repository> repository();
> @Nonnull Optional<Exception> exception();
> @Nonnull List<Exception> exceptions();
> @Nonnull Optional<RequestTrace> trace();
> }
> ```
>
> ### Listener hierarchy
> ```java
> @Experimental @Consumer
> public interface Listener {
> /** @deprecated Implement ExecutionListener or RepositoryListener
instead. */
> @Deprecated
> default void onEvent(@Nonnull Event event) {}
> }
>
> @Experimental @Consumer
> public interface ExecutionListener extends Listener {
> default void sessionStarted(@Nonnull ExecutionEvent event) {}
> default void sessionEnded(@Nonnull ExecutionEvent event) {}
> default void projectDiscoveryStarted(@Nonnull ExecutionEvent event) {}
> default void projectStarted(@Nonnull ExecutionEvent event) {}
> default void projectSucceeded(@Nonnull ExecutionEvent event) {}
> default void projectFailed(@Nonnull ExecutionEvent event) {}
> default void projectSkipped(@Nonnull ExecutionEvent event) {}
> default void mojoStarted(@Nonnull ExecutionEvent event) {}
> default void mojoSucceeded(@Nonnull ExecutionEvent event) {}
> default void mojoFailed(@Nonnull ExecutionEvent event) {}
> default void mojoSkipped(@Nonnull ExecutionEvent event) {}
> default void forkStarted(@Nonnull ExecutionEvent event) {}
> default void forkSucceeded(@Nonnull ExecutionEvent event) {}
> default void forkFailed(@Nonnull ExecutionEvent event) {}
> default void forkedProjectStarted(@Nonnull ExecutionEvent event) {}
> default void forkedProjectSucceeded(@Nonnull ExecutionEvent event) {}
> default void forkedProjectFailed(@Nonnull ExecutionEvent event) {}
> }
>
> @Experimental @Consumer
> public interface RepositoryListener extends Listener {
> default void artifactDescriptorInvalid(@Nonnull RepositoryEvent event)
{}
> default void artifactDescriptorMissing(@Nonnull RepositoryEvent event)
{}
> default void metadataInvalid(@Nonnull RepositoryEvent event) {}
> default void artifactResolving(@Nonnull RepositoryEvent event) {}
> default void artifactResolved(@Nonnull RepositoryEvent event) {}
> // ... etc (19 typed callbacks, as in this PR)
> }
> ```
>
> ### Session impact
> Single registration point — no overloaded methods needed:
>
> ```java
> // Session keeps ONE set of listener methods for both types:
> void registerListener(@Nonnull Listener listener);
> void unregisterListener(@Nonnull Listener listener);
> Collection<Listener> getListeners();
> ```
>
> The dispatcher routes via `instanceof ExecutionListener` / `instanceof
RepositoryListener`. A listener can even implement both.
>
> ### Benefits
> * **Uniform design** — both event families use typed `default` callbacks,
no more `@FunctionalInterface` single-dispatch
> * **Single registration** — no separate
`registerListener(RepositoryListener)` / `getRepositoryListeners()` on `Session`
> * **Extensible** — future event categories (transfer, toolchain) just add
`XxxEvent extends Event` + `XxxListener extends Listener`, no `Session` changes
> * **Backward compatible** — old `Listener.onEvent()` stays as a
`@Deprecated default`, existing consumers keep compiling
> * **Noun-style accessors** on new types (consistent with [Switch core API
value types to noun-style accessors
#13036](https://github.com/apache/maven/pull/13036)), `@Immutable` events
> * `EventType` → `ExecutionEventType` for symmetry with
`RepositoryEventType`
@gnodet Updated to one listener registration path with typed execution and
repository callbacks, including listeners implementing both interfaces. Added
noun-style event accessors and ExecutionEventType.
I kept Listener functional and retained the existing Event getters: making
onEvent a default would break existing lambdas, and replacing Event with a
marker would remove its current methods. SessionEvent provides the shared event
base, while TypedListener supplies the common default without conflicting
inherited methods.
The PR description includes the full integration results and successful
redirect-test retry.
--
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]
