Merge 'utils: loading_cache: add insert() that is a no-op when caching is disabled' from Dario Mirovic

When `permissions_validity_in_ms` is set to 0, executing a prepared statement under authentication crashes with:
```
    Assertion `caching_enabled()' failed.
        at utils/loading_cache.hh:319
        in authorized_prepared_statements_cache::insert
```

`loading_cache::get_ptr()` asserts when caching is disabled (expiry == 0), but `authorized_prepared_statements_cache::insert()` was using it purely for its side effect of populating the cache, which is meaningless when caching is off.

Add a new `loading_cache::insert(k, load)` method that is a no-op when caching is disabled and otherwise forwards to `get_ptr()`. Switch `authorized_prepared_statements_cache::insert()` to use it. This
completes the disabled-mode safety contract of the cache for the write side, mirroring the fallback that `get()` already provides for the read side.

Includes a regression test in `test/boost/loading_cache_test.cc` plus a positive test for the new `insert()` overload.

Fixes SCYLLADB-1699

The crash is introduced a long time ago. It is present on all the live versions, from 2025.1 onward. No client tickets, but it should be backported.

Closes scylladb/scylladb#29638

* github.com:scylladb/scylladb:
  test: boost: regression test for loading_cache::insert with caching disabled
  utils: loading_cache: add insert() that is a no-op when caching is disabled

cql3/authorized_prepared_statements_cache.hh

@@ -136,9 +136,9 @@ public:
     {}
 
     future<> insert(auth::authenticated_user user, cql3::prepared_cache_key_type prep_cache_key, value_type v) noexcept {
-        return _cache.get_ptr(key_type(std::move(user), std::move(prep_cache_key)), [v = std::move(v)] (const cache_key_type&) mutable {
+        return _cache.insert(key_type(std::move(user), std::move(prep_cache_key)), [v = std::move(v)] (const cache_key_type&) mutable {
             return make_ready_future<value_type>(std::move(v));
-        }).discard_result();
+        });
     }
 
     value_ptr find(const auth::authenticated_user& user, const cql3::prepared_cache_key_type& prep_cache_key) {

test/boost/loading_cache_test.cc

@@ -823,4 +823,42 @@ SEASTAR_TEST_CASE(test_prepared_statement_small_cache) {
     }, small_cache_config);
 }
 
+SEASTAR_THREAD_TEST_CASE(test_loading_cache_insert) {
+    using namespace std::chrono;
+    loader loader;
+    loading_cache_for_test<int, sstring> loading_cache(num_loaders, 1h, testlog);
+    auto stop_cache = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
+
+    // insert() must populate the cache and invoke the loader exactly once.
+    loading_cache.insert(0, loader.get()).get();
+    BOOST_REQUIRE_EQUAL(loader.load_count(), 1);
+    BOOST_REQUIRE_EQUAL(loading_cache.size(), 1);
+    auto vp = loading_cache.find(0);
+    BOOST_REQUIRE(vp != nullptr);
+    BOOST_REQUIRE_EQUAL(*vp, test_string);
+
+    // A second insert() for the same key must not re-invoke the loader.
+    loading_cache.insert(0, loader.get()).get();
+    BOOST_REQUIRE_EQUAL(loader.load_count(), 1);
+    BOOST_REQUIRE_EQUAL(loading_cache.size(), 1);
+}
+
+// Regression test for SCYLLADB-1699: insert() on a cache constructed with
+// expiry == 0 (caching disabled) must be a no-op rather than asserting in
+// loading_cache::get_ptr().
+SEASTAR_THREAD_TEST_CASE(test_loading_cache_insert_caching_disabled) {
+    using namespace std::chrono;
+    loader loader;
+    loading_cache_for_test<int, sstring> loading_cache(num_loaders, 0ms, testlog);
+    auto stop_cache = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
+
+    auto f = loading_cache.insert(0, loader.get());
+    loading_cache.insert(0, loader.get()).get();
+
+    // The loader must not have been invoked and the cache must remain empty.
+    BOOST_REQUIRE_EQUAL(loader.load_count(), 0);
+    BOOST_REQUIRE_EQUAL(loading_cache.size(), 0);
+    BOOST_REQUIRE(loading_cache.find(0) == nullptr);
+}
+
 BOOST_AUTO_TEST_SUITE_END()

utils/loading_cache.hh

@@ -65,8 +65,9 @@ struct do_nothing_loading_cache_stats {
 /// The values are going to be evicted from the cache if they are not accessed during the "expiration" period or haven't
 /// been reloaded even once during the same period.
 ///
-/// If "expiration" is set to zero - the caching is going to be disabled and get_XXX(...) is going to call the "loader" callback
-/// every time in order to get the requested value.
+/// If "expiration" is set to zero - the caching is going to be disabled and get(...) is going to call the "loader" callback
+/// every time in order to get the requested value. insert(...) is going to be a no-op in this mode. get_ptr(...) is not
+/// safe to call when caching is disabled (it asserts) since it returns a handle into the cache.
 ///
 /// \note In order to avoid the eviction of cached entries due to "aging" of the contained value the user has to choose
 /// the "expiration" to be at least ("refresh" + "max load latency"). This way the value is going to stay in the cache and is going to be
@@ -353,6 +354,24 @@ public:
         return get_ptr(k, _load);
     }
 
+    /// \brief Insert a value into the cache, loading it via \p load if not already present.
+    ///
+    /// Equivalent to get_ptr(k, load).discard_result() when caching is enabled,
+    /// but is a no-op when caching is disabled (i.e. the cache was constructed
+    /// with expiry == 0). Use this when you only want the side effect of
+    /// populating the cache and don't need a handle to the cached value.
+    ///
+    /// Unlike get_ptr(), it is safe to call this on a cache configured with
+    /// caching disabled.
+    template <typename LoadFunc>
+    requires std::is_invocable_r_v<future<value_type>, LoadFunc, const key_type&>
+    future<> insert(const Key& k, LoadFunc&& load) {
+        if (!caching_enabled()) {
+            return make_ready_future<>();
+        }
+        return get_ptr(k, std::forward<LoadFunc>(load)).discard_result();
+    }
+
     future<Tp> get(const Key& k) {
         static_assert(ReloadEnabled == loading_cache_reload_enabled::yes, "");