Skip to content

Commit e404a65

Browse files
jotojoto
committed
Call middle::after_nodes/ways/relations() in tests in right order
The middle should always be called with these functions in the right order if the middle data is changed in any way, because there might be postprocessing done or buffers flushed etc. This adds some code that checks in the middle that it is in the correct state. This code is only compiled in in debug mode.
1 parent 9ab3f89 commit e404a65

File tree

6 files changed

+134
-6
lines changed

6 files changed

+134
-6
lines changed

‎src/middle-pgsql.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,8 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db(
347347

348348
void middle_pgsql_t::node(osmium::Node const &node)
349349
{
350+
assert(m_middle_state == middle_state::node);
351+
350352
if (node.deleted()) {
351353
node_delete(node.id());
352354
} else {
@@ -358,6 +360,8 @@ void middle_pgsql_t::node(osmium::Node const &node)
358360
}
359361

360362
void middle_pgsql_t::way(osmium::Way const &way) {
363+
assert(m_middle_state == middle_state::way);
364+
361365
if (way.deleted()) {
362366
way_delete(way.id());
363367
} else {
@@ -369,6 +373,8 @@ void middle_pgsql_t::way(osmium::Way const &way) {
369373
}
370374

371375
void middle_pgsql_t::relation(osmium::Relation const &relation) {
376+
assert(m_middle_state == middle_state::relation);
377+
372378
if (relation.deleted()) {
373379
relation_delete(relation.id());
374380
} else {
@@ -664,6 +670,11 @@ void middle_pgsql_t::relation_delete(osmid_t osm_id)
664670

665671
void middle_pgsql_t::after_nodes()
666672
{
673+
assert(m_middle_state == middle_state::node);
674+
#ifndef NDEBUG
675+
m_middle_state = middle_state::way;
676+
#endif
677+
667678
m_db_copy.sync();
668679
if (!m_options->append && m_options->flat_node_file.empty()) {
669680
auto const &table = m_tables.nodes();
@@ -673,6 +684,11 @@ void middle_pgsql_t::after_nodes()
673684

674685
void middle_pgsql_t::after_ways()
675686
{
687+
assert(m_middle_state == middle_state::way);
688+
#ifndef NDEBUG
689+
m_middle_state = middle_state::relation;
690+
#endif
691+
676692
m_db_copy.sync();
677693
if (!m_options->append) {
678694
auto const &table = m_tables.ways();
@@ -682,6 +698,11 @@ void middle_pgsql_t::after_ways()
682698

683699
void middle_pgsql_t::after_relations()
684700
{
701+
assert(m_middle_state == middle_state::relation);
702+
#ifndef NDEBUG
703+
m_middle_state = middle_state::done;
704+
#endif
705+
685706
m_db_copy.sync();
686707
if (!m_options->append) {
687708
auto const &table = m_tables.relations();
@@ -706,6 +727,11 @@ middle_query_pgsql_t::middle_query_pgsql_t(
706727

707728
void middle_pgsql_t::start()
708729
{
730+
assert(m_middle_state == middle_state::constructed);
731+
#ifndef NDEBUG
732+
m_middle_state = middle_state::node;
733+
#endif
734+
709735
if (m_options->append) {
710736
// Disable JIT and parallel workers as they are known to cause
711737
// problems when accessing the intarrays.
@@ -732,6 +758,8 @@ void middle_pgsql_t::start()
732758

733759
void middle_pgsql_t::stop()
734760
{
761+
assert(m_middle_state == middle_state::done);
762+
735763
m_cache.reset();
736764
if (!m_options->flat_node_file.empty()) {
737765
m_persistent_cache.reset();

‎src/middle-ram.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ void middle_ram_t::set_requirements(output_requirements const &requirements)
6161

6262
void middle_ram_t::stop()
6363
{
64+
assert(m_middle_state == middle_state::done);
65+
6466
auto const mbyte = 1024 * 1024;
6567

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

149151
void middle_ram_t::node(osmium::Node const &node)
150152
{
153+
assert(m_middle_state == middle_state::node);
151154
assert(node.visible());
152155

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

163166
void middle_ram_t::way(osmium::Way const &way)
164167
{
168+
assert(m_middle_state == middle_state::way);
165169
assert(way.visible());
166170

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

178182
void middle_ram_t::relation(osmium::Relation const &relation)
179183
{
184+
assert(m_middle_state == middle_state::relation);
180185
assert(relation.visible());
181186

182187
if (m_store_options.relations) {

‎src/middle-ram.hpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,14 @@ class middle_ram_t : public middle_t, public middle_query_t
4747

4848
~middle_ram_t() noexcept override = default;
4949

50-
void start() override {}
50+
void start() override
51+
{
52+
assert(m_middle_state == middle_state::constructed);
53+
#ifndef NDEBUG
54+
m_middle_state = middle_state::node;
55+
#endif
56+
}
57+
5158
void stop() override;
5259

5360
void node(osmium::Node const &node) override;

‎src/middle.hpp‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,31 @@ class middle_t
122122
virtual void relation(osmium::Relation const &relation) = 0;
123123

124124
/// Called after all nodes from the input file(s) have been processed.
125-
virtual void after_nodes() {}
125+
virtual void after_nodes()
126+
{
127+
assert(m_middle_state == middle_state::node);
128+
#ifndef NDEBUG
129+
m_middle_state = middle_state::way;
130+
#endif
131+
}
126132

127133
/// Called after all ways from the input file(s) have been processed.
128-
virtual void after_ways() {}
134+
virtual void after_ways()
135+
{
136+
assert(m_middle_state == middle_state::way);
137+
#ifndef NDEBUG
138+
m_middle_state = middle_state::relation;
139+
#endif
140+
}
129141

130142
/// Called after all relations from the input file(s) have been processed.
131-
virtual void after_relations() {}
143+
virtual void after_relations()
144+
{
145+
assert(m_middle_state == middle_state::relation);
146+
#ifndef NDEBUG
147+
m_middle_state = middle_state::done;
148+
#endif
149+
}
132150

133151
virtual idlist_t get_ways_by_node(osmid_t) { return {}; }
134152
virtual idlist_t get_rels_by_node(osmid_t) { return {}; }
@@ -145,6 +163,21 @@ class middle_t
145163
return *m_thread_pool;
146164
}
147165

166+
#ifndef NDEBUG
167+
enum class middle_state
168+
{
169+
constructed,
170+
started,
171+
node,
172+
way,
173+
relation,
174+
done
175+
};
176+
177+
// NOLINTNEXTLINE(cppcoreguidelines-non-private-member-variables-in-classes, misc-non-private-member-variables-in-classes)
178+
middle_state m_middle_state = middle_state::constructed;
179+
#endif
180+
148181
private:
149182
std::shared_ptr<thread_pool_t> m_thread_pool;
150183
}; // class middle_t

‎tests/test-middle.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ TEMPLATE_TEST_CASE("middle import", "", options_slim_default,
130130
// set the node
131131
mid->node(node);
132132
mid->after_nodes();
133+
mid->after_ways();
134+
mid->after_relations();
133135

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

174177
// get it back
175178
osmium::memory::Buffer outbuf{4096,
@@ -372,6 +375,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
372375
mid->node(node10);
373376
mid->node(node11);
374377
mid->after_nodes();
378+
mid->after_ways();
375379
mid->after_relations();
376380

377381
check_node(mid, node10);
@@ -402,6 +406,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
402406
mid->node(node10d);
403407
mid->node(node42d);
404408
mid->after_nodes();
409+
mid->after_ways();
405410
mid->after_relations();
406411

407412
REQUIRE(no_node(mid, 5));
@@ -432,6 +437,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
432437
mid->node(node12d);
433438
mid->node(node12);
434439
mid->after_nodes();
440+
mid->after_ways();
435441
mid->after_relations();
436442

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

458464
mid->node(node12);
459465
mid->after_nodes();
466+
mid->after_ways();
460467
mid->after_relations();
461468

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

595+
mid->after_nodes();
588596
mid->way(way20);
589597
mid->way(way21);
590598
mid->after_ways();
@@ -614,6 +622,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
614622
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
615623
mid->start();
616624

625+
mid->after_nodes();
617626
mid->way(way5d);
618627
mid->way(way20d);
619628
mid->way(way42d);
@@ -643,6 +652,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
643652
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
644653
mid->start();
645654

655+
mid->after_nodes();
646656
mid->way(way20d);
647657
mid->way(way20a);
648658
mid->way(way22d);
@@ -675,6 +685,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
675685
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
676686
mid->start();
677687

688+
mid->after_nodes();
678689
mid->way(way22);
679690
mid->after_ways();
680691
mid->after_relations();
@@ -728,6 +739,7 @@ TEMPLATE_TEST_CASE("middle: add way with attributes", "", options_slim_default,
728739
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
729740
mid->start();
730741

742+
mid->after_nodes();
731743
mid->way(way20);
732744
mid->after_ways();
733745
mid->after_relations();
@@ -831,6 +843,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
831843
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
832844
mid->start();
833845

846+
mid->after_nodes();
847+
mid->after_ways();
834848
mid->relation(relation30);
835849
mid->relation(relation31);
836850
mid->after_relations();
@@ -859,6 +873,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
859873
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
860874
mid->start();
861875

876+
mid->after_nodes();
877+
mid->after_ways();
862878
mid->relation(relation5d);
863879
mid->relation(relation30d);
864880
mid->relation(relation42d);
@@ -887,6 +903,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
887903
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
888904
mid->start();
889905

906+
mid->after_nodes();
907+
mid->after_ways();
890908
mid->relation(relation30d);
891909
mid->relation(relation30a);
892910
mid->relation(relation32d);
@@ -918,6 +936,8 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
918936
auto mid = std::make_shared<middle_pgsql_t>(thread_pool, &options);
919937
mid->start();
920938

939+
mid->after_nodes();
940+
mid->after_ways();
921941
mid->relation(relation32);
922942
mid->after_relations();
923943

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

990+
mid->after_nodes();
991+
mid->after_ways();
970992
mid->relation(relation30);
971993
mid->after_relations();
972994

@@ -1052,6 +1074,8 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
10521074
mid->node(node10a);
10531075
dependency_manager.node_changed(10);
10541076
mid->after_nodes();
1077+
mid->after_ways();
1078+
mid->after_relations();
10551079

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

1094+
mid->after_nodes();
10701095
mid->way(way22);
10711096
mid->after_ways();
10721097
mid->after_relations();
1098+
10731099
check_way(mid, way22);
10741100
}
10751101
{
@@ -1081,6 +1107,8 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
10811107
mid->node(node10a);
10821108
dependency_manager.node_changed(10);
10831109
mid->after_nodes();
1110+
mid->after_ways();
1111+
mid->after_relations();
10841112

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

1130+
mid->after_nodes();
11021131
mid->way(way20d);
11031132
mid->way(way20a);
11041133
mid->after_ways();
@@ -1117,6 +1146,8 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
11171146
mid->node(node10a);
11181147
dependency_manager.node_changed(10);
11191148
mid->after_nodes();
1149+
mid->after_ways();
1150+
mid->after_relations();
11201151

11211152
REQUIRE_FALSE(dependency_manager.has_pending());
11221153
}
@@ -1179,6 +1210,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
11791210
mid->node(node10a);
11801211
dependency_manager.node_changed(10);
11811212
mid->after_nodes();
1213+
mid->after_ways();
11821214
mid->after_relations();
11831215

11841216
REQUIRE(dependency_manager.has_pending());
@@ -1198,6 +1230,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
11981230
mid->node(node11a);
11991231
dependency_manager.node_changed(11);
12001232
mid->after_nodes();
1233+
mid->after_ways();
12011234
mid->after_relations();
12021235

12031236
REQUIRE(dependency_manager.has_pending());

0 commit comments

Comments
 (0)