Add distinct store/update methods to rlm_cache

diff --git a/raddb/mods-available/cache b/raddb/mods-available/cache
index a5314d938d3dd6eaea2bdc005a4e1ba3d594bfd2..1d5a8b64cd9503e687bcac0a7ed05bbe61e9d788 100644 (file)
--- a/raddb/mods-available/cache
+++ b/raddb/mods-available/cache
@@ -314,6 +314,7 @@ cache {
        #  | Return     | Description
        #  | `ok`       | if a cache entry was found.
        #  | `notfound` | if no cache entry was found.
+       #  | `fail`     | if the cache was unavailable.
        #  |===
        #
        #  cache.load:: Load an existing cache entry and merge it into the request.
@@ -323,16 +324,27 @@ cache {
        #  | Return     | Description
        #  | `updated`  | if a cache entry was found and loaded.
        #  | `notfound` | if no cache entry was found.
+       #  | `fail`     | if the cache was unavailable.
        #  |===
        #
-       #  cache.store:: Perform an upset against the data store. (Not affect the existing
-       #  request).
+       #  cache.update:: Perform an upsert against the data store, updating the entry TTL
        #
        #  [options="header,autowidth"]
        #  |===
        #  | Return     | Description
        #  | `updated`  | if we added cache entry.
-       #  | `noop`     | if a cache entry ready exists.
+       #  | `fail`     | if the cache was unavailable.
+       #  |===
+       #
+       #  cache.store:: Inserts data into the cache if, and only if, it is not already present
+       #  Will not update the entry TTL.
+       #
+       #  [options="header,autowidth"]
+       #  |===
+       #  | Return     | Description
+       #  | `updated`  | we created or updated a cache entry.
+       #  | `noop`     | if a cache entry aready existed.
+       #  | `fail`     | if the cache was unavailable.
        #  |===
        #
        #  cache.clear:: Delete cache entry from the data store without checking if the entry
@@ -341,8 +353,9 @@ cache {
        #  [options="header,autowidth"]
        #  |===
        #  | Return     | Description
-       #  | `ok`       | if we found and remove a entry.
+       #  | `ok`       | if we found and removed a entry.
        #  | `notfound` | if no cache entry was found.
+       #  | `fail`     | if the cache was unavailable.
        #  |===
        #
        #  cache.ttl:: Change the TTL on an existing entry.
@@ -352,6 +365,7 @@ cache {
        #  | Return     | Description
        #  | `updated`  | if we found entry and updated the ttl.
        #  | `notfound` | if no cache entry was found.
+       #  | `fail`     | if the cache was unavailable.
        #  |===
        #
        #  ### Examples

diff --git a/raddb/sites-available/default b/raddb/sites-available/default
index b9916b4eaa57d00f58a934a5db947312ab0b7e79..1bf0b37aaf76b0ee11dac8f5a2873c52e896b5f2 100644 (file)
--- a/raddb/sites-available/default
+++ b/raddb/sites-available/default
@@ -759,9 +759,6 @@ recv Access-Request {
        #  We also recommend doing user lookups in the `inner-tunnel`
        #  virtual server.
        #
-       eap {
-               ok = return
-       }
 
        #
        #  The `unix` module will obtain passwords from `/etc/passwd`
@@ -969,7 +966,6 @@ authenticate ldap {
 #  For EAP-MD5, EAP-MSCHAP, EAP-TLS, EAP-TTLS, EAP-PEAP, EAP-PWD, etc.
 #
 authenticate eap {
-       eap
 }
 
 
@@ -1072,7 +1068,6 @@ send Access-Accept {
        #  For EAP, ensure that the Access-Accept contains a User-Name
        #  attribute.
        #
-       eap
 
        #
        #  Get an address from the IP Pool.
@@ -1231,7 +1226,6 @@ send Access-Reject {
        #  Insert an EAP-Failure message if the request was rejected by
        #  policy, instead of from an authentication failure.
        #
-       eap
 
        #
        #  Call an instance of `linelog` to log the authentication failure

diff --git a/src/modules/rlm_cache/rlm_cache.c b/src/modules/rlm_cache/rlm_cache.c
index 2b0f8e834ee7d1d4afecfa84c8c47c139481695e..58755bef390cc376f8e3b770e215787886675b8a 100644 (file)
--- a/src/modules/rlm_cache/rlm_cache.c
+++ b/src/modules/rlm_cache/rlm_cache.c
@@ -934,8 +934,6 @@ static unlang_action_t CC_HINT(nonnull) mod_method_status(rlm_rcode_t *p_result,
        rlm_cache_entry_t       *entry = NULL;
        rlm_cache_handle_t      *handle = NULL;
 
-       DEBUG3("Calling %s.status", mctx->inst->name);
-
        if (env->key->vb_length == 0) {
                REDEBUG("Zero length key string is invalid");
                RETURN_MODULE_FAIL;
@@ -974,8 +972,6 @@ static unlang_action_t CC_HINT(nonnull) mod_method_load(rlm_rcode_t *p_result, m
        rlm_cache_entry_t       *entry = NULL;
        rlm_cache_handle_t      *handle = NULL;
 
-       DEBUG3("Calling %s.load", mctx->inst->name);
-
        if (env->key->vb_length == 0) {
                REDEBUG("Zero length key string is invalid");
                RETURN_MODULE_FAIL;
@@ -1003,14 +999,14 @@ finish:
        RETURN_MODULE_RCODE(rcode);
 }
 
-/** Create and insert a cache entry
+/** Create, or update a cache entry
  *
  * @return
  *     - #RLM_MODULE_OK on success.
  *     - #RLM_MODULE_UPDATED if we merged the cache entry.
  *     - #RLM_MODULE_FAIL on failure.
  */
-static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request)
+static unlang_action_t CC_HINT(nonnull) mod_method_update(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request)
 {
        rlm_cache_t const       *inst = talloc_get_type_abort(mctx->inst->data, rlm_cache_t);
        cache_call_env_t        *env = talloc_get_type_abort(mctx->env_data, cache_call_env_t);
@@ -1021,8 +1017,6 @@ static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result,
        rlm_cache_handle_t      *handle = NULL;
        fr_pair_t               *vp;
 
-       DEBUG3("Calling %s.store", mctx->inst->name);
-
        if (env->key->vb_length == 0) {
                REDEBUG("Zero length key string is invalid");
                RETURN_MODULE_FAIL;
@@ -1074,12 +1068,77 @@ static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result,
         *      should perform upserts.
         */
        if (expire) {
-               DEBUG4("Set the cache expire");
+               DEBUG3("Expiring cache entry");
 
                cache_expire(&rcode, inst, request, &handle, env->key);
                if (rcode == RLM_MODULE_FAIL) goto finish;
        }
 
+       /*
+        *      Inserts are upserts, so we don't care about the
+        *      entry state.
+        */
+       cache_insert(&rcode, inst, request, &handle, env->key, env->maps, ttl);
+       if (rcode == RLM_MODULE_OK) rcode = RLM_MODULE_UPDATED;
+
+finish:
+       cache_unref(request, inst, entry, handle);
+
+       RETURN_MODULE_RCODE(rcode);
+}
+
+/** Create, or update a cache entry
+ *
+ * @return
+ *     - #RLM_MODULE_NOOP if an entry already existed.
+ *     - #RLM_MODULE_UPDATED if we inserted a cache entry.
+ *     - #RLM_MODULE_FAIL on failure.
+ */
+static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request)
+{
+       rlm_cache_t const       *inst = talloc_get_type_abort(mctx->inst->data, rlm_cache_t);
+       cache_call_env_t        *env = talloc_get_type_abort(mctx->env_data, cache_call_env_t);
+       rlm_rcode_t             rcode = RLM_MODULE_NOOP;
+       fr_time_delta_t         ttl;
+       rlm_cache_entry_t       *entry = NULL;
+       rlm_cache_handle_t      *handle = NULL;
+       fr_pair_t               *vp;
+
+       if (env->key->vb_length == 0) {
+               REDEBUG("Zero length key string is invalid");
+               RETURN_MODULE_FAIL;
+       }
+
+       if (cache_acquire(&handle, inst, request) < 0) {
+               RETURN_MODULE_FAIL;
+       }
+
+       /* Process the TTL */
+       ttl = inst->config.ttl; /* Set the default value from cache { ttl=... } */
+       vp = fr_pair_find_by_da(&request->control_pairs, NULL, attr_cache_ttl);
+       if (vp && (vp->vp_int32 > 0)) {
+               ttl = fr_time_delta_from_sec(vp->vp_int32);
+
+               DEBUG3("Overriding default TTL %pV -> %d", fr_box_time_delta(ttl), vp->vp_int32);
+       }
+
+       /*
+        *      We can only alter the TTL on an entry if it exists.
+        */
+       cache_find(&rcode, &entry, inst, request, &handle, env->key);
+       switch (rcode) {
+       default:
+       case RLM_MODULE_OK:
+               rcode = RLM_MODULE_NOOP;
+               goto finish;
+
+       case RLM_MODULE_FAIL:
+               goto finish;
+
+       case RLM_MODULE_NOTFOUND:
+               break;
+       }
+
        /*
         *      Inserts are upserts, so we don't care about the
         *      entry state, just that we're not meant to be
@@ -1087,10 +1146,10 @@ static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result,
         *      insert.
         */
        cache_insert(&rcode, inst, request, &handle, env->key, env->maps, ttl);
-       if (rcode == RLM_MODULE_OK) rcode = RLM_MODULE_UPDATED;
 
 finish:
        cache_unref(request, inst, entry, handle);
+       if (rcode == RLM_MODULE_OK) rcode = RLM_MODULE_UPDATED;
 
        RETURN_MODULE_RCODE(rcode);
 }
@@ -1342,6 +1401,7 @@ module_rlm_t rlm_cache = {
        .method_names = (module_method_name_t[]){
                { .name1 = "status", .name2 = CF_IDENT_ANY,             .method = mod_method_status,    .method_env = &cache_method_env },
                { .name1 = "load", .name2 = CF_IDENT_ANY,               .method = mod_method_load,      .method_env = &cache_method_env },
+               { .name1 = "update", .name2 = CF_IDENT_ANY,             .method = mod_method_update,    .method_env = &cache_method_env },
                { .name1 = "store", .name2 = CF_IDENT_ANY,              .method = mod_method_store,     .method_env = &cache_method_env },
                { .name1 = "clear", .name2 = CF_IDENT_ANY,              .method = mod_method_clear,     .method_env = &cache_method_env },
                { .name1 = "ttl", .name2 = CF_IDENT_ANY,                .method = mod_method_ttl,       .method_env = &cache_method_env },

diff --git a/src/tests/modules/cache_rbtree/cache-method-logic.unlang b/src/tests/modules/cache_rbtree/cache-method-logic.unlang
index a526452782eebfd92effb92d017d2c460896056a..f39efbbeb3ba6eb19278509c5874369d2e86c0c8 100644 (file)
--- a/src/tests/modules/cache_rbtree/cache-method-logic.unlang
+++ b/src/tests/modules/cache_rbtree/cache-method-logic.unlang
@@ -4,11 +4,11 @@
 &Filter-Id := 'testkey1'
 
 #
-# 0.  Basic store and retrieve
+# 0.  Basic update and retrieve
 #
 &control.Callback-Id := 'cache me'
 
-cache.store
+cache.update
 if (!updated) {
        test_fail
 }
@@ -63,13 +63,13 @@ if (!notfound) {
 # 14. This should still allow the creation of a new entry
 &control.Cache-TTL := -2
 
-cache.store
+cache.update
 if (!updated) {
        test_fail
 }
 
 # 12. We have nothing to do if it is ready added.
-cache.store
+cache.update
 if (!updated) {
        test_fail
 }
@@ -102,7 +102,7 @@ if (&Callback-Id == &control.Callback-Id) {
 # 20. Check that a new entry is created
 &control.Cache-TTL := -2
 
-cache.store
+cache.update
 if (!updated) {
        test_fail
 }
@@ -128,7 +128,7 @@ if (&Callback-Id != &control.Callback-Id) {
 &control.Cache-TTL := -2
 &control.Cache-Merge-New := yes
 
-cache.store
+cache.update
 if (!updated) {
        test_fail
 }
@@ -148,4 +148,21 @@ if (&Cache-Entry-Hits != 1) {
        test_fail
 }
 
+# 27. Try and store an existing entry, should do nothing
+cache.store
+if (!noop) {
+       test_fail
+}
+
+# 28. But with the entry removed, we can now create a new entry
+cache.clear
+if (!ok) {
+       test_fail
+}
+
+cache.store
+if (!updated) {
+       test_fail
+}
+
 test_pass

diff --git a/src/tests/modules/cache_rbtree/cache-method-update.unlang b/src/tests/modules/cache_rbtree/cache-method-update.unlang
index a5cc65398d9f9afd27de390cb02b0f4d961eaa96..6242ee10b07d28413584fd31c35940df52c59fb2 100644 (file)
--- a/src/tests/modules/cache_rbtree/cache-method-update.unlang
+++ b/src/tests/modules/cache_rbtree/cache-method-update.unlang
@@ -17,17 +17,17 @@
 }
 
 #
-#  Basic store and retrieve
+#  Basic update and retrieve
 #
 &control.Callback-Id := 'cache me'
 
-cache_update.store
+cache_update.update
 if (!updated) {
        test_fail
 }
 
 # Merge
-cache_update.store
+cache_update.update
 if (!updated) {
        test_fail
 }

diff --git a/src/tests/modules/cache_rbtree/cache-update.unlang b/src/tests/modules/cache_rbtree/cache-update.unlang
index 962fcec86d370dd9fdde2560fef0953891a5cfd3..d09bac4020e3106e11a20137921361fd9ffe388f 100644 (file)
--- a/src/tests/modules/cache_rbtree/cache-update.unlang
+++ b/src/tests/modules/cache_rbtree/cache-update.unlang
@@ -17,7 +17,7 @@
 }
 
 #
-#  Basic store and retrieve
+#  Basic update and retrieve
 #
 &control.Callback-Id := 'cache me'