gnodet commented on code in PR #13011:
URL: https://github.com/apache/maven/pull/13011#discussion_r3951703244
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Event.java:
##########
@@ -21,54 +21,99 @@
import java.util.Optional;
import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
import org.apache.maven.api.annotations.Nonnull;
/**
- * Event sent by maven during various phases of the build process.
- * Such events can be listened to using {@link Listener}s objects
- * registered in the {@link Session}.
+ * Base interface for all Maven events.
+ * Specific event families extend this interface to provide typed event data.
+ * Events can be listened to using {@link Listener} objects registered in the
{@link Session}.
*
+ * @see ExecutionEvent
+ * @see RepositoryEvent
+ * @see Listener
* @since 4.0.0
*/
@Experimental
+@Immutable
public interface Event {
/**
- * Gets the type of the event.
+ * Returns the session from which this event originates.
*
- * @return the type of the event, never {@code null}
+ * @return the current session, never {@code null}
*/
@Nonnull
- EventType getType();
+ Session session();
/**
* Gets the session from which this event originates.
*
* @return the current session, never {@code null}
+ * @deprecated Use {@link #session()} instead.
+ */
+ @Deprecated(since = "4.1.0", forRemoval = true)
+ @Nonnull
+ default Session getSession() {
+ return session();
+ }
+
+ /**
+ * Gets the type of the event.
+ *
+ * @return the type of the event, never {@code null}
Review Comment:
⚠️ **Javadoc/contract mismatch on deprecated `getType()`:** The comment says
`@return the type of the event, never {@code null}`, and `@Nonnull` is present,
but the implementation throws `UnsupportedOperationException` for any `Event`
that is not an `ExecutionEvent`. Those guarantees are now false. The correct
Javadoc should reflect the restricted scope:
```suggestion
/**
* Gets the type of the event.
*
* @return the type of the event, never {@code null}
* @throws UnsupportedOperationException if this event is not an {@link
ExecutionEvent}
* @deprecated Use {@link ExecutionEvent#type()} instead.
*/
```
Alternatively — and probably cleaner — leave the `@Nonnull` off the
deprecated bridge entirely and just let the `UnsupportedOperationException`
speak for itself in the deprecation note.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/RepositoryEvent.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.maven.api;
+
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.services.RequestTrace;
+
+/**
+ * Describes an artifact or metadata operation performed against a repository.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+@Immutable
+public interface RepositoryEvent extends Event {
+
+ /**
+ * Returns the kind of repository operation represented by this event.
+ */
+ @Nonnull
+ RepositoryEventType type();
+
+ /**
+ * Returns the Maven session associated with the underlying repository
system session.
+ * Sessions derived from it share the same repository event and listener
scope.
+ */
+ @Nonnull
+ Session session();
Review Comment:
**Inconsistency: redundant `session()` re-declaration.** `Event` already
declares `@Nonnull Session session()`, so sub-interfaces get it for free.
`ExecutionEvent` does not re-declare it; `RepositoryEvent` does, with a
different Javadoc that adds implementation details (`"Sessions derived from it
share the same repository event and listener scope."`). That implementation
detail belongs in the `DefaultRepositoryEvent` or in `MavenRepositoryListener`,
not in the public API interface.
Drop the override to keep the two sub-interfaces consistent:
```suggestion
```
(Remove lines 45–50 entirely; the inherited declaration from `Event` is
sufficient.)
--
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]
