-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Parse.Cloud.startJob
and Parse.Push.send
not returning status ID when setting Parse Server option directAccess: true
#8766
fix: Parse.Cloud.startJob
and Parse.Push.send
not returning status ID when setting Parse Server option directAccess: true
#8766
Conversation
Thanks for opening this pull request! |
Parse.Job
and Parse.Push
statusId when directAccess
is falseParse.Job
and Parse.Push
not returning status ID when Parse Server option directAccess: true
Parse.Job
and Parse.Push
not returning status ID when Parse Server option directAccess: true
Parse.Cloud.startJob
and Parse.Push
not returning status ID when Parse Server option directAccess: true
Parse.Cloud.startJob
and Parse.Push
not returning status ID when Parse Server option directAccess: true
Parse.Cloud.startJob
and Parse.Push.send
not returning status ID when Parse Server option directAccess: true
Parse.Cloud.startJob
and Parse.Push.send
not returning status ID when Parse Server option directAccess: true
Parse.Cloud.startJob
and Parse.Push.send
not returning status ID when setting Parse Server option directAccess: true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR provides a quick fix, but I'm not sure it's a sustainable fix as it duplicates codes across 2 repositories which will be difficult to maintain.
The underlying issue seems to be that the REST controller makes the decision to convert response headers as needed for methods (runJob, sendPush). This has the pattern of a circular dependency where the method depends on the REST controller returning the headers and the REST controller depends on the methods to decide which headers to return.
The REST controller should be caller-agnostic. If there are any response headers it should provide them to the caller, so the caller decides which headers to process further. Just like it does with the response body.
Just imagine that in the future runJob
should return the status ID and the value from another header. We would need to start building the response object of runJob
in the REST controller which doesn't make sense.
Could the REST Controllers simply pass an additional headers
object back to the calling function? This way the runJob
can pick up the header and compose the response. This would avoid code duplication, work regardless of REST controller and it provides us more flexibility in the future.
We would have to reuse |
Can this PR be closed thanks to parse-community/Parse-SDK-JS#2033? |
We have to wait for parse-community/Parse-SDK-JS#2033 to be released for the test cases here to pass. I've updated the PR to pass headers in the ParseRestController similarly to the JS RestController. The JS SDK will handle the headers. |
That's a great fix; so should be able to merge this in Nov when we'll do a stable release of the JS SDK with that fix included. |
parse-community/Parse-SDK-JS#2033 has been merged, but we cannot use the latest Parse JS SDK release in Parse Server due to #8818; so I believe we need fix #8818 first before we can merge this PR here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parse-community/Parse-SDK-JS#2033 has been merged and it's in the JS SDK that PS7 is using, but tests fail.
@mtrezza Tests have been updated. This is ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## alpha #8766 +/- ##
==========================================
+ Coverage 94.13% 94.16% +0.02%
==========================================
Files 186 186
Lines 14687 14724 +37
==========================================
+ Hits 13826 13865 +39
+ Misses 861 859 -2 ☔ View full report in Codecov by Sentry. |
# [7.1.0-alpha.6](7.1.0-alpha.5...7.1.0-alpha.6) (2024-04-14) ### Bug Fixes * `Parse.Cloud.startJob` and `Parse.Push.send` not returning status ID when setting Parse Server option `directAccess: true` ([#8766](#8766)) ([5b0efb2](5b0efb2))
🎉 This change has been released in version 7.1.0-alpha.6 |
# [7.1.0-beta.1](7.0.0...7.1.0-beta.1) (2024-06-30) ### Bug Fixes * `Parse.Cloud.startJob` and `Parse.Push.send` not returning status ID when setting Parse Server option `directAccess: true` ([#8766](#8766)) ([5b0efb2](5b0efb2)) * `Required` option not handled correctly for special fields (File, GeoPoint, Polygon) on GraphQL API mutations ([#8915](#8915)) ([907ad42](907ad42)) * Facebook Limited Login not working due to incorrect domain in JWT validation ([#9122](#9122)) ([9d0bd2b](9d0bd2b)) * Live query throws error when constraint `notEqualTo` is set to `null` ([#8835](#8835)) ([11d3e48](11d3e48)) * Parse Server option `extendSessionOnUse` not working for session lengths < 24 hours ([#9113](#9113)) ([0a054e6](0a054e6)) * Rate limiting can fail when using Parse Server option `rateLimit.redisUrl` with clusters ([#8632](#8632)) ([c277739](c277739)) * SQL injection when using Parse Server with PostgreSQL; fixes security vulnerability [GHSA-c2hr-cqg6-8j6r](GHSA-c2hr-cqg6-8j6r) ([#9167](#9167)) ([2edf1e4](2edf1e4)) ### Features * Add `silent` log level for Cloud Code ([#8803](#8803)) ([5f81efb](5f81efb)) * Add server security check status `security.enableCheck` to Features Router ([#8679](#8679)) ([b07ec15](b07ec15)) * Prevent Parse Server start in case of unknown option in server configuration ([#8987](#8987)) ([8758e6a](8758e6a)) * Upgrade to @parse/push-adapter 6.0.0 ([#9066](#9066)) ([18bdbf8](18bdbf8)) * Upgrade to @parse/push-adapter 6.2.0 ([#9127](#9127)) ([ca20496](ca20496)) * Upgrade to Parse JS SDK 5.2.0 ([#9128](#9128)) ([665b8d5](665b8d5))
🎉 This change has been released in version 7.1.0-beta.1 |
# [7.1.0](7.0.0...7.1.0) (2024-06-30) ### Bug Fixes * `Parse.Cloud.startJob` and `Parse.Push.send` not returning status ID when setting Parse Server option `directAccess: true` ([#8766](#8766)) ([5b0efb2](5b0efb2)) * `Required` option not handled correctly for special fields (File, GeoPoint, Polygon) on GraphQL API mutations ([#8915](#8915)) ([907ad42](907ad42)) * Facebook Limited Login not working due to incorrect domain in JWT validation ([#9122](#9122)) ([9d0bd2b](9d0bd2b)) * Live query throws error when constraint `notEqualTo` is set to `null` ([#8835](#8835)) ([11d3e48](11d3e48)) * Parse Server option `extendSessionOnUse` not working for session lengths < 24 hours ([#9113](#9113)) ([0a054e6](0a054e6)) * Rate limiting can fail when using Parse Server option `rateLimit.redisUrl` with clusters ([#8632](#8632)) ([c277739](c277739)) * SQL injection when using Parse Server with PostgreSQL; fixes security vulnerability [GHSA-c2hr-cqg6-8j6r](GHSA-c2hr-cqg6-8j6r) ([#9167](#9167)) ([2edf1e4](2edf1e4)) ### Features * Add `silent` log level for Cloud Code ([#8803](#8803)) ([5f81efb](5f81efb)) * Add server security check status `security.enableCheck` to Features Router ([#8679](#8679)) ([b07ec15](b07ec15)) * Prevent Parse Server start in case of unknown option in server configuration ([#8987](#8987)) ([8758e6a](8758e6a)) * Upgrade to @parse/push-adapter 6.0.0 ([#9066](#9066)) ([18bdbf8](18bdbf8)) * Upgrade to @parse/push-adapter 6.2.0 ([#9127](#9127)) ([ca20496](ca20496)) * Upgrade to Parse JS SDK 5.2.0 ([#9128](#9128)) ([665b8d5](665b8d5))
🎉 This change has been released in version 7.1.0 |
Pull Request
Issue
Calling
Parse.Cloud.startJob
orParse.Push.send
from a Cloud Code function while directAccess is false doesn't return the status Id.Closes: #8770
Related: parse-community/Parse-SDK-JS#2033
Approach
Support status Id headers in the direct access REST Controller
Tasks