gnodet-bot commented on code in PR #13613: URL: https://github.com/apache/maven/pull/13613#discussion_r4322035753
########## api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelDialectManager.java: ########## @@ -0,0 +1,77 @@ +/* + * 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.services; + +import java.nio.file.Path; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import org.apache.maven.api.Dialect; +import org.apache.maven.api.Service; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; +import org.apache.maven.api.model.Model; + +/** + * The {@code ModelDialectManager} provides a mechanism for managing and + * translating between different dialects of Maven models. + * It allows reading and writing models in various dialects supported by the system. + * + * @since 4.1.0 + */ +public interface ModelDialectManager extends Service { + + /** + * Retrieves the set of all available dialects. + * The result is never {@code null} and always contains at least one element, + * the core {@link Dialect#XML} dialect. + * + * @return a non-null {@link Set} of available dialects. + */ + @Nonnull + Set<Dialect> getAvailableDialects(); + + /** + * Reads a Maven model from the specified directory using the specified dialect. + * + * @param dir the directory from which to read the model, must not be {@code null}. + * @param dialect the dialect to be used for reading, must not be {@code null}. + * @param options optional parameters for reading, may be {@code null}. + * @return an {@link Optional} containing the read model, or an empty {@link Optional} if reading fails. Review Comment: 📝 **Misleading Javadoc:** The `@return` says _"or an empty `Optional` if reading fails"_, but that's not accurate. Looking at `DefaultModelDialectManager.readModel()`, it delegates to `locateAndParse(dir, options)`: an empty `Optional` is returned only when `locate()` finds no file in the directory. If parsing actually *fails*, `ModelParserException` (unchecked, extends `MavenException`) is thrown — it doesn't yield an empty `Optional`. Suggested fix: ```suggestion * @return an {@link Optional} containing the read model, or an empty {@link Optional} if no model file is found in the directory. ``` ########## api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelDialectProvider.java: ########## @@ -0,0 +1,66 @@ +/* + * 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.spi; + +import org.apache.maven.api.Dialect; +import org.apache.maven.api.annotations.Consumer; +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.di.Named; + +/** + * SPI to provide a symmetric pair of {@link ModelParser} and {@link ModelWriter} for a given {@link Dialect}. + * <p> + * Implement this interface to register support for reading and writing Maven models in an alternative dialect. + * The implementation must be registered as a named DI component so that Maven can discover it. + * + * @since 4.1.0 + * @see ModelParser + * @see ModelWriter + * @see DialectProvider + */ +@Experimental +@Consumer +@Named +public interface ModelDialectProvider { Review Comment: 💡 **SPI consistency:** `ModelDialectProvider` is DI-discovered via `@Named` and injected as `List<ModelDialectProvider>` in `DefaultModelDialectManager`, participating in the same SPI discovery mechanism as `ModelParser` and `ModelWriter` — both of which extend `SpiService`. Consider whether `ModelDialectProvider` should also extend `SpiService` for consistency: ```suggestion public interface ModelDialectProvider extends SpiService { ``` The counterargument is that `ModelDialectProvider` is a composite delegating to components that already extend `SpiService`, but from a SPI-contract standpoint, the interface itself is the extension point — it's what Maven discovers and injects. ########## api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelDialectManager.java: ########## @@ -0,0 +1,77 @@ +/* + * 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.services; + +import java.nio.file.Path; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import org.apache.maven.api.Dialect; +import org.apache.maven.api.Service; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; +import org.apache.maven.api.model.Model; + +/** + * The {@code ModelDialectManager} provides a mechanism for managing and + * translating between different dialects of Maven models. + * It allows reading and writing models in various dialects supported by the system. + * + * @since 4.1.0 + */ +public interface ModelDialectManager extends Service { Review Comment: ⚠️ **Missing `@Experimental`:** `ModelDialectManager` is the only new interface in this PR without the `@Experimental` annotation. All other new interfaces (`Dialect`, `DialectProvider`, `ModelWriter`, `ModelDialectProvider`) carry it — this one should too for API-stability consistency. ```suggestion @org.apache.maven.api.annotations.Experimental public interface ModelDialectManager extends Service { ``` -- 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]
