From 9ed3e33f97748bfc773b56393de7a81fb2be3b68 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 24 Feb 2025 14:37:11 -0700 Subject: [PATCH] Completely rework return values from storage operations --- scripts/base/frameworks/storage/async.zeek | 50 ++-- scripts/base/frameworks/storage/main.zeek | 6 +- scripts/base/frameworks/storage/sync.zeek | 50 ++-- scripts/base/init-bare.zeek | 53 ++++- .../storage/backend/redis/main.zeek | 8 +- .../storage/backend/sqlite/main.zeek | 8 +- src/IntrusivePtr.h | 22 +- src/storage/Backend.cc | 185 ++++++++------- src/storage/Backend.h | 114 ++++----- src/storage/CMakeLists.txt | 1 + src/storage/Manager.cc | 33 ++- src/storage/Manager.h | 22 +- src/storage/ReturnCode.cc | 77 ++++++ src/storage/ReturnCode.h | 37 +++ src/storage/backend/redis/Redis.cc | 113 +++++---- src/storage/backend/redis/Redis.h | 22 +- src/storage/backend/sqlite/SQLite.cc | 88 +++---- src/storage/backend/sqlite/SQLite.h | 14 +- src/storage/storage.bif | 223 ++++++++++-------- testing/btest/Baseline/plugins.storage/output | 5 +- .../Baseline/plugins.storage/zeek-stderr | 3 - .../out | 7 +- .../scripts.base.frameworks.storage.erase/out | 5 +- .../out | 7 +- .../out | 5 +- .../out | 5 +- .../out | 6 +- .../worker-1..stdout | 4 +- .../worker-2..stdout | 2 +- .../out | 7 +- .../out | 9 +- .../out | 5 +- .../out | 6 +- .../.stderr | 2 - .../out | 5 +- .../storage-plugin/src/StorageDummy.cc | 48 ++-- .../plugins/storage-plugin/src/StorageDummy.h | 18 +- testing/btest/plugins/storage.zeek | 20 +- .../frameworks/storage/compound-types.zeek | 4 +- .../base/frameworks/storage/erase.zeek | 17 +- .../base/frameworks/storage/expiration.zeek | 12 +- .../base/frameworks/storage/overwriting.zeek | 8 +- .../storage/redis-async-reading-pcap.zeek | 8 +- .../base/frameworks/storage/redis-async.zeek | 43 +++- .../frameworks/storage/redis-cluster.zeek | 3 +- .../frameworks/storage/redis-expiration.zeek | 9 +- .../base/frameworks/storage/redis-sync.zeek | 13 +- .../storage/sqlite-basic-reading-pcap.zeek | 9 +- .../base/frameworks/storage/sqlite-basic.zeek | 9 +- .../storage/sqlite-error-handling.zeek | 15 +- 50 files changed, 859 insertions(+), 586 deletions(-) create mode 100644 src/storage/ReturnCode.cc create mode 100644 src/storage/ReturnCode.h diff --git a/scripts/base/frameworks/storage/async.zeek b/scripts/base/frameworks/storage/async.zeek index 3b2ba08dcd..8fbadb6e1b 100644 --- a/scripts/base/frameworks/storage/async.zeek +++ b/scripts/base/frameworks/storage/async.zeek @@ -22,17 +22,19 @@ export { ## well as for type conversions for return values from ## :zeek:see:`Storage::Async::get`. ## - ## Returns: A handle to the new backend connection, or ``F`` if the connection - ## failed. + ## Returns: A record containing the status of the operation, and either an error + ## string on failure or a value on success. The value returned here will + ## be an ``opaque of BackendHandle``. global open_backend: function(btype: Storage::Backend, options: Storage::BackendOptions, key_type: any, - val_type: any): opaque of Storage::BackendHandle; + val_type: any): Storage::OperationResult; ## Closes an existing backend connection asynchronously. ## ## backend: A handle to a backend connection. ## - ## Returns: A boolean indicating success or failure of the operation. - global close_backend: function(backend: opaque of Storage::BackendHandle): bool; + ## Returns: A record containing the status of the operation and an optional error + ## string for failures. + global close_backend: function(backend: opaque of Storage::BackendHandle): Storage::OperationResult; ## Inserts a new entry into a backend asynchronously. ## @@ -41,11 +43,9 @@ export { ## args: A :zeek:see:`Storage::PutArgs` record containing the arguments for the ## operation. ## - ## Returns: A boolean indicating success or failure of the operation. Type - ## comparison failures against the types passed to - ## :zeek:see:`Storage::open_backend` for the backend will cause ``F`` to - ## be returned. - global put: function(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): bool; + ## Returns: A record containing the status of the operation and an optional error + ## string for failures. + global put: function(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): Storage::OperationResult; ## Gets an entry from the backend asynchronously. ## @@ -53,13 +53,11 @@ export { ## ## key: The key to look up. ## - ## Returns: A boolean indicating success or failure of the operation. Type - ## comparison failures against the types passed to - ## :zeek:see:`Storage::open_backend` for the backend will cause ``F`` to - ## be returned. The caller should check the validity of the value before - ## attempting to use it. If the value is unset, an error string may be - ## available to describe the failure. - global get: function(backend: opaque of Storage::BackendHandle, key: any): val_result; + ## Returns: A record containing the status of the operation, an optional error + ## string for failures, and an optional value for success. The value + ## returned here will be of the type passed into + ## :zeek:see:`Storage::open_backend`. + global get: function(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult; ## Erases an entry from the backend asynchronously. ## @@ -67,35 +65,33 @@ export { ## ## key: The key to erase. ## - ## Returns: A boolean indicating success or failure of the operation. Type - ## comparison failures against the types passed to - ## :zeek:see:`Storage::open_backend` for the backend will cause ``F`` to - ## be returned. - global erase: function(backend: opaque of Storage::BackendHandle, key: any): bool; + ## Returns: A record containing the status of the operation and an optional error + ## string for failures. + global erase: function(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult; } function open_backend(btype: Storage::Backend, options: Storage::BackendOptions, key_type: any, - val_type: any): opaque of Storage::BackendHandle + val_type: any): Storage::OperationResult { return Storage::Async::__open_backend(btype, options, key_type, val_type); } -function close_backend(backend: opaque of Storage::BackendHandle): bool +function close_backend(backend: opaque of Storage::BackendHandle): Storage::OperationResult { return Storage::Async::__close_backend(backend); } -function put(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): bool +function put(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): Storage::OperationResult { return Storage::Async::__put(backend, args$key, args$value, args$overwrite, args$expire_time); } -function get(backend: opaque of Storage::BackendHandle, key: any): val_result +function get(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult { return Storage::Async::__get(backend, key); } -function erase(backend: opaque of Storage::BackendHandle, key: any): bool +function erase(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult { return Storage::Async::__erase(backend, key); } diff --git a/scripts/base/frameworks/storage/main.zeek b/scripts/base/frameworks/storage/main.zeek index 66ab1c2744..ee928cbc28 100644 --- a/scripts/base/frameworks/storage/main.zeek +++ b/scripts/base/frameworks/storage/main.zeek @@ -5,9 +5,9 @@ module Storage; export { - ## Base record for backend options. Backend plugins can redef this record to add - ## relevant fields to it. - type BackendOptions: record {}; + ## Base record for backend options. Backend plugins can redef this record to add + ## relevant fields to it. + type BackendOptions: record { }; ## Record for passing arguments to :zeek:see:`Storage::Async::put` and ## :zeek:see:`Storage::Sync::put`. diff --git a/scripts/base/frameworks/storage/sync.zeek b/scripts/base/frameworks/storage/sync.zeek index 90aaa6a302..7eb5fa5a1d 100644 --- a/scripts/base/frameworks/storage/sync.zeek +++ b/scripts/base/frameworks/storage/sync.zeek @@ -20,17 +20,19 @@ export { ## as for type conversions for return values from ## :zeek:see:`Storage::Sync::get`. ## - ## Returns: A handle to the new backend connection, or ``F`` if the connection - ## failed. + ## Returns: A record containing the status of the operation, and either an error + ## string on failure or a value on success. The value returned here will + ## be an ``opaque of BackendHandle``. global open_backend: function(btype: Storage::Backend, options: Storage::BackendOptions, key_type: any, - val_type: any): opaque of Storage::BackendHandle; + val_type: any): Storage::OperationResult; ## Closes an existing backend connection. ## ## backend: A handle to a backend connection. ## - ## Returns: A boolean indicating success or failure of the operation. - global close_backend: function(backend: opaque of Storage::BackendHandle): bool; + ## Returns: A record containing the status of the operation and an optional error + ## string for failures. + global close_backend: function(backend: opaque of Storage::BackendHandle): Storage::OperationResult; ## Inserts a new entry into a backend. ## @@ -39,11 +41,9 @@ export { ## args: A :zeek:see:`Storage::PutArgs` record containing the arguments for the ## operation. ## - ## Returns: A boolean indicating success or failure of the operation. Type - ## comparison failures against the types passed to - ## :zeek:see:`Storage::open_backend` for the backend will cause ``F`` to - ## be returned. - global put: function(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): bool; + ## Returns: A record containing the status of the operation and an optional error + ## string for failures. + global put: function(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): Storage::OperationResult; ## Gets an entry from the backend. ## @@ -51,13 +51,11 @@ export { ## ## key: The key to look up. ## - ## Returns: A boolean indicating success or failure of the operation. Type - ## comparison failures against the types passed to - ## :zeek:see:`Storage::open_backend` for the backend will cause ``F`` to - ## be returned. The caller should check the validity of the value before - ## attempting to use it. If the value is unset, an error string may be - ## available to describe the failure. - global get: function(backend: opaque of Storage::BackendHandle, key: any): val_result; + ## Returns: A record containing the status of the operation, an optional error + ## string for failures, and an optional value for success. The value + ## returned here will be of the type passed into + ## :zeek:see:`Storage::open_backend`. + global get: function(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult; ## Erases an entry from the backend. ## @@ -65,35 +63,33 @@ export { ## ## key: The key to erase. ## - ## Returns: A boolean indicating success or failure of the operation. Type - ## comparison failures against the types passed to - ## :zeek:see:`Storage::open_backend` for the backend will cause ``F`` to - ## be returned. - global erase: function(backend: opaque of Storage::BackendHandle, key: any): bool; + ## Returns: A record containing the status of the operation and an optional error + ## string for failures. + global erase: function(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult; } function open_backend(btype: Storage::Backend, options: Storage::BackendOptions, key_type: any, - val_type: any): opaque of Storage::BackendHandle + val_type: any): Storage::OperationResult { return Storage::Sync::__open_backend(btype, options, key_type, val_type); } -function close_backend(backend: opaque of Storage::BackendHandle): bool +function close_backend(backend: opaque of Storage::BackendHandle): Storage::OperationResult { return Storage::Sync::__close_backend(backend); } -function put(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): bool +function put(backend: opaque of Storage::BackendHandle, args: Storage::PutArgs): Storage::OperationResult { return Storage::Sync::__put(backend, args$key, args$value, args$overwrite, args$expire_time); } -function get(backend: opaque of Storage::BackendHandle, key: any): val_result +function get(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult { return Storage::Sync::__get(backend, key); } -function erase(backend: opaque of Storage::BackendHandle, key: any): bool +function erase(backend: opaque of Storage::BackendHandle, key: any): Storage::OperationResult { return Storage::Sync::__erase(backend, key); } diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index 625415586f..a71963f1fd 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -356,14 +356,6 @@ type ftp_port: record { valid: bool; ##< True if format was right. Only then are *h* and *p* valid. }; -## A generic type for returning either a value or an error string from a -## function or a BIF method. This is sort of equivalent to std::expected -## in C++. -type val_result: record { - val: any &optional; - error: string &optional; -}; - ## Statistics about what a TCP endpoint sent. ## ## .. zeek:see:: conn_stats @@ -6224,7 +6216,50 @@ export { ## The interval used by the storage framework for automatic expiration ## of elements in all backends that don't support it natively, or if ## using expiration while reading pcap files. - const expire_interval = 15.0 secs &redef; + const expire_interval = 15.0secs &redef; + + ## Common set of statuses that can be returned by storage operations. Backend plugins + ## can add to this enum if custom values are needed. + type ReturnCode: enum { + ## Operation succeeded. + SUCCESS, + ## Type of value passed to operation does not match type of + ## value passed when opening backend. + VAL_TYPE_MISMATCH, + ## Type of key passed to operation does not match type of + ## key passed when opening backend. + KEY_TYPE_MISMATCH, + ## Backend is not connected. + NOT_CONNECTED, + ## Operation timed out. + TIMEOUT, + ## Connection to backed was lost. + CONNECTION_LOST, + ## Generic operation failed. + OPERATION_FAILED, + ## Key requested was not found in backend. + KEY_NOT_FOUND, + ## Key requested for overwrite already exists. + KEY_EXISTS, + ## Generic connection failure. + CONNECTION_FAILED, + ## Generic disconnection failure. + DISCONNECTION_FAILED, + ## Generic initialization failure. + INITIALIZATION_FAILED + } &redef; + + ## Returned as the result of the various storage operations. + type OperationResult: record { + ## One of a set of backend-redefinable return codes. + code: ReturnCode; + ## An optional error string. This should be set when the + ## ``code`` field is not set ``SUCCESS``. + error_str: string &optional; + ## An optional value returned by ``get`` operations when a match + ## was found the key requested. + value: any &optional; + }; } module GLOBAL; diff --git a/scripts/policy/frameworks/storage/backend/redis/main.zeek b/scripts/policy/frameworks/storage/backend/redis/main.zeek index f37fe0b3c8..a921fefc3a 100644 --- a/scripts/policy/frameworks/storage/backend/redis/main.zeek +++ b/scripts/policy/frameworks/storage/backend/redis/main.zeek @@ -23,8 +23,8 @@ export { # backend opened. key_prefix: string &default=""; }; - - redef record Storage::BackendOptions += { - redis: Options &optional; - }; } + +redef record Storage::BackendOptions += { + redis: Storage::Backend::Redis::Options &optional; +}; diff --git a/scripts/policy/frameworks/storage/backend/sqlite/main.zeek b/scripts/policy/frameworks/storage/backend/sqlite/main.zeek index 9b4fd386f8..24e15fd857 100644 --- a/scripts/policy/frameworks/storage/backend/sqlite/main.zeek +++ b/scripts/policy/frameworks/storage/backend/sqlite/main.zeek @@ -28,8 +28,8 @@ export { ["temp_store"] = "memory" ); }; - - redef record Storage::BackendOptions += { - sqlite: Options &optional; - }; } + +redef record Storage::BackendOptions += { + sqlite: Storage::Backend::SQLite::Options &optional; +}; diff --git a/src/IntrusivePtr.h b/src/IntrusivePtr.h index b384b66e9a..6d44edb9df 100644 --- a/src/IntrusivePtr.h +++ b/src/IntrusivePtr.h @@ -144,10 +144,7 @@ public: } IntrusivePtr& operator=(std::nullptr_t) noexcept { - if ( ptr_ ) { - Unref(ptr_); - ptr_ = nullptr; - } + reset(); return *this; } @@ -161,6 +158,23 @@ public: explicit operator bool() const noexcept { return ptr_ != nullptr; } + void reset() noexcept { + if ( ptr_ ) { + Unref(ptr_); + ptr_ = nullptr; + } + } + + void reset(T* ptr) { + if ( ptr_ ) + Unref(ptr_); + + if ( ptr ) + Ref(ptr); + + ptr_ = ptr; + } + private: pointer ptr_ = nullptr; }; diff --git a/src/storage/Backend.cc b/src/storage/Backend.cc index 12bd1eb60b..b1ceb92a54 100644 --- a/src/storage/Backend.cc +++ b/src/storage/Backend.cc @@ -2,152 +2,157 @@ #include "zeek/storage/Backend.h" -#include "zeek/Desc.h" #include "zeek/Trigger.h" #include "zeek/broker/Data.h" +#include "zeek/storage/ReturnCode.h" namespace zeek::storage { +RecordValPtr OperationResult::BuildVal() { + static auto op_result_type = zeek::id::find_type("Storage::OperationResult"); + + auto rec = zeek::make_intrusive(op_result_type); + rec->Assign(0, code); + if ( ! err_str.empty() ) + rec->Assign(1, err_str); + if ( value ) + rec->Assign(2, value); + + return rec; +} + ResultCallback::ResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc) : trigger(std::move(trigger)), assoc(assoc) {} -ResultCallback::~ResultCallback() {} - void ResultCallback::Timeout() { + static const auto& op_result_type = zeek::id::find_type("Storage::OperationResult"); + if ( ! IsSyncCallback() ) { - auto v = make_intrusive("Timeout during request"); - trigger->Cache(assoc, v.get()); + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::TIMEOUT); + + trigger->Cache(assoc, op_result.release()); } } -void ResultCallback::ValComplete(Val* result) { - if ( ! IsSyncCallback() ) { - trigger->Cache(assoc, result); - trigger->Release(); +OperationResultCallback::OperationResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc) + : ResultCallback(std::move(trigger), assoc) {} + +void OperationResultCallback::Complete(OperationResult res) { + // If this is a sync callback, there isn't a trigger to process. Store the result and bail. + if ( IsSyncCallback() ) { + result = std::move(res); + return; } - Unref(result); -} + static auto op_result_type = zeek::id::find_type("Storage::OperationResult"); + auto* op_result = new zeek::RecordVal(op_result_type); -ErrorResultCallback::ErrorResultCallback(IntrusivePtr trigger, const void* assoc) - : ResultCallback(std::move(trigger), assoc) {} - -void ErrorResultCallback::Complete(const ErrorResult& res) { - if ( IsSyncCallback() ) - result = res; - - zeek::Val* val_result; - - if ( res ) - val_result = new StringVal(res.value()); + op_result->Assign(0, res.code); + if ( res.code->Get() != 0 ) + op_result->Assign(1, res.err_str); else - val_result = val_mgr->Bool(true).get(); + op_result->Assign(2, res.value); - ValComplete(val_result); + trigger->Cache(assoc, op_result); + trigger->Release(); + + Unref(op_result); } -ValResultCallback::ValResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc) - : ResultCallback(std::move(trigger), assoc) {} +OpenResultCallback::OpenResultCallback(IntrusivePtr backend) + : ResultCallback(), backend(std::move(backend)) {} -void ValResultCallback::Complete(const ValResult& res) { - if ( IsSyncCallback() ) - result = res; +OpenResultCallback::OpenResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc, + IntrusivePtr backend) + : ResultCallback(std::move(trigger), assoc), backend(std::move(backend)) {} - static auto val_result_type = zeek::id::find_type("val_result"); - auto* val_result = new zeek::RecordVal(val_result_type); +void OpenResultCallback::Complete(OperationResult res) { + // If this is a sync callback, there isn't a trigger to process. Store the result and bail. Always + // set result's value to the backend pointer so that it comes across in the result. This ensures + // the handle is always available in the result even on failures. + if ( IsSyncCallback() ) { + result = std::move(res); + result.value = backend; + return; + } - if ( res ) - val_result->Assign(0, res.value()); - else - val_result->Assign(1, zeek::make_intrusive(res.error())); + static auto op_result_type = zeek::id::find_type("Storage::OperationResult"); + auto* op_result = new zeek::RecordVal(op_result_type); - ValComplete(val_result); + op_result->Assign(0, res.code); + if ( res.code != ReturnCode::SUCCESS ) + op_result->Assign(1, res.err_str); + op_result->Assign(2, backend); + + trigger->Cache(assoc, op_result); + trigger->Release(); + + Unref(op_result); } -OpenResultCallback::OpenResultCallback(detail::BackendHandleVal* backend) : ResultCallback(), backend(backend) {} - -OpenResultCallback::OpenResultCallback(IntrusivePtr trigger, const void* assoc, - detail::BackendHandleVal* backend) - : ResultCallback(std::move(trigger), assoc), backend(backend) {} - -void OpenResultCallback::Complete(const ErrorResult& res) { - if ( IsSyncCallback() ) - result = res; - - zeek::Val* val_result; - - if ( res ) - val_result = new StringVal(res.value()); - else - val_result = backend; - - ValComplete(val_result); -} - -ErrorResult Backend::Open(RecordValPtr options, TypePtr kt, TypePtr vt, OpenResultCallback* cb) { +OperationResult Backend::Open(RecordValPtr options, TypePtr kt, TypePtr vt, OpenResultCallback* cb) { key_type = std::move(kt); val_type = std::move(vt); - return DoOpen(std::move(options)); + auto ret = DoOpen(std::move(options), cb); + if ( ! ret.value ) + ret.value = cb->Backend(); + + return ret; } -ErrorResult Backend::Close(ErrorResultCallback* cb) { return DoClose(cb); } +OperationResult Backend::Close(OperationResultCallback* cb) { return DoClose(cb); } -ErrorResult Backend::Put(ValPtr key, ValPtr value, bool overwrite, double expiration_time, ErrorResultCallback* cb) { +OperationResult Backend::Put(ValPtr key, ValPtr value, bool overwrite, double expiration_time, + OperationResultCallback* cb) { // The intention for this method is to do some other heavy lifting in regard // to backends that need to pass data through the manager instead of directly // through the workers. For the first versions of the storage framework it // just calls the backend itself directly. - if ( ! same_type(key->GetType(), key_type) ) - return util::fmt("type of key passed (%s) does not match backend's key type (%s)", - obj_desc_short(key->GetType().get()).c_str(), key_type->GetName().c_str()); - if ( ! same_type(value->GetType(), val_type) ) - return util::fmt("type of value passed (%s) does not match backend's value type (%s)", - obj_desc_short(value->GetType().get()).c_str(), val_type->GetName().c_str()); + if ( ! same_type(key->GetType(), key_type) ) { + auto ret = OperationResult{ReturnCode::KEY_TYPE_MISMATCH}; + CompleteCallback(cb, ret); + return ret; + } + if ( ! same_type(value->GetType(), val_type) ) { + auto ret = OperationResult{ReturnCode::VAL_TYPE_MISMATCH}; + CompleteCallback(cb, ret); + return ret; + } return DoPut(std::move(key), std::move(value), overwrite, expiration_time, cb); } -ValResult Backend::Get(ValPtr key, ValResultCallback* cb) { +OperationResult Backend::Get(ValPtr key, OperationResultCallback* cb) { // See the note in Put(). - if ( ! same_type(key->GetType(), key_type) ) - return zeek::unexpected(util::fmt("type of key passed (%s) does not match backend's key type (%s)", - key->GetType()->GetName().c_str(), key_type->GetName().c_str())); + if ( ! same_type(key->GetType(), key_type) ) { + auto ret = OperationResult{ReturnCode::KEY_TYPE_MISMATCH}; + CompleteCallback(cb, ret); + return ret; + } return DoGet(std::move(key), cb); } -ErrorResult Backend::Erase(ValPtr key, ErrorResultCallback* cb) { +OperationResult Backend::Erase(ValPtr key, OperationResultCallback* cb) { // See the note in Put(). - if ( ! same_type(key->GetType(), key_type) ) - return util::fmt("type of key passed (%s) does not match backend's key type (%s)", - key->GetType()->GetName().c_str(), key_type->GetName().c_str()); + if ( ! same_type(key->GetType(), key_type) ) { + auto ret = OperationResult{ReturnCode::KEY_TYPE_MISMATCH}; + CompleteCallback(cb, ret); + return ret; + } return DoErase(std::move(key), cb); } -void Backend::CompleteCallback(ValResultCallback* cb, const ValResult& data) const { +void Backend::CompleteCallback(ResultCallback* cb, const OperationResult& data) const { cb->Complete(data); if ( ! cb->IsSyncCallback() ) { delete cb; } } -void Backend::CompleteCallback(ErrorResultCallback* cb, const ErrorResult& data) const { - cb->Complete(data); - if ( ! cb->IsSyncCallback() ) { - delete cb; - } -} - -void Backend::CompleteCallback(OpenResultCallback* cb, const ErrorResult& data) const { - cb->Complete(data); - if ( ! cb->IsSyncCallback() ) { - delete cb; - } -} - - zeek::OpaqueTypePtr detail::backend_opaque; IMPLEMENT_OPAQUE_VALUE(detail::BackendHandleVal) diff --git a/src/storage/Backend.h b/src/storage/Backend.h index ab89fb619f..139a1ab96c 100644 --- a/src/storage/Backend.h +++ b/src/storage/Backend.h @@ -4,7 +4,6 @@ #include "zeek/OpaqueVal.h" #include "zeek/Val.h" -#include "zeek/util.h" namespace zeek::detail::trigger { class Trigger; @@ -15,14 +14,13 @@ namespace zeek::storage { class Manager; -// Result from storage operations that may return an error message. If the -// optional value is unset, the operation succeeded. -using ErrorResult = std::optional; +struct OperationResult { + EnumValPtr code; + std::string err_str; + ValPtr value; -// Result from storage operations that return Vals. The ValPtr is an -// IntrusivePtr to some result, and can be null if the operation failed. The -// string value will store an error message if the result is null. -using ValResult = zeek::expected; + RecordValPtr BuildVal(); +}; // Base callback object for async operations. This is just here to allow some @@ -30,41 +28,29 @@ using ValResult = zeek::expected; class ResultCallback { public: ResultCallback() = default; - ResultCallback(IntrusivePtr trigger, const void* assoc); - virtual ~ResultCallback(); + ResultCallback(detail::trigger::TriggerPtr trigger, const void* assoc); + virtual ~ResultCallback() = default; void Timeout(); bool IsSyncCallback() const { return ! trigger; } -protected: - void ValComplete(Val* result); + virtual void Complete(OperationResult res) = 0; + +protected: + void CompleteWithVal(Val* result); -private: IntrusivePtr trigger; const void* assoc = nullptr; }; -// A callback result that returns an ErrorResult. -class ErrorResultCallback : public ResultCallback { +class OperationResultCallback : public ResultCallback { public: - ErrorResultCallback() = default; - ErrorResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc); - virtual void Complete(const ErrorResult& res); - ErrorResult Result() { return result; } + OperationResultCallback() = default; + OperationResultCallback(detail::trigger::TriggerPtr trigger, const void* assoc); + void Complete(OperationResult res) override; + OperationResult Result() { return result; } private: - ErrorResult result; -}; - -// A callback result that returns a ValResult. -class ValResultCallback : public ResultCallback { -public: - ValResultCallback() = default; - ValResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc); - void Complete(const ValResult& res); - ValResult Result() { return result; } - -private: - ValResult result; + OperationResult result; }; class OpenResultCallback; @@ -88,31 +74,31 @@ public: * removed. Set to zero to disable expiration. This time is based on the current network * time. * @param cb An optional callback object if being called via an async context. - * @return An optional value potentially containing an error string if - * needed Will be unset if the operation succeeded. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ErrorResult Put(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, - ErrorResultCallback* cb = nullptr); + OperationResult Put(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, + OperationResultCallback* cb = nullptr); /** * Retrieve a value from the backend for a provided key. * * @param key the key to lookup in the backend. * @param cb An optional callback object if being called via an async context. - * @return A std::expected containing either a valid ValPtr with the result - * of the operation or a string containing an error message for failure. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ValResult Get(ValPtr key, ValResultCallback* cb = nullptr); + OperationResult Get(ValPtr key, OperationResultCallback* cb = nullptr); /** * Erases the value for a key from the backend. * * @param key the key to erase * @param cb An optional callback object if being called via an async context. - * @return An optional value potentially containing an error string if - * needed. Will be unset if the operation succeeded. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ErrorResult Erase(ValPtr key, ErrorResultCallback* cb = nullptr); + OperationResult Erase(ValPtr key, OperationResultCallback* cb = nullptr); /** * Returns whether the backend is opened. @@ -151,45 +137,45 @@ protected: * @param vt The script-side type of the values stored in the backend. Used for * validation of types and conversion during retrieval. * @param cb An optional callback object if being called via an async context. - * @return An optional value potentially containing an error string if - * needed. Will be unset if the operation succeeded. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ErrorResult Open(RecordValPtr options, TypePtr kt, TypePtr vt, OpenResultCallback* cb = nullptr); + OperationResult Open(RecordValPtr options, TypePtr kt, TypePtr vt, OpenResultCallback* cb = nullptr); /** * Finalizes the backend when it's being closed. * * @param cb An optional callback object if being called via an async context. - * @return An optional value potentially containing an error string if - * needed. Will be unset if the operation succeeded. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ErrorResult Close(ErrorResultCallback* cb = nullptr); + OperationResult Close(OperationResultCallback* cb = nullptr); /** * The workhorse method for Open(). */ - virtual ErrorResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) = 0; + virtual OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) = 0; /** * The workhorse method for Close(). */ - virtual ErrorResult DoClose(ErrorResultCallback* cb = nullptr) = 0; + virtual OperationResult DoClose(OperationResultCallback* cb = nullptr) = 0; /** * The workhorse method for Put(). */ - virtual ErrorResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, - ErrorResultCallback* cb = nullptr) = 0; + virtual OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, + OperationResultCallback* cb = nullptr) = 0; /** * The workhorse method for Get(). */ - virtual ValResult DoGet(ValPtr key, ValResultCallback* cb = nullptr) = 0; + virtual OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) = 0; /** * The workhorse method for Erase(). */ - virtual ErrorResult DoErase(ValPtr key, ErrorResultCallback* cb = nullptr) = 0; + virtual OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) = 0; /** * Removes any entries in the backend that have expired. Can be overridden by @@ -197,9 +183,7 @@ protected: */ virtual void Expire() {} - void CompleteCallback(ValResultCallback* cb, const ValResult& data) const; - void CompleteCallback(ErrorResultCallback* cb, const ErrorResult& data) const; - void CompleteCallback(OpenResultCallback* cb, const ErrorResult& data) const; + void CompleteCallback(ResultCallback* cb, const OperationResult& data) const; TypePtr key_type; TypePtr val_type; @@ -235,15 +219,17 @@ protected: // A callback for the Backend::Open() method that returns an error or a backend handle. class OpenResultCallback : public ResultCallback { public: - OpenResultCallback(detail::BackendHandleVal* backend); - OpenResultCallback(IntrusivePtr trigger, const void* assoc, - detail::BackendHandleVal* backend); - void Complete(const ErrorResult& res); - ErrorResult Result() { return result; } + OpenResultCallback(IntrusivePtr backend); + OpenResultCallback(zeek::detail::trigger::TriggerPtr trigger, const void* assoc, + IntrusivePtr backend); + void Complete(OperationResult res) override; + + OperationResult Result() const { return result; } + IntrusivePtr Backend() const { return backend; } private: - ErrorResult result; - detail::BackendHandleVal* backend = nullptr; + OperationResult result{}; + IntrusivePtr backend; }; } // namespace zeek::storage diff --git a/src/storage/CMakeLists.txt b/src/storage/CMakeLists.txt index 86071e81bd..d1401a3ee1 100644 --- a/src/storage/CMakeLists.txt +++ b/src/storage/CMakeLists.txt @@ -4,6 +4,7 @@ zeek_add_subdir_library( Manager.cc Backend.cc Component.cc + ReturnCode.cc BIFS storage.bif) diff --git a/src/storage/Manager.cc b/src/storage/Manager.cc index 18f3c9cb35..6f991af5c2 100644 --- a/src/storage/Manager.cc +++ b/src/storage/Manager.cc @@ -6,6 +6,7 @@ #include "zeek/Desc.h" #include "zeek/RunState.h" +#include "zeek/storage/ReturnCode.h" #include "const.bif.netvar_h" @@ -32,7 +33,17 @@ void detail::ExpirationTimer::Dispatch(double t, bool is_expire) { Manager::Manager() : plugin::ComponentManager("Storage", "Backend") {} +Manager::~Manager() { + // TODO: should we shut down any existing backends? force-poll until all of their existing + // operations finish and close them? + + // Don't leave all of these static objects to leak. + ReturnCode::Cleanup(); +} + void Manager::InitPostScript() { + ReturnCode::Initialize(); + detail::backend_opaque = make_intrusive("Storage::Backend"); StartExpirationTimer(); } @@ -62,20 +73,22 @@ zeek::expected Manager::Instantiate(const Tag& type) { return bp; } -ErrorResult Manager::OpenBackend(BackendPtr backend, RecordValPtr options, TypePtr key_type, TypePtr val_type, - OpenResultCallback* cb) { - if ( auto res = backend->Open(std::move(options), std::move(key_type), std::move(val_type), cb); res.has_value() ) { - return util::fmt("Failed to open backend %s: %s", backend->Tag(), res.value().c_str()); +OperationResult Manager::OpenBackend(BackendPtr backend, RecordValPtr options, TypePtr key_type, TypePtr val_type, + OpenResultCallback* cb) { + auto res = backend->Open(std::move(options), std::move(key_type), std::move(val_type), cb); + if ( res.code != ReturnCode::SUCCESS ) { + res.err_str = util::fmt("Failed to open backend %s: %s", backend->Tag(), res.err_str.c_str()); + return res; } RegisterBackend(std::move(backend)); // TODO: post Storage::backend_opened event - return std::nullopt; + return res; } -ErrorResult Manager::CloseBackend(BackendPtr backend, ErrorResultCallback* cb) { +OperationResult Manager::CloseBackend(BackendPtr backend, OperationResultCallback* cb) { // Expiration runs on a separate thread and loops over the vector of backends. The mutex // here ensures exclusive access. This one happens in a block because we can remove the // backend from the vector before actually closing it. @@ -86,13 +99,11 @@ ErrorResult Manager::CloseBackend(BackendPtr backend, ErrorResultCallback* cb) { backends.erase(it); } - if ( auto res = backend->Close(cb); res.has_value() ) { - return util::fmt("Failed to close backend %s: %s", backend->Tag(), res.value().c_str()); - } - - return std::nullopt; + auto res = backend->Close(cb); // TODO: post Storage::backend_lost event + + return res; } void Manager::Expire() { diff --git a/src/storage/Manager.h b/src/storage/Manager.h index 2f240e0486..badd2c6c5f 100644 --- a/src/storage/Manager.h +++ b/src/storage/Manager.h @@ -27,7 +27,7 @@ public: class Manager final : public plugin::ComponentManager { public: Manager(); - ~Manager() = default; + ~Manager(); /** * Initialization of the manager. This is called late during Zeek's @@ -36,8 +36,8 @@ public: void InitPostScript(); /** - * Instantiates a new backend object. The backend will be in a closed state, and OpenBackend() - * will need to be called to fully initialize it. + * Instantiates a new backend object. The backend will be in a closed state, + * and OpenBackend() will need to be called to fully initialize it. * * @param type The tag for the type of backend being opened. * @return A std::expected containing either a valid BackendPtr with the @@ -56,16 +56,22 @@ public: * validation of types. * @param val_type The script-side type of the values stored in the backend. Used for * validation of types and conversion during retrieval. - * @return An optional value potentially containing an error string if needed. Will be - * unset if the operation succeeded. + * @param cb An optional callback object if being called via an async context. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ErrorResult OpenBackend(BackendPtr backend, RecordValPtr options, TypePtr key_type, TypePtr val_type, - OpenResultCallback* cb = nullptr); + OperationResult OpenBackend(BackendPtr backend, RecordValPtr options, TypePtr key_type, TypePtr val_type, + OpenResultCallback* cb = nullptr); /** * Closes a storage backend. + * + * @param backend A pointer to the backend being closed. + * @param cb An optional callback object if being called via an async context. + * @return A struct describing the result of the operation, containing a code, an + * optional error string, and a ValPtr for operations that return values. */ - ErrorResult CloseBackend(BackendPtr backend, ErrorResultCallback* cb = nullptr); + OperationResult CloseBackend(BackendPtr backend, OperationResultCallback* cb = nullptr); void Expire(); diff --git a/src/storage/ReturnCode.cc b/src/storage/ReturnCode.cc new file mode 100644 index 0000000000..50d13554ce --- /dev/null +++ b/src/storage/ReturnCode.cc @@ -0,0 +1,77 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#include "zeek/storage/ReturnCode.h" + +#include "zeek/Val.h" + +namespace zeek::storage { + +EnumValPtr ReturnCode::SUCCESS; +EnumValPtr ReturnCode::VAL_TYPE_MISMATCH; +EnumValPtr ReturnCode::KEY_TYPE_MISMATCH; +EnumValPtr ReturnCode::NOT_CONNECTED; +EnumValPtr ReturnCode::TIMEOUT; +EnumValPtr ReturnCode::CONNECTION_LOST; +EnumValPtr ReturnCode::OPERATION_FAILED; +EnumValPtr ReturnCode::KEY_NOT_FOUND; +EnumValPtr ReturnCode::KEY_EXISTS; +EnumValPtr ReturnCode::CONNECTION_FAILED; +EnumValPtr ReturnCode::DISCONNECTION_FAILED; +EnumValPtr ReturnCode::INITIALIZATION_FAILED; + +void ReturnCode::Initialize() { + static const auto& return_code_type = zeek::id::find_type("Storage::ReturnCode"); + + auto tmp = return_code_type->Lookup("Storage::SUCCESS"); + SUCCESS = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::VAL_TYPE_MISMATCH"); + VAL_TYPE_MISMATCH = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::KEY_TYPE_MISMATCH"); + KEY_TYPE_MISMATCH = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::NOT_CONNECTED"); + NOT_CONNECTED = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::TIMEOUT"); + TIMEOUT = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::CONNECTION_LOST"); + CONNECTION_LOST = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::OPERATION_FAILED"); + OPERATION_FAILED = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::KEY_NOT_FOUND"); + KEY_NOT_FOUND = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::KEY_EXISTS"); + KEY_EXISTS = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::CONNECTION_FAILED"); + CONNECTION_FAILED = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::DISCONNECTION_FAILED"); + DISCONNECTION_FAILED = return_code_type->GetEnumVal(tmp); + + tmp = return_code_type->Lookup("Storage::INITIALIZATION_FAILED"); + INITIALIZATION_FAILED = return_code_type->GetEnumVal(tmp); +} + +void ReturnCode::Cleanup() { + SUCCESS.reset(); + VAL_TYPE_MISMATCH.reset(); + KEY_TYPE_MISMATCH.reset(); + NOT_CONNECTED.reset(); + TIMEOUT.reset(); + CONNECTION_LOST.reset(); + OPERATION_FAILED.reset(); + KEY_NOT_FOUND.reset(); + KEY_EXISTS.reset(); + CONNECTION_FAILED.reset(); + DISCONNECTION_FAILED.reset(); + INITIALIZATION_FAILED.reset(); +} + +} // namespace zeek::storage diff --git a/src/storage/ReturnCode.h b/src/storage/ReturnCode.h new file mode 100644 index 0000000000..826408c9df --- /dev/null +++ b/src/storage/ReturnCode.h @@ -0,0 +1,37 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#pragma once + +#include "zeek/IntrusivePtr.h" + +namespace zeek { +class EnumVal; +using EnumValPtr = IntrusivePtr; + +namespace storage { + +/** + * A collection of EnumValPtrs for the default set of result codes in the storage framework. + * should be kept up-to-date with the Storage::ReturnCodes script-level enum. + */ +class ReturnCode { +public: + static void Initialize(); + static void Cleanup(); + + static EnumValPtr SUCCESS; + static EnumValPtr VAL_TYPE_MISMATCH; + static EnumValPtr KEY_TYPE_MISMATCH; + static EnumValPtr NOT_CONNECTED; + static EnumValPtr TIMEOUT; + static EnumValPtr CONNECTION_LOST; + static EnumValPtr OPERATION_FAILED; + static EnumValPtr KEY_NOT_FOUND; + static EnumValPtr KEY_EXISTS; + static EnumValPtr CONNECTION_FAILED; + static EnumValPtr DISCONNECTION_FAILED; + static EnumValPtr INITIALIZATION_FAILED; +}; + +} // namespace storage +} // namespace zeek diff --git a/src/storage/backend/redis/Redis.cc b/src/storage/backend/redis/Redis.cc index d23f62a4e4..b1fb29ed03 100644 --- a/src/storage/backend/redis/Redis.cc +++ b/src/storage/backend/redis/Redis.cc @@ -7,6 +7,7 @@ #include "zeek/RunState.h" #include "zeek/Val.h" #include "zeek/iosource/Manager.h" +#include "zeek/storage/ReturnCode.h" #include "hiredis/adapters/poll.h" #include "hiredis/async.h" @@ -37,21 +38,21 @@ void redisOnDisconnect(const redisAsyncContext* ctx, int status) { void redisPut(redisAsyncContext* ctx, void* reply, void* privdata) { auto t = Tracer("put"); auto backend = static_cast(ctx->data); - auto callback = static_cast(privdata); + auto callback = static_cast(privdata); backend->HandlePutResult(static_cast(reply), callback); } void redisGet(redisAsyncContext* ctx, void* reply, void* privdata) { auto t = Tracer("get"); auto backend = static_cast(ctx->data); - auto callback = static_cast(privdata); + auto callback = static_cast(privdata); backend->HandleGetResult(static_cast(reply), callback); } void redisErase(redisAsyncContext* ctx, void* reply, void* privdata) { auto t = Tracer("erase"); auto backend = static_cast(ctx->data); - auto callback = static_cast(privdata); + auto callback = static_cast(privdata); backend->HandleEraseResult(static_cast(reply), callback); } @@ -122,7 +123,7 @@ storage::BackendPtr Redis::Instantiate(std::string_view tag) { return make_intru /** * Called by the manager system to open the backend. */ -ErrorResult Redis::DoOpen(RecordValPtr options, OpenResultCallback* cb) { +OperationResult Redis::DoOpen(RecordValPtr options, OpenResultCallback* cb) { RecordValPtr backend_options = options->GetField("redis"); key_prefix = backend_options->GetField("key_prefix")->ToStdString(); @@ -137,9 +138,10 @@ ErrorResult Redis::DoOpen(RecordValPtr options, OpenResultCallback* cb) { } else { StringValPtr unix_sock = backend_options->GetField("server_unix_socket"); - if ( ! unix_sock ) - return util::fmt( - "Either server_host/server_port or server_unix_socket must be set in Redis options record"); + if ( ! unix_sock ) { + return {ReturnCode::CONNECTION_FAILED, + "Either server_host/server_port or server_unix_socket must be set in Redis options record"}; + } server_addr = unix_sock->ToStdString(); REDIS_OPTIONS_SET_UNIX(&opt, server_addr.c_str()); @@ -164,11 +166,16 @@ ErrorResult Redis::DoOpen(RecordValPtr options, OpenResultCallback* cb) { redisAsyncFree(async_ctx); async_ctx = nullptr; - return errmsg; + return {ReturnCode::CONNECTION_FAILED, errmsg}; } ++active_ops; + // There's no way to pass privdata down to the connect handler like there is for + // the other callbacks. Store the open callback so that it can be dealt with from + // OnConnect(). + open_cb = cb; + // TODO: Sort out how to pass the zeek callbacks for both open/done to the async // callbacks from hiredis so they can return errors. @@ -199,13 +206,13 @@ ErrorResult Redis::DoOpen(RecordValPtr options, OpenResultCallback* cb) { async_ctx->ev.addWrite = redisAddWrite; async_ctx->ev.delWrite = redisDelWrite; - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * Finalizes the backend when it's being closed. */ -ErrorResult Redis::DoClose(ErrorResultCallback* cb) { +OperationResult Redis::DoClose(OperationResultCallback* cb) { connected = false; redisAsyncDisconnect(async_ctx); @@ -216,19 +223,22 @@ ErrorResult Redis::DoClose(ErrorResultCallback* cb) { // TODO: handle response } + CompleteCallback(cb, {ReturnCode::SUCCESS}); + redisAsyncFree(async_ctx); async_ctx = nullptr; - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * The workhorse method for Put(). This must be implemented by plugins. */ -ErrorResult Redis::DoPut(ValPtr key, ValPtr value, bool overwrite, double expiration_time, ErrorResultCallback* cb) { +OperationResult Redis::DoPut(ValPtr key, ValPtr value, bool overwrite, double expiration_time, + OperationResultCallback* cb) { // The async context will queue operations until it's connected fully. if ( ! connected && ! async_ctx ) - return "Connection is not open"; + return {ReturnCode::NOT_CONNECTED}; std::string format = "SET %s:%s %s"; if ( ! overwrite ) @@ -250,7 +260,7 @@ ErrorResult Redis::DoPut(ValPtr key, ValPtr value, bool overwrite, double expira json_value.data()); if ( connected && status == REDIS_ERR ) - return util::fmt("Failed to queue put operation: %s", async_ctx->errstr); + return {ReturnCode::OPERATION_FAILED, util::fmt("Failed to queue put operation: %s", async_ctx->errstr)}; ++active_ops; @@ -265,52 +275,52 @@ ErrorResult Redis::DoPut(ValPtr key, ValPtr value, bool overwrite, double expira status = redisAsyncCommand(async_ctx, redisGeneric, NULL, format.c_str(), key_prefix.data(), expiration_time, json_key.data()); if ( connected && status == REDIS_ERR ) - return util::fmt("ZADD operation failed: %s", async_ctx->errstr); + return {ReturnCode::OPERATION_FAILED, util::fmt("ZADD operation failed: %s", async_ctx->errstr)}; ++active_ops; } - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * The workhorse method for Get(). This must be implemented for plugins. */ -ValResult Redis::DoGet(ValPtr key, ValResultCallback* cb) { +OperationResult Redis::DoGet(ValPtr key, OperationResultCallback* cb) { // The async context will queue operations until it's connected fully. if ( ! connected && ! async_ctx ) - return zeek::unexpected("Connection is not open"); + return {ReturnCode::NOT_CONNECTED}; int status = redisAsyncCommand(async_ctx, redisGet, cb, "GET %s:%s", key_prefix.data(), key->ToJSON()->ToStdStringView().data()); if ( connected && status == REDIS_ERR ) - return zeek::unexpected(util::fmt("Failed to queue get operation: %s", async_ctx->errstr)); + return {ReturnCode::OPERATION_FAILED, util::fmt("Failed to queue get operation: %s", async_ctx->errstr)}; ++active_ops; // There isn't a result to return here. That happens in HandleGetResult for // async operations. - return zeek::unexpected(""); + return {ReturnCode::SUCCESS}; } /** * The workhorse method for Erase(). This must be implemented for plugins. */ -ErrorResult Redis::DoErase(ValPtr key, ErrorResultCallback* cb) { +OperationResult Redis::DoErase(ValPtr key, OperationResultCallback* cb) { // The async context will queue operations until it's connected fully. if ( ! connected && ! async_ctx ) - return "Connection is not open"; + return {ReturnCode::NOT_CONNECTED}; int status = redisAsyncCommand(async_ctx, redisErase, cb, "DEL %s:%s", key_prefix.data(), key->ToJSON()->ToStdStringView().data()); if ( connected && status == REDIS_ERR ) - return util::fmt("Failed to queue erase operation failed: %s", async_ctx->errstr); + return {ReturnCode::OPERATION_FAILED, async_ctx->errstr}; ++active_ops; - return std::nullopt; + return {ReturnCode::SUCCESS}; } void Redis::Expire() { @@ -335,11 +345,15 @@ void Redis::Expire() { redisReply* reply = reply_queue.front(); reply_queue.pop_front(); - if ( reply && reply->elements == 0 ) { + if ( reply->elements == 0 ) { freeReplyObject(reply); return; } + // The data from the reply to ZRANGEBYSCORE gets deleted as part of the + // commands below so we don't need to free it manually. Doing so results in + // a double-free. + // TODO: it's possible to pass multiple keys to a DEL operation but it requires // building an array of the strings, building up the DEL command with entries, // and passing the array as a block somehow. There's no guarantee it'd be faster @@ -357,33 +371,29 @@ void Redis::Expire() { ++active_ops; Poll(); - - // This can't be freed until the other commands finish because the memory for - // the strings doesn't get copied when making the DEL commands. - // freeReplyObject(reply); } -void Redis::HandlePutResult(redisReply* reply, ErrorResultCallback* callback) { +void Redis::HandlePutResult(redisReply* reply, OperationResultCallback* callback) { --active_ops; - ErrorResult res; + OperationResult res{ReturnCode::SUCCESS}; if ( ! connected ) - res = util::fmt("Connection is not open"); + res = {ReturnCode::NOT_CONNECTED}; else if ( ! reply ) - res = util::fmt("Async put operation returned null reply"); + res = {ReturnCode::OPERATION_FAILED, "Async put operation returned null reply"}; else if ( reply && reply->type == REDIS_REPLY_ERROR ) - res = util::fmt("Async put operation failed: %s", reply->str); + res = {ReturnCode::OPERATION_FAILED, util::fmt("Async put operation failed: %s", reply->str)}; freeReplyObject(reply); CompleteCallback(callback, res); } -void Redis::HandleGetResult(redisReply* reply, ValResultCallback* callback) { +void Redis::HandleGetResult(redisReply* reply, OperationResultCallback* callback) { --active_ops; - ValResult res; + OperationResult res; if ( ! connected ) - res = zeek::unexpected("Connection is not open"); + res = {ReturnCode::NOT_CONNECTED}; else res = ParseGetReply(reply); @@ -391,19 +401,20 @@ void Redis::HandleGetResult(redisReply* reply, ValResultCallback* callback) { CompleteCallback(callback, res); } -void Redis::HandleEraseResult(redisReply* reply, ErrorResultCallback* callback) { +void Redis::HandleEraseResult(redisReply* reply, OperationResultCallback* callback) { --active_ops; if ( callback->IsSyncCallback() ) reply_queue.push_back(reply); else { - ErrorResult res; + OperationResult res{ReturnCode::SUCCESS}; + if ( ! connected ) - res = "Connection is not open"; + res = {ReturnCode::NOT_CONNECTED}; else if ( ! reply ) - res = util::fmt("Async erase operation returned null reply"); + res = {ReturnCode::OPERATION_FAILED, "Async erase operation returned null reply"}; else if ( reply && reply->type == REDIS_REPLY_ERROR ) - res = util::fmt("Async erase operation failed: %s", reply->str); + res = {ReturnCode::OPERATION_FAILED, util::fmt("Async erase operation failed: %s", reply->str)}; freeReplyObject(reply); CompleteCallback(callback, res); @@ -421,9 +432,14 @@ void Redis::OnConnect(int status) { if ( status == REDIS_OK ) { connected = true; + CompleteCallback(open_cb, {ReturnCode::SUCCESS}); + // TODO: post connect event return; } + connected = false; + CompleteCallback(open_cb, {ReturnCode::CONNECTION_FAILED}); + // TODO: we could attempt to reconnect here } @@ -436,6 +452,7 @@ void Redis::OnDisconnect(int status) { } else { // TODO: this was unintentional, should we reconnect? + // TODO: post disconnect event } connected = false; @@ -448,19 +465,19 @@ void Redis::ProcessFd(int fd, int flags) { redisAsyncHandleWrite(async_ctx); } -ValResult Redis::ParseGetReply(redisReply* reply) const { - ValResult res; +OperationResult Redis::ParseGetReply(redisReply* reply) const { + OperationResult res; if ( ! reply ) - res = zeek::unexpected("GET returned null reply"); + res = {ReturnCode::OPERATION_FAILED, "GET returned null reply"}; else if ( ! reply->str ) - res = zeek::unexpected("GET returned key didn't exist"); + res = {ReturnCode::KEY_NOT_FOUND}; else { auto val = zeek::detail::ValFromJSON(reply->str, val_type, Func::nil); if ( std::holds_alternative(val) ) - res = std::get(val); + res = {ReturnCode::SUCCESS, "", std::get(val)}; else - res = zeek::unexpected(std::get(val)); + res = {ReturnCode::OPERATION_FAILED, std::get(val)}; } return res; diff --git a/src/storage/backend/redis/Redis.h b/src/storage/backend/redis/Redis.h index 3a7dd21dcc..b5a65d12d2 100644 --- a/src/storage/backend/redis/Redis.h +++ b/src/storage/backend/redis/Redis.h @@ -29,12 +29,12 @@ public: /** * Called by the manager system to open the backend. */ - ErrorResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override; + OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override; /** * Finalizes the backend when it's being closed. */ - ErrorResult DoClose(ErrorResultCallback* cb = nullptr) override; + OperationResult DoClose(OperationResultCallback* cb = nullptr) override; /** * Returns whether the backend is opened. @@ -44,18 +44,18 @@ public: /** * The workhorse method for Retrieve(). */ - ErrorResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, - ErrorResultCallback* cb = nullptr) override; + OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, + OperationResultCallback* cb = nullptr) override; /** * The workhorse method for Get(). */ - ValResult DoGet(ValPtr key, ValResultCallback* cb = nullptr) override; + OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) override; /** * The workhorse method for Erase(). */ - ErrorResult DoErase(ValPtr key, ErrorResultCallback* cb = nullptr) override; + OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) override; /** * Removes any entries in the backend that have expired. Can be overridden by @@ -72,9 +72,9 @@ public: void OnConnect(int status); void OnDisconnect(int status); - void HandlePutResult(redisReply* reply, ErrorResultCallback* callback); - void HandleGetResult(redisReply* reply, ValResultCallback* callback); - void HandleEraseResult(redisReply* reply, ErrorResultCallback* callback); + void HandlePutResult(redisReply* reply, OperationResultCallback* callback); + void HandleGetResult(redisReply* reply, OperationResultCallback* callback); + void HandleEraseResult(redisReply* reply, OperationResultCallback* callback); void HandleZRANGEBYSCORE(redisReply* reply); // HandleGeneric exists so that async-running-as-sync operations can remove @@ -85,7 +85,7 @@ protected: void Poll() override; private: - ValResult ParseGetReply(redisReply* reply) const; + OperationResult ParseGetReply(redisReply* reply) const; redisAsyncContext* async_ctx = nullptr; @@ -94,6 +94,8 @@ private: // poll. std::deque reply_queue; + OpenResultCallback* open_cb; + std::string server_addr; std::string key_prefix; std::atomic connected = false; diff --git a/src/storage/backend/sqlite/SQLite.cc b/src/storage/backend/sqlite/SQLite.cc index 34f1aebcfd..ddac358994 100644 --- a/src/storage/backend/sqlite/SQLite.cc +++ b/src/storage/backend/sqlite/SQLite.cc @@ -5,6 +5,7 @@ #include "zeek/3rdparty/sqlite3.h" #include "zeek/Func.h" #include "zeek/Val.h" +#include "zeek/storage/ReturnCode.h" namespace zeek::storage::backend::sqlite { @@ -13,13 +14,13 @@ storage::BackendPtr SQLite::Instantiate(std::string_view tag) { return make_intr /** * Called by the manager system to open the backend. */ -ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { +OperationResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { if ( sqlite3_threadsafe() == 0 ) { std::string res = "SQLite reports that it is not threadsafe. Zeek needs a threadsafe version of " "SQLite. Aborting"; Error(res.c_str()); - return res; + return {ReturnCode::INITIALIZATION_FAILED, res}; } // Allow connections to same DB to use single data/schema cache. Also @@ -33,10 +34,10 @@ ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { full_path = zeek::filesystem::path(path->ToStdString()).string(); table_name = backend_options->GetField("table_name")->ToStdString(); - auto open_res = - checkError(sqlite3_open_v2(full_path.c_str(), &db, - SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL)); - if ( open_res.has_value() ) { + if ( auto open_res = + checkError(sqlite3_open_v2(full_path.c_str(), &db, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL)); + open_res.code != ReturnCode::SUCCESS ) { sqlite3_close_v2(db); db = nullptr; return open_res; @@ -51,7 +52,7 @@ ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { Error(err.c_str()); sqlite3_free(errorMsg); Close(); - return err; + return {ReturnCode::INITIALIZATION_FAILED, err}; } if ( int res = sqlite3_exec(db, "pragma integrity_check", NULL, NULL, &errorMsg); res != SQLITE_OK ) { @@ -59,7 +60,7 @@ ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { Error(err.c_str()); sqlite3_free(errorMsg); Close(); - return err; + return {ReturnCode::INITIALIZATION_FAILED, err}; } auto tuning_params = backend_options->GetField("tuning_params")->ToMap(); @@ -73,7 +74,7 @@ ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { Error(err.c_str()); sqlite3_free(errorMsg); Close(); - return err; + return {ReturnCode::INITIALIZATION_FAILED, err}; } } @@ -91,7 +92,7 @@ ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { for ( const auto& [key, stmt] : statements ) { sqlite3_stmt* ps; if ( auto prep_res = checkError(sqlite3_prepare_v2(db, stmt.c_str(), stmt.size(), &ps, NULL)); - prep_res.has_value() ) { + prep_res.code != ReturnCode::SUCCESS ) { Close(); return prep_res; } @@ -99,14 +100,14 @@ ErrorResult SQLite::DoOpen(RecordValPtr options, OpenResultCallback* cb) { prepared_stmts.insert({key, ps}); } - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * Finalizes the backend when it's being closed. */ -ErrorResult SQLite::DoClose(ErrorResultCallback* cb) { - ErrorResult err_res; +OperationResult SQLite::DoClose(OperationResultCallback* cb) { + OperationResult op_res{ReturnCode::SUCCESS}; if ( db ) { for ( const auto& [k, stmt] : prepared_stmts ) { @@ -117,27 +118,29 @@ ErrorResult SQLite::DoClose(ErrorResultCallback* cb) { char* errmsg; if ( int res = sqlite3_exec(db, "pragma optimize", NULL, NULL, &errmsg); res != SQLITE_OK ) { - err_res = util::fmt("Sqlite failed to optimize at shutdown: %s", errmsg); + op_res = {ReturnCode::DISCONNECTION_FAILED, util::fmt("Sqlite failed to optimize at shutdown: %s", errmsg)}; sqlite3_free(&errmsg); + // TODO: we're shutting down. does this error matter other than being informational? } if ( int res = sqlite3_close_v2(db); res != SQLITE_OK ) { - if ( ! err_res.has_value() ) - err_res = "Sqlite could not close connection"; + if ( op_res.err_str.empty() ) + op_res.err_str = "Sqlite could not close connection"; } db = nullptr; } - return err_res; + return op_res; } /** * The workhorse method for Put(). This must be implemented by plugins. */ -ErrorResult SQLite::DoPut(ValPtr key, ValPtr value, bool overwrite, double expiration_time, ErrorResultCallback* cb) { +OperationResult SQLite::DoPut(ValPtr key, ValPtr value, bool overwrite, double expiration_time, + OperationResultCallback* cb) { if ( ! db ) - return "Database was not open"; + return {ReturnCode::NOT_CONNECTED}; auto json_key = key->ToJSON(); auto json_value = value->ToJSON(); @@ -150,56 +153,56 @@ ErrorResult SQLite::DoPut(ValPtr key, ValPtr value, bool overwrite, double expir auto key_str = json_key->ToStdStringView(); if ( auto res = checkError(sqlite3_bind_text(stmt, 1, key_str.data(), key_str.size(), SQLITE_STATIC)); - res.has_value() ) { + res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); return res; } auto value_str = json_value->ToStdStringView(); if ( auto res = checkError(sqlite3_bind_text(stmt, 2, value_str.data(), value_str.size(), SQLITE_STATIC)); - res.has_value() ) { + res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); return res; } - if ( auto res = checkError(sqlite3_bind_double(stmt, 3, expiration_time)); res.has_value() ) { + if ( auto res = checkError(sqlite3_bind_double(stmt, 3, expiration_time)); res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); return res; } if ( overwrite ) { if ( auto res = checkError(sqlite3_bind_text(stmt, 4, value_str.data(), value_str.size(), SQLITE_STATIC)); - res.has_value() ) { + res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); return res; } } - if ( auto res = checkError(sqlite3_step(stmt)); res.has_value() ) { + if ( auto res = checkError(sqlite3_step(stmt)); res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); return res; } sqlite3_reset(stmt); - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * The workhorse method for Get(). This must be implemented for plugins. */ -ValResult SQLite::DoGet(ValPtr key, ValResultCallback* cb) { +OperationResult SQLite::DoGet(ValPtr key, OperationResultCallback* cb) { if ( ! db ) - return zeek::unexpected("Database was not open"); + return {ReturnCode::NOT_CONNECTED}; auto json_key = key->ToJSON(); auto stmt = prepared_stmts["get"]; auto key_str = json_key->ToStdStringView(); if ( auto res = checkError(sqlite3_bind_text(stmt, 1, key_str.data(), key_str.size(), SQLITE_STATIC)); - res.has_value() ) { + res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); - return zeek::unexpected(res.value()); + return res; } int errorcode = sqlite3_step(stmt); @@ -210,38 +213,38 @@ ValResult SQLite::DoGet(ValPtr key, ValResultCallback* cb) { sqlite3_reset(stmt); if ( std::holds_alternative(val) ) { ValPtr val_v = std::get(val); - return val_v; + return {ReturnCode::SUCCESS, "", val_v}; } else { - return zeek::unexpected(std::get(val)); + return {ReturnCode::OPERATION_FAILED, std::get(val)}; } } - return zeek::unexpected(util::fmt("Failed to find row for key: %s", sqlite3_errstr(errorcode))); + return {ReturnCode::KEY_NOT_FOUND}; } /** * The workhorse method for Erase(). This must be implemented for plugins. */ -ErrorResult SQLite::DoErase(ValPtr key, ErrorResultCallback* cb) { +OperationResult SQLite::DoErase(ValPtr key, OperationResultCallback* cb) { if ( ! db ) - return "Database was not open"; + return {ReturnCode::NOT_CONNECTED}; auto json_key = key->ToJSON(); auto stmt = prepared_stmts["erase"]; auto key_str = json_key->ToStdStringView(); if ( auto res = checkError(sqlite3_bind_text(stmt, 1, key_str.data(), key_str.size(), SQLITE_STATIC)); - res.has_value() ) { + res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); return res; } - if ( auto res = checkError(sqlite3_step(stmt)); res.has_value() ) { + if ( auto res = checkError(sqlite3_step(stmt)); res.code != ReturnCode::SUCCESS ) { return res; } - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** @@ -251,23 +254,24 @@ ErrorResult SQLite::DoErase(ValPtr key, ErrorResultCallback* cb) { void SQLite::Expire() { auto stmt = prepared_stmts["expire"]; - if ( auto res = checkError(sqlite3_bind_double(stmt, 1, run_state::network_time)); res.has_value() ) { + if ( auto res = checkError(sqlite3_bind_double(stmt, 1, run_state::network_time)); + res.code != ReturnCode::SUCCESS ) { sqlite3_reset(stmt); // TODO: do something with the error here? } - if ( auto res = checkError(sqlite3_step(stmt)); res.has_value() ) { + if ( auto res = checkError(sqlite3_step(stmt)); res.code != ReturnCode::SUCCESS ) { // TODO: do something with the error here? } } // returns true in case of error -ErrorResult SQLite::checkError(int code) { +OperationResult SQLite::checkError(int code) { if ( code != SQLITE_OK && code != SQLITE_DONE ) { - return util::fmt("SQLite call failed: %s", sqlite3_errmsg(db)); + return {ReturnCode::OPERATION_FAILED, util::fmt("SQLite call failed: %s", sqlite3_errmsg(db)), nullptr}; } - return std::nullopt; + return {ReturnCode::SUCCESS}; } } // namespace zeek::storage::backend::sqlite diff --git a/src/storage/backend/sqlite/SQLite.h b/src/storage/backend/sqlite/SQLite.h index ad0869bad2..17e3e9716d 100644 --- a/src/storage/backend/sqlite/SQLite.h +++ b/src/storage/backend/sqlite/SQLite.h @@ -20,12 +20,12 @@ public: /** * Called by the manager system to open the backend. */ - ErrorResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override; + OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override; /** * Finalizes the backend when it's being closed. */ - ErrorResult DoClose(ErrorResultCallback* cb = nullptr) override; + OperationResult DoClose(OperationResultCallback* cb = nullptr) override; /** * Returns whether the backend is opened. @@ -35,18 +35,18 @@ public: /** * The workhorse method for Put(). */ - ErrorResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, - ErrorResultCallback* cb = nullptr) override; + OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0, + OperationResultCallback* cb = nullptr) override; /** * The workhorse method for Get(). */ - ValResult DoGet(ValPtr key, ValResultCallback* cb = nullptr) override; + OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) override; /** * The workhorse method for Erase(). */ - ErrorResult DoErase(ValPtr key, ErrorResultCallback* cb = nullptr) override; + OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) override; /** * Removes any entries in the backend that have expired. Can be overridden by @@ -55,7 +55,7 @@ public: void Expire() override; private: - ErrorResult checkError(int code); + OperationResult checkError(int code); sqlite3* db = nullptr; std::unordered_map prepared_stmts; diff --git a/src/storage/storage.bif b/src/storage/storage.bif index a29c5fd801..013b95e8be 100644 --- a/src/storage/storage.bif +++ b/src/storage/storage.bif @@ -1,8 +1,9 @@ %%{ -#include "zeek/storage/Backend.h" -#include "zeek/storage/Manager.h" #include "zeek/Trigger.h" #include "zeek/Frame.h" +#include "zeek/storage/Backend.h" +#include "zeek/storage/Manager.h" +#include "zeek/storage/ReturnCode.h" using namespace zeek; using namespace zeek::storage; @@ -52,11 +53,11 @@ event Storage::backend_lost%(%); module Storage::Async; -function Storage::Async::__open_backend%(btype: Storage::Backend, options: any, key_type: any, val_type: any%): opaque of Storage::BackendHandle +function Storage::Async::__open_backend%(btype: Storage::Backend, options: any, key_type: any, val_type: any%): Storage::OperationResult %{ auto trigger = init_trigger(frame); if ( ! trigger ) - return val_mgr->Bool(false); + return nullptr; auto btype_val = IntrusivePtr{NewRef{}, btype->AsEnumVal()}; Tag tag{btype_val}; @@ -64,13 +65,13 @@ function Storage::Async::__open_backend%(btype: Storage::Backend, options: any, auto b = storage_mgr->Instantiate(tag); if ( ! b.has_value() ) { - emit_builtin_error(b.error().c_str()); + emit_builtin_error(util::fmt("Failed to instantiate backend: %s", b.error().c_str())); return nullptr; } auto bh = make_intrusive(b.value()); - auto cb = new OpenResultCallback(trigger, frame->GetTriggerAssoc(), bh.release()); + auto cb = new OpenResultCallback(trigger, frame->GetTriggerAssoc(), bh); auto kt = key_type->AsTypeVal()->GetType()->AsTypeType()->GetType(); auto vt = val_type->AsTypeVal()->GetType()->AsTypeType()->GetType(); auto options_val = IntrusivePtr{NewRef{}, options->AsRecordVal()}; @@ -91,21 +92,28 @@ function Storage::Async::__open_backend%(btype: Storage::Backend, options: any, return nullptr; %} -function Storage::Async::__close_backend%(backend: opaque of Storage::BackendHandle%) : bool +function Storage::Async::__close_backend%(backend: opaque of Storage::BackendHandle%) : Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto op_result = make_intrusive(op_result_type); + auto trigger = init_trigger(frame); if ( ! trigger ) - return val_mgr->Bool(false); + return nullptr; auto b = dynamic_cast(backend); if ( ! b ) { - emit_builtin_error("Invalid storage handle", backend); - return val_mgr->Bool(false); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; + } + else if ( ! b->backend->IsOpen() ) { + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - else if ( ! b->backend->IsOpen() ) - return val_mgr->Bool(true); - auto cb = new ErrorResultCallback(trigger, frame->GetTriggerAssoc()); + auto cb = new OperationResultCallback(trigger, frame->GetTriggerAssoc()); auto close_res = storage_mgr->CloseBackend(b->backend, cb); if ( ! b->backend->SupportsAsync() ) { @@ -124,24 +132,31 @@ function Storage::Async::__close_backend%(backend: opaque of Storage::BackendHan %} function Storage::Async::__put%(backend: opaque of Storage::BackendHandle, key: any, value: any, - overwrite: bool, expire_time: interval%): bool + overwrite: bool, expire_time: interval%): Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto op_result = make_intrusive(op_result_type); + auto trigger = init_trigger(frame); if ( ! trigger ) - return val_mgr->Bool(false); + return nullptr; auto b = dynamic_cast(backend); if ( ! b ) { - emit_builtin_error("Invalid storage handle", backend); - return val_mgr->Bool(false); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; + } + else if ( ! b->backend->IsOpen() ) { + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - else if ( ! b->backend->IsOpen() ) - return val_mgr->Bool(false); if ( expire_time > 0.0 ) expire_time += run_state::network_time; - auto cb = new ErrorResultCallback(trigger, frame->GetTriggerAssoc()); + auto cb = new OperationResultCallback(trigger, frame->GetTriggerAssoc()); auto key_v = IntrusivePtr{NewRef{}, key}; auto val_v = IntrusivePtr{NewRef{}, value}; auto put_res = b->backend->Put(key_v, val_v, overwrite, expire_time, cb); @@ -161,28 +176,28 @@ function Storage::Async::__put%(backend: opaque of Storage::BackendHandle, key: return nullptr; %} -function Storage::Async::__get%(backend: opaque of Storage::BackendHandle, key: any%): val_result +function Storage::Async::__get%(backend: opaque of Storage::BackendHandle, key: any%): Storage::OperationResult %{ - static auto val_result_type = id::find_type("val_result"); - auto val_result = make_intrusive(val_result_type); + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto op_result = make_intrusive(op_result_type); auto trigger = init_trigger(frame); - if ( ! trigger ) { - val_result->Assign(1, make_intrusive("Failed to create trigger")); - return val_result; - } + if ( ! trigger ) + return nullptr; auto b = dynamic_cast(backend); if ( ! b ) { - val_result->Assign(1, make_intrusive("Invalid storage handlle")); - return val_result; + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; } else if ( ! b->backend->IsOpen() ) { - val_result->Assign(1, make_intrusive("Backend is closed")); - return val_result; + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - auto cb = new ValResultCallback(trigger, frame->GetTriggerAssoc()); + auto cb = new OperationResultCallback(trigger, frame->GetTriggerAssoc()); auto key_v = IntrusivePtr{NewRef{}, key}; auto get_res = b->backend->Get(key_v, cb); @@ -201,21 +216,28 @@ function Storage::Async::__get%(backend: opaque of Storage::BackendHandle, key: return nullptr; %} -function Storage::Async::__erase%(backend: opaque of Storage::BackendHandle, key: any%): bool +function Storage::Async::__erase%(backend: opaque of Storage::BackendHandle, key: any%): Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto op_result = make_intrusive(op_result_type); + auto trigger = init_trigger(frame); if ( ! trigger ) - return val_mgr->Bool(false); + return nullptr; auto b = dynamic_cast(backend); if ( ! b ) { - emit_builtin_error("Invalid storage handle", backend); - return val_mgr->Bool(false); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; + } + else if ( ! b->backend->IsOpen() ) { + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - else if ( ! b->backend->IsOpen() ) - return val_mgr->Bool(false); - auto cb = new ErrorResultCallback(trigger, frame->GetTriggerAssoc()); + auto cb = new OperationResultCallback(trigger, frame->GetTriggerAssoc()); auto key_v = IntrusivePtr{NewRef{}, key}; auto erase_res = b->backend->Erase(key_v, cb); @@ -236,8 +258,10 @@ function Storage::Async::__erase%(backend: opaque of Storage::BackendHandle, key module Storage::Sync; -function Storage::Sync::__open_backend%(btype: Storage::Backend, options: any, key_type: any, val_type: any%): opaque of Storage::BackendHandle +function Storage::Sync::__open_backend%(btype: Storage::Backend, options: any, key_type: any, val_type: any%): Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto btype_val = IntrusivePtr{NewRef{}, btype->AsEnumVal()}; Tag tag{btype_val}; @@ -250,7 +274,7 @@ function Storage::Sync::__open_backend%(btype: Storage::Backend, options: any, k auto bh = make_intrusive(b.value()); - auto cb = new OpenResultCallback(bh.get()); + auto cb = new OpenResultCallback(bh); auto kt = key_type->AsTypeVal()->GetType()->AsTypeType()->GetType(); auto vt = val_type->AsTypeVal()->GetType()->AsTypeType()->GetType(); auto options_val = IntrusivePtr{NewRef{}, options->AsRecordVal()}; @@ -265,26 +289,28 @@ function Storage::Sync::__open_backend%(btype: Storage::Backend, options: any, k delete cb; - if ( open_res.has_value() ) { - emit_builtin_error(open_res.value().c_str()); - return val_mgr->Bool(false); - } - - return bh; + return open_res.BuildVal(); %} -function Storage::Sync::__close_backend%(backend: opaque of Storage::BackendHandle%) : bool +function Storage::Sync::__close_backend%(backend: opaque of Storage::BackendHandle%) : Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto b = dynamic_cast(backend); if ( ! b ) { - emit_builtin_error("Invalid storage handle", backend); - return val_mgr->Bool(false); + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; + } + else if ( ! b->backend->IsOpen() ) { + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - else if ( ! b->backend->IsOpen() ) - // Return true here since the backend is already closed - return val_mgr->Bool(true); - auto cb = new ErrorResultCallback(); + auto cb = new OperationResultCallback(); auto close_res = storage_mgr->CloseBackend(b->backend, cb); // If the backend only supports async, block until it's ready and then pull the result out of @@ -296,26 +322,29 @@ function Storage::Sync::__close_backend%(backend: opaque of Storage::BackendHand delete cb; - if ( close_res.has_value() ) { - emit_builtin_error(close_res.value().c_str()); - return val_mgr->Bool(false); - } - - return val_mgr->Bool(true); + return close_res.BuildVal(); %} function Storage::Sync::__put%(backend: opaque of Storage::BackendHandle, key: any, value: any, - overwrite: bool, expire_time: interval%): bool + overwrite: bool, expire_time: interval%): Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto b = dynamic_cast(backend); if ( ! b ) { - emit_builtin_error("Invalid storage handle", backend); - return val_mgr->Bool(false); + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; + } + else if ( ! b->backend->IsOpen() ) { + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - else if ( ! b->backend->IsOpen() ) - return val_mgr->Bool(false); - auto cb = new ErrorResultCallback(); + auto cb = new OperationResultCallback(); auto key_v = IntrusivePtr{NewRef{}, key}; auto val_v = IntrusivePtr{NewRef{}, value}; auto put_res = b->backend->Put(key_v, val_v, overwrite, expire_time, cb); @@ -329,31 +358,29 @@ function Storage::Sync::__put%(backend: opaque of Storage::BackendHandle, key: a delete cb; - if ( put_res.has_value() ) { - emit_builtin_error(util::fmt("Failed to store data: %s", put_res.value().c_str())); - return val_mgr->Bool(false); - } - - return val_mgr->Bool(true); + return put_res.BuildVal(); %} -function Storage::Sync::__get%(backend: opaque of Storage::BackendHandle, key: any%): val_result +function Storage::Sync::__get%(backend: opaque of Storage::BackendHandle, key: any%): Storage::OperationResult %{ - static auto val_result_type = id::find_type("val_result"); - auto val_result = make_intrusive(val_result_type); + static auto op_result_type = id::find_type("Storage::OperationResult"); auto b = dynamic_cast(backend); if ( ! b ) { - val_result->Assign(1, make_intrusive("Invalid storage handlle")); - return val_result; + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; } else if ( ! b->backend->IsOpen() ) { - val_result->Assign(1, make_intrusive("Backend is closed")); - return val_result; + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } auto key_v = IntrusivePtr{NewRef{}, key}; - auto cb = new ValResultCallback(); + auto cb = new OperationResultCallback(); auto get_res = b->backend->Get(key_v, cb); // If the backend only supports async, block until it's ready and then pull the result out of @@ -365,27 +392,28 @@ function Storage::Sync::__get%(backend: opaque of Storage::BackendHandle, key: a delete cb; - if ( ! get_res.has_value() ) { - val_result->Assign(1, make_intrusive( - util::fmt("Failed to retrieve data: %s", get_res.error().c_str()))); - return val_result; - } - - val_result->Assign(0, get_res.value()); - return val_result; + return get_res.BuildVal(); %} -function Storage::Sync::__erase%(backend: opaque of Storage::BackendHandle, key: any%): bool +function Storage::Sync::__erase%(backend: opaque of Storage::BackendHandle, key: any%): Storage::OperationResult %{ + static auto op_result_type = id::find_type("Storage::OperationResult"); + auto b = dynamic_cast(backend); if ( ! b ) { - emit_builtin_error("Invalid storage handle", backend); - return val_mgr->Bool(false); + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::OPERATION_FAILED); + op_result->Assign(1, make_intrusive("Invalid storage handlle")); + return op_result; + } + else if ( ! b->backend->IsOpen() ) { + auto op_result = make_intrusive(op_result_type); + op_result->Assign(0, ReturnCode::NOT_CONNECTED); + op_result->Assign(1, make_intrusive("Backend is closed")); + return op_result; } - else if ( ! b->backend->IsOpen() ) - return val_mgr->Bool(false); - auto cb = new ErrorResultCallback(); + auto cb = new OperationResultCallback(); auto key_v = IntrusivePtr{NewRef{}, key}; auto erase_res = b->backend->Erase(key_v, cb); @@ -398,10 +426,5 @@ function Storage::Sync::__erase%(backend: opaque of Storage::BackendHandle, key: delete cb; - if ( erase_res.has_value() ) { - emit_builtin_error(util::fmt("Failed to erase data for key: %s", erase_res.value().c_str())); - return val_mgr->Bool(false); - } - - return val_mgr->Bool(true); + return erase_res.BuildVal(); %} diff --git a/testing/btest/Baseline/plugins.storage/output b/testing/btest/Baseline/plugins.storage/output index 0812d557db..0e95ed4a97 100644 --- a/testing/btest/Baseline/plugins.storage/output +++ b/testing/btest/Baseline/plugins.storage/output @@ -1,2 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -results of trying to use closed handle: get: 0, put: 0, erase: 0 +open result, [code=Storage::SUCCESS, error_str=, value=] +results of trying to use closed handle: get: Storage::NOT_CONNECTED, put: Storage::NOT_CONNECTED, erase: Storage::NOT_CONNECTED +open result 2, [code=Storage::OPERATION_FAILED, error_str=Failed to open backend Storage::STORAGEDUMMY: open_fail was set to true, returning error, value=] +close result of closed handle, [code=Storage::NOT_CONNECTED, error_str=Backend is closed, value=] diff --git a/testing/btest/Baseline/plugins.storage/zeek-stderr b/testing/btest/Baseline/plugins.storage/zeek-stderr index 8dcb29da7c..49d861c74c 100644 --- a/testing/btest/Baseline/plugins.storage/zeek-stderr +++ b/testing/btest/Baseline/plugins.storage/zeek-stderr @@ -1,4 +1 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/storage.zeek, line 45: Failed to retrieve data: Failed to find key -error in <...>/sync.zeek, line 75: Failed to open backend Storage::STORAGEDUMMY: open_fail was set to true, returning error (Storage::Sync::__open_backend(Storage::Sync::btype, to_any_coerce Storage::Sync::options, Storage::Sync::key_type, Storage::Sync::val_type)) -error in <...>/sync.zeek, line 80: Invalid storage handle (Storage::Sync::__close_backend(Storage::Sync::backend) and F) diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.compound-types/out b/testing/btest/Baseline/scripts.base.frameworks.storage.compound-types/out index 0b2661d2e3..96bc442e9c 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.compound-types/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.compound-types/out @@ -1,7 +1,8 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val={ +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value={ [2] = b, [1] = a, [3] = c -}, error=] +}] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.erase/out b/testing/btest/Baseline/scripts.base.frameworks.storage.erase/out index 7ca5354b61..5649e321fd 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.erase/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.erase/out @@ -1,3 +1,4 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -erase result, T -get result, Failed to retrieve data: Failed to find row for key: no more rows available +open result, [code=Storage::SUCCESS, error_str=, value=] +erase result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::KEY_NOT_FOUND, error_str=, value=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.expiration/out b/testing/btest/Baseline/scripts.base.frameworks.storage.expiration/out index 08758e1bf2..c5b4964ce2 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.expiration/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.expiration/out @@ -1,5 +1,6 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value7890, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value7890] get result same as inserted, T -get result, Failed to retrieve data: Failed to find row for key: no more rows available +get result, [code=Storage::KEY_NOT_FOUND, error_str=, value=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.overwriting/out b/testing/btest/Baseline/scripts.base.frameworks.storage.overwriting/out index d1338d7ba9..c65062e83a 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.overwriting/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.overwriting/out @@ -1,4 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value7890, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value7890] get result same as inserted, T diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async-reading-pcap/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async-reading-pcap/out index b32c104878..960761de7b 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async-reading-pcap/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async-reading-pcap/out @@ -1,4 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value5678, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value5678] get result same as inserted, T diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async/out index b32c104878..3c40bed4be 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-async/out @@ -1,4 +1,6 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value5678, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value5678] get result same as inserted, T +close result, [code=Storage::SUCCESS, error_str=, value=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-1..stdout b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-1..stdout index 01c6005755..8fc986fdaf 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-1..stdout +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-1..stdout @@ -1,4 +1,4 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -worker-1, put result, T +worker-1, put result, [code=Storage::SUCCESS, error_str=, value=] redis_data_written -worker-1, [val=5678, error=] +worker-1, [code=Storage::SUCCESS, error_str=, value=5678] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-2..stdout b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-2..stdout index 76ee9cc9df..bd57bb7fe5 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-2..stdout +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-cluster/worker-2..stdout @@ -1,3 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. redis_data_written -worker-2, [val=5678, error=] +worker-2, [code=Storage::SUCCESS, error_str=, value=5678] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-expiration/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-expiration/out index 996cf2ebac..e2421b325f 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-expiration/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-expiration/out @@ -1,5 +1,6 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value7890, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value7890] get result same as inserted, T -get result after expiration, [val=, error=Failed to retrieve data: GET returned key didn't exist] +get result after expiration, [code=Storage::KEY_NOT_FOUND, error_str=, value=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out index b77104dcd8..2f1d82d855 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.redis-sync/out @@ -1,7 +1,8 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value1234, error=] +open_result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value1234] get result same as inserted, T -overwrite put result, T -get result, [val=value5678, error=] +overwrite put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value5678] get result same as inserted, T diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic-reading-pcap/out b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic-reading-pcap/out index b32c104878..960761de7b 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic-reading-pcap/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic-reading-pcap/out @@ -1,4 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -put result, T -get result, [val=value5678, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value5678] get result same as inserted, T diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic/out b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic/out index 1eca70373d..c9184cdc98 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-basic/out @@ -1,6 +1,6 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -open successful -put result, T -get result, [val=value5678, error=] +open result, [code=Storage::SUCCESS, error_str=, value=] +put result, [code=Storage::SUCCESS, error_str=, value=] +get result, [code=Storage::SUCCESS, error_str=, value=value5678] get result same as inserted, T closed succesfully diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/.stderr b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/.stderr index 1013a7f2f4..49d861c74c 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/.stderr +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/.stderr @@ -1,3 +1 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/sync.zeek, line 75: Failed to open backend Storage::SQLITE: SQLite call failed: unable to open database file (Storage::Sync::__open_backend(Storage::Sync::btype, to_any_coerce Storage::Sync::options, Storage::Sync::key_type, Storage::Sync::val_type)) -error in <...>/sync.zeek, line 85: Failed to store data: type of key passed (count) does not match backend's key type (str) (Storage::Sync::__put(Storage::Sync::backend, Storage::Sync::args$key, Storage::Sync::args$value, Storage::Sync::args$overwrite, Storage::Sync::args$expire_time)) diff --git a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/out b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/out index c3ff9662ac..0f0dfe41cb 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/out +++ b/testing/btest/Baseline/scripts.base.frameworks.storage.sqlite-error-handling/out @@ -1,2 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -Put result on closed handle: 0 +Open result, [code=Storage::OPERATION_FAILED, error_str=Failed to open backend Storage::SQLITE: SQLite call failed: unable to open database file, value=] +Open result 2, [code=Storage::SUCCESS, error_str=, value=] +Put result with bad key type, [code=Storage::KEY_TYPE_MISMATCH, error_str=, value=] +Put result on closed handle, [code=Storage::NOT_CONNECTED, error_str=Backend is closed, value=] diff --git a/testing/btest/plugins/storage-plugin/src/StorageDummy.cc b/testing/btest/plugins/storage-plugin/src/StorageDummy.cc index 8007131f3d..03e4360553 100644 --- a/testing/btest/plugins/storage-plugin/src/StorageDummy.cc +++ b/testing/btest/plugins/storage-plugin/src/StorageDummy.cc @@ -4,12 +4,14 @@ #include "zeek/Func.h" #include "zeek/Val.h" +#include "zeek/storage/ReturnCode.h" + +using namespace zeek; +using namespace zeek::storage; namespace btest::storage::backend { -zeek::storage::BackendPtr StorageDummy::Instantiate(std::string_view tag) { - return zeek::make_intrusive(tag); -} +BackendPtr StorageDummy::Instantiate(std::string_view tag) { return make_intrusive(tag); } /** * Called by the manager system to open the backend. @@ -18,65 +20,65 @@ zeek::storage::BackendPtr StorageDummy::Instantiate(std::string_view tag) { * implementation must call \a Opened(); if not, it must call Error() * with a corresponding message. */ -zeek::storage::ErrorResult StorageDummy::DoOpen(zeek::RecordValPtr options, zeek::storage::OpenResultCallback* cb) { - zeek::RecordValPtr backend_options = options->GetField("dummy"); - bool open_fail = backend_options->GetField("open_fail")->Get(); +OperationResult StorageDummy::DoOpen(RecordValPtr options, OpenResultCallback* cb) { + RecordValPtr backend_options = options->GetField("dummy"); + bool open_fail = backend_options->GetField("open_fail")->Get(); if ( open_fail ) - return "open_fail was set to true, returning error"; + return {ReturnCode::OPERATION_FAILED, "open_fail was set to true, returning error"}; open = true; - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * Finalizes the backend when it's being closed. */ -zeek::storage::ErrorResult StorageDummy::DoClose(zeek::storage::ErrorResultCallback* cb) { +OperationResult StorageDummy::DoClose(OperationResultCallback* cb) { open = false; - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * The workhorse method for Put(). This must be implemented by plugins. */ -zeek::storage::ErrorResult StorageDummy::DoPut(zeek::ValPtr key, zeek::ValPtr value, bool overwrite, - double expiration_time, zeek::storage::ErrorResultCallback* cb) { +OperationResult StorageDummy::DoPut(ValPtr key, ValPtr value, bool overwrite, double expiration_time, + OperationResultCallback* cb) { auto json_key = key->ToJSON()->ToStdString(); auto json_value = value->ToJSON()->ToStdString(); data[json_key] = json_value; - return std::nullopt; + return {ReturnCode::SUCCESS}; } /** * The workhorse method for Get(). This must be implemented for plugins. */ -zeek::storage::ValResult StorageDummy::DoGet(zeek::ValPtr key, zeek::storage::ValResultCallback* cb) { +OperationResult StorageDummy::DoGet(ValPtr key, OperationResultCallback* cb) { auto json_key = key->ToJSON(); auto it = data.find(json_key->ToStdString()); if ( it == data.end() ) - return zeek::unexpected("Failed to find key"); + return {ReturnCode::KEY_NOT_FOUND}; - auto val = zeek::detail::ValFromJSON(it->second.c_str(), val_type, zeek::Func::nil); - if ( std::holds_alternative(val) ) { - zeek::ValPtr val_v = std::get(val); - return val_v; + auto val = zeek::detail::ValFromJSON(it->second.c_str(), val_type, Func::nil); + if ( std::holds_alternative(val) ) { + ValPtr val_v = std::get(val); + return {ReturnCode::SUCCESS, "", val_v}; } - return zeek::unexpected(std::get(val)); + return {ReturnCode::OPERATION_FAILED, std::get(val)}; } /** * The workhorse method for Erase(). This must be implemented for plugins. */ -zeek::storage::ErrorResult StorageDummy::DoErase(zeek::ValPtr key, zeek::storage::ErrorResultCallback* cb) { +OperationResult StorageDummy::DoErase(ValPtr key, OperationResultCallback* cb) { auto json_key = key->ToJSON(); auto it = data.find(json_key->ToStdString()); if ( it == data.end() ) - return "Failed to find key"; + return {ReturnCode::KEY_NOT_FOUND}; data.erase(it); - return std::nullopt; + return {ReturnCode::SUCCESS}; } } // namespace btest::storage::backend diff --git a/testing/btest/plugins/storage-plugin/src/StorageDummy.h b/testing/btest/plugins/storage-plugin/src/StorageDummy.h index 4c8a2dfc29..fbd0abc032 100644 --- a/testing/btest/plugins/storage-plugin/src/StorageDummy.h +++ b/testing/btest/plugins/storage-plugin/src/StorageDummy.h @@ -21,13 +21,13 @@ public: /** * Called by the manager system to open the backend. */ - zeek::storage::ErrorResult DoOpen(zeek::RecordValPtr options, - zeek::storage::OpenResultCallback* cb = nullptr) override; + zeek::storage::OperationResult DoOpen(zeek::RecordValPtr options, + zeek::storage::OpenResultCallback* cb = nullptr) override; /** * Finalizes the backend when it's being closed. */ - zeek::storage::ErrorResult DoClose(zeek::storage::ErrorResultCallback* cb = nullptr) override; + zeek::storage::OperationResult DoClose(zeek::storage::OperationResultCallback* cb = nullptr) override; /** * Returns whether the backend is opened. @@ -37,19 +37,21 @@ public: /** * The workhorse method for Put(). */ - zeek::storage::ErrorResult DoPut(zeek::ValPtr key, zeek::ValPtr value, bool overwrite = true, - double expiration_time = 0, - zeek::storage::ErrorResultCallback* cb = nullptr) override; + zeek::storage::OperationResult DoPut(zeek::ValPtr key, zeek::ValPtr value, bool overwrite = true, + double expiration_time = 0, + zeek::storage::OperationResultCallback* cb = nullptr) override; /** * The workhorse method for Get(). */ - zeek::storage::ValResult DoGet(zeek::ValPtr key, zeek::storage::ValResultCallback* cb = nullptr) override; + zeek::storage::OperationResult DoGet(zeek::ValPtr key, + zeek::storage::OperationResultCallback* cb = nullptr) override; /** * The workhorse method for Erase(). */ - zeek::storage::ErrorResult DoErase(zeek::ValPtr key, zeek::storage::ErrorResultCallback* cb = nullptr) override; + zeek::storage::OperationResult DoErase(zeek::ValPtr key, + zeek::storage::OperationResultCallback* cb = nullptr) override; private: std::map data; diff --git a/testing/btest/plugins/storage.zeek b/testing/btest/plugins/storage.zeek index fe876ff31a..40948d5ba2 100644 --- a/testing/btest/plugins/storage.zeek +++ b/testing/btest/plugins/storage.zeek @@ -30,10 +30,12 @@ event zeek_init() { # Test basic operation. The second get() should return an error # as the key should have been erased. - local b = Storage::Sync::open_backend(Storage::STORAGEDUMMY, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::STORAGEDUMMY, opts, str, str); + print "open result", open_res; + local b = open_res$value; local put_res = Storage::Sync::put(b, [$key=key, $value=value, $overwrite=F]); local get_res = Storage::Sync::get(b, key); - if ( get_res is bool ) { + if ( get_res$code != Storage::SUCCESS ) { print("Got an invalid value in response!"); } @@ -41,19 +43,21 @@ event zeek_init() { get_res = Storage::Sync::get(b, key); Storage::Sync::close_backend(b); - if ( get_res?$error ) - Reporter::error(get_res$error); + if ( get_res$code != Storage::SUCCESS && get_res?$error_str ) + Reporter::error(get_res$error_str); # Test attempting to use the closed handle. put_res = Storage::Sync::put(b, [$key="a", $value="b", $overwrite=F]); get_res = Storage::Sync::get(b, "a"); erase_res = Storage::Sync::erase(b, "a"); - print(fmt("results of trying to use closed handle: get: %d, put: %d, erase: %d", - get_res?$val, put_res, erase_res)); + print(fmt("results of trying to use closed handle: get: %s, put: %s, erase: %s", + get_res$code, put_res$code, erase_res$code)); # Test failing to open the handle and test closing an invalid handle. opts$dummy$open_fail = T; - local b2 = Storage::Sync::open_backend(Storage::STORAGEDUMMY, opts, str, str); - Storage::Sync::close_backend(b2); + open_res = Storage::Sync::open_backend(Storage::STORAGEDUMMY, opts, str, str); + print "open result 2", open_res; + local close_res = Storage::Sync::close_backend(open_res$value); + print "close result of closed handle", close_res; } diff --git a/testing/btest/scripts/base/frameworks/storage/compound-types.zeek b/testing/btest/scripts/base/frameworks/storage/compound-types.zeek index ba68c4e006..419a1f3fc0 100644 --- a/testing/btest/scripts/base/frameworks/storage/compound-types.zeek +++ b/testing/btest/scripts/base/frameworks/storage/compound-types.zeek @@ -63,7 +63,9 @@ event zeek_init() { value[2] = "b"; value[3] = "c"; - local b = Storage::Sync::open_backend(Storage::SQLITE, opts, Rec, tbl); + local open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, Rec, tbl); + print "open result", open_res; + local b = open_res$value; local res = Storage::Sync::put(b, [$key=key, $value=value]); print "put result", res; diff --git a/testing/btest/scripts/base/frameworks/storage/erase.zeek b/testing/btest/scripts/base/frameworks/storage/erase.zeek index 585da798b2..7380695b91 100644 --- a/testing/btest/scripts/base/frameworks/storage/erase.zeek +++ b/testing/btest/scripts/base/frameworks/storage/erase.zeek @@ -10,23 +10,26 @@ # Create a typename here that can be passed down into get(). type str: string; -event zeek_init() { +event zeek_init() + { # Create a database file in the .tmp directory with a 'testing' table - local opts : Storage::BackendOptions; - opts$sqlite = [$database_path = "storage-test.sqlite", $table_name = "testing"]; + local opts: Storage::BackendOptions; + opts$sqlite = [ $database_path="storage-test.sqlite", $table_name="testing" ]; local key = "key1234"; # Test inserting/retrieving a key/value pair that we know won't be in # the backend yet. - local b = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + print "open result", open_res; + local b = open_res$value; local res = Storage::Sync::erase(b, key); print "erase result", res; local res2 = Storage::Sync::get(b, key); - if ( res2?$error ) - print "get result", res2$error; + if ( res2$code != Storage::SUCCESS ) + print "get result", res2; Storage::Sync::close_backend(b); -} + } diff --git a/testing/btest/scripts/base/frameworks/storage/expiration.zeek b/testing/btest/scripts/base/frameworks/storage/expiration.zeek index d53e23fa36..fc0efd6819 100644 --- a/testing/btest/scripts/base/frameworks/storage/expiration.zeek +++ b/testing/btest/scripts/base/frameworks/storage/expiration.zeek @@ -20,8 +20,8 @@ event check_removed() { # This should return an error from the sqlite backend that there aren't any more # rows available. local res2 = Storage::Sync::get(backend, key); - if ( res2?$error ) - print "get result", res2$error; + if ( res2$code != Storage::SUCCESS ) + print "get result", res2; Storage::Sync::close_backend(backend); terminate(); @@ -31,15 +31,17 @@ event setup_test() { local opts : Storage::BackendOptions; opts$sqlite = [$database_path = "storage-test.sqlite", $table_name = "testing"]; - backend = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + print "open result", open_res; + backend = open_res$value; local res = Storage::Sync::put(backend, [$key=key, $value=value, $expire_time=2 secs]); print "put result", res; local res2 = Storage::Sync::get(backend, key); print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == (res2$val as string); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value == (res2$value as string); schedule 5 secs { check_removed() }; } diff --git a/testing/btest/scripts/base/frameworks/storage/overwriting.zeek b/testing/btest/scripts/base/frameworks/storage/overwriting.zeek index a31360802f..4a2f7be53b 100644 --- a/testing/btest/scripts/base/frameworks/storage/overwriting.zeek +++ b/testing/btest/scripts/base/frameworks/storage/overwriting.zeek @@ -17,15 +17,17 @@ event zeek_init() { local key = "key1234"; local value = "value7890"; - local b = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + print "open result", open_res; + local b = open_res$value; local res = Storage::Sync::put(b, [$key=key, $value=value]); print "put result", res; local res2 = Storage::Sync::get(b, key); print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == (res2$val as string); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value == (res2$value as string); Storage::Sync::close_backend(b); } diff --git a/testing/btest/scripts/base/frameworks/storage/redis-async-reading-pcap.zeek b/testing/btest/scripts/base/frameworks/storage/redis-async-reading-pcap.zeek index 2fe19ddb37..fba38d3e59 100644 --- a/testing/btest/scripts/base/frameworks/storage/redis-async-reading-pcap.zeek +++ b/testing/btest/scripts/base/frameworks/storage/redis-async-reading-pcap.zeek @@ -28,7 +28,9 @@ event zeek_init() local key = "key1234"; local value = "value5678"; - local b = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + print "open result", open_res; + local b = open_res$value; when [b, key, value] ( local res = Storage::Async::put(b, [ $key=key, $value=value ]) ) @@ -38,8 +40,8 @@ event zeek_init() when [b, key, value] ( local res2 = Storage::Async::get(b, key) ) { print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == ( res2$val as string ); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value == ( res2$value as string ); Storage::Sync::close_backend(b); } diff --git a/testing/btest/scripts/base/frameworks/storage/redis-async.zeek b/testing/btest/scripts/base/frameworks/storage/redis-async.zeek index 225da69719..7923aa6f46 100644 --- a/testing/btest/scripts/base/frameworks/storage/redis-async.zeek +++ b/testing/btest/scripts/base/frameworks/storage/redis-async.zeek @@ -30,32 +30,49 @@ event zeek_init() local key = "key1234"; local value = "value5678"; - local b = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); - - when [b, key, value] ( local res = Storage::Async::put(b, [ $key=key, - $value=value ]) ) + when [opts, key, value] ( local open_res = Storage::Async::open_backend( + Storage::REDIS, opts, str, str) ) { - print "put result", res; + print "open result", open_res; + local b = open_res$value; - when [b, key, value] ( local res2 = Storage::Async::get(b, key) ) + when [b, key, value] ( local put_res = Storage::Async::put(b, [ $key=key, + $value=value ]) ) { - print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == ( res2$val as string ); + print "put result", put_res; - Storage::Sync::close_backend(b); + when [b, key, value] ( local get_res = Storage::Async::get(b, key) ) + { + print "get result", get_res; + if ( get_res$code == Storage::SUCCESS && get_res?$value ) + print "get result same as inserted", value == ( get_res$value as string ); - terminate(); + when [b] ( local close_res = Storage::Async::close_backend(b) ) + { + print "close result", close_res; + terminate(); + } + timeout 5sec + { + print "close request timed out"; + terminate(); + } + } + timeout 5sec + { + print "get request timed out"; + terminate(); + } } timeout 5sec { - print "get request timed out"; + print "put request timed out"; terminate(); } } timeout 5sec { - print "put request timed out"; + print "open request timed out"; terminate(); } } diff --git a/testing/btest/scripts/base/frameworks/storage/redis-cluster.zeek b/testing/btest/scripts/base/frameworks/storage/redis-cluster.zeek index 8d6ede1d90..7c4996f7f7 100644 --- a/testing/btest/scripts/base/frameworks/storage/redis-cluster.zeek +++ b/testing/btest/scripts/base/frameworks/storage/redis-cluster.zeek @@ -45,7 +45,8 @@ event zeek_init() opts$redis = [ $server_host="127.0.0.1", $server_port=to_port(getenv( "REDIS_PORT")), $key_prefix="testing" ]; - backend = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + backend = open_res$value; } event redis_data_written() diff --git a/testing/btest/scripts/base/frameworks/storage/redis-expiration.zeek b/testing/btest/scripts/base/frameworks/storage/redis-expiration.zeek index fcc6d03439..59ff5b540c 100644 --- a/testing/btest/scripts/base/frameworks/storage/redis-expiration.zeek +++ b/testing/btest/scripts/base/frameworks/storage/redis-expiration.zeek @@ -40,15 +40,18 @@ event setup_test() opts$redis = [ $server_host="127.0.0.1", $server_port=to_port(getenv( "REDIS_PORT")), $key_prefix="testing" ]; - b = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + print "open result", open_res; + + b = open_res$value; local res = Storage::Sync::put(b, [ $key=key, $value=value, $expire_time=2secs ]); print "put result", res; local res2 = Storage::Sync::get(b, key); print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == ( res2$val as string ); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value == ( res2$value as string ); schedule 5secs { check_removed() }; } diff --git a/testing/btest/scripts/base/frameworks/storage/redis-sync.zeek b/testing/btest/scripts/base/frameworks/storage/redis-sync.zeek index bf49a30230..42f2b8d0f5 100644 --- a/testing/btest/scripts/base/frameworks/storage/redis-sync.zeek +++ b/testing/btest/scripts/base/frameworks/storage/redis-sync.zeek @@ -27,15 +27,18 @@ event zeek_init() local key = "key1234"; local value = "value1234"; - local b = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::REDIS, opts, str, str); + print "open_result", open_res; + + local b = open_res$value; local res = Storage::Sync::put(b, [ $key=key, $value=value ]); print "put result", res; local res2 = Storage::Sync::get(b, key); print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == ( res2$val as string ); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value == ( res2$value as string ); local value2 = "value5678"; res = Storage::Sync::put(b, [ $key=key, $value=value2, $overwrite=T ]); @@ -43,8 +46,8 @@ event zeek_init() res2 = Storage::Sync::get(b, key); print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value2 == ( res2$val as string ); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value2 == ( res2$value as string ); Storage::Sync::close_backend(b); } diff --git a/testing/btest/scripts/base/frameworks/storage/sqlite-basic-reading-pcap.zeek b/testing/btest/scripts/base/frameworks/storage/sqlite-basic-reading-pcap.zeek index 8a933de3c6..b5c0ec8adf 100644 --- a/testing/btest/scripts/base/frameworks/storage/sqlite-basic-reading-pcap.zeek +++ b/testing/btest/scripts/base/frameworks/storage/sqlite-basic-reading-pcap.zeek @@ -23,7 +23,10 @@ event zeek_init() # Test inserting/retrieving a key/value pair that we know won't be in # the backend yet. - local b = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + print "open result", open_res; + + local b = open_res$value; when [b, key, value] ( local res = Storage::Async::put(b, [ $key=key, $value=value ]) ) @@ -33,8 +36,8 @@ event zeek_init() when [b, key, value] ( local res2 = Storage::Async::get(b, key) ) { print "get result", res2; - if ( res2?$val ) - print "get result same as inserted", value == ( res2$val as string ); + if ( res2$code == Storage::SUCCESS && res2?$value ) + print "get result same as inserted", value == ( res2$value as string ); Storage::Sync::close_backend(b); diff --git a/testing/btest/scripts/base/frameworks/storage/sqlite-basic.zeek b/testing/btest/scripts/base/frameworks/storage/sqlite-basic.zeek index c815996f7c..92f2a34059 100644 --- a/testing/btest/scripts/base/frameworks/storage/sqlite-basic.zeek +++ b/testing/btest/scripts/base/frameworks/storage/sqlite-basic.zeek @@ -22,10 +22,11 @@ event zeek_init() # Test inserting/retrieving a key/value pair that we know won't be in # the backend yet. - when [opts, key, value] ( local b = Storage::Async::open_backend( + when [opts, key, value] ( local open_res = Storage::Async::open_backend( Storage::SQLITE, opts, str, str) ) { - print "open successful"; + print "open result", open_res; + local b = open_res$value; when [b, key, value] ( local put_res = Storage::Async::put(b, [ $key=key, $value=value ]) ) @@ -35,8 +36,8 @@ event zeek_init() when [b, key, value] ( local get_res = Storage::Async::get(b, key) ) { print "get result", get_res; - if ( get_res?$val ) - print "get result same as inserted", value == ( get_res$val as string ); + if ( get_res$code == Storage::SUCCESS && get_res?$value ) + print "get result same as inserted", value == ( get_res$value as string ); when [b] ( local close_res = Storage::Async::close_backend(b) ) { diff --git a/testing/btest/scripts/base/frameworks/storage/sqlite-error-handling.zeek b/testing/btest/scripts/base/frameworks/storage/sqlite-error-handling.zeek index 899ba497aa..24c1ed40d0 100644 --- a/testing/btest/scripts/base/frameworks/storage/sqlite-error-handling.zeek +++ b/testing/btest/scripts/base/frameworks/storage/sqlite-error-handling.zeek @@ -17,18 +17,23 @@ event zeek_init() { $table_name = "testing"]; # This should report an error in .stderr and reporter.log - local b = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + local open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + print "Open result", open_res; # Open a valid database file opts$sqlite$database_path = "test.sqlite"; - b = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + open_res = Storage::Sync::open_backend(Storage::SQLITE, opts, str, str); + print "Open result 2", open_res; + + local b = open_res$value; local bad_key: count = 12345; local value = "abcde"; - Storage::Sync::put(b, [$key=bad_key, $value=value]); + local res = Storage::Sync::put(b, [$key=bad_key, $value=value]); + print "Put result with bad key type", res; # Close the backend and then attempt to use the closed handle Storage::Sync::close_backend(b); - local res = Storage::Sync::put(b, [$key="a", $value="b"]); - print fmt("Put result on closed handle: %d", res); + local res2 = Storage::Sync::put(b, [$key="a", $value="b"]); + print "Put result on closed handle", res2; }