Query records generated incorrectly

RESOLVED FIXED in Firefox 17

Status

Android Background Services
Android Sync
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

unspecified
mozilla19
ARM
Android

Firefox Tracking Flags

(firefox16 affected, firefox17 fixed, firefox18 fixed, firefox19 fixed)

Details

(Whiteboard: [qa+])

Attachments

(1 attachment, 1 obsolete attachment)

We're getting records without a URI, and with a blank folderName and queryId. Android Sync is at fault.
Blocks: 806460
https://github.com/mozilla-services/android-sync/pull/265
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.
status-firefox16: --- → affected
status-firefox17: --- → affected
status-firefox18: --- → affected
status-firefox19: --- → affected
Whiteboard: [waiting for review]
Created attachment 676485 [details] [diff] [review]
Proposed patch. v1

Same as the GitHub branch.
Attachment #676485 - Flags: review?(liuche)
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.
Whiteboard: [waiting for review] → [qa+]
Target Milestone: --- → mozilla19
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.
Attachment #676485 - Attachment is obsolete: true
Attachment #676485 - Flags: review?(liuche)
Attachment #676767 - Flags: review?(liuche)
Attachment #676767 - Flags: approval-mozilla-beta?
Attachment #676767 - Flags: approval-mozilla-aurora?
(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?
QA Contact: twalker
(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 on attachment 676767 [details] [diff] [review]
Aurora: fix.

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

Like rnewman said, reviewed on github. Looks good here.
Attachment #676767 - Flags: review?(liuche) → review+

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/759b0e64b8b7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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).
Attachment #676767 - Flags: approval-mozilla-beta?
Attachment #676767 - Flags: approval-mozilla-beta+
Attachment #676767 - Flags: approval-mozilla-aurora?
Attachment #676767 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/267a86de82df
https://hg.mozilla.org/releases/mozilla-beta/rev/0ddd071d9ff2
status-firefox17: affected → fixed
status-firefox18: affected → fixed
status-firefox19: affected → fixed
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services

Updated

3 years ago
QA Contact: twalker
You need to log in before you can comment on or make changes to this bug.