Add callback_noop, and various tweaks from review

ecmwf · Jun 27, 2024 · a95432a · a95432a
1 parent 1a6b7d7
commit a95432a
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 42 deletions.
diff --git a/src/fdb5/api/FDB.h b/src/fdb5/api/FDB.h
@@ -155,7 +155,6 @@ class FDB {
 
     FDBStats stats_;
 
-    ArchiveCallback callback_ = nullptr;
 };
 
 //----------------------------------------------------------------------------------------------------------------------

diff --git a/src/fdb5/api/FDBFactory.h b/src/fdb5/api/FDBFactory.h
@@ -129,7 +129,7 @@ class FDBBase : private eckit::NonCopyable {
 
     bool disabled_;
 
-    ArchiveCallback callback_;
+    ArchiveCallback callback_ = CALLBACK_NOOP;
 };
 
 //----------------------------------------------------------------------------------------------------------------------

diff --git a/src/fdb5/api/LocalFDB.cc b/src/fdb5/api/LocalFDB.cc
@@ -51,10 +51,10 @@ void LocalFDB::archive(const Key& key, const void* data, size_t length) {
 
     if (!archiver_) {
         LOG_DEBUG_LIB(LibFdb5) << *this << ": Constructing new archiver" << std::endl;
-        archiver_.reset(new Archiver(config_));
+        archiver_.reset(new Archiver(config_, callback_));
     }
 
-    archiver_->archive(key, data, length, callback_);
+    archiver_->archive(key, data, length);
 }
 
 ListIterator LocalFDB::inspect(const metkit::mars::MarsRequest &request) {

diff --git a/src/fdb5/api/helpers/ArchiveCallback.h b/src/fdb5/api/helpers/ArchiveCallback.h
@@ -21,5 +21,7 @@
 namespace fdb5 {
 
 using ArchiveCallback = std::function<void(const Key&, const FieldLocation&)>;
-
+
+static const ArchiveCallback CALLBACK_NOOP = [](const Key&, const FieldLocation&) {};
+
 } // namespace fdb5
diff --git a/src/fdb5/database/ArchiveVisitor.cc b/src/fdb5/database/ArchiveVisitor.cc
@@ -16,7 +16,7 @@
 
 namespace fdb5 {
 
-ArchiveVisitor::ArchiveVisitor(Archiver &owner, const Key &field, const void *data, size_t size, ArchiveCallback callback) :
+ArchiveVisitor::ArchiveVisitor(Archiver &owner, const Key &field, const void *data, size_t size, const ArchiveCallback& callback) :
     BaseArchiveVisitor(owner, field),
     data_(data),
     size_(size),
@@ -30,7 +30,7 @@ bool ArchiveVisitor::selectDatum(const Key &key, const Key &full) {
 
     ASSERT(current());
 
-    current()->archive(key, data_, size_, field_, callback_); //field_ is the user key! (or rather, "internal key")
+    current()->archive(key, data_, size_, field_, callback_);
 
     return true;
 }

diff --git a/src/fdb5/database/ArchiveVisitor.h b/src/fdb5/database/ArchiveVisitor.h
@@ -30,7 +30,7 @@ class ArchiveVisitor : public BaseArchiveVisitor {
 
 public: // methods
 
-    ArchiveVisitor(Archiver &owner, const Key &field, const void *data, size_t size, ArchiveCallback callback = nullptr);
+    ArchiveVisitor(Archiver &owner, const Key &field, const void *data, size_t size, const ArchiveCallback& callback = CALLBACK_NOOP);
 
 protected: // methods
 
@@ -43,7 +43,7 @@ class ArchiveVisitor : public BaseArchiveVisitor {
     const void *data_;
     size_t size_;
 
-    ArchiveCallback callback_;
+    const ArchiveCallback& callback_;
 
 };
 

diff --git a/src/fdb5/database/Archiver.cc b/src/fdb5/database/Archiver.cc
@@ -24,9 +24,10 @@ namespace fdb5 {
 //----------------------------------------------------------------------------------------------------------------------
 
 
-Archiver::Archiver(const Config& dbConfig) :
+Archiver::Archiver(const Config& dbConfig, const ArchiveCallback& callback) :
     dbConfig_(dbConfig),
-    current_(nullptr) {
+    current_(nullptr),
+    callback_(callback) {
 }
 
 Archiver::~Archiver() {
@@ -36,8 +37,8 @@ Archiver::~Archiver() {
     databases_.clear(); //< explicitly delete the DBs before schemas are destroyed
 }
 
-void Archiver::archive(const Key &key, const void* data, size_t len, ArchiveCallback callback) {
-    ArchiveVisitor visitor(*this, key, data, len, callback);
+void Archiver::archive(const Key &key, const void* data, size_t len) {
+    ArchiveVisitor visitor(*this, key, data, len, callback_);
     archive(key, visitor);
 }
 

diff --git a/src/fdb5/database/Archiver.h b/src/fdb5/database/Archiver.h
@@ -42,12 +42,12 @@ class Archiver : public eckit::NonCopyable {
 
 public: // methods
 
-    Archiver(const Config& dbConfig = Config().expandConfig());
+    Archiver(const Config& dbConfig = Config().expandConfig(), const ArchiveCallback& callback = CALLBACK_NOOP);
 
     virtual ~Archiver();
 
     void archive(const Key &key, BaseArchiveVisitor& visitor);
-    void archive(const Key &key, const void* data, size_t len, ArchiveCallback callback = nullptr);
+    void archive(const Key &key, const void* data, size_t len);
 
     /// Flushes all buffers and closes all data handles into a consistent DB state
     /// @note always safe to call
@@ -77,6 +77,8 @@ class Archiver : public eckit::NonCopyable {
     std::vector<Key> prev_;
 
     DB* current_;
+
+    const ArchiveCallback& callback_;
 };
 
 //----------------------------------------------------------------------------------------------------------------------

diff --git a/src/fdb5/database/BaseArchiveVisitor.h b/src/fdb5/database/BaseArchiveVisitor.h
@@ -48,7 +48,7 @@ class BaseArchiveVisitor : public WriteVisitor {
 
 protected: // members
 
-    const Key &field_;
+    const Key& field_;
 
 private: // members
 

diff --git a/src/fdb5/database/DB.cc b/src/fdb5/database/DB.cc
@@ -104,17 +104,15 @@ eckit::DataHandle *DB::retrieve(const Key& key) {
     return nullptr;
 }
 
-void DB::archive(const Key& key, const void* data, eckit::Length length, const Key& internalKey, ArchiveCallback callback) {
+void DB::archive(const Key& key, const void* data, eckit::Length length, const Key& field, const ArchiveCallback& callback) {
 
     CatalogueWriter* cat = dynamic_cast<CatalogueWriter*>(catalogue_.get());
     ASSERT(cat);
 
     const Index& idx = cat->currentIndex();
     std::unique_ptr<FieldLocation> location(store().archive(idx.key(), data, length));
 
-    if (callback) {
-        callback(internalKey, *location);
-    }
+    callback(field, *location);
 
     cat->archive(key, std::move(location));
 }

diff --git a/src/fdb5/database/DB.h b/src/fdb5/database/DB.h
@@ -57,7 +57,7 @@ class DB final : public eckit::OwnedLock {
     bool axis(const std::string &keyword, eckit::StringSet &s) const;
     bool inspect(const Key& key, Field& field);
     eckit::DataHandle *retrieve(const Key &key);
-    void archive(const Key &key, const void *data, eckit::Length length, const Key &full, ArchiveCallback callback = nullptr);
+    void archive(const Key &key, const void *data, eckit::Length length, const Key &field, const ArchiveCallback& callback = CALLBACK_NOOP);
 
     bool open();
     void flush();

diff --git a/tests/fdb/api/test_archive_callback.cc b/tests/fdb/api/test_archive_callback.cc
@@ -7,20 +7,20 @@ namespace fdb5::test {
 CASE("Archive callback") {
     FDB fdb;
 
-	std::string data_str = "Raining cats and dogs";
+    std::string data_str = "Raining cats and dogs";
     const void* data = static_cast<const void *>(data_str.c_str());
     size_t length = data_str.size();
 
-	Key key;
-	key.set("class","od");
-	key.set("expver","xxxx");
-	key.set("type","fc");
-	key.set("stream","oper");
-	key.set("date","20101010");
-	key.set("time","0000");
-	key.set("domain","g");
-	key.set("levtype","sfc");
-	key.set("param","130");
+    Key key;
+    key.set("class","od");
+    key.set("expver","xxxx");
+    key.set("type","fc");
+    key.set("stream","oper");
+    key.set("date","20101010");
+    key.set("time","0000");
+    key.set("domain","g");
+    key.set("levtype","sfc");
+    key.set("param","130");
 
     std::map<fdb5::Key, eckit::URI> map;
     std::vector<Key> internalKeys;
@@ -29,26 +29,23 @@ CASE("Archive callback") {
         map[internalKey] = location.fullUri();
     });
 
-	key.set("step","1");
+    key.set("step","1");
     internalKeys.push_back(fdb.archive(key, data, length));
 
-	key.set("step","2");
+    key.set("date","20111213");
     internalKeys.push_back(fdb.archive(key, data, length));
 
-	key.set("step","3");
+    key.set("type","pf");
     internalKeys.push_back(fdb.archive(key, data, length));
 
-	fdb.flush();
+    fdb.flush();
 
     EXPECT(map.size() == 3);
 
-    // Print out the map
-    for (const auto& [key, uri] : map) {
-        std::cout << key << " -> " << uri << std::endl;
-    }
+    // for (const auto& [key, uri] : map) {
+    //     std::cout << key << " -> " << uri << std::endl;
+    // }
 
-    // Note that the keys are not the same, but they are equivalent
-    // so iterate over the map keys and check that there is a corresponding key in the internalKeys vector
     for (const auto& [key, uri] : map) {
         bool found = false;
         for (const auto& internalKey : internalKeys) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -155,7 +155,6 @@ class FDB { @@
         FDBStats stats_;
-        ArchiveCallback callback_ = nullptr;
     };
     //----------------------------------------------------------------------------------------------------------------------
@@ Expand Down @@