Handle scenarios when store get fails or doesn't find data when it should have.

TODO.md

@@ -88,9 +88,12 @@
 - ~~Add docs/explanation around ranking calc~~
 - ~~UTF-8 normalization~~
 - ~~Use rocksdb batch put for atomic insertion~~
+- ~~Proper logging~~
+- ~~Handle store-get() not finding a key~~
+- Deprecate union type punning
+- Deprecate converting integer to string verbatim 
 - NOT operator support
 - > INT32_MAX validation for float field
-- Proper logging
 - highlight of string arrays?
 - test for token ranking on float field
 - test for float int field deletion during doc deletion
@@ -99,7 +102,6 @@
 - Query token ids should match query token ordering
 - ID should not have "/"
 - Group results by field
-- Handle store-get() not finding a key
 - Delete using range: https://github.com/facebook/rocksdb/wiki/Delete-A-Range-Of-Keys
 - Test for string utils
 - Prevent string copy during indexing

include/collection.h

@@ -80,7 +80,7 @@ public:
 
     static uint32_t deserialize_seq_id_key(std::string serialized_seq_id);
 
-    uint32_t doc_id_to_seq_id(std::string doc_id);
+    Option<uint32_t> doc_id_to_seq_id(std::string doc_id);
 
     std::vector<std::string> get_facet_fields();
 

include/store.h

@@ -32,7 +32,6 @@ public:
 
 enum StoreStatus {
     FOUND,
-    OK,
     NOT_FOUND,
     ERROR
 };
@@ -97,15 +96,16 @@ public:
     StoreStatus get(const std::string& key, std::string& value) const {
         rocksdb::Status status = db->Get(rocksdb::ReadOptions(), key, &value);
 
+        if(status.ok()) {
+            return StoreStatus::FOUND;
+        }
+
         if(status.IsNotFound()) {
             return StoreStatus::NOT_FOUND;
         }
 
-        if(!status.ok()) {
-            return StoreStatus::ERROR;
-        }
-
-        return StoreStatus::FOUND;
+        LOG(WARNING) << "Error while fetching the key: " << key << " - status is: " << status.ToString();
+        return StoreStatus::ERROR;
     }
 
     bool remove(const std::string& key) {

src/collection.cpp

@@ -514,14 +514,19 @@ Option<nlohmann::json> Collection::search(std::string query, const std::vector<s
         const auto & field_order_kv = field_order_kvs[field_order_kv_index];
         const std::string& seq_id_key = get_seq_id_key((uint32_t) field_order_kv.second.key);
 
-        std::string value;
-        store->get(seq_id_key, value);
+        std::string json_doc_str;
+        StoreStatus json_doc_status = store->get(seq_id_key, json_doc_str);
+
+        if(json_doc_status != StoreStatus::FOUND) {
+            LOG(WARNING) << "Could not locate the JSON document for sequence ID: " << seq_id_key;
+            continue;
+        }
 
         nlohmann::json wrapper_doc;
         nlohmann::json document;
 
         try {
-            document = nlohmann::json::parse(value);
+            document = nlohmann::json::parse(json_doc_str);
         } catch(...) {
             return Option<nlohmann::json>(500, "Error while parsing stored document.");
         }
@@ -639,16 +644,29 @@ Option<nlohmann::json> Collection::search(std::string query, const std::vector<s
 
 Option<nlohmann::json> Collection::get(const std::string & id) {
     std::string seq_id_str;
-    StoreStatus status = store->get(get_doc_id_key(id), seq_id_str);
+    StoreStatus seq_id_status = store->get(get_doc_id_key(id), seq_id_str);
 
-    if(status == StoreStatus::NOT_FOUND) {
+    if(seq_id_status == StoreStatus::NOT_FOUND) {
         return Option<nlohmann::json>(404, "Could not find a document with id: " + id);
     }
 
+    if(seq_id_status == StoreStatus::ERROR) {
+        return Option<nlohmann::json>(500, "Error while fetching the document.");
+    }
+
     uint32_t seq_id = (uint32_t) std::stol(seq_id_str);
 
     std::string parsed_document;
-    store->get(get_seq_id_key(seq_id), parsed_document);
+    StoreStatus doc_status = store->get(get_seq_id_key(seq_id), parsed_document);
+
+    if(doc_status == StoreStatus::NOT_FOUND) {
+        LOG(WARNING) << "Sequence ID exists, but document is missing for id: " << id;
+        return Option<nlohmann::json>(404, "Could not find a document with id: " + id);
+    }
+
+    if(doc_status == StoreStatus::ERROR) {
+        return Option<nlohmann::json>(500, "Error while fetching the document.");
+    }
 
     nlohmann::json document;
     try {
@@ -662,16 +680,29 @@ Option<nlohmann::json> Collection::get(const std::string & id) {
 
 Option<std::string> Collection::remove(const std::string & id, const bool remove_from_store) {
     std::string seq_id_str;
-    StoreStatus status = store->get(get_doc_id_key(id), seq_id_str);
+    StoreStatus seq_id_status = store->get(get_doc_id_key(id), seq_id_str);
 
-    if(status == StoreStatus::NOT_FOUND) {
+    if(seq_id_status == StoreStatus::NOT_FOUND) {
         return Option<std::string>(404, "Could not find a document with id: " + id);
     }
 
+    if(seq_id_status == StoreStatus::ERROR) {
+        return Option<std::string>(500, "Error while fetching the document.");
+    }
+
     uint32_t seq_id = (uint32_t) std::stol(seq_id_str);
 
     std::string parsed_document;
-    store->get(get_seq_id_key(seq_id), parsed_document);
+    StoreStatus doc_status = store->get(get_seq_id_key(seq_id), parsed_document);
+
+    if(doc_status == StoreStatus::NOT_FOUND) {
+        LOG(WARNING) << "Sequence ID exists, but document is missing for id: " << id;
+        return Option<std::string>(404, "Could not find a document with id: " + id);
+    }
+
+    if(doc_status == StoreStatus::ERROR) {
+        return Option<std::string>(500, "Error while fetching the document.");
+    }
 
     nlohmann::json document;
     try {
@@ -731,11 +762,19 @@ uint32_t Collection::get_collection_id() {
     return collection_id;
 }
 
-uint32_t Collection::doc_id_to_seq_id(std::string doc_id) {
+Option<uint32_t> Collection::doc_id_to_seq_id(std::string doc_id) {
     std::string seq_id_str;
-    store->get(get_doc_id_key(doc_id), seq_id_str);
-    uint32_t seq_id = (uint32_t) std::stoi(seq_id_str);
-    return seq_id;
+    StoreStatus status = store->get(get_doc_id_key(doc_id), seq_id_str);
+    if(status == StoreStatus::FOUND) {
+        uint32_t seq_id = (uint32_t) std::stoi(seq_id_str);
+        return Option<uint32_t>(seq_id);
+    }
+
+    if(status == StoreStatus::NOT_FOUND) {
+        return Option<uint32_t>(404, "Not found.");
+    }
+
+    return Option<uint32_t>(500, "Error while fetching doc_id from store.");
 }
 
 std::vector<std::string> Collection::get_facet_fields() {

src/collection_manager.cpp

@@ -43,8 +43,13 @@ Option<bool> CollectionManager::init(Store *store, const std::string & auth_key,
     this->search_only_auth_key = search_only_auth_key;
 
     std::string next_collection_id_str;
-    store->get(NEXT_COLLECTION_ID_KEY, next_collection_id_str);
-    if(!next_collection_id_str.empty()) {
+    StoreStatus next_coll_id_status = store->get(NEXT_COLLECTION_ID_KEY, next_collection_id_str);
+
+    if(next_coll_id_status == StoreStatus::ERROR) {
+        return Option<bool>(500, "Error while fetching the next collection id from the disk.");
+    }
+
+    if(next_coll_id_status == StoreStatus::FOUND) {
         next_collection_id = (uint32_t) stoi(next_collection_id_str);
     } else {
         next_collection_id = 0;
@@ -64,8 +69,20 @@ Option<bool> CollectionManager::init(Store *store, const std::string & auth_key,
 
         const std::string & this_collection_name = collection_meta[COLLECTION_NAME_KEY].get<std::string>();
         std::string collection_next_seq_id_str;
-        store->get(Collection::get_next_seq_id_key(this_collection_name), collection_next_seq_id_str);
-        uint32_t collection_next_seq_id = collection_next_seq_id_str.size() == 0 ? 0 :
+        StoreStatus next_seq_id_status = store->get(Collection::get_next_seq_id_key(this_collection_name),
+                                                    collection_next_seq_id_str);
+
+        if(next_seq_id_status == StoreStatus::ERROR) {
+            return Option<bool>(500, "Error while fetching collection's next sequence ID from the disk for collection "
+                                     "`" + this_collection_name + "`");
+        }
+
+        if(next_seq_id_status == StoreStatus::NOT_FOUND && next_coll_id_status == StoreStatus::FOUND) {
+            return Option<bool>(500, "Next collection id was found, but collection's next sequence ID is missing for "
+                                     "`" + this_collection_name + "`");
+        }
+
+        uint32_t collection_next_seq_id = next_seq_id_status == StoreStatus::NOT_FOUND ? 0 :
                                           (const uint32_t) std::stoi(collection_next_seq_id_str);
 
         Collection* collection = init_collection(collection_meta, collection_next_seq_id);
@@ -83,13 +100,19 @@ Option<bool> CollectionManager::init(Store *store, const std::string & auth_key,
                 document = nlohmann::json::parse(doc_json_str);
             } catch(...) {
                 delete iter;
-                return Option<bool>(500,
-                       std::string("Error while parsing stored document from collection " + collection->get_name() +
-                                   " with key: ") + iter->key().ToString());
+                return Option<bool>(500, std::string("Error while parsing stored document from collection " +
+                                                     collection->get_name() + " with key: ") + iter->key().ToString());
             }
 
-            uint32_t seq_id = collection->doc_id_to_seq_id(document["id"]);
-            collection->index_in_memory(document, seq_id);
+            Option<uint32_t> seq_id_op = collection->doc_id_to_seq_id(document["id"]);
+            if(!seq_id_op.ok()) {
+               delete iter;
+               return Option<bool>(500, std::string("Error while fetching sequence id of document id " +
+                                        document["id"].get<std::string>() + " in collection `" +
+                                        collection->get_name() + "`"));
+            }
+
+            collection->index_in_memory(document, seq_id_op.get());
             iter->Next();
         }