From d55ee9fb6e951fc74785fbe16f290df16cac9e83 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 1 Jul 2018 14:15:55 -0400 Subject: [PATCH] Add better caching to jsonpath-ng wrapper functions This patchset uses beaker (used by Promenade and Drydock) to achieve better caching around jsonpath-ng wrapper functions (jsonpath_replace and jsonpath_parse). Change-Id: Ifae24775b4741ade7673dc91776c35d2de5b9065 --- deckhand/common/utils.py | 24 +++++++----- deckhand/conf/config.py | 22 +++++++++-- deckhand/tests/unit/common/test_utils.py | 49 +++++++++++++++++++++++- requirements.txt | 1 + 4 files changed, 81 insertions(+), 15 deletions(-) diff --git a/deckhand/common/utils.py b/deckhand/common/utils.py index b5385a20..e2f2481a 100644 --- a/deckhand/common/utils.py +++ b/deckhand/common/utils.py @@ -17,18 +17,26 @@ import copy import re import string +from beaker.cache import CacheManager +from beaker.util import parse_cache_config_options import jsonpath_ng from oslo_log import log as logging from oslo_utils import excutils import six +from deckhand.conf import config from deckhand import errors +CONF = config.CONF LOG = logging.getLogger(__name__) # Cache for JSON paths computed from path strings because jsonpath_ng # is computationally expensive. -_PATH_CACHE = dict() +_CACHE_OPTS = { + 'cache.type': 'memory', + 'expire': CONF.jsonpath.cache_timeout, +} +_CACHE = CacheManager(**parse_cache_config_options(_CACHE_OPTS)) _ARRAY_RE = re.compile(r'.*\[\d+\].*') @@ -54,17 +62,13 @@ def _normalize_jsonpath(jsonpath): return jsonpath -def _jsonpath_parse_cache(jsonpath): +@_CACHE.cache() +def _jsonpath_parse(jsonpath): """Retrieve the parsed jsonpath path Utilizes a cache of parsed values to eliminate re-parsing """ - if jsonpath not in _PATH_CACHE: - p = jsonpath_ng.parse(jsonpath) - _PATH_CACHE[jsonpath] = p - else: - p = _PATH_CACHE[jsonpath] - return p + return jsonpath_ng.parse(jsonpath) def jsonpath_parse(data, jsonpath, match_all=False): @@ -96,7 +100,7 @@ def jsonpath_parse(data, jsonpath, match_all=False): # Do something with the extracted secret from the source document. """ jsonpath = _normalize_jsonpath(jsonpath) - p = _jsonpath_parse_cache(jsonpath) + p = _jsonpath_parse(jsonpath) matches = p.find(data) if matches: @@ -179,7 +183,7 @@ def jsonpath_replace(data, value, jsonpath, pattern=None): 'or "$"' % jsonpath) def _do_replace(): - p = _jsonpath_parse_cache(jsonpath) + p = _jsonpath_parse(jsonpath) p_to_change = p.find(data) if p_to_change: diff --git a/deckhand/conf/config.py b/deckhand/conf/config.py index 5b89da74..78f74ae8 100644 --- a/deckhand/conf/config.py +++ b/deckhand/conf/config.py @@ -21,9 +21,8 @@ CONF = cfg.CONF barbican_group = cfg.OptGroup( name='barbican', title='Barbican Options', - help=""" -Barbican options for allowing Deckhand to communicate with Barbican. -""") + help="Barbican options for allowing Deckhand to communicate with " + "Barbican.") barbican_opts = [ # TODO(fmontei): Drop these options and related group once Keystone @@ -35,6 +34,19 @@ barbican_opts = [ ] +jsonpath_group = cfg.OptGroup( + name='jsonpath', + title='JSONPath Options', + help="JSONPath options for allowing JSONPath logic to be configured.") + + +jsonpath_opts = [ + cfg.IntOpt('cache_timeout', default=3600, + help="How long JSONPath lookup results should remain cached " + "in memory.") +] + + default_opts = [ cfg.BoolOpt('profiler', default=False, help="Enables profiling of API requests. Do NOT use in " @@ -48,6 +60,7 @@ default_opts = [ def register_opts(conf): conf.register_group(barbican_group) conf.register_opts(barbican_opts, group=barbican_group) + conf.register_opts(jsonpath_opts, group=jsonpath_group) conf.register_opts(default_opts) ks_loading.register_auth_conf_options(conf, group='keystone_authtoken') ks_loading.register_auth_conf_options(conf, group=barbican_group.name) @@ -68,7 +81,8 @@ def list_opts(): ks_loading.get_session_conf_options() + ks_loading.get_auth_common_conf_options() + ks_loading.get_auth_plugin_conf_options('v3password') - ) + ), + jsonpath_group: jsonpath_opts } return opts diff --git a/deckhand/tests/unit/common/test_utils.py b/deckhand/tests/unit/common/test_utils.py index 68ca439d..1c645827 100644 --- a/deckhand/tests/unit/common/test_utils.py +++ b/deckhand/tests/unit/common/test_utils.py @@ -12,11 +12,18 @@ # See the License for the specific language governing permissions and # limitations under the License. +import jsonpath_ng +import mock + +from testtools.matchers import Equals +from testtools.matchers import MatchesAny + from deckhand.common import utils from deckhand.tests.unit import base as test_base -class TestUtils(test_base.DeckhandTestCase): +class TestJSONPathReplace(test_base.DeckhandTestCase): + """Validate that JSONPath replace function works.""" def test_jsonpath_replace_creates_object(self): path = ".values.endpoints.admin" @@ -40,3 +47,43 @@ class TestUtils(test_base.DeckhandTestCase): expected = {'values': {'endpoints0': {'admin': 'foo'}}} result = utils.jsonpath_replace({}, 'foo', path) self.assertEqual(expected, result) + + +class TestJSONPathUtilsCaching(test_base.DeckhandTestCase): + """Validate that JSONPath caching works.""" + + def setUp(self): + super(TestJSONPathUtilsCaching, self).setUp() + self.jsonpath_call_count = 0 + + def fake_parse(value): + self.jsonpath_call_count += 1 + return jsonpath_ng.parse(value) + + self.fake_jsonpath_ng = fake_parse + + def test_jsonpath_parse_replace_cache(self): + """Validate caching for both parsing and replacing functions.""" + path = ".values.endpoints.admin" + expected = {'values': {'endpoints': {'admin': 'foo'}}} + + # Mock jsonpath_ng to return a monkey-patched parse function that + # keeps track of call count and yet calls the actual function. + with mock.patch.object(utils, 'jsonpath_ng', # noqa: H210 + parse=self.fake_jsonpath_ng): + # Though this is called 3 times, the cached function should only + # be called once, with the cache returning the cached value early. + for _ in range(3): + result = utils.jsonpath_replace({}, 'foo', path) + self.assertEqual(expected, result) + + # Though this is called 3 times, the cached function should only + # be called once, with the cache returning the cached value early. + for _ in range(3): + result = utils.jsonpath_parse(expected, path) + self.assertEqual('foo', result) + + # Assert that the actual function was called <= 1 times. (Allow for 0 + # in case CI jobs clash.) + self.assertThat( + self.jsonpath_call_count, MatchesAny(Equals(0), Equals(1))) diff --git a/requirements.txt b/requirements.txt index bf0099b2..b9bda38d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,6 +23,7 @@ psycopg2==2.7.4 uwsgi==2.0.17 jsonpath-ng==1.4.3 jsonschema==2.6.0 +beaker==1.9.1 oslo.cache>=1.30.1 # Apache-2.0 oslo.concurrency>=3.27.0 # Apache-2.0