gnodet-bot commented on code in PR #13491: URL: https://github.com/apache/maven/pull/13491#discussion_r4289042752
########## api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassWorldListener.java: ########## @@ -0,0 +1,49 @@ +/* + * 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.classworlds; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Nonnull; + +/** + * Listener interface for class realm lifecycle events. + * <p> + * Implementations of this interface can be registered with a {@link ClassWorld} + * to receive notifications when realms are created or disposed. + * </p> + * + * @since 4.1.0 + */ +@Experimental +public interface ClassWorldListener { + + /** + * Called when a new class realm is created. + * + * @param realm the newly created realm + */ + void realmCreated(@Nonnull ClassRealm realm); + + /** + * Called when a class realm is disposed. + * + * @param realm the disposed realm + */ Review Comment: 💡 **Both methods are abstract — callers must implement both even when they only care about one event.** For a stable listener API with forward compatibility, consider adding default no-op implementations. This also avoids breaking existing implementations if a third method is ever added. ```suggestion default void realmCreated(@Nonnull ClassRealm realm) {} /** * Called when a class realm is disposed. * * @param realm the disposed realm */ default void realmDisposed(@Nonnull ClassRealm realm) {} ``` ########## impl/maven-classworlds/src/test/java/org/apache/maven/api/classworlds/ApiTest.java: ########## @@ -0,0 +1,128 @@ +/* + * 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.classworlds; + +import java.net.URL; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Test to demonstrate the Maven 4 ClassWorlds API. + */ +class ApiTest { + + @Test + void testApiUsage() throws Exception { + // Create a ClassWorld using the implementation + ClassWorld world = new org.codehaus.plexus.classworlds.ClassWorld(); + + // Test basic API methods + assertNotNull(world); + + // Create a realm + ClassRealm realm = world.newRealm("test-realm"); + assertNotNull(realm); + assertEquals("test-realm", realm.getId()); + assertSame(world, realm.getWorld()); + + // Test getClassLoader() method + ClassLoader classLoader = realm.getClassLoader(); + assertNotNull(classLoader); + assertSame(realm, classLoader); // Should return the realm itself Review Comment: ⚠️ **`assertSame(realm, classLoader)` tests an implementation detail, not an API contract.** The Javadoc for `ClassRealm.getClassLoader()` says "Returns the underlying ClassLoader for this realm" — it does not guarantee that the realm IS-A ClassLoader. A delegation-based future implementation (wrapping a ClassLoader rather than extending URLClassLoader) would break this assertion without violating the API contract. Change to: ```suggestion assertNotNull(classLoader); assertTrue(classLoader instanceof ClassLoader); ``` -- 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]
