table: StrategyChoice no longer supports installed instances
All strategies should be registered in the strategy registry.
refs #3868
Change-Id: Ie89b0ffaf9943e591f9f2d80546b013d5cc16ff8
diff --git a/daemon/table/strategy-choice-entry.cpp b/daemon/table/strategy-choice-entry.cpp
index b9e0b48..8a73c5d 100644
--- a/daemon/table/strategy-choice-entry.cpp
+++ b/daemon/table/strategy-choice-entry.cpp
@@ -32,7 +32,6 @@
Entry::Entry(const Name& prefix)
: m_prefix(prefix)
- , m_strategyPtr(nullptr)
, m_nameTreeEntry(nullptr)
{
}
@@ -46,21 +45,9 @@
}
void
-Entry::setStrategy(fw::Strategy& strategy)
-{
- /// \todo #3868
- /// Every entry should have its own Strategy instance in unique_ptr;
- /// m_strategyPtr and this overload should be eliminated.
-
- m_strategy.reset();
- m_strategyPtr = &strategy;
-}
-
-void
Entry::setStrategy(unique_ptr<fw::Strategy> strategy)
{
m_strategy = std::move(strategy);
- m_strategyPtr = m_strategy.get();
}
} // namespace strategy_choice
diff --git a/daemon/table/strategy-choice-entry.hpp b/daemon/table/strategy-choice-entry.hpp
index c6a1326..e1f1eb3 100644
--- a/daemon/table/strategy-choice-entry.hpp
+++ b/daemon/table/strategy-choice-entry.hpp
@@ -67,21 +67,16 @@
fw::Strategy&
getStrategy() const
{
- BOOST_ASSERT(m_strategyPtr != nullptr);
- return *m_strategyPtr;
+ BOOST_ASSERT(m_strategy != nullptr);
+ return *m_strategy;
}
- DEPRECATED(
- void
- setStrategy(fw::Strategy& strategy));
-
void
setStrategy(unique_ptr<fw::Strategy> strategy);
private:
Name m_prefix;
unique_ptr<fw::Strategy> m_strategy;
- fw::Strategy* m_strategyPtr;
name_tree::Entry* m_nameTreeEntry;
friend class name_tree::Entry;
diff --git a/daemon/table/strategy-choice.cpp b/daemon/table/strategy-choice.cpp
index b251a97..7e2f5bf 100644
--- a/daemon/table/strategy-choice.cpp
+++ b/daemon/table/strategy-choice.cpp
@@ -67,75 +67,20 @@
}
bool
-StrategyChoice::hasStrategy(const Name& strategyName, bool isExact) const
-{
- if (isExact) {
- return m_strategyInstances.count(strategyName) > 0;
- }
- else {
- return Strategy::canCreate(strategyName) ||
- this->getStrategy(strategyName) != nullptr;
- }
-}
-
-std::pair<bool, Strategy*>
-StrategyChoice::install(unique_ptr<Strategy> strategy)
-{
- /// \todo #3868
- /// Strategy instance should always be created from Strategy::create;
- /// m_strategyInstances, install, getStrategy, hasStrategy should be eliminated.
-
- BOOST_ASSERT(strategy != nullptr);
- Name strategyName = strategy->getInstanceName();
- // copying Name, so that strategyName remains available even if strategy is deallocated
-
- bool isInserted = false;
- StrategyInstanceTable::iterator it;
- std::tie(it, isInserted) = m_strategyInstances.emplace(strategyName, std::move(strategy));
-
- if (!isInserted) {
- NFD_LOG_ERROR("install(" << strategyName << ") duplicate strategyName");
- }
- return std::make_pair(isInserted, it->second.get());
-}
-
-Strategy*
-StrategyChoice::getStrategy(const Name& strategyName) const
-{
- Strategy* candidate = nullptr;
- for (auto it = m_strategyInstances.lower_bound(strategyName);
- it != m_strategyInstances.end() && strategyName.isPrefixOf(it->first); ++it) {
- switch (it->first.size() - strategyName.size()) {
- case 0: // exact match
- return it->second.get();
- case 1: // unversioned strategyName matches versioned strategy
- candidate = it->second.get();
- break;
- }
- }
- return candidate;
-}
-
-bool
StrategyChoice::insert(const Name& prefix, const Name& strategyName)
{
- unique_ptr<Strategy> createdStrategy;
+ unique_ptr<Strategy> strategy;
try {
- createdStrategy = Strategy::create(strategyName, m_forwarder);
+ strategy = Strategy::create(strategyName, m_forwarder);
}
catch (const std::invalid_argument& e) {
NFD_LOG_ERROR("insert(" << prefix << "," << strategyName << ") cannot create strategy: " << e.what());
return false;
}
- Strategy* strategy = createdStrategy.get();
if (strategy == nullptr) {
- strategy = this->getStrategy(strategyName);
- if (strategy == nullptr) {
- NFD_LOG_ERROR("insert(" << prefix << "," << strategyName << ") strategy not registered or installed");
- return false;
- }
- NFD_LOG_WARN("insert(" << prefix << "," << strategyName << ") using shared strategy instance");
+ NFD_LOG_ERROR("insert(" << prefix << "," << strategyName << ") strategy not registered");
+ return false;
}
name_tree::Entry& nte = m_nameTree.lookup(prefix);
@@ -160,12 +105,7 @@
}
this->changeStrategy(*entry, *oldStrategy, *strategy);
- if (createdStrategy != nullptr) {
- entry->setStrategy(std::move(createdStrategy));
- }
- else {
- entry->setStrategy(*strategy);
- }
+ entry->setStrategy(std::move(strategy));
return true;
}
diff --git a/daemon/table/strategy-choice.hpp b/daemon/table/strategy-choice.hpp
index 2c492cd..e007acf 100644
--- a/daemon/table/strategy-choice.hpp
+++ b/daemon/table/strategy-choice.hpp
@@ -67,27 +67,6 @@
void
setDefaultStrategy(const Name& strategyName);
- /** \brief determines if a strategy is installed
- * \param strategyName name of the strategy
- * \param isExact true to require exact match, false to permit unversioned strategyName
- * \return true if strategy is installed
- */
- DEPRECATED(
- bool
- hasStrategy(const Name& strategyName, bool isExact = false) const);
-
- // DEPRECATED macro does not work when this type appears inline on install function.
- typedef std::pair<bool, fw::Strategy*> InstallResult;
-
- /** \brief install a strategy
- * \return if installed, true, and a pointer to the strategy instance;
- * if not installed due to duplicate strategyName, false,
- * and a pointer to the existing strategy instance
- */
- DEPRECATED(
- InstallResult
- install(unique_ptr<fw::Strategy> strategy));
-
public: // Strategy Choice table
/** \brief set strategy of prefix to be strategyName
* \param prefix the name prefix for which \p strategyName should be used
@@ -160,12 +139,6 @@
}
private:
- /** \brief get Strategy instance by strategyName
- * \param strategyName a versioned or unversioned strategyName
- */
- fw::Strategy*
- getStrategy(const Name& strategyName) const;
-
void
changeStrategy(Entry& entry,
fw::Strategy& oldStrategy,
@@ -184,9 +157,6 @@
Forwarder& m_forwarder;
NameTree& m_nameTree;
size_t m_nItems;
-
- typedef std::map<Name, unique_ptr<fw::Strategy>> StrategyInstanceTable;
- StrategyInstanceTable m_strategyInstances;
};
} // namespace strategy_choice
diff --git a/tests/daemon/fw/dummy-strategy.hpp b/tests/daemon/fw/dummy-strategy.hpp
index 6d4d45d..fb82add 100644
--- a/tests/daemon/fw/dummy-strategy.hpp
+++ b/tests/daemon/fw/dummy-strategy.hpp
@@ -97,6 +97,36 @@
shared_ptr<Face> interestOutFace;
};
+/** \brief DummyStrategy with specific version
+ */
+template<uint64_t VERSION>
+class VersionedDummyStrategy : public DummyStrategy
+{
+public:
+ static void
+ registerAs(const Name& strategyName)
+ {
+ DummyStrategy::registerAsImpl<VersionedDummyStrategy<VERSION>>(strategyName);
+ }
+
+ static Name
+ getStrategyName()
+ {
+ return DummyStrategy::getStrategyName(VERSION);
+ }
+
+ /** \brief constructor
+ *
+ * The strategy instance name is taken from \p name ; if it does not contain a version component,
+ * \p VERSION will be appended.
+ */
+ explicit
+ VersionedDummyStrategy(Forwarder& forwarder, const Name& name = getStrategyName())
+ : DummyStrategy(forwarder, Strategy::makeInstanceName(name, getStrategyName()))
+ {
+ }
+};
+
} // namespace tests
} // namespace nfd
diff --git a/tests/daemon/table/strategy-choice.t.cpp b/tests/daemon/table/strategy-choice.t.cpp
index e827321..4508391 100644
--- a/tests/daemon/table/strategy-choice.t.cpp
+++ b/tests/daemon/table/strategy-choice.t.cpp
@@ -74,6 +74,36 @@
using fw::Strategy;
+BOOST_AUTO_TEST_CASE(Versioning)
+{
+ const Name strategyNameV("/strategy-choice-V");
+ const Name strategyNameV0("/strategy-choice-V/%FD%00");
+ const Name strategyNameV1("/strategy-choice-V/%FD%01");
+ const Name strategyNameV2("/strategy-choice-V/%FD%02");
+ const Name strategyNameV3("/strategy-choice-V/%FD%03");
+ const Name strategyNameV4("/strategy-choice-V/%FD%04");
+ const Name strategyNameV5("/strategy-choice-V/%FD%05");
+
+ VersionedDummyStrategy<1>::registerAs(strategyNameV1);
+ VersionedDummyStrategy<3>::registerAs(strategyNameV3);
+ VersionedDummyStrategy<4>::registerAs(strategyNameV4);
+
+ // unversioned: choose latest version
+ BOOST_CHECK_EQUAL(this->insertAndGet("/A", strategyNameV), strategyNameV4);
+
+ // exact version: choose same version
+ BOOST_CHECK_EQUAL(this->insertAndGet("/B", strategyNameV1), strategyNameV1);
+ BOOST_CHECK_EQUAL(this->insertAndGet("/C", strategyNameV3), strategyNameV3);
+ BOOST_CHECK_EQUAL(this->insertAndGet("/D", strategyNameV4), strategyNameV4);
+
+ // lower version: choose next higher version
+ // BOOST_CHECK_EQUAL(this->insertAndGet("/E", strategyNameV0), strategyNameV1);
+ // BOOST_CHECK_EQUAL(this->insertAndGet("/F", strategyNameV2), strategyNameV3);
+
+ // higher version: failure
+ BOOST_CHECK_EQUAL(sc.insert("/G", strategyNameV5), false);
+}
+
BOOST_AUTO_TEST_CASE(Parameters)
{
// no parameters