Skip to content

Commit 4f71bc8

Browse files
authored
fix: upstream inline count now aggregates all format endpoints (#27)
* fix: upstream inline count now aggregates all format endpoints - Move all-format upstream aggregation into shared helper in upstreamChecker.js - Inline indicator and WebView now show consistent counts - Add test coverage for mixed-format aggregation and partial failure caching * fix: harden upstream cache validation and make persistence non-fatal - Require valid object, finite timestamp, and object groupedUpstreams in cache entries - Wrap cache persistence in try/catch so write failures do not break upstream loading
1 parent 3fdcdcb commit 4f71bc8

4 files changed

Lines changed: 667 additions & 268 deletions

File tree

models/repositoryNode.js

Lines changed: 8 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22

33
const vscode = require("vscode");
44
const { CloudsmithAPI } = require("../util/cloudsmithAPI");
5+
const { UpstreamChecker } = require("../util/upstreamChecker");
56
const UpstreamIndicatorNode = require("./upstreamIndicatorNode");
67
const { activeFilters } = require("../util/filterState");
78
const InfoNode = require("./infoNode");
89
const { EntitlementSummaryNode } = require("./entitlementNode");
910

10-
const UPSTREAM_CACHE_TTL_MS = 10 * 60 * 1000; // 10 minutes
11-
1211
class RepositoryNode {
1312
constructor(repo, workspace, context) {
1413
this.context = context;
@@ -17,6 +16,7 @@ class RepositoryNode {
1716
this.name = repo.name;
1817
this.workspace = workspace;
1918
this.storageRegion = repo.storage_region || repo.region || null;
19+
this.upstreamChecker = new UpstreamChecker(context);
2020
}
2121

2222
/** Get the active filter from the module-level singleton Map. */
@@ -160,51 +160,13 @@ class RepositoryNode {
160160
}
161161

162162
/**
163-
* Fetch upstream configs for this repo by inferring formats from loaded packages.
164-
* Results are cached in globalState for 10 minutes.
163+
* Fetch upstream configs for this repo across every supported format.
164+
* Results are cached by UpstreamChecker for 10 minutes.
165165
*
166-
* @param packageNodes Array of PackageNode instances to infer formats from.
167166
* @returns Array of upstream config objects (may be empty).
168167
*/
169-
async getUpstreams(packageNodes) {
170-
const workspace = this.workspace;
171-
const repo = this.slug;
172-
const cacheKey = `cloudsmith-upstreams:${workspace}:${repo}`;
173-
174-
// Check cache
175-
const cached = this.context.globalState.get(cacheKey);
176-
if (cached && (Date.now() - cached.timestamp) < UPSTREAM_CACHE_TTL_MS) {
177-
return cached.upstreams;
178-
}
179-
180-
// Infer unique formats from loaded packages
181-
const formats = new Set();
182-
for (const node of packageNodes) {
183-
const format = node.format || (node.pkgDetails && node.pkgDetails.format);
184-
if (format) {
185-
formats.add(format);
186-
}
187-
}
188-
189-
if (formats.size === 0) {
190-
return [];
191-
}
192-
193-
// Fetch upstream configs for each format in parallel
194-
const cloudsmithAPI = new CloudsmithAPI(this.context);
195-
const promises = Array.from(formats).map(format =>
196-
cloudsmithAPI.getUpstreams(workspace, repo, format)
197-
);
198-
const results = await Promise.all(promises);
199-
const allUpstreams = results.flat();
200-
201-
// Cache the results
202-
this.context.globalState.update(cacheKey, {
203-
timestamp: Date.now(),
204-
upstreams: allUpstreams,
205-
});
206-
207-
return allUpstreams;
168+
async getUpstreams() {
169+
return this.upstreamChecker.getRepositoryUpstreams(this.workspace, this.slug);
208170
}
209171

210172
/**
@@ -229,9 +191,9 @@ class RepositoryNode {
229191

230192
const children = [];
231193

232-
// Fetch upstreams lazily (only when repo is expanded) using loaded packages
194+
// Fetch upstreams lazily only when the repository is expanded.
233195
if (packages.length > 0) {
234-
const upstreams = await this.getUpstreams(packages);
196+
const upstreams = await this.getUpstreams();
235197
if (upstreams.length > 0) {
236198
children.push(new UpstreamIndicatorNode(
237199
upstreams,

test/upstreamChecker.test.js

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
const assert = require("assert");
2+
const { CloudsmithAPI } = require("../util/cloudsmithAPI");
3+
const { CredentialManager } = require("../util/credentialManager");
4+
const {
5+
UpstreamChecker,
6+
SUPPORTED_UPSTREAM_FORMATS,
7+
} = require("../util/upstreamChecker");
8+
9+
suite("UpstreamChecker Test Suite", () => {
10+
let originalMakeRequest;
11+
let originalGetApiKey;
12+
let formatResponses;
13+
let requestCount;
14+
let store;
15+
let context;
16+
17+
setup(() => {
18+
originalMakeRequest = CloudsmithAPI.prototype.makeRequest;
19+
originalGetApiKey = CredentialManager.prototype.getApiKey;
20+
formatResponses = {};
21+
requestCount = 0;
22+
store = new Map();
23+
context = {
24+
globalState: {
25+
get(key) {
26+
return store.get(key);
27+
},
28+
async update(key, value) {
29+
if (value === undefined) {
30+
store.delete(key);
31+
return;
32+
}
33+
store.set(key, value);
34+
},
35+
},
36+
};
37+
38+
CredentialManager.prototype.getApiKey = async () => "test-api-key";
39+
CloudsmithAPI.prototype.makeRequest = async function(endpoint) {
40+
requestCount += 1;
41+
const match = endpoint.match(/upstream\/([^/]+)\/$/);
42+
const format = match ? match[1] : null;
43+
const response = formatResponses[format];
44+
45+
if (response instanceof Error) {
46+
throw response;
47+
}
48+
49+
if (typeof response === "function") {
50+
return response();
51+
}
52+
53+
if (response !== undefined) {
54+
return response;
55+
}
56+
57+
return [];
58+
};
59+
});
60+
61+
teardown(() => {
62+
CloudsmithAPI.prototype.makeRequest = originalMakeRequest;
63+
CredentialManager.prototype.getApiKey = originalGetApiKey;
64+
});
65+
66+
function createCachedEntry(overrides = {}) {
67+
return {
68+
timestamp: Date.now(),
69+
successfulFormats: 1,
70+
groupedUpstreams: {
71+
python: [
72+
{ name: "PyPI", upstream_url: "https://pypi.org/simple/" },
73+
],
74+
},
75+
...overrides,
76+
};
77+
}
78+
79+
async function assertInvalidCachedEntry(entry) {
80+
const checker = new UpstreamChecker(context);
81+
const cacheKey = checker._getRepositoryUpstreamCacheKey("workspace-a", "repo-a");
82+
store.set(cacheKey, entry);
83+
84+
const cachedState = checker._getCachedRepositoryUpstreamState("workspace-a", "repo-a");
85+
86+
await Promise.resolve();
87+
88+
assert.strictEqual(cachedState, null);
89+
assert.strictEqual(store.has(cacheKey), false);
90+
}
91+
92+
test("aggregates repository upstreams across formats and reuses the shared cache", async () => {
93+
formatResponses = {
94+
python: [
95+
{ name: "PyPI", upstream_url: "https://pypi.org/simple/" },
96+
{ name: "Internal mirror", upstream_url: "https://mirror.example/python" },
97+
{ name: "Legacy", upstream_url: "https://legacy.example/python" },
98+
],
99+
npm: [
100+
{ name: "npmjs", upstream_url: "https://registry.npmjs.org/" },
101+
{ name: "Disabled", upstream_url: "https://disabled.example/npm", is_active: false },
102+
],
103+
docker: [
104+
{ name: "Docker Hub", upstream_url: "https://registry-1.docker.io/" },
105+
],
106+
conda: "Response status: 404 - Not Found - ",
107+
};
108+
109+
const checker = new UpstreamChecker(context);
110+
const firstState = await checker.getRepositoryUpstreamState("workspace-a", "repo-a");
111+
112+
assert.strictEqual(requestCount, SUPPORTED_UPSTREAM_FORMATS.length);
113+
assert.strictEqual(firstState.total, 6);
114+
assert.strictEqual(firstState.active, 5);
115+
assert.deepStrictEqual(firstState.failedFormats, []);
116+
assert.strictEqual(firstState.groupedUpstreams.get("python").length, 3);
117+
assert.strictEqual(firstState.groupedUpstreams.get("npm").length, 2);
118+
assert.strictEqual(firstState.groupedUpstreams.get("docker").length, 1);
119+
assert.strictEqual(firstState.groupedUpstreams.get("docker")[0].format, "docker");
120+
assert.strictEqual(store.size, 1);
121+
122+
const secondState = await checker.getRepositoryUpstreamState("workspace-a", "repo-a");
123+
124+
assert.strictEqual(requestCount, SUPPORTED_UPSTREAM_FORMATS.length);
125+
assert.strictEqual(secondState.total, 6);
126+
assert.strictEqual(secondState.active, 5);
127+
assert.strictEqual(secondState.groupedUpstreams.get("python")[0].name, "Internal mirror");
128+
});
129+
130+
test("does not cache partial upstream data when any format fails", async () => {
131+
formatResponses = {
132+
python: [
133+
{ name: "PyPI", upstream_url: "https://pypi.org/simple/" },
134+
],
135+
npm: () => {
136+
throw new Error("Response status: 503 - Service Unavailable - ");
137+
},
138+
};
139+
140+
const checker = new UpstreamChecker(context);
141+
const firstState = await checker.getRepositoryUpstreamState("workspace-a", "repo-a");
142+
143+
assert.ok(firstState.failedFormats.includes("npm"));
144+
assert.strictEqual(firstState.total, 1);
145+
assert.strictEqual(firstState.active, 1);
146+
assert.strictEqual(store.size, 0);
147+
148+
await checker.getRepositoryUpstreamState("workspace-a", "repo-a");
149+
150+
assert.strictEqual(requestCount, SUPPORTED_UPSTREAM_FORMATS.length * 2);
151+
assert.strictEqual(store.size, 0);
152+
});
153+
154+
test("treats missing timestamp as an invalid cached repository upstream state", async () => {
155+
const entry = createCachedEntry();
156+
delete entry.timestamp;
157+
await assertInvalidCachedEntry(entry);
158+
});
159+
160+
test("treats non-number timestamp as an invalid cached repository upstream state", async () => {
161+
await assertInvalidCachedEntry(createCachedEntry({ timestamp: "123" }));
162+
});
163+
164+
test("treats non-finite timestamp as an invalid cached repository upstream state", async () => {
165+
await assertInvalidCachedEntry(createCachedEntry({ timestamp: Number.NaN }));
166+
});
167+
168+
test("treats missing groupedUpstreams as an invalid cached repository upstream state", async () => {
169+
const entry = createCachedEntry();
170+
delete entry.groupedUpstreams;
171+
await assertInvalidCachedEntry(entry);
172+
});
173+
174+
test("treats non-object groupedUpstreams as an invalid cached repository upstream state", async () => {
175+
await assertInvalidCachedEntry(createCachedEntry({ groupedUpstreams: [] }));
176+
});
177+
178+
test("treats expired cached repository upstream state as invalid", () => {
179+
const checker = new UpstreamChecker(context);
180+
const cacheKey = checker._getRepositoryUpstreamCacheKey("workspace-a", "repo-a");
181+
store.set(cacheKey, createCachedEntry({ timestamp: Date.now() - (11 * 60 * 1000) }));
182+
183+
const cachedState = checker._getCachedRepositoryUpstreamState("workspace-a", "repo-a");
184+
185+
assert.strictEqual(cachedState, null);
186+
});
187+
188+
test("accepts a valid cached repository upstream state", () => {
189+
const checker = new UpstreamChecker(context);
190+
const cacheKey = checker._getRepositoryUpstreamCacheKey("workspace-a", "repo-a");
191+
store.set(cacheKey, createCachedEntry({ successfulFormats: 7 }));
192+
193+
const cachedState = checker._getCachedRepositoryUpstreamState("workspace-a", "repo-a");
194+
195+
assert.ok(cachedState);
196+
assert.strictEqual(cachedState.successfulFormats, 7);
197+
assert.strictEqual(cachedState.total, 1);
198+
assert.strictEqual(cachedState.active, 1);
199+
assert.strictEqual(cachedState.groupedUpstreams.get("python").length, 1);
200+
});
201+
202+
test("returns computed upstream state when repository cache persistence fails", async () => {
203+
formatResponses = {
204+
python: [
205+
{ name: "PyPI", upstream_url: "https://pypi.org/simple/" },
206+
],
207+
npm: [
208+
{ name: "npmjs", upstream_url: "https://registry.npmjs.org/" },
209+
],
210+
};
211+
212+
const originalUpdate = context.globalState.update;
213+
const logCalls = [];
214+
215+
context.globalState.update = async () => {
216+
throw new Error("quota exceeded");
217+
};
218+
219+
try {
220+
const checker = new UpstreamChecker(context);
221+
checker._logRepositoryUpstreamCacheError = (...args) => logCalls.push(args);
222+
const state = await checker.getRepositoryUpstreamState("workspace-a", "repo-a");
223+
224+
assert.strictEqual(state.total, 2);
225+
assert.strictEqual(state.active, 2);
226+
assert.deepStrictEqual(state.failedFormats, []);
227+
assert.strictEqual(store.size, 0);
228+
assert.strictEqual(logCalls.length, 1);
229+
assert.deepStrictEqual(logCalls[0].slice(0, 3), [
230+
"persist",
231+
"workspace-a",
232+
"repo-a",
233+
]);
234+
assert.strictEqual(logCalls[0][3].message, "quota exceeded");
235+
} finally {
236+
context.globalState.update = originalUpdate;
237+
}
238+
});
239+
});

0 commit comments

Comments
 (0)