Skip to content

Commit

Permalink
Merge pull request #181 from jaydenseric/fix-issue-175
Browse files Browse the repository at this point in the history
Ensure `GraphQLUpload` scalar errors when parsing invalid values, fixing #175 .
  • Loading branch information
jaydenseric committed Jan 19, 2020
2 parents 2829732 + faa5db2 commit 15bd494
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 54 deletions.
1 change: 1 addition & 0 deletions changelog.md
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion 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
Expand Down Expand Up @@ -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)
},
Expand Down
25 changes: 15 additions & 10 deletions lib/Upload.js
@@ -1,48 +1,53 @@
'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<FileUpload>}
* @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

resolve(file)
}

/**
* 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
})
Expand Down
2 changes: 2 additions & 0 deletions lib/index.js
Expand Up @@ -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
Expand Down Expand Up @@ -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<GraphQLOperation | Array<GraphQLOperation>>} GraphQL operation or batch of operations for a GraphQL server to consume (usually as the request body).
* @see [`processRequest`]{@link processRequest}.
*/

/**
Expand Down
15 changes: 10 additions & 5 deletions lib/processRequest.js
Expand Up @@ -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}
Expand Down Expand Up @@ -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(
Expand Down
45 changes: 44 additions & 1 deletion readme.md
Expand Up @@ -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)
Expand Down Expand Up @@ -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&lt;[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.
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -261,6 +300,10 @@ Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-m

**Returns:** Promise&lt;[GraphQLOperation](#type-graphqloperation) | Array&lt;[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
Expand Down
24 changes: 23 additions & 1 deletion 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(
() => {
Expand Down

0 comments on commit 15bd494

Please sign in to comment.