Skip to content

Commit 86b7979

Browse files
committed
Remove setPackages
1 parent b7633ab commit 86b7979

9 files changed

Lines changed: 76 additions & 120 deletions

File tree

api/src/main.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -682,18 +682,6 @@ export interface PackageManager {
682682
*/
683683
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
684684

685-
/**
686-
* Updates the cached packages for the specified environment and fires a change event.
687-
* @param environment - The Python environment whose packages changed.
688-
* @param packages - The new list of packages.
689-
* @param changes - The list of changes describing what was added or removed.
690-
*/
691-
setPackages(
692-
environment: PythonEnvironment,
693-
packages: Package[],
694-
changes: { kind: PackageChangeKind; pkg: Package }[],
695-
): void;
696-
697685
/**
698686
* Event that is fired when packages change.
699687
*/

examples/sample1/src/api.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -616,18 +616,6 @@ export interface PackageManager {
616616
*/
617617
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
618618

619-
/**
620-
* Updates the cached packages for the specified environment and fires a change event.
621-
* @param environment - The Python environment whose packages changed.
622-
* @param packages - The new list of packages.
623-
* @param changes - The list of changes describing what was added or removed.
624-
*/
625-
setPackages(
626-
environment: PythonEnvironment,
627-
packages: Package[],
628-
changes: { kind: PackageChangeKind; pkg: Package }[],
629-
): void;
630-
631619
/**
632620
* Event that is fired when packages change.
633621
*/

src/api.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -676,18 +676,6 @@ export interface PackageManager {
676676
*/
677677
getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined>;
678678

679-
/**
680-
* Updates the cached packages for the specified environment and fires a change event.
681-
* @param environment - The Python environment whose packages changed.
682-
* @param packages - The new list of packages.
683-
* @param changes - The list of changes describing what was added or removed.
684-
*/
685-
setPackages(
686-
environment: PythonEnvironment,
687-
packages: Package[],
688-
changes: { kind: PackageChangeKind; pkg: Package }[],
689-
): void;
690-
691679
/**
692680
* Event that is fired when packages change.
693681
*/

src/internal.api.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -372,14 +372,6 @@ export class InternalPackageManager implements PackageManager {
372372
return this.manager.getPackages(environment, options);
373373
}
374374

375-
setPackages(
376-
environment: PythonEnvironment,
377-
packages: Package[],
378-
changes: { kind: PackageChangeKind; pkg: Package }[],
379-
): void {
380-
this.manager.setPackages(environment, packages, changes);
381-
}
382-
383375
onDidChangePackages(handler: (e: DidChangePackagesEventArgs) => void): Disposable {
384376
return this.manager.onDidChangePackages ? this.manager.onDidChangePackages(handler) : new Disposable(() => {});
385377
}

src/managers/builtin/pipPackageManager.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
GetPackagesOptions,
1515
IconPath,
1616
Package,
17-
PackageChangeKind,
1817
PackageManagementOptions,
1918
PackageManager,
2019
PythonEnvironment,
@@ -77,7 +76,14 @@ export class PipPackageManager implements PackageManager, Disposable {
7776
async (_progress, token) => {
7877
try {
7978
await managePackages(environment, manageOptions, this, token);
80-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id));
79+
await updatePackagesAndNotify(
80+
this,
81+
environment,
82+
this.packages.get(environment.envId.id),
83+
(changes) => {
84+
this._onDidChangePackages.fire({ environment, manager: this, changes });
85+
},
86+
);
8187
} catch (e) {
8288
if (e instanceof CancellationError) {
8389
throw e;
@@ -102,7 +108,14 @@ export class PipPackageManager implements PackageManager, Disposable {
102108
title: 'Refreshing packages',
103109
},
104110
async () => {
105-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id));
111+
await updatePackagesAndNotify(
112+
this,
113+
environment,
114+
this.packages.get(environment.envId.id),
115+
(changes) => {
116+
this._onDidChangePackages.fire({ environment, manager: this, changes });
117+
},
118+
);
106119
},
107120
);
108121
}
@@ -121,15 +134,4 @@ export class PipPackageManager implements PackageManager, Disposable {
121134
this._onDidChangePackages.dispose();
122135
this.packages.clear();
123136
}
124-
125-
setPackages(
126-
environment: PythonEnvironment,
127-
packages: Package[],
128-
changes: { kind: PackageChangeKind; pkg: Package }[],
129-
): void {
130-
this.packages.set(environment.envId.id, packages);
131-
if (changes.length > 0) {
132-
this._onDidChangePackages.fire({ environment, manager: this, changes });
133-
}
134-
}
135137
}

src/managers/common/packageChanges.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33

44
import { Package, PackageChangeKind, PackageManager, PythonEnvironment } from '../../api';
55

6+
/**
7+
* Callback invoked with the computed changes when at least one change is detected.
8+
*/
9+
export type PackageChangesCallback = (changes: { kind: PackageChangeKind; pkg: Package }[]) => void;
10+
611
/**
712
* Computes the list of package changes between a before and after snapshot.
813
* @param before - The previous list of packages.
@@ -40,9 +45,12 @@ export function getPackageChanges(before: Package[], after: Package[]): { kind:
4045
export async function updatePackagesAndNotify(
4146
packageManager: PackageManager,
4247
environment: PythonEnvironment,
43-
before?: Package[],
48+
before: Package[] | undefined,
49+
onChanges: PackageChangesCallback,
4450
): Promise<void> {
4551
const after = (await packageManager.getPackages(environment, { skipCache: true })) ?? [];
4652
const changes = getPackageChanges(before ?? [], after);
47-
packageManager.setPackages(environment, after, changes);
53+
if (changes.length > 0) {
54+
onChanges(changes);
55+
}
4856
}

src/managers/conda/condaPackageManager.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
GetPackagesOptions,
1313
IconPath,
1414
Package,
15-
PackageChangeKind,
1615
PackageManagementOptions,
1716
PackageManager,
1817
PythonEnvironment,
@@ -74,7 +73,14 @@ export class CondaPackageManager implements PackageManager, Disposable {
7473
async (_progress, token) => {
7574
try {
7675
await managePackages(environment, manageOptions, token, this.log);
77-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id));
76+
await updatePackagesAndNotify(
77+
this,
78+
environment,
79+
this.packages.get(environment.envId.id),
80+
(changes) => {
81+
this._onDidChangePackages.fire({ environment, manager: this, changes });
82+
},
83+
);
7884
} catch (e) {
7985
if (e instanceof CancellationError) {
8086
throw e;
@@ -96,7 +102,14 @@ export class CondaPackageManager implements PackageManager, Disposable {
96102
title: CondaStrings.condaRefreshingPackages,
97103
},
98104
async () => {
99-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id));
105+
await updatePackagesAndNotify(
106+
this,
107+
environment,
108+
this.packages.get(environment.envId.id),
109+
(changes) => {
110+
this._onDidChangePackages.fire({ environment, manager: this, changes });
111+
},
112+
);
100113
},
101114
);
102115
}
@@ -141,15 +154,4 @@ export class CondaPackageManager implements PackageManager, Disposable {
141154
this._onDidChangePackages.dispose();
142155
this.packages.clear();
143156
}
144-
145-
setPackages(
146-
environment: PythonEnvironment,
147-
packages: Package[],
148-
changes: { kind: PackageChangeKind; pkg: Package }[],
149-
): void {
150-
this.packages.set(environment.envId.id, packages);
151-
if (changes.length > 0) {
152-
this._onDidChangePackages.fire({ environment, manager: this, changes });
153-
}
154-
}
155157
}

src/managers/poetry/poetryPackageManager.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
GetPackagesOptions,
1818
IconPath,
1919
Package,
20-
PackageChangeKind,
2120
PackageManagementOptions,
2221
PackageManager,
2322
PythonEnvironment,
@@ -84,7 +83,14 @@ export class PoetryPackageManager implements PackageManager, Disposable {
8483
async (_progress, token) => {
8584
try {
8685
await this.runPoetryManage({ install: toInstall, uninstall: toUninstall }, token);
87-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id));
86+
await updatePackagesAndNotify(
87+
this,
88+
environment,
89+
this.packages.get(environment.envId.id),
90+
(changes) => {
91+
this._onDidChangePackages.fire({ environment, manager: this, changes });
92+
},
93+
);
8894
} catch (e) {
8995
if (e instanceof CancellationError) {
9096
throw e;
@@ -110,7 +116,14 @@ export class PoetryPackageManager implements PackageManager, Disposable {
110116
},
111117
async () => {
112118
try {
113-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id));
119+
await updatePackagesAndNotify(
120+
this,
121+
environment,
122+
this.packages.get(environment.envId.id),
123+
(changes) => {
124+
this._onDidChangePackages.fire({ environment, manager: this, changes });
125+
},
126+
);
114127
} catch (error) {
115128
this.log.error(`Failed to refresh packages: ${error}`);
116129
// Show error to user but don't break the UI
@@ -250,17 +263,6 @@ export class PoetryPackageManager implements PackageManager, Disposable {
250263
// Convert to Package objects using the API
251264
return poetryPackages.map((pkg) => this.api.createPackageItem(pkg, environment, this));
252265
}
253-
254-
setPackages(
255-
environment: PythonEnvironment,
256-
packages: Package[],
257-
changes: { kind: PackageChangeKind; pkg: Package }[],
258-
): void {
259-
this.packages.set(environment.envId.id, packages);
260-
if (changes.length > 0) {
261-
this._onDidChangePackages.fire({ environment, manager: this, changes });
262-
}
263-
}
264266
}
265267

266268
export async function runPoetry(

src/test/managers/common/packageChanges.unit.test.ts

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -99,85 +99,71 @@ suite('packageChanges', () => {
9999

100100
suite('updatePackagesAndNotify', () => {
101101
let environment: PythonEnvironment;
102-
let cache: Package[] | undefined;
103102
let getPackagesStub: sinon.SinonStub;
104103
let packageManager: PackageManager;
105104

106105
setup(() => {
107106
environment = {} as PythonEnvironment;
108-
cache = undefined;
109107
getPackagesStub = sinon.stub();
110-
111108
packageManager = {
112109
name: 'test',
113110
manage: sinon.stub(),
114111
refresh: sinon.stub(),
115112
getPackages: getPackagesStub,
116-
setPackages: sinon.stub().callsFake((_env: PythonEnvironment, pkgs: Package[]) => {
117-
cache = pkgs;
118-
}),
119113
} as unknown as PackageManager;
120114
});
121115

122-
test('updates cache and reports adds on first load', async () => {
116+
test('reports adds on first load', async () => {
123117
const fetched = [{ name: 'requests', version: '2.31.0' } as Package];
124118
getPackagesStub.resolves(fetched);
119+
const onChanges = sinon.stub();
125120

126-
await updatePackagesAndNotify(packageManager, environment, cache);
121+
await updatePackagesAndNotify(packageManager, environment, undefined, onChanges);
127122

128-
const setPackages = packageManager.setPackages as sinon.SinonStub;
129-
assert.ok(setPackages.calledOnce);
130-
const [env, pkgs, changes] = setPackages.firstCall.args;
131-
assert.strictEqual(env, environment);
132-
assert.deepStrictEqual(pkgs, fetched);
123+
assert.ok(onChanges.calledOnce);
124+
const [changes] = onChanges.firstCall.args;
133125
assert.strictEqual(changes.length, 1);
134126
assert.strictEqual(changes[0].kind, PackageChangeKind.add);
135-
assert.deepStrictEqual(cache, fetched);
136127
});
137128

138-
test('updates cache with empty changes when nothing changed', async () => {
129+
test('does not fire callback when nothing changed', async () => {
139130
const pkgs = [{ name: 'requests', version: '2.31.0' } as Package];
140-
cache = pkgs;
141131
getPackagesStub.resolves(pkgs);
132+
const onChanges = sinon.stub();
142133

143-
await updatePackagesAndNotify(packageManager, environment, cache);
134+
await updatePackagesAndNotify(packageManager, environment, pkgs, onChanges);
144135

145-
const setPackages = packageManager.setPackages as sinon.SinonStub;
146-
assert.ok(setPackages.calledOnce);
147-
const [, , changes] = setPackages.firstCall.args;
148-
assert.strictEqual(changes.length, 0);
136+
assert.ok(onChanges.notCalled);
149137
});
150138

151139
test('detects removals correctly', async () => {
152140
const before = [
153141
{ name: 'requests', version: '2.31.0' } as Package,
154142
{ name: 'flask', version: '3.0.0' } as Package,
155143
];
156-
cache = before;
157144
const after = [{ name: 'requests', version: '2.31.0' } as Package];
158145
getPackagesStub.resolves(after);
146+
const onChanges = sinon.stub();
159147

160-
await updatePackagesAndNotify(packageManager, environment, cache);
148+
await updatePackagesAndNotify(packageManager, environment, before, onChanges);
161149

162-
const setPackages = packageManager.setPackages as sinon.SinonStub;
163-
assert.ok(setPackages.calledOnce);
164-
const [, pkgs, changes] = setPackages.firstCall.args;
165-
assert.deepStrictEqual(pkgs, after);
150+
assert.ok(onChanges.calledOnce);
151+
const [changes] = onChanges.firstCall.args;
166152
assert.strictEqual(changes.length, 1);
167153
assert.strictEqual(changes[0].kind, PackageChangeKind.remove);
168154
assert.strictEqual(changes[0].pkg.name, 'flask');
169155
});
170156

171157
test('detects mixed adds and removals', async () => {
172-
cache = [{ name: 'flask', version: '3.0.0' } as Package];
158+
const before = [{ name: 'flask', version: '3.0.0' } as Package];
173159
const after = [{ name: 'django', version: '5.0.0' } as Package];
174160
getPackagesStub.resolves(after);
161+
const onChanges = sinon.stub();
175162

176-
await updatePackagesAndNotify(packageManager, environment, cache);
163+
await updatePackagesAndNotify(packageManager, environment, before, onChanges);
177164

178-
const setPackages = packageManager.setPackages as sinon.SinonStub;
179-
assert.ok(setPackages.calledOnce);
180-
const [, , changes] = setPackages.firstCall.args;
165+
assert.ok(onChanges.calledOnce);
166+
const [changes] = onChanges.firstCall.args;
181167
assert.strictEqual(changes.length, 2);
182168
assert.ok(changes.some((c: { kind: PackageChangeKind }) => c.kind === PackageChangeKind.add));
183169
assert.ok(changes.some((c: { kind: PackageChangeKind }) => c.kind === PackageChangeKind.remove));

0 commit comments

Comments
 (0)