[Tile-serving] [openstreetmap/osm2pgsql] Middle refactorings and tests (#1110)

[Sarah Hoffmann]

@lonvia commented on this pull request.



> @@ -78,6 +79,17 @@ struct options_slim_dense_cache : options_slim_default
     }
 };
 
+struct options_flat_node_cache : options_slim_default
+{
+    static options_t options(pg::tempdb_t const &tmpdb)
+    {
+        options_t o = options_slim_default::options(tmpdb);
+        o.flat_node_cache_enabled = true;
+        o.flat_node_file = "flat-node.dat";

Use o.flatnodes()

or more concise:

return options_slim_default::options(tmpdb).flatnodes();

> @@ -232,7 +232,9 @@ class taglist_t
 
 struct idlist_t : public std::vector<osmid_t>
 {
-    idlist_t() {}
+    using vector<osmid_t>::vector;
+
+    idlist_t() = default;

This deserves a comment.

> +    REQUIRE(cache.get(23) == osmium::Location{0, 1});
+    REQUIRE(cache.get(42) == osmium::Location{4, 2});
+
+    cache.remove(17);
+    REQUIRE_FALSE(cache.get(17).valid());
+}
+
+TEMPLATE_TEST_CASE("Out of order access to sparse cache throws", "[NoDB]",
+                   strategy_sparse)
+{
+    node_ram_cache cache{(int)TestType::value, 10};
+
+    cache.set(17, osmium::Location{1, 7});
+    cache.set(42, osmium::Location{2, 3});
+    REQUIRE_THROWS_WITH(cache.set(23, osmium::Location{2, 3}),
+                        "Node ids are out of order. Please use slim mode.");

What is the difference with the test in line 83?

> +    REQUIRE(cache.get(42) == osmium::Location{2, 3});
+
+    cache.set(42, osmium::Location{4, 2});
+    cache.set(23, osmium::Location{});
+
+    REQUIRE(cache.get(17) == osmium::Location{1, 7});
+    REQUIRE_FALSE(cache.get(23).valid());
+    REQUIRE(cache.get(42) == osmium::Location{4, 2});
+
+    cache.set(23, osmium::Location{0, 1});
+    REQUIRE(cache.get(17) == osmium::Location{1, 7});
+    REQUIRE(cache.get(23) == osmium::Location{0, 1});
+    REQUIRE(cache.get(42) == osmium::Location{4, 2});
+
+    cache.remove(17);
+    REQUIRE_FALSE(cache.get(17).valid());

The ram node cache is a write-once affair, no deletes, no changes.

> @@ -59,6 +65,9 @@ class node_ram_cache
     ~node_ram_cache();
 
     void set(osmid_t id, osmium::Location location);
+
+    void remove(osmid_t id) { set(id, osmium::Location{}); }

It is intentional that the node_ram_cache has no remove().

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/osm2pgsql/pull/1110#pullrequestreview-390333328
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20200409/841fc20a/attachment.htm>