diff --git a/api/src/main.ts b/api/src/main.ts index 32cc9ff9..aa381414 100644 --- a/api/src/main.ts +++ b/api/src/main.ts @@ -677,9 +677,10 @@ export interface PackageManager { /** * Retrieves the list of packages for the specified Python environment. * @param environment - The Python environment for which to retrieve packages. + * @param options - Optional settings for package retrieval. * @returns An array of packages, or undefined if the packages could not be retrieved. */ - getPackages(environment: PythonEnvironment): Promise; + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; /** * Event that is fired when packages change. @@ -794,6 +795,17 @@ export interface DidChangePythonProjectsEventArgs { removed: PythonProject[]; } +/** + * Options for retrieving packages from a package manager. + */ +export interface GetPackagesOptions { + /** + * When `true`, bypasses the cache and fetches the latest packages from the underlying tool. + * Defaults to `false`. + */ + skipCache?: boolean; +} + export type PackageManagementOptions = | { /** @@ -1025,9 +1037,10 @@ export interface PythonPackageGetterApi { * Get the list of packages in a Python Environment. * * @param environment The Python Environment for which the list of packages is required. + * @param options Optional settings for package retrieval. * @returns The list of packages in the Python Environment. */ - getPackages(environment: PythonEnvironment): Promise; + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; /** * Event raised when the list of packages in a Python Environment changes. diff --git a/examples/sample1/src/api.ts b/examples/sample1/src/api.ts index 220cb947..b44e79b2 100644 --- a/examples/sample1/src/api.ts +++ b/examples/sample1/src/api.ts @@ -611,9 +611,10 @@ export interface PackageManager { /** * Retrieves the list of packages for the specified Python environment. * @param environment - The Python environment for which to retrieve packages. + * @param options - Optional settings for package retrieval. * @returns An array of packages, or undefined if the packages could not be retrieved. */ - getPackages(environment: PythonEnvironment): Promise; + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; /** * Event that is fired when packages change. @@ -714,6 +715,17 @@ export interface DidChangePythonProjectsEventArgs { removed: PythonProject[]; } +/** + * Options for retrieving packages from a package manager. + */ +export interface GetPackagesOptions { + /** + * When `true`, bypasses the cache and fetches the latest packages from the underlying tool. + * Defaults to `false`. + */ + skipCache?: boolean; +} + /** * Options for package management. */ @@ -915,7 +927,8 @@ export interface PythonProjectEnvironmentApi { } export interface PythonEnvironmentManagerApi - extends PythonEnvironmentManagerRegistrationApi, + extends + PythonEnvironmentManagerRegistrationApi, PythonEnvironmentItemApi, PythonEnvironmentManagementApi, PythonEnvironmentsApi, @@ -949,9 +962,10 @@ export interface PythonPackageGetterApi { * Get the list of packages in a Python Environment. * * @param environment The Python Environment for which the list of packages is required. + * @param options Optional settings for package retrieval. * @returns The list of packages in the Python Environment. */ - getPackages(environment: PythonEnvironment): Promise; + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; /** * Event raised when the list of packages in a Python Environment changes. @@ -984,7 +998,8 @@ export interface PythonPackageManagementApi { } export interface PythonPackageManagerApi - extends PythonPackageManagerRegistrationApi, + extends + PythonPackageManagerRegistrationApi, PythonPackageGetterApi, PythonPackageManagementApi, PythonPackageItemApi {} @@ -1203,10 +1218,7 @@ export interface PythonBackgroundRunApi { } export interface PythonExecutionApi - extends PythonTerminalCreateApi, - PythonTerminalRunApi, - PythonTaskRunApi, - PythonBackgroundRunApi {} + extends PythonTerminalCreateApi, PythonTerminalRunApi, PythonTaskRunApi, PythonBackgroundRunApi {} /** * Event arguments for when the monitored `.env` files or any other sources change. @@ -1255,7 +1267,8 @@ export interface PythonEnvironmentVariablesApi { * The API for interacting with Python environments, package managers, and projects. */ export interface PythonEnvironmentApi - extends PythonEnvironmentManagerApi, + extends + PythonEnvironmentManagerApi, PythonPackageManagerApi, PythonProjectApi, PythonExecutionApi, diff --git a/src/api.ts b/src/api.ts index b641ad3f..b3ab24cb 100644 --- a/src/api.ts +++ b/src/api.ts @@ -671,9 +671,10 @@ export interface PackageManager { /** * Retrieves the list of packages for the specified Python environment. * @param environment - The Python environment for which to retrieve packages. + * @param options - Optional settings for package retrieval. * @returns An array of packages, or undefined if the packages could not be retrieved. */ - getPackages(environment: PythonEnvironment): Promise; + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; /** * Event that is fired when packages change. @@ -788,6 +789,17 @@ export interface DidChangePythonProjectsEventArgs { removed: PythonProject[]; } +/** + * Options for retrieving packages from a package manager. + */ +export interface GetPackagesOptions { + /** + * When `true`, bypasses the cache and fetches the latest packages from the underlying tool. + * Defaults to `false`. + */ + skipCache?: boolean; +} + export type PackageManagementOptions = | { /** @@ -1019,9 +1031,10 @@ export interface PythonPackageGetterApi { * Get the list of packages in a Python Environment. * * @param environment The Python Environment for which the list of packages is required. + * @param options Optional settings for package retrieval. * @returns The list of packages in the Python Environment. */ - getPackages(environment: PythonEnvironment): Promise; + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; /** * Event raised when the list of packages in a Python Environment changes. diff --git a/src/features/pythonApi.ts b/src/features/pythonApi.ts index f071b0f5..b20de34f 100644 --- a/src/features/pythonApi.ts +++ b/src/features/pythonApi.ts @@ -10,6 +10,7 @@ import { EnvironmentManager, GetEnvironmentScope, GetEnvironmentsScope, + GetPackagesOptions, Package, PackageId, PackageInfo, @@ -96,9 +97,9 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi { // *selected* manager's changes propagate (refreshEnvironment checks // getEnvironmentManager(scope) internally). It updates the cache and // fires onDidChangeActiveEnvironment, which the Python API listens to. - this.envManagers.refreshEnvironment(e.uri).catch((err) => - traceError('Failed to refresh environment on change:', err), - ); + this.envManagers + .refreshEnvironment(e.uri) + .catch((err) => traceError('Failed to refresh environment on change:', err)); }); }), ); @@ -257,13 +258,13 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi { } return manager.refresh(context); } - async getPackages(context: PythonEnvironment): Promise { + async getPackages(context: PythonEnvironment, options?: GetPackagesOptions): Promise { await waitForEnvManagerId([context.envId.managerId]); const manager = this.envManagers.getPackageManager(context); if (!manager) { return Promise.resolve(undefined); } - return manager.getPackages(context); + return manager.getPackages(context, options); } onDidChangePackages: Event = this._onDidChangePackages.event; createPackageItem(info: PackageInfo, environment: PythonEnvironment, manager: PackageManager): Package { diff --git a/src/internal.api.ts b/src/internal.api.ts index 3d896c0c..4c80b527 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -9,6 +9,7 @@ import { EnvironmentManager, GetEnvironmentScope, GetEnvironmentsScope, + GetPackagesOptions, IconPath, Package, PackageChangeKind, @@ -367,8 +368,8 @@ export class InternalPackageManager implements PackageManager { return this.manager.refresh(environment); } - getPackages(environment: PythonEnvironment): Promise { - return this.manager.getPackages(environment); + getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { + return this.manager.getPackages(environment, options); } onDidChangePackages(handler: (e: DidChangePackagesEventArgs) => void): Disposable { diff --git a/src/managers/builtin/main.ts b/src/managers/builtin/main.ts index fdda603b..4fff2dae 100644 --- a/src/managers/builtin/main.ts +++ b/src/managers/builtin/main.ts @@ -4,7 +4,7 @@ import { createSimpleDebounce } from '../../common/utils/debounce'; import { createFileSystemWatcher, onDidDeleteFiles } from '../../common/workspace.apis'; import { getPythonApi } from '../../features/pythonApi'; import { NativePythonFinder } from '../common/nativePythonFinder'; -import { PipPackageManager } from './pipManager'; +import { PipPackageManager } from './pipPackageManager'; import { SysPythonManager } from './sysPythonManager'; import { VenvManager } from './venvManager'; @@ -60,10 +60,10 @@ export async function registerSystemPythonFeatures( ); }); const packageWatcher = createFileSystemWatcher( - '**/site-packages/*.dist-info/METADATA', + '**/site-packages/*.dist-info/METADATA', false, // don't ignore create events (pip install) - true, // ignore change events (content changes in METADATA don't affect package list) - false // don't ignore delete events (pip uninstall) + true, // ignore change events (content changes in METADATA don't affect package list) + false, // don't ignore delete events (pip uninstall) ); disposables.push( packageDebouncedRefresh, diff --git a/src/managers/builtin/pipManager.ts b/src/managers/builtin/pipPackageManager.ts similarity index 72% rename from src/managers/builtin/pipManager.ts rename to src/managers/builtin/pipPackageManager.ts index 81d26ea0..1a517adc 100644 --- a/src/managers/builtin/pipManager.ts +++ b/src/managers/builtin/pipPackageManager.ts @@ -11,29 +11,19 @@ import { } from 'vscode'; import { DidChangePackagesEventArgs, + GetPackagesOptions, IconPath, Package, - PackageChangeKind, PackageManagementOptions, PackageManager, PythonEnvironment, PythonEnvironmentApi, } from '../../api'; +import { updatePackagesAndNotify } from '../common/packageChanges'; import { getWorkspacePackagesToInstall } from './pipUtils'; -import { managePackages, refreshPackages } from './utils'; +import { managePackages, refreshPipPackages } from './utils'; import { VenvManager } from './venvManager'; -function getChanges(before: Package[], after: Package[]): { kind: PackageChangeKind; pkg: Package }[] { - const changes: { kind: PackageChangeKind; pkg: Package }[] = []; - before.forEach((pkg) => { - changes.push({ kind: PackageChangeKind.remove, pkg }); - }); - after.forEach((pkg) => { - changes.push({ kind: PackageChangeKind.add, pkg }); - }); - return changes; -} - export class PipPackageManager implements PackageManager, Disposable { private readonly _onDidChangePackages = new EventEmitter(); onDidChangePackages: Event = this._onDidChangePackages.event; @@ -85,11 +75,15 @@ export class PipPackageManager implements PackageManager, Disposable { }, async (_progress, token) => { try { - const before = this.packages.get(environment.envId.id) ?? []; - const after = await managePackages(environment, manageOptions, this.api, this, token); - const changes = getChanges(before, after); - this.packages.set(environment.envId.id, after); - this._onDidChangePackages.fire({ environment, manager: this, changes }); + await managePackages(environment, manageOptions, this, token); + await updatePackagesAndNotify( + this, + environment, + this.packages.get(environment.envId.id), + (changes) => { + this._onDidChangePackages.fire({ environment, manager: this, changes }); + }, + ); } catch (e) { if (e instanceof CancellationError) { throw e; @@ -114,19 +108,19 @@ export class PipPackageManager implements PackageManager, Disposable { title: 'Refreshing packages', }, async () => { - const before = this.packages.get(environment.envId.id) ?? []; - const after = await refreshPackages(environment, this.api, this); - const changes = getChanges(before, after); - this.packages.set(environment.envId.id, after); - if (changes.length > 0) { + await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id), (changes) => { this._onDidChangePackages.fire({ environment, manager: this, changes }); - } + }); }, ); } - async getPackages(environment: PythonEnvironment): Promise { - if (!this.packages.has(environment.envId.id)) { - await this.refresh(environment); + + async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { + if (options?.skipCache || !this.packages.has(environment.envId.id)) { + const data = await refreshPipPackages(environment, this.log); + const packages = (data ?? []).map((pkg) => this.api.createPackageItem(pkg, environment, this)); + this.packages.set(environment.envId.id, packages); + return packages; } return this.packages.get(environment.envId.id); } diff --git a/src/managers/builtin/utils.ts b/src/managers/builtin/utils.ts index 3bab6770..354a1390 100644 --- a/src/managers/builtin/utils.ts +++ b/src/managers/builtin/utils.ts @@ -185,12 +185,12 @@ export async function refreshPythons( const PIP_LIST_TIMEOUT_MS = 30_000; -async function refreshPipPackagesRaw(environment: PythonEnvironment, log?: LogOutputChannel): Promise { +async function execPipList(environment: PythonEnvironment, log?: LogOutputChannel, args?: string[]): Promise { // Use environmentPath directly for consistency with UV environment tracking const useUv = await shouldUseUv(log, environment.environmentPath.fsPath); if (useUv) { return await runUV( - ['pip', 'list', '--python', environment.execInfo.run.executable, '--format=json'], + ['pip', 'list', '--python', environment.execInfo.run.executable, '--format=json', ...(args ?? [])], undefined, log, undefined, @@ -228,11 +228,11 @@ export async function refreshPipPackages( location: ProgressLocation.Notification, }, async () => { - return await refreshPipPackagesRaw(environment, log); + return await execPipList(environment, log); }, ); } else { - data = await refreshPipPackagesRaw(environment, log); + data = await execPipList(environment, log); } return parsePipListJson(data); @@ -243,22 +243,12 @@ export async function refreshPipPackages( } } -export async function refreshPackages( - environment: PythonEnvironment, - api: PythonEnvironmentApi, - manager: PackageManager, -): Promise { - const data = await refreshPipPackages(environment, manager.log); - return (data ?? []).map((pkg) => api.createPackageItem(pkg, environment, manager)); -} - export async function managePackages( environment: PythonEnvironment, options: PackageManagementOptions, - api: PythonEnvironmentApi, manager: PackageManager, token?: CancellationToken, -): Promise { +): Promise { if (environment.version.startsWith('2.')) { throw new Error('Python 2.* is not supported (deprecated)'); } @@ -310,8 +300,6 @@ export async function managePackages( ); } } - - return await refreshPackages(environment, api, manager); } /** diff --git a/src/managers/common/packageChanges.ts b/src/managers/common/packageChanges.ts new file mode 100644 index 00000000..c2afa122 --- /dev/null +++ b/src/managers/common/packageChanges.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Package, PackageChangeKind, PackageManager, PythonEnvironment } from '../../api'; + +/** + * Callback invoked with the computed changes when at least one change is detected. + */ +export type PackageChangesCallback = (changes: { kind: PackageChangeKind; pkg: Package }[]) => void; + +/** + * Computes the list of package changes between a before and after snapshot. + * @param before - The previous list of packages. + * @param after - The new list of packages. + * @returns An array of changes indicating which packages were added or removed. + */ +export function getPackageChanges(before: Package[], after: Package[]): { kind: PackageChangeKind; pkg: Package }[] { + const beforeSet = new Set(before.map(({ name, version }) => `${name}==${version}`)); + const afterSet = new Set(after.map(({ name, version }) => `${name}==${version}`)); + const changes: { kind: PackageChangeKind; pkg: Package }[] = []; + + for (const pkg of after) { + if (!beforeSet.has(`${pkg.name}==${pkg.version}`)) { + changes.push({ kind: PackageChangeKind.add, pkg }); + } + } + for (const pkg of before) { + if (!afterSet.has(`${pkg.name}==${pkg.version}`)) { + changes.push({ kind: PackageChangeKind.remove, pkg }); + } + } + + return changes; +} + +/** + * Fetches the latest packages, computes changes against the current cache, + * and updates the cache. Fires a change event only when there are actual changes. + * + * This function calls {@link PackageManager.getPackages} with `skipCache` to fetch + * the latest snapshot. The caller should pass the previously cached packages + * so changes can be computed against the pre-refresh state. + */ +export async function updatePackagesAndNotify( + packageManager: PackageManager, + environment: PythonEnvironment, + before: Package[] | undefined, + onChanges: PackageChangesCallback, +): Promise { + const after = (await packageManager.getPackages(environment, { skipCache: true })) ?? []; + const changes = getPackageChanges(before ?? [], after); + if (changes.length > 0) { + onChanges(changes); + } +} diff --git a/src/managers/conda/condaPackageManager.ts b/src/managers/conda/condaPackageManager.ts index c012ea91..6a492ffa 100644 --- a/src/managers/conda/condaPackageManager.ts +++ b/src/managers/conda/condaPackageManager.ts @@ -9,9 +9,9 @@ import { } from 'vscode'; import { DidChangePackagesEventArgs, + GetPackagesOptions, IconPath, Package, - PackageChangeKind, PackageManagementOptions, PackageManager, PythonEnvironment, @@ -19,19 +19,10 @@ import { } from '../../api'; import { showErrorMessageWithLogs } from '../../common/errors/utils'; import { CondaStrings } from '../../common/localize'; +import { traceError } from '../../common/logging'; import { withProgress } from '../../common/window.apis'; -import { getCommonCondaPackagesToInstall, managePackages, refreshPackages } from './condaUtils'; - -function getChanges(before: Package[], after: Package[]): { kind: PackageChangeKind; pkg: Package }[] { - const changes: { kind: PackageChangeKind; pkg: Package }[] = []; - before.forEach((pkg) => { - changes.push({ kind: PackageChangeKind.remove, pkg }); - }); - after.forEach((pkg) => { - changes.push({ kind: PackageChangeKind.add, pkg }); - }); - return changes; -} +import { updatePackagesAndNotify } from '../common/packageChanges'; +import { getCommonCondaPackagesToInstall, managePackages, runCondaExecutable } from './condaUtils'; export class CondaPackageManager implements PackageManager, Disposable { private readonly _onDidChangePackages = new EventEmitter(); @@ -39,7 +30,10 @@ export class CondaPackageManager implements PackageManager, Disposable { private packages: Map = new Map(); - constructor(public readonly api: PythonEnvironmentApi, public readonly log: LogOutputChannel) { + constructor( + public readonly api: PythonEnvironmentApi, + public readonly log: LogOutputChannel, + ) { this.name = 'conda'; this.displayName = 'Conda'; this.description = CondaStrings.condaPackageMgr; @@ -78,11 +72,15 @@ export class CondaPackageManager implements PackageManager, Disposable { }, async (_progress, token) => { try { - const before = this.packages.get(environment.envId.id) ?? []; - const after = await managePackages(environment, manageOptions, this.api, this, token, this.log); - const changes = getChanges(before, after); - this.packages.set(environment.envId.id, after); - this._onDidChangePackages.fire({ environment: environment, manager: this, changes }); + await managePackages(environment, manageOptions, token, this.log); + await updatePackagesAndNotify( + this, + environment, + this.packages.get(environment.envId.id), + (changes) => { + this._onDidChangePackages.fire({ environment, manager: this, changes }); + }, + ); } catch (e) { if (e instanceof CancellationError) { throw e; @@ -104,20 +102,45 @@ export class CondaPackageManager implements PackageManager, Disposable { title: CondaStrings.condaRefreshingPackages, }, async () => { - const before = this.packages.get(environment.envId.id) ?? []; - const after = await refreshPackages(environment, this.api, this); - const changes = getChanges(before, after); - this.packages.set(environment.envId.id, after); - if (changes.length > 0) { + await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id), (changes) => { this._onDidChangePackages.fire({ environment, manager: this, changes }); - } + }); }, ); } - async getPackages(environment: PythonEnvironment): Promise { - if (!this.packages.has(environment.envId.id)) { - await this.refresh(environment); + async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { + if (options?.skipCache || !this.packages.has(environment.envId.id)) { + const args = ['list', '-p', environment.environmentPath.fsPath, '--json']; + const data = await runCondaExecutable(args); + + let condaPackages: { name: string; version: string }[]; + try { + condaPackages = JSON.parse(data) as { name: string; version: string }[]; + } catch (e) { + traceError(`Failed to parse conda list JSON output: ${data}`, e); + return []; + } + + const packages: Package[] = []; + for (const condaPkg of condaPackages) { + if (condaPkg.name && condaPkg.version) { + packages.push( + this.api.createPackageItem( + { + name: condaPkg.name, + displayName: condaPkg.name, + version: condaPkg.version, + description: condaPkg.version, + }, + environment, + this, + ), + ); + } + } + this.packages.set(environment.envId.id, packages); + return packages; } return this.packages.get(environment.envId.id); } diff --git a/src/managers/conda/condaUtils.ts b/src/managers/conda/condaUtils.ts index 51b1988e..590284c5 100644 --- a/src/managers/conda/condaUtils.ts +++ b/src/managers/conda/condaUtils.ts @@ -15,9 +15,7 @@ import { import which from 'which'; import { EnvironmentManager, - Package, PackageManagementOptions, - PackageManager, PythonCommandRunConfiguration, PythonEnvironment, PythonEnvironmentApi, @@ -1276,68 +1274,12 @@ export async function deleteCondaEnvironment(environment: PythonEnvironment, log ); } -/** - * JSON structure returned by `conda list --json` - */ -interface CondaPackageJson { - name: string; - version: string; - build_string?: string; - channel?: string; -} - -/** - * Refreshes the list of packages installed in a conda environment. - * Uses `conda list -p --json` for reliable parsing. - * - * @param environment The Python environment to get packages for - * @param api The Python environment API - * @param manager The package manager instance - * @returns Promise resolving to an array of Package objects - */ -export async function refreshPackages( - environment: PythonEnvironment, - api: PythonEnvironmentApi, - manager: PackageManager, -): Promise { - const args = ['list', '-p', environment.environmentPath.fsPath, '--json']; - const data = await runCondaExecutable(args); - - let condaPackages: CondaPackageJson[]; - try { - condaPackages = JSON.parse(data) as CondaPackageJson[]; - } catch (e) { - traceError(`Failed to parse conda list JSON output: ${data}`, e); - return []; - } - - const packages: Package[] = []; - for (const condaPkg of condaPackages) { - if (condaPkg.name && condaPkg.version) { - const pkg = api.createPackageItem( - { - name: condaPkg.name, - displayName: condaPkg.name, - version: condaPkg.version, - description: condaPkg.version, - }, - environment, - manager, - ); - packages.push(pkg); - } - } - return packages; -} - export async function managePackages( environment: PythonEnvironment, options: PackageManagementOptions, - api: PythonEnvironmentApi, - manager: PackageManager, token: CancellationToken, log: LogOutputChannel, -): Promise { +): Promise { if (options.uninstall && options.uninstall.length > 0) { await runCondaExecutable( ['remove', '--prefix', environment.environmentPath.fsPath, '--yes', ...options.uninstall], @@ -1353,7 +1295,6 @@ export async function managePackages( args.push(...options.install); await runCondaExecutable(args, log, token); } - return refreshPackages(environment, api, manager); } async function getCommonPackages(): Promise { diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts index 21d5fb82..0498e225 100644 --- a/src/managers/poetry/poetryPackageManager.ts +++ b/src/managers/poetry/poetryPackageManager.ts @@ -14,9 +14,9 @@ import { import { Disposable } from 'vscode-jsonrpc'; import { DidChangePackagesEventArgs, + GetPackagesOptions, IconPath, Package, - PackageChangeKind, PackageManagementOptions, PackageManager, PythonEnvironment, @@ -24,20 +24,10 @@ import { } from '../../api'; import { spawnProcess } from '../../common/childProcess.apis'; import { showErrorMessage, showInputBox, withProgress } from '../../common/window.apis'; +import { updatePackagesAndNotify } from '../common/packageChanges'; import { PoetryManager } from './poetryManager'; import { getPoetry } from './poetryUtils'; -function getChanges(before: Package[], after: Package[]): { kind: PackageChangeKind; pkg: Package }[] { - const changes: { kind: PackageChangeKind; pkg: Package }[] = []; - before.forEach((pkg) => { - changes.push({ kind: PackageChangeKind.remove, pkg }); - }); - after.forEach((pkg) => { - changes.push({ kind: PackageChangeKind.add, pkg }); - }); - return changes; -} - export class PoetryPackageManager implements PackageManager, Disposable { private readonly _onDidChangePackages = new EventEmitter(); onDidChangePackages: Event = this._onDidChangePackages.event; @@ -92,15 +82,15 @@ export class PoetryPackageManager implements PackageManager, Disposable { }, async (_progress, token) => { try { - const before = this.packages.get(environment.envId.id) ?? []; - const after = await this.managePackages( + await this.runPoetryManage({ install: toInstall, uninstall: toUninstall }, token); + await updatePackagesAndNotify( + this, environment, - { install: toInstall, uninstall: toUninstall }, - token, + this.packages.get(environment.envId.id), + (changes) => { + this._onDidChangePackages.fire({ environment, manager: this, changes }); + }, ); - const changes = getChanges(before, after); - this.packages.set(environment.envId.id, after); - this._onDidChangePackages.fire({ environment, manager: this, changes }); } catch (e) { if (e instanceof CancellationError) { throw e; @@ -126,13 +116,14 @@ export class PoetryPackageManager implements PackageManager, Disposable { }, async () => { try { - const before = this.packages.get(environment.envId.id) ?? []; - const after = await this.refreshPackages(environment); - const changes = getChanges(before, after); - this.packages.set(environment.envId.id, after); - if (changes.length > 0) { - this._onDidChangePackages.fire({ environment, manager: this, changes }); - } + await updatePackagesAndNotify( + this, + environment, + this.packages.get(environment.envId.id), + (changes) => { + this._onDidChangePackages.fire({ environment, manager: this, changes }); + }, + ); } catch (error) { this.log.error(`Failed to refresh packages: ${error}`); // Show error to user but don't break the UI @@ -147,9 +138,11 @@ export class PoetryPackageManager implements PackageManager, Disposable { ); } - async getPackages(environment: PythonEnvironment): Promise { - if (!this.packages.has(environment.envId.id)) { - await this.refresh(environment); + async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { + if (options?.skipCache || !this.packages.has(environment.envId.id)) { + const packages = await this.fetchPackagesFromTool(environment); + this.packages.set(environment.envId.id, packages); + return packages; } return this.packages.get(environment.envId.id); } @@ -159,11 +152,10 @@ export class PoetryPackageManager implements PackageManager, Disposable { this.packages.clear(); } - private async managePackages( - environment: PythonEnvironment, + private async runPoetryManage( options: { install?: string[]; uninstall?: string[] }, token?: CancellationToken, - ): Promise { + ): Promise { const poetry = await getPoetry(); if (!poetry) { throw new Error( @@ -198,12 +190,9 @@ export class PoetryPackageManager implements PackageManager, Disposable { throw err; } } - - // Refresh the packages list after changes - return this.refreshPackages(environment); } - private async refreshPackages(environment: PythonEnvironment): Promise { + private async fetchPackagesFromTool(environment: PythonEnvironment): Promise { const poetry = await getPoetry(); if (!poetry) { throw new Error( diff --git a/src/test/features/packageManager.api.unit.test.ts b/src/test/features/packageManager.api.unit.test.ts index 9f4b98b1..c4f2416a 100644 --- a/src/test/features/packageManager.api.unit.test.ts +++ b/src/test/features/packageManager.api.unit.test.ts @@ -343,7 +343,7 @@ suite('PythonPackageManagerApi Tests', () => { }, ]; packageManager - .setup((pm) => pm.getPackages(environment.object)) + .setup((pm) => pm.getPackages(environment.object, typeMoq.It.isAny())) .returns(() => Promise.resolve(mockPackages)) .verifiable(typeMoq.Times.once()); @@ -360,7 +360,7 @@ suite('PythonPackageManagerApi Tests', () => { test('Should return undefined when no packages found', async () => { // Mock - Package manager returns undefined packageManager - .setup((pm) => pm.getPackages(environment.object)) + .setup((pm) => pm.getPackages(environment.object, typeMoq.It.isAny())) .returns(() => Promise.resolve(undefined)) .verifiable(typeMoq.Times.once()); @@ -375,7 +375,7 @@ suite('PythonPackageManagerApi Tests', () => { test('Should return empty array when environment has no packages', async () => { // Mock - Package manager returns empty array packageManager - .setup((pm) => pm.getPackages(environment.object)) + .setup((pm) => pm.getPackages(environment.object, typeMoq.It.isAny())) .returns(() => Promise.resolve([])) .verifiable(typeMoq.Times.once()); @@ -402,7 +402,7 @@ suite('PythonPackageManagerApi Tests', () => { }, ]; packageManager - .setup((pm) => pm.getPackages(environment.object)) + .setup((pm) => pm.getPackages(environment.object, typeMoq.It.isAny())) .returns(() => Promise.resolve(mockPackages)); // Run @@ -422,7 +422,9 @@ suite('PythonPackageManagerApi Tests', () => { test('Should propagate errors from package manager getPackages method', async () => { // Mock - Package manager throws error const testError = new Error('Failed to get packages'); - packageManager.setup((pm) => pm.getPackages(environment.object)).returns(() => Promise.reject(testError)); + packageManager + .setup((pm) => pm.getPackages(environment.object, typeMoq.It.isAny())) + .returns(() => Promise.reject(testError)); // Run & Assert - Should reject with same error await assert.rejects( diff --git a/src/test/managers/common/packageChanges.unit.test.ts b/src/test/managers/common/packageChanges.unit.test.ts new file mode 100644 index 00000000..81e8235f --- /dev/null +++ b/src/test/managers/common/packageChanges.unit.test.ts @@ -0,0 +1,173 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { Package, PackageChangeKind, PackageManager, PythonEnvironment } from '../../../api'; +import { getPackageChanges, updatePackagesAndNotify } from '../../../managers/common/packageChanges'; + +suite('packageChanges', () => { + teardown(() => { + sinon.restore(); + }); + + suite('getPackageChanges', () => { + test('returns empty array when before and after are identical', () => { + const pkgs = [{ name: 'requests', version: '2.31.0' } as Package]; + + const changes = getPackageChanges(pkgs, pkgs); + + assert.strictEqual(changes.length, 0); + }); + + test('returns empty array when both before and after are empty', () => { + const changes = getPackageChanges([], []); + + assert.strictEqual(changes.length, 0); + }); + + test('returns add changes for new packages', () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'flask', version: '3.0.0' } as Package, + ]; + + const changes = getPackageChanges([], after); + + assert.strictEqual(changes.length, 2); + assert.deepStrictEqual( + changes.map((c) => c.kind), + [PackageChangeKind.add, PackageChangeKind.add], + ); + assert.deepStrictEqual( + changes.map((c) => c.pkg.name), + ['requests', 'flask'], + ); + }); + + test('returns remove changes for removed packages', () => { + const before = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'flask', version: '3.0.0' } as Package, + ]; + + const changes = getPackageChanges(before, []); + + assert.strictEqual(changes.length, 2); + assert.deepStrictEqual( + changes.map((c) => c.kind), + [PackageChangeKind.remove, PackageChangeKind.remove], + ); + }); + + test('detects version upgrade as add and remove', () => { + const before = [{ name: 'requests', version: '2.30.0' } as Package]; + const after = [{ name: 'requests', version: '2.31.0' } as Package]; + + const changes = getPackageChanges(before, after); + + assert.strictEqual(changes.length, 2); + const add = changes.find((c) => c.kind === PackageChangeKind.add); + const remove = changes.find((c) => c.kind === PackageChangeKind.remove); + assert.ok(add); + assert.ok(remove); + assert.strictEqual(add.pkg.version, '2.31.0'); + assert.strictEqual(remove.pkg.version, '2.30.0'); + }); + + test('handles mixed additions and removals', () => { + const before = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'flask', version: '3.0.0' } as Package, + ]; + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'django', version: '5.0.0' } as Package, + ]; + + const changes = getPackageChanges(before, after); + + assert.strictEqual(changes.length, 2); + const add = changes.find((c) => c.kind === PackageChangeKind.add); + const remove = changes.find((c) => c.kind === PackageChangeKind.remove); + assert.ok(add); + assert.ok(remove); + assert.strictEqual(add.pkg.name, 'django'); + assert.strictEqual(remove.pkg.name, 'flask'); + }); + }); + + suite('updatePackagesAndNotify', () => { + let environment: PythonEnvironment; + let getPackagesStub: sinon.SinonStub; + let packageManager: PackageManager; + + setup(() => { + environment = {} as PythonEnvironment; + getPackagesStub = sinon.stub(); + packageManager = { + name: 'test', + manage: sinon.stub(), + refresh: sinon.stub(), + getPackages: getPackagesStub, + } as unknown as PackageManager; + }); + + test('reports adds on first load', async () => { + const fetched = [{ name: 'requests', version: '2.31.0' } as Package]; + getPackagesStub.resolves(fetched); + const onChanges = sinon.stub(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(getPackagesStub.calledOnceWithExactly(environment, sinon.match({ skipCache: true }))); + assert.ok(onChanges.calledOnce); + const [changes] = onChanges.firstCall.args; + assert.strictEqual(changes.length, 1); + assert.strictEqual(changes[0].kind, PackageChangeKind.add); + }); + + test('does not fire callback when nothing changed', async () => { + const pkgs = [{ name: 'requests', version: '2.31.0' } as Package]; + getPackagesStub.resolves(pkgs); + const onChanges = sinon.stub(); + + await updatePackagesAndNotify(packageManager, environment, pkgs, onChanges); + + assert.ok(onChanges.notCalled); + }); + + test('detects removals correctly', async () => { + const before = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'flask', version: '3.0.0' } as Package, + ]; + const after = [{ name: 'requests', version: '2.31.0' } as Package]; + getPackagesStub.resolves(after); + const onChanges = sinon.stub(); + + await updatePackagesAndNotify(packageManager, environment, before, onChanges); + + assert.ok(onChanges.calledOnce); + const [changes] = onChanges.firstCall.args; + assert.strictEqual(changes.length, 1); + assert.strictEqual(changes[0].kind, PackageChangeKind.remove); + assert.strictEqual(changes[0].pkg.name, 'flask'); + }); + + test('detects mixed adds and removals', async () => { + const before = [{ name: 'flask', version: '3.0.0' } as Package]; + const after = [{ name: 'django', version: '5.0.0' } as Package]; + getPackagesStub.resolves(after); + const onChanges = sinon.stub(); + + await updatePackagesAndNotify(packageManager, environment, before, onChanges); + + assert.ok(onChanges.calledOnce); + const [changes] = onChanges.firstCall.args; + assert.strictEqual(changes.length, 2); + assert.ok(changes.some((c: { kind: PackageChangeKind }) => c.kind === PackageChangeKind.add)); + assert.ok(changes.some((c: { kind: PackageChangeKind }) => c.kind === PackageChangeKind.remove)); + }); + }); +});