As soon as one property is found to be different, we know an update is
not spurious. At that point any further testing is redundant, so we can
return from isSpuriousUpdate early and save ourselves the extra work to
continue testing other properties.
The logic in devices.isSpuriousUpdate was incorrectly failing to negate
a boolean condition, meaning that matching device commands were being
treated as non-spurious. This change fixes that so they are deemed
spurious instead.
We used to filter target devices in the push module, but that's about to be
refactored, so this moves responsibilities around to make the refactor easier.
Previously we could accept URLs with unescaped special characters
such as newlines or unicode, which means we were depending on other
layers of the code to handle them correctly. This change makes the
requestor responsible for properly escaping any special characters
in their URLs before passing them in to us.
Fixesmozilla/fxa-amplitude-send#58.
The user-agent parser was originally written with synthesized device
names in mind, so it didn't always return the full version string for
browsers and operating systems. Since then, we started using the same
code for our event data, meaning that we're getting an incomplete
picture of the browser/os that our users are on.
This change tweaks the user-agent code so that it returns full version
info, and tweaks the code for synthesizing device names so that it
remains consistent with its old behaviour.
Fixes#2106.
Prevents us from accidentally calling db.devices more than once per request. I saw one definite case of this in /recovery_email/verify_code and it's possible there were others. I'll also be making use of this property heavily for the amplitude events, so it will get further usage imminently.
Making the change necessitated pulling calls to db.devices out of lib/push, which triggered some refactoring that almost got away from me. I'll add inline commentary to call out why things have changed the way they have, but most push methods now take an extra devices argument and a few other methods became redundant so I deleted them. I don't think I've broken anything.
This settles our dance of `Buffer` vs `String` down to simply this:
> You have a `String`. You should (almost) never have a `Buffer`.
Buffers are useful for talking about a specific set of bytes, without an
encoding. In our app, the places where this is useful are:
- crypto
- mysql
We don't actually speak MySQL in this repo anywhere, so that leaves us
with only crypto. Instead of requiring the mental overhead of "Do I have
a buffer or a string?" throughout all our code base, we can just push
that completely into the crypto code.
This *should* reduce bugs where we aren't sure if we have a `Buffer` or
a `String`. If you're not in crypto, you should just have a `String`.
vladikoff
hritvi
Shane Tomlinson
Phil Booth
Ryan Kelly
Edouard Oger
Sean McArthur