diff --git a/hoot-core/src/main/cpp/hoot/core/elements/ElementComparer.cpp b/hoot-core/src/main/cpp/hoot/core/elements/ElementComparer.cpp
index e25663b..d6d66ed 100644
--- a/hoot-core/src/main/cpp/hoot/core/elements/ElementComparer.cpp
+++ b/hoot-core/src/main/cpp/hoot/core/elements/ElementComparer.cpp
@@ -29,111 +29,58 @@
#include <hoot/core/elements/Node.h>
#include <hoot/core/elements/Way.h>
#include <hoot/core/elements/Relation.h>
-#include <hoot/core/util/GeometryUtils.h>
#include <hoot/core/util/Log.h>
#include <hoot/core/schema/MetadataTags.h>
-#include <hoot/core/visitors/CalculateHashVisitor2.h>
+#include <hoot/core/visitors/ElementHashVisitor.h>
namespace hoot
{
-ElementComparer::ElementComparer(Meters threshold) :
-_threshold(threshold),
-_ignoreElementId(false)
+ElementComparer::ElementComparer() :
+_ignoreElementId(false),
+_ignoreVersion(false)
{
}
-void ElementComparer::_removeTagsNotImportantForComparison(Tags& tags) const
+QString ElementComparer::toHashString(const ConstElementPtr& e) const
{
- LOG_TRACE("Removing tags...");
-
- // having trouble making this work here in place of the two following lines due to the
- // ServiceChangesetReplacement* tests...will revisit
- //tags.removeMetadata();
-
- tags.remove(MetadataTags::HootStatus());
- //this is ok b/c we have the actual id to compare to, but it should still probably be fixed to
- //carry along the hoot:id tag for consistency's sake when that is desired
- tags.remove(MetadataTags::HootId());
-
- // not sure where "status" is coming from...should be "hoot:status"...bug somewhere?
- tags.remove("status");
- tags.remove(MetadataTags::SourceDateTime());
+ ElementHashVisitor hashVis;
+ hashVis.setOsmMap(_map.get());
+ // TODO: This doesn't seem right, but its been working this way for awhile now...remove later?
+ hashVis.setIncludeCircularError(true);
+ return hashVis.toHashString(e);
}
bool ElementComparer::isSame(ElementPtr e1, ElementPtr e2) const
{
- if (_ignoreElementId && !_map.get())
- {
- throw IllegalArgumentException(
- "If ignoring element IDs in ElementComparer a map must be passed in.");
- }
-
- LOG_VART(e1->getElementId());
- LOG_VART(e2->getElementId());
+ //LOG_VART(e1->getElementId());
+ //LOG_VART(e2->getElementId());
+ // different types?
if (e1->getElementType() != e2->getElementType())
{
+ LOG_TRACE("compare failed on type: " << e1->getElementId() << ", " << e2->getElementId());
return false;
}
- if (e1->getElementType() == ElementType::Node)
+ // different IDs?
+ if (!_ignoreElementId && (e1->getElementId() != e2->getElementId()))
{
- // Set the hoot hash tag here if it doesn't exist, since its required for node comparisons.
- CalculateHashVisitor2 hashVis;
- if (!e1->getTags().contains(MetadataTags::HootHash()))
- {
- hashVis.visit(e1);
- }
- if (!e2->getTags().contains(MetadataTags::HootHash()))
- {
- hashVis.visit(e2);
- }
+ LOG_TRACE("compare failed on ID: " << e1->getElementId() << ", " << e2->getElementId());
+ return false;
}
- //only nodes have been converted over to use hash comparisons so far
- else
- {
- //create modified copies of the tags for comparing, as we don't care if some tags are identical
- Tags tags1 = e1->getTags();
- _removeTagsNotImportantForComparison(tags1);
- Tags tags2 = e2->getTags();
- _removeTagsNotImportantForComparison(tags2);
- // not checking status here b/c if only the status changed on the element and no other tags or
- // geometries, there's no point in detecting a change
- if ((!_ignoreElementId && e1->getElementId() != e2->getElementId()) ||
- !(tags1 == tags2) ||
- (e1->getVersion() != e2->getVersion()) ||
- fabs(e1->getCircularError() - e2->getCircularError()) > _threshold)
- {
- if (Log::getInstance().getLevel() == Log::Trace)
- {
- if ( e1->getElementId() != e2->getElementId())
- {
- LOG_TRACE("compare failed on IDs");
- }
- else if (!(tags1 == tags2))
- {
- LOG_TRACE("compare failed on tags");
- }
- else if (e1->getVersion() != e2->getVersion())
- {
- LOG_TRACE("compare failed on version");
- }
- else if (fabs(e1->getCircularError() - e2->getCircularError()) > _threshold)
- {
- LOG_TRACE("compare failed on circular error:");
- LOG_VART(fabs(e1->getCircularError() - e2->getCircularError()));
- LOG_VART(_threshold);
- }
-
- LOG_TRACE("elements failed comparison:");
- LOG_VART(tags1);
- LOG_VART(tags2);
- }
+ // different versions?
+ if (!_ignoreVersion && (e1->getVersion() != e2->getVersion()))
+ {
+ LOG_TRACE("compare failed on version: " << e1->getElementId() << ", " << e2->getElementId());
+ return false;
+ }
- return false;
- }
+ if (_ignoreElementId && !_map.get())
+ {
+ throw IllegalArgumentException(
+ "If ignoring element IDs in ElementComparer a map must be passed in.");
}
switch (e1->getElementType().getEnum())
@@ -149,109 +96,169 @@ bool ElementComparer::isSame(ElementPtr e1, ElementPtr e2) const
}
}
-bool ElementComparer::_compareNode(const std::shared_ptr<const Element>& re,
- const std::shared_ptr<const Element>& e) const
+void ElementComparer::_setHash(ElementPtr element) const
{
- if (!re->getTags().contains(MetadataTags::HootHash()) ||
- !e->getTags().contains(MetadataTags::HootHash()))
+ // Set the hoot hash tag here if it doesn't exist.
+ ElementHashVisitor hashVis;
+ hashVis.setOsmMap(_map.get());
+ // TODO: This doesn't seem right, but its been working this way for awhile now...remove later?
+ hashVis.setIncludeCircularError(true);
+ if (!element->getTags().contains(MetadataTags::HootHash()))
{
- throw HootException(
- QString("ElementComparer requires the %1 tag be set for node comparison. Nodes: %2, %3")
- .arg(MetadataTags::HootHash())
- .arg(re->getElementId().toString())
- .arg(e->getElementId().toString()));
+ hashVis.visit(element);
}
+}
- if (re->getElementId().getId() == DEBUG_ID || e->getElementId().getId() == DEBUG_ID)
- {
- LOG_VARD(re);
- LOG_VARD(e);
- }
+bool ElementComparer::_compareNode(ElementPtr re, ElementPtr e) const
+{
+ _setHash(re);
+ _setHash(e);
- bool same = false;
- if (re->getTags()[MetadataTags::HootHash()] == e->getTags()[MetadataTags::HootHash()])
- {
- same = true;
- }
- else
+ if (re->getElementId().getId() == DEBUG_ID || e->getElementId().getId() == DEBUG_ID)
{
- LOG_TRACE("Compare failed:");
LOG_VART(re);
LOG_VART(e);
}
- return same;
+ return _haveSameHash(re, e);
}
-bool ElementComparer::_compareWay(const std::shared_ptr<const Element>& re,
- const std::shared_ptr<const Element>& e) const
+bool ElementComparer::_compareWay(ElementPtr re, ElementPtr e) const
{
ConstWayPtr rw = std::dynamic_pointer_cast<const Way>(re);
ConstWayPtr w = std::dynamic_pointer_cast<const Way>(e);
- if (rw->getNodeIds().size() != w->getNodeIds().size())
+ // optimization; tag count comparison isn't a good opt b/c you need to count metadata tags only
+ // and that count isn't available without extra computation
+ if (rw->getNodeCount() != w->getNodeCount())
{
LOG_TRACE(
"Ways " << rw->getElementId() << " and " << w->getElementId() <<
- " failed comparison on way node count: " << rw->getNodeIds().size() << " and " <<
- w->getNodeIds().size());
+ " failed comparison on way node count: " << rw->getNodeCount() << " and " <<
+ w->getNodeCount());
return false;
}
if (!_ignoreElementId)
{
- for (size_t i = 0; i < rw->getNodeIds().size(); ++i)
+ // If we're not ignoring the element IDs of the way nodes, then we'll perform just these checks
+ // instead of computing a hash and avoid having to use a passed in map for callers that don't
+ // have one.
+
+ if (!rw->hasSameNodes(*w))
{
- if (rw->getNodeIds()[i] != w->getNodeIds()[i])
- {
- LOG_TRACE(
- "Ways " << rw->getElementId() << " and " << w->getElementId() <<
- " failed comparison on way nodes: " << rw->getNodeIds() << " and " << w->getNodeIds());
- return false;
- }
+ LOG_TRACE(
+ "Ways " << rw->getElementId() << " and " << w->getElementId() <<
+ " failed comparison on way nodes: " << rw->getNodeIds() << " and " << w->getNodeIds());
+ return false;
}
+
+ if (!rw->hasSameNonMetadataTags(*w))
+ {
+ LOG_TRACE(
+ "Ways " << rw->getElementId() << " and " << w->getElementId() <<
+ " failed comparison on tags.");
+ return false;
+ }
+
+ LOG_TRACE("Ways " << re->getElementId() << " and " << e->getElementId() << " are the same.");
+ return true;
}
else
{
- for (size_t i = 0; i < rw->getNodeIds().size(); ++i)
- {
- NodePtr nodeRw = _map->getNode(ElementId(ElementType::Node, rw->getNodeIds()[i]));
- NodePtr nodeW = _map->getNode(ElementId(ElementType::Node, w->getNodeIds()[i]));
- if (!nodeRw || !nodeW || !isSame(nodeRw, nodeW))
- {
- LOG_TRACE(
- "Ways " << rw->getElementId() << " and " << w->getElementId() <<
- " failed comparison on way nodes: " << rw->getNodeIds() << " and " << w->getNodeIds());
- return false;
- }
- }
+ // If we're ignoring element ID, then we have to dig deeper and compare each way node, which the
+ // hashing takes care of. Hashing also takes tags into account.
+ _setHash(re);
+ _setHash(e);
+ return _haveSameHash(re, e);
}
-
- return true;
}
-bool ElementComparer::_compareRelation(const std::shared_ptr<const Element>& re,
- const std::shared_ptr<const Element>& e) const
+bool ElementComparer::_compareRelation(ElementPtr re, ElementPtr e) const
{
ConstRelationPtr rr = std::dynamic_pointer_cast<const Relation>(re);
ConstRelationPtr r = std::dynamic_pointer_cast<const Relation>(e);
- if (rr->getType() != r->getType() ||
- rr->getMembers().size() != r->getMembers().size())
+ // optimization; tag count comparison isn't a good opt b/c you need to count metadata tags only
+ // and that count isn't available without extra computation
+ if (rr->getType() != r->getType())
{
+ LOG_TRACE(
+ "Relations " << rr->getElementId() << " and " << r->getElementId() <<
+ " failed comparison on type: " << rr->getType() << " and " << r->getType());
+ return false;
+ }
+ if (rr->getMemberCount() != r->getMemberCount())
+ {
+ LOG_TRACE(
+ "Relations " << rr->getElementId() << " and " << r->getElementId() <<
+ " failed comparison on relation member count: " << rr->getMemberCount() << " and " <<
+ r->getMemberCount());
return false;
}
- for (size_t i = 0; i < rr->getMembers().size(); i++)
+ if (!_ignoreElementId)
{
- if (rr->getMembers()[i].role != r->getMembers()[i].role ||
- rr->getMembers()[i].getElementId() != r->getMembers()[i].getElementId())
+ // If we're not ignoring the element IDs of the relation members, then we'll perform just these
+ // checks instead of computing a hash and avoid having to use a passed in map for callers that
+ // don't have one.
+
+ if (!rr->hasSameNonMetadataTags(*r))
{
+ LOG_TRACE(
+ "Relations " << rr->getElementId() << " and " << r->getElementId() <<
+ " failed comparison on tags.");
return false;
}
- }
+ for (size_t i = 0; i < rr->getMembers().size(); i++)
+ {
+ if (rr->getMembers()[i].role != r->getMembers()[i].role ||
+ rr->getMembers()[i].getElementId() != r->getMembers()[i].getElementId())
+ {
+ LOG_TRACE(
+ "Relations " << rr->getElementId() << " and " << r->getElementId() <<
+ " failed comparison on relation members.");
+ return false;
+ }
+ }
return true;
+ }
+ else
+ {
+ // If we're ignoring element ID, then we have to dig deeper and compare each member element,
+ // which the hashing takes care of. Hashing also takes tags into account.
+ _setHash(re);
+ _setHash(e);
+ return _haveSameHash(re, e);
+ }
+}
+
+bool ElementComparer::_haveSameHash(ElementPtr re, ElementPtr e) const
+{
+ if (!re->getTags().contains(MetadataTags::HootHash()) ||
+ !e->getTags().contains(MetadataTags::HootHash()))
+ {
+ throw HootException(
+ QString("ElementComparer requires the %1 tag be set for element comparison. Elements: %2, %3")
+ .arg(MetadataTags::HootHash())
+ .arg(re->getElementId().toString())
+ .arg(e->getElementId().toString()));
+ }
+
+ bool same = false;
+ if (re->getTags()[MetadataTags::HootHash()] == e->getTags()[MetadataTags::HootHash()])
+ {
+ LOG_TRACE(re->getElementId() << " and " << e->getElementId() << " are the same.");
+ same = true;
+ }
+ else
+ {
+ LOG_TRACE("Compare failed:");
+ LOG_VART(re);
+ LOG_VART(e);
+ }
+ return same;
}
}