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.

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

Store and forward #2423

Closed
wants to merge 6 commits into from
Closed

Store and forward #2423

wants to merge 6 commits into from

Conversation

moxie0
Copy link
Contributor

@moxie0 moxie0 commented Feb 7, 2015

No description provided.

1) Make credentials dynamic, so that registration changes are
   reflected in existing service threads.

2) Dedicate a single thread to websocket message retrieval,
   rather than using an executor.  Wrap the whole thing in
   catch blocks, so that okhttp's internal problems don't
   take out our thread.
private static class GcmRegistrationId {
private String gcmRegistrationId;
private boolean webSocketChannel;
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't look like it's used anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a JSON POJO. It's transmitted to the server.

Copy link
Contributor

Choose a reason for hiding this comment

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

why not do private boolean webSocketChannel = true;?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really how json pojos are written, and i want to be explicit to the caller

@moxie0
Copy link
Contributor Author

moxie0 commented Feb 10, 2015

updated the copyright header and indentation

@moxie0 moxie0 added this to the v2.6.0 milestone Feb 13, 2015

try {
if (isTextSecureEnvelope(request)) {
TextSecureEnvelope envelope = new TextSecureEnvelope(request.getBody().toByteArray(),
Copy link
Contributor

Choose a reason for hiding this comment

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

since body is optional in the protobuf, could someone just send an envelope without one and cause NPE here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"someone" here is the server, which i'm willing to trust with an npe

@mcginty
Copy link
Contributor

mcginty commented Feb 18, 2015

haven't run it yet, but code lgtm :)

@JavaJens JavaJens mentioned this pull request Feb 18, 2015
@moxie0 moxie0 closed this Feb 18, 2015
@greyson-signal greyson-signal deleted the store_and_forward branch November 6, 2020 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants