Project-OSRM · TheMarex · Jul 28, 2017 · Jul 27, 2017
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,8 @@
       - Reorders arguments to `WayHandlers` functions to match `process_way()`.
       - Profiles must return a hash of profile functions. This makes it easier for profiles to include each other.
       - Guidance: add support for throughabouts
+    - Bugfixes
+      - Properly save/retrieve datasource annotations for road segments ([#4346](https://github.com/Project-OSRM/osrm-backend/issues/4346))
 
 # 5.9.2
     - API:

diff --git a/features/testbot/annotations.feature b/features/testbot/annotations.feature
@@ -1,11 +1,9 @@
-@routing @speed @annotations @turn_penalty
+@routing @speed @annotations
 Feature: Annotations
 
-    Background:
+    Scenario: Ensure that turn penalties aren't included in annotations
         Given the profile "turnbot"
         Given a grid size of 100 meters
-
-    Scenario: Ensure that turn penalties aren't included in annotations
         Given the node map
             """
               h i
@@ -27,4 +25,37 @@ Feature: Annotations
             | from | to | route    | a:speed     | a:weight |
             | h    | j  | hk,jk,jk | 6.7:6.7     | 15:15    |
             | i    | m  | il,lm,lm | 6.7:6.7     | 15:15    |
-            | j    | m  | jk,lm    | 6.7:6.7:6.7 | 15:15:15 |
+            | j    | m  | jk,lm    | 6.7:6.7:6.7 | 15:15:15 |
+
+
+    Scenario: There should be different forward/reverse datasources
+        Given the profile "testbot"
+
+        And the node map
+            """
+            a b c d e f g h i
+            """
+
+        And the ways
+            | nodes     | highway |
+            | abcdefghi | primary |
+
+        And the contract extra arguments "--segment-speed-file {speeds_file}"
+        And the customize extra arguments "--segment-speed-file {speeds_file}"
+
+        # Note: 180km/h == 50m/s for speed annotations
+        And the speed file
+        """
+        1,2,180,1
+        2,1,180,1
+        3,4,180,1
+        5,6,180,1
+        8,7,180,1
+        """
+        And the query options
+          | annotations | datasources,speed |
+
+        When I route I should get
+          | from | to | route               | a:datasources   | a:speed                 |
+          | a    | i  | abcdefghi,abcdefghi | 1:0:1:0:1:0:0:0 | 50:10:50:10:50:10:10:10 |
+          | i    | a  | abcdefghi,abcdefghi | 0:1:0:0:0:0:0:1 | 10:50:10:10:10:10:10:50 |
diff --git a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp
@@ -480,18 +480,26 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade
                 data_layout.num_entries[storage::DataLayout::GEOMETRIES_REV_DURATION_LIST]),
             num_entries);
 
-        auto datasources_list_ptr = data_layout.GetBlockPtr<DatasourceID>(
-            memory_block, storage::DataLayout::DATASOURCES_LIST);
-        util::vector_view<DatasourceID> datasources_list(
-            datasources_list_ptr, data_layout.num_entries[storage::DataLayout::DATASOURCES_LIST]);
+        auto geometries_fwd_datasources_list_ptr = data_layout.GetBlockPtr<DatasourceID>(
+            memory_block, storage::DataLayout::GEOMETRIES_FWD_DATASOURCES_LIST);
+        util::vector_view<DatasourceID> geometry_fwd_datasources_list(
+            geometries_fwd_datasources_list_ptr,
+            data_layout.num_entries[storage::DataLayout::GEOMETRIES_FWD_DATASOURCES_LIST]);
+
+        auto geometries_rev_datasources_list_ptr = data_layout.GetBlockPtr<DatasourceID>(
+            memory_block, storage::DataLayout::GEOMETRIES_REV_DATASOURCES_LIST);
+        util::vector_view<DatasourceID> geometry_rev_datasources_list(
+            geometries_rev_datasources_list_ptr,
+            data_layout.num_entries[storage::DataLayout::GEOMETRIES_REV_DATASOURCES_LIST]);
 
         segment_data = extractor::SegmentDataView{std::move(geometry_begin_indices),
                                                   std::move(geometry_node_list),
                                                   std::move(geometry_fwd_weight_list),
                                                   std::move(geometry_rev_weight_list),
                                                   std::move(geometry_fwd_duration_list),
                                                   std::move(geometry_rev_duration_list),
-                                                  std::move(datasources_list)};
+                                                  std::move(geometry_fwd_datasources_list),
+                                                  std::move(geometry_rev_datasources_list)};
 
         m_datasources = data_layout.GetBlockPtr<extractor::Datasources>(
             memory_block, storage::DataLayout::DATASOURCES_NAMES);

diff --git a/include/extractor/segment_data_container.hpp b/include/extractor/segment_data_container.hpp
@@ -55,6 +55,7 @@ template <storage::Ownership Ownership> class SegmentDataContainerImpl
     using SegmentOffset = std::uint32_t;
     using SegmentWeightVector = PackedVector<SegmentWeight, SEGMENT_WEIGHT_BITS>;
     using SegmentDurationVector = PackedVector<SegmentDuration, SEGMENT_DURAITON_BITS>;
+    using SegmentDatasourceVector = Vector<DatasourceID>;
 
     SegmentDataContainerImpl() = default;
 
@@ -64,10 +65,12 @@ template <storage::Ownership Ownership> class SegmentDataContainerImpl
                              SegmentWeightVector rev_weights_,
                              SegmentDurationVector fwd_durations_,
                              SegmentDurationVector rev_durations_,
-                             Vector<DatasourceID> datasources_)
+                             SegmentDatasourceVector fwd_datasources_,
+                             SegmentDatasourceVector rev_datasources_)
         : index(std::move(index_)), nodes(std::move(nodes_)), fwd_weights(std::move(fwd_weights_)),
           rev_weights(std::move(rev_weights_)), fwd_durations(std::move(fwd_durations_)),
-          rev_durations(std::move(rev_durations_)), datasources(std::move(datasources_))
+          rev_durations(std::move(rev_durations_)), fwd_datasources(std::move(fwd_datasources_)),
+          rev_datasources(std::move(rev_datasources_))
     {
     }
 
@@ -118,16 +121,16 @@ template <storage::Ownership Ownership> class SegmentDataContainerImpl
 
     auto GetForwardDatasources(const DirectionalGeometryID id)
     {
-        const auto begin = datasources.begin() + index[id] + 1;
-        const auto end = datasources.begin() + index[id + 1];
+        const auto begin = fwd_datasources.begin() + index[id] + 1;
+        const auto end = fwd_datasources.begin() + index[id + 1];
 
         return boost::make_iterator_range(begin, end);
     }
 
     auto GetReverseDatasources(const DirectionalGeometryID id)
     {
-        const auto begin = datasources.begin() + index[id];
-        const auto end = datasources.begin() + index[id + 1] - 1;
+        const auto begin = rev_datasources.begin() + index[id];
+        const auto end = rev_datasources.begin() + index[id + 1] - 1;
 
         return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
     }
@@ -179,16 +182,16 @@ template <storage::Ownership Ownership> class SegmentDataContainerImpl
 
     auto GetForwardDatasources(const DirectionalGeometryID id) const
     {
-        const auto begin = datasources.cbegin() + index[id] + 1;
-        const auto end = datasources.cbegin() + index[id + 1];
+        const auto begin = fwd_datasources.cbegin() + index[id] + 1;
+        const auto end = fwd_datasources.cbegin() + index[id + 1];
 
         return boost::make_iterator_range(begin, end);
     }
 
     auto GetReverseDatasources(const DirectionalGeometryID id) const
     {
-        const auto begin = datasources.cbegin() + index[id];
-        const auto end = datasources.cbegin() + index[id + 1] - 1;
+        const auto begin = rev_datasources.cbegin() + index[id];
+        const auto end = rev_datasources.cbegin() + index[id + 1] - 1;
 
         return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
     }
@@ -210,7 +213,8 @@ template <storage::Ownership Ownership> class SegmentDataContainerImpl
     SegmentWeightVector rev_weights;
     SegmentDurationVector fwd_durations;
     SegmentDurationVector rev_durations;
-    Vector<DatasourceID> datasources;
+    SegmentDatasourceVector fwd_datasources;
+    SegmentDatasourceVector rev_datasources;
 };
 }
 

diff --git a/include/extractor/serialization.hpp b/include/extractor/serialization.hpp
@@ -74,7 +74,8 @@ inline void read(storage::io::FileReader &reader,
     util::serialization::read(reader, segment_data.rev_weights);
     util::serialization::read(reader, segment_data.fwd_durations);
     util::serialization::read(reader, segment_data.rev_durations);
-    storage::serialization::read(reader, segment_data.datasources);
+    storage::serialization::read(reader, segment_data.fwd_datasources);
+    storage::serialization::read(reader, segment_data.rev_datasources);
 }
 
 template <storage::Ownership Ownership>
@@ -87,7 +88,8 @@ inline void write(storage::io::FileWriter &writer,
     util::serialization::write(writer, segment_data.rev_weights);
     util::serialization::write(writer, segment_data.fwd_durations);
     util::serialization::write(writer, segment_data.rev_durations);
-    storage::serialization::write(writer, segment_data.datasources);
+    storage::serialization::write(writer, segment_data.fwd_datasources);
+    storage::serialization::write(writer, segment_data.rev_datasources);
 }
 
 // read/write for turn data file

diff --git a/include/storage/shared_datatype.hpp b/include/storage/shared_datatype.hpp
@@ -38,11 +38,12 @@ const constexpr char *block_id_to_name[] = {"NAME_CHAR_DATA",
                                             "GEOMETRIES_REV_WEIGHT_LIST",
                                             "GEOMETRIES_FWD_DURATION_LIST",
                                             "GEOMETRIES_REV_DURATION_LIST",
+                                            "GEOMETRIES_FWD_DATASOURCES_LIST",
+                                            "GEOMETRIES_REV_DATASOURCES_LIST",
                                             "HSGR_CHECKSUM",
                                             "TIMESTAMP",
                                             "FILE_INDEX_PATH",
                                             "CH_CORE_MARKER",
-                                            "DATASOURCES_LIST",
                                             "DATASOURCES_NAMES",
                                             "PROPERTIES",
                                             "BEARING_CLASSID",
@@ -95,11 +96,12 @@ struct DataLayout
         GEOMETRIES_REV_WEIGHT_LIST,
         GEOMETRIES_FWD_DURATION_LIST,
         GEOMETRIES_REV_DURATION_LIST,
+        GEOMETRIES_FWD_DATASOURCES_LIST,
+        GEOMETRIES_REV_DATASOURCES_LIST,
         HSGR_CHECKSUM,
         TIMESTAMP,
         FILE_INDEX_PATH,
         CH_CORE_MARKER,
-        DATASOURCES_LIST,
         DATASOURCES_NAMES,
         PROPERTIES,
         BEARING_CLASSID,

diff --git a/src/extractor/compressed_edge_container.cpp b/src/extractor/compressed_edge_container.cpp
@@ -245,7 +245,8 @@ void CompressedEdgeContainer::InitializeBothwayVector()
     segment_data->rev_weights.reserve(m_compressed_oneway_geometries.size());
     segment_data->fwd_durations.reserve(m_compressed_oneway_geometries.size());
     segment_data->rev_durations.reserve(m_compressed_oneway_geometries.size());
-    segment_data->datasources.reserve(m_compressed_oneway_geometries.size());
+    segment_data->fwd_datasources.reserve(m_compressed_oneway_geometries.size());
+    segment_data->rev_datasources.reserve(m_compressed_oneway_geometries.size());
 }
 
 unsigned CompressedEdgeContainer::ZipEdges(const EdgeID f_edge_id, const EdgeID r_edge_id)
@@ -270,7 +271,8 @@ unsigned CompressedEdgeContainer::ZipEdges(const EdgeID f_edge_id, const EdgeID
     segment_data->rev_weights.emplace_back(first_node.weight);
     segment_data->fwd_durations.emplace_back(INVALID_SEGMENT_DURATION);
     segment_data->rev_durations.emplace_back(first_node.duration);
-    segment_data->datasources.emplace_back(LUA_SOURCE);
+    segment_data->fwd_datasources.emplace_back(LUA_SOURCE);
+    segment_data->rev_datasources.emplace_back(LUA_SOURCE);
 
     for (std::size_t i = 0; i < forward_bucket.size() - 1; ++i)
     {
@@ -284,7 +286,8 @@ unsigned CompressedEdgeContainer::ZipEdges(const EdgeID f_edge_id, const EdgeID
         segment_data->rev_weights.emplace_back(rev_node.weight);
         segment_data->fwd_durations.emplace_back(fwd_node.duration);
         segment_data->rev_durations.emplace_back(rev_node.duration);
-        segment_data->datasources.emplace_back(LUA_SOURCE);
+        segment_data->fwd_datasources.emplace_back(LUA_SOURCE);
+        segment_data->rev_datasources.emplace_back(LUA_SOURCE);
     }
 
     const auto &last_node = forward_bucket.back();
@@ -294,7 +297,8 @@ unsigned CompressedEdgeContainer::ZipEdges(const EdgeID f_edge_id, const EdgeID
     segment_data->rev_weights.emplace_back(INVALID_SEGMENT_WEIGHT);
     segment_data->fwd_durations.emplace_back(last_node.duration);
     segment_data->rev_durations.emplace_back(INVALID_SEGMENT_DURATION);
-    segment_data->datasources.emplace_back(LUA_SOURCE);
+    segment_data->fwd_datasources.emplace_back(LUA_SOURCE);
+    segment_data->rev_datasources.emplace_back(LUA_SOURCE);
 
     return zipped_geometry_id;
 }

diff --git a/src/storage/storage.cpp b/src/storage/storage.cpp
@@ -383,7 +383,9 @@ void Storage::PopulateLayout(DataLayout &layout)
             DataLayout::GEOMETRIES_FWD_DURATION_LIST, number_of_segment_duration_blocks);
         layout.SetBlockSize<extractor::SegmentDataView::SegmentDurationVector::block_type>(
             DataLayout::GEOMETRIES_REV_DURATION_LIST, number_of_segment_duration_blocks);
-        layout.SetBlockSize<DatasourceID>(DataLayout::DATASOURCES_LIST,
+        layout.SetBlockSize<DatasourceID>(DataLayout::GEOMETRIES_FWD_DATASOURCES_LIST,
+                                          number_of_compressed_geometries);
+        layout.SetBlockSize<DatasourceID>(DataLayout::GEOMETRIES_REV_DATASOURCES_LIST,
                                           number_of_compressed_geometries);
     }
 
@@ -720,18 +722,26 @@ void Storage::PopulateData(const DataLayout &layout, char *memory_ptr)
                 layout.num_entries[storage::DataLayout::GEOMETRIES_REV_DURATION_LIST]),
             num_entries);
 
-        auto datasources_list_ptr = layout.GetBlockPtr<DatasourceID, true>(
-            memory_ptr, storage::DataLayout::DATASOURCES_LIST);
-        util::vector_view<DatasourceID> datasources_list(
-            datasources_list_ptr, layout.num_entries[storage::DataLayout::DATASOURCES_LIST]);
+        auto geometries_fwd_datasources_list_ptr = layout.GetBlockPtr<DatasourceID, true>(
+            memory_ptr, storage::DataLayout::GEOMETRIES_FWD_DATASOURCES_LIST);
+        util::vector_view<DatasourceID> geometry_fwd_datasources_list(
+            geometries_fwd_datasources_list_ptr,
+            layout.num_entries[storage::DataLayout::GEOMETRIES_FWD_DATASOURCES_LIST]);
+
+        auto geometries_rev_datasources_list_ptr = layout.GetBlockPtr<DatasourceID, true>(
+            memory_ptr, storage::DataLayout::GEOMETRIES_REV_DATASOURCES_LIST);
+        util::vector_view<DatasourceID> geometry_rev_datasources_list(
+            geometries_rev_datasources_list_ptr,
+            layout.num_entries[storage::DataLayout::GEOMETRIES_REV_DATASOURCES_LIST]);
 
         extractor::SegmentDataView segment_data{std::move(geometry_begin_indices),
                                                 std::move(geometry_node_list),
                                                 std::move(geometry_fwd_weight_list),
                                                 std::move(geometry_rev_weight_list),
                                                 std::move(geometry_fwd_duration_list),
                                                 std::move(geometry_rev_duration_list),
-                                                std::move(datasources_list)};
+                                                std::move(geometry_fwd_datasources_list),
+                                                std::move(geometry_rev_datasources_list)};
 
         extractor::files::readSegmentData(config.GetPath(".osrm.geometry"), segment_data);
     }

diff --git a/src/updater/updater.cpp b/src/updater/updater.cpp
@@ -320,7 +320,7 @@ updateSegmentData(const UpdaterConfig &config,
             auto new_fwd_datasources_range = segment_data.GetForwardDatasources(geometry_id);
             auto new_rev_durations_range =
                 boost::adaptors::reverse(segment_data.GetReverseDurations(geometry_id));
-            auto new_rev_datasources_range = segment_data.GetForwardDatasources(geometry_id);
+            auto new_rev_datasources_range = segment_data.GetReverseDatasources(geometry_id);
             auto old_fwd_durations_range = segment_data_backup->GetForwardDurations(geometry_id);
             auto old_rev_durations_range =
                 boost::adaptors::reverse(segment_data_backup->GetReverseDurations(geometry_id));