1800087 - ASRouter & friends refers to remote settings collections incorrectly as buckets, causing dev confusion

Status: RESOLVED FIXED

(Firefox :: Messaging System, defect, P3)

Description

[Dan Mosedale (:dmosedale, :dmose)]

https://bugzilla.mozilla.org/show_bug.cgi?id=1797423#c18 has the details. Remote Settings does have a concept of buckets as well, but they are something else. This makes debugging / working with ASRouter code that calls into Remote Settings unnecessarily confusing.

Comment 1

[Shane Hughes [:aminomancer]]

This looks fun 😂

Comment 2

[Shane Hughes [:aminomancer]]

This bug will require changing the name of the bucket property for all our message provider prefs like browser.newtabpage.activity-stream.asrouter.providers.cfr. Thankfully the pref values themselves don't seem to be schema validated, but since this will invalidate pref values that we heavily use, we will need to migrate them and change our documentation. And if anyone was tinkering with that property (we do seem to adjust some of the other props for engineering purposes but I don't think we mess with the bucket value), they'll need to relearn that it's now going to be renamed collection. Just worth noting since that could be an argument for leaving the error as-is to avoid rocking the boat. But I'm going to go ahead with patching it in the meantime. If anyone feels strongly about this, speak now or forever hold your peace 😅

Comment 3

[Shane Hughes [:aminomancer]]

Attachment: Bug 1800087 - Fix ASRouter references to RS collections. r=dmose,Mardak

ASRouter's remote message providers were written to include a property
to specify the desired collection on the Remote Settings Kinto server.
But this property was mistakenly labeled bucket in many places. This
could be confusing because Kinto storage is organized by both buckets
and collections, with buckets being a higher level of organization and
collections belonging to buckets. Each individual message provider
specifies a collection like "cfr", while the bucket is set by the router
code ("main"). This carried over to everything that interacted with the
providers, so this patch effectively replaces every reference to a
"message provider bucket" with "message provider collection." But more
importantly, provider definitions are stored as JSON strings in
preference values, so these definitions erroneously contained key-value
pairs like bucket: "cfr" which should have been collection: "cfr".
That means the pref values themselves must be migrated to ensure that
modified prefs continue to work. Existing tests are updated and a new
test is added for the migrator, to meet minimum coverage requirements.
Documentation is also updated to reflect the correct property name.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aadb19250eb6
Fix ASRouter references to RS collections. r=barret

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/aadb19250eb6