- output only get escaped when actually output. Better seperation of functionality

- refactor facade::GetEscapeName() into get_name_for_id() call that is implemented in subclasses
- remove dead code
- fix failing tests where names got double-escaped
- fixes https://github.com/Project-OSRM/node-osrm/issues/83
This commit is contained in:
Dennis Luxen 2015-02-26 10:11:33 +01:00
parent 37fb89c691
commit 51e42ded44
9 changed files with 50 additions and 84 deletions

View File

@ -1,6 +1,6 @@
/*
Copyright (c) 2013, Project OSRM contributors
Copyright (c) 2015, Project OSRM contributors
All rights reserved.
Redistribution and use in source and binary forms, with or without modification,
@ -147,15 +147,13 @@ template <class DataFacadeT, class SegmentT> struct ExtractRouteNames
}
// fetching names for the selected segments
route_names.shortest_path_name_1 =
facade->GetEscapedNameForNameID(shortest_segment_1.name_id);
route_names.shortest_path_name_2 =
facade->GetEscapedNameForNameID(shortest_segment_2.name_id);
route_names.shortest_path_name_1 = facade->get_name_for_id(shortest_segment_1.name_id);
route_names.shortest_path_name_2 = facade->get_name_for_id(shortest_segment_2.name_id);
route_names.alternative_path_name_1 =
facade->GetEscapedNameForNameID(alternative_segment_1.name_id);
facade->get_name_for_id(alternative_segment_1.name_id);
route_names.alternative_path_name_2 =
facade->GetEscapedNameForNameID(alternative_segment_2.name_id);
facade->get_name_for_id(alternative_segment_2.name_id);
return route_names;
}

View File

@ -33,6 +33,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "../data_structures/phantom_node.hpp"
#include "../typedefs.h"
#include <boost/assert.hpp>
#include <osrm/json_container.hpp>
#include <string>

View File

@ -113,9 +113,6 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
}
// check if first segment is non-zero
std::string road_name = facade->GetEscapedNameForNameID(
raw_route.segment_end_coordinates.front().source_phantom.name_id);
BOOST_ASSERT(raw_route.unpacked_path_segments.size() ==
raw_route.segment_end_coordinates.size());
@ -157,9 +154,9 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
json_route_summary.values["total_distance"] = description_factory.summary.distance;
json_route_summary.values["total_time"] = description_factory.summary.duration;
json_route_summary.values["start_point"] =
facade->GetEscapedNameForNameID(description_factory.summary.source_name_id);
facade->get_name_for_id(description_factory.summary.source_name_id);
json_route_summary.values["end_point"] =
facade->GetEscapedNameForNameID(description_factory.summary.target_name_id);
facade->get_name_for_id(description_factory.summary.target_name_id);
json_result.values["route_summary"] = json_route_summary;
BOOST_ASSERT(!raw_route.segment_end_coordinates.empty());
@ -241,10 +238,10 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
alternate_description_factory.summary.distance;
json_alternate_route_summary.values["total_time"] =
alternate_description_factory.summary.duration;
json_alternate_route_summary.values["start_point"] = facade->GetEscapedNameForNameID(
alternate_description_factory.summary.source_name_id);
json_alternate_route_summary.values["end_point"] = facade->GetEscapedNameForNameID(
alternate_description_factory.summary.target_name_id);
json_alternate_route_summary.values["start_point"] =
facade->get_name_for_id(alternate_description_factory.summary.source_name_id);
json_alternate_route_summary.values["end_point"] =
facade->get_name_for_id(alternate_description_factory.summary.target_name_id);
json_alternate_route_summary_array.values.push_back(json_alternate_route_summary);
json_result.values["alternative_summaries"] = json_alternate_route_summary_array;
@ -349,8 +346,7 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
}
json_instruction_row.values.push_back(current_turn_instruction);
json_instruction_row.values.push_back(
facade->GetEscapedNameForNameID(segment.name_id));
json_instruction_row.values.push_back(facade->get_name_for_id(segment.name_id));
json_instruction_row.values.push_back(std::round(segment.length));
json_instruction_row.values.push_back(necessary_segments_running_index);
json_instruction_row.values.push_back(std::round(segment.duration / 10.));

View File

@ -88,9 +88,8 @@ template <class DataFacadeT> class NearestPlugin final : public BasePlugin
json_coordinate.values.push_back(phantom_node_vector.at(i).location.lon /
COORDINATE_PRECISION);
result.values["mapped coordinate"] = json_coordinate;
std::string temp_string;
facade->GetName(phantom_node_vector.at(i).name_id, temp_string);
result.values["name"] = temp_string;
result.values["name"] =
facade->get_name_for_id(phantom_node_vector.at(i).name_id);
results.values.push_back(result);
}
json_result.values["results"] = results;
@ -103,9 +102,8 @@ template <class DataFacadeT> class NearestPlugin final : public BasePlugin
json_coordinate.values.push_back(phantom_node_vector.front().location.lon /
COORDINATE_PRECISION);
json_result.values["mapped_coordinate"] = json_coordinate;
std::string temp_string;
facade->GetName(phantom_node_vector.front().name_id, temp_string);
json_result.values["name"] = temp_string;
json_result.values["name"] =
facade->get_name_for_id(phantom_node_vector.front().name_id);
}
}
return 200;

View File

@ -109,14 +109,7 @@ template <class EdgeDataT> class BaseDataFacade
virtual unsigned GetNameIndexFromEdgeID(const unsigned id) const = 0;
virtual void GetName(const unsigned name_id, std::string &result) const = 0;
std::string GetEscapedNameForNameID(const unsigned name_id) const
{
std::string temporary_string;
GetName(name_id, temporary_string);
return EscapeJSONString(temporary_string);
}
virtual std::string get_name_for_id(const unsigned name_id) const = 0;
virtual std::string GetTimestamp() const = 0;
};

View File

@ -46,6 +46,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <osrm/coordinate.hpp>
#include <osrm/server_paths.hpp>
#include <limits>
template <class EdgeDataT> class InternalDataFacade final : public BaseDataFacade<EdgeDataT>
{
@ -418,24 +420,25 @@ template <class EdgeDataT> class InternalDataFacade final : public BaseDataFacad
unsigned GetNameIndexFromEdgeID(const unsigned id) const override final
{
return m_name_ID_list.at(id);
};
}
void GetName(const unsigned name_id, std::string &result) const override final
std::string get_name_for_id(const unsigned name_id) const override final
{
if (UINT_MAX == name_id)
if (std::numeric_limits<unsigned>::max() == name_id)
{
result = "";
return;
return "";
}
auto range = m_name_table.GetRange(name_id);
result.clear();
std::string result;
result.reserve(range.size());
if (range.begin() != range.end())
{
result.resize(range.back() - range.front() + 1);
std::copy(m_names_char_list.begin() + range.front(),
m_names_char_list.begin() + range.back() + 1, result.begin());
}
return result;
}
virtual unsigned GetGeometryIndexForEdgeID(const unsigned id) const override final

View File

@ -41,6 +41,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "../../util/simple_logger.hpp"
#include <algorithm>
#include <limits>
#include <memory>
template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<EdgeDataT>
@ -409,22 +410,23 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
return m_name_ID_list.at(id);
};
void GetName(const unsigned name_id, std::string &result) const override final
std::string get_name_for_id(const unsigned name_id) const override final
{
if (UINT_MAX == name_id)
if (std::numeric_limits<unsigned>::max() == name_id)
{
result = "";
return;
return "";
}
auto range = m_name_table->GetRange(name_id);
result.clear();
std::string result;
result.reserve(range.size());
if (range.begin() != range.end())
{
result.resize(range.back() - range.front() + 1);
std::copy(m_names_char_list.begin() + range.front(),
m_names_char_list.begin() + range.back() + 1, result.begin());
}
return result;
}
std::string GetTimestamp() const override final { return m_timestamp; }

View File

@ -32,6 +32,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define JSON_RENDERER_HPP
#include "cast.hpp"
#include "string_util.hpp"
#include <osrm/json_container.hpp>
@ -40,31 +41,14 @@ namespace osrm
namespace json
{
struct Renderer : mapbox::util::static_visitor<>
{
explicit Renderer(std::ostream &_out) : out(_out) {}
void operator()(const String &string) const {
void operator()(const String &string) const
{
out << "\"";
// check if we need escaping
std::size_t pos = string.value.find_first_of('\\');
if (pos != std::string::npos)
{
std::string escapedString(string.value);
do
{
escapedString.insert(pos, 1, '\\');
pos = escapedString.find_first_of('\\', pos);
} while (pos != std::string::npos);
}
// no need to escape
else
{
out << string.value;
}
out << escape_JSON(string.value);
out << "\"";
}
@ -123,25 +107,8 @@ struct ArrayRenderer : mapbox::util::static_visitor<>
void operator()(const String &string) const
{
out.push_back('\"');
// check if we need escaping
std::size_t pos = string.value.find_first_of('\\');
if (pos != std::string::npos)
{
std::string escapedString(string.value);
do
{
escapedString.insert(pos, 1, '\\');
pos = escapedString.find_first_of('\\', pos+2);
} while (pos != std::string::npos);
out.insert(out.end(), escapedString.begin(), escapedString.end());
}
// no need to escape
else
{
out.insert(out.end(), string.value.begin(), string.value.end());
}
const auto string_to_insert = escape_JSON(string.value);
out.insert(std::end(out), std::begin(string_to_insert), std::end(string_to_insert));
out.push_back('\"');
}

View File

@ -77,10 +77,17 @@ inline void replaceAll(std::string &s, const std::string &sub, const std::string
boost::replace_all(s, sub, other);
}
inline std::string EscapeJSONString(const std::string &input)
inline std::string escape_JSON(const std::string &input)
{
// return the input if no backslash can be found -> no allocations
if (input.find_first_of('\\') == std::string::npos)
{
return input;
}
// escape and skip reallocations if possible
std::string output;
output.reserve(input.size());
output.reserve(input.size() + 4); // +4 assumes two backslashes on avg
for (auto iter = input.begin(); iter != input.end(); ++iter)
{
switch (iter[0])