From 65acbae8089fc1426dc55d4e4058fa67dc6a662d Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Tue, 22 Jan 2019 18:54:43 +0300 Subject: [PATCH] review fixes --- docs/http.md | 2 +- include/engine/api/route_api.hpp | 6 +++++- .../contiguous_internalmem_datafacade.hpp | 13 +++---------- include/extractor/extractor_config.hpp | 1 + include/storage/serialization.hpp | 12 ------------ include/storage/view_factory.hpp | 2 +- include/util/typedefs.hpp | 2 +- src/extractor/extractor.cpp | 14 ++++++++------ src/storage/storage.cpp | 8 ++++++-- src/tools/extract.cpp | 4 ++++ unit_tests/engine/offline_facade.cpp | 2 +- unit_tests/library/route.cpp | 1 - unit_tests/mocks/mock_datafacade.hpp | 2 +- 13 files changed, 32 insertions(+), 37 deletions(-) diff --git a/docs/http.md b/docs/http.md index f3ad7c463..7caf2509c 100644 --- a/docs/http.md +++ b/docs/http.md @@ -91,7 +91,7 @@ Every response object has a `code` property containing one of the strings below #### Data version -Every response object has a `data_version` propetry containing timestamp from the original OpenStreetMap file. May be `n/a` if original OSM file has not `osmosis_replication_timestamp` section. +Every response object has a `data_version` propetry containing timestamp from the original OpenStreetMap file. This field is optional. It can be ommited if data_version parametr was not set on osrm-extract stage or OSM file has not `osmosis_replication_timestamp` section. #### Example response diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index e5f537e0e..5e8040507 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -68,7 +68,11 @@ class RouteAPI : public BaseAPI response.values["waypoints"] = BaseAPI::MakeWaypoints(all_start_end_points); response.values["routes"] = std::move(jsRoutes); response.values["code"] = "Ok"; - response.values["data_version"] = facade.GetTimestamp(); + auto data_timestamp = facade.GetTimestamp(); + if (!data_timestamp.empty()) + { + response.values["data_version"] = facade.GetTimestamp(); + } } protected: diff --git a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp index 66cfad502..9bbf0f859 100644 --- a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp +++ b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp @@ -137,7 +137,7 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade extractor::Datasources *m_datasources; std::uint32_t m_check_sum; - DataTimestamp m_data_timestamp; + StringView m_data_timestamp; util::vector_view m_coordinate_list; extractor::PackedOSMIDsView m_osmnodeid_list; util::vector_view m_lane_description_offsets; @@ -184,7 +184,7 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade m_check_sum = *index.GetBlockPtr("/common/connectivity_checksum"); - m_data_timestamp = *index.GetBlockPtr("/common/timestamp"); + m_data_timestamp = make_timestamp_view(index, "/common/timestamp"); std::tie(m_coordinate_list, m_osmnodeid_list) = make_nbn_data_view(index, "/common/nbn_data"); @@ -437,14 +437,7 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade std::string GetTimestamp() const override final { - if (!m_data_timestamp) - { - return "n/a"; - } - time_t ts(m_data_timestamp); - char buf[21]; // sizeof "2019-01-01T01:01:01Z\n"]; - strftime(buf, 21, "%FT%TZ", gmtime(&ts)); - return std::string(buf, 20); + return std::string(m_data_timestamp.begin(), m_data_timestamp.end()); } GeometryID GetGeometryIndex(const NodeID id) const override final diff --git a/include/extractor/extractor_config.hpp b/include/extractor/extractor_config.hpp index 2b6ae064e..1ca72a7dd 100644 --- a/include/extractor/extractor_config.hpp +++ b/include/extractor/extractor_config.hpp @@ -83,6 +83,7 @@ struct ExtractorConfig final : storage::IOConfig boost::filesystem::path input_path; boost::filesystem::path profile_path; std::vector location_dependent_data_paths; + std::string data_version; unsigned requested_num_threads; unsigned small_component_size; diff --git a/include/storage/serialization.hpp b/include/storage/serialization.hpp index ee50b361e..97cba94ef 100644 --- a/include/storage/serialization.hpp +++ b/include/storage/serialization.hpp @@ -201,18 +201,6 @@ inline void read(tar::FileReader &reader, const std::string &name, std::string & reader.ReadInto(name, const_cast(data.data()), count); } -inline void write(tar::FileWriter &writer, const std::string &name, const std::uint64_t &data) -{ - writer.WriteElementCount64(name, data); - writer.WriteFrom(name, &data, 1); -} - -inline void read(tar::FileReader &reader, const std::string &name, std::uint64_t &data) -{ - data = reader.ReadElementCount64(name); - reader.ReadInto(name, &data, 1); -} - template inline void read(tar::FileReader &reader, const std::string &name, std::vector &data) { diff --git a/include/storage/view_factory.hpp b/include/storage/view_factory.hpp index 659c4786c..fb45b57d6 100644 --- a/include/storage/view_factory.hpp +++ b/include/storage/view_factory.hpp @@ -274,7 +274,7 @@ inline auto make_partition_view(const SharedDataIndex &index, const std::string inline auto make_timestamp_view(const SharedDataIndex &index, const std::string &name) { - return index.template GetBlockPtr(name); + return util::StringView(index.GetBlockPtr(name), index.GetBlockEntries(name)); } inline auto make_cell_storage_view(const SharedDataIndex &index, const std::string &name) diff --git a/include/util/typedefs.hpp b/include/util/typedefs.hpp index 540645200..f29406d8b 100644 --- a/include/util/typedefs.hpp +++ b/include/util/typedefs.hpp @@ -79,7 +79,7 @@ using EdgeDistance = float; using SegmentWeight = std::uint32_t; using SegmentDuration = std::uint32_t; using TurnPenalty = std::int16_t; // turn penalty in 100ms units -using DataTimestamp = std::uint64_t; +using DataTimestamp = std::string; static const std::size_t INVALID_INDEX = std::numeric_limits::max(); diff --git a/src/extractor/extractor.cpp b/src/extractor/extractor.cpp index 1de4c8889..36f137293 100644 --- a/src/extractor/extractor.cpp +++ b/src/extractor/extractor.cpp @@ -426,17 +426,19 @@ Extractor::ParseOSMData(ScriptingEnvironment &scripting_environment, // write .timestamp data file std::string timestamp = header.get("osmosis_replication_timestamp"); - osmium::Timestamp ts; + if (config.data_version == "osmosis") + { + files::writeTimestamp(config.GetPath(".osrm.timestamp").string(), timestamp); + } + else + { + files::writeTimestamp(config.GetPath(".osrm.timestamp").string(), config.data_version); + } if (timestamp.empty()) { timestamp = "n/a"; } - else - { - ts = osmium::Timestamp(timestamp); - } util::Log() << "timestamp: " << timestamp; - files::writeTimestamp(config.GetPath(".osrm.timestamp").string(), DataTimestamp(ts)); } // Extraction containers and restriction parser diff --git a/src/storage/storage.cpp b/src/storage/storage.cpp index 832f7b0d7..35b59c896 100644 --- a/src/storage/storage.cpp +++ b/src/storage/storage.cpp @@ -404,8 +404,12 @@ void Storage::PopulateStaticData(const SharedDataIndex &index) // Timestamp mark { - auto timestamp = make_timestamp_view(index, "/common/timestamp"); - extractor::files::readTimestamp(config.GetPath(".osrm.timestamp"), *timestamp); + auto timestamp_ref = make_timestamp_view(index, "/common/timestamp"); + std::string ts; + extractor::files::readTimestamp(config.GetPath(".osrm.timestamp"), ts); + if (!ts.empty()) { + memcpy(const_cast(timestamp_ref.data()), ts.data(), ts.size()); + } } // Turn lane data diff --git a/src/tools/extract.cpp b/src/tools/extract.cpp index bca3ef8d1..f44744bfe 100644 --- a/src/tools/extract.cpp +++ b/src/tools/extract.cpp @@ -43,6 +43,10 @@ return_code parseArguments(int argc, boost::program_options::value(&extractor_config.profile_path) ->default_value("profiles/car.lua"), "Path to LUA routing profile")( + "data_version,d", + boost::program_options::value(&extractor_config.data_version) + ->default_value(""), + "Data version. Leave blank to avoid. osmosis - to get timestamp from file")( "threads,t", boost::program_options::value(&extractor_config.requested_num_threads) ->default_value(tbb::task_scheduler_init::default_num_threads()), diff --git a/unit_tests/engine/offline_facade.cpp b/unit_tests/engine/offline_facade.cpp index 001140f98..dab7e2327 100644 --- a/unit_tests/engine/offline_facade.cpp +++ b/unit_tests/engine/offline_facade.cpp @@ -341,7 +341,7 @@ class ContiguousInternalMemoryDataFacade StringView GetDestinationsForID(const NameID /*id*/) const override { return StringView{}; } StringView GetExitsForID(const NameID /*id*/) const override { return StringView{}; } bool GetContinueStraightDefault() const override { return false; } - std::string GetTimestamp() const override { return "n/a"; } + std::string GetTimestamp() const override { return ""; } double GetMapMatchingMaxSpeed() const override { return 0; } const char *GetWeightName() const override { return ""; } unsigned GetWeightPrecision() const override { return 0; } diff --git a/unit_tests/library/route.cpp b/unit_tests/library/route.cpp index 7bb148745..6a6faad96 100644 --- a/unit_tests/library/route.cpp +++ b/unit_tests/library/route.cpp @@ -47,7 +47,6 @@ BOOST_AUTO_TEST_CASE(test_route_same_coordinates_fixture) json::Object reference{ {{"code", "Ok"}, - {"data_version", "2016-03-05T00:26:02Z"}, {"waypoints", json::Array{{json::Object{{{"name", "Boulevard du Larvotto"}, {"location", location}, diff --git a/unit_tests/mocks/mock_datafacade.hpp b/unit_tests/mocks/mock_datafacade.hpp index 07ff051bb..efea099d0 100644 --- a/unit_tests/mocks/mock_datafacade.hpp +++ b/unit_tests/mocks/mock_datafacade.hpp @@ -53,7 +53,7 @@ class MockBaseDataFacade : public engine::datafacade::BaseDataFacade { return 0; } - std::string GetTimestamp() const override { return "n/a"; } + std::string GetTimestamp() const override { return ""; } NodeForwardRange GetUncompressedForwardGeometry(const EdgeID /* id */) const override { static NodeID data[] = {0, 1, 2, 3};