You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/05/27 03:42:46 UTC

[GitHub] [maven] elharo commented on a change in pull request #347: [MNG-6802] Fix bug in FileProfileActivator

elharo commented on a change in pull request #347:
URL: https://github.com/apache/maven/pull/347#discussion_r430077310



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/path/ProfileActivationFilePathInterpolator.java
##########
@@ -0,0 +1,38 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.profile.ProfileActivationContext;
+import org.codehaus.plexus.interpolation.InterpolationException;
+
+/**
+ * Interpolates path for {@link org.apache.maven.model.ActivationFile}.
+ *
+ * @author Ravil Galeyev
+ */
+public interface ProfileActivationFilePathInterpolator

Review comment:
       why do we need this interface? A class seems like enough. 

##########
File path: maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/FileProfileActivatorTest.java
##########
@@ -0,0 +1,145 @@
+package org.apache.maven.model.profile.activation;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.Activation;
+import org.apache.maven.model.ActivationFile;
+import org.apache.maven.model.Profile;
+import org.apache.maven.model.building.DefaultModelBuilder;
+import org.apache.maven.model.path.DefaultPathTranslator;
+import org.apache.maven.model.path.DefaultProfileActivationFilePathInterpolator;
+import org.apache.maven.model.profile.DefaultProfileActivationContext;
+
+import java.io.File;
+
+/**
+ * Tests {@link FileProfileActivator}.
+ *
+ * @author Ravil Galeyev
+ */
+public class FileProfileActivatorTest extends AbstractProfileActivatorTest<FileProfileActivator>
+{
+    private static final String PATH = "src/test/resources/";
+    private static final String FILE = "file.txt";
+
+    private final DefaultProfileActivationContext context = new DefaultProfileActivationContext();
+
+    public FileProfileActivatorTest()
+    {
+        super( FileProfileActivator.class );
+    }
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        activator.setProfileActivationFilePathInterpolator(
+                new DefaultProfileActivationFilePathInterpolator().setPathTranslator( new DefaultPathTranslator()  ) );
+
+        context.setProjectDirectory( new File( PATH ) );
+
+        File file = new File( PATH + FILE );
+        if ( !file.exists() )
+        {
+            if ( !file.createNewFile() )
+            {
+                throw new RuntimeException( String.format( "Can't create %s", file ) );
+            }
+        }
+    }
+
+
+    public void test_isActive_noFile()
+    {
+        assertActivation( false, newExistsProfile( null ), context );
+        assertActivation( false, newExistsProfile( "someFile.txt" ), context );
+        assertActivation( false, newExistsProfile( "${basedir}/someFile.txt" ), context );
+
+        assertActivation( false, newMissingProfile( null ), context );
+        assertActivation( true, newMissingProfile( "someFile.txt" ), context );
+        assertActivation( true, newMissingProfile( "${basedir}/someFile.txt" ), context );
+    }
+
+    public void test_isActiveExists_fileExists()
+    {
+        assertActivation( true, newExistsProfile( FILE ), context );
+        assertActivation( true, newExistsProfile( "${basedir}" ), context );
+        assertActivation( true, newExistsProfile( "${basedir}/" + FILE ), context );
+
+        assertActivation( false, newMissingProfile( FILE ), context );
+        assertActivation( false, newMissingProfile( "${basedir}" ), context );
+        assertActivation( false, newMissingProfile( "${basedir}/" + FILE ), context );
+    }
+
+    public void test_isActiveExists_leavesFileUnchanged()
+    {
+        Profile profile = newExistsProfile( FILE );
+        assertEquals( profile.getActivation().getFile().getExists(), FILE );
+
+        assertActivation( true, profile, context );
+
+        assertEquals( profile.getActivation().getFile().getExists(), FILE );
+    }
+
+    private Profile newExistsProfile( String filePath )
+    {
+        return newProfile( filePath, true );
+    }
+
+    private Profile newMissingProfile( String filePath )
+    {
+        return newProfile( filePath, false );
+    }
+
+    private Profile newProfile( String filePath, boolean exists )
+    {
+        ActivationFile activationFile = new ActivationFile();
+        if ( exists )
+        {
+            activationFile.setExists( filePath );
+        }
+        else
+        {
+            activationFile.setMissing( filePath );
+        }
+
+        Activation activation = new Activation();
+        activation.setFile( activationFile );
+
+        Profile profile = new Profile();
+        profile.setActivation( activation );
+
+        return profile;
+    }
+
+    @Override
+    protected void tearDown() throws Exception
+    {
+        super.tearDown();

Review comment:
       super.tearDown should be called last.
   
   also this method usually comes right after setUp() before the tests

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/path/ProfileActivationFilePathInterpolator.java
##########
@@ -0,0 +1,102 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.profile.ProfileActivationContext;
+import org.codehaus.plexus.interpolation.AbstractValueSource;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.MapBasedValueSource;
+import org.codehaus.plexus.interpolation.RegexBasedInterpolator;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+
+/**
+ * Interpolates path for {@link org.apache.maven.model.ActivationFile}.
+ *
+ * @author Ravil Galeyev
+ */
+@Named
+@Singleton
+public class ProfileActivationFilePathInterpolator
+{
+
+    @Inject
+    private PathTranslator pathTranslator;
+
+    public ProfileActivationFilePathInterpolator setPathTranslator( PathTranslator pathTranslator )
+    {
+        this.pathTranslator = pathTranslator;
+        return this;
+    }
+
+    /**
+     * Interpolates given {@code path}.
+     *
+     * @return absolute path or {@code null} if the input was {@code null}.

Review comment:
       per oracle guidelines, doc comments that aren't complete sentences don't end with periods

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
##########
@@ -444,6 +461,64 @@ else if ( !parentIds.add( parentData.getId() ) )
         return result;
     }
 
+    private Map<String, Activation> getInterpolatedActivations( Model rawModel,
+                                                                DefaultProfileActivationContext context,
+                                                                DefaultModelProblemCollector problems )
+    {
+        Map<String, Activation> interpolatedActivations = getProfileActivations( rawModel, true );
+        for ( Activation activation : interpolatedActivations.values() )
+        {
+            if ( activation.getFile() != null )
+            {
+                replaceWithInterpolatedValue( activation.getFile(), context, problems );
+            }
+        }
+        return interpolatedActivations;
+    }
+
+    private void replaceWithInterpolatedValue( ActivationFile file, ProfileActivationContext context,
+                                               DefaultModelProblemCollector problems  )
+    {
+        String path;
+        boolean missing;
+
+        if ( StringUtils.isNotEmpty( file.getExists() ) )

Review comment:
       This is weird. Why does getExists return a string?
   
   Also, we shouldn't need StringUtils any more in java 7. 

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
##########
@@ -444,6 +461,64 @@ else if ( !parentIds.add( parentData.getId() ) )
         return result;
     }
 
+    private Map<String, Activation> getInterpolatedActivations( Model rawModel,
+                                                                DefaultProfileActivationContext context,
+                                                                DefaultModelProblemCollector problems )
+    {
+        Map<String, Activation> interpolatedActivations = getProfileActivations( rawModel, true );
+        for ( Activation activation : interpolatedActivations.values() )
+        {
+            if ( activation.getFile() != null )
+            {
+                replaceWithInterpolatedValue( activation.getFile(), context, problems );
+            }
+        }
+        return interpolatedActivations;
+    }
+
+    private void replaceWithInterpolatedValue( ActivationFile file, ProfileActivationContext context,
+                                               DefaultModelProblemCollector problems  )
+    {
+        String path;
+        boolean missing;
+
+        if ( StringUtils.isNotEmpty( file.getExists() ) )
+        {
+            path = file.getExists();
+            missing = false;
+        }
+        else if ( StringUtils.isNotEmpty( file.getMissing() ) )
+        {
+            path = file.getMissing();
+            missing = true;
+        }
+        else
+        {
+            return;
+        }
+
+        try
+        {
+            path = profileActivationFilePathInterpolator.interpolate( path, context );
+        }
+        catch ( InterpolationException e )
+        {
+            problems.add( new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE ).setMessage(
+                    "Failed to interpolate file location " + path + ": " + e.getMessage() ).setLocation(
+                    file.getLocation( missing ? "missing" : "exists" ) ).setException( e ) );
+            return;
+        }
+
+        if ( missing )

Review comment:
       This can be moved into the try block

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
##########
@@ -444,6 +461,64 @@ else if ( !parentIds.add( parentData.getId() ) )
         return result;
     }
 
+    private Map<String, Activation> getInterpolatedActivations( Model rawModel,
+                                                                DefaultProfileActivationContext context,
+                                                                DefaultModelProblemCollector problems )
+    {
+        Map<String, Activation> interpolatedActivations = getProfileActivations( rawModel, true );
+        for ( Activation activation : interpolatedActivations.values() )
+        {
+            if ( activation.getFile() != null )
+            {
+                replaceWithInterpolatedValue( activation.getFile(), context, problems );
+            }
+        }
+        return interpolatedActivations;
+    }
+
+    private void replaceWithInterpolatedValue( ActivationFile file, ProfileActivationContext context,
+                                               DefaultModelProblemCollector problems  )
+    {
+        String path;
+        boolean missing;
+
+        if ( StringUtils.isNotEmpty( file.getExists() ) )
+        {
+            path = file.getExists();
+            missing = false;
+        }
+        else if ( StringUtils.isNotEmpty( file.getMissing() ) )
+        {
+            path = file.getMissing();
+            missing = true;
+        }
+        else
+        {
+            return;
+        }
+
+        try
+        {
+            path = profileActivationFilePathInterpolator.interpolate( path, context );

Review comment:
       try to avoid reusing local variables as it's hard to follow

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/path/ProfileActivationFilePathInterpolator.java
##########
@@ -0,0 +1,102 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.profile.ProfileActivationContext;
+import org.codehaus.plexus.interpolation.AbstractValueSource;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.MapBasedValueSource;
+import org.codehaus.plexus.interpolation.RegexBasedInterpolator;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+
+/**
+ * Interpolates path for {@link org.apache.maven.model.ActivationFile}.

Review comment:
       I don't know what "interpolates" means in this context.

##########
File path: maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/FileProfileActivatorTest.java
##########
@@ -0,0 +1,144 @@
+package org.apache.maven.model.profile.activation;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.Activation;
+import org.apache.maven.model.ActivationFile;
+import org.apache.maven.model.Profile;
+import org.apache.maven.model.path.DefaultPathTranslator;
+import org.apache.maven.model.path.ProfileActivationFilePathInterpolator;
+import org.apache.maven.model.profile.DefaultProfileActivationContext;
+
+import java.io.File;
+
+/**
+ * Tests {@link FileProfileActivator}.
+ *
+ * @author Ravil Galeyev
+ */
+public class FileProfileActivatorTest extends AbstractProfileActivatorTest<FileProfileActivator>
+{
+    private static final String PATH = "src/test/resources/";
+    private static final String FILE = "file.txt";
+
+    private final DefaultProfileActivationContext context = new DefaultProfileActivationContext();
+
+    public FileProfileActivatorTest()
+    {
+        super( FileProfileActivator.class );
+    }
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        activator.setProfileActivationFilePathInterpolator(
+                new ProfileActivationFilePathInterpolator().setPathTranslator( new DefaultPathTranslator() ) );
+
+        context.setProjectDirectory( new File( PATH ) );
+
+        File file = new File( PATH + FILE );
+        if ( !file.exists() )
+        {
+            if ( !file.createNewFile() )
+            {
+                throw new RuntimeException( String.format( "Can't create %s", file ) );

Review comment:
       String.format doesn't add anything here. String concatenation is simpler and clearer.
   Possibly an IOException or simply fail. 

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/path/ProfileActivationFilePathInterpolator.java
##########
@@ -0,0 +1,102 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.profile.ProfileActivationContext;
+import org.codehaus.plexus.interpolation.AbstractValueSource;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.MapBasedValueSource;
+import org.codehaus.plexus.interpolation.RegexBasedInterpolator;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+
+/**
+ * Interpolates path for {@link org.apache.maven.model.ActivationFile}.
+ *
+ * @author Ravil Galeyev
+ */
+@Named
+@Singleton
+public class ProfileActivationFilePathInterpolator
+{
+
+    @Inject
+    private PathTranslator pathTranslator;
+
+    public ProfileActivationFilePathInterpolator setPathTranslator( PathTranslator pathTranslator )
+    {
+        this.pathTranslator = pathTranslator;
+        return this;
+    }
+
+    /**
+     * Interpolates given {@code path}.
+     *
+     * @return absolute path or {@code null} if the input was {@code null}.
+     */
+    public String interpolate( String path, ProfileActivationContext context ) throws InterpolationException
+    {
+        if ( path == null )
+        {
+            return null;
+        }
+
+        RegexBasedInterpolator interpolator = new RegexBasedInterpolator();
+
+        final File basedir = context.getProjectDirectory();
+
+        if ( basedir != null )
+        {
+            interpolator.addValueSource( new AbstractValueSource( false )
+            {
+                @Override
+                public Object getValue( String expression )
+                {
+                    /*
+                     * NOTE: We intentionally only support ${basedir} and not ${project.basedir} as the latter form
+                     * would suggest that other project.* expressions can be used which is however beyond the design.

Review comment:
       delete "however" and "NOTE:"

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/path/ProfileActivationFilePathInterpolator.java
##########
@@ -0,0 +1,102 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.profile.ProfileActivationContext;
+import org.codehaus.plexus.interpolation.AbstractValueSource;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.MapBasedValueSource;
+import org.codehaus.plexus.interpolation.RegexBasedInterpolator;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+
+/**
+ * Interpolates path for {@link org.apache.maven.model.ActivationFile}.
+ *
+ * @author Ravil Galeyev
+ */
+@Named
+@Singleton
+public class ProfileActivationFilePathInterpolator
+{
+
+    @Inject
+    private PathTranslator pathTranslator;
+
+    public ProfileActivationFilePathInterpolator setPathTranslator( PathTranslator pathTranslator )
+    {
+        this.pathTranslator = pathTranslator;
+        return this;
+    }
+
+    /**
+     * Interpolates given {@code path}.
+     *
+     * @return absolute path or {@code null} if the input was {@code null}.
+     */
+    public String interpolate( String path, ProfileActivationContext context ) throws InterpolationException
+    {
+        if ( path == null )
+        {
+            return null;
+        }
+
+        RegexBasedInterpolator interpolator = new RegexBasedInterpolator();
+
+        final File basedir = context.getProjectDirectory();
+
+        if ( basedir != null )
+        {
+            interpolator.addValueSource( new AbstractValueSource( false )
+            {
+                @Override
+                public Object getValue( String expression )
+                {
+                    /*
+                     * NOTE: We intentionally only support ${basedir} and not ${project.basedir} as the latter form
+                     * would suggest that other project.* expressions can be used which is however beyond the design.
+                     */
+                    if ( "basedir".equals( expression ) )
+                    {
+                        return basedir.getAbsolutePath();
+                    }
+                    return null;
+                }
+            } );
+        }
+        else if ( path.contains( "${basedir}" ) )
+        {
+            return null;
+        }
+
+        interpolator.addValueSource( new MapBasedValueSource( context.getProjectProperties() ) );
+
+        interpolator.addValueSource( new MapBasedValueSource( context.getUserProperties() ) );
+
+        interpolator.addValueSource( new MapBasedValueSource( context.getSystemProperties() ) );
+
+        path = interpolator.interpolate( path, "" );

Review comment:
       try to avoid reusing local variables

##########
File path: maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderFactoryTest.java
##########
@@ -56,4 +59,29 @@ public void testCompleteWiring()
         assertEquals( "  1.5  ", conf.getChild( "target" ).getValue() );
     }
 
+    public void test_pom_changes() throws Exception
+    {
+        ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
+        assertNotNull( builder );
+        File pom = getPom( "simple" );
+
+        String originalExists = readPom( pom ).getProfiles().get( 1 ).getActivation().getFile().getExists();
+
+        DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+        request.setProcessPlugins( true );
+        request.setPomFile( pom );
+        ModelBuildingResult result = builder.build( request );
+        String resultedExists = result.getRawModel().getProfiles().get( 1 ).getActivation().getFile().getExists();
+
+        assertEquals( originalExists, resultedExists );
+        assertTrue( result.getEffectiveModel().getProfiles().get( 1 ).getActivation().getFile().getExists()
+                .contains( "src/test/resources/poms/factory/" ) );
+    }
+
+    private static Model readPom( File file ) throws Exception
+    {
+        MavenXpp3Reader reader = new MavenXpp3Reader();
+
+        return reader.read( new FileReader( file ) );

Review comment:
       don't use FileReader; it doesn't handle encodings properly

##########
File path: maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/FileProfileActivatorTest.java
##########
@@ -0,0 +1,144 @@
+package org.apache.maven.model.profile.activation;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.Activation;
+import org.apache.maven.model.ActivationFile;
+import org.apache.maven.model.Profile;
+import org.apache.maven.model.path.DefaultPathTranslator;
+import org.apache.maven.model.path.ProfileActivationFilePathInterpolator;
+import org.apache.maven.model.profile.DefaultProfileActivationContext;
+
+import java.io.File;
+
+/**
+ * Tests {@link FileProfileActivator}.
+ *
+ * @author Ravil Galeyev
+ */
+public class FileProfileActivatorTest extends AbstractProfileActivatorTest<FileProfileActivator>
+{
+    private static final String PATH = "src/test/resources/";
+    private static final String FILE = "file.txt";
+
+    private final DefaultProfileActivationContext context = new DefaultProfileActivationContext();
+
+    public FileProfileActivatorTest()
+    {
+        super( FileProfileActivator.class );
+    }
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        activator.setProfileActivationFilePathInterpolator(
+                new ProfileActivationFilePathInterpolator().setPathTranslator( new DefaultPathTranslator() ) );
+
+        context.setProjectDirectory( new File( PATH ) );
+
+        File file = new File( PATH + FILE );
+        if ( !file.exists() )
+        {
+            if ( !file.createNewFile() )
+            {
+                throw new RuntimeException( String.format( "Can't create %s", file ) );
+            }
+        }
+    }
+
+
+    public void test_isActive_noFile()
+    {
+        assertActivation( false, newExistsProfile( null ), context );
+        assertActivation( false, newExistsProfile( "someFile.txt" ), context );
+        assertActivation( false, newExistsProfile( "${basedir}/someFile.txt" ), context );
+
+        assertActivation( false, newMissingProfile( null ), context );
+        assertActivation( true, newMissingProfile( "someFile.txt" ), context );
+        assertActivation( true, newMissingProfile( "${basedir}/someFile.txt" ), context );
+    }
+
+    public void test_isActiveExists_fileExists()
+    {
+        assertActivation( true, newExistsProfile( FILE ), context );
+        assertActivation( true, newExistsProfile( "${basedir}" ), context );
+        assertActivation( true, newExistsProfile( "${basedir}/" + FILE ), context );
+
+        assertActivation( false, newMissingProfile( FILE ), context );
+        assertActivation( false, newMissingProfile( "${basedir}" ), context );
+        assertActivation( false, newMissingProfile( "${basedir}/" + FILE ), context );
+    }
+
+    public void test_isActiveExists_leavesFileUnchanged()
+    {
+        Profile profile = newExistsProfile( FILE );
+        assertEquals( profile.getActivation().getFile().getExists(), FILE );
+
+        assertActivation( true, profile, context );
+
+        assertEquals( profile.getActivation().getFile().getExists(), FILE );
+    }
+
+    private Profile newExistsProfile( String filePath )
+    {
+        return newProfile( filePath, true );
+    }
+
+    private Profile newMissingProfile( String filePath )
+    {
+        return newProfile( filePath, false );
+    }
+
+    private Profile newProfile( String filePath, boolean exists )
+    {
+        ActivationFile activationFile = new ActivationFile();
+        if ( exists )
+        {
+            activationFile.setExists( filePath );
+        }
+        else
+        {
+            activationFile.setMissing( filePath );
+        }
+
+        Activation activation = new Activation();
+        activation.setFile( activationFile );
+
+        Profile profile = new Profile();
+        profile.setActivation( activation );
+
+        return profile;
+    }
+
+    @Override
+    protected void tearDown() throws Exception
+    {
+        File file = new File( PATH + FILE );
+        if ( file.exists() )
+        {
+            if ( !file.delete() )
+            {
+                throw new RuntimeException( String.format( "Can't delete %s", file ) );

Review comment:
       not sure you need to fail here, but if you do an IOException matches the context

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
##########
@@ -444,6 +461,64 @@ else if ( !parentIds.add( parentData.getId() ) )
         return result;
     }
 
+    private Map<String, Activation> getInterpolatedActivations( Model rawModel,
+                                                                DefaultProfileActivationContext context,
+                                                                DefaultModelProblemCollector problems )
+    {
+        Map<String, Activation> interpolatedActivations = getProfileActivations( rawModel, true );
+        for ( Activation activation : interpolatedActivations.values() )
+        {
+            if ( activation.getFile() != null )
+            {
+                replaceWithInterpolatedValue( activation.getFile(), context, problems );
+            }
+        }
+        return interpolatedActivations;
+    }
+
+    private void replaceWithInterpolatedValue( ActivationFile file, ProfileActivationContext context,
+                                               DefaultModelProblemCollector problems  )
+    {
+        String path;
+        boolean missing;
+
+        if ( StringUtils.isNotEmpty( file.getExists() ) )

Review comment:
       We shouldn't be using this class at all. Prefer org.apache.commons.lang3.StringUtils though even that is overkill here IMHO
   
   




----------------------------------------------------------------
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:
users@infra.apache.org