From: Joel Rosdahl <joel@rosdahl.net>
Date: Wed, 6 Aug 2025 14:55:35 +0000 (+0200)
Subject: refactor: Improve name and type of only_if_missing parameter
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=5a2ca5cc565035365d409183a36a41e55390992e;p=thirdparty%2Fccache.git

refactor: Improve name and type of only_if_missing parameter
---

diff --git a/src/ccache/ccache.cpp b/src/ccache/ccache.cpp
index 50a7de2b..053c5d6a 100644
--- a/src/ccache/ccache.cpp
+++ b/src/ccache/ccache.cpp
@@ -2279,7 +2279,8 @@ get_result_key_from_manifest(Context& ctx, const Hash::Digest& manifest_key)
     core::CacheEntry::Header header(ctx.config, core::CacheEntryType::manifest);
     ctx.storage.local.put(manifest_key,
                           core::CacheEntryType::manifest,
-                          core::CacheEntry::serialize(header, ctx.manifest));
+                          core::CacheEntry::serialize(header, ctx.manifest),
+                          storage::Overwrite::yes);
   }
 
   return result_key;
diff --git a/src/ccache/storage/local/localstorage.cpp b/src/ccache/storage/local/localstorage.cpp
index ccf2cf4c..d21410b9 100644
--- a/src/ccache/storage/local/localstorage.cpp
+++ b/src/ccache/storage/local/localstorage.cpp
@@ -527,10 +527,10 @@ void
 LocalStorage::put(const Hash::Digest& key,
                   const core::CacheEntryType type,
                   nonstd::span<const uint8_t> value,
-                  bool only_if_missing)
+                  Overwrite overwrite)
 {
   const auto cache_file = look_up_cache_file(key, type);
-  if (only_if_missing && cache_file.dir_entry.exists()) {
+  if (overwrite == Overwrite::no && cache_file.dir_entry.exists()) {
     LOG("Not storing {} in local storage since it already exists",
         cache_file.path);
     return;
diff --git a/src/ccache/storage/local/localstorage.hpp b/src/ccache/storage/local/localstorage.hpp
index 962cfed0..58366f9f 100644
--- a/src/ccache/storage/local/localstorage.hpp
+++ b/src/ccache/storage/local/localstorage.hpp
@@ -25,6 +25,7 @@
 #include <ccache/hash.hpp>
 #include <ccache/storage/local/statsfile.hpp>
 #include <ccache/storage/local/util.hpp>
+#include <ccache/storage/types.hpp>
 #include <ccache/util/bytes.hpp>
 #include <ccache/util/direntry.hpp>
 #include <ccache/util/lockfile.hpp>
@@ -76,7 +77,7 @@ public:
   void put(const Hash::Digest& key,
            core::CacheEntryType type,
            nonstd::span<const uint8_t> value,
-           bool only_if_missing = false);
+           Overwrite overwrite);
 
   void remove(const Hash::Digest& key, core::CacheEntryType type);
 
diff --git a/src/ccache/storage/remote/filestorage.cpp b/src/ccache/storage/remote/filestorage.cpp
index 26f6af4a..eca30a4d 100644
--- a/src/ccache/storage/remote/filestorage.cpp
+++ b/src/ccache/storage/remote/filestorage.cpp
@@ -54,7 +54,7 @@ public:
 
   tl::expected<bool, Failure> put(const Hash::Digest& key,
                                   nonstd::span<const uint8_t> value,
-                                  bool only_if_missing) override;
+                                  Overwrite overwrite) override;
 
   tl::expected<bool, Failure> remove(const Hash::Digest& key) override;
 
@@ -140,11 +140,11 @@ FileStorageBackend::get(const Hash::Digest& key)
 tl::expected<bool, RemoteStorage::Backend::Failure>
 FileStorageBackend::put(const Hash::Digest& key,
                         const nonstd::span<const uint8_t> value,
-                        const bool only_if_missing)
+                        const Overwrite overwrite)
 {
   const auto path = get_entry_path(key);
 
-  if (only_if_missing && DirEntry(path).exists()) {
+  if (overwrite == Overwrite::no && DirEntry(path).exists()) {
     LOG("{} already in cache", path);
     return false;
   }
diff --git a/src/ccache/storage/remote/httpstorage.cpp b/src/ccache/storage/remote/httpstorage.cpp
index 8ed1cb96..dfca4964 100644
--- a/src/ccache/storage/remote/httpstorage.cpp
+++ b/src/ccache/storage/remote/httpstorage.cpp
@@ -49,7 +49,7 @@ public:
 
   tl::expected<bool, Failure> put(const Hash::Digest& key,
                                   nonstd::span<const uint8_t> value,
-                                  bool only_if_missing) override;
+                                  Overwrite overwrite) override;
 
   tl::expected<bool, Failure> remove(const Hash::Digest& key) override;
 
@@ -193,11 +193,11 @@ HttpStorageBackend::get(const Hash::Digest& key)
 tl::expected<bool, RemoteStorage::Backend::Failure>
 HttpStorageBackend::put(const Hash::Digest& key,
                         const nonstd::span<const uint8_t> value,
-                        const bool only_if_missing)
+                        const Overwrite overwrite)
 {
   const auto url_path = get_entry_path(key);
 
-  if (only_if_missing) {
+  if (overwrite == Overwrite::no) {
     const auto result = m_http_client.Head(url_path);
     LOG("HEAD {}{} -> {}", m_url.str(), url_path, result->status);
 
diff --git a/src/ccache/storage/remote/redisstorage.cpp b/src/ccache/storage/remote/redisstorage.cpp
index bc891139..73df93ce 100644
--- a/src/ccache/storage/remote/redisstorage.cpp
+++ b/src/ccache/storage/remote/redisstorage.cpp
@@ -73,7 +73,7 @@ public:
 
   tl::expected<bool, Failure> put(const Hash::Digest& key,
                                   nonstd::span<const uint8_t> value,
-                                  bool only_if_missing) override;
+                                  Overwrite overwrite) override;
 
   tl::expected<bool, Failure> remove(const Hash::Digest& key) override;
 
@@ -187,11 +187,11 @@ RedisStorageBackend::get(const Hash::Digest& key)
 tl::expected<bool, RemoteStorage::Backend::Failure>
 RedisStorageBackend::put(const Hash::Digest& key,
                          nonstd::span<const uint8_t> value,
-                         bool only_if_missing)
+                         Overwrite overwrite)
 {
   const auto key_string = get_key_string(key);
 
-  if (only_if_missing) {
+  if (overwrite == Overwrite::no) {
     LOG("Redis EXISTS {}", key_string);
     TRY_ASSIGN(const auto reply,
                redis_command("EXISTS %s", key_string.c_str()));
diff --git a/src/ccache/storage/remote/remotestorage.hpp b/src/ccache/storage/remote/remotestorage.hpp
index c47cb53c..60f03b8a 100644
--- a/src/ccache/storage/remote/remotestorage.hpp
+++ b/src/ccache/storage/remote/remotestorage.hpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -19,6 +19,7 @@
 #pragma once
 
 #include <ccache/hash.hpp>
+#include <ccache/storage/types.hpp>
 #include <ccache/util/bytes.hpp>
 
 #include <cxxurl/url.hpp>
@@ -75,12 +76,11 @@ public:
     virtual tl::expected<std::optional<util::Bytes>, Failure>
     get(const Hash::Digest& key) = 0;
 
-    // Put `value` associated to `key` in the storage. A true `only_if_missing`
-    // is a hint that the value does not have to be set if already present.
-    // Returns true if the entry was stored, otherwise false.
+    // Put `value` associated to `key` in the storage. Returns true if the entry
+    // was stored, otherwise false.
     virtual tl::expected<bool, Failure> put(const Hash::Digest& key,
                                             nonstd::span<const uint8_t> value,
-                                            bool only_if_missing = false) = 0;
+                                            Overwrite overwrite) = 0;
 
     // Remove `key` and its associated value. Returns true if the entry was
     // removed, otherwise false.
diff --git a/src/ccache/storage/storage.cpp b/src/ccache/storage/storage.cpp
index e3c140ff..cbcb967d 100644
--- a/src/ccache/storage/storage.cpp
+++ b/src/ccache/storage/storage.cpp
@@ -276,7 +276,7 @@ Storage::get(const Hash::Hash::Digest& key,
     auto value = local.get(key, type);
     if (value) {
       if (m_config.reshare()) {
-        put_in_remote_storage(key, *value, true);
+        put_in_remote_storage(key, *value, Overwrite::no);
       }
       if (entry_receiver(std::move(*value))) {
         return;
@@ -286,7 +286,7 @@ Storage::get(const Hash::Hash::Digest& key,
 
   get_from_remote_storage(key, type, [&](util::Bytes&& data) {
     if (!m_config.remote_only()) {
-      local.put(key, type, data, true);
+      local.put(key, type, data, Overwrite::no);
     }
     return entry_receiver(std::move(data));
   });
@@ -298,9 +298,9 @@ Storage::put(const Hash::Digest& key,
              nonstd::span<const uint8_t> value)
 {
   if (!m_config.remote_only()) {
-    local.put(key, type, value);
+    local.put(key, type, value, Overwrite::yes);
   }
-  put_in_remote_storage(key, value, false);
+  put_in_remote_storage(key, value, Overwrite::yes);
 }
 
 void
@@ -502,7 +502,7 @@ Storage::get_from_remote_storage(const Hash::Digest& key,
 void
 Storage::put_in_remote_storage(const Hash::Digest& key,
                                nonstd::span<const uint8_t> value,
-                               bool only_if_missing)
+                               Overwrite overwrite)
 {
   if (!core::CacheEntry::Header(value).self_contained) {
     LOG("Not putting {} in remote storage since it's not self-contained",
@@ -517,7 +517,7 @@ Storage::put_in_remote_storage(const Hash::Digest& key,
     }
 
     Timer timer;
-    const auto result = backend->impl->put(key, value, only_if_missing);
+    const auto result = backend->impl->put(key, value, overwrite);
     const auto ms = timer.measure_ms();
     if (!result) {
       // The backend is expected to log details about the error.
diff --git a/src/ccache/storage/storage.hpp b/src/ccache/storage/storage.hpp
index a0c5a971..4525ee91 100644
--- a/src/ccache/storage/storage.hpp
+++ b/src/ccache/storage/storage.hpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -22,6 +22,7 @@
 #include <ccache/hash.hpp>
 #include <ccache/storage/local/localstorage.hpp>
 #include <ccache/storage/remote/remotestorage.hpp>
+#include <ccache/storage/types.hpp>
 #include <ccache/util/bytes.hpp>
 
 #include <nonstd/span.hpp>
@@ -88,7 +89,7 @@ private:
 
   void put_in_remote_storage(const Hash::Digest& key,
                              nonstd::span<const uint8_t> value,
-                             bool only_if_missing);
+                             Overwrite overwrite);
 
   void remove_from_remote_storage(const Hash::Digest& key);
 };
diff --git a/src/ccache/storage/types.hpp b/src/ccache/storage/types.hpp
index 16e70cd7..392ea8a5 100644
--- a/src/ccache/storage/types.hpp
+++ b/src/ccache/storage/types.hpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -25,4 +25,9 @@ namespace storage {
 
 using EntryWriter = std::function<bool(const std::string& path)>;
 
+enum class Overwrite {
+  yes, // Overwrite any preexisting value
+  no,  // OK to not overwrite any preexisting value (but OK to overwrite anyway)
+};
+
 } // namespace storage