Merge pull request #25895 from vespa-engine/balder/virtual-getComponentRegister

Balder/virtual get component register MERGEOK

4 files changed, 31 insertions, 77 deletions

diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp
index a92cf121fab..290d05e9a59 100644
--- a/storage/src/tests/common/metricstest.cpp
+++ b/storage/src/tests/common/metricstest.cpp
@@ -37,10 +37,10 @@ struct MetricsTest : public Test {
     std::shared_ptr<BucketManagerMetrics> _bucketManagerMetrics;
     std::shared_ptr<VisitorMetrics> _visitorMetrics;
 
-    void createSnapshotForPeriod(std::chrono::seconds secs);
+    void createSnapshotForPeriod(std::chrono::seconds secs) const;
     void assertMetricLastValue(const std::string& name,
                                int interval,
-                               uint64_t expected);
+                               uint64_t expected) const;
 
     MetricsTest();
     ~MetricsTest() override;
@@ -55,8 +55,8 @@ namespace {
     {
         framework::Clock& _clock;
         explicit MetricClock(framework::Clock& c) : _clock(c) {}
-        time_t getTime() const override { return _clock.getTimeInSeconds().getTime(); }
-        time_t getTimeInMilliSecs() const override { return _clock.getTimeInMillis().getTime(); }
+        [[nodiscard]] time_t getTime() const override { return _clock.getTimeInSeconds().getTime(); }
+        time_t getTimeInMilliSecs() const override { return vespalib::count_ms(_clock.getMonotonicTime().time_since_epoch()); }
     };
 }
 
@@ -137,8 +137,8 @@ void MetricsTest::createFakeLoad()
         disk.queueSize.addValue(4 * n);
         disk.averageQueueWaitingTime.addValue(10 * n);
         disk.pendingMerges.addValue(4 * n);
-        for (uint32_t j=0; j<disk.threads.size(); ++j) {
-            FileStorThreadMetrics& thread(*disk.threads[j]);
+        for (const auto & metric : disk.threads) {
+            FileStorThreadMetrics& thread(*metric);
             thread.operations.inc(120 * n);
             thread.failedOperations.inc(2 * n);
 
@@ -180,8 +180,8 @@ void MetricsTest::createFakeLoad()
             thread.merge_handler_metrics.mergeAverageDataReceivedNeeded.addValue(0.8);
         }
     }
-    for (uint32_t i=0; i<_visitorMetrics->threads.size(); ++i) {
-        VisitorThreadMetrics& thread(*_visitorMetrics->threads[i]);
+    for (const auto & metric : _visitorMetrics->threads) {
+        VisitorThreadMetrics& thread(*metric);
         thread.queueSize.addValue(2);
         thread.averageQueueWaitingTime.addValue(10);
         thread.averageVisitorLifeTime.addValue(1000);
@@ -192,9 +192,7 @@ void MetricsTest::createFakeLoad()
     }
     _clock->addSecondsToTime(60);
     _metricManager->timeChangedNotification();
-    while (uint64_t(_metricManager->getLastProcessedTime())
-                < _clock->getTimeInSeconds().getTime())
-    {
+    while (uint64_t(_metricManager->getLastProcessedTime()) < _clock->getTimeInSeconds().getTime()) {
         std::this_thread::sleep_for(5ms);
         _metricManager->timeChangedNotification();
     }
@@ -284,9 +282,7 @@ TEST_F(MetricsTest, html_metrics_report) {
 }
 
 void
-MetricsTest::assertMetricLastValue(const std::string& name,
-                                   int interval,
-                                   uint64_t expected)
+MetricsTest::assertMetricLastValue(const std::string& name, int interval, uint64_t expected) const
 {
     std::ostringstream path;
     path << "metrics?interval=" << interval
@@ -305,7 +301,7 @@ MetricsTest::assertMetricLastValue(const std::string& name,
 using namespace std::chrono_literals;
 
 void
-MetricsTest::createSnapshotForPeriod(std::chrono::seconds secs)
+MetricsTest::createSnapshotForPeriod(std::chrono::seconds secs) const
 {
     _clock->addSecondsToTime(secs.count());
     _metricManager->timeChangedNotification();
diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp
index 91fdf5aa602..ea35c2de36a 100644
--- a/storage/src/tests/common/teststorageapp.cpp
+++ b/storage/src/tests/common/teststorageapp.cpp
@@ -22,18 +22,6 @@ using storage::framework::defaultimplementation::ComponentRegisterImpl;
 
 namespace storage {
 
-namespace {
-    template<typename T>
-    struct ConfigReader : public T::Subscriber,
-                          public T
-    {
-        ConfigReader(const std::string& configId) {
-            T::subscribe(configId, *this);
-        }
-        void configure(const T& c) { dynamic_cast<T&>(*this) = c; }
-    };
-}
-
 TestStorageApp::TestStorageApp(StorageComponentRegisterImpl::UP compReg,
                                const lib::NodeType& type, NodeIndex index,
                                vespalib::stringref configId)
@@ -84,7 +72,7 @@ TestStorageApp::setDistribution(Redundancy redundancy, NodeCount nodeCount)
 void
 TestStorageApp::setTypeRepo(std::shared_ptr<const document::DocumentTypeRepo> repo)
 {
-    _compReg.setDocumentTypeRepo(repo);
+    _compReg.setDocumentTypeRepo(std::move(repo));
 }
 
 void
@@ -93,30 +81,6 @@ TestStorageApp::setClusterState(const lib::ClusterState& c)
     _nodeStateUpdater.setClusterState(std::make_shared<lib::ClusterState>(c));
 }
 
-void
-TestStorageApp::waitUntilInitialized(
-        StorageBucketDBInitializer* initializer, framework::SecondTime timeout)
-{
-        // Always use real clock for wait timeouts. Component clock may be faked
-        // in tests
-    framework::defaultimplementation::RealClock clock;
-    framework::MilliSecTime endTime(clock.getTimeInMillis() + timeout.getMillis());
-    while (!isInitialized()) {
-        std::this_thread::sleep_for(1ms);
-        framework::MilliSecTime currentTime(clock.getTimeInMillis());
-        if (currentTime > endTime) {
-            std::ostringstream error;
-            error << "Failed to initialize service layer within timeout of "
-                  << timeout << " seconds.";
-            if (initializer != 0) {
-                error << " ";
-                LOG(error, "%s", error.str().c_str());
-                throw std::runtime_error(error.str());
-            }
-        }
-    }
-}
-
 namespace {
 NodeIndex getIndexFromConfig(vespalib::stringref configId) {
     if (!configId.empty()) {
@@ -174,7 +138,7 @@ TestServiceLayerApp::setPersistenceProvider(PersistenceProviderUP provider)
 spi::PersistenceProvider&
 TestServiceLayerApp::getPersistenceProvider()
 {
-    if (_persistenceProvider.get() == 0) {
+    if ( ! _persistenceProvider) {
         throw vespalib::IllegalStateException("Persistence provider requested but not initialized.", VESPA_STRLOC);
     }
     return *_persistenceProvider;
@@ -182,7 +146,7 @@ TestServiceLayerApp::getPersistenceProvider()
 
 namespace {
     template<typename T>
-    const T getConfig(vespalib::stringref configId) {
+    T getConfig(vespalib::stringref configId) {
         config::ConfigUri uri(configId);
         return *config::ConfigGetter<T>::getConfig(uri.getConfigId(), uri.getContext());
     }
@@ -192,9 +156,9 @@ void
 TestDistributorApp::configure(vespalib::stringref id)
 {
     if (id.empty()) return;
-    DistributorConfig dc(getConfig<vespa::config::content::core::StorDistributormanagerConfig>(id));
+    auto dc(getConfig<vespa::config::content::core::StorDistributormanagerConfig>(id));
     _compReg.setDistributorConfig(dc);
-    VisitorConfig vc(getConfig<vespa::config::content::core::StorVisitordispatcherConfig>(id));
+    auto vc(getConfig<vespa::config::content::core::StorVisitordispatcherConfig>(id));
     _compReg.setVisitorConfig(vc);
 }
 
diff --git a/storage/src/tests/common/teststorageapp.h b/storage/src/tests/common/teststorageapp.h
index de1dc99bb6e..5cee161599c 100644
--- a/storage/src/tests/common/teststorageapp.h
+++ b/storage/src/tests/common/teststorageapp.h
@@ -68,7 +68,7 @@ public:
     TestStorageApp(StorageComponentRegisterImpl::UP compReg,
                    const lib::NodeType&, NodeIndex = NodeIndex(0xffff),
                    vespalib::stringref configId = "");
-    ~TestStorageApp();
+    ~TestStorageApp() override;
 
     // Set functions, to be able to modify content while running.
     void setDistribution(Redundancy, NodeCount);
@@ -77,15 +77,12 @@ public:
 
     // Utility functions for getting a hold of currently used bits. Practical
     // to avoid adding extra components in the tests.
-    StorageComponentRegisterImpl& getComponentRegister() { return _compReg; }
+    StorageComponentRegisterImpl& getComponentRegister() override { return _compReg; }
     document::TestDocMan& getTestDocMan() { return _docMan; }
-    std::shared_ptr<const document::DocumentTypeRepo> getTypeRepo()
-        { return _compReg.getTypeRepo(); }
-    const document::BucketIdFactory& getBucketIdFactory()
-        { return _compReg.getBucketIdFactory(); }
+    std::shared_ptr<const document::DocumentTypeRepo> getTypeRepo() { return _compReg.getTypeRepo(); }
+    const document::BucketIdFactory& getBucketIdFactory() { return _compReg.getBucketIdFactory(); }
     TestNodeStateUpdater& getStateUpdater() { return _nodeStateUpdater; }
-    std::shared_ptr<lib::Distribution> & getDistribution()
-        { return _compReg.getDistribution(); }
+    std::shared_ptr<lib::Distribution> & getDistribution() { return _compReg.getDistribution(); }
     TestNodeStateUpdater& getNodeStateUpdater() { return _nodeStateUpdater; }
     uint16_t getIndex() const { return _compReg.getIndex(); }
     const NodeIdentity& node_identity() const noexcept { return _node_identity; }
@@ -95,9 +92,6 @@ public:
     DoneInitializeHandler& getDoneInitializeHandler() { return *this; }
     void notifyDoneInitializing() override { _initialized = true; }
     bool isInitialized() const { return _initialized; }
-    void waitUntilInitialized(
-            StorageBucketDBInitializer* initializer = 0,
-            framework::SecondTime timeout = framework::SecondTime(30));
 
 private:
     // Storage server interface implementation (until we can remove it)
@@ -116,14 +110,14 @@ class TestServiceLayerApp : public TestStorageApp
     HostInfo _host_info;
 
 public:
-    TestServiceLayerApp(vespalib::stringref configId);
-    TestServiceLayerApp(NodeIndex = NodeIndex(0xffff), vespalib::stringref configId = "");
-    ~TestServiceLayerApp();
+    explicit TestServiceLayerApp(vespalib::stringref configId);
+    explicit TestServiceLayerApp(NodeIndex = NodeIndex(0xffff), vespalib::stringref configId = "");
+    ~TestServiceLayerApp() override;
 
     void setupDummyPersistence();
     void setPersistenceProvider(PersistenceProviderUP);
 
-    ServiceLayerComponentRegisterImpl& getComponentRegister() { return _compReg; }
+    ServiceLayerComponentRegisterImpl& getComponentRegister() override { return _compReg; }
     HostInfo &get_host_info() noexcept { return _host_info; }
 
     spi::PersistenceProvider& getPersistenceProvider();
@@ -153,7 +147,7 @@ public:
     explicit TestDistributorApp(NodeIndex index, vespalib::stringref configId = "");
     ~TestDistributorApp() override;
 
-    DistributorComponentRegisterImpl& getComponentRegister() {
+    DistributorComponentRegisterImpl& getComponentRegister() override {
         return _compReg;
     }
 
diff --git a/storage/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.h b/storage/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.h
index bd4afa6c9ad..d228dace1ed 100644
--- a/storage/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.h
+++ b/storage/src/vespa/storageframework/defaultimplementation/component/testcomponentregister.h
@@ -21,14 +21,14 @@ namespace storage::framework::defaultimplementation {
 
 class TestComponentRegister {
     ComponentRegisterImpl::UP _compReg;
-    FakeClock _clock;
-    ThreadPoolImpl _threadPool;
+    FakeClock                 _clock;
+    ThreadPoolImpl            _threadPool;
 
 public:
-    TestComponentRegister(ComponentRegisterImpl::UP compReg);
-    ~TestComponentRegister();
+    explicit TestComponentRegister(ComponentRegisterImpl::UP compReg);
+    virtual ~TestComponentRegister();
 
-    ComponentRegisterImpl& getComponentRegister() { return *_compReg; }
+    virtual ComponentRegisterImpl& getComponentRegister() { return *_compReg; }
     FakeClock& getClock() { return _clock; }
     ThreadPoolImpl& getThreadPoolImpl() { return _threadPool; }
     FastOS_ThreadPool& getThreadPool() { return _threadPool.getThreadPool(); }