Skip to content

Commit

Permalink
feedback 3
Browse files Browse the repository at this point in the history
  • Loading branch information
mxschmitt committed Jun 7, 2024
1 parent bb6be58 commit f87b89e
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 28 deletions.
32 changes: 13 additions & 19 deletions packages/playwright-core/src/client/elementHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,28 +267,27 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
throw new Error('File paths must be all files or a single directory');

if (context._connection.isRemote()) {
let streams: channels.WritableStreamChannel[] | undefined;
let localPaths: string[] | undefined;
const streams: channels.WritableStreamChannel[] = [];
await Promise.all((items as string[]).map(async item => {
const isDirectory = (await fs.promises.stat(item)).isDirectory();
const files = isDirectory ? (await fs.promises.readdir(item, { withFileTypes: true, recursive: true })).filter(f => f.isFile()).map(f => path.join(item, f.name)) : [item];
const { writableStreams, remoteDir } = await context._wrapApiCall(async () => context._channel.createTempFiles({
rootDirName: isDirectory ? item : undefined,
items: await Promise.all(files.map(f => fileToTempFileParams(f))),
const files = isDirectory ? (await fs.promises.readdir(item, { withFileTypes: true, recursive: true })).filter(f => f.isFile()).map(f => path.join(f.path, f.name)) : [item];
const { writableStreams } = await context._wrapApiCall(async () => context._channel.createTempFiles({
rootDirName: isDirectory ? path.basename(item) : undefined,
items: await Promise.all(files.map(async file => {
const lastModifiedMs = (await fs.promises.stat(file)).mtimeMs;
return {
name: isDirectory ? path.relative(item, file) : path.basename(file),
lastModifiedMs
};
})),
}), true);
for (let i = 0; i < files.length; i++) {
const writable = WritableStream.from(writableStreams[i]);
await pipelineAsync(fs.createReadStream(files[i]), writable.stream());
}
if (isDirectory) {
localPaths ??= [];
localPaths.push(remoteDir);
} else {
streams ??= [];
streams.push(...writableStreams);
}
streams.push(...writableStreams);
}));
return { streams, localPaths };
return { streams };
}
return { localPaths: items.map(f => path.resolve(f as string)) as string[] };
}
Expand All @@ -299,11 +298,6 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
return { payloads };
}

async function fileToTempFileParams(file: string): Promise<channels.BrowserContextCreateTempFilesParams['items'][0]> {
const lastModifiedMs = (await fs.promises.stat(file)).mtimeMs;
return { name: path.basename(file), lastModifiedMs };
}

export function determineScreenshotType(options: { path?: string, type?: 'png' | 'jpeg' }): 'png' | 'jpeg' | undefined {
if (options.path) {
const mimeType = mime.getType(options.path);
Expand Down
1 change: 0 additions & 1 deletion packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,6 @@ scheme.BrowserContextCreateTempFilesParams = tObject({
})),
});
scheme.BrowserContextCreateTempFilesResult = tObject({
remoteDir: tString,
writableStreams: tArray(tChannel(['WritableStream'])),
});
scheme.BrowserContextUpdateSubscriptionParams = tObject({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,14 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
async createTempFiles(params: channels.BrowserContextCreateTempFilesParams): Promise<channels.BrowserContextCreateTempFilesResult> {
const dir = this._context._browser.options.artifactsDir;
const tmpDir = path.join(dir, 'upload-' + createGuid());
const tempDirWithRootName = path.join(tmpDir, params.rootDirName ? path.basename(params.rootDirName) : '');
const tempDirWithRootName = params.rootDirName ? path.join(tmpDir, path.basename(params.rootDirName)) : tmpDir;
await fs.promises.mkdir(tempDirWithRootName, { recursive: true });
this._context._tempDirs.push(tmpDir);
return {
remoteDir: tempDirWithRootName,
writableStreams: await Promise.all(params.items.map(async item => {
await fs.promises.mkdir(path.dirname(path.join(tempDirWithRootName, item.name)), { recursive: true });
const file = fs.createWriteStream(path.join(tempDirWithRootName, item.name));
return new WritableStreamDispatcher(this, file, item.lastModifiedMs);
return new WritableStreamDispatcher(this, file, item.lastModifiedMs, params.rootDirName ? tempDirWithRootName : undefined);
}))
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ import type { BrowserContextDispatcher } from './browserContextDispatcher';
export class WritableStreamDispatcher extends Dispatcher<{ guid: string, stream: fs.WriteStream }, channels.WritableStreamChannel, BrowserContextDispatcher> implements channels.WritableStreamChannel {
_type_WritableStream = true;
private _lastModifiedMs: number | undefined;
private _rootDir: string | undefined;

constructor(scope: BrowserContextDispatcher, stream: fs.WriteStream, lastModifiedMs?: number) {
constructor(scope: BrowserContextDispatcher, stream: fs.WriteStream, lastModifiedMs?: number, rootDir?: string) {
super(scope, { guid: 'writableStream@' + createGuid(), stream }, 'WritableStream', {});
this._lastModifiedMs = lastModifiedMs;
this._rootDir = rootDir;
}

async write(params: channels.WritableStreamWriteParams): Promise<channels.WritableStreamWriteResult> {
Expand All @@ -51,4 +53,8 @@ export class WritableStreamDispatcher extends Dispatcher<{ guid: string, stream:
path(): string {
return this._object.stream.path as string;
}

rootDir(): string | undefined {
return this._rootDir;
}
}
9 changes: 7 additions & 2 deletions packages/playwright-core/src/server/fileUploadUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,13 @@ export async function prepareFilesForUpload(frame: Frame, params: channels.Eleme
if ([payloads, localPaths, streams].filter(Boolean).length !== 1)
throw new Error('Exactly one of payloads, localPaths and streams must be provided');

if (streams)
localPaths = streams.map(c => (c as WritableStreamDispatcher).path());
if (streams) {
const directoryMode = streams.every(c => (c as WritableStreamDispatcher).rootDir());
if (directoryMode)
localPaths = Array.from(new Set(streams.map(c => (c as WritableStreamDispatcher).rootDir()!)));
else
localPaths = streams.map(c => (c as WritableStreamDispatcher).path());
}
if (localPaths) {
for (const p of localPaths)
assert(path.isAbsolute(p) && path.resolve(p) === p, 'Paths provided to localPaths must be absolute and fully resolved.');
Expand Down
1 change: 0 additions & 1 deletion packages/protocol/src/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1748,7 +1748,6 @@ export type BrowserContextCreateTempFilesOptions = {
rootDirName?: string,
};
export type BrowserContextCreateTempFilesResult = {
remoteDir: string,
writableStreams: WritableStreamChannel[],
};
export type BrowserContextUpdateSubscriptionParams = {
Expand Down
1 change: 0 additions & 1 deletion packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1195,7 +1195,6 @@ BrowserContext:
name: string
lastModifiedMs: number?
returns:
remoteDir: string
writableStreams:
type: array
items: WritableStream
Expand Down

0 comments on commit f87b89e

Please sign in to comment.