From 8a929b707620a0ccc49a688badfc1549520d849b Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 24 Jul 2015 18:13:37 +0200 Subject: [PATCH] Implement new Feature Profile signature mechanism in the API (bug 1172487) Instead of encoding all features into a single huge bitfield, use several smaller ones that are then encoded in base64. This allows us to have more than 53 features in JavaScript, where integers are not safe to use past 2^53. --- mkt/constants/features.py | 117 ++++++++++++++++++++++++++- mkt/constants/tests/test_features.py | 81 ++++++++++++++++++- mkt/search/tests/test_views.py | 17 ++-- 3 files changed, 207 insertions(+), 8 deletions(-) diff --git a/mkt/constants/features.py b/mkt/constants/features.py index 2a33d5b2f66..2a88a7b02fc 100644 --- a/mkt/constants/features.py +++ b/mkt/constants/features.py @@ -1,4 +1,6 @@ +import base64 import itertools +import math from collections import OrderedDict from django.conf import settings @@ -361,6 +363,56 @@ ] +class FeaturesBitField(object): + """ + BitField class that stores the bits into several integers, and can + import/export from/to base64. Designed that way to be compatible with the + way we export the features signature in JavaScript. + """ + + def __init__(self, size, values=None): + """ + Instantiate a FeaturesBitField of size `size`. Optional parameter + `values` allows you to override the initial list of integers used to + store the values. + """ + self.size = size + if values is not None: + self.values = values + else: + self.values = [0] * int(math.ceil(self.size / 8.0)) + + def get(self, i): + index = int(math.ceil(i / 8)) + bit = i % 8 + return (self.values[index] & (1 << bit)) != 0 + + def set(self, i, value): + index = int(math.ceil(i / 8)) + bit = i % 8 + if value: + self.values[index] |= 1 << bit + else: + self.values[index] &= ~(1 << bit) + + def to_base64(self): + return base64.b64encode(''.join([chr(i) for i in self.values])) + + def to_list(self): + return [self.get(i) for i in range(0, self.size)] + + @classmethod + def from_list(cls, data): + instance = cls(len(data)) + for i, v in enumerate(data): + instance.set(i, v) + return instance + + @classmethod + def from_base64(cls, string, size): + return cls(size, values=[ord(c) for c in base64.b64decode(string)]) + + class FeatureProfile(OrderedDict): """ Convenience class for performing conversion operations on feature profile @@ -403,7 +455,24 @@ def from_int(cls, features, limit=None): return instance @classmethod - def from_signature(cls, signature): + def from_list(cls, features, limit=None): + """ + Construct a FeatureProfile object from a list of boolean values. + + >>> FeatureProfile.from_list([True, False, ...]) + FeatureProfile([('apps', True), ('packaged_apps', False), ...) + """ + instance = cls() # Defaults to everything set to False. + if limit is None: + limit = len(APP_FEATURES) + app_features_to_consider = OrderedDict( + itertools.islice(APP_FEATURES.iteritems(), limit)) + for i, k in enumerate(app_features_to_consider): + instance[k.lower()] = bool(features[i]) + return instance + + @classmethod + def from_decimal_signature(cls, signature): """ Construct a FeatureProfile object from a decimal signature. @@ -415,6 +484,41 @@ def from_signature(cls, signature): number, limit, version = signature.split('.') return cls.from_int(int(number, 16), limit=int(limit)) + @classmethod + def from_base64_signature(cls, signature): + """ + Construct a FeatureProfile object from a base64 signature. + + >>> FeatureProfile.from_signature('=////////Hw==.53.9') + FeatureProfile([('apps', True), ('packaged_apps', True), ...) + """ + # If the signature is invalid, let the ValueError be raised, it's up to + # the caller to decide what to do with it. + string, limit, version = signature.split('.') + limit = int(limit) + + # Decode base64 string (ignoring the leading '=' that is used to + # indicate we are dealing with a base64 signature) using our bit field. + bitfield = FeaturesBitField.from_base64(string[1:], limit) + # Build the FeatureProfile from our list of boolean values. + return cls.from_list(bitfield.to_list(), limit=limit) + + @classmethod + def from_signature(cls, signature): + """ + Construct a FeatureProfile object from a signature, base64 + (starting with '=') or decimal (everything else). + + >>> FeatureProfile.from_signature('40000000.32.1') + FeatureProfile([('apps', False), ('packaged_apps', True), ...) + + >>> FeatureProfile.from_signature('=////////Hw==.53.9') + FeatureProfile([('apps', True), ('packaged_apps', True), ...) + """ + if signature.startswith('='): + return cls.from_base64_signature(signature) + return cls.from_decimal_signature(signature) + def to_int(self): """ Convert a FeatureProfile object to an integer bitfield. @@ -437,6 +541,17 @@ def to_signature(self): return '%x.%s.%s' % (self.to_int(), len(self), settings.APP_FEATURES_VERSION) + def to_base64_signature(self): + """ + Convert a FeatureProfile object to its base64 signature. + + >>> profile.to_signature() + '=////////Hw==.53.9' + """ + self.bitfield = FeaturesBitField.from_list(self.values()) + return '=%s.%s.%s' % (self.bitfield.to_base64(), len(self), + settings.APP_FEATURES_VERSION) + def to_list(self): """ Returns a list representing the true values of this profile. diff --git a/mkt/constants/tests/test_features.py b/mkt/constants/tests/test_features.py index dce6bb8450c..d9b999a50d7 100644 --- a/mkt/constants/tests/test_features.py +++ b/mkt/constants/tests/test_features.py @@ -7,7 +7,8 @@ from nose.tools import eq_, ok_ import mkt.site.tests -from mkt.constants.features import APP_FEATURES, FeatureProfile +from mkt.constants.features import (APP_FEATURES, FeaturesBitField, + FeatureProfile) MOCK_APP_FEATURES_LIMIT = 45 @@ -20,6 +21,8 @@ class TestFeaturesMixin(object): signature = '110022000000.%d.%d' % ( MOCK_APP_FEATURES_LIMIT, settings.APP_FEATURES_VERSION) expected_features = ['apps', 'proximity', 'light_events', 'vibrate'] + base64_signature = '=EYAIAAAA.%d.%d' % ( + MOCK_APP_FEATURES_LIMIT, settings.APP_FEATURES_VERSION) def _test_profile_values(self, profile): for k, v in profile.iteritems(): @@ -55,10 +58,23 @@ def test_from_int_all_false(self): self.expected_features = [] self.test_from_int() + def test_from_list(self): + bools = [False] * MOCK_APP_FEATURES_LIMIT + bools[0] = True # apps + bools[4] = True # light events + bools[15] = True # proximity + bools[19] = True # vibrate + profile = FeatureProfile.from_list(bools) + self._test_profile(profile) + def test_from_signature(self): profile = FeatureProfile.from_signature(self.signature) self._test_profile(profile) + def test_from_base64(self): + profile = FeatureProfile.from_signature(self.base64_signature) + self._test_profile(profile) + def _test_kwargs(self, prefix): profile = FeatureProfile.from_int(self.features) kwargs = profile.to_kwargs(prefix=prefix) @@ -72,6 +88,11 @@ def test_to_kwargs(self): self._test_kwargs('') self._test_kwargs('prefix_') + def test_to_base64(self): + profile = FeatureProfile.from_int(self.features) + signature = profile.to_base64_signature() + eq_(signature, self.base64_signature) + class TestFeatureProfileDynamic(TestFeaturesMixin, mkt.site.tests.TestCase): def test_from_int_limit(self): @@ -86,3 +107,61 @@ def test_from_old_signature(self): ok_(new_signature != self.signature) profile = FeatureProfile.from_signature(new_signature) self._test_profile_values(profile) + + +class TestFeaturesBitField(mkt.site.tests.TestCase): + test_data = [True, False, False, False, False, False, False, True, True] + + def test_basic(self): + bitfield = FeaturesBitField(8) + eq_(bitfield.values, [0]) + bitfield = FeaturesBitField(9) + eq_(bitfield.values, [0, 0]) + bitfield = FeaturesBitField(16) + eq_(bitfield.values, [0, 0]) + bitfield = FeaturesBitField(53) + eq_(bitfield.values, [0, 0, 0, 0, 0, 0, 0]) + + def test_basic_values(self): + bitfield = FeaturesBitField(8, values=[0, 1, 3]) + eq_(bitfield.values, [0, 1, 3]) + + def test_set(self): + bitfield = FeaturesBitField(9) + bitfield.set(0, True) + eq_(bitfield.values, [1, 0]) + bitfield.set(1, True) + eq_(bitfield.values, [3, 0]) + bitfield.set(8, True) + eq_(bitfield.values, [3, 1]) + + def test_get(self): + bitfield = FeaturesBitField(9) + eq_(bitfield.get(0), False) + bitfield.set(0, True) + eq_(bitfield.get(0), True) + + eq_(bitfield.get(8), False) + bitfield.set(8, True) + eq_(bitfield.get(8), True) + + def test_to_list(self): + bitfield = FeaturesBitField(9) + bitfield.set(0, True) + bitfield.set(7, True) + bitfield.set(8, True) + eq_(bitfield.to_list(), self.test_data) + + def test_from_list(self): + bitfield = FeaturesBitField.from_list(self.test_data) + eq_(bitfield.values, [129, 1]) + eq_(bitfield.to_list(), self.test_data) + + def test_to_base64(self): + bitfield = FeaturesBitField.from_list(self.test_data) + eq_(bitfield.to_base64(), 'gQE=') + + def test_from_base64(self): + bitfield = FeaturesBitField.from_base64('gQE=', len(self.test_data)) + eq_(bitfield.to_list(), self.test_data) + eq_(bitfield.to_base64(), 'gQE=') diff --git a/mkt/search/tests/test_views.py b/mkt/search/tests/test_views.py index bdb28aa49db..a6440a19078 100644 --- a/mkt/search/tests/test_views.py +++ b/mkt/search/tests/test_views.py @@ -901,10 +901,11 @@ def setUp(self): self.webapp = Webapp.objects.get(pk=337141) self.webapp.addondevicetype_set.create(device_type=mkt.DEVICE_GAIA.id) # Pick a few common device features. - self.features = FeatureProfile(apps=True, audio=True, fullscreen=True, - geolocation=True, indexeddb=True, - sms=True).to_signature() - self.qs = {'q': 'something', 'pro': self.features, 'dev': 'firefoxos'} + self.features = FeatureProfile( + apps=True, audio=True, fullscreen=True, geolocation=True, + indexeddb=True, sms=True) + self.profile = self.features.to_signature() + self.qs = {'q': 'something', 'pro': self.profile, 'dev': 'firefoxos'} def test_no_features(self): # Base test to make sure we find the app. @@ -927,6 +928,11 @@ def test_one_good_feature(self): obj = json.loads(res.content)['objects'][0] eq_(obj['slug'], self.webapp.app_slug) + def test_one_good_feature_base64(self): + self.profile = self.features.to_base64_signature() + self.qs['pro'] = self.profile + self.test_one_good_feature() + def test_one_bad_feature(self): # Enable an app feature that doesn't match one in our profile. self.webapp.current_version.features.update(has_pay=True) @@ -940,9 +946,8 @@ def test_one_bad_feature(self): def test_all_good_features(self): # Enable app features so they exactly match our device profile. - fp = FeatureProfile.from_signature(self.features) self.webapp.current_version.features.update( - **dict(('has_%s' % k, v) for k, v in fp.items())) + **dict(('has_%s' % k, v) for k, v in self.features.items())) self.webapp.save() self.refresh('webapp')