Skip to content
This repository has been archived by the owner on Jan 7, 2020. It is now read-only.

Commit

Permalink
Fixing infinite recursion where two mapped properties reference one a…
Browse files Browse the repository at this point in the history
…nother...switch to using StringSearchInterpolater, which is what Maven uses to deal with the same problem in various places (assembly plugin for instance).
  • Loading branch information
jdcasey committed Aug 9, 2013
1 parent 7fb5a0a commit d70d363
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 67 deletions.
15 changes: 8 additions & 7 deletions src/main/java/com/redhat/rcm/version/mgr/mod/PropertyModder.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,25 @@

package com.redhat.rcm.version.mgr.mod;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Properties;
import java.util.Set;

import org.apache.maven.model.Model;
import org.codehaus.plexus.component.annotations.Component;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.redhat.rcm.version.mgr.session.PropertyMappings;
import com.redhat.rcm.version.mgr.session.VersionManagerSession;
import com.redhat.rcm.version.model.Project;

import java.util.Properties;
import java.util.Set;

@Component( role = ProjectModder.class, hint = "property" )
public class PropertyModder
implements ProjectModder
{
private final Logger logger = LoggerFactory.getLogger( getClass() );

@Override
public String getDescription()
{
return "Change property mappings to use those declared in the supplied BOM file(s).";
Expand All @@ -55,9 +56,9 @@ public boolean inject( final Project project, final VersionManagerSession sessio

for ( final String key : commonKeys )
{
final String value = propertyMappings.getMappedValue( key );
final String value = propertyMappings.getMappedValue( key, session );

if (value != null)
if ( value != null )
{
logger.info( "Replacing " + key + '/' + currentModel.get( key ) + " with: '" + value + "'" );
currentModel.put( key, value );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,21 @@

import java.io.File;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.codehaus.plexus.interpolation.InterpolationException;
import org.codehaus.plexus.interpolation.MapBasedValueSource;
import org.codehaus.plexus.interpolation.StringSearchInterpolator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.redhat.rcm.version.VManException;

public class PropertyMappings
{
private final Logger logger = LoggerFactory.getLogger( getClass() );
Expand All @@ -48,13 +52,14 @@ public PropertyMappings( final Map<String, String> newMappings, final VersionMan
addMappings( null, newMappings, session );
}

PropertyMappings addBomPropertyMappings( final File bom, Properties properties, final Map<String, String> newMappings )
PropertyMappings addBomPropertyMappings( final File bom, final Properties properties,
final Map<String, String> newMappings )
{
addMappings( properties, newMappings, session );
return this;
}

public String getMappedValue( final String key )
public String getMappedValue( final String key, final VersionManagerSession session )
{
final String raw = mappings.get( key );

Expand All @@ -63,54 +68,23 @@ public String getMappedValue( final String key )
return null;
}

final StringBuffer buffer = new StringBuffer( raw );
final Set<String> missing = new HashSet<String>();
boolean changed = false;

do
final StringSearchInterpolator interpolator = new StringSearchInterpolator( "@", "@" );
interpolator.addValueSource( new MapBasedValueSource( mappings ) );
try
{
changed = false;
final Pattern pattern = Pattern.compile( EXPRESSION_PATTERN );
final Matcher matcher = pattern.matcher( buffer.toString() );

if ( matcher.find() )
{
buffer.setLength( 0 );
matcher.reset();
}
else
{
return buffer.toString();
}

while ( matcher.find() )
{
final String k = matcher.group( 1 );
if ( missing.contains( k ) )
{
continue;
}

final String value = mappings.get( k );
if ( value != null )
{
matcher.appendReplacement( buffer, value );
changed = true;
}
else
{
missing.add( k );
}
}

matcher.appendTail( buffer );
return interpolator.interpolate( raw );
}
catch ( final InterpolationException e )
{
logger.error( "Invalid expression: '%s'. Reason: %s", e, raw, e.getMessage() );
session.addError( new VManException( "Invalid expression: '%s'. Reason: %s", e, raw, e.getMessage() ) );
}
while ( changed );

return buffer.toString();
return null;
}

private void addMappings( Properties properties, final Map<String, String> newMappings, final VersionManagerSession session )
private void addMappings( final Properties properties, final Map<String, String> newMappings,
final VersionManagerSession session )
{
final Pattern pattern = Pattern.compile( EXPRESSION_PATTERN );

Expand All @@ -125,8 +99,8 @@ private void addMappings( Properties properties, final Map<String, String> newMa
{
final String k = matcher.group( 1 );
if ( ( !mappings.containsKey( k ) && !newMappings.containsKey( k ) ) ||
// Its also an expression if the property exists in the global properties map.
(properties != null && properties.containsKey( k ) ) )
// Its also an expression if the property exists in the global properties map.
( properties != null && properties.containsKey( k ) ) )
{
expressions.put( entry.getKey(), matcher.group( 1 ) );
}
Expand Down Expand Up @@ -154,7 +128,7 @@ public Set<String> getKeys()
*/
void updateProjectMap( final Properties properties )
{
Set<Map.Entry<String, String>> contents = expressions.entrySet();
final Set<Map.Entry<String, String>> contents = expressions.entrySet();
for ( final Iterator<Map.Entry<String, String>> i = contents.iterator(); i.hasNext(); )
{
final Map.Entry<String, String> v = i.next();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.junit.Test;


public class PropertyMappingsTest
{

Expand All @@ -20,9 +19,10 @@ public void addMappingsAndRetrieveOneByKey()
mappings.put( "foo", "bar" );
mappings.put( "baz", "thbbbt" );

final PropertyMappings pm = new PropertyMappings( mappings, new SessionBuilder( null ).build() );
final VersionManagerSession session = new SessionBuilder( null ).build();
final PropertyMappings pm = new PropertyMappings( mappings, session );

assertThat( pm.getMappedValue( "foo" ), equalTo( "bar" ) );
assertThat( pm.getMappedValue( "foo", session ), equalTo( "bar" ) );
}

@Test
Expand All @@ -32,9 +32,10 @@ public void addMappingsAndRetrieveOneNullForMissingKey()
mappings.put( "foo", "bar" );
mappings.put( "baz", "thbbbt" );

final PropertyMappings pm = new PropertyMappings( mappings, new SessionBuilder( null ).build() );
final VersionManagerSession session = new SessionBuilder( null ).build();
final PropertyMappings pm = new PropertyMappings( mappings, session );

assertThat( pm.getMappedValue( "blat" ), nullValue() );
assertThat( pm.getMappedValue( "blat", session ), nullValue() );
}

@Test
Expand All @@ -44,9 +45,10 @@ public void addMappingWithExpressionAndRetrieveFullyResolved()
mappings.put( "foo", "@baz@" );
mappings.put( "baz", "thbbbt" );

final PropertyMappings pm = new PropertyMappings( mappings, new SessionBuilder( null ).build() );
final VersionManagerSession session = new SessionBuilder( null ).build();
final PropertyMappings pm = new PropertyMappings( mappings, session );

assertThat( pm.getMappedValue( "foo" ), equalTo( "thbbbt" ) );
assertThat( pm.getMappedValue( "foo", session ), equalTo( "thbbbt" ) );
}

@Test
Expand All @@ -56,9 +58,10 @@ public void addMappingWithEmbeddedExpressionAndRetrieveFullyResolved()
mappings.put( "foo", "blat, @baz@" );
mappings.put( "baz", "thbbbt" );

final PropertyMappings pm = new PropertyMappings( mappings, new SessionBuilder( null ).build() );
final VersionManagerSession session = new SessionBuilder( null ).build();
final PropertyMappings pm = new PropertyMappings( mappings, session );

assertThat( pm.getMappedValue( "foo" ), equalTo( "blat, thbbbt" ) );
assertThat( pm.getMappedValue( "foo", session ), equalTo( "blat, thbbbt" ) );
}

@Test
Expand All @@ -68,9 +71,23 @@ public void addMappingWithTwoCopiesOfEmbeddedExpressionAndOneMissingExpression_R
mappings.put( "foo", "blat, @baz@ >@missing@< @baz@" );
mappings.put( "baz", "thbbbt" );

final PropertyMappings pm = new PropertyMappings( mappings, new SessionBuilder( null ).build() );
final VersionManagerSession session = new SessionBuilder( null ).build();
final PropertyMappings pm = new PropertyMappings( mappings, session );

assertThat( pm.getMappedValue( "foo", session ), equalTo( "blat, thbbbt >@missing@< thbbbt" ) );
}

@Test
public void addMappingWithExpressionCycle_RetrieveNull()
{
final Map<String, String> mappings = new HashMap<String, String>();
mappings.put( "foo", "@baz@" );
mappings.put( "baz", "@foo@" );

final VersionManagerSession session = new SessionBuilder( null ).build();
final PropertyMappings pm = new PropertyMappings( mappings, session );

assertThat( pm.getMappedValue( "foo" ), equalTo( "blat, thbbbt >@missing@< thbbbt" ) );
assertThat( pm.getMappedValue( "foo", session ), nullValue() );
}

}

0 comments on commit d70d363

Please sign in to comment.