Last Comment Bug 748215 - use v2 user info with YSI to get storage values
: use v2 user info with YSI to get storage values
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Message Compose Window (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 15.0
Assigned To: David :Bienvenu
:
Mentors:
Depends on:
Blocks: 744010
  Show dependency treegraph
 
Reported: 2012-04-23 21:15 PDT by David :Bienvenu
Modified: 2012-04-26 11:08 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
fix ysi component (1.30 KB, patch)
2012-04-23 21:15 PDT, David :Bienvenu
no flags Details | Diff | Review
fix mozmill test (4.33 KB, patch)
2012-04-24 11:51 PDT, David :Bienvenu
mconley: review-
Details | Diff | Review
Old patch plus code to fix stale token handler for YouSendIt (5.55 KB, patch)
2012-04-26 10:31 PDT, Mike Conley (:mconley) - (Away until June 29th)
mozilla: review+
mozilla: approval‑comm‑aurora+
mozilla: approval‑comm‑beta+
Details | Diff | Review

Description David :Bienvenu 2012-04-23 21:15:19 PDT
Created attachment 617773 [details] [diff] [review]
fix ysi component

we should use v2 of the YSI user info command in order to get storage numbers. This patch does that. I think we need to tweak the fake ysi server to return the same kind of info.
Comment 1 David :Bienvenu 2012-04-24 11:51:01 PDT
Created attachment 617973 [details] [diff] [review]
fix mozmill test

I left the check for null storage in there just in case YSI does something different than I expect.

I changed the error code we check for because now it's 401, not 500, in the case of stale auth tokens.

YSI is still returning 0 for the storage used but I believe they'll fix that.
Comment 2 Mike Conley (:mconley) - (Away until June 29th) 2012-04-25 08:24:05 PDT
David:

The code looks good, but upon testing, I'm getting the same issue you were getting before:

2012-04-25 11:22:49	YouSendIt	INFO	request status = 200 response = {"key":null,"id":null,"type":null,"policy":null,"version":null,"password":null,"role":null,"email":null,"firstname":null,"lastname":null,"created":null,"account":null,"storage":null,"status":null,"errorStatus":{"message":"Login session may have timed out. Please try logging again.","code":401}}

What's the story with that?

-Mike
Comment 3 David :Bienvenu 2012-04-25 12:30:40 PDT
(In reply to Mike Conley (:mconley) from comment #2)
> David:
> 
> The code looks good, but upon testing, I'm getting the same issue you were
> getting before:
> 
> 2012-04-25 11:22:49	YouSendIt	INFO	request status = 200 response =
> {"key":null,"id":null,"type":null,"policy":null,"version":null,"password":
> null,"role":null,"email":null,"firstname":null,"lastname":null,"created":
> null,"account":null,"storage":null,"status":null,"errorStatus":{"message":
> "Login session may have timed out. Please try logging again.","code":401}}
> 
> What's the story with that?
> 
Is it failing to authenticate? Or do we refresh the token?

The only thing I can figure is that if you've done a v1 user info with a token, doing a v2 user info with that same token fails.
Comment 4 Mike Conley (:mconley) - (Away until June 29th) 2012-04-26 07:47:31 PDT
Comment on attachment 617973 [details] [diff] [review]
fix mozmill test

Review of attachment 617973 [details] [diff] [review]:
-----------------------------------------------------------------

r-'ing because in nsYouSendIt.js, that block inside the conditional on line 272 will never be executed (since it checks for an impossibility, considering the block that it's contained within).
Comment 5 Mike Conley (:mconley) - (Away until June 29th) 2012-04-26 10:31:47 PDT
Created attachment 618708 [details] [diff] [review]
Old patch plus code to fix stale token handler for YouSendIt

I was wrong about my last review - I got confused between request.status and docResponse.errorStatus.  So r+ for bienvenu's work.

In my testing, I noticed that if we didn't have the YSI password stored in nsILoginManager, then attempting to re-auth when tokens have gone stale would always hit the failure case, since the old token was not being cleared out.
Comment 6 David :Bienvenu 2012-04-26 10:40:54 PDT
Comment on attachment 618708 [details] [diff] [review]
Old patch plus code to fix stale token handler for YouSendIt

thx for the additional fix!
Comment 7 Mike Conley (:mconley) - (Away until June 29th) 2012-04-26 10:51:35 PDT
Comment on attachment 618708 [details] [diff] [review]
Old patch plus code to fix stale token handler for YouSendIt

I think we'll want to switch TB 13 / 14 over to this API as well.

This'll clear the way for displaying the YSI usage graph - assuming they start delivering some real numbers soon.

Note You need to log in before you can comment on or make changes to this bug.