Skip to content

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Oct 19, 2025

Problem

The cacheAnswers setting in both MultiDelimiterStringSearchInterpolator and RegexBasedInterpolator had no effect. While the code attempted to read from the existingAnswers cache, it never populated the cache with resolved values, making the cache optimization completely ineffective.

Solution

Added the missing cache population logic that stores resolved values into existingAnswers when cacheAnswers is enabled. This brings both interpolators in line with the implementation pattern used in StringSearchInterpolator.

Code change applied to both classes:

if (cacheAnswers) {
    existingAnswers.put(realExpr, value);
}

Impact

Users who enable caching via setCacheAnswers(true) will now see significant performance improvements when interpolating strings with repeated expressions:

  • Before fix: Cache had no effect - expressions were re-evaluated on every interpolate() call
  • After fix: First occurrence is evaluated and cached; subsequent calls use the cached value

Example improvement for MultiDelimiterStringSearchInterpolator:

interpolator.setCacheAnswers(true);
// Input: "${key}-${key}-${key}-${key}"
// Before: 8 ValueSource lookups (per interpolate call)
// After:  1 ValueSource lookup (first call), 0 lookups (subsequent calls)

Example improvement for RegexBasedInterpolator:

interpolator.setCacheAnswers(true);
// Input: "${key}-${key}-${key}-${key}"
// Before: 1 ValueSource lookup per interpolate() call
// After:  1 ValueSource lookup (first call), 0 lookups (subsequent calls)

Testing

  • Added comprehensive test coverage for both cached and non-cached scenarios in both interpolator classes
  • All 138 tests pass (134 existing + 4 new)
  • CodeQL security scan: no vulnerabilities detected

Files Changed

  • MultiDelimiterStringSearchInterpolator.java: Added cache population (4 lines)
  • MultiDelimiterStringSearchInterpolatorTest.java: Added 2 tests (58 lines)
  • RegexBasedInterpolator.java: Added cache population (4 lines)
  • RegexBasedInterpolatorTest.java: Added 2 tests (69 lines)

Fixes #19

Original prompt

This section details on the original issue you should resolve

<issue_title>MultiDelimiterStringSearchInterpolator does not cache answers</issue_title>
<issue_description>MultiDelimiterStringSearchInterpolator's cacheAnswers has no effect at all, since existingAnswers Map is never populated.</issue_description>

Comments on the Issue (you are @copilot in this section)

@slawekjaranowski similar to: - #20 @slawekjaranowski maybe we can drop at all `cacheAnswers`, as source are in many time a maps in memory why we need build another map

Fixes #19


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

…nswers map when cacheAnswers is true

Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Fix caching issue in MultiDelimiterStringSearchInterpolator Fix MultiDelimiterStringSearchInterpolator cache implementation - populate existingAnswers map Oct 19, 2025
Copilot finished work on behalf of slachiewicz October 19, 2025 13:23
@Copilot Copilot AI requested a review from slachiewicz October 19, 2025 13:23
@slachiewicz
Copy link
Member

slachiewicz commented Oct 19, 2025

@copilot RegexBasedInterpolator also needs similar optimalizaton #20

…cacheAnswers is true

Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com>
Copy link
Author

Copilot AI commented Oct 19, 2025

@copilot RegexBasedInterpolator also needs similar optimalizaton #20

Fixed in commit dadb1e8. Applied the same cache population pattern to RegexBasedInterpolator - added 4 lines to populate existingAnswers when cacheAnswers is true, plus comprehensive test coverage.

@Copilot Copilot AI changed the title Fix MultiDelimiterStringSearchInterpolator cache implementation - populate existingAnswers map Fix cache implementation in MultiDelimiterStringSearchInterpolator and RegexBasedInterpolator Oct 19, 2025
Copilot finished work on behalf of slachiewicz October 19, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MultiDelimiterStringSearchInterpolator does not cache answers

2 participants