Skip to content

Align the comments in the check revoked (verify id token)#23

Merged
avishalom merged 15 commits intomasterfrom
align2
Feb 15, 2018
Merged

Align the comments in the check revoked (verify id token)#23
avishalom merged 15 commits intomasterfrom
align2

Conversation

@avishalom
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Couple of nits.

// Token is invalid.
} catch (ExecutionException e) {
if (e.getCause() instanceof FirebaseAuthException) {
if ( ((FirebaseAuthException) e.getCause()).getErrorCode().equals("id-token-revoked")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Break into a couple of lines:

FirebaseAuthException authError = ...;

// [START save_revocation_in_db]
DatabaseReference ref = FirebaseDatabase.getInstance().getReference("metadata/" + uid);
ref.setValueAsync(MapBuilder.of("revokeTime", revocationSecond)).get();
ref.setValueAsync(ImmutableMap.<String, Object>of("revokeTime", revocationSecond)).get();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just use HashMap here. ImmutableMap is a guava type and I don't think we want it to appear in snippets.

@hiranya911 hiranya911 assigned avishalom and unassigned hiranya911 Feb 14, 2018
Copy link
Copy Markdown
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM

@avishalom avishalom merged commit b75ae82 into master Feb 15, 2018
@avishalom avishalom deleted the align2 branch February 17, 2018 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants