This is an automated email from the ASF dual-hosted git repository.

gancho pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 244288f  cachekey/cacheurl key string compatibility enhancements.
244288f is described below

commit 244288fab01bdad823f9de19dcece62a7e2a0c11
Author: Gancho Tenev <gan...@apache.com>
AuthorDate: Mon Jul 31 14:43:33 2017 -0700

    cachekey/cacheurl key string compatibility enhancements.
    
    - Added a parameter to override the default cache key element separator.
      --separator=<string>
    
    - Added flags allowing path and prefix not to be appended by default.
      --remove-path=<true|false|yes|no|0|1>
      --remove-prefix=<true|false|yes|no|0|1>
    
    - Documentated the new features and provided cacheurl to cachekey 
migrationion examples.
    
    - If /replacement/ in <capture_definition> is empty don't add any captures, 
i.e.
      --capture-path=/.*// - would remove the whole path (nothing captured);
      --capture-prefix=/(.*)// - would remove the whole prefix (nothing 
captured);
---
 doc/admin-guide/plugins/cachekey.en.rst   | 52 +++++++++++++++++++++++++++++++
 plugins/experimental/cachekey/cachekey.cc | 13 ++++----
 plugins/experimental/cachekey/cachekey.h  |  5 +--
 plugins/experimental/cachekey/configs.cc  | 40 +++++++++++++++++++++++-
 plugins/experimental/cachekey/configs.h   | 26 +++++++++++++++-
 plugins/experimental/cachekey/pattern.cc  | 24 ++++++++------
 plugins/experimental/cachekey/pattern.h   |  5 ++-
 plugins/experimental/cachekey/plugin.cc   | 12 ++++---
 8 files changed, 151 insertions(+), 26 deletions(-)

diff --git a/doc/admin-guide/plugins/cachekey.en.rst 
b/doc/admin-guide/plugins/cachekey.en.rst
index 75e134d..39f7c55 100644
--- a/doc/admin-guide/plugins/cachekey.en.rst
+++ b/doc/admin-guide/plugins/cachekey.en.rst
@@ -74,6 +74,8 @@ Cache key structure and related plugin parameters
 * ``--capture-prefix=<capture_definition>`` (default: empty string) - if 
specified and not empty then strings are captured from ``host:port`` based on 
the ``<capture_definition>`` and are added to the cache key.
 * ``--capture-prefix-uri=<capture_definition>`` (default: empty string) - if 
specified and not empty then strings are captured from the entire URI based on 
the ``<capture_definition>`` and are added to the cache key.
 * If any of the "Prefix" related plugin parameters are used together in the 
plugin configuration they are added to the cache key in the order shown in the 
diagram.
+* ``--remove-prefix=<true|false|yes|no|0|1`` (default: false) - if specified 
the prefix elements (host, port) are not processed nor appended to the 
cachekey. All prefix related plugin paramenters are ignored if this parameter 
is ``true``, ``yes`` or ``1``.
+
 
 
 "User-Agent" section
@@ -139,6 +141,7 @@ Cache key structure and related plugin parameters
 * if no path related plugin parameters are used, the URI path string is 
included in the cache key.
 * ``--capture-path=<capture_definition>`` (default: empty string) - if 
specified and not empty then strings are captured from URI path based on the 
``<capture_definition>`` and are added to the cache key.
 * ``--capture-path-uri=<capture_definition>`` (default: empty string) - if 
specified and not empty then strings are captured from the entire URI based on 
the ``<capture_definition>`` and are added to the cache key.
+* ``--remove-path=<true|false|yes|no|0|1`` (default: false) - if specified the 
HTTP URI path element is not processed nor appended to the cachekey. All path 
related plugin paramenters are ignored if this parameter is ``true``, ``yes`` 
or ``1``.
 
 "Query" section
 ^^^^^^^^^^^^^^^
@@ -162,6 +165,11 @@ All parameters are optional, and if not used, their 
default values are as mentio
     * ``/<regex>/<replacement>/`` - ``<regex>`` defines regex capturing 
groups, ``<replacement>`` defines a pattern where the captured strings 
referenced with ``$0`` ... ``$9`` will be substituted and the result will be 
added to the cache key.
 
 
+Cache key elements separator
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+* ``--separator=<string>`` - the cache key is constructed by extracting 
elements from HTTP URI and headers or by using the UA classifiers and they are 
appended during the key construction and separated by ``/`` (by default). This 
options allows to override the dafault separator to any string (including an 
empty string)
+
+
 Detailed examples and troubleshooting
 =====================================
 
@@ -504,3 +512,47 @@ and if ``tool_agents.config`` contains: ::
   ^curl.*
 
 then ``browser`` will be used when constructing the key.
+
+
+Cacheurl plugin to cachekey plugin migration
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+The plugin `cachekey` was not meant to replace the cacheurl plugin in terms of 
having exactly the same cache key strings generated. It just allows the 
operator to exctract elements from the HTTP URI in the same way the `cacheurl` 
does (through a regular expression, please see `<capture_definition>` above).
+
+The following examples demonstrate different ways to achieve `cacheurl` 
compatibility on a cache key string level in order to avoid invalidation of the 
cache. 
+
+The operator could use `--capture-path-uri`, `--capture-path`, 
`--capture-prefix-uri`, `--capture-prefix` to capture elements from the URI, 
path and authority elements.
+
+By using `--separator=<string>` the operator could override the default 
separator to an empty string `--separator=` and thus make sure there are no 
cache key element separators.
+
+
+Example 1: Let us say we have a capture definition used in `cacheurl`. Now by 
using `--capture-prefix-uri` one could extract elements through the same 
caplture definition used with `cacheurl`, remove the cache key element 
separator `--separator=` and by using `--capture-path-uri` could remove the URI 
path and by using `--remove-all-params=true` could remove the query string::
+
+  @plugin=cachekey.so \
+      @pparam=--capture-prefix-uri=/.*/$0/ \
+      @pparam=--capture-path-uri=/.*// \
+      @pparam=--remove-all-params=true \
+      @pparam=--separator=
+
+Example 2: A more efficient way would be achieved by using 
`--capture-prefix-uri` to capture from the URI, remove the cache key element 
separator `--separator=`  and by using `--remove-path` to remove the URI path 
and `--remove-all-params=true` to remove the query string::
+
+  @plugin=cachekey.so \
+      @pparam=--capture-prefix-uri=/.*/$0/ \
+      @pparam=--remove-path=true \
+      @pparam=--remove-all-params=true \
+      @pparam=--separator=
+
+Example 3: Same result as the above but this time by using 
`--capture-path-uri` to capture from the URI, remove the cache key element 
separator `--separator=` and by using `--remove-prefix` to remove the URI 
authority elements and by using `--remove-all-params=true` to remove the query 
string::
+
+    @plugin=cachekey.so \
+        @pparam=--capture-path-uri=/(.*)/$0/ \
+        @pparam=--remove-prefix=true \
+        @pparam=--remove-all-params=true \
+        @pparam=--separator=
+
+Example 4: Let us say that we would like to capture from URI in similar to 
`cacheurl` way but also sort the query parameters (which is not supported by 
`cacheurl`). We could achieve that by using `--capture-prefix-uri` to capture 
by using a caplture definition to process the URI before `?`  and using 
`--remove-path` to remove the URI path and `--sort-params=true` to sort the 
query parameters::
+
+    @plugin=cachekey.so \
+        @pparam=--capture-prefix-uri=/([^?]*)/$1/ \
+        @pparam=--remove-path=true \
+        @pparam=--sort-params=true \
+        @pparam=--separator=
diff --git a/plugins/experimental/cachekey/cachekey.cc 
b/plugins/experimental/cachekey/cachekey.cc
index 8a6d62b..2a0b140 100644
--- a/plugins/experimental/cachekey/cachekey.cc
+++ b/plugins/experimental/cachekey/cachekey.cc
@@ -179,7 +179,8 @@ classifyUserAgent(const Classifier &c, TSMBuffer buf, 
TSMLoc hdrs, String &class
  * @param url URI handle
  * @param hdrs headers handle
  */
-CacheKey::CacheKey(TSHttpTxn txn, TSMBuffer buf, TSMLoc url, TSMLoc hdrs) : 
_txn(txn), _buf(buf), _url(url), _hdrs(hdrs)
+CacheKey::CacheKey(TSHttpTxn txn, TSMBuffer buf, TSMLoc url, TSMLoc hdrs, 
String separator)
+  : _txn(txn), _buf(buf), _url(url), _hdrs(hdrs), _separator(separator)
 {
   _key.reserve(512);
 }
@@ -191,7 +192,7 @@ CacheKey::CacheKey(TSHttpTxn txn, TSMBuffer buf, TSMLoc 
url, TSMLoc hdrs) : _txn
 void
 CacheKey::append(unsigned n)
 {
-  _key.append("/");
+  _key.append(_separator);
   ::append(_key, n);
 }
 
@@ -202,7 +203,7 @@ CacheKey::append(unsigned n)
 void
 CacheKey::append(const String &s)
 {
-  _key.append("/");
+  _key.append(_separator);
   ::appendEncoded(_key, s.data(), s.size());
 }
 
@@ -213,7 +214,7 @@ CacheKey::append(const String &s)
 void
 CacheKey::append(const char *s)
 {
-  _key.append("/");
+  _key.append(_separator);
   ::appendEncoded(_key, s, strlen(s));
 }
 
@@ -225,7 +226,7 @@ CacheKey::append(const char *s)
 void
 CacheKey::append(const char *s, unsigned n)
 {
-  _key.append("/");
+  _key.append(_separator);
   ::appendEncoded(_key, s, n);
 }
 
@@ -417,7 +418,7 @@ CacheKey::appendHeaders(const ConfigHeaders &config)
   }
 
   /* It doesn't make sense to have the headers unordered in the cache key. */
-  String headers_key = containerToString<StringSet, 
StringSet::const_iterator>(hset, "", "/");
+  String headers_key = containerToString<StringSet, 
StringSet::const_iterator>(hset, "", _separator);
   if (!headers_key.empty()) {
     append(headers_key);
   }
diff --git a/plugins/experimental/cachekey/cachekey.h 
b/plugins/experimental/cachekey/cachekey.h
index 61ea097..fb2337a 100644
--- a/plugins/experimental/cachekey/cachekey.h
+++ b/plugins/experimental/cachekey/cachekey.h
@@ -49,7 +49,7 @@
 class CacheKey
 {
 public:
-  CacheKey(TSHttpTxn txn, TSMBuffer buf, TSMLoc url, TSMLoc hdrs);
+  CacheKey(TSHttpTxn txn, TSMBuffer buf, TSMLoc url, TSMLoc hdrs, String 
separator);
 
   void append(unsigned number);
   void append(const String &);
@@ -77,7 +77,8 @@ private:
   TSMLoc _url;    /**< @brief URI handle */
   TSMLoc _hdrs;   /**< @brief headers handle */
 
-  String _key; /**< @brief cache key */
+  String _key;       /**< @brief cache key */
+  String _separator; /**< @brief a separator used to separate the cache key 
elements extracted from the URI */
 };
 
 #endif /* PLUGINS_EXPERIMENTAL_CACHEKEY_CACHEKEY_H_ */
diff --git a/plugins/experimental/cachekey/configs.cc 
b/plugins/experimental/cachekey/configs.cc
index b5fb65a..788b56d 100644
--- a/plugins/experimental/cachekey/configs.cc
+++ b/plugins/experimental/cachekey/configs.cc
@@ -43,7 +43,7 @@ commaSeparateString(ContainerType &c, const String &input)
 static bool
 isTrue(const char *arg)
 {
-  return (0 == strncasecmp("true", arg, 4) || 0 == strncasecmp("1", arg, 1) || 
0 == strncasecmp("yes", arg, 3));
+  return (nullptr == arg || 0 == strncasecmp("true", arg, 4) || 0 == 
strncasecmp("1", arg, 1) || 0 == strncasecmp("yes", arg, 3));
 }
 
 void
@@ -341,6 +341,9 @@ Configs::init(int argc, char *argv[])
     {const_cast<char *>("capture-prefix-uri"), optional_argument, nullptr, 
'n'},
     {const_cast<char *>("capture-path"), optional_argument, nullptr, 'o'},
     {const_cast<char *>("capture-path-uri"), optional_argument, nullptr, 'p'},
+    {const_cast<char *>("remove-prefix"), optional_argument, nullptr, 'q'},
+    {const_cast<char *>("remove-path"), optional_argument, nullptr, 'r'},
+    {const_cast<char *>("separator"), optional_argument, nullptr, 's'},
     {nullptr, 0, nullptr, 0},
   };
 
@@ -430,6 +433,15 @@ Configs::init(int argc, char *argv[])
         status = false;
       }
       break;
+    case 'q': /* remove-prefix */
+      _prefixToBeRemoved = isTrue(optarg);
+      break;
+    case 'r': /* remove-path */
+      _pathToBeRemoved = isTrue(optarg);
+      break;
+    case 's': /* separator */
+      setSeparator(optarg);
+      break;
     }
   }
 
@@ -448,3 +460,29 @@ Configs::finalize()
 {
   return _query.finalize() && _headers.finalize() && _cookies.finalize();
 }
+
+bool
+Configs::prefixToBeRemoved()
+{
+  return _prefixToBeRemoved;
+}
+
+bool
+Configs::pathToBeRemoved()
+{
+  return _pathToBeRemoved;
+}
+
+void
+Configs::setSeparator(const char *arg)
+{
+  if (nullptr != arg) {
+    _separator.assign(arg);
+  }
+}
+
+const String &
+Configs::getSeparator()
+{
+  return _separator;
+}
diff --git a/plugins/experimental/cachekey/configs.h 
b/plugins/experimental/cachekey/configs.h
index f1376fe..9bf71cb 100644
--- a/plugins/experimental/cachekey/configs.h
+++ b/plugins/experimental/cachekey/configs.h
@@ -122,7 +122,7 @@ private:
 class Configs
 {
 public:
-  Configs() {}
+  Configs() : _prefixToBeRemoved(false), _pathToBeRemoved(false), 
_separator("/") {}
   /**
    * @brief initializes plugin configuration.
    * @param argc number of plugin parameters
@@ -137,6 +137,26 @@ public:
    */
   bool finalize();
 
+  /**
+   * @brief Tells the caller if the prefix is to be removed (not processed at 
all).
+   */
+  bool prefixToBeRemoved();
+
+  /**
+   * @brief Tells the caller if the path is to be removed (not processed at 
all).
+   */
+  bool pathToBeRemoved();
+
+  /**
+   * @brief set the cache key elements separator string.
+   */
+  void setSeparator(const char *arg);
+
+  /**
+   * @brief get the cache key elements separator string.
+   */
+  const String &getSeparator();
+
   /* Make the following members public to avoid unnecessary accessors */
   ConfigQuery _query;        /**< @brief query parameter related configuration 
*/
   ConfigHeaders _headers;    /**< @brief headers related configuration */
@@ -157,6 +177,10 @@ private:
    * @return true if successful, false otherwise.
    */
   bool loadClassifiers(const String &args, bool blacklist = true);
+
+  bool _prefixToBeRemoved; /**< @brief instructs the prefix (i.e. host:port) 
not to added to the cache key */
+  bool _pathToBeRemoved;   /**< @brief instructs the path not to added to the 
cache key */
+  String _separator;       /**< @brief a separator used to separate the cache 
key elements extracted from the URI */
 };
 
 #endif // PLUGINS_EXPERIMENTAL_CACHEKEY_CONFIGS_H_
diff --git a/plugins/experimental/cachekey/pattern.cc 
b/plugins/experimental/cachekey/pattern.cc
index 35ef0dd..40dfae7 100644
--- a/plugins/experimental/cachekey/pattern.cc
+++ b/plugins/experimental/cachekey/pattern.cc
@@ -38,7 +38,7 @@ replaceString(String &str, const String &from, const String 
&to)
   }
 }
 
-Pattern::Pattern() : _re(nullptr), _extra(nullptr), _pattern(""), 
_replacement(""), _tokenCount(0)
+Pattern::Pattern() : _re(nullptr), _extra(nullptr), _pattern(""), 
_replacement(""), _replace(false), _tokenCount(0)
 {
 }
 
@@ -49,12 +49,13 @@ Pattern::Pattern() : _re(nullptr), _extra(nullptr), 
_pattern(""), _replacement("
  * @return true if successful, false if failure
  */
 bool
-Pattern::init(const String &pattern, const String &replacenemt)
+Pattern::init(const String &pattern, const String &replacenemt, bool replace)
 {
   pcreFree();
 
   _pattern.assign(pattern);
   _replacement.assign(replacenemt);
+  _replace = replace;
 
   _tokenCount = 0;
 
@@ -115,9 +116,9 @@ Pattern::init(const String &config)
     ::replaceString(pattern, "\\/", "/");
     ::replaceString(replacement, "\\/", "/");
 
-    return this->init(pattern, replacement);
+    return this->init(pattern, replacement, /* replace */ true);
   } else {
-    return this->init(config, "");
+    return this->init(config, /* replacement */ "", /*replace */ false);
   }
 
   /* Should never get here. */
@@ -170,7 +171,7 @@ Pattern::~Pattern()
 bool
 Pattern::process(const String &subject, StringVector &result)
 {
-  if (!_replacement.empty()) {
+  if (_replace) {
     /* Replacement pattern was provided in the configuration - capture and 
replace. */
     String element;
     if (replace(subject, element)) {
@@ -235,9 +236,10 @@ Pattern::capture(const String &subject, StringVector 
&result)
   int matchCount;
   int ovector[OVECOUNT];
 
-  CacheKeyDebug("matching '%s' to '%s'", _pattern.c_str(), subject.c_str());
+  CacheKeyDebug("capturing '%s' from '%s'", _pattern.c_str(), subject.c_str());
 
   if (!_re) {
+    CacheKeyError("regular expression not initialized");
     return false;
   }
 
@@ -274,9 +276,10 @@ Pattern::replace(const String &subject, String &result)
   int matchCount;
   int ovector[OVECOUNT];
 
-  CacheKeyDebug("matching '%s' to '%s'", _pattern.c_str(), subject.c_str());
+  CacheKeyDebug("replacing:'%s' in pattern:'%s', subject:'%s'", 
_replacement.c_str(), _pattern.c_str(), subject.c_str());
 
-  if (!_re) {
+  if (!_re || !_replace) {
+    CacheKeyError("regular expression not initialized or not configured to 
replace");
     return false;
   }
 
@@ -330,7 +333,8 @@ Pattern::compile()
   const char *errPtr; /* PCRE error */
   int errOffset;      /* PCRE error offset */
 
-  CacheKeyDebug("compiling pattern:'%s', replacement:'%s'", _pattern.c_str(), 
_replacement.c_str());
+  CacheKeyDebug("compiling pattern:'%s', replace: %s, replacement:'%s'", 
_pattern.c_str(), _replace ? "true" : "false",
+                _replacement.c_str());
 
   _re = pcre_compile(_pattern.c_str(), /* the pattern */
                      0,                /* options */
@@ -354,7 +358,7 @@ Pattern::compile()
     return false;
   }
 
-  if (_replacement.empty()) {
+  if (!_replace) {
     /* No replacement necessary - we are done. */
     return true;
   }
diff --git a/plugins/experimental/cachekey/pattern.h 
b/plugins/experimental/cachekey/pattern.h
index 0ddd383..1448862 100644
--- a/plugins/experimental/cachekey/pattern.h
+++ b/plugins/experimental/cachekey/pattern.h
@@ -46,7 +46,7 @@ public:
   Pattern();
   virtual ~Pattern();
 
-  bool init(const String &pattern, const String &replacenemt);
+  bool init(const String &pattern, const String &replacenemt, bool replace);
   bool init(const String &config);
   bool empty() const;
   bool match(const String &subject);
@@ -64,6 +64,9 @@ private:
   String _pattern;     /**< @brief PCRE pattern string, containing PCRE 
patterns and capturing groups. */
   String _replacement; /**< @brief PCRE replacement string, containing $0..$9 
to be replaced with content of the capturing groups */
 
+  bool _replace; /**< @brief true if a replacement is needed, false if not, 
this is to distinguish between an empty replacement
+                    string and no replacement needed case */
+
   int _tokenCount;              /**< @brief number of replacements $0..$9 
found in the replacement string if not empty */
   int _tokens[TOKENCOUNT];      /**< @brief replacement index 0..9, since they 
can be used in the replacement string in any order */
   int _tokenOffset[TOKENCOUNT]; /**< @brief replacement offset inside the 
replacement string */
diff --git a/plugins/experimental/cachekey/plugin.cc 
b/plugins/experimental/cachekey/plugin.cc
index 2378264..d413bd3 100644
--- a/plugins/experimental/cachekey/plugin.cc
+++ b/plugins/experimental/cachekey/plugin.cc
@@ -92,11 +92,12 @@ TSRemapDoRemap(void *instance, TSHttpTxn txn, 
TSRemapRequestInfo *rri)
 
   if (nullptr != config) {
     /* Initial cache key facility from the requested URL. */
-    CacheKey cachekey(txn, rri->requestBufp, rri->requestUrl, 
rri->requestHdrp);
+    CacheKey cachekey(txn, rri->requestBufp, rri->requestUrl, 
rri->requestHdrp, config->getSeparator());
 
     /* Append custom prefix or the host:port */
-    cachekey.appendPrefix(config->_prefix, config->_prefixCapture, 
config->_prefixCaptureUri);
-
+    if (!config->prefixToBeRemoved()) {
+      cachekey.appendPrefix(config->_prefix, config->_prefixCapture, 
config->_prefixCaptureUri);
+    }
     /* Classify User-Agent and append the class name to the cache key if 
matched. */
     cachekey.appendUaClass(config->_classifier);
 
@@ -110,8 +111,9 @@ TSRemapDoRemap(void *instance, TSHttpTxn txn, 
TSRemapRequestInfo *rri)
     cachekey.appendCookies(config->_cookies);
 
     /* Append the path to the cache key. */
-    cachekey.appendPath(config->_pathCapture, config->_pathCaptureUri);
-
+    if (!config->pathToBeRemoved()) {
+      cachekey.appendPath(config->_pathCapture, config->_pathCaptureUri);
+    }
     /* Append query parameters to the cache key. */
     cachekey.appendQuery(config->_query);
 

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Reply via email to