Skip to content

Commit

Permalink
Merge pull request mozilla-services#3245 from peterbe/bug-1257631-rem…
Browse files Browse the repository at this point in the history
…ove-models-fetch-of-middleware-file-cache

fixes bug 1257631 - Remove models fetch of middleware file cache
  • Loading branch information
Peter Bengtsson committed Mar 28, 2016
2 parents e95020b + 337abbc commit 59707da
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 272 deletions.
1 change: 0 additions & 1 deletion docs/configuring-crash-stats.rst
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ NOTE - variables surrounded by @@@ are placeholders and need to be filled in app
MWARE_BASE_URL='http://localhost'
MWARE_HTTP_HOST='socorro-middleware'
CACHE_MIDDLEWARE=True
CACHE_MIDDLEWARE_FILES=False
# Your default product name, e.g. "Firefox"
DEFAULT_PRODUCT='@@@DEFAULT_PRODUCT@@@'
# You most likely want to use memcached
Expand Down
82 changes: 0 additions & 82 deletions webapp-django/crashstats/crashstats/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,8 @@
import datetime
import functools
import hashlib
import os
import urlparse
import json
import logging
import requests
import stat
import time

import ujson
Expand Down Expand Up @@ -253,7 +249,6 @@ def fetch(
implementation = url_or_implementation

cache_key = None
cache_file = None

if settings.CACHE_MIDDLEWARE and not dont_cache and self.cache_seconds:
if url:
Expand Down Expand Up @@ -285,76 +280,6 @@ def fetch(
)
return result, True

# not in the memcache/locmem but is it in cache files?
if settings.CACHE_MIDDLEWARE_FILES:
root = settings.CACHE_MIDDLEWARE_FILES
if isinstance(root, bool):
cache_file = os.path.join(
settings.ROOT,
'models-cache'
)
else:
cache_file = root

# We need to conjure up a string filename to represent
# this call. If it's a URL we use the URL path to
# as the file path.
# If it's an implementation we use the class name
if implementation:
cache_file = os.path.join(
cache_file,
implementation.__class__.__name__
)
else:
split = urlparse.urlparse(url)
cache_file = os.path.join(
cache_file,
split.netloc,
_clean_path(split.path)
)
if split.query:
cache_file = os.path.join(
cache_file,
_clean_query(split.query)
)
if expect_json:
cache_file = os.path.join(
cache_file,
'%s.json' % cache_key
)
else:
cache_file = os.path.join(
cache_file,
'%s.dump' % cache_key
)

if os.path.isfile(cache_file):
# but is it fresh enough?
age = time.time() - os.stat(cache_file)[stat.ST_MTIME]
if age > self.cache_seconds:
logger.debug("CACHE FILE TOO OLD")
os.remove(cache_file)
else:
logger.debug("CACHE FILE HIT %s" % url)
delete_cache_file = False
with open(cache_file) as f:
if expect_json:
try:
return json.load(f), True
except ValueError:
logger.warn(
"%s is not a valid JSON file and "
"will be deleted" % (
cache_file,
),
exc_info=True
)
delete_cache_file = True
else:
return f.read(), True
if delete_cache_file:
os.remove(cache_file)

if url:
if method == 'post':
request_method = requests.post
Expand Down Expand Up @@ -414,13 +339,6 @@ def fetch(

if cache_key:
cache.set(cache_key, result, self.cache_seconds)
if cache_file:
if not os.path.isdir(os.path.dirname(cache_file)):
os.makedirs(os.path.dirname(cache_file))
if expect_json:
json.dump(result, open(cache_file, 'w'), indent=2)
else:
open(cache_file, 'w').write(result)

return result, False

Expand Down
185 changes: 0 additions & 185 deletions webapp-django/crashstats/crashstats/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import json
import os
import shutil
import tempfile
import datetime
import time
import random

import mock
Expand Down Expand Up @@ -46,7 +42,6 @@ def setUp(self):
super(TestModels, self).setUp()
# thanks to crashstats.settings.test
assert settings.CACHE_MIDDLEWARE
assert not settings.CACHE_MIDDLEWARE_FILES
cache.clear()

def tearDown(self):
Expand Down Expand Up @@ -1549,186 +1544,6 @@ def mocked_get(**params):
}
)


class TestModelsWithFileCaching(TestCase):

def setUp(self):
super(TestModelsWithFileCaching, self).setUp()
self.tempdir = tempfile.mkdtemp()
# Using CACHE_MIDDLEWARE_FILES is mainly for debugging but good to test
self._cache_middleware = settings.CACHE_MIDDLEWARE
self._cache_middleware_files = settings.CACHE_MIDDLEWARE_FILES
settings.CACHE_MIDDLEWARE = True
settings.CACHE_MIDDLEWARE_FILES = self.tempdir
cache.clear()

def tearDown(self):
super(TestModelsWithFileCaching, self).tearDown()
settings.CACHE_MIDDLEWARE = self._cache_middleware
settings.CACHE_MIDDLEWARE_FILES = self._cache_middleware_files
if os.path.isdir(self.tempdir):
shutil.rmtree(self.tempdir)

@mock.patch('requests.get')
def test_bugzilla_api_to_file(self, rget):
model = models.BugzillaBugInfo

api = model()

def mocked_get(**options):
assert options['url'].startswith(models.BugzillaAPI.base_url)
return Response('{"bugs": [{"product": "mozilla.org"}]}')

rget.side_effect = mocked_get
info = api.get('747237', 'product')
eq_(info['bugs'], [{u'product': u'mozilla.org'}])

# prove that it's cached by default
def new_mocked_get(**options):
return Response('{"bugs": [{"product": "DIFFERENT"}]}')

rget.side_effect = new_mocked_get
info = api.get('747237', 'product')
eq_(info['bugs'], [{u'product': u'mozilla.org'}])

info = api.get('747238', 'product')
eq_(info['bugs'], [{u'product': u'DIFFERENT'}])

def _get_cached_file(self, in_):
files = []
for f in os.listdir(in_):
path = os.path.join(in_, f)
if os.path.isdir(path):
files.extend(self._get_cached_file(path))
else:
files.append(path)
return files

@mock.patch('crashstats.crashstats.models.time.time')
@mock.patch('requests.get')
def test_get_current_version_cache_invalidation(self, rget, mocked_time):
def mocked_get(**options):
assert '/products/' in options['url']
return Response("""
{"hits": {
"SeaMonkey": [{
"product": "SeaMonkey",
"throttle": "100.00",
"end_date": "2012-05-10 00:00:00",
"start_date": "2012-03-08 00:00:00",
"featured": true,
"version": "2.1.3pre",
"release": "Beta",
"id": 922}]
},
"products": ["SeaMonkey"]
}
""")
rget.side_effect = mocked_get

# Because we're mock patching time.time, it becomes impossible
# to do something like `some_time > time.time()` which is exactly
# what the django.core.cache.backends.locmem.LocMemCache class
# does inside its get() method.
# So we need to hijack that mock specifically for the "less than"
# operator.
def mocked_time_lt(other):
return False

mocked_time().__lt__.side_effect = mocked_time_lt

assert not self._get_cached_file(self.tempdir)

# the first time, we rely on the mocket request.get
model = models.CurrentVersions
api = model()
info = api.get()
eq_(info[0]['product'], 'SeaMonkey')

json_file = self._get_cached_file(self.tempdir)[0]
with open(json_file) as f:
assert 'hits' in json.loads(f.read())

# if we now loose the memcache/locmem
cache.clear()

info = api.get()
eq_(info[0]['product'], 'SeaMonkey')

now = time.time()
extra = models.CurrentVersions.cache_seconds

def my_time():
return now + extra

# now we're going to mess with the modification time so that
# the cache file gets wiped
rget.side_effect = mocked_get

mocked_time.side_effect = my_time
info = api.get()
eq_(info[0]['product'], 'SeaMonkey')

@mock.patch('requests.get')
def test_get_current_version_by_file_cache(self, rget):
calls = []

def mocked_get(**options):
calls.append(options)
assert '/products/' in options['url']
return Response("""
{"hits": {
"SeaMonkey": [{
"product": "SeaMonkey",
"throttle": "100.00",
"end_date": "2012-05-10 00:00:00",
"start_date": "2012-03-08 00:00:00",
"featured": true,
"version": "2.1.3pre",
"release": "Beta",
"id": 922}]
},
"products": ["SeaMonkey"]
}
""")
rget.side_effect = mocked_get

assert not self._get_cached_file(self.tempdir)

# the first time, we rely on the mocket request.get
model = models.CurrentVersions
api = model()
info = api.get()
eq_(info[0]['product'], 'SeaMonkey')
eq_(len(calls), 1)

json_file = self._get_cached_file(self.tempdir)[0]
with open(json_file) as f:
json_file_content = f.read()
assert 'hits' in json.loads(json_file_content)

# if we now loose the memcache/locmem
cache.clear()

info_second_time = api.get()
eq_(info_second_time, info)
eq_(len(calls), 1)

# now let's mess with the file
with open(json_file, 'w') as f:
f.write(json_file_content.replace('}', '!'))

info_third_time = api.get()
# The JSON file was broken, so it had to do another network
# request.
eq_(len(calls), 2)
eq_(info_third_time, info)
eq_(info_second_time, info_third_time)
# file gets recreated and should now be valid JSON again
assert os.path.isfile(json_file)
with open(json_file) as f:
json.load(f) # should work

@mock.patch('requests.get')
def test_signature_urls(self, rget):
model = models.SignatureURLs
Expand Down
3 changes: 0 additions & 3 deletions webapp-django/crashstats/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,6 @@ def path(*dirs):
# environment variables.

CACHE_MIDDLEWARE = config('CACHE_MIDDLEWARE', True, cast=bool)
# creates "./models-cache" dir
# only applicable if CACHE_MIDDLEWARE is True
CACHE_MIDDLEWARE_FILES = config('CACHE_MIDDLEWARE_FILES', False, cast=bool)

# Socorro middleware instance to use
MWARE_BASE_URL = config('MWARE_BASE_URL', 'http://localhost:5200')
Expand Down
1 change: 0 additions & 1 deletion webapp-django/crashstats/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"""

CACHE_MIDDLEWARE = True
CACHE_MIDDLEWARE_FILES = False

DEFAULT_PRODUCT = 'WaterWolf'

Expand Down

0 comments on commit 59707da

Please sign in to comment.