Skip to content

feat(kvdb): add findEntry returning optional instead of throwing on missing key #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kisielewski wants to merge 1 commit into
base: feat/search-api
Choose a base branch
from
Open

feat(kvdb): add findEntry returning optional instead of throwing on missing key #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions endpoint/kvdb/include/privmx/endpoint/kvdb/KvdbApiImpl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class KvdbApiImpl : public privmx::utils::ManualManagedClass<KvdbApiImpl>, prote
);

KvdbEntry getEntry(const std::string& kvdbId, const std::string& key);
std::optional<KvdbEntry> findEntry(const std::string& kvdbId, const std::string& key);
bool hasEntry(const std::string& kvdbId, const std::string& key);
core::PagingList<std::string> listEntriesKeys(const std::string& kvdbId, const core::PagingQuery& pagingQuery);
core::PagingList<KvdbEntry> listEntries(const std::string& kvdbId, const core::PagingQuery& pagingQuery);
Expand Down
1 change: 1 addition & 0 deletions endpoint/kvdb/include/privmx/endpoint/kvdb/ServerApi.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class ServerApi {
server::KvdbGetResult kvdbGet(server::KvdbGetModel model);
server::KvdbListResult kvdbList(server::KvdbListModel model);
server::KvdbEntryGetResult kvdbEntryGet(server::KvdbEntryGetModel model);
server::KvdbEntryFindResult kvdbEntryFind(server::KvdbEntryGetModel model);
void kvdbEntrySet(server::KvdbEntrySetModel model);
void kvdbEntryDelete(server::KvdbEntryDeleteModel model);
server::KvdbListKeysResult kvdbListKeys(server::KvdbListKeysModel model);
Expand Down
3 changes: 3 additions & 0 deletions endpoint/kvdb/include/privmx/endpoint/kvdb/ServerTypes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ JSON_STRUCT(KvdbEntryGetModel, KVDB_ENTRY_GET_MODEL_FIELDS);
#define KVDB_ENTRY_GET_RESULT_FIELDS(F) F(kvdbEntry, KvdbEntryInfo)
JSON_STRUCT(KvdbEntryGetResult, KVDB_ENTRY_GET_RESULT_FIELDS);

#define KVDB_ENTRY_FIND_RESULT_FIELDS(F) F(kvdbEntry, std::optional<KvdbEntryInfo>)
JSON_STRUCT(KvdbEntryFindResult, KVDB_ENTRY_FIND_RESULT_FIELDS);

#define KVDB_ENTRY_SET_MODEL_FIELDS(F) \
F(kvdbId, std::string) \
F(kvdbEntryKey, std::string) \
Expand Down
2 changes: 2 additions & 0 deletions endpoint/kvdb/include/privmx/endpoint/kvdb/varinterface/KvdbApiVarInterface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class KvdbApiVarInterface {
UnsubscribeFrom = 18,
BuildSubscriptionQuery = 19,
BuildSubscriptionQueryForSelectedEntry = 20,
FindEntry = 21,
};

KvdbApiVarInterface(core::Connection connection, const core::VarSerializer& serializer)
Expand All @@ -68,6 +69,7 @@ class KvdbApiVarInterface {
Poco::Dynamic::Var unsubscribeFrom(const Poco::Dynamic::Var& args);
Poco::Dynamic::Var buildSubscriptionQuery(const Poco::Dynamic::Var& args);
Poco::Dynamic::Var buildSubscriptionQueryForSelectedEntry(const Poco::Dynamic::Var& args);
Poco::Dynamic::Var findEntry(const Poco::Dynamic::Var& args);

Poco::Dynamic::Var exec(METHOD method, const Poco::Dynamic::Var& args);

Expand Down
9 changes: 9 additions & 0 deletions endpoint/kvdb/include_pub/privmx/endpoint/kvdb/KvdbApi.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,15 @@ class KvdbApi : public privmx::endpoint::core::ExtendedPointer<KvdbApiImpl> {
*/
KvdbEntry getEntry(const std::string& kvdbId, const std::string& key);

/**
* Finds a KVDB entry by given KVDB entry key and KVDB ID, returning nullopt if not found.
*
* @param kvdbId KVDB ID of the KVDB entry to find
* @param key key of the KVDB entry to find
* @return optional struct containing the KVDB entry, or nullopt if not found
*/
std::optional<KvdbEntry> findEntry(const std::string& kvdbId, const std::string& key);

/**
* Check whether the KVDB entry exists.
*
Expand Down
11 changes: 11 additions & 0 deletions endpoint/kvdb/src/KvdbApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ KvdbEntry KvdbApi::getEntry(const std::string& kvdbId, const std::string& key) {
}
}

std::optional<KvdbEntry> KvdbApi::findEntry(const std::string& kvdbId, const std::string& key) {
auto impl = getImpl();
core::Validator::validateId(kvdbId, "field:kvdbId ");
try {
return impl->findEntry(kvdbId, key);
} catch (const privmx::utils::PrivmxException& e) {
core::ExceptionConverter::rethrowAsCoreException(e);
throw core::Exception("ExceptionConverter rethrow error");
}
}

bool KvdbApi::hasEntry(const std::string& kvdbId, const std::string& key) {
auto impl = getImpl();
core::Validator::validateId(kvdbId, "field:kvdbId ");
Expand Down
12 changes: 12 additions & 0 deletions endpoint/kvdb/src/KvdbApiImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ KvdbEntry KvdbApiImpl::getEntry(const std::string& kvdbId, const std::string& ke
return result;
}

std::optional<KvdbEntry> KvdbApiImpl::findEntry(const std::string& kvdbId, const std::string& key) {
PRIVMX_DEBUG_TIME_START(PlatformKvdb, findEntry)
server::KvdbEntryGetModel model{.kvdbId = kvdbId, .kvdbEntryKey = key};
PRIVMX_DEBUG_TIME_CHECKPOINT(PlatformKvdb, findEntry, getting entry)
auto entryOpt = _serverApi.kvdbEntryFind(model).kvdbEntry;
PRIVMX_DEBUG_TIME_STOP(PlatformKvdb, findEntry, data received)
if (!entryOpt.has_value()) {
return std::nullopt;
}
return validateDecryptAndConvertEntryDataToEntry(entryOpt.value(), getEntryDecryptionKeys(entryOpt.value()));
}

bool KvdbApiImpl::hasEntry(const std::string& kvdbId, const std::string& key) {
try {
server::KvdbEntryGetModel model{.kvdbId = kvdbId, .kvdbEntryKey = key};
Expand Down
4 changes: 4 additions & 0 deletions endpoint/kvdb/src/ServerApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ server::KvdbEntryGetResult ServerApi::kvdbEntryGet(server::KvdbEntryGetModel mod
return request<server::KvdbEntryGetResult>("kvdbEntryGet", model.toJSON());
}

server::KvdbEntryFindResult ServerApi::kvdbEntryFind(server::KvdbEntryGetModel model) {
return request<server::KvdbEntryFindResult>("kvdbEntryFind", model.toJSON());
}

void ServerApi::kvdbEntrySet(server::KvdbEntrySetModel model) {
request("kvdbEntrySet", model.toJSON());
}
Expand Down
11 changes: 10 additions & 1 deletion endpoint/kvdb/src/varinterface/KvdbApiVarInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ std::map<KvdbApiVarInterface::METHOD, Poco::Dynamic::Var (KvdbApiVarInterface::*
{SubscribeFor, &KvdbApiVarInterface::subscribeFor},
{UnsubscribeFrom, &KvdbApiVarInterface::unsubscribeFrom},
{BuildSubscriptionQuery, &KvdbApiVarInterface::buildSubscriptionQuery},
{BuildSubscriptionQueryForSelectedEntry, &KvdbApiVarInterface::buildSubscriptionQueryForSelectedEntry}
{BuildSubscriptionQueryForSelectedEntry, &KvdbApiVarInterface::buildSubscriptionQueryForSelectedEntry},
{FindEntry, &KvdbApiVarInterface::findEntry},
};

Poco::Dynamic::Var KvdbApiVarInterface::create(const Poco::Dynamic::Var& args) {
Expand Down Expand Up @@ -189,6 +190,14 @@ Poco::Dynamic::Var KvdbApiVarInterface::buildSubscriptionQueryForSelectedEntry(c
return _serializer.serialize(result);
}

Poco::Dynamic::Var KvdbApiVarInterface::findEntry(const Poco::Dynamic::Var& args) {
auto argsArr = core::VarInterfaceUtil::validateAndExtractArray(args, 2);
auto kvdbId = _deserializer.deserialize<std::string>(argsArr->get(0), "kvdbId");
auto key = _deserializer.deserialize<std::string>(argsArr->get(1), "key");
auto result = _kvdbApi.findEntry(kvdbId, key);
return _serializer.serialize(result);
}

Poco::Dynamic::Var KvdbApiVarInterface::exec(METHOD method, const Poco::Dynamic::Var& args) {
auto it = methodMap.find(method);
if (it == methodMap.end()) {
Expand Down
23 changes: 10 additions & 13 deletions endpoint/search/src/PrivmxFS.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ std::shared_ptr<PrivmxFile> PrivmxFS::openFile(const std::string& path) {

bool PrivmxFS::access(const std::string& path) {
LOG_TRACE("PrivmxFS::access - ", path, " | kvdbId: ",_session->kvdbId)
return _session->kvdbApi.hasEntry(_session->kvdbId, path);
return _session->kvdbApi.findEntry(_session->kvdbId, path).has_value();
}

void PrivmxFS::deleteFile(const std::string& path) {
Expand All @@ -172,20 +172,17 @@ PrivmxFS::PrivmxFS(

std::string PrivmxFS::getFileId(const std::string& name) {
LOG_TRACE("PrivmxFS::getFileId - ", name, " | kvdbId: ",_session->kvdbId)
try {
privmx::endpoint::kvdb::KvdbEntry kvdbEntry = _session->kvdbApi.getEntry(_session->kvdbId, name);
if(kvdbEntry.statusCode != 0) {
throw MalformedInternalFileIdException();
auto entry = _session->kvdbApi.findEntry(_session->kvdbId, name);
if (entry.has_value()) {
if(entry->statusCode != 0) {
throw MalformedInternalFileIdException();
}
std::string fileId = kvdbEntry.data.stdString();
return fileId;
} catch (const privmx::endpoint::server::KvdbEntryDoesNotExistException& e) {
LOG_DEBUG("PrivmxFS::getFileId file not found, creating new file - ", name)
int64_t fh = _session->storeApi.createFile(_session->storeId, META, META, 0, true);
std::string fileId = _session->storeApi.closeFile(fh);
_session->kvdbApi.setEntry(_session->kvdbId, name, META, META, privmx::endpoint::core::Buffer::from(fileId));
return fileId;
return entry->data.stdString();
}
int64_t fh = _session->storeApi.createFile(_session->storeId, META, META, 0, true);

@djenczewski djenczewski Jun 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is a file creating in the getFileId method?

@kisielewski kisielewski Jun 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getFileId is a get-or-create operation — the equivalent of POSIX open(..., O_CREAT):

  1. It checks KVDB for an existing path → fileId mapping (PrivmxFS.cpp:139-144)
  2. If found — returns the existing ID
  3. If not — creates the file in the Store, registers it in KVDB, and returns the new ID (PrivmxFS.cpp:146-148)

Creation happens here because the PrivMX Store requires a file to exist before it can be opened — storeApi.openFile is not O_CREAT, it opens an already-existing resource. The initialization logic must therefore live at the single point that translates a path into an ID, which is getFileId.

The caller (openFile at line 114) doesn't know and doesn't need to know whether the file is new or pre-existing — it just receives a valid ID.

That said, I agree the name is misleading. getFileId implies a pure read. Better alternatives:

  • getOrCreateFileId
  • resolveFileId
  • ensureFileId

This is a classic verb-mismatch issue — the behaviour is correct, but the name doesn't reflect the get-or-create semantics.

std::string fileId = _session->storeApi.closeFile(fh);
_session->kvdbApi.setEntry(_session->kvdbId, name, META, META, privmx::endpoint::core::Buffer::from(fileId));
return fileId;
}

std::shared_ptr<PrivmxFile> PrivmxExtFS::openFile(const std::string& path) {
Expand Down