Skip to content

Commit

Permalink
Improve caching of parsed UA strings
Browse files Browse the repository at this point in the history 
   * Show field in failed test error message
   * Skip testing that browser version matches as we
     don't care about matching Firefox build names
  • Loading branch information
@thinkwelltwd
thinkwelltwd committed Sep 4, 2019
1 parent e641bc8 commit dab74c3
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 112 deletions.
126 changes: 35 additions & 91 deletions device_detector/device_detector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
ApplicationIDExtractor,
NameVersionExtractor,
WholeNameExtractor,

DESKTOP_OS,
)
from .settings import DDCache, WORTHLESS_UA_TYPES
from .utils import (
Expand Down Expand Up @@ -108,9 +110,6 @@ def __init__(self, user_agent, skip_bot_detection=False, skip_device_detection=F
self.all_details = {'normalized': ''}
self.parsed = False

if self.ua_hash not in DDCache['user_agents']:
DDCache['user_agents'][self.ua_hash] = {}

@property
def class_name(self) -> str:
return self.__class__.__name__
Expand All @@ -122,13 +121,15 @@ def ua_spaceless(self) -> str:
return self._ua_spaceless

def get_parse_cache(self):
return DDCache['user_agents'][self.ua_hash].get('parsed', None)
return DDCache['user_agents'].get(self.ua_hash, {}).get('parsed', {})

def set_parse_cache(self):
if not self.all_details:
return self.all_details
try:
DDCache['user_agents'][self.ua_hash]['parsed'] = self
DDCache['user_agents'][self.ua_hash]['parsed'] = self.all_details
except KeyError:
DDCache['user_agents'][self.ua_hash] = {'parsed': self}
DDCache['user_agents'][self.ua_hash] = {'parsed': self.all_details}
return self

# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -190,8 +191,9 @@ def normalize(self):
that is of no use outside the application itself. Remove such information to present a
cleaner UA string with fewer duplicates
"""
if self.all_details['normalized']:
return self.all_details['normalized']
normalized = self.all_details.get('normalized', '')
if normalized:
return normalized

if self.is_digit():
self.all_details['normalized'] = 'Numeric'
Expand All @@ -207,6 +209,8 @@ def normalize(self):
if ua != self.user_agent:
self.all_details['normalized'] = ua
break
else:
self.all_details['normalized'] = ''

return self.all_details['normalized']

Expand All @@ -218,9 +222,9 @@ def is_worthless(self):
return self.all_details['normalized'] in WORTHLESS_UA_TYPES

def parse(self):
parsed = self.get_parse_cache()
if parsed:
return parsed
self.all_details = self.get_parse_cache()
if self.all_details:
return self

if not self.user_agent:
return self
Expand Down Expand Up @@ -420,10 +424,7 @@ def uses_mobile_browser(self) -> bool:
def engine(self) -> str:
if 'browser' not in self.client_type():
return ''
try:
return self.client.engine()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('engine', '')

def is_mobile(self) -> bool:
if self.device_type() in MOBILE_DEVICE_TYPES:
Expand All @@ -433,53 +434,28 @@ def is_mobile(self) -> bool:
def is_desktop(self) -> bool:
if self.uses_mobile_browser():
return False
try:
return self.os.is_desktop()
except AttributeError:
pass
return False
return self.all_details.get('os', {}).get('family', '') in DESKTOP_OS

def client_name(self) -> str:
try:
return self.client.name()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('name', '')

def client_short_name(self) -> str:
try:
return self.client.short_name()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('short_name', '')

def client_version(self) -> str:
try:
return self.client.version()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('version', '')

def client_type(self) -> str:
try:
return self.client.dtype()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('type', '')

def secondary_client_name(self) -> str:
try:
return self.client.secondary_name()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('secondary_client', {}).get('name', '')

def secondary_client_version(self) -> str:
try:
return self.client.secondary_version()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('secondary_client', {}).get('version', '')

def secondary_client_type(self) -> str:
try:
return self.client.secondary_type()
except AttributeError:
return ''
return self.all_details.get('client', {}).get('secondary_client', {}).get('type', '')

def preferred_client_name(self):
"""
Expand Down Expand Up @@ -507,12 +483,9 @@ def device_type(self) -> str:
if self.android_feature_phone():
return 'smartphone'

try:
dt = self.device.dtype()
if dt:
return dt
except AttributeError:
pass
dt = self.all_details.get('device', {}).get('type', '')
if dt:
return dt

aat = self.android_device_type()
if aat:
Expand All @@ -533,9 +506,9 @@ def device_type(self) -> str:
return ''

def device_model(self) -> str:
if not self.device or self.skip_device_detection:
if self.skip_device_detection:
return ''
return self.device.model()
return self.all_details.get('device', {}).get('model', '')

def device_brand_name(self) -> str:
if self.skip_device_detection:
Expand All @@ -546,54 +519,25 @@ def device_brand_name(self) -> str:
def device_brand(self) -> str:
if self.skip_device_detection:
return ''
try:
brand = self.device.brand_short_name()
except AttributeError:
brand = ''

brand = self.all_details.get('device', {}).get('brand', '')
if brand:
return brand

# Assume all devices running iOS / Mac OS are from Apple
if self.os_short_name() in MAC_iOS:
return 'AP'

def os_name(self) -> str:
try:
return self.os.name()
except AttributeError:
pass
return ''

def os_name(self) -> str:
return self.all_details.get('os', {}).get('name', '')

def os_short_name(self) -> str:
try:
return self.os.short_name()
except AttributeError:
pass
return ''
return self.all_details.get('os', {}).get('short_name', '')

def os_version(self) -> str:
try:
return self.os.version()
except AttributeError:
pass
return ''

def update_device_details(self) -> None:
"""
Update device details, after all parsing is complete,
and we have the maximum context available.
"""
if self.skip_device_detection:
return

if 'device' not in self.all_details:
return

self.all_details['device'].update({
'type': self.device_type(),
'name': self.device_brand_name(),
})
return self.all_details.get('os', {}).get('version', '')

def pretty_name(self) -> str:
return self.all_details['normalized'] or self.user_agent
Expand Down
36 changes: 20 additions & 16 deletions device_detector/tests/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Base(unittest.TestCase):
fixture_files = []
Parser = None

def assertEqual(self, first, second, msg=None):
def assertEqual(self, first, second, msg=None, **kwargs):
if first == 'None':
first = None
if second == 'None':
Expand All @@ -58,10 +58,17 @@ def assertEqual(self, first, second, msg=None):
return

if not msg and hasattr(self, 'user_agent'):
msg = '\n\nFailed to parse "{}"\n' \
'Expected value "{}" != Parsed value "{}"'.format(
getattr(self, 'user_agent'), first, second,
)
field = kwargs.get('field')
ua_msg = '\n\nFailed to parse "{}"\n'.format(getattr(self, 'user_agent'))

if field:
field_msg = 'Field "{}" expected value "{}" != Parsed value "{}"'.format(
field, first, second
)
else:
field_msg = 'Expected value "{}" != Parsed value "{}"'.format(first, second)

msg = '{} {}'.format(ua_msg, field_msg)

super().assertEqual(first, second, msg)

Expand All @@ -88,7 +95,7 @@ def confirm_client_name(self, fixture, parsed_name):

# if there's a fixture name, then the parsed value should match
if fixture_name:
self.assertEqual(fixture_name, parsed_name)
self.assertEqual(fixture_name, parsed_name, field='client_name')

def confirm_client_type(self, fixture, parsed_value):
"""
Expand All @@ -103,7 +110,7 @@ def confirm_client_type(self, fixture, parsed_value):
return

if fixture_value:
self.assertEqual(str(fixture_value), str(parsed_value))
self.assertEqual(str(fixture_value), str(parsed_value), field='client_type')

def confirm_version(self, fixture, parsed_value):
"""
Expand All @@ -118,7 +125,7 @@ def confirm_version(self, fixture, parsed_value):
# otherwise, skip checking. The generic version extractor
# will push/pull values a bit.
if fixture_value:
self.assertEqual(str(fixture_value), str(parsed_value))
self.assertEqual(str(fixture_value), str(parsed_value), field='client_version')

def matches_fixture_or_generic(self, fixture, key1, key2, parsed_value):
"""
Expand Down Expand Up @@ -161,10 +168,10 @@ def test_parsing(self):
device = DeviceDetector(self.user_agent)
device.parse()

# OS properties
self.assertEqual(device.os_name(), self.get_value(fixture, 'os', 'name'))
self.assertEqual(device.os_short_name(), self.get_value(fixture, 'os', 'short_name'))
self.assertEqual(device.os_version(), self.get_value(fixture, 'os', 'version'))
# # OS properties
self.assertEqual(device.os_name(), self.get_value(fixture, 'os', 'name'), field='os_name')
self.assertEqual(device.os_short_name(), self.get_value(fixture, 'os', 'short_name'), field='os_short_name')
self.assertEqual(device.os_version(), self.get_value(fixture, 'os', 'version'), field='os_version')

# Client properties
parsed_name = device.client_name()
Expand Down Expand Up @@ -222,10 +229,7 @@ def test_parsing(self):
self.assertEqual(
str(expect.get(field, '')),
str(data.get(field, '')),
msg='Error parsing {}. \n'
'Field "{}" parsed value "{}" != expected value "{}"'.format(
self.user_agent, field, data.get(field, ''), expect.get(field, '')
)
field=field,
)


Expand Down
2 changes: 1 addition & 1 deletion device_detector/tests/parser/test_clients.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TestBrowser(ParserBaseTest):
'tests/parser/fixtures/local/client/browser.yml',
'tests/parser/fixtures/upstream/client/browser.yml',
]
fields = ('name', 'type', 'short_name', 'version')
fields = ('name', 'type', 'short_name')
Parser = Browser


Expand Down
5 changes: 1 addition & 4 deletions device_detector/tests/parser/test_device.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ def test_parse(self):
self.assertEqual(
str(expect[field]),
str(data[field]),
msg='Error parsing {}. \n'
'Field "{}" parsed value "{}" != expected value "{}"'.format(
self.user_agent, field, data[field], expect[field]
)
field=field,
)


Expand Down

0 comments on commit dab74c3

Please sign in to comment.