Skip to content

Commit c0a619f

Browse files
authored
[Blueprints] ensure git:directory resource returns non-empty-name (#2779)
## Motivation for the change, related issues Before this PR, this step would fail: ```js { "step": "installPlugin", "pluginData": { "resource": "git:directory", "url": "https://github.com/juanma-wp/meetup-cheltenham-workshop/", "ref": "final", "path": "/" }, "options": { "activate": true } }, ``` This is because `installPlugin` names the plugin directory inside `wp-content` after the filename of the resolved resource, and the `git:directory` resource would use `path` to generate a filename called `/`. With this PR, `git:directory` generates a directory name using the full set of information from the resource spec: URL, ref, path. The names are now more like this: `https-github.com-WordPress-wordpress-playground-trunk`. ## Testing Instructions (or ideally a Blueprint) CI – this PR comes with tests cc @juanmaguitar
1 parent 5a2a0f2 commit c0a619f

File tree

2 files changed

+68
-18
lines changed

2 files changed

+68
-18
lines changed

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

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,6 @@ describe('GitDirectoryResource', () => {
5555

5656
it('defaults to the repo root when path is omitted', async () => {
5757
const url = 'https://github.com/WordPress/wordpress-playground';
58-
const fallbackName = url
59-
.replaceAll(/[^a-zA-Z0-9-.]/g, '-')
60-
.replaceAll(/-+/g, '-');
6158
const resource = new GitDirectoryResource({
6259
resource: 'git:directory',
6360
url,
@@ -67,11 +64,53 @@ describe('GitDirectoryResource', () => {
6764
});
6865
const { files, name } = await resource.resolve();
6966

70-
expect(name).toBe(fallbackName);
71-
expect(resource.name).toBe('.github');
67+
// Human-readable name
68+
expect(resource.name).toBe(
69+
'https://github.com/WordPress/wordpress-playground (trunk) at .github'
70+
);
71+
72+
// Filename
73+
expect(name).toBe(
74+
'https-github.com-WordPress-wordpress-playground-trunk-at-.github'
75+
);
7276
expect(files['dependabot.yml']).toBeInstanceOf(Uint8Array);
7377
});
7478
});
79+
80+
describe('name', () => {
81+
it('should return a non-empty name when path is omitted', async () => {
82+
const resource = new GitDirectoryResource({
83+
resource: 'git:directory',
84+
url: 'https://github.com/WordPress/link-manager',
85+
ref: 'trunk',
86+
});
87+
const { name } = await resource.resolve();
88+
expect(name).toBe('https-github.com-WordPress-link-manager-trunk');
89+
});
90+
91+
it('should return a non-empty name when path is empty', async () => {
92+
const resource = new GitDirectoryResource({
93+
resource: 'git:directory',
94+
url: 'https://github.com/WordPress/link-manager',
95+
ref: 'trunk',
96+
path: '',
97+
});
98+
const { name } = await resource.resolve();
99+
expect(name).toBe('https-github.com-WordPress-link-manager-trunk');
100+
});
101+
102+
it('should return a non-empty name when path has no letters', async () => {
103+
const resource = new GitDirectoryResource({
104+
resource: 'git:directory',
105+
url: 'https://github.com/WordPress/link-manager',
106+
ref: 'trunk',
107+
// A path with only a few files to avoid timing out.
108+
path: '/',
109+
});
110+
const { name } = await resource.resolve();
111+
expect(name).toBe('https-github.com-WordPress-link-manager-trunk');
112+
});
113+
});
75114
});
76115

77116
describe('BlueprintResource', () => {

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

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
} from '@php-wasm/progress';
66
import type { FileTree, UniversalPHP } from '@php-wasm/universal';
77
import type { Semaphore } from '@php-wasm/util';
8-
import { dirname } from '@php-wasm/util';
8+
import { randomFilename } from '@php-wasm/util';
99
import {
1010
listDescendantFiles,
1111
listGitFiles,
@@ -586,24 +586,35 @@ export class GitDirectoryResource extends Resource<Directory> {
586586
name.substring(requestedPath.length).replace(/^\/+/, '')
587587
);
588588
return {
589-
name:
590-
dirname(this.reference.path || '') ||
591-
this.reference.url
592-
.replaceAll(/[^a-zA-Z0-9-.]/g, '-')
593-
.replaceAll(/-+/g, '-'),
589+
name: this.filename,
594590
files,
595591
};
596592
}
597593

594+
/**
595+
* Generate a nice, non-empty filename – the installPlugin step depends on it.
596+
*/
597+
get filename() {
598+
return (
599+
this.name
600+
.replaceAll(/[^a-zA-Z0-9-.]/g, '-')
601+
.replaceAll(/-+/g, '-')
602+
.replace(/^[^a-zA-Z0-9]+|[^a-zA-Z0-9]+$/g, '') ||
603+
randomFilename()
604+
);
605+
}
606+
598607
/** @inheritDoc */
599608
get name() {
600-
const path = this.reference.path ?? '';
601-
if (!path) {
602-
return this.reference.url
603-
.replaceAll(/[^a-zA-Z0-9-.]/g, '-')
604-
.replaceAll(/-+/g, '-');
605-
}
606-
return path.split('/').pop() || '';
609+
return [
610+
this.reference.url,
611+
this.reference.ref ? `(${this.reference.ref})` : '',
612+
this.reference.path?.replace(/^\/+/, '')
613+
? `at ${this.reference.path}`
614+
: '',
615+
]
616+
.filter((segment) => segment.length > 0)
617+
.join(' ');
607618
}
608619
}
609620

0 commit comments

Comments
 (0)