Skip to content

fix(context): контракт addWorkspace(URI) — нормализованный через Absolute.uri#3923

Merged
nixel2007 merged 1 commit into
feature/type-system-v2from
fix/server-context-provider-cleanup
May 21, 2026
Merged

fix(context): контракт addWorkspace(URI) — нормализованный через Absolute.uri#3923
nixel2007 merged 1 commit into
feature/type-system-v2from
fix/server-context-provider-cleanup

Conversation

@nixel2007
Copy link
Copy Markdown
Member

@nixel2007 nixel2007 commented May 21, 2026

Summary

  • AbstractServerContextAwareTest.initServerContext(Path, boolean) — оборачивает configurationRoot.toUri() в Absolute.uri(...) перед addWorkspace. Это центральный путь инициализации тестового workspace'а; callsite-ы передают сюда в том числе не-канонизованные Path (@TempDir от JUnit, относительные Path.of("./...")).
  • AnalyzeProjectOnStartTest.TEST_WORKSPACE_URI инициализируется через Absolute.uri("file:///test-analyze-workspace") — фикспойнт нормализации на JVM-инициализации.
  • ServerContextProvider.addWorkspace(URI, ?) — короткая Javadoc-нота: URI должен быть нормализован через Absolute.uri, иначе clear() / removeWorkspace не найдут запись по ключу.
  • TestUtils.getServerContextForFile — старый throw on >1 contexts восстановлен (отменяет test-only смягчение из 2e19715c93).

Why

Симптом на macOS: Multiple ServerContexts registered. Use getDocumentContextFromFile(path, serverContext) ... валит 7 тест-классов через FAKE_DOCUMENT_URI.

Корневая утечка — на macOS JUnit @TempDir создаёт путь под /var/folders/..., а это симлинк на /private/var/folders/.... Цепочка:

@TempDir Path tempDir            # = /var/folders/.../junit-NNN/  (через симлинк)
initServerContext(tempDir, ...)  # configurationRoot.toUri() = file:///var/folders/.../
addWorkspace(uri)                # contexts.put(non-canonical-uri)
...
provider.clear()                 # removeWorkspace(WF(uri.toString()))
  → Absolute.uri(uri.toString()) # внутри File.getCanonicalFile() резолвит симлинк
                                 # → file:///private/var/folders/.../
  → contexts.remove(canonical)   # промах: stored key != canonical
                                 # workspace переживает cleanup

Дальше leak копится в contexts, и в следующем тест-классе initServerContext(PATH_TO_METADATA) делает contexts.size() ≥ 2. TestUtils.getServerContextForFile ловит это throw'ом.

На Linux то же @TempDir лежит под /tmp/junit-NNN/, /tmp — обычная директория, не симлинк, Absolute.uri round-trip identity, утечки нет.

Фикс — нормализовать URI в одной точке (initServerContext), через которую идут все callsite-ы с произвольным Path. Тогда ключ в contexts всегда фикспойнт Absolute.uri, и clear() стабильно находит и удаляет.

Test plan

  • ./gradlew test -PmaxParallelForks=2 локально (Linux) — все 1664 теста зелёные.
  • Java CI matrix (Ubuntu/macOS/Windows × JDK 21/25) — проверить, что 7 ранее упавших Mac/Win-тестов (ExpressionTreeComparersTest, ExpressionTreeBuildingVisitorTest, ExpressionParseTreeRewriterTest, ModuleReferenceTest, UseDirectiveScannerTest, BlockKeywordMatcherTest, ConfigurationModuleMembersProviderTest) теперь зелёные.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 21, 2026 09:11
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bf0e4c79-2b7c-4a59-9e38-0fb44eb2be4e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/server-context-provider-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and documents the ServerContextProvider.addWorkspace(URI, …) contract by requiring a URI normalized via Absolute.uri(...), adding a runtime check to fail fast when callers violate it. This prevents workspace cleanup/key mismatches across OSes (notably macOS/Windows canonicalization differences) and restores a stricter guard in test utilities to surface leaked workspaces instead of masking them.

Changes:

  • Enforced addWorkspace(URI, …) to only accept Absolute.uri(...)-normalized URIs via a runtime assertion.
  • Updated CLI and test call sites to pass normalized workspace URIs and to use the same normalized URI when setting WorkspaceContextHolder.
  • Restored TestUtils behavior to throw when multiple ServerContext instances are registered (forcing tests to be explicit and helping detect cleanup leaks).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContextProvider.java Documents and enforces the normalized-URI contract for workspace registration to prevent cleanup/remove mismatches.
src/main/java/com/github/_1c_syntax/bsl/languageserver/cli/AnalyzeCommand.java Normalizes workspace URI before registering workspace and before entering WorkspaceContextHolder.
src/main/java/com/github/_1c_syntax/bsl/languageserver/cli/FormatCommand.java Normalizes workspace URI before registering workspace and before entering WorkspaceContextHolder.
src/test/java/com/github/_1c_syntax/bsl/languageserver/util/TestUtils.java Restores strict exception on multiple contexts to catch leaked workspaces; keeps deterministic context selection rules.
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java Updates test workspace registration to use normalized workspace URI.
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputerTest.java Updates test workspace registration to use normalized workspace URI.
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/AbstractServerContextAwareTest.java Normalizes workspace URIs used to create contexts to satisfy the new addWorkspace contract across OSes.
src/test/java/com/github/_1c_syntax/bsl/languageserver/AnalyzeProjectOnStartTest.java Normalizes the synthetic test workspace URI to satisfy the new addWorkspace contract.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

Test Results

2 328 files  ±0  2 328 suites  ±0   1h 20m 10s ⏱️ +18s
1 664 tests ±0  1 664 ✅ ±0  0 💤 ±0  0 ❌ ±0 
9 984 runs  ±0  9 984 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a8522d9. ± Comparison against base commit 2e19715.

♻️ This comment has been updated with latest results.

nixel2007 added a commit that referenced this pull request May 21, 2026
…lute.uri

Источник флака на macOS/Windows: AnalyzeProjectOnStartTest регистрировал
synthetic workspace через URI.create("file:///test-analyze-workspace") —
ключ в contexts ложился КАК ЕСТЬ, а provider.clear() / removeWorkspace
ищут по Absolute.uri(uri.toString()). На macOS getCanonicalFile() в
Absolute.uri может выдать другую форму (несуществующий путь, симлинки),
remove промахивается, workspace переживает cleanup и копится в contexts.
В следующем тест-классе allContexts.size() оказывался ≥2, TestUtils.
getServerContextForFile ронял ExpressionTreeComparersTest и Co.

* TEST_WORKSPACE_URI = Absolute.uri("file:///test-analyze-workspace") —
  фиксирует фикспойнт Absolute.uri на JVM-инициализации, не на каждом
  CI-прогоне.
* В Javadoc ServerContextProvider.addWorkspace(URI) добавил short note
  о контракте — URI должен быть нормализован, иначе clear()/remove не
  попадут в ключ.
* TestUtils.getServerContextForFile — восстановил старый
  "throw on >1 contexts": с фиксом TEST_WORKSPACE_URI workspace'ы реально
  вычищаются между тестами, и этот throw снова работает как защитный
  guard, а не симптом-маскирующий fallback (отменяет PR #3923's
  test-only смягчение из 2e19715).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@nixel2007 nixel2007 force-pushed the fix/server-context-provider-cleanup branch from 7df5a6c to 58dbb8e Compare May 21, 2026 09:44
nixel2007 added a commit that referenced this pull request May 21, 2026
…lute.uri

Источник флака на macOS/Windows: AnalyzeProjectOnStartTest регистрировал
synthetic workspace через URI.create("file:///test-analyze-workspace") —
ключ в contexts ложился КАК ЕСТЬ, а provider.clear() / removeWorkspace
ищут по Absolute.uri(uri.toString()). На macOS getCanonicalFile() в
Absolute.uri может выдать другую форму (несуществующий путь, симлинки),
remove промахивается, workspace переживает cleanup и копится в contexts.
В следующем тест-классе allContexts.size() оказывался ≥2, TestUtils.
getServerContextForFile ронял ExpressionTreeComparersTest и Co.

* TEST_WORKSPACE_URI = Absolute.uri("file:///test-analyze-workspace") —
  фиксирует фикспойнт Absolute.uri на JVM-инициализации, не на каждом
  CI-прогоне.
* В Javadoc ServerContextProvider.addWorkspace(URI) добавил short note
  о контракте — URI должен быть нормализован, иначе clear()/remove не
  попадут в ключ.
* TestUtils.getServerContextForFile — восстановил старый
  "throw on >1 contexts": с фиксом TEST_WORKSPACE_URI workspace'ы реально
  вычищаются между тестами, и этот throw снова работает как защитный
  guard, а не симптом-маскирующий fallback (отменяет PR #3923's
  test-only смягчение из 2e19715).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@nixel2007 nixel2007 force-pushed the fix/server-context-provider-cleanup branch from 58dbb8e to 68a8854 Compare May 21, 2026 09:46
…lute.uri

Источник флака на macOS: тесты с JUnit @tempdir (ConventionalLibraryDiscoveryTest
и др.) передают tempDir прямо в initServerContext(Path, ...). На macOS
@tempdir лежит под /var/folders/..., а это симлинк на /private/var/folders/...
tempDir.toUri() возвращает не-канонизованную форму с /var/folders/...,
addWorkspace кладёт её в contexts как ключ. provider.clear() ходит через
removeWorkspace(WF(uri.toString())) → Absolute.uri(...) внутри резолвит
симлинк → ищет /private/var/folders/... — промах, workspace переживает
cleanup. В следующем тест-классе allContexts.size() оказывается ≥2, и
TestUtils.getServerContextForFile роняет ExpressionTreeComparersTest и Co.

* AbstractServerContextAwareTest.initServerContext(Path, boolean) нормализует
  URI через Absolute.uri(configurationRoot.toUri()) — закрывает кейс с
  @tempdir и любыми другими не-канонизованными Path на входе.
* AnalyzeProjectOnStartTest.TEST_WORKSPACE_URI инициализируется через
  Absolute.uri("file:///test-analyze-workspace") — фикспойнт Absolute.uri
  на JVM-инициализации.
* ServerContextProvider.addWorkspace(URI) Javadoc фиксирует контракт:
  URI должен быть нормализован через Absolute.uri, иначе clear()/remove
  не попадут в ключ.
* TestUtils.getServerContextForFile — восстановил старый "throw on >1
  contexts" (отменяет test-only смягчение из 2e19715).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@nixel2007 nixel2007 force-pushed the fix/server-context-provider-cleanup branch from 68a8854 to a8522d9 Compare May 21, 2026 10:26
@sonarqubecloud
Copy link
Copy Markdown

@nixel2007 nixel2007 merged commit 7232834 into feature/type-system-v2 May 21, 2026
32 of 33 checks passed
@nixel2007 nixel2007 deleted the fix/server-context-provider-cleanup branch May 21, 2026 11:01
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.

2 participants