Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Push Status API #1004

Merged
merged 8 commits into from
Mar 17, 2016
Merged

Push Status API #1004

merged 8 commits into from
Mar 17, 2016

Conversation

flovilmart
Copy link
Contributor

  • Stores a _PushStatus object when sending a push
  • Updates the _PushStatus object when sent has been called (set running)

Nits:

  • Centralizes the system classes list

@codecov-io
Copy link

Current coverage is 91.43%

Merging #1004 into master will decrease coverage by -0.19% as of 2bc3aed

@@            master   #1004   diff @@
======================================
  Files           73      74     +1
  Stmts         4432    4509    +77
  Branches       888     904    +16
  Methods          0       0       
======================================
+ Hit           4061    4123    +62
- Partial         10      11     +1
- Missed         361     375    +14

Review entire Coverage Diff as of 2bc3aed

Powered by Codecov. Updated on successful CI builds.

@drew-gross
Copy link
Contributor

The system classes list is only supposed to be for things that are accessible to clients and contain Parse Objects. You don't want clients writing directly to/from _PushStatus. Thats why _SCHEMA isn't included in most places, and there is a TODO for removing it from that one place.

@flovilmart
Copy link
Contributor Author

@drew-gross ok, will RestWrite/RestQuery work correctly then?

@drew-gross
Copy link
Contributor

I don't think so. If you want to access it through RestWrite/RestQuery we should add a concept of classes that are managed by Parse Server, but aren't part of the client interface. _SCHEMA can be part of it as well as _PushStatus, _Audience, _QueryToolQuery, and other stuff that is internal to Parse Server.

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@flovilmart
Copy link
Contributor Author

I wanna use as much as possible of the RestWrite for updating the _PushStatus, that will be easier and cleaner.

@drew-gross
Copy link
Contributor

OK maybe we could add it as a parse object, but locked down by default to master key only. I think how we handle this is actually a pretty major point that will affect the future of Parse Server in a big way, so we really want to get this right.

@flovilmart
Copy link
Contributor Author

Yes definitely, we can discuss different architecture options offline as I want to get it right too, and have something that help the codebase scale easily if we need more things like that in the future. I could do CLP on it.

@flovilmart
Upgrade APNS to use HTTP/2
dad50d1 
- uses universal certificate
- removes tests logs
- standardized returned promises from APNS and GCM to something usable in _PushStatus
@flovilmart
Fix tests, uses fork of http2 to support node 4.3.0 without ALPN
05baf36 
- the push server don't send TLS NPN and makes the http2 lib break
- the fork forces the next protocol negociation to h2 as we're sure
@flovilmart
Adds support to store push results
7c387e1
@flovilmart
Copy link
Contributor Author

I may revert to use apn as we'd require node >= 5.0 to properly use the HTTP/2 endpoint as the server don't advertise the TLS NPN. That being said, because of the callback structure the code will end-up messy. What do you think @drew-gross ?

@flovilmart flovilmart changed the title WIP: Push Status API Push Status API Mar 14, 2016
flovilmart
flovilmart reviewed Mar 14, 2016
View reviewed changes
spec/ParseACL.spec.js
@@ -1153,7 +1153,6 @@ describe('Parse.ACL', () => {
var query = new Parse.Query("TestClassMasterACL");
return query.find();
}).then((results) => {
console.log(JSON.stringify(results[0]));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removes the undefined log in the tests

@flovilmart
Copy link
Contributor Author

btw, I removed HTTP/2 API as we'd need to bump to engine >= 5.0, maybe later...

flovilmart added a commit that referenced this pull request Mar 17, 2016
@flovilmart
Merge pull request #1004 from ParsePlatform/flovilmart.PushStatus
9624970 
Push Status API
@flovilmart flovilmart merged commit 9624970 into master Mar 17, 2016
@flovilmart flovilmart deleted the flovilmart.PushStatus branch March 17, 2016 12:16
@gfosco gfosco mentioned this pull request Mar 21, 2016
Closed
@wangmengyan95 wangmengyan95 mentioned this pull request Mar 24, 2016
Closed
@khirr
Copy link

khirr commented Apr 28, 2016

Hello,
Is possible to use beforeSave, afterSave callbacks for _PushStatus in CloudCode?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@drew-gross drew-gross

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@flovilmart @codecov-io @drew-gross @facebook-github-bot @khirr @gfosco