Last Comment Bug 806656 - Query records generated incorrectly
: Query records generated incorrectly
Status: RESOLVED FIXED
[qa+]
:
Product: Android Background Services
Classification: Client Software
Component: Android Sync (show other bugs)
: unspecified
: ARM Android
: -- normal
: mozilla19
Assigned To: Richard Newman [:rnewman]
:
:
Mentors:
Depends on:
Blocks: 806460
  Show dependency treegraph
 
Reported: 2012-10-29 18:16 PDT by Richard Newman [:rnewman]
Modified: 2014-02-19 13:08 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
affected
fixed
fixed
fixed


Attachments
Proposed patch. v1 (3.89 KB, patch)
2012-10-29 22:35 PDT, Richard Newman [:rnewman]
no flags Details | Diff | Splinter Review
Aurora: fix. (3.96 KB, patch)
2012-10-30 14:04 PDT, Richard Newman [:rnewman]
liuche: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Richard Newman [:rnewman] 2012-10-29 18:16:02 PDT
We're getting records without a URI, and with a blank folderName and queryId. Android Sync is at fault.
Comment 1 Richard Newman [:rnewman] 2012-10-29 18:39:09 PDT
https://github.com/mozilla-services/android-sync/pull/265
Comment 2 Richard Newman [:rnewman] 2012-10-29 18:42:17 PDT
After the fixes in that branch, we:

* Correctly include bmkUri in produced bookmark records for query URIs.
* Omit fields which were encoded as empty strings prior to these fixes (e.g., if a query had no queryId, it now doesn't produce a blank string in the JSON output).
* No longer encode missing fields as empty strings in the first place.

This will prevent the upload of records which caused Bug 806460.
Comment 3 Richard Newman [:rnewman] 2012-10-29 22:35:03 PDT
Created attachment 676485 [details] [diff] [review]
Proposed patch. v1

Same as the GitHub branch.
Comment 4 Richard Newman [:rnewman] 2012-10-30 14:02:16 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/759b0e64b8b7

See Bug 806580 Comment 12 for how to verify this.

Will wait for verification before requesting uplift.
Comment 5 Richard Newman [:rnewman] 2012-10-30 14:04:22 PDT
Created attachment 676767 [details] [diff] [review]
Aurora: fix.

Reviewed on GitHub. liuche, please rubberstamp here.

Setting Aurora and Beta flags here for release drivers to take a look at. Will wait for verification before landing, of course.

This is the counterpart to Bug 806460, the fix for which was just uplifted.

This is a pretty straightforward fix, and now comes with tests in the services codebase.
Comment 6 Tracy Walker [:tracy] 2012-10-30 14:17:30 PDT
(In reply to Richard Newman [:rnewman] from comment #4)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/759b0e64b8b7
> 
> See Bug 806580 Comment 12 for how to verify this.
> 
> Will wait for verification before requesting uplift.

I believe you meant https://bugzilla.mozilla.org/show_bug.cgi?id=806460#c12. How is this the same verification?
Comment 7 Richard Newman [:rnewman] 2012-10-30 14:26:00 PDT
(In reply to Tracy Walker [:tracy] from comment #6)
> (In reply to Richard Newman [:rnewman] from comment #4)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/759b0e64b8b7
> > 
> > See Bug 806580 Comment 12 for how to verify this.
> > 
> > Will wait for verification before requesting uplift.
> 
> I believe you meant https://bugzilla.mozilla.org/show_bug.cgi?id=806460#c12.
> How is this the same verification?

"and those queries won't appear (e.g., smart bookmarks on your toolbar won't appear). After Bug 806656 lands, those queries should sync successfully."
Comment 8 Chenxia Liu [:liuche] (inactive on Fennec, expect slow responses) 2012-10-30 15:41:57 PDT
Comment on attachment 676767 [details] [diff] [review]
Aurora: fix.

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

Like rnewman said, reviewed on github. Looks good here.
Comment 9 Ed Morley [:emorley] 2012-10-31 07:19:51 PDT
https://hg.mozilla.org/mozilla-central/rev/759b0e64b8b7
Comment 10 Lukas Blakk [:lsblakk] use ?needinfo 2012-10-31 10:50:58 PDT
Comment on attachment 676767 [details] [diff] [review]
Aurora: fix.

Approving for uplift to go out with the fix in bug 806460 so that we stop creating bad records in the first place (as explained by rnewman in IRC).

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