diff options
author | Tor Egge <Tor.Egge@broadpark.no> | 2019-07-30 21:49:37 +0200 |
---|---|---|
committer | Tor Egge <Tor.Egge@broadpark.no> | 2019-07-30 21:49:37 +0200 |
commit | 0d58172a97648bd6823b675fcd7df49be3729562 (patch) | |
tree | fa4d031538ffbf212f5aabbe89c95925f185499d /persistence/src | |
parent | 753e9f322ee8a1dccd598c0dbab2c937e3c552cc (diff) |
Use enum class for storage::spi::Result::ErrorType.
Diffstat (limited to 'persistence/src')
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: /** |