diff --git a/changelog.md b/changelog.md index fc87535..be3cc58 100644 --- a/changelog.md +++ b/changelog.md @@ -9,6 +9,7 @@ - Replaced the [`tap`](https://npm.im/tap) dev dependency with [`test-director`](https://npm.im/test-director), [`coverage-node`](https://npm.im/coverage-node), and [`hard-rejection`](https://npm.im/hard-rejection) to improve the dev experience and reduce the dev install size by ~75.7 MB. These new dev dependencies require Node.js v10+. - Reorganized files. This is only a breaking change for projects using undocumented deep imports. - Removed now redundant Node.js version compatibility logic in the `processRequest` function. +- The `processRequest` function now places references to instances of the now exported and documented `Upload` class in the GraphQL operation for the `GraphQLUpload` scalar to derive its value, and the `GraphQLUpload` scalar now throws a `GraphQLError` when it parses an invalid value, fixing [#175](https://github.com/jaydenseric/graphql-upload/issues/175) via [#181](https://github.com/jaydenseric/graphql-upload/pull/181). - The `GraphQLUpload` scalar `parseLiteral` and `serialize` methods now throw `GraphQLError` (instead of `Error`) instances, with tweaked messages. ### Patch diff --git a/lib/GraphQLUpload.js b/lib/GraphQLUpload.js index 2ea8406..da2f8a1 100644 --- a/lib/GraphQLUpload.js +++ b/lib/GraphQLUpload.js @@ -1,6 +1,7 @@ 'use strict' const { GraphQLScalarType, GraphQLError } = require('graphql') +const Upload = require('./Upload') /** * A GraphQL `Upload` scalar that can be used in a @@ -60,7 +61,10 @@ const { GraphQLScalarType, GraphQLError } = require('graphql') module.exports = new GraphQLScalarType({ name: 'Upload', description: 'The `Upload` scalar type represents a file upload.', - parseValue: value => value, + parseValue(value) { + if (value instanceof Upload) return value.promise + throw new GraphQLError('Upload value invalid.') + }, parseLiteral(ast) { throw new GraphQLError('Upload literal unsupported.', ast) }, diff --git a/lib/Upload.js b/lib/Upload.js index 63cc47f..c3e2dce 100644 --- a/lib/Upload.js +++ b/lib/Upload.js @@ -1,36 +1,41 @@ 'use strict' /** - * An expected file upload. + * A file expected to be uploaded as it has been declared in the `map` field of + * a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). + * The [`processRequest`]{@link processRequest} function places references to an + * instance of this class wherever the file is expected in the + * [GraphQL operation]{@link GraphQLOperation}. The + * [`Upload` scalar]{@link GraphQLUpload} derives it’s value from the + * [`promise`]{@link Upload#promise} property. * @kind class * @name Upload - * @ignore */ module.exports = class Upload { constructor() { /** - * Promise that resolves file upload details. + * Promise that resolves file upload details. This should only be utilized + * by [`GraphQLUpload`]{@link GraphQLUpload}. * @kind member * @name Upload#promise * @type {Promise} - * @ignore */ this.promise = new Promise((resolve, reject) => { /** - * Resolves the upload promise with the file upload details. + * Resolves the upload promise with the file upload details. This should + * only be utilized by [`processRequest`]{@link processRequest}. * @kind function * @name Upload#resolve * @param {FileUpload} file File upload details. - * @ignore */ this.resolve = file => { /** * The file upload details, available when the - * [upload promise]{@link Upload#promise} resolves. + * [upload promise]{@link Upload#promise} resolves. This should only be + * utilized by [`processRequest`]{@link processRequest}. * @kind member * @name Upload#file * @type {undefined|FileUpload} - * @ignore */ this.file = file @@ -38,11 +43,11 @@ module.exports = class Upload { } /** - * Rejects the upload promise with an error. + * Rejects the upload promise with an error. This method should only be + * utilized by [`processRequest`]{@link processRequest}. * @kind function * @name Upload#reject * @param {object} error Error instance. - * @ignore */ this.reject = reject }) diff --git a/lib/index.js b/lib/index.js index ee272bc..5525038 100644 --- a/lib/index.js +++ b/lib/index.js @@ -4,6 +4,7 @@ exports.GraphQLUpload = require('./GraphQLUpload') exports.processRequest = require('./processRequest') exports.graphqlUploadKoa = require('./graphqlUploadKoa') exports.graphqlUploadExpress = require('./graphqlUploadExpress') +exports.Upload = require('./Upload') /** * File upload details that are only available after the file’s field in the @@ -40,6 +41,7 @@ exports.graphqlUploadExpress = require('./graphqlUploadExpress') * @param {ServerResponse} response [Node.js HTTP server response instance](https://nodejs.org/api/http.html#http_class_http_serverresponse). * @param {ProcessRequestOptions} [options] Options for processing the request. * @returns {Promise>} GraphQL operation or batch of operations for a GraphQL server to consume (usually as the request body). + * @see [`processRequest`]{@link processRequest}. */ /** diff --git a/lib/processRequest.js b/lib/processRequest.js index ce56ea0..aa374cd 100644 --- a/lib/processRequest.js +++ b/lib/processRequest.js @@ -11,11 +11,16 @@ const ignoreStream = require('./ignoreStream') /** * Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). - * Errors are created with [`http-errors`](https://npm.im/http-errors) to assist - * in sending responses with appropriate HTTP status codes. Used in + * It parses the `operations` and `map` fields to create an + * [`Upload`]{@link Upload} instance for each expected file upload, placing + * references wherever the file is expected in the + * [GraphQL operation]{@link GraphQLOperation} for the + * [`Upload` scalar]{@link GraphQLUpload} to derive it’s value. Errors are + * created with [`http-errors`](https://npm.im/http-errors) to assist in + * sending responses with appropriate HTTP status codes. Used in * [`graphqlUploadExpress`]{@link graphqlUploadExpress} and - * [`graphqlUploadKoa`]{@link graphqlUploadKoa} and can be used to - * create custom middleware. + * [`graphqlUploadKoa`]{@link graphqlUploadKoa} and can be used to create + * custom middleware. * @kind function * @name processRequest * @type {ProcessRequestFunction} @@ -211,7 +216,7 @@ module.exports = function processRequest( ) try { - operationsPath.set(path, map.get(fieldName).promise) + operationsPath.set(path, map.get(fieldName)) } catch (error) { return exit( createError( diff --git a/readme.md b/readme.md index eb446b2..baf9e3d 100644 --- a/readme.md +++ b/readme.md @@ -66,6 +66,11 @@ The [GraphQL multipart request spec](https://github.com/jaydenseric/graphql-mult ### Table of contents - [class GraphQLUpload](#class-graphqlupload) +- [class Upload](#class-upload) + - [Upload instance method reject](#upload-instance-method-reject) + - [Upload instance method resolve](#upload-instance-method-resolve) + - [Upload instance property file](#upload-instance-property-file) + - [Upload instance property promise](#upload-instance-property-promise) - [function graphqlUploadExpress](#function-graphqluploadexpress) - [function graphqlUploadKoa](#function-graphqluploadkoa) - [function processRequest](#function-processrequest) @@ -133,6 +138,40 @@ _A manually constructed schema with an image upload mutation._ --- +### class Upload + +A file expected to be uploaded as it has been declared in the `map` field of a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). The [`processRequest`](#function-processrequest) function places references to an instance of this class wherever the file is expected in the [GraphQL operation](#type-graphqloperation). The [`Upload` scalar](#class-graphqlupload) derives it’s value from the [`promise`](#upload-instance-property-promise) property. + +#### Upload instance method reject + +Rejects the upload promise with an error. This method should only be utilized by [`processRequest`](#function-processrequest). + +| Parameter | Type | Description | +| :-------- | :----- | :-------------- | +| `error` | object | Error instance. | + +#### Upload instance method resolve + +Resolves the upload promise with the file upload details. This should only be utilized by [`processRequest`](#function-processrequest). + +| Parameter | Type | Description | +| :-------- | :----------------------------- | :------------------- | +| `file` | [FileUpload](#type-fileupload) | File upload details. | + +#### Upload instance property file + +The file upload details, available when the [upload promise](#upload-instance-property-promise) resolves. This should only be utilized by [`processRequest`](#function-processrequest). + +**Type:** `undefined` | [FileUpload](#type-fileupload) + +#### Upload instance property promise + +Promise that resolves file upload details. This should only be utilized by [`GraphQLUpload`](#class-graphqlupload). + +**Type:** Promise<[FileUpload](#type-fileupload)> + +--- + ### function graphqlUploadExpress Creates [Express](https://expressjs.com) middleware that processes [GraphQL multipart requests](https://github.com/jaydenseric/graphql-multipart-request-spec) using [`processRequest`](#function-processrequest), ignoring non-multipart requests. It sets the request body to be [similar to a conventional GraphQL POST request](#type-graphqloperation) for following GraphQL middleware to consume. @@ -199,7 +238,7 @@ _Basic [`graphql-api-koa`](https://npm.im/graphql-api-koa) setup._ ### function processRequest -Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). Errors are created with [`http-errors`](https://npm.im/http-errors) to assist in sending responses with appropriate HTTP status codes. Used in [`graphqlUploadExpress`](#function-graphqluploadexpress) and [`graphqlUploadKoa`](#function-graphqluploadkoa) and can be used to create custom middleware. +Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). It parses the `operations` and `map` fields to create an [`Upload`](#class-upload) instance for each expected file upload, placing references wherever the file is expected in the [GraphQL operation](#type-graphqloperation) for the [`Upload` scalar](#class-graphqlupload) to derive it’s value. Errors are created with [`http-errors`](https://npm.im/http-errors) to assist in sending responses with appropriate HTTP status codes. Used in [`graphqlUploadExpress`](#function-graphqluploadexpress) and [`graphqlUploadKoa`](#function-graphqluploadkoa) and can be used to create custom middleware. **Type:** [ProcessRequestFunction](#type-processrequestfunction) @@ -261,6 +300,10 @@ Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-m **Returns:** Promise<[GraphQLOperation](#type-graphqloperation) | Array<[GraphQLOperation](#type-graphqloperation)>> — GraphQL operation or batch of operations for a GraphQL server to consume (usually as the request body). +#### See + +- [`processRequest`](#function-processrequest). + --- ### type ProcessRequestOptions diff --git a/test/lib/GraphQLUpload.test.js b/test/lib/GraphQLUpload.test.js index 89fcc48..4524e87 100644 --- a/test/lib/GraphQLUpload.test.js +++ b/test/lib/GraphQLUpload.test.js @@ -1,10 +1,32 @@ 'use strict' -const { throws } = require('assert') +const { doesNotThrow, throws } = require('assert') const { parseValue } = require('graphql') const GraphQLUpload = require('../../lib/GraphQLUpload') +const Upload = require('../../lib/Upload') module.exports = tests => { + tests.add('`GraphQLUpload` scalar `parseValue` with a valid value.', () => { + doesNotThrow(() => { + GraphQLUpload.parseValue(new Upload()) + }) + }) + + tests.add( + '`GraphQLUpload` scalar `parseValue` with an invalid value.', + () => { + throws( + () => { + GraphQLUpload.parseValue(true) + }, + { + name: 'GraphQLError', + message: 'Upload value invalid.' + } + ) + } + ) + tests.add('`GraphQLUpload` scalar `parseLiteral`.', () => { throws( () => { diff --git a/test/lib/processRequest.test.js b/test/lib/processRequest.test.js index f42d0cd..b08b6fc 100644 --- a/test/lib/processRequest.test.js +++ b/test/lib/processRequest.test.js @@ -5,6 +5,7 @@ const http = require('http') const FormData = require('form-data') const { ReadStream } = require('fs-capacitor') const fetch = require('node-fetch') +const Upload = require('../../lib/Upload') const processRequest = require('../../lib/processRequest') const abortingMultipartRequest = require('../abortingMultipartRequest') const listen = require('../listen') @@ -18,9 +19,9 @@ module.exports = tests => { try { const operation = await processRequest(request, response) - ok(operation.variables.file instanceof Promise) + ok(operation.variables.file instanceof Upload) - const upload = await operation.variables.file + const upload = await operation.variables.file.promise strictEqual(upload.filename, 'a.txt') strictEqual(upload.mimetype, 'text/plain') @@ -61,9 +62,9 @@ module.exports = tests => { try { const operations = await processRequest(request, response) - ok(operations[0].variables.file instanceof Promise) + ok(operations[0].variables.file instanceof Upload) - const uploadA = await operations[0].variables.file + const uploadA = await operations[0].variables.file.promise strictEqual(uploadA.filename, 'a.txt') strictEqual(uploadA.mimetype, 'text/plain') @@ -74,9 +75,9 @@ module.exports = tests => { ok(streamA instanceof ReadStream) strictEqual(await streamToString(streamA), 'a') - ok(operations[1].variables.file instanceof Promise) + ok(operations[1].variables.file instanceof Upload) - const uploadB = await operations[1].variables.file + const uploadB = await operations[1].variables.file.promise strictEqual(uploadB.filename, 'b.txt') strictEqual(uploadB.mimetype, 'text/plain') @@ -127,13 +128,13 @@ module.exports = tests => { try { const operation = await processRequest(request, response) - ok(operation.variables.files[0] instanceof Promise) - ok(operation.variables.files[1] instanceof Promise) + ok(operation.variables.files[0] instanceof Upload) + ok(operation.variables.files[1] instanceof Upload) strictEqual(operation.variables.files[0], operation.variables.files[1]) const [upload1, upload2] = await Promise.all([ - operation.variables.files[0], - operation.variables.files[1] + operation.variables.files[0].promise, + operation.variables.files[1].promise ]) strictEqual(upload1, upload2) @@ -192,9 +193,9 @@ module.exports = tests => { try { const operation = await processRequest(request, response) - ok(operation.variables.fileB instanceof Promise) + ok(operation.variables.fileB instanceof Upload) - const uploadB = await operation.variables.fileB + const uploadB = await operation.variables.fileB.promise const streamB = uploadB.createReadStream() await streamToString(streamB) @@ -238,9 +239,9 @@ module.exports = tests => { try { const operation = await processRequest(request, response) - ok(operation.variables.file instanceof Promise) + ok(operation.variables.file instanceof Upload) - const upload = await operation.variables.file + const upload = await operation.variables.file.promise strictEqual(upload.filename, 'a.txt') strictEqual(upload.mimetype, 'text/plain') @@ -285,8 +286,8 @@ module.exports = tests => { try { const operation = await processRequest(request, response) - ok(operation.variables.file instanceof Promise) - await rejects(() => operation.variables.file, { + ok(operation.variables.file instanceof Upload) + await rejects(() => operation.variables.file.promise, { name: 'BadRequestError', message: 'File missing in the request.', status: 400, @@ -375,9 +376,9 @@ module.exports = tests => { maxFiles: 2 }) - ok(operation.variables.files[0] instanceof Promise) + ok(operation.variables.files[0] instanceof Upload) - const uploadA = await operation.variables.files[0] + const uploadA = await operation.variables.files[0].promise strictEqual(uploadA.filename, 'a.txt') strictEqual(uploadA.mimetype, 'text/plain') @@ -387,8 +388,8 @@ module.exports = tests => { ok(streamA instanceof ReadStream) strictEqual(await streamToString(streamA), 'a') - ok(operation.variables.files[1] instanceof Promise) - await rejects(() => operation.variables.files[1], { + ok(operation.variables.files[1] instanceof Upload) + await rejects(() => operation.variables.files[1].promise, { name: 'PayloadTooLargeError', message: '2 max file uploads exceeded.', status: 413, @@ -439,9 +440,9 @@ module.exports = tests => { maxFileSize: 1 }) - ok(operation.variables.files[0] instanceof Promise) + ok(operation.variables.files[0] instanceof Upload) - const { createReadStream } = await operation.variables.files[0] + const { createReadStream } = await operation.variables.files[0].promise await throws( () => { @@ -455,9 +456,9 @@ module.exports = tests => { } ) - ok(operation.variables.files[0] instanceof Promise) + ok(operation.variables.files[0] instanceof Upload) - const uploadB = await operation.variables.files[1] + const uploadB = await operation.variables.files[1].promise strictEqual(uploadB.filename, 'b.txt') strictEqual(uploadB.mimetype, 'text/plain') @@ -570,9 +571,9 @@ module.exports = tests => { const operation = await processRequest(request, response) const testUploadA = async () => { - ok(operation.variables.fileA instanceof Promise) + ok(operation.variables.fileA instanceof Upload) - const upload = await operation.variables.fileA + const upload = await operation.variables.fileA.promise strictEqual(upload.filename, 'a.txt') strictEqual(upload.mimetype, 'text/plain') @@ -585,9 +586,9 @@ module.exports = tests => { } const testUploadB = async () => { - ok(operation.variables.fileB instanceof Promise) + ok(operation.variables.fileB instanceof Upload) - const upload = await operation.variables.fileB + const upload = await operation.variables.fileB.promise strictEqual(upload.filename, 'b.txt') strictEqual(upload.mimetype, 'text/plain') @@ -615,8 +616,8 @@ module.exports = tests => { } const testUploadC = async () => { - ok(operation.variables.fileC instanceof Promise) - await rejects(() => operation.variables.fileC, { + ok(operation.variables.fileC instanceof Upload) + await rejects(() => operation.variables.fileC.promise, { name: 'BadRequestError', message: 'Request disconnected during file upload stream parsing.', @@ -712,9 +713,9 @@ module.exports = tests => { }) const testUploadA = async () => { - ok(operation.variables.fileA instanceof Promise) + ok(operation.variables.fileA instanceof Upload) - const upload = await operation.variables.fileA + const upload = await operation.variables.fileA.promise strictEqual(upload.filename, 'a.txt') strictEqual(upload.mimetype, 'text/plain') @@ -730,9 +731,9 @@ module.exports = tests => { } const testUploadB = async () => { - ok(operation.variables.fileB instanceof Promise) + ok(operation.variables.fileB instanceof Upload) - const upload = await operation.variables.fileB + const upload = await operation.variables.fileB.promise strictEqual(upload.filename, 'b.txt') strictEqual(upload.mimetype, 'text/plain') @@ -747,8 +748,8 @@ module.exports = tests => { } const testUploadC = async () => { - ok(operation.variables.fileC instanceof Promise) - await rejects(() => operation.variables.fileC, { + ok(operation.variables.fileC instanceof Upload) + await rejects(() => operation.variables.fileC.promise, { name: 'BadRequestError', message: 'Request disconnected during file upload stream parsing.',