[GitHub] [maven] michael-o edited a comment on pull request #451: [MNG-7107] relax profile id validation, different from coordinate id

[GitBox]

michael-o edited a comment on pull request #451:
URL: https://github.com/apache/maven/pull/451#issuecomment-786917698


   @hboutemy Why didn't you wait for me to test? This one is incomplete. It 
misses a handling for these:
   ```
   diff --git 
a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
 
b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
   index 2e71520d1..f3e686851 100644
   --- 
a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
   +++ 
b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
   @@ -921,7 +921,7 @@ private boolean isValidProfileId( String id )
                    return false;
                default:
            }
   -        return true;
   +        return !( id.contains( "," ) || id.contains( " " ) );
        }
   
        @SuppressWarnings( "checkstyle:parameternumber" )
   diff --git 
a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
 
b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
   index 5de48c4f2..7645b16d5 100644
   --- 
a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
   +++ 
b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
   @@ -409,12 +409,17 @@ public void testInvalidProfileId()
        {
            SimpleProblemCollector result = validateRaw( 
"invalid-profile-ids.xml" );
   
   -        assertViolations( result, 0, 4, 0 );
   +        assertViolations( result, 0, 9, 0 );
   
            assertTrue( result.getErrors().get( 0 ).contains( "+invalid-id" ) );
            assertTrue( result.getErrors().get( 1 ).contains( "-invalid-id" ) );
            assertTrue( result.getErrors().get( 2 ).contains( "!invalid-id" ) );
            assertTrue( result.getErrors().get( 3 ).contains( "?invalid-id" ) );
   +        assertTrue( result.getErrors().get( 4 ).contains( "-?invalid-id" ) 
);
   +        assertTrue( result.getErrors().get( 5 ).contains( "+?invalid-id" ) 
);
   +        assertTrue( result.getErrors().get( 6 ).contains( "!?invalid-id" ) 
);
   +        assertTrue( result.getErrors().get( 7 ).contains( "invalid,id" ) );
   +        assertTrue( result.getErrors().get( 8 ).contains( "invalid id" ) );
        }
   
        public void testDuplicateProfileId()
   diff --git 
a/maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml
 
b/maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml
   index 74b670b9d..7419fd83d 100644
   --- 
a/maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml
   +++ 
b/maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml
   @@ -37,6 +37,21 @@ under the License.
            <profile>
                <id>?invalid-id</id>
            </profile>
   +        <profile>
   +            <id>-?invalid-id</id>
   +        </profile>
   +        <profile>
   +            <id>+?invalid-id</id>
   +        </profile>
   +        <profile>
   +            <id>!?invalid-id</id>
   +        </profile>
   +        <profile>
   +            <id>invalid,id</id>
   +        </profile>
   +        <profile>
   +            <id>invalid id</id>
   +        </profile>
            <profile>
                <id>valid-id</id>
            </profile>
   
   ```


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org