Skip to content

Commit 3d8b189

Browse files
chmjkbchmjkb
committed
partial review changes
1 parent 32a80a2 commit 3d8b189

5 files changed

Lines changed: 95 additions & 66 deletions

File tree

‎packages/bare-resource-fetcher/src/ResourceFetcher.ts‎

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525

2626
import * as RNFS from '@dr.pogodin/react-native-fs';
27+
import { Platform } from 'react-native';
2728
import { RNEDirectory } from './constants/directories';
2829
import {
2930
ResourceSource,
@@ -71,59 +72,83 @@ class BareResourceFetcherClass extends BaseResourceFetcherClass<ActiveDownload>
7172
}
7273

7374
protected async pause(source: ResourceSource): Promise<void> {
74-
const dl = this.downloads.get(source)!;
75-
if (dl.status === DownloadStatus.PAUSED) {
75+
const downloadHandle = this.downloads.get(source)!;
76+
if (downloadHandle.status === DownloadStatus.PAUSED) {
7677
throw new RnExecutorchError(
7778
RnExecutorchErrorCode.ResourceFetcherAlreadyPaused,
7879
"The file download is currently paused. Can't pause the download of the same file twice."
7980
);
8081
}
81-
if (!dl.task) {
82+
if (Platform.OS === 'android') {
8283
throw new RnExecutorchError(
8384
RnExecutorchErrorCode.ResourceFetcherPlatformNotSupported,
8485
'Pause is not supported on Android. Use cancelFetching and re-fetch instead.'
8586
);
8687
}
87-
dl.status = DownloadStatus.PAUSED;
88-
dl.task.pause();
88+
if (!downloadHandle.task) {
89+
throw new RnExecutorchError(
90+
RnExecutorchErrorCode.ResourceFetcherDownloadFailed,
91+
'Download task is missing. This should not happen on iOS.'
92+
);
93+
}
94+
downloadHandle.status = DownloadStatus.PAUSED;
95+
downloadHandle.task.pause();
8996
}
9097

9198
protected async resume(source: ResourceSource): Promise<void> {
92-
const dl = this.downloads.get(source)!;
93-
if (dl.status === DownloadStatus.ONGOING) {
99+
const downloadHandle = this.downloads.get(source)!;
100+
if (downloadHandle.status === DownloadStatus.ONGOING) {
94101
throw new RnExecutorchError(
95102
RnExecutorchErrorCode.ResourceFetcherAlreadyOngoing,
96103
"The file download is currently ongoing. Can't resume the ongoing download."
97104
);
98105
}
99-
if (!dl.task) {
106+
if (Platform.OS === 'android') {
100107
throw new RnExecutorchError(
101108
RnExecutorchErrorCode.ResourceFetcherPlatformNotSupported,
102109
'Resume is not supported on Android. Use fetch to restart the download.'
103110
);
104111
}
112+
if (!downloadHandle.task) {
113+
throw new RnExecutorchError(
114+
RnExecutorchErrorCode.ResourceFetcherDownloadFailed,
115+
'Download task is missing. This should not happen on iOS.'
116+
);
117+
}
105118
// Set status back to ONGOING before resuming so the .done() callback
106119
// registered in handleRemote knows it's safe to proceed (not paused/canceled).
107-
dl.status = DownloadStatus.ONGOING;
108-
dl.task.resume();
120+
downloadHandle.status = DownloadStatus.ONGOING;
121+
downloadHandle.task.resume();
109122
}
110123

111124
protected async cancel(source: ResourceSource): Promise<void> {
112-
const dl = this.downloads.get(source)!;
113-
114-
if (dl.task) {
115-
// iOS: background downloader cancel
116-
dl.task.stop();
117-
} else if (dl.jobId !== undefined) {
118-
// Android: RNFS cancel + cleanup of partial file
119-
RNFS.stopDownload(dl.jobId);
120-
if (await ResourceFetcherUtils.checkFileExists(dl.cacheFileUri)) {
121-
await RNFS.unlink(dl.cacheFileUri);
125+
const downloadHandle = this.downloads.get(source)!;
126+
127+
if (Platform.OS === 'ios') {
128+
if (!downloadHandle.task) {
129+
throw new RnExecutorchError(
130+
RnExecutorchErrorCode.ResourceFetcherDownloadFailed,
131+
'Download task is missing. This should not happen on iOS.'
132+
);
133+
}
134+
downloadHandle.task.stop();
135+
} else if (Platform.OS === 'android') {
136+
if (downloadHandle.jobId === undefined) {
137+
throw new RnExecutorchError(
138+
RnExecutorchErrorCode.ResourceFetcherDownloadFailed,
139+
'Download job ID is missing. This should not happen on Android.'
140+
);
141+
}
142+
RNFS.stopDownload(downloadHandle.jobId);
143+
if (
144+
await ResourceFetcherUtils.checkFileExists(downloadHandle.cacheFileUri)
145+
) {
146+
await RNFS.unlink(downloadHandle.cacheFileUri);
122147
}
123148
}
124149

125150
this.downloads.delete(source);
126-
dl.reject(
151+
downloadHandle.reject(
127152
new RnExecutorchError(
128153
RnExecutorchErrorCode.DownloadInterrupted,
129154
'Download was canceled.'

‎packages/bare-resource-fetcher/src/handlers.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ export interface ActiveDownload {
2020
uri: string;
2121
fileUri: string;
2222
cacheFileUri: string;
23-
// settle and reject are the resolve/reject of the Promise returned by handleRemote.
23+
// resolve and reject are the resolve/reject of the Promise returned by handleRemote.
2424
// They are stored here so that cancel() and resume() in the fetcher class can
2525
// unblock the fetch() loop from outside the download flow.
26-
settle: (path: string) => void;
26+
resolve: (path: string) => void;
2727
reject: (error: unknown) => void;
2828
// iOS only: background downloader task, used for pause/resume/cancel
2929
task?: DownloadTask;
@@ -108,11 +108,11 @@ export async function handleRemote(
108108

109109
// We need a Promise whose resolution can be triggered from outside this function —
110110
// by cancel() or resume() in the fetcher class. A plain async function can't do that,
111-
// so we create the Promise manually and store settle/reject in the downloads map.
112-
let settle: (path: string) => void = () => {};
111+
// so we create the Promise manually and store resolve/reject in the downloads map.
112+
let resolve: (path: string) => void = () => {};
113113
let reject: (error: unknown) => void = () => {};
114114
const promise = new Promise<string>((res, rej) => {
115-
settle = res;
115+
resolve = res;
116116
reject = rej;
117117
});
118118

@@ -133,7 +133,7 @@ export async function handleRemote(
133133
uri,
134134
fileUri,
135135
cacheFileUri,
136-
settle,
136+
resolve,
137137
reject,
138138
jobId: rnfsDownload.jobId,
139139
});
@@ -162,8 +162,7 @@ export async function handleRemote(
162162
}
163163

164164
downloads.delete(source);
165-
ResourceFetcherUtils.triggerHuggingFaceDownloadCounter(uri);
166-
settle(ResourceFetcherUtils.removeFilePrefix(fileUri));
165+
resolve(ResourceFetcherUtils.removeFilePrefix(fileUri));
167166
})
168167
.catch((error: unknown) => {
169168
if (!downloads.has(source)) return; // canceled externally
@@ -186,25 +185,25 @@ export async function handleRemote(
186185
progressCallback(progress.bytesDownloaded / progress.bytesTotal);
187186
})
188187
.done(async () => {
189-
const dl = downloads.get(source);
190-
// If paused or canceled, settle/reject will be called externally — do nothing here.
191-
if (!dl || dl.status === DownloadStatus.PAUSED) return;
188+
const downloadHandle = downloads.get(source);
189+
// If paused or canceled, resolve/reject will be called externally — do nothing here.
190+
if (!downloadHandle || downloadHandle.status === DownloadStatus.PAUSED)
191+
return;
192192

193193
try {
194194
await RNFS.moveFile(cacheFileUri, fileUri);
195195
// Required by the background downloader library to signal iOS that the
196196
// background download session is complete.
197197
const fn = fileUri.split('/').pop();
198-
if (fn) await completeHandler(fn);
198+
if (fn) completeHandler(fn);
199199
} catch (error) {
200200
downloads.delete(source);
201201
reject(error);
202202
return;
203203
}
204204

205205
downloads.delete(source);
206-
ResourceFetcherUtils.triggerHuggingFaceDownloadCounter(uri);
207-
settle(ResourceFetcherUtils.removeFilePrefix(fileUri));
206+
resolve(ResourceFetcherUtils.removeFilePrefix(fileUri));
208207
})
209208
.error((error: any) => {
210209
if (!downloads.has(source)) return; // canceled externally
@@ -224,7 +223,7 @@ export async function handleRemote(
224223
uri,
225224
fileUri,
226225
cacheFileUri,
227-
settle,
226+
resolve,
228227
reject,
229228
task,
230229
});

‎packages/expo-resource-fetcher/src/ResourceFetcher.ts‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,29 +79,29 @@ class ExpoResourceFetcherClass extends BaseResourceFetcherClass<ActiveDownload>
7979
}
8080

8181
protected async pause(source: ResourceSource): Promise<void> {
82-
const dl = this.downloads.get(source)!;
83-
if (dl.status === DownloadStatus.PAUSED) {
82+
const downloadHandle = this.downloads.get(source)!;
83+
if (downloadHandle.status === DownloadStatus.PAUSED) {
8484
throw new RnExecutorchError(
8585
RnExecutorchErrorCode.ResourceFetcherAlreadyPaused,
8686
"The file download is currently paused. Can't pause the download of the same file twice."
8787
);
8888
}
89-
dl.status = DownloadStatus.PAUSED;
90-
await dl.downloadResumable.pauseAsync();
89+
downloadHandle.status = DownloadStatus.PAUSED;
90+
await downloadHandle.downloadResumable.pauseAsync();
9191
}
9292

9393
protected async resume(source: ResourceSource): Promise<void> {
94-
const dl = this.downloads.get(source)!;
95-
if (dl.status === DownloadStatus.ONGOING) {
94+
const downloadHandle = this.downloads.get(source)!;
95+
if (downloadHandle.status === DownloadStatus.ONGOING) {
9696
throw new RnExecutorchError(
9797
RnExecutorchErrorCode.ResourceFetcherAlreadyOngoing,
9898
"The file download is currently ongoing. Can't resume the ongoing download."
9999
);
100100
}
101-
dl.status = DownloadStatus.ONGOING;
102-
const result = await dl.downloadResumable.resumeAsync();
101+
downloadHandle.status = DownloadStatus.ONGOING;
102+
const result = await downloadHandle.downloadResumable.resumeAsync();
103103
const current = this.downloads.get(source);
104-
// Paused again or canceled during resume — settle/reject handled elsewhere.
104+
// Paused again or canceled during resume — resolve/reject handled elsewhere.
105105
if (!current || current.status === DownloadStatus.PAUSED) return;
106106

107107
if (
@@ -110,27 +110,32 @@ class ExpoResourceFetcherClass extends BaseResourceFetcherClass<ActiveDownload>
110110
result.status !== HTTP_CODE.PARTIAL_CONTENT)
111111
) {
112112
this.downloads.delete(source);
113-
dl.reject(
113+
downloadHandle.reject(
114114
new RnExecutorchError(
115115
RnExecutorchErrorCode.ResourceFetcherDownloadFailed,
116-
`Failed to resume download from '${dl.uri}', status: ${result?.status}`
116+
`Failed to resume download from '${downloadHandle.uri}', status: ${result?.status}`
117117
)
118118
);
119119
return;
120120
}
121121

122122
const { moveAsync } = await import('expo-file-system/legacy');
123-
await moveAsync({ from: dl.cacheFileUri, to: dl.fileUri });
123+
await moveAsync({
124+
from: downloadHandle.cacheFileUri,
125+
to: downloadHandle.fileUri,
126+
});
124127
this.downloads.delete(source);
125-
ResourceFetcherUtils.triggerHuggingFaceDownloadCounter(dl.uri);
126-
dl.settle(ResourceFetcherUtils.removeFilePrefix(dl.fileUri));
128+
ResourceFetcherUtils.triggerHuggingFaceDownloadCounter(downloadHandle.uri);
129+
downloadHandle.resolve(
130+
ResourceFetcherUtils.removeFilePrefix(downloadHandle.fileUri)
131+
);
127132
}
128133

129134
protected async cancel(source: ResourceSource): Promise<void> {
130-
const dl = this.downloads.get(source)!;
131-
await dl.downloadResumable.cancelAsync();
135+
const downloadHandle = this.downloads.get(source)!;
136+
await downloadHandle.downloadResumable.cancelAsync();
132137
this.downloads.delete(source);
133-
dl.reject(
138+
downloadHandle.reject(
134139
new RnExecutorchError(
135140
RnExecutorchErrorCode.DownloadInterrupted,
136141
'Download was canceled.'

‎packages/expo-resource-fetcher/src/handlers.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ export interface ActiveDownload {
2828
uri: string;
2929
fileUri: string;
3030
cacheFileUri: string;
31-
// settle and reject are the resolve/reject of the Promise returned by handleRemote.
31+
// resolve and reject are the resolve/reject of the Promise returned by handleRemote.
3232
// They are stored here so that cancel() and resume() in the fetcher class can
3333
// unblock the fetch() loop from outside the download flow.
34-
settle: (path: string) => void;
34+
resolve: (path: string) => void;
3535
reject: (error: unknown) => void;
3636
}
3737

@@ -113,11 +113,11 @@ export async function handleRemote(
113113

114114
// We need a Promise whose resolution can be triggered from outside this function —
115115
// by cancel() or resume() in the fetcher class. A plain async function can't do that,
116-
// so we create the Promise manually and store settle/reject in the downloads map.
117-
let settle: (path: string) => void = () => {};
116+
// so we create the Promise manually and store resolve/reject in the downloads map.
117+
let resolve: (path: string) => void = () => {};
118118
let reject: (error: unknown) => void = () => {};
119119
const promise = new Promise<string>((res, rej) => {
120-
settle = res;
120+
resolve = res;
121121
reject = rej;
122122
});
123123

@@ -146,17 +146,18 @@ export async function handleRemote(
146146
uri,
147147
fileUri,
148148
cacheFileUri,
149-
settle,
149+
resolve,
150150
reject,
151151
});
152152

153153
downloadResumable
154154
.downloadAsync()
155155
.then(async (result) => {
156-
const dl = downloads.get(source);
157-
// If paused or canceled during the download, settle/reject will be called
156+
const downloadHandle = downloads.get(source);
157+
// If paused or canceled during the download, resolve/reject will be called
158158
// externally by resume() or cancel() — do nothing here.
159-
if (!dl || dl.status === DownloadStatus.PAUSED) return;
159+
if (!downloadHandle || downloadHandle.status === DownloadStatus.PAUSED)
160+
return;
160161

161162
if (
162163
!result ||
@@ -182,8 +183,7 @@ export async function handleRemote(
182183
}
183184

184185
downloads.delete(source);
185-
ResourceFetcherUtils.triggerHuggingFaceDownloadCounter(uri);
186-
settle(ResourceFetcherUtils.removeFilePrefix(fileUri));
186+
resolve(ResourceFetcherUtils.removeFilePrefix(fileUri));
187187
})
188188
.catch((error) => {
189189
downloads.delete(source);

‎packages/react-native-executorch/src/utils/BaseResourceFetcherClass.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export abstract class BaseResourceFetcherClass<
8989
* @remarks
9090
* The returned Promise must be resolvable from outside this function —
9191
* `cancel()` and `resume()` need to unblock the `fetch()` loop by calling
92-
* `settle`/`reject` stored on the `downloads` map entry. See handlers.ts
92+
* `resolve`/`reject` stored on the `downloads` map entry. See handlers.ts
9393
* for the leaked-resolver pattern used to achieve this.
9494
*/
9595
protected abstract handleRemote(
@@ -108,7 +108,7 @@ export abstract class BaseResourceFetcherClass<
108108

109109
/**
110110
* Resume a paused download for `source`. The result must flow back through
111-
* the original `fetch()` promise — call `settle(path)` on the `downloads`
111+
* the original `fetch()` promise — call `resolve(path)` on the `downloads`
112112
* map entry rather than creating a new Promise. Should throw
113113
* `ResourceFetcherAlreadyOngoing` if the download is not paused, and
114114
* `ResourceFetcherPlatformNotSupported` if the platform does not support

0 commit comments

Comments
 (0)