Skip to content

Commit a1eb8dc

Browse files
cyfung1031CodFrm
andauthored
🐛 修复 packages/filesystem 中认证恢复与删除幂等问题 (#1375)
* 修复 `packages/filesystem` 中认证恢复与删除幂等问题 * test(filesystem): 修复 fetch stub 未恢复导致的测试隔离问题 - auth.test.ts / onedrive.test.ts 增加 afterEach 恢复原 fetch, 避免 stubGlobal("fetch") 泄漏到其它测试套件。 - 不使用 vi.unstubAllGlobals():会连带移除 setup 中注入的全局 chrome mock,导致后续用例 chrome 未定义。 - onedrive.delete() 移除多余的 return resp;,与 Promise<void> 签名一致。 --------- Co-authored-by: 王一之 <yz@ggnb.top>
1 parent a4a7f44 commit a1eb8dc

10 files changed

Lines changed: 314 additions & 39 deletions

File tree

packages/filesystem/auth.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { AuthVerify } from "./auth";
3+
import { LocalStorageDAO } from "@App/app/repo/localStorage";
4+
5+
describe("AuthVerify", () => {
6+
const localStorageDAO = new LocalStorageDAO();
7+
const key = "netdisk:token:onedrive";
8+
let originalFetch: typeof fetch;
9+
10+
beforeEach(async () => {
11+
vi.clearAllMocks();
12+
await chrome.storage.local.clear();
13+
originalFetch = globalThis.fetch;
14+
});
15+
16+
afterEach(() => {
17+
vi.stubGlobal("fetch", originalFetch);
18+
});
19+
20+
it("expired token refresh network failure should reject, not fallback old token", async () => {
21+
await localStorageDAO.saveValue(key, {
22+
accessToken: "old-access",
23+
refreshToken: "old-refresh",
24+
createtime: Date.now() - 3600000 - 1000,
25+
});
26+
27+
vi.stubGlobal("fetch", vi.fn().mockRejectedValueOnce(new Error("refresh network failed")));
28+
29+
await expect(AuthVerify("onedrive")).rejects.toThrow("refresh network failed");
30+
});
31+
32+
it("non-expired token should return cached token without refresh", async () => {
33+
await localStorageDAO.saveValue(key, {
34+
accessToken: "cached-access",
35+
refreshToken: "cached-refresh",
36+
createtime: Date.now(),
37+
});
38+
39+
const fetchMock = vi.fn();
40+
vi.stubGlobal("fetch", fetchMock);
41+
42+
await expect(AuthVerify("onedrive")).resolves.toBe("cached-access");
43+
expect(fetchMock).not.toHaveBeenCalled();
44+
});
45+
});

packages/filesystem/auth.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ export async function AuthVerify(netDiskType: NetDiskType, invalid?: boolean) {
100100
await localStorageDAO.saveValue(key, token);
101101
}
102102
// token过期或者失效
103-
if (Date.now() >= token.createtime + 3600000 || invalid) {
103+
const expired = Date.now() >= token.createtime + 3600000;
104+
if (expired || invalid) {
104105
// 大于一小时刷新token
105106
try {
106107
const resp = await RefreshToken(netDiskType, token.refreshToken);
@@ -119,9 +120,10 @@ export async function AuthVerify(netDiskType: NetDiskType, invalid?: boolean) {
119120
};
120121
// 更新token
121122
await localStorageDAO.saveValue(key, token);
122-
} catch (_) {
123-
// 报错返回原token
124-
return token.accessToken;
123+
} catch (e) {
124+
// 已过期或已被服务端判定失效的 token 不能继续回退使用
125+
console.warn(e);
126+
throw e;
125127
}
126128
} else {
127129
return token.accessToken;
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import DropboxFileSystem from "./dropbox";
3+
4+
describe("DropboxFileSystem", () => {
5+
beforeEach(() => {
6+
vi.clearAllMocks();
7+
});
8+
9+
it("delete should be idempotent on path not found", async () => {
10+
const fs = new DropboxFileSystem("/", "token");
11+
vi.spyOn(fs, "request").mockRejectedValue(
12+
new Error('Dropbox API Error: 409 - {"error_summary":"path_lookup/not_found/..."}')
13+
);
14+
15+
await expect(fs.delete("missing.txt")).resolves.toBeUndefined();
16+
});
17+
});

packages/filesystem/dropbox/dropbox.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,28 @@ export default class DropboxFileSystem implements FileSystem {
7878
request(url: string, config?: RequestInit, nothen?: boolean) {
7979
config = config || {};
8080
const headers = <Headers>config.headers || new Headers();
81-
headers.append(`Authorization`, `Bearer ${this.accessToken}`);
81+
headers.set(`Authorization`, `Bearer ${this.accessToken}`);
8282
config.headers = headers;
83-
const ret = fetch(url, config);
83+
const doFetch = () => fetch(url, config);
84+
const retryWithFreshToken = async () => {
85+
const token = await AuthVerify("dropbox", true);
86+
this.accessToken = token;
87+
headers.set(`Authorization`, `Bearer ${this.accessToken}`);
88+
return doFetch();
89+
};
8490
if (nothen) {
85-
return <Promise<Response>>ret;
91+
return doFetch().then(async (resp) => {
92+
if (resp.status === 401) {
93+
return retryWithFreshToken();
94+
}
95+
return resp;
96+
});
8697
}
87-
return ret
98+
return doFetch()
8899
.then(async (response) => {
100+
if (response.status === 401) {
101+
response = await retryWithFreshToken();
102+
}
89103
if (!response.ok) {
90104
const errorText = await response.text();
91105
throw new Error(`Dropbox API Error: ${response.status} - ${errorText}`);
@@ -126,13 +140,20 @@ export default class DropboxFileSystem implements FileSystem {
126140
const myHeaders = new Headers();
127141
myHeaders.append("Content-Type", "application/json");
128142

129-
await this.request("https://api.dropboxapi.com/2/files/delete_v2", {
130-
method: "POST",
131-
headers: myHeaders,
132-
body: JSON.stringify({
133-
path: fullPath,
134-
}),
135-
});
143+
try {
144+
await this.request("https://api.dropboxapi.com/2/files/delete_v2", {
145+
method: "POST",
146+
headers: myHeaders,
147+
body: JSON.stringify({
148+
path: fullPath,
149+
}),
150+
});
151+
} catch (e: any) {
152+
if (e.message?.includes("path_lookup/not_found") || e.message?.includes("path/not_found")) {
153+
return;
154+
}
155+
throw e;
156+
}
136157

137158
// 清除相关缓存
138159
this.clearRelatedCache(fullPath);
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import GoogleDriveFileSystem from "./googledrive";
3+
4+
describe("GoogleDriveFileSystem", () => {
5+
beforeEach(() => {
6+
vi.clearAllMocks();
7+
});
8+
9+
it("delete should be idempotent when file id is missing", async () => {
10+
const fs = new GoogleDriveFileSystem("/", "token");
11+
vi.spyOn(fs, "getFileId").mockResolvedValue(null);
12+
const requestSpy = vi.spyOn(fs, "request");
13+
14+
await expect(fs.delete("missing.txt")).resolves.toBeUndefined();
15+
expect(requestSpy).not.toHaveBeenCalled();
16+
});
17+
18+
it("delete should be idempotent on 404 response", async () => {
19+
const fs = new GoogleDriveFileSystem("/", "token");
20+
vi.spyOn(fs, "getFileId").mockResolvedValue("file-1");
21+
vi.spyOn(fs, "request").mockResolvedValue({
22+
status: 404,
23+
text: vi.fn().mockResolvedValue("not found"),
24+
} as unknown as Response);
25+
26+
await expect(fs.delete("missing.txt")).resolves.toBeUndefined();
27+
});
28+
});

packages/filesystem/googledrive/googledrive.ts

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,23 +110,44 @@ export default class GoogleDriveFileSystem implements FileSystem {
110110
request(url: string, config?: RequestInit, nothen?: boolean) {
111111
config = config || {};
112112
const headers = <Headers>config.headers || new Headers();
113-
headers.append(`Authorization`, `Bearer ${this.accessToken}`);
113+
headers.set(`Authorization`, `Bearer ${this.accessToken}`);
114114
config.headers = headers;
115-
const ret = fetch(url, config);
115+
const doFetch = () => fetch(url, config);
116+
const retryWithFreshToken = async () => {
117+
const token = await AuthVerify("googledrive", true);
118+
this.accessToken = token;
119+
headers.set(`Authorization`, `Bearer ${this.accessToken}`);
120+
return doFetch();
121+
};
116122
if (nothen) {
117-
return <Promise<Response>>ret;
123+
return doFetch().then(async (resp) => {
124+
if (resp.status === 401) {
125+
return retryWithFreshToken();
126+
}
127+
return resp;
128+
});
118129
}
119-
return ret
120-
.then((data) => data.json())
130+
return doFetch()
131+
.then(async (resp) => {
132+
if (resp.status === 401) {
133+
resp = await retryWithFreshToken();
134+
}
135+
if (!resp.ok) {
136+
throw new Error(await resp.text());
137+
}
138+
return resp.json();
139+
})
121140
.then(async (data) => {
122141
if (data.error) {
123142
if (data.error.code === 401) {
124143
// Token可能过期,尝试刷新
125-
const token = await AuthVerify("googledrive", true);
126-
this.accessToken = token;
127-
headers.set(`Authorization`, `Bearer ${this.accessToken}`);
128-
return fetch(url, config)
129-
.then((retryData) => retryData.json())
144+
return retryWithFreshToken()
145+
.then(async (retryResp) => {
146+
if (!retryResp.ok) {
147+
throw new Error(await retryResp.text());
148+
}
149+
return retryResp.json();
150+
})
130151
.then((retryData) => {
131152
if (retryData.error) {
132153
throw new Error(JSON.stringify(retryData));
@@ -145,7 +166,7 @@ export default class GoogleDriveFileSystem implements FileSystem {
145166
// 首先,找到要删除的文件或文件夹
146167
const fileId = await this.getFileId(fullPath);
147168
if (!fileId) {
148-
throw new Error(`File or directory not found: ${fullPath}`);
169+
return;
149170
}
150171

151172
// 删除文件或文件夹
@@ -156,6 +177,9 @@ export default class GoogleDriveFileSystem implements FileSystem {
156177
},
157178
true
158179
).then(async (resp) => {
180+
if (resp.status === 404) {
181+
return;
182+
}
159183
if (resp.status !== 204 && resp.status !== 200) {
160184
throw new Error(await resp.text());
161185
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import OneDriveFileSystem from "./onedrive";
3+
import { LocalStorageDAO } from "@App/app/repo/localStorage";
4+
5+
function createMockResponse(options: { ok?: boolean; status?: number; text?: string; json?: any }): Response {
6+
const { ok = true, status = 200, text = "", json = {} } = options;
7+
return {
8+
ok,
9+
status,
10+
text: vi.fn().mockResolvedValue(text),
11+
json: vi.fn().mockResolvedValue(json),
12+
headers: new Headers(),
13+
} as unknown as Response;
14+
}
15+
16+
describe("OneDriveFileSystem", () => {
17+
const localStorageDAO = new LocalStorageDAO();
18+
let originalFetch: typeof fetch;
19+
20+
beforeEach(async () => {
21+
vi.clearAllMocks();
22+
await chrome.storage.local.clear();
23+
originalFetch = globalThis.fetch;
24+
});
25+
26+
afterEach(() => {
27+
vi.stubGlobal("fetch", originalFetch);
28+
});
29+
30+
it("request should return retry result after token refresh", async () => {
31+
await localStorageDAO.saveValue("netdisk:token:onedrive", {
32+
accessToken: "expired-token",
33+
refreshToken: "refresh-token",
34+
createtime: Date.now(),
35+
});
36+
37+
const fs = new OneDriveFileSystem("/", "expired-token");
38+
const fetchMock = vi
39+
.fn()
40+
.mockResolvedValueOnce(
41+
createMockResponse({
42+
ok: true,
43+
status: 200,
44+
json: {
45+
error: {
46+
code: "InvalidAuthenticationToken",
47+
},
48+
},
49+
})
50+
)
51+
.mockResolvedValueOnce({
52+
json: vi.fn().mockResolvedValue({
53+
code: 0,
54+
data: {
55+
token: {
56+
access_token: "fresh-token",
57+
refresh_token: "fresh-refresh-token",
58+
},
59+
},
60+
}),
61+
} as unknown as Response)
62+
.mockResolvedValueOnce(
63+
createMockResponse({
64+
ok: true,
65+
status: 200,
66+
json: {
67+
value: [
68+
{
69+
name: "ok.txt",
70+
size: 1,
71+
eTag: "tag",
72+
createdDateTime: new Date().toISOString(),
73+
lastModifiedDateTime: new Date().toISOString(),
74+
},
75+
],
76+
},
77+
})
78+
);
79+
vi.stubGlobal("fetch", fetchMock);
80+
81+
const data = await fs.request("https://graph.microsoft.com/v1.0/me/drive/special/approot/children");
82+
83+
expect(data.value).toHaveLength(1);
84+
expect(fetchMock).toHaveBeenCalledTimes(3);
85+
});
86+
87+
it("delete should be idempotent on 404", async () => {
88+
const fs = new OneDriveFileSystem("/", "token");
89+
vi.spyOn(fs, "request").mockResolvedValue({
90+
status: 404,
91+
text: vi.fn().mockResolvedValue("not found"),
92+
} as unknown as Response);
93+
94+
await expect(fs.delete("missing.txt")).resolves.toBeUndefined();
95+
});
96+
});

0 commit comments

Comments
 (0)