[
https://issues.apache.org/jira/browse/MNG-6401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17739901#comment-17739901
]
ASF GitHub Bot commented on MNG-6401:
-------------------------------------
elharo commented on code in PR #1194:
URL: https://github.com/apache/maven/pull/1194#discussion_r1251925120
##########
api/maven-api-settings/src/main/mdo/settings.mdo:
##########
@@ -535,6 +539,62 @@
<type>String</type>
</field>
</fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0/1.3.0</version>
+ <code>
+ <![CDATA[
+ public boolean isActive() {
+ return (getActiveString() != null) ?
Boolean.parseBoolean(getActiveString()) : true;
+ }
+
+ public void setActive(boolean active) {
+ setActiveString(String.valueOf(active));
+ }
+
+ public int getPort() {
+ return (getPortString() != null) ? Integer.parseInt(getPortString()) :
8080;
+ }
+
+ public void setPort(int port) {
+ setPortString(String.valueOf(port));
+ }
+ ]]>
+ </code>
+ </codeSegment>
+ <codeSegment>
+ <version>2.0.0+</version>
+ <code>
+ <![CDATA[
+ /**
+ * Indicates if this proxy is active or not.
+ * To allow interpolation of this field, this method lazily parses
+ * the {@link #getActiveString()} value as a boolean and defaults to
{@code true}
+ * if not set.
+ *
+ * @return a boolean indicating whether this proxy is active or not
+ */
+ public boolean isActive() {
+ return (getActiveString() != null) ?
Boolean.parseBoolean(getActiveString()) : true;
+ }
+
+ /**
+ * Returns the port to use for this proxy.
+ * To allow interpolation of this field, this method lazily parses
+ * the {@link #getPortString()} value as an integer and defaults to {@code
8080}
+ * if not set.
+ *
+ * @return an integer indicating the port to use for this proxy
+ * @throws NumberFormatException if the port string is not a valid integer
Review Comment:
NumberFormatException is an IllegalArgumentException. It is very weird for a
method with no arguments to throw an IllegalArgumentException and very
unexpected for a getter to throw any exception at all.
Where/when does interpolation happen? Can you throw the exception at that
point and store an int for later use here?
##########
api/maven-api-settings/src/main/mdo/settings.mdo:
##########
@@ -459,17 +459,19 @@
The <code><proxy></code> element contains informations required
to a proxy settings.
]]></description>
<fields>
- <field>
- <name>active</name>
+ <field xml.tagName="active">
+ <name>activeString</name>
<version>1.0.0+</version>
<required>false</required>
<defaultValue>true</defaultValue>
<description>
<![CDATA[
Review Comment:
It's syntax sugar, unnecessary here though and just excess code. It can be
fixed one by one, or not, as you like. There's no need to change everything at
once.
##########
api/maven-api-settings/src/main/mdo/settings.mdo:
##########
@@ -535,6 +539,62 @@
<type>String</type>
</field>
</fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0/1.3.0</version>
+ <code>
+ <![CDATA[
+ public boolean isActive() {
+ return (getActiveString() != null) ?
Boolean.parseBoolean(getActiveString()) : true;
+ }
+
+ public void setActive(boolean active) {
+ setActiveString(String.valueOf(active));
+ }
+
+ public int getPort() {
+ return (getPortString() != null) ? Integer.parseInt(getPortString()) :
8080;
+ }
+
+ public void setPort(int port) {
+ setPortString(String.valueOf(port));
Review Comment:
Interesting. Why don't we? In general, the ealrier one can detect an error
such as a negative port the better.
> Support proxy port interpolation in settings.xml
> ------------------------------------------------
>
> Key: MNG-6401
> URL: https://issues.apache.org/jira/browse/MNG-6401
> Project: Maven
> Issue Type: New Feature
> Components: Bootstrap & Build, Settings
> Affects Versions: 3.0, 3.5.3
> Reporter: KATADA Junya
> Assignee: Guillaume Nodet
> Priority: Minor
> Fix For: 4.0.0-alpha-8
>
>
> If you use a property in proxy port of settings.xml, the property is not
> replaced and the port number is "0" instead of property value.
> My minimal demo about this problem is
> [here|https://github.com/jkatada/maven-property-demo].
> In my demo, export two environment variables as follows.
> {code}
> export MAVEN_PROXY_HOST_STRING=proxy.foo.com
> export MAVEN_PROXY_PORT_INT=18080
> {code}
> These variables are used in settings.xml for proxy settings.
> {code:xml}
> <proxy>
> <id>my_proxy</id>
> <active>true</active>
> <protocol>http</protocol>
> <host>${env.MAVEN_PROXY_HOST_STRING}</host>
> <port>${env.MAVEN_PROXY_PORT_INT}</port>
> <nonProxyHosts>local.net|some.host.com</nonProxyHosts>
> </proxy>
> {code}
> Execute maven-help-plugin to show effective settings.xml.
> {code}
> mvn help:effective-settings -X
> {code}
> The result is as follows.
> {code:xml}
> <proxy>
> <port>0</port>
> <host>proxy.foo.com</host>
> <nonProxyHosts>local.net|some.host.com</nonProxyHosts>
> <id>my_proxy</id>
> </proxy>
> {code}
> `*${env.MAVEN_PROXY_HOST_STRING}*` is replaced with `*proxy.foo.com*`,
> but `*${env.MAVEN_PROXY_PORT_INT}*` is not replaced with `*18080*`.
> I found the following WARNING message in console.
> {code:java}
> [WARNING] Some problems were encountered while building the effective settings
> [WARNING] Unable to parse element 'port', must be an integer (position:
> END_TAG seen ...<port>${env.MAVEN_PROXY_PORT_INT}</port>... @12:47) caused
> by: java.lang.NumberFormatException: For input string:
> "${env.MAVEN_PROXY_PORT_INT}" @ /root/.m2/settings.xml, line 12, column 47
> {code}
> I think that the cause of this problem is to parse settings.xml before
> interpolation.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
