ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: packages/storage-driver-s3/src/index.ts (TypeScript)

Code before:
		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		await this.client.send(new CopyObjectCommand(params));
	}

	async write(filepath: string, content: Readable, type?: string) {
		const params: PutObjectCommandInput = {
			Key: this.fullPath(filepath),
			Body: content,
			Bucket: this.config.bucket,
		};

		if (type) {
			params.ContentType = type;
		}

		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		if (this.config.serverSideEncryption) {
			params.ServerSideEncryption = this.config.serverSideEncryption;

			if (this.config.serverSideEncryption === 'aws:kms' && this.config.serverSideEncryptionKmsKeyId) {
				params.SSEKMSKeyId = this.config.serverSideEncryptionKmsKeyId;
			}
		}

		const upload = new Upload({
			client: this.client,
			params,
		});

		await upload.done();
	}

	async delete(filepath: string) {
		await this.client.send(new DeleteObjectCommand({ Key: this.fullPath(filepath), Bucket: this.config.bucket }));
	}

	async *list(prefix = '') {
		let Prefix = this.fullPath(prefix);

		// Current dir (`.`) isn't known to S3, needs to be an empty prefix instead
		if (Prefix === '.') Prefix = '';

		let continuationToken: string | undefined = undefined;

		do {

Review comment (suggestion):
`aws:kms:dsse` was added to the test but not checked. Let's implement this on all occurrences

```suggestion
			if (
				['aws:kms', 'aws:kms:dsse'].includes(this.config.serverSideEncryption) &&
				this.config.serverSideEncryptionKmsKeyId
			) {
```

Code after:
				params.SSEKMSKeyId = this.config.serverSideEncryptionKmsKeyId;
			}
		}

		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		await this.client.send(new CopyObjectCommand(params));
	}

	async write(filepath: string, content: Readable, type?: string) {
		const params: PutObjectCommandInput = {
			Key: this.fullPath(filepath),
			Body: content,
			Bucket: this.config.bucket,
		};

		if (type) {
			params.ContentType = type;
		}

		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		if (this.config.serverSideEncryption) {
			params.ServerSideEncryption = this.config.serverSideEncryption;

			if (kmsKeyIdCheck.includes(this.config.serverSideEncryption) && this.config.serverSideEncryptionKmsKeyId) {
				params.SSEKMSKeyId = this.config.serverSideEncryptionKmsKeyId;
			}
		}

		const upload = new Upload({
			client: this.client,
			params,
		});

		await upload.done();
	}

	async delete(filepath: string) {
		await this.client.send(new DeleteObjectCommand({ Key: this.fullPath(filepath), Bucket: this.config.bucket }));
	}

	async *list(prefix = '') {
		let Prefix = this.fullPath(prefix);

		// Current dir (`.`) isn't known to S3, needs to be an empty prefix instead
		if (Prefix === '.') Prefix = '';
Repository: directus/directus
File: packages/storage-driver-s3/src/index.ts (TypeScript)

Code before:
		try {
			await this.stat(filepath);
			return true;
		} catch {
			return false;
		}
	}

	async move(src: string, dest: string) {
		await this.copy(src, dest);
		await this.delete(src);
	}

	async copy(src: string, dest: string) {
		const params: CopyObjectCommandInput = {
			Key: this.fullPath(dest),
			Bucket: this.config.bucket,
			CopySource: `/${this.config.bucket}/${this.fullPath(src)}`,
		};

		if (this.config.serverSideEncryption) {
			params.ServerSideEncryption = this.config.serverSideEncryption;

			if (this.config.serverSideEncryption === 'aws:kms' && this.config.serverSideEncryptionKmsKeyId) {
				params.SSEKMSKeyId = this.config.serverSideEncryptionKmsKeyId;
			}
		}

		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		await this.client.send(new CopyObjectCommand(params));
	}

	async write(filepath: string, content: Readable, type?: string) {
		const params: PutObjectCommandInput = {
			Key: this.fullPath(filepath),
			Body: content,
			Bucket: this.config.bucket,
		};

		if (type) {
			params.ContentType = type;
		}

		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		if (this.config.serverSideEncryption) {

Review comment (suggestion):
I think this check should be added for `createChunkedUpload` as it's also another upload command. Let's also add tests for `createChunkedUpload` 🫡 

Code after:
		};
	}

	async exists(filepath: string) {
		try {
			await this.stat(filepath);
			return true;
		} catch {
			return false;
		}
	}

	async move(src: string, dest: string) {
		await this.copy(src, dest);
		await this.delete(src);
	}

	async copy(src: string, dest: string) {
		const params: CopyObjectCommandInput = {
			Key: this.fullPath(dest),
			Bucket: this.config.bucket,
			CopySource: `/${this.config.bucket}/${this.fullPath(src)}`,
		};

		if (this.config.serverSideEncryption) {
			params.ServerSideEncryption = this.config.serverSideEncryption;

			if (kmsKeyIdCheck.includes(this.config.serverSideEncryption) && this.config.serverSideEncryptionKmsKeyId) {
				params.SSEKMSKeyId = this.config.serverSideEncryptionKmsKeyId;
			}
		}

		if (this.config.acl) {
			params.ACL = this.config.acl;
		}

		await this.client.send(new CopyObjectCommand(params));
	}

	async write(filepath: string, content: Readable, type?: string) {
		const params: PutObjectCommandInput = {
			Key: this.fullPath(filepath),
			Body: content,
			Bucket: this.config.bucket,
		};

		if (type) {
			params.ContentType = type;
		}

		if (this.config.acl) {
Repository: directus/directus
File: app/src/utils/upload-files.ts (TypeScript)

Code before:
): Promise<(File | undefined)[]> {
	const progressHandler = options?.onProgressChange || (() => undefined);
	const progressForFiles = files.map(() => 0);
	const uploadControllers: (Upload | null)[] = Array(files.length).fill(null);
	const serverStore = useServerStore();
	const maxParallelUploads = serverStore.info.uploads?.maxParallel ?? 0;
	const uploadQueue = maxParallelUploads > 0 ? new PQueue({ concurrency: maxParallelUploads }) : null;

	const startUpload = (file: globalThis.File, index: number) =>
		uploadFile(file, {
			...options,
			onProgressChange: (percentage: number) => {
				progressForFiles[index] = percentage;
				progressHandler(progressForFiles);
			},
			onChunkedUpload: (controller: Upload) => {
				uploadControllers[index] = controller;
				options?.onChunkedUpload?.(uploadControllers);
			},
		});

	try {
		const uploadedFiles = (
			maxParallelUploads === 0
				? await Promise.all(files.map((file, index) => startUpload(file, index)))
				: await Promise.all(files.map((file, index) => uploadQueue!.add(() => startUpload(file, index))))
		).filter((v) => v);

		if (options?.notifications) {
			notify({
				title: i18n.global.t('upload_files_success', { count: files.length }),
			});
		}

		return uploadedFiles;
	} catch (error) {
		unexpectedError(error);
	}

	return [];
}


Review comment (suggestion):
```suggestion
			await Promise.all(files.map((file, index) => uploadQueue.add(() => startUpload(file, index))))
```

Code after:
): Promise<(File | undefined)[]> {
	const progressHandler = options?.onProgressChange || (() => undefined);
	const progressForFiles = files.map(() => 0);
	const uploadControllers: (Upload | null)[] = Array(files.length).fill(null);

	const uploadQueue = new PQueue({
		concurrency: options?.maxConcurrency && options.maxConcurrency > 0 ? options.maxConcurrency : Infinity,
	});

	const startUpload = (file: globalThis.File, index: number) =>
		uploadFile(file, {
			...options,
			onProgressChange: (percentage: number) => {
				progressForFiles[index] = percentage;
				progressHandler(progressForFiles);
			},
			onChunkedUpload: (controller: Upload) => {
				uploadControllers[index] = controller;
				options?.onChunkedUpload?.(uploadControllers);
			},
		});

	try {
		const uploadedFiles = (
			await Promise.all(
				files.map((file, index) => uploadQueue.add(() => startUpload(file, index), { throwOnTimeout: true })),
			)
		).filter((v) => v);

		if (options?.notifications) {
			notify({
				title: i18n.global.t('upload_files_success', { count: files.length }),
			});
		}

		return uploadedFiles;
	} catch (error) {
		unexpectedError(error);
	}

	return [];
}
Repository: directus/directus
File: api/src/services/server.ts (TypeScript)

Code before:
							path: env['WEBSOCKETS_GRAPHQL_PATH'],
						}
					: false;

				info['websocket'].heartbeat = toBoolean(env['WEBSOCKETS_HEARTBEAT_ENABLED'])
					? env['WEBSOCKETS_HEARTBEAT_PERIOD']
					: false;

				info['websocket'].logs =
					toBoolean(env['WEBSOCKETS_LOGS_ENABLED']) && this.accountability.admin
						? {
								allowedLogLevels: getAllowedLogLevels((env['WEBSOCKETS_LOGS_LEVEL'] as string) || 'info'),
							}
						: false;
			} else {
				info['websocket'] = false;
			}

			if (RESUMABLE_UPLOADS.ENABLED) {
				info['uploads'] = {
					tusEnabled: true,
					chunkSize: RESUMABLE_UPLOADS.CHUNK_SIZE,
					maxParallel: FILE_UPLOADS.MAX_PARALLEL,
				};
			} else {
				info['uploads'] = {
					tusEnabled: false,
					maxParallel: FILE_UPLOADS.MAX_PARALLEL,
				};
			}
		}

		if (this.accountability?.user || !setupComplete) info['version'] = version;

		return info;
	}

	async health(): Promise<Record<string, any>> {
		const { nanoid } = await import('nanoid');

		const checkID = nanoid(5);

		// Based on https://tools.ietf.org/id/draft-inadarei-api-health-check-05.html#name-componenttype
		type HealthData = {
			status: 'ok' | 'warn' | 'error';
			releaseId: string;
			serviceId: string;
			checks: {
				[service: string]: HealthCheck[];
			};
		};

Review comment (suggestion):
As upload info will always be defined now let's do it as a fall through
```
info['uploads'] = {
 maxConcurrency: FILE_UPLOADS.MAX_PARALLEL,
};

if (RESUMABLE_UPLOADS.ENABLED) {
  info['uploads'] = {
    ...info['uploads'],
	  tus: true,
	  chunkSize: RESUMABLE_UPLOADS.CHUNK_SIZE,
  };
}


```






Code after:
							authentication: env['WEBSOCKETS_GRAPHQL_AUTH'],
							path: env['WEBSOCKETS_GRAPHQL_PATH'],
						}
					: false;

				info['websocket'].heartbeat = toBoolean(env['WEBSOCKETS_HEARTBEAT_ENABLED'])
					? env['WEBSOCKETS_HEARTBEAT_PERIOD']
					: false;

				info['websocket'].logs =
					toBoolean(env['WEBSOCKETS_LOGS_ENABLED']) && this.accountability.admin
						? {
								allowedLogLevels: getAllowedLogLevels((env['WEBSOCKETS_LOGS_LEVEL'] as string) || 'info'),
							}
						: false;
			} else {
				info['websocket'] = false;
			}

			if (FILE_UPLOADS.MAX_CONCURRENCY && FILE_UPLOADS.MAX_CONCURRENCY !== Infinity) {
				info['uploads'] = {
					maxConcurrency: FILE_UPLOADS.MAX_CONCURRENCY,
				};
			}

			if (RESUMABLE_UPLOADS.ENABLED) {
				info['uploads'] = {
					...info['uploads'],
					tus: true,
					chunkSize: RESUMABLE_UPLOADS.CHUNK_SIZE,
				};
			}
		}

		if (this.accountability?.user || !setupComplete) info['version'] = version;

		return info;
	}

	async health(): Promise<Record<string, any>> {
		const { nanoid } = await import('nanoid');

		const checkID = nanoid(5);

		// Based on https://tools.ietf.org/id/draft-inadarei-api-health-check-05.html#name-componenttype
		type HealthData = {
			status: 'ok' | 'warn' | 'error';
			releaseId: string;
			serviceId: string;
			checks: {
				[service: string]: HealthCheck[];
Repository: directus/directus
File: app/src/main.ts (TypeScript)

Code before:
	console.time('🕓 Application Loaded');

	const app = createApp(App);

	app.use(i18n);
	app.use(createPinia());
	app.use(createHead());

	app.config.errorHandler = (err, vm, info) => {
		const source = getVueComponentName(vm);
		console.warn(`[app-${source}-error] ${info}`);
		console.warn(err);
		return false;
	};

	registerDirectives(app);
	registerComponents(app);
	registerViews(app);

	await loadExtensions();
	registerExtensions(app);

	// Add router after loading of extensions to ensure all routes are registered
	app.use(router);

	// #26411 'inject' occured in pinia store should need vue instance level 'provide'
	useSystem(app);
	app.mount('#app');

	console.timeEnd('🕓 Application Loaded');

	console.group(`%c✨ Project Information`, 'color:DodgerBlue'); // groupCollapsed

	console.info(`%cEnvironment: ${import.meta.env.MODE}`, 'color:DodgerBlue');
	console.groupEnd();

	// Prevent the browser from opening files that are dragged on the window
	window.addEventListener('dragover', (e) => e.preventDefault(), false);
	window.addEventListener('drop', (e) => e.preventDefault(), false);
}


Review comment (suggestion):
```suggestion
	// #26411 'inject' occurred in pinia store should need vue instance level 'provide'
```

Code after:
	console.time('🕓 Application Loaded');

	const app = createApp(App);

	app.use(i18n);
	app.use(createPinia());
	app.use(createHead());

	app.config.errorHandler = (err, vm, info) => {
		const source = getVueComponentName(vm);
		console.warn(`[app-${source}-error] ${info}`);
		console.warn(err);
		return false;
	};

	registerDirectives(app);
	registerComponents(app);
	registerViews(app);

	await loadExtensions();
	registerExtensions(app);

	// Add router after loading of extensions to ensure all routes are registered
	app.use(router);

	// #26411 'inject' occurred in pinia store should need vue instance level 'provide'
	useSystem(app);
	app.mount('#app');

	console.timeEnd('🕓 Application Loaded');

	console.group(`%c✨ Project Information`, 'color:DodgerBlue'); // groupCollapsed

	console.info(`%cEnvironment: ${import.meta.env.MODE}`, 'color:DodgerBlue');
	console.groupEnd();

	// Prevent the browser from opening files that are dragged on the window
	window.addEventListener('dragover', (e) => e.preventDefault(), false);
	window.addEventListener('drop', (e) => e.preventDefault(), false);
}
Repository: directus/directus
File: app/src/interfaces/file/file.vue (Vue)

Code before:

					<VListItem v-if="createAllowed && enableCreate" clickable @click="activeDialog = 'url'">
						<VListItemIcon><VIcon name="link" /></VListItemIcon>
						<VListItemContent>
							{{ $t(file ? 'replace_from_url' : 'import_from_url') }}
						</VListItemContent>
					</VListItem>
				</template>
			</VList>
		</VMenu>

		<DrawerItem
			v-if="file"
			v-model:active="editDrawerActive"
			collection="directus_files"
			:primary-key="file.id"
			:edits="edits"
			:disabled="internalDisabled"
			:non-editable="nonEditable"
			@input="update"
		>
			<template #actions>
				<VButton
					secondary
					rounded
					small
					icon
					:download="file.filename_download"
					:href="getAssetUrl(file.id, { isDownload: true })"
				>
					<VIcon name="download" />
				</VButton>
			</template>
		</DrawerItem>

		<VDialog
			:model-value="activeDialog === 'upload'"
			@esc="activeDialog = null"
			@update:model-value="activeDialog = null"
		>
			<VCard>
				<VCardTitle>{{ $t('upload_from_device') }}</VCardTitle>
				<VCardText>
					<VUpload from-url :folder="folder" @input="onUpload" />
				</VCardText>
				<VCardActions>
					<VButton secondary @click="activeDialog = null">{{ $t('cancel') }}</VButton>
				</VCardActions>
			</VCard>
		</VDialog>


Review comment (suggestion):
Lets standardize this to use the `PrivateViewHeaderBarActionButton`. Looks like I missed the `DrawerItem` uses in https://github.com/directus/directus/pull/26375 :) I'll push up a commit here~

Code after:
					</VListItem>

					<VListItem v-if="createAllowed && enableCreate" clickable @click="activeDialog = 'url'">
						<VListItemIcon><VIcon name="link" /></VListItemIcon>
						<VListItemContent>
							{{ $t(file ? 'replace_from_url' : 'import_from_url') }}
						</VListItemContent>
					</VListItem>
				</template>
			</VList>
		</VMenu>

		<DrawerItem
			v-if="file"
			v-model:active="editDrawerActive"
			collection="directus_files"
			:primary-key="file.id"
			:edits="edits"
			:disabled="internalDisabled"
			:non-editable="nonEditable"
			@input="update"
		>
			<template #actions>
				<PrivateViewHeaderBarActionButton
					icon="download"
					:href="getAssetUrl(file.id, { isDownload: true })"
					:download="file.filename_download"
					secondary
				/>
			</template>
		</DrawerItem>

		<VDialog
			:model-value="activeDialog === 'upload'"
			@esc="activeDialog = null"
			@update:model-value="activeDialog = null"
		>
			<VCard>
				<VCardTitle>{{ $t('upload_from_device') }}</VCardTitle>
				<VCardText>
					<VUpload from-url :folder="folder" @input="onUpload" />
				</VCardText>
				<VCardActions>
					<VButton secondary @click="activeDialog = null">{{ $t('cancel') }}</VButton>
				</VCardActions>
			</VCard>
		</VDialog>

		<DrawerFiles
			v-if="activeDialog === 'choose'"
			:folder="folder"
Repository: directus/directus
File: sdk/src/realtime/composable.ts (TypeScript)

Code before:
				if (!message) continue;

				if (isAuthError(message)) {
					await handleAuthError(message, currentClient);
					state.firstMessage = false;
					continue;
				}

				if (config.heartbeat && message['type'] === 'ping') {
					state.connection.send(pong());
					state.firstMessage = false;
					continue;
				}

				eventHandlers['message'].forEach((handler) => {
					if (state.code === 'open') handler.call(state.connection, message);
				});

				state.firstMessage = false;
			}
		};

		return {
			async isConnected() {
				if (state.code === 'connecting') {
					await state.connection;
				}

				return state.code === 'open';
			},
			async connect() {
				wasManuallyDisconnected = false;

				if (state.code === 'connecting') {
					// wait for the current connection to open
					return await state.connection;
				} else if (state.code !== 'closed') {
					// error state
					throw new Error(`Cannot connect when state is "${state.code}"`);
				}

				const { promise: connectPromise, resolve, reject } = Promise.withResolvers<WebSocketInterface>();

				state = {
					code: 'connecting',
					connection: connectPromise,
				};

				// we need to use THIS here instead of client to access overridden functions
				const self = this as AuthWSClient<Schema>;
				const url = await getSocketUrl(self);

Review comment (suggestion):
the connection may throw an unexpected error

```suggestion
					try {
						await state.connection;
					} catch {
						return false;
					}
```

Code after:
				if (!message) continue;

				if (isAuthError(message)) {
					await handleAuthError(message, currentClient);
					state.firstMessage = false;
					continue;
				}

				if (config.heartbeat && message['type'] === 'ping') {
					state.connection.send(pong());
					state.firstMessage = false;
					continue;
				}

				eventHandlers['message'].forEach((handler) => {
					if (state.code === 'open') handler.call(state.connection, message);
				});

				state.firstMessage = false;
			}
		};

		return {
			/**
			 * Checks if a websocket connection has been established.
			 * Does not check authentication status.
			 */
			async isConnected() {
				if (state.code === 'connecting') {
					try {
						await state.connection;
					} catch {
						return false;
					}
				}

				return state.code === 'open';
			},
			async connect() {
				wasManuallyDisconnected = false;

				if (state.code === 'connecting') {
					// wait for the current connection to open
					return await state.connection;
				} else if (state.code !== 'closed') {
					// error state
					throw new Error(`Cannot connect when state is "${state.code}"`);
				}

				// Eventually update to Promise.withResolvers()
				let resolve!: (value: WebSocketInterface | PromiseLike<WebSocketInterface>) => void;
Repository: directus/directus
File: sdk/src/realtime/composable.ts (TypeScript)

Code before:
				if (isAuthError(message)) {
					await handleAuthError(message, currentClient);
					state.firstMessage = false;
					continue;
				}

				if (config.heartbeat && message['type'] === 'ping') {
					state.connection.send(pong());
					state.firstMessage = false;
					continue;
				}

				eventHandlers['message'].forEach((handler) => {
					if (state.code === 'open') handler.call(state.connection, message);
				});

				state.firstMessage = false;
			}
		};

		return {
			async isConnected() {
				if (state.code === 'connecting') {
					try {
						await state.connection;
					} catch {
						return false;
					}
				}

				return state.code === 'open';
			},
			async connect() {
				wasManuallyDisconnected = false;

				if (state.code === 'connecting') {
					// wait for the current connection to open
					return await state.connection;
				} else if (state.code !== 'closed') {
					// error state
					throw new Error(`Cannot connect when state is "${state.code}"`);
				}

				const { promise: connectPromise, resolve, reject } = Promise.withResolvers<WebSocketInterface>();

				state = {
					code: 'connecting',
					connection: connectPromise,
				};

				// we need to use THIS here instead of client to access overridden functions

Review comment (security):
As discussed, ideally we should have `isConnected`, `isAuthenticated`, and `isReady` to indicate connection, authentication, and full readiness. Each should also have corresponding events to support listening over polling when applicable.

Code after:
				if (isAuthError(message)) {
					await handleAuthError(message, currentClient);
					state.firstMessage = false;
					continue;
				}

				if (config.heartbeat && message['type'] === 'ping') {
					state.connection.send(pong());
					state.firstMessage = false;
					continue;
				}

				eventHandlers['message'].forEach((handler) => {
					if (state.code === 'open') handler.call(state.connection, message);
				});

				state.firstMessage = false;
			}
		};

		return {
			/**
			 * Checks if a websocket connection has been established.
			 * Does not check authentication status.
			 */
			async isConnected() {
				if (state.code === 'connecting') {
					try {
						await state.connection;
					} catch {
						return false;
					}
				}

				return state.code === 'open';
			},
			async connect() {
				wasManuallyDisconnected = false;

				if (state.code === 'connecting') {
					// wait for the current connection to open
					return await state.connection;
				} else if (state.code !== 'closed') {
					// error state
					throw new Error(`Cannot connect when state is "${state.code}"`);
				}

				// Eventually update to Promise.withResolvers()
				let resolve!: (value: WebSocketInterface | PromiseLike<WebSocketInterface>) => void;
				let reject!: (reason?: any) => void;