diff options
13 files changed, 60 insertions, 92 deletions
diff --git a/document/src/tests/documentidtest.cpp b/document/src/tests/documentidtest.cpp index a6255cc148a..d3f6e8a8fcd 100644 --- a/document/src/tests/documentidtest.cpp +++ b/document/src/tests/documentidtest.cpp @@ -44,16 +44,8 @@ TEST(DocumentIdTest, generateJavaComplianceFile) TEST(DocumentIdTest, testOutput) { DocumentId id("id:ns:news::crawler:http://www.yahoo.com"); - - std::ostringstream ost; - ost << id; - std::string expected("id:ns:news::crawler:http://www.yahoo.com"); - EXPECT_EQ(expected, ost.str()); - - EXPECT_EQ(vespalib::string(expected), id.toString()); - - expected = "DocumentId(id = id:ns:news::crawler:http://www.yahoo.com, gid(0xa516a5abd7c7fa26944b72f7))"; - EXPECT_EQ(expected, static_cast<Printable&>(id).toString(true)); + vespalib::string expected("id:ns:news::crawler:http://www.yahoo.com"); + EXPECT_EQ(expected, id.toString()); } namespace { diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp index fa59358f6d3..b460d318099 100644 --- a/document/src/tests/documenttestcase.cpp +++ b/document/src/tests/documenttestcase.cpp @@ -28,7 +28,7 @@ using namespace fieldvalue; TEST(DocumentTest, testSizeOf) { - EXPECT_EQ(136ul, sizeof(Document)); + EXPECT_EQ(128ul, sizeof(Document)); EXPECT_EQ(72ul, sizeof(StructFieldValue)); EXPECT_EQ(24ul, sizeof(StructuredFieldValue)); EXPECT_EQ(64ul, sizeof(SerializableArray)); diff --git a/document/src/tests/fieldvalue/referencefieldvalue_test.cpp b/document/src/tests/fieldvalue/referencefieldvalue_test.cpp index 17fb5ac74e6..b42e2d9cd7a 100644 --- a/document/src/tests/fieldvalue/referencefieldvalue_test.cpp +++ b/document/src/tests/fieldvalue/referencefieldvalue_test.cpp @@ -122,7 +122,7 @@ TEST_F("clone()ing creates new instance with same ID and type", Fixture) { ReferenceFieldValue src(f.refType, DocumentId("id:ns:foo::yoshi")); std::unique_ptr<ReferenceFieldValue> cloned(src.clone()); - ASSERT_TRUE(cloned.get() != nullptr); + ASSERT_TRUE(cloned); ASSERT_TRUE(cloned->hasValidDocumentId()); EXPECT_EQUAL(src.getDocumentId(), cloned->getDocumentId()); EXPECT_EQUAL(src.getDataType(), cloned->getDataType()); @@ -133,7 +133,7 @@ TEST_F("Can clone() value without document ID", Fixture) { ReferenceFieldValue src(f.refType); std::unique_ptr<ReferenceFieldValue> cloned(src.clone()); - ASSERT_TRUE(cloned.get() != nullptr); + ASSERT_TRUE(cloned); EXPECT_FALSE(cloned->hasValidDocumentId()); EXPECT_EQUAL(src.getDataType(), cloned->getDataType()); EXPECT_TRUE(cloned->hasChanged()); diff --git a/document/src/vespa/document/base/documentid.cpp b/document/src/vespa/document/base/documentid.cpp index 3d5657d093c..69e2aacd073 100644 --- a/document/src/vespa/document/base/documentid.cpp +++ b/document/src/vespa/document/base/documentid.cpp @@ -9,30 +9,26 @@ using vespalib::nbostream; namespace document { DocumentId::DocumentId() - : Printable(), - _globalId(), + : _globalId(), _id(new NullIdString()) { } DocumentId::DocumentId(vespalib::stringref id) - : Printable(), - _globalId(), + : _globalId(), _id(IdString::createIdString(id.data(), id.size()).release()) { } DocumentId::DocumentId(vespalib::nbostream & is) - : Printable(), - _globalId(), + : _globalId(), _id(IdString::createIdString(is.peek(), strlen(is.peek())).release()) { is.adjustReadPos(strlen(is.peek()) + 1); } DocumentId::DocumentId(const IdString& id) - : Printable(), - _globalId(), + : _globalId(), _id(id.clone()) { } @@ -51,30 +47,12 @@ void DocumentId::set(vespalib::stringref id) { _globalId.first = false; } -void -DocumentId::print(std::ostream& out, bool verbose, const std::string& indent) const -{ - (void) indent; - if (verbose) { - out << "DocumentId(id = "; - } - out << _id->toString().c_str(); - if (verbose) { - out << ", " << getGlobalId().toString() << ")"; - } -} - size_t DocumentId::getSerializedSize() const { return _id->toString().size() + 1; } -void DocumentId::swap(DocumentId & rhs) { - _id.swap(rhs._id); - std::swap(_globalId, rhs._globalId); -} - void DocumentId::calculateGlobalId() const { @@ -90,5 +68,9 @@ DocumentId::calculateGlobalId() const _globalId.second.set(key); } +std::ostream & +operator << (std::ostream & os, const DocumentId & id) { + return os << id.toString(); +} } // document diff --git a/document/src/vespa/document/base/documentid.h b/document/src/vespa/document/base/documentid.h index d395b751011..b69097e52fd 100644 --- a/document/src/vespa/document/base/documentid.h +++ b/document/src/vespa/document/base/documentid.h @@ -21,7 +21,6 @@ #include "idstring.h" #include "globalid.h" -#include <vespa/document/util/printable.h> namespace vespalib { class nbostream; } @@ -29,7 +28,7 @@ namespace document { class DocumentType; -class DocumentId : public Printable +class DocumentId { public: typedef std::unique_ptr<DocumentId> UP; @@ -41,7 +40,7 @@ public: DocumentId & operator = (DocumentId && rhs) = default; DocumentId(const DocumentId & rhs); DocumentId & operator = (const DocumentId & rhs); - ~DocumentId() override; + ~DocumentId(); /** * Parse the given document identifier given as string, and create an * identifier object from it. @@ -62,8 +61,6 @@ public: */ vespalib::string toString() const; - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - bool operator==(const DocumentId& other) const { return *_id == *other._id; } bool operator!=(const DocumentId& other) const { return ! (*_id == *other._id); } @@ -76,9 +73,7 @@ public: return _globalId.second; } - DocumentId* clone() const { return new DocumentId(*this); } - virtual size_t getSerializedSize() const; - void swap(DocumentId & rhs); + size_t getSerializedSize() const; private: mutable std::pair<bool, GlobalId> _globalId; vespalib::CloneablePtr<IdString> _id; @@ -86,5 +81,7 @@ private: void calculateGlobalId() const; }; +std::ostream & operator << (std::ostream & os, const DocumentId & id); + } // document diff --git a/document/src/vespa/document/fieldvalue/document.cpp b/document/src/vespa/document/fieldvalue/document.cpp index 29414c901f8..ae0cd5ed9eb 100644 --- a/document/src/vespa/document/fieldvalue/document.cpp +++ b/document/src/vespa/document/fieldvalue/document.cpp @@ -226,9 +226,9 @@ Document::print(std::ostream& out, bool verbose, const std::string& indent) const { if (!verbose) { - out << "Document(" << getId() << ", " << getType() << ")"; + out << "Document(" << getId().toString() << ", " << getType() << ")"; } else { - out << "Document(" << getId() << "\n" << indent << " "; + out << "Document(" << getId().toString() << "\n" << indent << " "; getType().print(out, true, indent + " "); for (const_iterator it = begin(); it != end(); ++it) { out << "\n" << indent << " " << it.field().getName() << ": "; diff --git a/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp b/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp index 281161fccbf..b642ebdc775 100644 --- a/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp @@ -37,8 +37,7 @@ ReferenceFieldValue::ReferenceFieldValue( requireIdOfMatchingType(_documentId, _dataType->getTargetType()); } -ReferenceFieldValue::~ReferenceFieldValue() { -} +ReferenceFieldValue::~ReferenceFieldValue() = default; void ReferenceFieldValue::requireIdOfMatchingType( const DocumentId& id, const DocumentType& type) @@ -105,9 +104,7 @@ int ReferenceFieldValue::compare(const FieldValue& rhs) const { void ReferenceFieldValue::print(std::ostream& os, bool verbose, const std::string& indent) const { (void) verbose; assert(_dataType != nullptr); - os << indent << "ReferenceFieldValue(" << *_dataType << ", DocumentId("; - _documentId.print(os, false, ""); - os << "))"; + os << indent << "ReferenceFieldValue(" << *_dataType << ", DocumentId(" << _documentId.toString() << "))"; } bool ReferenceFieldValue::hasChanged() const { diff --git a/document/src/vespa/document/select/valuenodes.cpp b/document/src/vespa/document/select/valuenodes.cpp index 9cfdae14de1..70cdbc10e8b 100644 --- a/document/src/vespa/document/select/valuenodes.cpp +++ b/document/src/vespa/document/select/valuenodes.cpp @@ -571,7 +571,7 @@ IdValueNode::traceValue(const DocumentId& id, std::ostream& out) const value = id.getScheme().getDocType(); out << "Resolved id.type to value\"" << value << "\".\n"; } else { - out << "Could not resolve type of doc " << id << ".\n"; + out << "Could not resolve type of doc " << id.toString() << ".\n"; return std::make_unique<InvalidValue>(); } break; @@ -589,7 +589,7 @@ IdValueNode::traceValue(const DocumentId& id, std::ostream& out) const out << "Resolved group of doc (type " << id.getScheme().getType() << ") to \"" << value << "\".\n"; } else { - out << "Can't resolve group of doc \"" << id << "\".\n"; + out << "Can't resolve group of doc \"" << id.toString() << "\".\n"; return std::make_unique<InvalidValue>(); } break; @@ -604,7 +604,7 @@ IdValueNode::traceValue(const DocumentId& id, std::ostream& out) const << " to " << *result << ".\n"; return result; } else { - out << "Could not resolve user of doc " << id << ".\n"; + out << "Could not resolve user of doc " << id.toString() << ".\n"; return std::make_unique<InvalidValue>(); } } diff --git a/persistence/src/vespa/persistence/spi/docentry.cpp b/persistence/src/vespa/persistence/spi/docentry.cpp index 8ddbe6d9a9d..4c2707f3a5e 100644 --- a/persistence/src/vespa/persistence/spi/docentry.cpp +++ b/persistence/src/vespa/persistence/spi/docentry.cpp @@ -90,9 +90,9 @@ DocEntry::toString() const std::ostringstream out; out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", "; if (_documentId.get() != 0) { - out << *_documentId; + out << _documentId->toString(); } else if (_document.get()) { - out << "Doc(" << _document->getId() << ")"; + out << "Doc(" << _document->getId().toString() << ")"; } else { out << "metadata only"; } @@ -111,9 +111,9 @@ DocEntry::prettyPrint(std::ostream& out) const out << "DocEntry(Timestamp: " << _timestamp << ", size " << getPersistedDocumentSize() << ", "; if (_documentId.get() != 0) { - out << *_documentId; + out << _documentId->toString(); } else if (_document.get()) { - out << "Doc(" << _document->getId() << ")"; + out << "Doc(" << _document->getId().toString() << ")"; } else { out << "metadata only"; } diff --git a/storage/src/vespa/storage/persistence/processallhandler.cpp b/storage/src/vespa/storage/persistence/processallhandler.cpp index 7cb373279a6..39708f33793 100644 --- a/storage/src/vespa/storage/persistence/processallhandler.cpp +++ b/storage/src/vespa/storage/persistence/processallhandler.cpp @@ -56,12 +56,12 @@ public: void process(spi::DocEntry& e) override { ost << " Timestamp: " << e.getTimestamp() << ", "; - if (e.getDocument() != 0) { - ost << "Doc(" << e.getDocument()->getId() << ")" + if (e.getDocument() != nullptr) { + ost << "Doc(" << e.getDocument()->getId().toString() << ")" << ", " << e.getDocument()->getId().getGlobalId().toString() << ", size: " << e.getPersistedDocumentSize(); - } else if (e.getDocumentId() != 0) { - ost << *e.getDocumentId() + } else if (e.getDocumentId() != nullptr) { + ost << e.getDocumentId()->toString() << ", " << e.getDocumentId()->getGlobalId().toString(); } else { ost << "metadata only"; diff --git a/storage/src/vespa/storage/persistence/splitbitdetector.cpp b/storage/src/vespa/storage/persistence/splitbitdetector.cpp index 1133c470bb0..3b91b5a52ea 100644 --- a/storage/src/vespa/storage/persistence/splitbitdetector.cpp +++ b/storage/src/vespa/storage/persistence/splitbitdetector.cpp @@ -91,7 +91,7 @@ struct BucketVisitor : public BucketProcessor::EntryProcessor { for (uint32_t i=0; i<_firstDocs.size(); ++i) { out << "\n" << _firstDocs[i].timestamp << ' ' << _firstDocs[i].bucketId << ' ' - << _firstDocs[i].docId; + << _firstDocs[i].docId.toString(); } } @@ -109,7 +109,7 @@ BucketVisitor::BucketVisitor(const document::BucketIdFactory& factory) _splitMask = (_splitMask << 1) | 1; } } -BucketVisitor::~BucketVisitor() { } +BucketVisitor::~BucketVisitor() = default; bool smallerThanSizeLimit(uint32_t minCount, diff --git a/storage/src/vespa/storage/storageserver/opslogger.cpp b/storage/src/vespa/storage/storageserver/opslogger.cpp index feb48b97b5b..c67f956904d 100644 --- a/storage/src/vespa/storage/storageserver/opslogger.cpp +++ b/storage/src/vespa/storage/storageserver/opslogger.cpp @@ -14,7 +14,7 @@ OpsLogger::OpsLogger(StorageComponentRegister& compReg, : StorageLink("Operations logger"), _lock(), _fileName(), - _targetFile(0), + _targetFile(nullptr), _component(compReg, "opslogger"), _configFetcher(configUri.getContext()) { @@ -46,9 +46,9 @@ OpsLogger::configure(std::unique_ptr<vespa::config::content::core::StorOpslogger // If no change in state, ignore if (config->targetfile == _fileName) return; // If a change we need to close old handle if open - if (_targetFile != 0) { + if (_targetFile != nullptr) { fclose(_targetFile); - _targetFile = 0; + _targetFile = nullptr; } // Set up the new operations log file _fileName = config->targetfile; @@ -73,14 +73,14 @@ OpsLogger::print(std::ostream& out, bool verbose, bool OpsLogger::onPutReply(const std::shared_ptr<api::PutReply>& msg) { - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() - << "\tPUT\t" << msg->getDocumentId() << "\t" + << "\tPUT\t" << msg->getDocumentId().toString() << "\t" << msg->getResult().toString() << "\n"; { vespalib::LockGuard lock(_lock); - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; fwrite(ost.str().c_str(), ost.str().length(), 1, _targetFile); fflush(_targetFile); } @@ -90,14 +90,14 @@ OpsLogger::onPutReply(const std::shared_ptr<api::PutReply>& msg) bool OpsLogger::onUpdateReply(const std::shared_ptr<api::UpdateReply>& msg) { - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() - << "\tUPDATE\t" << msg->getDocumentId() << "\t" + << "\tUPDATE\t" << msg->getDocumentId().toString() << "\t" << msg->getResult().toString() << "\n"; { vespalib::LockGuard lock(_lock); - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; fwrite(ost.str().c_str(), ost.str().length(), 1, _targetFile); fflush(_targetFile); } @@ -107,14 +107,14 @@ OpsLogger::onUpdateReply(const std::shared_ptr<api::UpdateReply>& msg) bool OpsLogger::onRemoveReply(const std::shared_ptr<api::RemoveReply>& msg) { - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() - << "\tREMOVE\t" << msg->getDocumentId() << "\t" + << "\tREMOVE\t" << msg->getDocumentId().toString() << "\t" << msg->getResult().toString() << "\n"; { vespalib::LockGuard lock(_lock); - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; fwrite(ost.str().c_str(), ost.str().length(), 1, _targetFile); fflush(_targetFile); } @@ -124,14 +124,14 @@ OpsLogger::onRemoveReply(const std::shared_ptr<api::RemoveReply>& msg) bool OpsLogger::onGetReply(const std::shared_ptr<api::GetReply>& msg) { - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() - << "\tGET\t" << msg->getDocumentId() << "\t" + << "\tGET\t" << msg->getDocumentId().toString() << "\t" << msg->getResult().toString() << "\n"; { vespalib::LockGuard lock(_lock); - if (_targetFile == 0) return false; + if (_targetFile == nullptr) return false; fwrite(ost.str().c_str(), ost.str().length(), 1, _targetFile); fflush(_targetFile); } diff --git a/storageapi/src/vespa/storageapi/message/persistence.cpp b/storageapi/src/vespa/storageapi/message/persistence.cpp index 8bd4f648f30..2ab73236ca6 100644 --- a/storageapi/src/vespa/storageapi/message/persistence.cpp +++ b/storageapi/src/vespa/storageapi/message/persistence.cpp @@ -31,7 +31,7 @@ PutCommand::PutCommand(const document::Bucket &bucket, const DocumentSP& doc, Ti _timestamp(time), _updateTimestamp(0) { - if (_doc.get() == 0) { + if ( !_doc ) { throw vespalib::IllegalArgumentException("Cannot put a null document", VESPA_STRLOC); } } @@ -58,7 +58,7 @@ PutCommand::getSummary() const void PutCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "Put(" << getBucketId() << ", " << _doc->getId() + out << "Put(" << getBucketId() << ", " << _doc->getId().toString() << ", timestamp " << _timestamp << ", size " << _doc->serialize()->getLength() << ")"; if (verbose) { @@ -104,7 +104,7 @@ UpdateCommand::UpdateCommand(const document::Bucket &bucket, const document::Doc _timestamp(time), _oldTimestamp(0) { - if (_update.get() == 0) { + if ( ! _update) { throw vespalib::IllegalArgumentException("Cannot update a null update", VESPA_STRLOC); } } @@ -132,7 +132,7 @@ UpdateCommand::getSummary() const { void UpdateCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "Update(" << getBucketId() << ", " << _update->getId() << ", timestamp " << _timestamp; + out << "Update(" << getBucketId() << ", " << _update->getId().toString() << ", timestamp " << _timestamp; if (_oldTimestamp != 0) { out << ", old timestamp " << _oldTimestamp; } @@ -200,7 +200,7 @@ GetCommand::getSummary() const void GetCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "Get(" << getBucketId() << ", " << _docId << ")"; + out << "Get(" << getBucketId() << ", " << _docId.toString() << ")"; if (verbose) { out << " : "; BucketCommand::print(out, verbose, indent); @@ -226,7 +226,7 @@ GetReply::~GetReply() = default; void GetReply::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "GetReply(" << getBucketId() << ", " << _docId << ", timestamp " << _lastModifiedTime << ")"; + out << "GetReply(" << getBucketId() << ", " << _docId.toString() << ", timestamp " << _lastModifiedTime << ")"; if (verbose) { out << " : "; BucketReply::print(out, verbose, indent); @@ -253,7 +253,7 @@ RemoveCommand::getSummary() const { void RemoveCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "Remove(" << getBucketId() << ", " << _docId << ", timestamp " << _timestamp << ")"; + out << "Remove(" << getBucketId() << ", " << _docId.toString() << ", timestamp " << _timestamp << ")"; if (verbose) { out << " : "; BucketInfoCommand::print(out, verbose, indent); @@ -273,7 +273,7 @@ RemoveReply::~RemoveReply() = default; void RemoveReply::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "RemoveReply(" << getBucketId() << ", " << _docId << ", timestamp " << _timestamp; + out << "RemoveReply(" << getBucketId() << ", " << _docId.toString() << ", timestamp " << _timestamp; if (_oldTimestamp != 0) { out << ", removed doc from " << _oldTimestamp; } else { @@ -300,8 +300,8 @@ RevertCommand::print(std::ostream& out, bool verbose, const std::string& indent) out << "Revert(" << getBucketId(); if (verbose) { out << ","; - for (uint32_t i=0; i<_tokens.size(); ++i) { - out << "\n" << indent << " " << _tokens[i]; + for (Timestamp token : _tokens) { + out << "\n" << indent << " " << token; } } out << ")"; |