Skip to content

Commit

Permalink
Merge pull request #1985 from joto/middle-test-clean
Browse files Browse the repository at this point in the history 
Call middle::after_nodes/ways/relations() in tests in right order
  • Loading branch information
@lonvia
lonvia authored Jun 28, 2023
2 parents 9ab3f89 + e404a65 commit 2038358
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 6 deletions.
28 changes: 28 additions & 0 deletions src/middle-pgsql.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db(

void middle_pgsql_t::node(osmium::Node const &node)
{
assert(m_middle_state == middle_state::node);

if (node.deleted()) {
node_delete(node.id());
} else {
Expand All @@ -358,6 +360,8 @@ void middle_pgsql_t::node(osmium::Node const &node)
}

void middle_pgsql_t::way(osmium::Way const &way) {
assert(m_middle_state == middle_state::way);

if (way.deleted()) {
way_delete(way.id());
} else {
Expand All @@ -369,6 +373,8 @@ void middle_pgsql_t::way(osmium::Way const &way) {
}

void middle_pgsql_t::relation(osmium::Relation const &relation) {
assert(m_middle_state == middle_state::relation);

if (relation.deleted()) {
relation_delete(relation.id());
} else {
Expand Down Expand Up @@ -664,6 +670,11 @@ void middle_pgsql_t::relation_delete(osmid_t osm_id)

void middle_pgsql_t::after_nodes()
{
assert(m_middle_state == middle_state::node);
#ifndef NDEBUG
m_middle_state = middle_state::way;
#endif

m_db_copy.sync();
if (!m_options->append && m_options->flat_node_file.empty()) {
auto const &table = m_tables.nodes();
Expand All @@ -673,6 +684,11 @@ void middle_pgsql_t::after_nodes()

void middle_pgsql_t::after_ways()
{
assert(m_middle_state == middle_state::way);
#ifndef NDEBUG
m_middle_state = middle_state::relation;
#endif

m_db_copy.sync();
if (!m_options->append) {
auto const &table = m_tables.ways();
Expand All @@ -682,6 +698,11 @@ void middle_pgsql_t::after_ways()

void middle_pgsql_t::after_relations()
{
assert(m_middle_state == middle_state::relation);
#ifndef NDEBUG
m_middle_state = middle_state::done;
#endif

m_db_copy.sync();
if (!m_options->append) {
auto const &table = m_tables.relations();
Expand All @@ -706,6 +727,11 @@ middle_query_pgsql_t::middle_query_pgsql_t(

void middle_pgsql_t::start()
{
assert(m_middle_state == middle_state::constructed);
#ifndef NDEBUG
m_middle_state = middle_state::node;
#endif

if (m_options->append) {
// Disable JIT and parallel workers as they are known to cause
// problems when accessing the intarrays.
Expand All @@ -732,6 +758,8 @@ void middle_pgsql_t::start()

void middle_pgsql_t::stop()
{
assert(m_middle_state == middle_state::done);

m_cache.reset();
if (!m_options->flat_node_file.empty()) {
m_persistent_cache.reset();
Expand Down
5 changes: 5 additions & 0 deletions src/middle-ram.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ void middle_ram_t::set_requirements(output_requirements const &requirements)

void middle_ram_t::stop()
{
assert(m_middle_state == middle_state::done);

auto const mbyte = 1024 * 1024;

log_debug("Middle 'ram': Node locations: size={} bytes={}M",
Expand Down Expand Up @@ -148,6 +150,7 @@ static void add_delta_encoded_way_node_list(std::string *data,

void middle_ram_t::node(osmium::Node const &node)
{
assert(m_middle_state == middle_state::node);
assert(node.visible());

if (m_store_options.locations) {
Expand All @@ -162,6 +165,7 @@ void middle_ram_t::node(osmium::Node const &node)

void middle_ram_t::way(osmium::Way const &way)
{
assert(m_middle_state == middle_state::way);
assert(way.visible());

if (m_store_options.way_nodes) {
Expand All @@ -177,6 +181,7 @@ void middle_ram_t::way(osmium::Way const &way)

void middle_ram_t::relation(osmium::Relation const &relation)
{
assert(m_middle_state == middle_state::relation);
assert(relation.visible());

if (m_store_options.relations) {
Expand Down
9 changes: 8 additions & 1 deletion src/middle-ram.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ class middle_ram_t : public middle_t, public middle_query_t

~middle_ram_t() noexcept override = default;

void start() override {}
void start() override
{
assert(m_middle_state == middle_state::constructed);
#ifndef NDEBUG
m_middle_state = middle_state::node;
#endif
}

void stop() override;

void node(osmium::Node const &node) override;
Expand Down
39 changes: 36 additions & 3 deletions src/middle.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,31 @@ class middle_t
virtual void relation(osmium::Relation const &relation) = 0;

/// Called after all nodes from the input file(s) have been processed.
virtual void after_nodes() {}
virtual void after_nodes()
{
assert(m_middle_state == middle_state::node);
#ifndef NDEBUG
m_middle_state = middle_state::way;
#endif
}

/// Called after all ways from the input file(s) have been processed.
virtual void after_ways() {}
virtual void after_ways()
{
assert(m_middle_state == middle_state::way);
#ifndef NDEBUG
m_middle_state = middle_state::relation;
#endif
}

/// Called after all relations from the input file(s) have been processed.
virtual void after_relations() {}
virtual void after_relations()
{
assert(m_middle_state == middle_state::relation);
#ifndef NDEBUG
m_middle_state = middle_state::done;
#endif
}

virtual idlist_t get_ways_by_node(osmid_t) { return {}; }
virtual idlist_t get_rels_by_node(osmid_t) { return {}; }
Expand All @@ -145,6 +163,21 @@ class middle_t
return *m_thread_pool;
}

#ifndef NDEBUG
enum class middle_state
{
constructed,
started,
node,
way,
relation,
done
};

// NOLINTNEXTLINE(cppcoreguidelines-non-private-member-variables-in-classes, misc-non-private-member-variables-in-classes)
middle_state m_middle_state = middle_state::constructed;
#endif

private:
std::shared_ptr<thread_pool_t> m_thread_pool;
}; // class middle_t
Expand Down
33 changes: 33 additions & 0 deletions tests/test-middle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ TEMPLATE_TEST_CASE("middle import", "", options_slim_default,
// set the node
mid->node(node);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

// getting it back works only via a waylist
auto &nodes = buffer.add_way("w3 Nn1234").nodes();
Expand Down Expand Up @@ -170,6 +172,7 @@ TEMPLATE_TEST_CASE("middle import", "", options_slim_default,
// set the way
mid->way(buffer.add_way(way_id, nds));
mid->after_ways();
mid->after_relations();

// get it back
osmium::memory::Buffer outbuf{4096,
Expand Down Expand Up @@ -372,6 +375,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
mid->node(node10);
mid->node(node11);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

check_node(mid, node10);
Expand Down Expand Up @@ -402,6 +406,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
mid->node(node10d);
mid->node(node42d);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE(no_node(mid, 5));
Expand Down Expand Up @@ -432,6 +437,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
mid->node(node12d);
mid->node(node12);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

check_node(mid, node10a);
Expand All @@ -457,6 +463,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",

mid->node(node12);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE(no_node(mid, 5));
Expand Down Expand Up @@ -585,6 +592,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way20);
mid->way(way21);
mid->after_ways();
Expand Down Expand Up @@ -614,6 +622,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way5d);
mid->way(way20d);
mid->way(way42d);
Expand Down Expand Up @@ -643,6 +652,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way20d);
mid->way(way20a);
mid->way(way22d);
Expand Down Expand Up @@ -675,6 +685,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way22);
mid->after_ways();
mid->after_relations();
Expand Down Expand Up @@ -728,6 +739,7 @@ TEMPLATE_TEST_CASE("middle: add way with attributes", "", options_slim_default,
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way20);
mid->after_ways();
mid->after_relations();
Expand Down Expand Up @@ -831,6 +843,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->after_ways();
mid->relation(relation30);
mid->relation(relation31);
mid->after_relations();
Expand Down Expand Up @@ -859,6 +873,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->after_ways();
mid->relation(relation5d);
mid->relation(relation30d);
mid->relation(relation42d);
Expand Down Expand Up @@ -887,6 +903,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->after_ways();
mid->relation(relation30d);
mid->relation(relation30a);
mid->relation(relation32d);
Expand Down Expand Up @@ -918,6 +936,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->after_ways();
mid->relation(relation32);
mid->after_relations();

Expand Down Expand Up @@ -967,6 +987,8 @@ TEMPLATE_TEST_CASE("middle: add relation with attributes", "",
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->after_ways();
mid->relation(relation30);
mid->after_relations();

Expand Down Expand Up @@ -1052,6 +1074,8 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
mid->node(node10a);
dependency_manager.node_changed(10);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE(dependency_manager.has_pending());
idlist_t const way_ids = dependency_manager.get_pending_way_ids();
Expand All @@ -1067,9 +1091,11 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way22);
mid->after_ways();
mid->after_relations();

check_way(mid, way22);
}
{
Expand All @@ -1081,6 +1107,8 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
mid->node(node10a);
dependency_manager.node_changed(10);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE(dependency_manager.has_pending());
idlist_t const way_ids = dependency_manager.get_pending_way_ids();
Expand All @@ -1099,6 +1127,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
mid->start();

mid->after_nodes();
mid->way(way20d);
mid->way(way20a);
mid->after_ways();
Expand All @@ -1117,6 +1146,8 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
mid->node(node10a);
dependency_manager.node_changed(10);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE_FALSE(dependency_manager.has_pending());
}
Expand Down Expand Up @@ -1179,6 +1210,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
mid->node(node10a);
dependency_manager.node_changed(10);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE(dependency_manager.has_pending());
Expand All @@ -1198,6 +1230,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
mid->node(node11a);
dependency_manager.node_changed(11);
mid->after_nodes();
mid->after_ways();
mid->after_relations();

REQUIRE(dependency_manager.has_pending());
Expand Down
Loading

0 comments on commit 2038358

Please sign in to comment.