Unverified Commit fe827c7e authored by Michal 'vorner' Vaner's avatar Michal 'vorner' Vaner
Browse files

Merge #2292

parents bcf21179 c6391ad7
......@@ -1080,55 +1080,25 @@ public:
/// of it. In that case, node parameter is left intact.
//@{
/// \brief Simple find.
/// \brief Simple find
///
/// Acts as described in the \ref find section.
Result find(const isc::dns::Name& name,
DomainTreeNode<T>** node) const {
DomainTreeNodeChain<T> node_path;
const isc::dns::LabelSequence ls(name);
return (find<void*>(ls, node, node_path, NULL, NULL));
}
/// \brief Simple find returning immutable node.
///
/// Acts as described in the \ref find section, but returns immutable node
/// pointer.
Result find(const isc::dns::Name& name,
const DomainTreeNode<T>** node) const {
DomainTreeNodeChain<T> node_path;
DomainTreeNode<T> *target_node = NULL;
const isc::dns::LabelSequence ls(name);
Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
if (ret != NOTFOUND) {
*node = target_node;
}
Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
return (ret);
}
/// \brief Simple find, with node_path tracking
///
/// Acts as described in the \ref find section.
Result find(const isc::dns::Name& name, DomainTreeNode<T>** node,
DomainTreeNodeChain<T>& node_path) const
{
const isc::dns::LabelSequence ls(name);
return (find<void*>(ls, node, node_path, NULL, NULL));
}
/// \brief Simple find returning immutable node, with node_path tracking
///
/// Acts as described in the \ref find section, but returns immutable node
/// pointer.
Result find(const isc::dns::Name& name, const DomainTreeNode<T>** node,
DomainTreeNodeChain<T>& node_path) const
{
DomainTreeNode<T> *target_node = NULL;
const isc::dns::LabelSequence ls(name);
Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
if (ret != NOTFOUND) {
*node = target_node;
}
Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
return (ret);
}
......@@ -1143,13 +1113,9 @@ public:
bool (*callback)(const DomainTreeNode<T>&, CBARG),
CBARG callback_arg) const
{
DomainTreeNode<T>* target_node = NULL;
const isc::dns::LabelSequence ls(name);
Result ret = find(ls, &target_node, node_path, callback,
Result ret = find(ls, node, node_path, callback,
callback_arg);
if (ret != NOTFOUND) {
*node = target_node;
}
return (ret);
}
......@@ -1229,30 +1195,10 @@ public:
/// \c true, it returns immediately with the current node.
template <typename CBARG>
Result find(const isc::dns::LabelSequence& target_labels_orig,
DomainTreeNode<T>** node,
DomainTreeNodeChain<T>& node_path,
bool (*callback)(const DomainTreeNode<T>&, CBARG),
CBARG callback_arg) const;
/// \brief Simple find returning immutable node.
///
/// Acts as described in the \ref find section, but returns immutable
/// node pointer.
template <typename CBARG>
Result find(const isc::dns::LabelSequence& target_labels,
const DomainTreeNode<T>** node,
DomainTreeNodeChain<T>& node_path,
bool (*callback)(const DomainTreeNode<T>&, CBARG),
CBARG callback_arg) const
{
DomainTreeNode<T>* target_node = NULL;
Result ret = find(target_labels, &target_node, node_path,
callback, callback_arg);
if (ret != NOTFOUND) {
*node = target_node;
}
return (ret);
}
CBARG callback_arg) const;
//@}
/// \brief return the next bigger node in DNSSEC order from a given node
......@@ -1515,7 +1461,7 @@ template <typename T>
template <typename CBARG>
typename DomainTree<T>::Result
DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
DomainTreeNode<T>** target,
const DomainTreeNode<T>** target,
DomainTreeNodeChain<T>& node_path,
bool (*callback)(const DomainTreeNode<T>&, CBARG),
CBARG callback_arg) const
......@@ -1526,11 +1472,11 @@ DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
" and label sequence");
}
DomainTreeNode<T>* node;
const DomainTreeNode<T>* node;
if (!node_path.isEmpty()) {
// Get the top node in the node chain
node = const_cast<DomainTreeNode<T>*>(node_path.top());
node = node_path.top();
// Start searching from its down pointer
node = node->getDown();
} else {
......
......@@ -627,22 +627,20 @@ InMemoryClient::InMemoryClientImpl::load(
// node must point to a valid node now
assert(node != NULL);
std::string* tstr = node->setData(new std::string(filename));
const std::string* tstr = node->setData(new std::string(filename));
delete tstr;
ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
zone_name));
const ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
zone_name,
holder.release()));
if (result.code == result::SUCCESS) {
// Only increment the zone count if the zone doesn't already
// exist.
++zone_count_;
}
ZoneTable::FindResult fr(zone_table_->setZoneData(zone_name,
holder.release()));
assert(fr.code == result::SUCCESS);
if (fr.zone_data != NULL) {
ZoneData::destroy(mem_sgmt_, fr.zone_data, rrclass_);
// Destroy the old instance of the zone if there was any
if (result.zone_data != NULL) {
ZoneData::destroy(mem_sgmt_, result.zone_data, rrclass_);
}
return (result.code);
......@@ -732,9 +730,9 @@ InMemoryClient::load(const isc::dns::Name& zone_name,
const std::string
InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
FileNameNode* node(NULL);
FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
&node);
const FileNameNode* node(NULL);
const FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
&node);
if (result == FileNameTree::EXACTMATCH) {
return (*node->getData());
} else {
......@@ -742,24 +740,6 @@ InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
}
}
result::Result
InMemoryClient::add(const isc::dns::Name& zone_name,
const ConstRRsetPtr& rrset)
{
const ZoneTable::FindResult result =
impl_->zone_table_->findZone(zone_name);
if (result.code != result::SUCCESS) {
isc_throw(DataSourceError, "No such zone: " + zone_name.toText());
}
const ConstRRsetPtr sig_rrset =
rrset ? rrset->getRRsig() : ConstRRsetPtr();
impl_->add(rrset, sig_rrset, zone_name, *result.zone_data);
// add() doesn't allow duplicate add, so we always return SUCCESS.
return (result::SUCCESS);
}
namespace {
class MemoryIterator : public ZoneIterator {
......
......@@ -139,35 +139,6 @@ public:
/// zone from a file before.
const std::string getFileName(const isc::dns::Name& zone_name) const;
/// \brief Inserts an rrset into the zone.
///
/// It puts another RRset into the zone.
///
/// In the current implementation, this method doesn't allow an existing
/// RRset to be updated or overridden. So the caller must make sure that
/// all RRs of the same type and name must be given in the form of a
/// single RRset. The current implementation will also require that
/// when an RRSIG is added, the RRset to be covered has already been
/// added. These restrictions are probably too strict when this data
/// source accepts various forms of input, so they should be revisited
/// later.
///
/// Except for NullRRset and OutOfZone, this method does not guarantee
/// strong exception safety (it is currently not needed, if it is needed
/// in future, it should be implemented).
///
/// \throw NullRRset \c rrset is a NULL pointer.
/// \throw OutOfZone The owner name of \c rrset is outside of the
/// origin of the zone.
/// \throw AddError Other general errors.
/// \throw Others This method might throw standard allocation exceptions.
///
/// \param rrset The set to add.
/// \return SUCCESS or EXIST (if an rrset for given name and type already
/// exists).
result::Result add(const isc::dns::Name& zone_name,
const isc::dns::ConstRRsetPtr& rrset);
/// \brief RRset is NULL exception.
///
/// This is thrown if the provided RRset parameter is NULL.
......
......@@ -428,7 +428,7 @@ FindNodeResult findNode(const ZoneData& zone_data,
ZoneFinder::FindOptions options,
bool out_of_zone_ok = false)
{
ZoneNode* node = NULL;
const ZoneNode* node = NULL;
FindState state((options & ZoneFinder::FIND_GLUE_OK) != 0);
const ZoneTree& tree(zone_data.getZoneTree());
......
......@@ -69,17 +69,13 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
ZoneTable::AddResult
ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
const Name& zone_name)
const Name& zone_name, ZoneData* content)
{
// Create a new ZoneData instance first. If the specified name already
// exists in the table, the new data will soon be destroyed, but we want
// to make sure if this allocation fails the tree won't be changed to
// provide as strong guarantee as possible. In practice, we generally
// expect the caller tries to add a zone only when it's a new one, so
// this should be a minor concern.
SegmentObjectHolder<ZoneData, RRClass> holder(
mem_sgmt, ZoneData::create(mem_sgmt, zone_name), zone_class);
if (content == NULL) {
isc_throw(isc::BadValue, "Zone content must not be NULL");
}
SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, content,
zone_class);
// Get the node where we put the zone
ZoneTableNode* node(NULL);
switch (zones_->insert(mem_sgmt, zone_name, &node)) {
......@@ -94,18 +90,18 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
// Can Not Happen
assert(node != NULL);
// Is it empty? We either just created it or it might be nonterminal
if (node->isEmpty()) {
node->setData(holder.get());
return (AddResult(result::SUCCESS, holder.release()));
} else { // There's something there already
return (AddResult(result::EXIST, node->getData()));
// We can release now, setData never throws
ZoneData* old = node->setData(holder.release());
if (old != NULL) {
return (AddResult(result::EXIST, old));
} else {
return (AddResult(result::SUCCESS, NULL));
}
}
ZoneTable::FindResult
ZoneTable::findZone(const Name& name) const {
ZoneTableNode* node(NULL);
const ZoneTableNode* node(NULL);
result::Result my_result;
// Translate the return codes
......@@ -132,20 +128,6 @@ ZoneTable::findZone(const Name& name) const {
return (FindResult(my_result, node->getData()));
}
ZoneTable::FindResult
ZoneTable::setZoneData(const Name& name, ZoneData* data)
{
ZoneTableNode* node(NULL);
ZoneTableTree::Result result(zones_->find(name, &node));
if (result != ZoneTableTree::EXACTMATCH) {
return (FindResult(result::NOTFOUND, NULL));
} else {
return (FindResult(result::SUCCESS, node->setData(data)));
}
}
} // end of namespace memory
} // end of namespace datasrc
} // end of namespace isc
......@@ -74,23 +74,23 @@ private:
typedef DomainTreeNode<ZoneData> ZoneTableNode;
public:
/// \brief Result data of addZone() method.
struct AddResult {
AddResult(result::Result param_code, ZoneData* param_zone_data) :
code(param_code), zone_data(param_zone_data)
{}
const result::Result code;
ZoneData* const zone_data;
};
/// \brief Result data of addZone() method.
struct AddResult {
AddResult(result::Result param_code, ZoneData* param_zone_data) :
code(param_code), zone_data(param_zone_data)
{}
const result::Result code;
ZoneData* const zone_data;
};
/// \brief Result data of findZone() method.
struct FindResult {
FindResult(result::Result param_code,
ZoneData* param_zone_data) :
const ZoneData* param_zone_data) :
code(param_code), zone_data(param_zone_data)
{}
const result::Result code;
ZoneData* const zone_data;
const ZoneData* const zone_data;
};
private:
......@@ -140,30 +140,29 @@ public:
/// Add a new zone to the \c ZoneTable.
///
/// This method creates a new \c ZoneData for the given zone name and
/// holds it in the internal table. The newly created zone data will be
/// returned via the \c zone_data member of the return value. If the given
/// zone name already exists in the table, a new data object won't be
/// created; instead, the existing corresponding data will be returned.
///
/// The zone table keeps the ownership of the created zone data; the
/// caller must not try to destroy it directly. (We'll eventually
/// add an interface to delete specific zone data from the table).
/// This method adds a given zone data to the internal table.
///
/// \throw std::bad_alloc Internal resource allocation fails.
///
/// \param mem_sgmt The \c MemorySegment to allocate zone data to be
/// created. It must be the same segment that was used to create
/// the zone table at the time of create().
/// created. It must be the same segment that was used to create
/// the zone table at the time of create().
/// \param zone_name The name of the zone to be added.
/// \param zone_class The RR class of the zone. It must be the RR class
/// that is supposed to be associated to the zone table.
/// that is supposed to be associated to the zone table.
/// \param content This one should hold the zone content (the ZoneData).
/// The ownership is passed onto the zone table. Must not be null.
/// Must correspond to the name and class and must be allocated from
/// mem_sgmt.
/// \return \c result::SUCCESS If the zone is successfully
/// added to the zone table.
/// \return \c result::EXIST The zone table already contains
/// zone of the same origin.
AddResult addZone(util::MemorySegment& mem_sgmt, dns::RRClass zone_class,
const dns::Name& zone_name);
/// added to the zone table.
/// \return \c result::EXIST The zone table already contained
/// zone of the same origin. The old data is replaced and returned
/// inside the result.
AddResult addZone(util::MemorySegment& mem_sgmt,
dns::RRClass zone_class,
const dns::Name& zone_name,
ZoneData* content);
/// Find a zone that best matches the given name in the \c ZoneTable.
///
......@@ -185,16 +184,6 @@ public:
/// \return A \c FindResult object enclosing the search result (see above).
FindResult findZone(const isc::dns::Name& name) const;
/// Override the ZoneData for a node (zone) in the zone tree.
///
/// \throw none
///
/// \param name A domain name for which the zone data is set.
/// \param data The new zone data to set.
/// \return A \c FindResult object containing the old data if the
/// zone was found.
FindResult setZoneData(const isc::dns::Name& name, ZoneData* data);
private:
boost::interprocess::offset_ptr<ZoneTableTree> zones_;
};
......
......@@ -256,8 +256,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
// "g.h" is not a subtree root
EXPECT_EQ(TestDomainTree::EXACTMATCH,
dtree_expose_empty_node.find(Name("g.h"), &dtnode));
EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
// fission the node "g.h"
EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
......@@ -270,8 +270,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
// "g.h" should be a subtree root now.
EXPECT_EQ(TestDomainTree::EXACTMATCH,
dtree_expose_empty_node.find(Name("g.h"), &dtnode));
EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
}
TEST_F(DomainTreeTest, additionalNodeFission) {
......@@ -286,8 +286,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
// "t.0" is not a subtree root
EXPECT_EQ(TestDomainTree::EXACTMATCH,
dtree_expose_empty_node.find(Name("t.0"), &dtnode));
EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
// fission the node "t.0"
EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
......@@ -300,8 +300,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
// "t.0" should be a subtree root now.
EXPECT_EQ(TestDomainTree::EXACTMATCH,
dtree_expose_empty_node.find(Name("t.0"), &dtnode));
EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
}
TEST_F(DomainTreeTest, findName) {
......@@ -328,10 +328,10 @@ TEST_F(DomainTreeTest, findName) {
EXPECT_EQ(TestDomainTree::PARTIALMATCH,
dtree_expose_empty_node.find(Name("m.d.e.f"), &cdtnode));
// find dtnode
// find cdtnode
EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("q.w.y.d.e.f"),
&dtnode));
EXPECT_EQ(Name("q"), dtnode->getName());
&cdtnode));
EXPECT_EQ(Name("q"), cdtnode->getName());
}
TEST_F(DomainTreeTest, findError) {
......@@ -411,11 +411,12 @@ performCallbackTest(TestDomainTree& dtree,
Name("example"),
&parentdtnode));
// the child/parent nodes shouldn't "inherit" the callback flag.
// "dtnode" may be invalid due to the insertion, so we need to re-find
// it.
// "dtnode" should still validly point to "callback.example", but we
// explicitly confirm it.
EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("callback.example"),
&dtnode));
EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
&cdtnode));
ASSERT_EQ(dtnode, cdtnode);
EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
EXPECT_FALSE(subdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
EXPECT_FALSE(parentdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
......
......@@ -543,29 +543,6 @@ TEST_F(MemoryClientTest, loadRRSIGs) {
EXPECT_EQ(1, client_->getZoneCount());
}
TEST_F(MemoryClientTest, loadRRSIGsRdataMixedCoveredTypes) {
client_->load(Name("example.org"),
TEST_DATA_DIR "/example.org-rrsigs.zone");
RRsetPtr rrset(new RRset(Name("example.org"),
RRClass::IN(), RRType::A(), RRTTL(3600)));
rrset->addRdata(in::A("192.0.2.1"));
rrset->addRdata(in::A("192.0.2.2"));
RRsetPtr rrsig(new RRset(Name("example.org"), zclass_,
RRType::RRSIG(), RRTTL(300)));
rrsig->addRdata(generic::RRSIG("A 5 3 3600 20000101000000 20000201000000 "
"12345 example.org. FAKEFAKEFAKE"));
rrsig->addRdata(generic::RRSIG("NS 5 3 3600 20000101000000 20000201000000 "
"54321 example.org. FAKEFAKEFAKEFAKE"));
rrset->addRRsig(rrsig);
EXPECT_THROW(client_->add(Name("example.org"), rrset),
InMemoryClient::AddError);
// Teardown checks for memory segment leaks
}
TEST_F(MemoryClientTest, getZoneCount) {
EXPECT_EQ(0, client_->getZoneCount());
client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
......@@ -655,75 +632,6 @@ TEST_F(MemoryClientTest, getIteratorGetSOAThrowsNotImplemented) {
EXPECT_THROW(iterator->getSOA(), isc::NotImplemented);
}
TEST_F(MemoryClientTest, addRRsetToNonExistentZoneThrows) {
// The zone "example.org" doesn't exist, so we can't add an RRset to
// it.
RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
RRTTL(300)));
rrset_a->addRdata(rdata::in::A("192.0.2.1"));
EXPECT_THROW(client_->add(Name("example.org"), rrset_a), DataSourceError);
}
TEST_F(MemoryClientTest, addOutOfZoneThrows) {
// Out of zone names should throw.
client_->load(Name("example.org"),
TEST_DATA_DIR "/example.org-empty.zone");
RRsetPtr rrset_a(new RRset(Name("a.example.com"),
RRClass::IN(), RRType::A(), RRTTL(300)));
rrset_a->addRdata(rdata::in::A("192.0.2.1"));
EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
OutOfZone);
// Teardown checks for memory segment leaks
}
TEST_F(MemoryClientTest, addNullRRsetThrows) {
client_->load(Name("example.org"),
TEST_DATA_DIR "/example.org-rrsigs.zone");
EXPECT_THROW(client_->add(Name("example.org"), ConstRRsetPtr()),
InMemoryClient::NullRRset);
// Teardown checks for memory segment leaks
}
TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
client_->load(Name("example.org"),
TEST_DATA_DIR "/example.org-rrsigs.zone");
RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
RRTTL(300)));
EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
InMemoryClient::AddError);
// Teardown checks for memory segment leaks
}
TEST_F(MemoryClientTest, add) {
client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
// Add another RRset
RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
RRTTL(300)));
rrset_a->addRdata(rdata::in::A("192.0.2.1"));
client_->add(Name("example.org"), rrset_a);
ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
// First we have the SOA
ConstRRsetPtr rrset(iterator->getNextRRset());
EXPECT_TRUE(rrset);
EXPECT_EQ(RRType::A(), rrset->getType());
rrset = iterator->getNextRRset();
EXPECT_TRUE(rrset);
EXPECT_EQ(RRType::SOA(), rrset->getType());
// There's nothing else in this zone
EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
}
TEST_F(MemoryClientTest, findZoneData) {
client_->load(Name("example.org"),
TEST_DATA_DIR "/example.org-rrsigs.zone");
......@@ -774,4 +682,16 @@ TEST_F(MemoryClientTest, getJournalReaderNotImplemented) {
EXPECT_THROW(client_->getJournalReader(Name("."), 0, 0),
isc::NotImplemented);
}
// TODO (upon merge of #2268): Re-add (and modify not to need
// InMemoryClient::add) the tests removed in
// 7a628baa1a158b5837d6f383e10b30542d2ac59b. Maybe some of them
// are really not needed.