parse-server
parse-server copied to clipboard
fix: file parsing defaultGraohQLTypes.js
Problem with executing GraphQL Cloud code.
Instead of parsing value object type File object is type of Upload
New Pull Request Checklist
- [x] I am not disclosing a vulnerability.
- [x] I am creating this PR in reference to an issue.
Issue Description
This PR solves problems with GraphQL input value of type File. After upgrading from version 4.2.0 to 4.3.0. All migrations calling cloud code functions with inputs of type File ends with:
{ "errors": [ { "message": "Variable \"$input\" got invalid value { resolve: [function], reject: [function], promise: {}, file: { filename: \"ff.jpg\", mimetype: \"image/jpeg\", encoding: \"7bit\", createReadStream: ...Expected type \"File\". [object Object] is not a valid File
Related issue: #7684
Approach
After debbuging the code and searching for where parts throwing exception. Found validation problem in src/GraphQL/loaders/defaultGraphQLTypes.js. Parts expecting an object instance of File receives Upload. Also __Type on object is undefined. So I access type name trough constructor.name. After these changes code works as it was before upgrading to 4.3.0.
TODOs before merging
- [ ] Add tests
- [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)
Thanks for opening this pull request!
- ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.
Proposed to fix: https://github.com/parse-community/parse-server/issues/7684
@brocklj could you please fill out the template?
@brocklj could you please fill out the template?
How long could take fix this problem into version 4.3.x?
I removed the TODOs that don't apply to this PR, only the test is missing. Could you please add a test to make sure this bug can't occur again?
How long could take fix this problem into version 4.3.x?
The process is to first fix a bug in the alpha branch, then backport it to the 4.x branch. So the first step is to determine whether this bug also exists in alpha branch:
- if it does, then this PR's target branch need to change to alpha, and after merge we may backport the fix into 4.x
- if it does not, then we need to look how this bug has been fixed in alpha and either backport it the same way into 4.x or consider an individual fix for the 4.x branch.
To answer your question exactly, we won't port this into a 4.3.x release (based on the 4.3 version), but we may port it into a 4.x release (based on the latest 4.x version). Portability depends on the required effort.
if it does, then this PR's target branch need to change to alpha, and after merge we may backport the fix into 4.x
Yes, it does. Tested on release 4.10.4.
Yes, it does. Tested on release 4.10.4.
The alpha branch is way ahead of 4.10.4. Please try with 5.0.0-alpha.5 which is the latest alpha release.
Tested on 5.0.0-alpha.5.
with same error result Variable \"$input\" got invalid value { resolve: [function], reject: [function], promise: {}, file: { filename: \"ff.jpg\", mimetype: \"image/jpeg\", encoding: \"7bit\", createReadStream: [function createReadStream] } } at \"input[0]\"; Expected type \"File\". [object Object] is not a valid File"
Great, thanks for testing. I changed the target branch of this PR to alpha, so the fix will be for that branch. Could you add a test that fails without the fix and passes with the fix? This will prevent future changes from introducing the same bug again. You can just copy/paste a similar test and modify it.
The tests do not pass. Either the tests need to be adapted, or the fix does not work as intended. I suggest to remove the fix for now and just add a failing test to demonstrate the issue. Then add a fix to make the new test pass.
Hi @brocklj could you provide the GraphQL request that you use in your migrations ? And how you inject your files into the graphQL request ? Feel free to provide your complete request + how you call it ?
Hi @brocklj could you provide the GraphQL request that you use in your migrations ? And how you inject your files into the graphQL request ? Feel free to provide your complete request + how you call it ?
While using apollo-upload-client": "^14.1.3",
`const UPLOAD_MERCHDET_IMAGE_QUERY = gql
mutation uploadMerchdetImage($input: [File], $mch_id: Int) {
uploadMerchdetImage(input: $input, mch_id: $mch_id) {
img_id
img_url
}
}
;
const [uploadMechdetImage] = useMutation(UPLOAD_MERCHDET_IMAGE_QUERY, {
onCompleted(data) {
inputRef.current.value = null;
setIsUploadInProgress(false);
setMerchdetState({
...merchdetState,
mch_img_id: data.uploadMerchdetImage.img_id,
img_url: data.uploadMerchdetImage.img_url
});
}
});`
if (validity.valid && !imageIsNotValid) {
setIsUploadInProgress(true);
await uploadMechdetImage({
variables: {
input: [file],
mch_id
}
});
}
};`
The request looks like.
operations: {"operationName":"uploadMerchdetImage","variables":{"input":null,"mch_id":881},"query":"mutation uploadMerchdetImage($input: File, $mch_id: Int) {\n uploadMerchdetImage(input: $input, mch_id: $mch_id) {\n img_id\n img_url\n __typename\n }\n}\n"}
map: {"1":["variables.input"]}
1: (binary)`
and error message from response:
Variable "$input" got invalid value { resolve: [function], reject: [function], promise: {}, file: { filename: "1634658697301.jpg", mimetype: "image/jpeg", encoding: "7bit", createReadStream: [function createReadStream] } }; Expected type "File". [object Object] is not a valid File
Hi @brocklj I see that the fix is super simple. Could rebase this PR on alpha and add the failing test at the bottom of the ParseGraphQLServer.spec.js
You have an example in ParseGraphQLServer.spec.js on how to set up a test with custom definitions and upload a file using multipart upload 🙂
Once it's covered, we can merge 👍
Hi @brocklj Parse Server has switched to GraphQL Yoga with new graphql upload/file handling.
Could you confirm that the issue is still present in Parse Server last alpha version?
Also, could you provide a failing test, it will help to avoid a regression in the future (like the one between 4.2 -> 4.3)
Tests are failing, there seem to be issues that need to be resolved; removed pending review. This PR is stale for some time; if there is no further activity we'll go ahead and close it.