Skip to content

Commit 8174ca0

Browse files
akirkakirk
committed
Simplify logic by avoiding processing cors wrapped urls
1 parent a7e01e2 commit 8174ca0

File tree

11 files changed

+146
-209
lines changed

11 files changed

+146
-209
lines changed

‎packages/playground/blueprints/src/lib/v1/compile.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export interface CompileBlueprintV1Options {
8787
* Additional headers to pass to git operations.
8888
* A function that returns headers based on the URL being accessed.
8989
*/
90-
gitAdditionalHeaders?: (url: string) => Record<string, string>;
90+
gitAdditionalHeadersCallback?: (url: string) => Record<string, string>;
9191
/**
9292
* Additional steps to add to the blueprint.
9393
*/
@@ -147,7 +147,7 @@ function compileBlueprintJson(
147147
onBlueprintValidated = () => {},
148148
corsProxy,
149149
streamBundledFile,
150-
gitAdditionalHeaders,
150+
gitAdditionalHeadersCallback,
151151
additionalSteps,
152152
}: CompileBlueprintV1Options = {}
153153
): CompiledBlueprintV1 {
@@ -327,7 +327,7 @@ function compileBlueprintJson(
327327
totalProgressWeight,
328328
corsProxy,
329329
streamBundledFile,
330-
gitAdditionalHeaders,
330+
gitAdditionalHeadersCallback,
331331
})
332332
);
333333

@@ -525,7 +525,7 @@ interface CompileStepArgsOptions {
525525
* Additional headers to pass to git operations.
526526
* A function that returns headers based on the URL being accessed.
527527
*/
528-
gitAdditionalHeaders?: (url: string) => Record<string, string>;
528+
gitAdditionalHeadersCallback?: (url: string) => Record<string, string>;
529529
}
530530

531531
/**
@@ -544,7 +544,7 @@ function compileStep<S extends StepDefinition>(
544544
totalProgressWeight,
545545
corsProxy,
546546
streamBundledFile,
547-
gitAdditionalHeaders,
547+
gitAdditionalHeadersCallback,
548548
}: CompileStepArgsOptions
549549
): { run: CompiledV1Step; step: S; resources: Array<Resource<any>> } {
550550
const stepProgress = rootProgressTracker.stage(
@@ -559,7 +559,7 @@ function compileStep<S extends StepDefinition>(
559559
semaphore,
560560
corsProxy,
561561
streamBundledFile,
562-
gitAdditionalHeaders,
562+
gitAdditionalHeadersCallback,
563563
});
564564
}
565565
args[key] = value;

‎packages/playground/blueprints/src/lib/v1/resources.ts‎

Lines changed: 69 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { FileTree, UniversalPHP } from '@php-wasm/universal';
77
import type { Semaphore } from '@php-wasm/util';
88
import { randomFilename } from '@php-wasm/util';
99
import {
10+
GitAuthenticationError,
1011
listDescendantFiles,
1112
listGitFiles,
1213
resolveCommitHash,
@@ -157,14 +158,16 @@ export abstract class Resource<T extends File | Directory> {
157158
progress,
158159
corsProxy,
159160
streamBundledFile,
160-
gitAdditionalHeaders,
161+
gitAdditionalHeadersCallback,
161162
}: {
162163
/** Optional semaphore to limit concurrent downloads */
163164
semaphore?: Semaphore;
164165
progress?: ProgressTracker;
165166
corsProxy?: string;
166167
streamBundledFile?: StreamBundledFile;
167-
gitAdditionalHeaders?: (url: string) => Record<string, string>;
168+
gitAdditionalHeadersCallback?: (
169+
url: string
170+
) => Record<string, string>;
168171
}
169172
): Resource<File | Directory> {
170173
let resource: Resource<File | Directory>;
@@ -187,7 +190,7 @@ export abstract class Resource<T extends File | Directory> {
187190
case 'git:directory':
188191
resource = new GitDirectoryResource(ref, progress, {
189192
corsProxy,
190-
additionalHeaders: gitAdditionalHeaders,
193+
additionalHeaders: gitAdditionalHeadersCallback,
191194
});
192195
break;
193196
case 'literal:directory':
@@ -579,60 +582,77 @@ export class GitDirectoryResource extends Resource<Directory> {
579582
}
580583

581584
async resolve() {
585+
const additionalHeaders =
586+
this.options?.additionalHeaders?.(this.reference.url) ?? {};
587+
582588
const repoUrl = this.options?.corsProxy
583589
? `${this.options.corsProxy}${this.reference.url}`
584590
: this.reference.url;
585591

586-
const commitHash = await resolveCommitHash(
587-
repoUrl,
588-
{
589-
value: this.reference.ref,
590-
type: this.reference.refType ?? 'infer',
591-
},
592-
this.options?.additionalHeaders
593-
);
594-
const allFiles = await listGitFiles(
595-
repoUrl,
596-
commitHash,
597-
this.options?.additionalHeaders
598-
);
599-
600-
const requestedPath = (this.reference.path ?? '').replace(/^\/+/, '');
601-
const filesToClone = listDescendantFiles(allFiles, requestedPath);
602-
const checkout = await sparseCheckout(
603-
repoUrl,
604-
commitHash,
605-
filesToClone,
606-
{
607-
withObjects: this.reference['.git'],
608-
additionalHeaders: this.options?.additionalHeaders,
609-
}
610-
);
611-
let files = checkout.files;
592+
try {
593+
const commitHash = await resolveCommitHash(
594+
repoUrl,
595+
{
596+
value: this.reference.ref,
597+
type: this.reference.refType ?? 'infer',
598+
},
599+
additionalHeaders
600+
);
601+
const allFiles = await listGitFiles(
602+
repoUrl,
603+
commitHash,
604+
additionalHeaders
605+
);
612606

613-
// Remove the path prefix from the cloned file names.
614-
files = mapKeys(files, (name) =>
615-
name.substring(requestedPath.length).replace(/^\/+/, '')
616-
);
617-
if (this.reference['.git']) {
618-
const gitFiles = await createDotGitDirectory({
619-
repoUrl: this.reference.url,
607+
const requestedPath = (this.reference.path ?? '').replace(
608+
/^\/+/,
609+
''
610+
);
611+
const filesToClone = listDescendantFiles(allFiles, requestedPath);
612+
const checkout = await sparseCheckout(
613+
repoUrl,
620614
commitHash,
621-
ref: this.reference.ref,
622-
refType: this.reference.refType,
623-
objects: checkout.objects ?? [],
624-
fileOids: checkout.fileOids ?? {},
625-
pathPrefix: requestedPath,
626-
});
627-
files = {
628-
...gitFiles,
629-
...files,
615+
filesToClone,
616+
{
617+
withObjects: this.reference['.git'],
618+
additionalHeaders,
619+
}
620+
);
621+
let files = checkout.files;
622+
623+
// Remove the path prefix from the cloned file names.
624+
files = mapKeys(files, (name) =>
625+
name.substring(requestedPath.length).replace(/^\/+/, '')
626+
);
627+
if (this.reference['.git']) {
628+
const gitFiles = await createDotGitDirectory({
629+
repoUrl: this.reference.url,
630+
commitHash,
631+
ref: this.reference.ref,
632+
refType: this.reference.refType,
633+
objects: checkout.objects ?? [],
634+
fileOids: checkout.fileOids ?? {},
635+
pathPrefix: requestedPath,
636+
});
637+
files = {
638+
...gitFiles,
639+
...files,
640+
};
641+
}
642+
return {
643+
name: this.filename,
644+
files,
630645
};
646+
} catch (error) {
647+
if (error instanceof GitAuthenticationError) {
648+
// Unwrap and re-throw with the original URL (without CORS proxy)
649+
throw new GitAuthenticationError(
650+
this.reference.url,
651+
error.status
652+
);
653+
}
654+
throw error;
631655
}
632-
return {
633-
name: this.filename,
634-
files,
635-
};
636656
}
637657

638658
/**

‎packages/playground/client/src/blueprints-v1-handler.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export class BlueprintsV1Handler {
2121
onBlueprintValidated,
2222
onBlueprintStepCompleted,
2323
corsProxy,
24-
gitAdditionalHeaders,
24+
gitAdditionalHeadersCallback,
2525
mounts,
2626
sapiName,
2727
scope,
@@ -73,7 +73,7 @@ export class BlueprintsV1Handler {
7373
onStepCompleted: onBlueprintStepCompleted,
7474
onBlueprintValidated,
7575
corsProxy,
76-
gitAdditionalHeaders,
76+
gitAdditionalHeadersCallback,
7777
});
7878
await runBlueprintV1Steps(compiled, playground);
7979
}

‎packages/playground/client/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export interface StartPlaygroundOptions {
8989
* Additional headers to pass to git operations.
9090
* A function that returns headers based on the URL being accessed.
9191
*/
92-
gitAdditionalHeaders?: (url: string) => Record<string, string>;
92+
gitAdditionalHeadersCallback?: (url: string) => Record<string, string>;
9393
/**
9494
* The version of the SQLite driver to use.
9595
* Defaults to the latest development version.

‎packages/playground/storage/src/lib/git-sparse-checkout.spec.ts‎

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -193,40 +193,28 @@ describe('listGitFiles', () => {
193193
});
194194
});
195195

196-
describe('gitAdditionalHeaders callback', () => {
196+
describe('gitAdditionalHeaders', () => {
197197
const repoUrl = 'https://github.com/WordPress/wordpress-playground.git';
198198

199-
it('should invoke callback with the actual URL being fetched', async () => {
200-
const headerCallback = vi.fn<GitAdditionalHeaders>(() => ({}));
199+
it('should successfully fetch when headers is empty object', async () => {
200+
const headers: GitAdditionalHeaders = {};
201201

202-
await listGitRefs(repoUrl, 'refs/heads/trunk', headerCallback);
203-
204-
expect(headerCallback).toHaveBeenCalledWith(repoUrl);
205-
});
206-
207-
it('should successfully fetch when callback returns empty object', async () => {
208-
const headerCallback: GitAdditionalHeaders = () => ({});
209-
210-
const refs = await listGitRefs(
211-
repoUrl,
212-
'refs/heads/trunk',
213-
headerCallback
214-
);
202+
const refs = await listGitRefs(repoUrl, 'refs/heads/trunk', headers);
215203

216204
expect(refs).toHaveProperty('refs/heads/trunk');
217205
expect(refs['refs/heads/trunk']).toMatch(/^[a-f0-9]{40}$/);
218206
});
219207

220-
it('should pass callback through the full call chain', async () => {
221-
const headerCallback = vi.fn<GitAdditionalHeaders>(() => ({}));
208+
it('should pass headers through the full call chain', async () => {
209+
const headers: GitAdditionalHeaders = {};
222210

223211
await resolveCommitHash(
224212
repoUrl,
225213
{ value: 'trunk', type: 'branch' },
226-
headerCallback
214+
headers
227215
);
228216

229-
expect(headerCallback).toHaveBeenCalledWith(repoUrl);
217+
expect(headers).toBeDefined();
230218
});
231219
});
232220

@@ -248,15 +236,15 @@ describe('authentication error handling', () => {
248236
statusText: 'Unauthorized',
249237
});
250238

251-
const headerCallback: GitAdditionalHeaders = () => ({
239+
const headers: GitAdditionalHeaders = {
252240
Authorization: 'Bearer token',
253-
});
241+
};
254242

255243
await expect(
256244
listGitRefs(
257245
'https://github.com/user/private-repo',
258246
'refs/heads/main',
259-
headerCallback
247+
headers
260248
)
261249
).rejects.toThrow(
262250
'Authentication required to access private repository'
@@ -270,15 +258,15 @@ describe('authentication error handling', () => {
270258
statusText: 'Forbidden',
271259
});
272260

273-
const headerCallback: GitAdditionalHeaders = () => ({
261+
const headers: GitAdditionalHeaders = {
274262
Authorization: 'Bearer token',
275-
});
263+
};
276264

277265
await expect(
278266
listGitRefs(
279267
'https://github.com/user/private-repo',
280268
'refs/heads/main',
281-
headerCallback
269+
headers
282270
)
283271
).rejects.toThrow(
284272
'Authentication required to access private repository'
@@ -292,15 +280,15 @@ describe('authentication error handling', () => {
292280
statusText: 'Not Found',
293281
});
294282

295-
const headerCallback: GitAdditionalHeaders = () => ({
283+
const headers: GitAdditionalHeaders = {
296284
Authorization: 'Bearer token',
297-
});
285+
};
298286

299287
await expect(
300288
listGitRefs(
301289
'https://github.com/user/repo-or-no-access',
302290
'refs/heads/main',
303-
headerCallback
291+
headers
304292
)
305293
).rejects.toThrow(
306294
'Failed to fetch git refs from https://github.com/user/repo-or-no-access: 404 Not Found'

0 commit comments

Comments
 (0)