Use enum class for storage::spi::Result::ErrorType.

4 files changed, 56 insertions, 49 deletions

diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
index f77d7705a66..3ae13d6b21c 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
@@ -157,7 +157,7 @@ doIterate(PersistenceProvider& spi,
         Context context(defaultLoadType, Priority(0), Trace::TraceLevel(0));
         IterateResult result(spi.iterate(id, maxByteSize, context));
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
 
         chunks.push_back(Chunk{result.steal_entries()});
         if (result.isCompleted()
@@ -214,13 +214,13 @@ iterateBucket(PersistenceProvider& spi,
             versions,
             context);
 
-    EXPECT_EQ(Result::NONE, iter.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, iter.getErrorCode());
 
     while (true) {
         IterateResult result =
             spi.iterate(iter.getIteratorId(),
                          std::numeric_limits<int64_t>().max(), context);
-        if (result.getErrorCode() != Result::NONE) {
+        if (result.getErrorCode() != Result::ErrorType::NONE) {
             return std::vector<DocEntry::UP>();
         }
         auto list = result.steal_entries();
@@ -639,7 +639,7 @@ TEST_F(ConformanceTest, testPutNewDocumentVersion)
     GetResult gr = spi->get(bucket, document::AllFields(), doc1->getId(),
                             context);
 
-    EXPECT_EQ(Result::NONE, gr.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, gr.getErrorCode());
     EXPECT_EQ(Timestamp(4), gr.getTimestamp());
 
     if (!((*doc2)==gr.getDocument())) {
@@ -691,7 +691,7 @@ TEST_F(ConformanceTest, testPutOlderDocumentVersion)
     GetResult gr = spi->get(bucket, document::AllFields(), doc1->getId(),
                             context);
 
-    EXPECT_EQ(Result::NONE, gr.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, gr.getErrorCode());
     EXPECT_EQ(Timestamp(5), gr.getTimestamp());
     EXPECT_EQ(*doc1, gr.getDocument());
 }
@@ -822,7 +822,7 @@ TEST_F(ConformanceTest, testRemove)
                                 doc1->getId(),
                                 context);
 
-    EXPECT_EQ(Result::NONE, getResult.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, getResult.getErrorCode());
     EXPECT_EQ(Timestamp(0), getResult.getTimestamp());
     EXPECT_TRUE(!getResult.hasDocument());
 }
@@ -848,7 +848,7 @@ TEST_F(ConformanceTest, testRemoveMerge)
                                                 removeId,
                                                 context);
         spi->flush(bucket, context);
-        EXPECT_EQ(Result::NONE, removeResult.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, removeResult.getErrorCode());
         EXPECT_EQ(false, removeResult.wasFound());
     }
     {
@@ -876,7 +876,7 @@ TEST_F(ConformanceTest, testRemoveMerge)
                                                 removeId,
                                                 context);
         spi->flush(bucket, context);
-        EXPECT_EQ(Result::NONE, removeResult.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, removeResult.getErrorCode());
         EXPECT_EQ(false, removeResult.wasFound());
     }
     // Old entry may or may not be present, depending on the provider.
@@ -904,7 +904,7 @@ TEST_F(ConformanceTest, testRemoveMerge)
                                                 removeId,
                                                 context);
         spi->flush(bucket, context);
-        EXPECT_EQ(Result::NONE, removeResult.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, removeResult.getErrorCode());
         EXPECT_EQ(false, removeResult.wasFound());
     }
     {
@@ -957,7 +957,7 @@ TEST_F(ConformanceTest, testUpdate)
                                           context);
         spi->flush(bucket, context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(3), result.getExistingTimestamp());
     }
 
@@ -967,7 +967,7 @@ TEST_F(ConformanceTest, testUpdate)
                                     doc1->getId(),
                                     context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(4), result.getTimestamp());
         EXPECT_EQ(document::IntFieldValue(42),
                              static_cast<document::IntFieldValue&>(
@@ -983,7 +983,7 @@ TEST_F(ConformanceTest, testUpdate)
                                     doc1->getId(),
                                     context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(0), result.getTimestamp());
         EXPECT_TRUE(!result.hasDocument());
     }
@@ -993,13 +993,13 @@ TEST_F(ConformanceTest, testUpdate)
                                           context);
         spi->flush(bucket, context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(0), result.getExistingTimestamp());
     }
 
     {
         GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context);
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(0), result.getTimestamp());
         EXPECT_TRUE(!result.hasDocument());
     }
@@ -1010,13 +1010,13 @@ TEST_F(ConformanceTest, testUpdate)
         // but since CreateIfNonExistent is set it should be auto-created anyway.
         UpdateResult result = spi->update(bucket, Timestamp(7), update, context);
         spi->flush(bucket, context);
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(7), result.getExistingTimestamp());
     }
 
     {
         GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context);
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(7), result.getTimestamp());
         EXPECT_EQ(document::IntFieldValue(42),
                              reinterpret_cast<document::IntFieldValue&>(
@@ -1039,7 +1039,7 @@ TEST_F(ConformanceTest, testGet)
         GetResult result = spi->get(bucket, document::AllFields(),
                                     doc1->getId(), context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(0), result.getTimestamp());
     }
 
@@ -1060,7 +1060,7 @@ TEST_F(ConformanceTest, testGet)
         GetResult result = spi->get(bucket, document::AllFields(),
                                     doc1->getId(), context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(0), result.getTimestamp());
     }
 }
@@ -1077,7 +1077,7 @@ TEST_F(ConformanceTest, testIterateCreateIterator)
 
     spi::CreateIteratorResult result(
             createIterator(*spi, b, createSelection("")));
-    EXPECT_EQ(Result::NONE, result.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
     // Iterator ID 0 means invalid iterator, so cannot be returned
     // from a successful createIterator call.
     EXPECT_TRUE(result.getIteratorId() != IteratorId(0));
@@ -1096,7 +1096,7 @@ TEST_F(ConformanceTest, testIterateWithUnknownId)
 
     IteratorId unknownId(123);
     IterateResult result(spi->iterate(unknownId, 1024, context));
-    EXPECT_EQ(Result::PERMANENT_ERROR, result.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::PERMANENT_ERROR, result.getErrorCode());
 }
 
 TEST_F(ConformanceTest, testIterateDestroyIterator)
@@ -1111,7 +1111,7 @@ TEST_F(ConformanceTest, testIterateDestroyIterator)
     CreateIteratorResult iter(createIterator(*spi, b, createSelection("")));
     {
         IterateResult result(spi->iterate(iter.getIteratorId(), 1024, context));
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
     }
 
     {
@@ -1122,7 +1122,7 @@ TEST_F(ConformanceTest, testIterateDestroyIterator)
     // Iteration should now fail
     {
         IterateResult result(spi->iterate(iter.getIteratorId(), 1024, context));
-        EXPECT_EQ(Result::PERMANENT_ERROR, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::PERMANENT_ERROR, result.getErrorCode());
     }
     {
         Result destroyResult(
@@ -1422,7 +1422,7 @@ TEST_F(ConformanceTest, testIterationRequiringDocumentIdOnlyMatching)
 
     CreateIteratorResult iter(
             createIterator(*spi, b, sel, NEWEST_DOCUMENT_OR_REMOVE));
-    EXPECT_TRUE(iter.getErrorCode() == Result::NONE);
+    EXPECT_TRUE(iter.getErrorCode() == Result::ErrorType::NONE);
 
     std::vector<Chunk> chunks = doIterate(*spi, iter.getIteratorId(), 4096);
     std::vector<DocAndTimestamp> docs;
@@ -1444,14 +1444,14 @@ TEST_F(ConformanceTest, testIterateBadDocumentSelection)
     {
         CreateIteratorResult iter(
                 createIterator(*spi, b, createSelection("the muppet show")));
-        if (iter.getErrorCode() == Result::NONE) {
+        if (iter.getErrorCode() == Result::ErrorType::NONE) {
             IterateResult result(
                     spi->iterate(iter.getIteratorId(), 4096, context));
-            EXPECT_EQ(Result::NONE, result.getErrorCode());
+            EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
             EXPECT_EQ(size_t(0), result.getEntries().size());
             EXPECT_EQ(true, result.isCompleted());
         } else {
-            EXPECT_EQ(Result::PERMANENT_ERROR, iter.getErrorCode());
+            EXPECT_EQ(Result::ErrorType::PERMANENT_ERROR, iter.getErrorCode());
             EXPECT_EQ(IteratorId(0), iter.getIteratorId());
         }
     }
@@ -1461,14 +1461,14 @@ TEST_F(ConformanceTest, testIterateBadDocumentSelection)
                                b,
                                createSelection(
                                        "unknownddoctype.something=thatthing")));
-        if (iter.getErrorCode() == Result::NONE) {
+        if (iter.getErrorCode() == Result::ErrorType::NONE) {
             IterateResult result(spi->iterate(
                     iter.getIteratorId(), 4096, context));
-            EXPECT_EQ(Result::NONE, result.getErrorCode());
+            EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
             EXPECT_EQ(size_t(0), result.getEntries().size());
             EXPECT_EQ(true, result.isCompleted());
         } else {
-            EXPECT_EQ(Result::PERMANENT_ERROR, iter.getErrorCode());
+            EXPECT_EQ(Result::ErrorType::PERMANENT_ERROR, iter.getErrorCode());
             EXPECT_EQ(IteratorId(0), iter.getIteratorId());
         }
     }
@@ -1491,7 +1491,7 @@ TEST_F(ConformanceTest, testIterateAlreadyCompleted)
     verifyDocs(docs, chunks);
 
     IterateResult result(spi->iterate(iter.getIteratorId(), 4096, context));
-    EXPECT_EQ(Result::NONE, result.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
     EXPECT_EQ(size_t(0), result.getEntries().size());
     EXPECT_TRUE(result.isCompleted());
 
@@ -1511,7 +1511,7 @@ TEST_F(ConformanceTest, testIterateEmptyBucket)
     CreateIteratorResult iter(createIterator(*spi, b, sel));
 
     IterateResult result(spi->iterate(iter.getIteratorId(), 4096, context));
-    EXPECT_EQ(Result::NONE, result.getErrorCode());
+    EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
     EXPECT_EQ(size_t(0), result.getEntries().size());
     EXPECT_TRUE(result.isCompleted());
 
@@ -1556,7 +1556,7 @@ testDeleteBucketPostCondition(const PersistenceProvider::UP &spi,
                                     doc1.getId(),
                                     context);
 
-        EXPECT_EQ(Result::NONE, result.getErrorCode());
+        EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode());
         EXPECT_EQ(Timestamp(0), result.getTimestamp());
     }
 }
@@ -2152,7 +2152,7 @@ TEST_F(ConformanceTest, testMaintain)
     spi->put(bucket, Timestamp(3), doc1, context);
     spi->flush(bucket, context);
 
-    EXPECT_EQ(Result::NONE,
+    EXPECT_EQ(Result::ErrorType::NONE,
                          spi->maintain(bucket, LOW).getErrorCode());
 }
 
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index 65759a0c783..6834f453695 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -409,7 +409,7 @@ DummyPersistence::setActiveState(const Bucket& b,
 
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
     if (!bc.get()) {
-        return BucketInfoResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
     (*bc)->setActive(newState == BucketInfo::ACTIVE);
     return Result();
@@ -424,7 +424,7 @@ DummyPersistence::getBucketInfo(const Bucket& b) const
     if (!bc.get()) {
         LOG(debug, "getBucketInfo(%s) : (bucket not found)",
             b.toString().c_str());
-        return BucketInfoResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
 
     BucketInfo info((*bc)->getBucketInfo());
@@ -446,7 +446,7 @@ DummyPersistence::put(const Bucket& b, Timestamp t, const Document::SP& doc,
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
     if (!bc.get()) {
-        return BucketInfoResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
 
     DocEntry::SP existing = (*bc)->getEntry(t);
@@ -454,7 +454,7 @@ DummyPersistence::put(const Bucket& b, Timestamp t, const Document::SP& doc,
         if (doc->getId() == *existing->getDocumentId()) {
             return Result();
         } else {
-            return Result(Result::TIMESTAMP_EXISTS,
+            return Result(Result::ErrorType::TIMESTAMP_EXISTS,
                           "Timestamp already existed");
         }
     }
@@ -474,7 +474,7 @@ DummyPersistence::maintain(const Bucket& b,
     if (_simulateMaintainFailure) {
         BucketContentGuard::UP bc(acquireBucketWithLock(b));
         if (!bc.get()) {
-            return BucketInfoResult(Result::TRANSIENT_ERROR, "Bucket not found");
+            return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
         }
 
         if (!(*bc)->_entries.empty()) {
@@ -503,7 +503,7 @@ DummyPersistence::remove(const Bucket& b,
 
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
     if (!bc.get()) {
-        return RemoveResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return RemoveResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
 
     DocEntry::SP entry((*bc)->getEntry(did));
@@ -564,13 +564,13 @@ DummyPersistence::createIterator(
                         true).release());
         if (!docSelection.get()) {
             return CreateIteratorResult(
-                    Result::PERMANENT_ERROR,
+                    Result::ErrorType::PERMANENT_ERROR,
                     "Got invalid/unparseable document selection string");
         }
     }
     BucketContentGuard::UP bc(acquireBucketWithLock(b, LockMode::Shared));
     if (!bc.get()) {
-        return CreateIteratorResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return CreateIteratorResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
 
     Iterator* it;
@@ -650,7 +650,7 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con
         vespalib::MonitorGuard lock(_monitor);
         std::map<IteratorId, Iterator::UP>::iterator iter(_iterators.find(id));
         if (iter == _iterators.end()) {
-            return IterateResult(Result::PERMANENT_ERROR,
+            return IterateResult(Result::ErrorType::PERMANENT_ERROR,
                         "Bug! Used iterate without sending createIterator first");
         }
         it = iter->second.get();
@@ -659,7 +659,7 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con
     BucketContentGuard::UP bc(acquireBucketWithLock(it->_bucket, LockMode::Shared));
     if (!bc.get()) {
         ctx.trace(9, "finished iterate(); bucket not found");
-        return IterateResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return IterateResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
     LOG(debug, "Iterator %" PRIu64 " acquired bucket lock", uint64_t(id));
 
@@ -776,7 +776,7 @@ DummyPersistence::split(const Bucket& source,
     BucketContentGuard::UP sourceGuard(acquireBucketWithLock(source));
     if (!sourceGuard.get()) {
         LOG(debug, "%s not found", source.toString().c_str());
-        return Result(Result::TRANSIENT_ERROR, "Bucket not found");
+        return Result(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
     BucketContentGuard::UP target1Guard(acquireBucketWithLock(target1));
     BucketContentGuard::UP target2Guard(acquireBucketWithLock(target2));
@@ -863,7 +863,7 @@ DummyPersistence::revert(const Bucket& b, Timestamp t, Context&)
 
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
     if (!bc.get()) {
-        return BucketInfoResult(Result::TRANSIENT_ERROR, "Bucket not found");
+        return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
     }
 
     BucketContent& content(**bc);
diff --git a/persistence/src/vespa/persistence/spi/result.cpp b/persistence/src/vespa/persistence/spi/result.cpp
index 4aa01d22649..024f5595102 100644
--- a/persistence/src/vespa/persistence/spi/result.cpp
+++ b/persistence/src/vespa/persistence/spi/result.cpp
@@ -3,6 +3,7 @@
 #include "result.h"
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/vespalib/stllike/asciistream.h>
+#include <ostream>
 
 namespace storage::spi {
 
@@ -13,7 +14,7 @@ Result::~Result() { }
 vespalib::string
 Result::toString() const {
     vespalib::asciistream os;
-    os << "Result(" << _errorCode << ", " << _errorMessage << ")";
+    os << "Result(" << static_cast<int>(_errorCode) << ", " << _errorMessage << ")";
     return os.str();
 }
 
@@ -22,6 +23,10 @@ operator << (std::ostream & os, const Result & r) {
     return os << r.toString();
 }
 
+std::ostream & operator << (std::ostream & os, const Result::ErrorType &errorCode) {
+    return os << static_cast<int>(errorCode);
+}
+
 GetResult::GetResult(Document::UP doc, Timestamp timestamp)
     : Result(),
       _timestamp(timestamp),
diff --git a/persistence/src/vespa/persistence/spi/result.h b/persistence/src/vespa/persistence/spi/result.h
index 0daa784caa4..fe8e74706bb 100644
--- a/persistence/src/vespa/persistence/spi/result.h
+++ b/persistence/src/vespa/persistence/spi/result.h
@@ -13,7 +13,7 @@ class Result {
 public:
     typedef std::unique_ptr<Result> UP;
 
-    enum ErrorType {
+    enum class ErrorType {
         NONE,
         TRANSIENT_ERROR,
         PERMANENT_ERROR,
@@ -26,7 +26,7 @@ public:
     /**
      * Constructor to use for a result where there is no error.
      */
-    Result() : _errorCode(NONE), _errorMessage() {}
+    Result() : _errorCode(ErrorType::NONE), _errorMessage() {}
 
     /**
      * Constructor to use when an error has been detected.
@@ -46,7 +46,7 @@ public:
     }
 
     bool hasError() const {
-        return _errorCode != NONE;
+        return _errorCode != ErrorType::NONE;
     }
 
     ErrorType getErrorCode() const {
@@ -66,6 +66,8 @@ private:
 
 std::ostream & operator << (std::ostream & os, const Result & r);
 
+std::ostream & operator << (std::ostream & os, const Result::ErrorType &errorCode);
+
 class BucketInfoResult : public Result {
 public:
     /**