1487495 - Preference Rollout allows for floating point numbers

Status: RESOLVED FIXED

(Firefox :: Normandy Client, enhancement)

Description

[Peter Bengtsson [:peterbe]]

The JSON Schema for Preference Rollout allows for 3 types: string, number and boolean. 

See
https://searchfox.org/mozilla-central/rev/2fe43133dbc774dda668d4597174d73e3969181a/toolkit/components/normandy/actions/schemas/index.js#43

But a "number", in JSON Schema, [0] can be both 2.9978 and 3.
But the "integer" [1] exists as an option and does not allow 2.9978. 

Granted, there's a big warning on that "integer" spec. In particular...:

> For example, a JavaScript-based validator may accept 1.0 as an integer, whereas the Python-based jsonschema does not.

We use the JSON Schemas from mozilla-central via https://www.npmjs.com/package/mozilla-normandy-action-argument-schemas into Normandy. If the list of types was `["string", "integer", "boolean"]` I know that Normandy's API would reject attempts to save something like: `{"slug": "x", "preferences": [{"preferenceName": "foo", "value": 12.34}]}`


[0] https://json-schema.org/understanding-json-schema/reference/numeric.html#number
[1] https://json-schema.org/understanding-json-schema/reference/numeric.html#integer

Comment 1

[Peter Bengtsson [:peterbe]]

A problem with non-integers is that it's unlikely to be a valid value to send into the client. Can the preference system accept changing a pref to a floating point number? If not, we could try to make it less likely to come to that.

Comment 2

[Ethan Glasser-Camp (:glasserc)]

Attachment: Bug 1487495: use integer for pref values instead of number

Comment 3

[Ethan Glasser-Camp (:glasserc)]

The pref system supports float preferences, but they're internally encoded as strings and accessed using a utility helper that converts them to floats (`nsIPrefBranch#getFloatPref`).

Comment 4

[Ethan Glasser-Camp (:glasserc)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2c3bf8cdd9eece27ad11d96192354bbca8be296

Comment 5

[Ethan Glasser-Camp (:glasserc)]

Are these schemas actually used by the Normandy client? I just tried to rebuild Firefox and to "break" this by setting the value to floating point numbers such as 1.9 or 2.8, but the recipe and action still download and run "successfully" (truncating the number to an integer).

Comment 6

[Michael Cooper [:mythmon]]

Normandy uses these schemas in the action super class, validating the data before it executes the action: https://searchfox.org/mozilla-central/source/toolkit/components/normandy/actions/BaseAction.jsm#99-106

Turns out that although our schema validator supports "integer" as a type, it doesn't actually treat it differently than "number": https://searchfox.org/mozilla-central/source/toolkit/components/utils/JsonSchemaValidator.jsm#184-187

This is surprising to me. I think that the way you have it now works well. We're more clearly advertising what we support to other users (such as the Normandy server), and the code that depends on this schema seems to be fine with the looser definition that JS provides.

Comment 7

[Ethan Glasser-Camp (:glasserc)]

I guess I'll land this then, I just wanted to make sure my change "worked".

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11132810d870
use integer for pref values instead of number r=Gijs

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/11132810d870