Skip to content

feat: Optimize extensionQuery - #2161

Open
cstamas wants to merge 1 commit into
eclipse-openvsx:mainfrom
cstamas:ext-query
Open

feat: Optimize extensionQuery#2161
cstamas wants to merge 1 commit into
eclipse-openvsx:mainfrom
cstamas:ext-query

Conversation

@cstamas

@cstamas cstamas commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary:

  • Root cause: LocalVSCodeService.extensionQuery (backing /vscode/gallery/extensionquery) unconditionally fetched every active version — all pre-releases included, unbounded by default — for every matched extension, solely to derive the single "latest version" for display, even when the client's query flags requested no version details at all.
  • Fix: Added a target-platform-aware bulk "latest version per extension" query (ExtensionVersionJooqRepository.findLatest(ids, targetPlatform) / RepositoryService.findLatestVersions(ids, targetPlatform)), mirroring the efficient CROSS APPLY pattern already used by the REST search path (LocalRegistryService). LocalVSCodeService.extensionQuery now: (1) only fetches the full active-version list when a flag actually needs it (FLAG_INCLUDE_VERSIONS/FLAG_INCLUDE_VERSION_PROPERTIES/FLAG_INCLUDE_LATEST_VERSION_ONLY), and (2) always computes the "latest" metadata via the new direct DB query instead of pulling the full history into Java and sorting it.

Summary:
- Root cause found: LocalVSCodeService.extensionQuery (backing /vscode/gallery/extensionquery) unconditionally fetched every active version — all pre-releases included, unbounded by default — for every matched extension, solely to derive the single "latest version" for display, even when the client's query flags requested no version details at all. The code even carried a comment acknowledging "extensionquery is really slow."
- Fix: Added a target-platform-aware bulk "latest version per extension" query (ExtensionVersionJooqRepository.findLatest(ids, targetPlatform) / RepositoryService.findLatestVersions(ids, targetPlatform)), mirroring the efficient CROSS APPLY pattern already used by the REST search path (LocalRegistryService). LocalVSCodeService.extensionQuery now: (1) only fetches the full active-version list when a flag actually needs it (FLAG_INCLUDE_VERSIONS/FLAG_INCLUDE_VERSION_PROPERTIES/FLAG_INCLUDE_LATEST_VERSION_ONLY), and (2) always computes the "latest" metadata via the new direct DB query instead of pulling the full history into Java and sorting it.
- Verified: full ./gradlew test suite passes (had to update mocks in VSCodeAPITest, LocalVSCodeServiceTest, and add coverage in RepositoryServiceSmokeTest for the new repository method).
@cstamas
cstamas requested a review from netomi September 4, 2026 09:07
@cstamas
cstamas marked this pull request as ready for review September 4, 2026 10:53
@netomi
netomi requested a lite review from Copilot September 4, 2026 13:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The new performance-critical branching in extensionQuery should be protected by an explicit unit-test interaction assertion to prevent regressions back to unconditional full-version fetching.

Pull request overview

This PR optimizes the /vscode/gallery/extensionquery path by avoiding an unconditional fetch of all active extension versions (including potentially unbounded pre-releases) when the client’s query flags don’t require per-version details, and by computing “latest version” metadata via a dedicated, target-platform-aware bulk DB query.

Changes:

  • Added a new bulk query path to fetch the latest ExtensionVersion per extension (optionally filtered by target platform).
  • Updated LocalVSCodeService.extensionQuery to only fetch the full active version list when version-related flags require it, while always deriving “latest” via the new repository query.
  • Updated tests to account for the new repository call.
File summaries
File Description
server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java Avoids fetching full active-version history unless needed; uses new bulk latest-version query.
server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java Adds findLatestVersions(ids, targetPlatform) façade method.
server/src/main/java/org/eclipse/openvsx/repositories/ExtensionVersionJooqRepository.java Adds target-platform-aware findLatest(ids, targetPlatform) and routes existing findLatest(ids) through it.
server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java Extends smoke coverage to execute the new overload.
server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java Mocks the new latest-version repository call.
server/src/test/java/org/eclipse/openvsx/adapter/LocalVSCodeServiceTest.java Stubs the new latest-version repository call for extensionQuery tests.
Review details

Suppressed comments (1)

server/src/test/java/org/eclipse/openvsx/adapter/LocalVSCodeServiceTest.java:81

  • This test now stubs the new findLatestVersions(…, targetPlatform) call, but it still doesn’t assert the optimization behavior introduced in LocalVSCodeService.extensionQuery: when no version-related flags are set, findActiveExtensionVersions should not be called (and findLatestVersions should be called instead). Adding interaction verifications here would guard against regressions where the full version list is fetched unconditionally again.
        Mockito.when(repositories.findLatestVersions(any(), any())).thenReturn(List.of(extensionVersion));
        Mockito.when(versions.getLatest(anyList(), anyBoolean())).thenReturn(extensionVersion);

        var result = vsCodeService.extensionQuery(param, 10);
        assertThat(result.results()).hasSize(1);
  • Files reviewed: 6/6 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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