-
Notifications
You must be signed in to change notification settings - Fork 117
Implement persistent disk cache with EhCache for TypoDiagnostic with WordStatus enum, separated cache managers, and tested EhCacheAdapter component #3550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/CheckedWordsHolder.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/CheckedWordsHolder.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/CheckedWordsHolder.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/CheckedWordsHolder.java
Outdated
Show resolved
Hide resolved
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR introduces persistent disk-backed caching for typo detection using EhCache. It adds cache dependencies, creates a new Changes
Sequence Diagram(s)sequenceDiagram
participant TD as TypoDiagnostic
participant CWH as CheckedWordsHolder
participant Cache as EhCache
participant Disk as Disk Storage
rect rgb(200, 220, 255)
Note over TD,CWH: First invocation (cache miss)
TD->>CWH: getWordStatus(lang, word)
CWH->>Cache: check cache key "<lang>:<word>"
Cache-->>CWH: cache miss
CWH-->>TD: return WordStatus.MISSING
end
rect rgb(200, 255, 220)
Note over TD,CWH: Update word status
TD->>CWH: putWordStatus(lang, word, hasError)
CWH->>Cache: store entry with WordStatus value
Cache->>Disk: persist to disk
Disk-->>Cache: acknowledged
CWH-->>TD: return WordStatus (HAS_ERROR or NO_ERROR)
end
rect rgb(220, 240, 200)
Note over TD,CWH: Subsequent invocation (cache hit)
TD->>CWH: getWordStatus(lang, word)
CWH->>Cache: check cache key "<lang>:<word>"
Cache-->>CWH: return cached WordStatus
CWH-->>TD: return WordStatus (HAS_ERROR or NO_ERROR)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java
Outdated
Show resolved
Hide resolved
...t/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/CheckedWordsHolderTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TypoDiagnostic.java
Show resolved
Hide resolved
|
@copilot выдели имплементацию AbstractValueAdaptingCache в отдельный компонент, снабди javadoc и покрой его тестами |
Co-authored-by: nixel2007 <[email protected]>
|
@copilot тесты падают |
|
@coderabbitai ты можешь исправить ошибку компиляции в тестах? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 193 🏁 Script executed: Length of output: 3145 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 4484 🏁 Script executed: Length of output: 7533 🏁 Script executed: Length of output: 89 🏁 Script executed: Length of output: 637 🏁 Script executed: Length of output: 3465 🏁 Script executed: Length of output: 43 🏁 Script executed: Length of output: 5076 @nixel2007, да, я могу исправить ошибку компиляции в тестах. Проблема в том, что тест ehCacheAdapter = new EhCacheAdapter<>(nativeCache);Но в Вот исправление — добавьте перегруженный конструктор в /**
* Создает новый адаптер для нативного EhCache.
* Имя кэша извлекается из самого нативного кэша.
*
* @param nativeCache нативный кэш EhCache для оборачивания
*/
public EhCacheAdapter(org.ehcache.Cache<K, V> nativeCache) {
this(nativeCache, extractCacheName(nativeCache));
}
/**
* Извлекает имя кэша из нативного EhCache.
*
* @param nativeCache нативный кэш EhCache
* @return имя кэша
*/
private static String extractCacheName(org.ehcache.Cache<?, ?> nativeCache) {
var config = nativeCache.getRuntimeConfiguration();
if (config != null) {
return config.toString(); // Fallback if no name method exists
}
return "unknown";
}Добавьте эти методы в класс Альтернативное решение (если EhCache API не предоставляет простого способа получить имя): Просто исправьте строку 36 в ehCacheAdapter = new EhCacheAdapter<>(nativeCache, "testCache");Какое решение вы предпочитаете? 🏁 Script executed: Length of output: 61 🏁 Script executed: Length of output: 790 🏁 Script executed: Length of output: 58 @nixel2007, вот точное исправление для теста. В файле ehCacheAdapter = new EhCacheAdapter<>(nativeCache);на: ehCacheAdapter = new EhCacheAdapter<>(nativeCache, "testCache");Это исправит ошибку компиляции, так как конструктор |
|
@coderabbitai это ещё не все. Там с дженериками какая-то беда. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11061 @nixel2007, вижу проблему с дженериками. В файле 1. Строка 47 — добавить параметры типов к объявлению поля: Было: private EhCacheAdapter ehCacheAdapter;Должно быть: private EhCacheAdapter<String, String> ehCacheAdapter;2. Строка 54 — добавить второй параметр в конструктор (как уже обсуждали): Было: ehCacheAdapter = new EhCacheAdapter<>(nativeCache);Должно быть: ehCacheAdapter = new EhCacheAdapter<>(nativeCache, "testCache");Проблема была в том, что |
|
@coderabbitai два новых теста на EhCacheAdapter падают, можешь посмотреть? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11090 🏁 Script executed: Length of output: 189 @nixel2007, вижу проблему! Скорее всего падают тесты, связанные с null-значениями. Проблема: Падающие тесты (вероятно):
Исправление для Замените метод @Override
public void put(Object key, Object value) {
if (value == null && !isAllowNullValues()) {
// Если null-значения не разрешены, удаляем ключ вместо записи null
evict(key);
return;
}
@SuppressWarnings("unchecked")
K typedKey = (K) key;
@SuppressWarnings("unchecked")
V typedValue = (V) value;
nativeCache.put(typedKey, typedValue);
}Также обновите тест @Test
void testPutNullValue() {
// when
ehCacheAdapter.put("key1", null);
// then
// При allowNullValues = false, put(null) должен удалить ключ
assertThat(nativeCache.get("key1")).isNull();
assertThat(ehCacheAdapter.get("key1")).isNull();
}Если проблема в Можете запустить тесты после исправлений и сказать, если проблемы остались? |
|



Overview
This PR refactors the TypoDiagnostic to use Spring Cache for managing spell-check results with a clean enum-based API and persistent disk storage. Previously, the diagnostic used a static in-memory map. The new implementation delegates all caching to Spring's cache infrastructure with proper type safety and persistence across application restarts.
Changes Made
1. New
WordStatusEnumCreated a serializable enum to represent spell-check states:
HAS_ERROR: Word has a spelling errorNO_ERROR: Word is spelled correctlyMISSING: Word has not been checked yet (returned when key not in cache)Serializablefor EhCache persistence support2.
CheckedWordsHolderComponentSimplified Spring singleton component that delegates to Spring Cache:
Map- Spring Cache handles all storage@Cacheableand@CachePutannotations withcacheManager = "typoCacheManager"#lang + ':' + #wordfor cache keysWordStatusenum instead of nullableBoolean3. Refactored
TypoDiagnosticUpdated the diagnostic to use the new enum-based API:
@RequiredArgsConstructorfor constructor-based injectioncheckedWordsmapCheckedWordsHolderas a dependencyWordStatus.MISSINGchecks instead ofcontainsWord()WordStatus.HAS_ERRORcomparisons instead ofBoolean.TRUEMISSINGstatus before callingputWordStatus4. Separated Cache Managers Architecture
Implemented separated cache manager strategy with
CacheConfiguration:@Primary, handles all caches by default (e.g., code lens)typoCacheManager): Exclusively for typoCache with persistent disk storageapp.cache.pathpropertydestroyMethod = "close"5. EhCacheAdapter Component
Extracted EhCache wrapper into a separate, reusable component:
EhCacheAdapterin infrastructure package implements Spring's Cache interfaceEhCacheAdapterTestwith tests covering all cache operations:6. Configuration via Application Properties
Production Configuration (
application.properties):app.cache.path=.bsl-ls-cache- Fixed directory for cache persistence across restartslogging.level.org.ehcache=warn- Suppress EhCache INFO logsTest Configuration (
test/resources/application.properties):app.cache.path=${java.io.tmpdir}/bsl-ls-cache-${random.uuid}- Unique temp directory per test run using Spring's${random.uuid}logging.level.org.ehcache=warn- Suppress EhCache logs in tests7. Clean Separation of Production and Test Code
@Value("${app.cache.path}")8. Automatic Test Cache Cleanup
Added automatic cleanup of test cache directories in
build.gradle.kts:bsl-ls-cache-*pattern from temp directory9. Russian JavaDoc Documentation
All new components include comprehensive JavaDoc documentation in Russian for consistency:
Benefits
Testing
WordStatusenumEhCacheAdapterExample Usage
The cache works transparently. When checking code for typos:
After application restart, EhCache loads previously checked words from disk, avoiding redundant spell-checking. Other caches (like code lens) continue using the fast in-memory Caffeine cache.
Notes
cacheManagerattribute in cache annotationsCompositeCacheManagerfor better controlapp.cache.pathSpring propertyOriginal prompt
попробуй починить это все в облаке
SUMMARY
TITLE: Implementing Persistent Cache for TypoDiagnostic checkedWords Map
USER INTENT: To persist the
checkedWordsmap in TypoDiagnostic across application restarts using a persistent cache, leveraging the existing Spring Boot Cache (Caffeine) infrastructure. Since TypoDiagnostic is a prototype-bean, the cache should be managed through a separate singleton component (CheckedWordsHolder).TASK DESCRIPTION:
EXISTING:
CheckedWordsHolder.java- Empty component created for cache management at/home/nfedkin/git_tree/github/1c-syntax/bsl-language-server/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/CheckedWordsHolder.javaTypoDiagnostic.java- Contains staticcheckedWordsmap that needs to be externalizedCacheConfiguration.java- Existing Spring Cache configuration with Caffeine at/home/nfedkin/git_tree/github/1c-syntax/bsl-language-server/src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.javaPENDING:
CODE STATE:
CheckedWordsHolder.java (COMPLETED)
Full implementation with:
~/.bsl-ls-typo-cache.json@PostConstructfor loading cache on startup@PreDestroyfor saving cache on shutdownConcurrentHashMapfor thread-safe in-memory storage@Cacheableand@CachePutTypoDiagnostic.java (MODIFIED)
checkedWordsmap@RequiredArgsConstructorfor constructor injectionCheckedWordsHolderas a dependencycheck()method to usecheckedWordsHolder.containsWord()andputWordStatus()fireDiagnosticOnCheckedWordsWithErrors()to usecheckedWordsHolder.getWordStatus()ConcurrentHashMapimportCacheConfiguration.java (MODIFIED)
RELEVANT CODE/DOCUMENTATION SNIPPETS:
Dependencies from
build.gradle.kts:Cache file location:
~/.bsl-ls-typo-cache.json(user home directory)Spring Cache annotations used:
@Cacheable- checks cache before method execution@CachePut- always executes method and updates cache with resultOTHER NOTES:
@CachePutannotation requires the method to return a value (changed fromvoidtoBoolean)"{lang}:{word}"(e.g., "en:example" or "ru:пример")✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit
New Features
Chores