goutamadwant commented on code in PR #13118: URL: https://github.com/apache/maven/pull/13118#discussion_r4012753676
########## api/maven-api-spi/src/main/java/org/apache/maven/api/spi/SettingsParser.java: ########## @@ -0,0 +1,83 @@ +/* + * 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 java.io.IOException; +import java.util.Map; + +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.annotations.Nullable; +import org.apache.maven.api.di.Named; +import org.apache.maven.api.services.Source; +import org.apache.maven.api.settings.Settings; + +/** + * Parses settings in an additional syntax. Maven selects a parser for each settings source, + * then performs interpolation, decryption, validation and merging on the returned settings. + * If no parser supports a source, Maven uses its XML settings reader. Multiple parsers + * supporting the same source are an error. + * <p> + * Parsers must be available in the container building the settings. In particular, a parser + * supplied by a core extension cannot read the bootstrap settings needed to resolve that + * extension. This SPI does not change settings file discovery or extension loading. + * + * @since 4.1.0 + */ +@Experimental +@Consumer +@Named +public interface SettingsParser extends SpiService { + /** + * Boolean parsing option indicating whether unknown input should be rejected. + */ + /** + * Option that can be specified in the options map. The value should be a {@code Boolean}; + * when {@code true} or absent, unknown input is rejected. + */ + String STRICT = "strict"; Review Comment: Fixed. Removed the duplicate block. STRICT now has a single Javadoc explaining the Boolean option and default strict behavior. ########## api/maven-api-spi/src/main/java/org/apache/maven/api/spi/SettingsParserException.java: ########## @@ -0,0 +1,71 @@ +/* + * 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.annotations.Experimental; +import org.apache.maven.api.services.MavenException; + +/** + * A syntax error in a settings source, with optional one-based line and column numbers. + * + * @since 4.1.0 + */ +@Experimental +public class SettingsParserException extends MavenException { + + /** + * The one-based index of the line containing the error. + */ + private final int lineNumber; + + /** + * The one-based index of the column containing the error. + */ + private final int columnNumber; + + public SettingsParserException() { + this(null, null); + } + + public SettingsParserException(String message) { + this(message, null); + } + + public SettingsParserException(String message, Throwable cause) { + this(message, -1, -1, cause); + } + + public SettingsParserException(String message, int lineNumber, int columnNumber, Throwable cause) { + super(message, cause); + this.lineNumber = lineNumber; + this.columnNumber = columnNumber; + } + + public SettingsParserException(Throwable cause) { + this(null, cause); + } Review Comment: Fixed. Message fallback now lives in the full constructor, covering every overload. Explicit messages are preserved; otherwise the cause's message is used, with a default when that is also null. Added constructor and builder-diagnostic tests, including preservation of causes and line/column information. ########## impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsBuilder.java: ########## @@ -159,35 +167,24 @@ private Settings readSettings( Settings settings; try { - try (InputStream is = settingsSource.openStream()) { - settings = settingsXmlFactory.read(XmlReaderRequest.builder() - .inputStream(is) - .location(settingsSource.getLocation()) - .strict(true) - .build()); - } catch (XmlReaderException e) { - try (InputStream is = settingsSource.openStream()) { - settings = settingsXmlFactory.read(XmlReaderRequest.builder() - .inputStream(is) - .location(settingsSource.getLocation()) - .strict(false) - .build()); - Location loc = e.getCause() instanceof XMLStreamException xe ? xe.getLocation() : null; - problems.reportProblem(new DefaultBuilderProblem( - settingsSource.getLocation(), - loc != null ? loc.getLineNumber() : -1, - loc != null ? loc.getColumnNumber() : -1, - e, - e.getMessage(), - BuilderProblem.Severity.WARNING)); - } + SettingsParser parser = selectParser(settingsSource); + try { + settings = parser.parse(settingsSource, Map.of(SettingsParser.STRICT, true)); + } catch (SettingsParserException e) { + settings = parser.parse(settingsSource, Map.of(SettingsParser.STRICT, false)); + problems.reportProblem(new DefaultBuilderProblem( + settingsSource.getLocation(), + e.getLineNumber(), + e.getColumnNumber(), + e, + e.getMessage(), + BuilderProblem.Severity.WARNING)); Review Comment: Fixed. The strict warning is now recorded before the lenient retry. Added regressions for retries ending in either a parse error or IOException, verifying that both the original warning and subsequent fatal problem are retained. Updated the SPI documentation accordingly. ########## impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsBuilder.java: ########## @@ -159,35 +167,24 @@ private Settings readSettings( Settings settings; try { - try (InputStream is = settingsSource.openStream()) { - settings = settingsXmlFactory.read(XmlReaderRequest.builder() - .inputStream(is) - .location(settingsSource.getLocation()) - .strict(true) - .build()); - } catch (XmlReaderException e) { - try (InputStream is = settingsSource.openStream()) { - settings = settingsXmlFactory.read(XmlReaderRequest.builder() - .inputStream(is) - .location(settingsSource.getLocation()) - .strict(false) - .build()); - Location loc = e.getCause() instanceof XMLStreamException xe ? xe.getLocation() : null; - problems.reportProblem(new DefaultBuilderProblem( - settingsSource.getLocation(), - loc != null ? loc.getLineNumber() : -1, - loc != null ? loc.getColumnNumber() : -1, - e, - e.getMessage(), - BuilderProblem.Severity.WARNING)); - } + SettingsParser parser = selectParser(settingsSource); + try { + settings = parser.parse(settingsSource, Map.of(SettingsParser.STRICT, true)); + } catch (SettingsParserException e) { + settings = parser.parse(settingsSource, Map.of(SettingsParser.STRICT, false)); + problems.reportProblem(new DefaultBuilderProblem( + settingsSource.getLocation(), + e.getLineNumber(), + e.getColumnNumber(), + e, + e.getMessage(), + BuilderProblem.Severity.WARNING)); } - } catch (XmlReaderException e) { - Location loc = e.getCause() instanceof XMLStreamException xe ? xe.getLocation() : null; + } catch (SettingsParserException e) { problems.reportProblem(new DefaultBuilderProblem( settingsSource.getLocation(), - loc != null ? loc.getLineNumber() : -1, - loc != null ? loc.getColumnNumber() : -1, + e.getLineNumber(), + e.getColumnNumber(), e, "Non-parseable settings " + settingsSource.getLocation() + ": " + e.getMessage(), BuilderProblem.Severity.FATAL)); Review Comment: Fixed. Parser conflicts now report a fatal builder problem directly, without the misleading “Non-parseable settings” prefix. This preserves the existing SettingsBuilderException handling. Strengthened the tests to check the complete diagnostic, source location, sorted provider names, and that neither parser runs. -- 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]
