Skip to content

Commit 8171831

Browse files
belingueresbelingueres
committed
Fix codehaus-plexus#16: StringSearchInterpolator does not cache answers.
- Added tests exposing the issue. - Fixed code. - Added missing @OverRide annotations.
1 parent e2fc61a commit 8171831

File tree

2 files changed

+119
-1
lines changed

2 files changed

+119
-1
lines changed

‎src/main/java/org/codehaus/plexus/interpolation/StringSearchInterpolator.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public StringSearchInterpolator( String startExpr, String endExpr )
6161
/**
6262
* {@inheritDoc}
6363
*/
64+
@Override
6465
public void addValueSource( ValueSource valueSource )
6566
{
6667
valueSources.add( valueSource );
@@ -69,6 +70,7 @@ public void addValueSource( ValueSource valueSource )
6970
/**
7071
* {@inheritDoc}
7172
*/
73+
@Override
7274
public void removeValuesSource( ValueSource valueSource )
7375
{
7476
valueSources.remove( valueSource );
@@ -77,6 +79,7 @@ public void removeValuesSource( ValueSource valueSource )
7779
/**
7880
* {@inheritDoc}
7981
*/
82+
@Override
8083
public void addPostProcessor( InterpolationPostProcessor postProcessor )
8184
{
8285
postProcessors.add( postProcessor );
@@ -85,23 +88,27 @@ public void addPostProcessor( InterpolationPostProcessor postProcessor )
8588
/**
8689
* {@inheritDoc}
8790
*/
91+
@Override
8892
public void removePostProcessor( InterpolationPostProcessor postProcessor )
8993
{
9094
postProcessors.remove( postProcessor );
9195
}
9296

97+
@Override
9398
public String interpolate( String input, String thisPrefixPattern )
9499
throws InterpolationException
95100
{
96101
return interpolate( input, new SimpleRecursionInterceptor() );
97102
}
98103

104+
@Override
99105
public String interpolate( String input, String thisPrefixPattern, RecursionInterceptor recursionInterceptor )
100106
throws InterpolationException
101107
{
102108
return interpolate( input, recursionInterceptor );
103109
}
104110

111+
@Override
105112
public String interpolate( String input )
106113
throws InterpolationException
107114
{
@@ -114,6 +121,7 @@ public String interpolate( String input )
114121
*
115122
* TODO: Ensure unresolvable expressions don't trigger infinite recursion.
116123
*/
124+
@Override
117125
public String interpolate( String input, RecursionInterceptor recursionInterceptor )
118126
throws InterpolationException
119127
{
@@ -186,7 +194,7 @@ private String interpolate( String input, RecursionInterceptor recursionIntercep
186194
recursionInterceptor.expressionResolutionStarted( realExpr );
187195
try
188196
{
189-
Object value = existingAnswers.get( realExpr );
197+
Object value = getExistingAnswer( realExpr );
190198
Object bestAnswer = null;
191199

192200
for ( ValueSource valueSource : valueSources )
@@ -235,6 +243,11 @@ private String interpolate( String input, RecursionInterceptor recursionIntercep
235243
// behaviour
236244
result.append( String.valueOf( value ) );
237245
resolved = true;
246+
247+
if (cacheAnswers)
248+
{
249+
existingAnswers.put( realExpr, value );
250+
}
238251
}
239252
else
240253
{
@@ -279,6 +292,7 @@ else if ( endIdx < input.length() )
279292
* @return a {@link List} that may be interspersed with {@link String} and
280293
* {@link Throwable} instances.
281294
*/
295+
@Override
282296
public List getFeedback()
283297
{
284298
List<?> messages = new ArrayList();
@@ -297,6 +311,7 @@ public List getFeedback()
297311
/**
298312
* Clear the feedback messages from previous interpolate(..) calls.
299313
*/
314+
@Override
300315
public void clearFeedback()
301316
{
302317
for ( ValueSource vs : valueSources )
@@ -305,16 +320,19 @@ public void clearFeedback()
305320
}
306321
}
307322

323+
@Override
308324
public boolean isCacheAnswers()
309325
{
310326
return cacheAnswers;
311327
}
312328

329+
@Override
313330
public void setCacheAnswers( boolean cacheAnswers )
314331
{
315332
this.cacheAnswers = cacheAnswers;
316333
}
317334

335+
@Override
318336
public void clearAnswers()
319337
{
320338
existingAnswers.clear();
@@ -330,4 +348,14 @@ public void setEscapeString( String escapeString )
330348
this.escapeString = escapeString;
331349
}
332350

351+
/**
352+
* For testing purposes only. Not part of the public API.
353+
* @param key the key of a possible existing answer.
354+
* @return the associated interpolated object, or null if there is none.
355+
*/
356+
protected Object getExistingAnswer(String key)
357+
{
358+
return existingAnswers.get( key );
359+
}
360+
333361
}

‎src/test/java/org/codehaus/plexus/interpolation/StringSearchInterpolatorTest.java

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public class StringSearchInterpolatorTest
3333
extends TestCase
3434
{
3535

36+
@Override
3637
@Before
3738
public void setUp()
3839
{
@@ -188,6 +189,7 @@ public void testShouldResolveByEnvar()
188189
{
189190
OperatingSystemUtils.setEnvVarSource( new OperatingSystemUtils.EnvVarSource()
190191
{
192+
@Override
191193
public Map<String, String> getEnvMap()
192194
{
193195
HashMap<String,String> map = new HashMap<String,String>();
@@ -218,6 +220,7 @@ public void testUsePostProcessor_DoesNotChangeValue()
218220

219221
rbi.addPostProcessor( new InterpolationPostProcessor()
220222
{
223+
@Override
221224
public Object execute( String expression, Object value )
222225
{
223226
return null;
@@ -242,6 +245,7 @@ public void testUsePostProcessor_ChangesValue()
242245

243246
rbi.addPostProcessor( new InterpolationPostProcessor()
244247
{
248+
@Override
245249
public Object execute( String expression, Object value )
246250
{
247251
return value + "2";
@@ -408,6 +412,7 @@ public void testInterruptedInterpolate()
408412
final boolean[] error = new boolean[] { false };
409413
interpolator.addValueSource( new ValueSource()
410414
{
415+
@Override
411416
public Object getValue( String expression ) {
412417
if ( expression.equals( "key" ) )
413418
{
@@ -422,10 +427,12 @@ public Object getValue( String expression ) {
422427
return null;
423428
}
424429
}
430+
@Override
425431
public List getFeedback()
426432
{
427433
return Collections.EMPTY_LIST;
428434
}
435+
@Override
429436
public void clearFeedback()
430437
{
431438
}
@@ -445,6 +452,89 @@ public void clearFeedback()
445452
assertEquals( "should not believe there is a cycle here", "-val-", interpolator.interpolate( "-${key}-", recursionInterceptor ) );
446453
}
447454

455+
public void testCacheAnswersTrue()
456+
throws InterpolationException
457+
{
458+
Properties p = new Properties();
459+
p.setProperty( "key", "value" );
460+
461+
class CountingStringSearchInterpolator
462+
extends StringSearchInterpolator
463+
{
464+
private int existingCallCount;
465+
466+
@Override
467+
protected Object getExistingAnswer( String key )
468+
{
469+
Object value = super.getExistingAnswer( key );
470+
if ( value != null )
471+
{
472+
++existingCallCount;
473+
}
474+
return value;
475+
}
476+
477+
public int getExistingCallCount()
478+
{
479+
return existingCallCount;
480+
}
481+
}
482+
483+
CountingStringSearchInterpolator interpolator = new CountingStringSearchInterpolator();
484+
interpolator.setCacheAnswers( true );
485+
interpolator.addValueSource( new PropertiesBasedValueSource( p ) );
486+
487+
String result = interpolator.interpolate( "${key}-${key}-${key}-${key}" );
488+
489+
assertEquals( "value-value-value-value", result );
490+
// first value is interpolated and saved, then the 3 next answers came from existing answer Map
491+
assertEquals( 3, interpolator.getExistingCallCount() );
492+
493+
// answers are preserved between calls:
494+
result = interpolator.interpolate( "${key}-${key}-${key}-${key}" );
495+
assertEquals( "value-value-value-value", result );
496+
// 3 from the first call to interpolate(), plus 4 from second call
497+
assertEquals( 3 + 4, interpolator.getExistingCallCount() );
498+
}
499+
500+
public void testCacheAnswersFalse()
501+
throws InterpolationException
502+
{
503+
Properties p = new Properties();
504+
p.setProperty( "key", "value" );
505+
506+
class CountingStringSearchInterpolator
507+
extends StringSearchInterpolator
508+
{
509+
private int existingCallCount;
510+
511+
@Override
512+
protected Object getExistingAnswer( String key )
513+
{
514+
Object value = super.getExistingAnswer( key );
515+
if ( value != null )
516+
{
517+
++existingCallCount;
518+
}
519+
return value;
520+
}
521+
522+
public int getExistingCallCount()
523+
{
524+
return existingCallCount;
525+
}
526+
}
527+
528+
CountingStringSearchInterpolator interpolator = new CountingStringSearchInterpolator();
529+
interpolator.addValueSource( new PropertiesBasedValueSource( p ) );
530+
531+
String result = interpolator.interpolate( "${key}-${key}-${key}-${key}" );
532+
533+
assertEquals( "value-value-value-value", result );
534+
// all values are interpolated each time
535+
assertEquals( 0, interpolator.getExistingCallCount() );
536+
}
537+
448538
public String getVar()
449539
{
450540
return "testVar";

0 commit comments

Comments
 (0)