Skip to content

Commit

Permalink
fix memory leak and two sp one ap
Browse files Browse the repository at this point in the history
  • Loading branch information
xlqian committed Jan 14, 2022
1 parent 48f0b03 commit 96e1385
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 90 deletions.
1 change: 1 addition & 0 deletions fixtures/ed/ntfs_v5/pathways.txt
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pathway_id,from_stop_id,to_stop_id,pathway_mode,is_bidirectional,length,traversal_time,stair_count,max_slope,min_width,signposted_as,reversed_signposted_as
SP:A:IO:1,SP:A,IO:1,3,1,68.58,87,,,,,
SP:B:IO:2,SP:B,IO:2,3,0,68.58,87,3,30,2,,
SP:B:IO:1,SP:B,IO:1,3,1,42,60,40,30,2,,
18 changes: 9 additions & 9 deletions source/ed/build_helper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,15 +697,15 @@ void builder::add_access_point(const std::string& stop_point,
const double x,
const double y) {
auto sp = data->pt_data->stop_points_map[stop_point];
auto* ap = new navitia::type::AccessPoint();
ap->name = access_point_name;
ap->uri = access_point_name;
ap->is_entrance = is_entrance;
ap->is_exit = is_exit;
ap->length = length;
ap->traversal_time = traversal_time;
ap->coord.set_lat(x);
ap->coord.set_lon(y);
auto ap = navitia::type::AccessPoint();
ap.name = access_point_name;
ap.uri = access_point_name;
ap.is_entrance = is_entrance;
ap.is_exit = is_exit;
ap.length = length;
ap.traversal_time = traversal_time;
ap.coord.set_lat(x);
ap.coord.set_lon(y);
sp->access_points.insert(ap);
}

Expand Down
83 changes: 55 additions & 28 deletions source/ed/docker_tests/ed_integration_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,38 +452,65 @@ BOOST_FIXTURE_TEST_CASE(ntfs_v5_test, ArgsFixture) {
// Access Point
BOOST_REQUIRE_EQUAL(data.pt_data->stop_points_map["stop_point:SP:A"]->access_points.size(), 1);
for (const auto& ap : data.pt_data->stop_points_map["stop_point:SP:A"]->access_points) {
BOOST_REQUIRE_EQUAL(ap->uri, "access_point:IO:1");
BOOST_REQUIRE_EQUAL(ap->stop_code, "stop_code_io_1");
BOOST_REQUIRE_EQUAL(ap->length, 68);
BOOST_REQUIRE_EQUAL(ap->traversal_time, 87);
BOOST_REQUIRE_EQUAL(ap->is_entrance, true);
BOOST_REQUIRE_EQUAL(ap->is_exit, true);
BOOST_REQUIRE_EQUAL(ap->stair_count, -1); // no value
BOOST_REQUIRE_EQUAL(ap->max_slope, -1); // no value
BOOST_REQUIRE_EQUAL(ap->min_width, -1); // no value
BOOST_REQUIRE_EQUAL(ap->signposted_as, ""); // no value
BOOST_REQUIRE_EQUAL(ap->reversed_signposted_as, ""); // no value
BOOST_CHECK_CLOSE(ap->coord.lat(), 45.0614, 0.001);
BOOST_CHECK_CLOSE(ap->coord.lon(), 0.6155, 0.001);
BOOST_REQUIRE_EQUAL(ap.uri, "access_point:IO:1");
BOOST_REQUIRE_EQUAL(ap.stop_code, "stop_code_io_1");
BOOST_REQUIRE_EQUAL(ap.length, 68);
BOOST_REQUIRE_EQUAL(ap.traversal_time, 87);
BOOST_REQUIRE_EQUAL(ap.is_entrance, true);
BOOST_REQUIRE_EQUAL(ap.is_exit, true);
BOOST_REQUIRE_EQUAL(ap.stair_count, -1); // no value
BOOST_REQUIRE_EQUAL(ap.max_slope, -1); // no value
BOOST_REQUIRE_EQUAL(ap.min_width, -1); // no value
BOOST_REQUIRE_EQUAL(ap.signposted_as, ""); // no value
BOOST_REQUIRE_EQUAL(ap.reversed_signposted_as, ""); // no value
BOOST_CHECK_CLOSE(ap.coord.lat(), 45.0614, 0.001);
BOOST_CHECK_CLOSE(ap.coord.lon(), 0.6155, 0.001);
}

BOOST_REQUIRE_EQUAL(data.pt_data->stop_points_map["stop_point:SP:B"]->access_points.size(), 1);
BOOST_REQUIRE_EQUAL(data.pt_data->stop_points_map["stop_point:SP:B"]->access_points.size(), 2);

auto checked_ap = 0;

auto test_ap_from_sp_B = [&checked_ap](const navitia::type::AccessPoint& ap) {
if (ap.uri == "access_point:IO:2") {
BOOST_REQUIRE_EQUAL(ap.stop_code, "stop_code_io_2");
BOOST_REQUIRE_EQUAL(ap.length, 68);
BOOST_REQUIRE_EQUAL(ap.traversal_time, 87);
BOOST_REQUIRE_EQUAL(ap.is_entrance, false);
BOOST_REQUIRE_EQUAL(ap.is_exit, true);
BOOST_REQUIRE_EQUAL(ap.stair_count, 3);
BOOST_REQUIRE_EQUAL(ap.max_slope, 30);
BOOST_REQUIRE_EQUAL(ap.min_width, 2);
BOOST_REQUIRE_EQUAL(ap.signposted_as, ""); // no value
BOOST_REQUIRE_EQUAL(ap.reversed_signposted_as, ""); // no value
BOOST_CHECK_CLOSE(ap.coord.lat(), 45.0614, 0.001);
BOOST_CHECK_CLOSE(ap.coord.lon(), 0.6155, 0.001);
checked_ap++;
return;
}
if (ap.uri == "access_point:IO:1") {
BOOST_REQUIRE_EQUAL(ap.stop_code, "stop_code_io_1");
BOOST_REQUIRE_EQUAL(ap.length, 42);
BOOST_REQUIRE_EQUAL(ap.traversal_time, 60);
BOOST_REQUIRE_EQUAL(ap.is_entrance, true);
BOOST_REQUIRE_EQUAL(ap.is_exit, true);
BOOST_REQUIRE_EQUAL(ap.stair_count, 40);
BOOST_REQUIRE_EQUAL(ap.max_slope, 30);
BOOST_REQUIRE_EQUAL(ap.min_width, 2);
BOOST_REQUIRE_EQUAL(ap.signposted_as, ""); // no value
BOOST_REQUIRE_EQUAL(ap.reversed_signposted_as, ""); // no value
BOOST_CHECK_CLOSE(ap.coord.lat(), 45.0614, 0.001);
BOOST_CHECK_CLOSE(ap.coord.lon(), 0.6155, 0.001);
checked_ap++;
return;
}
// Should never go here...
BOOST_CHECK(false);
};
for (const auto& ap : data.pt_data->stop_points_map["stop_point:SP:B"]->access_points) {
BOOST_REQUIRE_EQUAL(ap->uri, "access_point:IO:2");
BOOST_REQUIRE_EQUAL(ap->stop_code, "stop_code_io_2");
BOOST_REQUIRE_EQUAL(ap->length, 68);
BOOST_REQUIRE_EQUAL(ap->traversal_time, 87);
BOOST_REQUIRE_EQUAL(ap->is_entrance, false);
BOOST_REQUIRE_EQUAL(ap->is_exit, true);
BOOST_REQUIRE_EQUAL(ap->stair_count, 3);
BOOST_REQUIRE_EQUAL(ap->max_slope, 30);
BOOST_REQUIRE_EQUAL(ap->min_width, 2);
BOOST_REQUIRE_EQUAL(ap->signposted_as, ""); // no value
BOOST_REQUIRE_EQUAL(ap->reversed_signposted_as, ""); // no value
BOOST_CHECK_CLOSE(ap->coord.lat(), 45.0614, 0.001);
BOOST_CHECK_CLOSE(ap->coord.lon(), 0.6155, 0.001);
test_ap_from_sp_B(ap);
}

BOOST_CHECK_EQUAL(checked_ap, 2);
check_ntfs(data);
}

Expand Down
49 changes: 26 additions & 23 deletions source/ed/ed_reader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,55 +626,57 @@ void EdReader::fill_stop_points(nt::Data& data, pqxx::work& work) {
}
}

void EdReader::fill_access_point_field(navitia::type::AccessPoint* access_point,
void EdReader::fill_access_point_field(const navitia::type::AccessPoint& access_point,
const pqxx::result::iterator const_it,
const bool from_access_point,
const std::string& sp_id) {
auto new_access_point = navitia::type::AccessPoint(access_point);

if (!const_it["pathway_mode"].is_null()) {
access_point->pathway_mode = const_it["pathway_mode"].as<unsigned int>();
new_access_point.pathway_mode = const_it["pathway_mode"].as<unsigned int>();
}
if (!const_it["is_bidirectional"].is_null()) {
const bool is_bidirectional = const_it["is_bidirectional"].as<bool>();
if (is_bidirectional) {
access_point->is_entrance = true;
access_point->is_exit = true;
new_access_point.is_entrance = true;
new_access_point.is_exit = true;
} else {
if (from_access_point) {
access_point->is_entrance = true;
access_point->is_exit = false;
new_access_point.is_entrance = true;
new_access_point.is_exit = false;
} else {
access_point->is_entrance = false;
access_point->is_exit = true;
new_access_point.is_entrance = false;
new_access_point.is_exit = true;
}
}
}
if (!const_it["length"].is_null()) {
access_point->length = const_it["length"].as<unsigned int>();
new_access_point.length = const_it["length"].as<unsigned int>();
}
if (!const_it["traversal_time"].is_null()) {
access_point->traversal_time = const_it["traversal_time"].as<unsigned int>();
new_access_point.traversal_time = const_it["traversal_time"].as<unsigned int>();
}
if (!const_it["stair_count"].is_null()) {
access_point->stair_count = const_it["stair_count"].as<unsigned int>();
new_access_point.stair_count = const_it["stair_count"].as<unsigned int>();
}
if (!const_it["max_slope"].is_null()) {
access_point->max_slope = const_it["max_slope"].as<unsigned int>();
new_access_point.max_slope = const_it["max_slope"].as<unsigned int>();
}
if (!const_it["min_width"].is_null()) {
access_point->min_width = const_it["min_width"].as<unsigned int>();
new_access_point.min_width = const_it["min_width"].as<unsigned int>();
}
if (!const_it["signposted_as"].is_null()) {
const_it["signposted_as"].to(access_point->signposted_as);
const_it["signposted_as"].to(new_access_point.signposted_as);
}
if (!const_it["reversed_signposted_as"].is_null()) {
const_it["reversed_signposted_as"].to(access_point->reversed_signposted_as);
const_it["reversed_signposted_as"].to(new_access_point.reversed_signposted_as);
}
// link with SP
auto sp_key = uri_to_idx_stop_point.find("stop_point:" + sp_id);
if (sp_key != uri_to_idx_stop_point.end()) {
auto sp = stop_point_map.find(sp_key->second);
if (sp != stop_point_map.end()) {
sp->second->access_points.insert(access_point);
sp->second->access_points.insert(new_access_point);
}
} else {
LOG4CPLUS_ERROR(log, "pathway.to_stop_id not match with a stop point uri " << sp_id);
Expand All @@ -692,18 +694,19 @@ void EdReader::fill_access_points(nt::Data& data, pqxx::work& work) {

pqxx::result result = work.exec(request);
for (auto const_it = result.begin(); const_it != result.end(); ++const_it) {
auto* ap = new nt::AccessPoint();
const_it["uri"].to(ap->uri);
const_it["name"].to(ap->name);
const_it["stop_code"].to(ap->stop_code);
ap->coord.set_lon(const_it["lon"].as<double>());
ap->coord.set_lat(const_it["lat"].as<double>());
auto ap = nt::AccessPoint{};

const_it["uri"].to(ap.uri);
const_it["name"].to(ap.name);
const_it["stop_code"].to(ap.stop_code);
ap.coord.set_lon(const_it["lon"].as<double>());
ap.coord.set_lat(const_it["lat"].as<double>());
// parent station is Stop Area
auto sa_key = uri_to_idx_stop_area.find("stop_area:" + const_it["parent_station"].as<std::string>());
if (sa_key != uri_to_idx_stop_area.end()) {
auto sa = stop_area_map.find(sa_key->second);
if (sa != stop_area_map.end()) {
ap->parent_station = sa->second;
ap.parent_station = sa->second;
}
} else {
LOG4CPLUS_ERROR(log, "access_point.parent_station not match with a stop area uri "
Expand Down
4 changes: 2 additions & 2 deletions source/ed/ed_reader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ struct EdReader {
std::unordered_map<std::string, idx_t> uri_to_idx_stop_area;
std::unordered_map<idx_t, navitia::type::StopPoint*> stop_point_map;
std::unordered_map<std::string, idx_t> uri_to_idx_stop_point;
std::unordered_map<std::string, navitia::type::AccessPoint*> access_point_map;
std::unordered_map<std::string, navitia::type::AccessPoint> access_point_map;
std::unordered_map<idx_t, navitia::type::Line*> line_map;
std::unordered_map<idx_t, navitia::type::LineGroup*> line_group_map;
std::unordered_map<idx_t, navitia::type::Route*> route_map;
Expand Down Expand Up @@ -124,7 +124,7 @@ struct EdReader {

void fill_stop_areas(navitia::type::Data& data, pqxx::work& work);
void fill_stop_points(navitia::type::Data& data, pqxx::work& work);
void fill_access_point_field(navitia::type::AccessPoint* access_point,
void fill_access_point_field(const navitia::type::AccessPoint& access_point,
const pqxx::result::iterator const_it,
const bool from_access_point,
const std::string& sp_id);
Expand Down
52 changes: 26 additions & 26 deletions source/type/pb_converter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,45 +594,45 @@ void PbCreator::Filler::fill_pb_object(const ng::Admin* adm, pbnavitia::Administ
}
}

void PbCreator::Filler::fill_access_points(const std::set<nt::AccessPoint*>& access_points,
void PbCreator::Filler::fill_access_points(const std::set<nt::AccessPoint>& access_points,
pbnavitia::StopPoint* stop_point) {
for (const auto& access_point : access_points) {
pbnavitia::AccessPoint* ap = stop_point->add_access_points();
ap->set_name(access_point->name);
ap->set_uri(access_point->uri);
if (access_point->coord.is_initialized()) {
ap->mutable_coord()->set_lon(access_point->coord.lon());
ap->mutable_coord()->set_lat(access_point->coord.lat());
ap->set_name(access_point.name);
ap->set_uri(access_point.uri);
if (access_point.coord.is_initialized()) {
ap->mutable_coord()->set_lon(access_point.coord.lon());
ap->mutable_coord()->set_lat(access_point.coord.lat());
}
ap->set_is_entrance(access_point->is_entrance);
ap->set_is_exit(access_point->is_exit);
if (access_point->pathway_mode != nt::ap_default_value) {
ap->set_pathway_mode(access_point->pathway_mode);
ap->set_is_entrance(access_point.is_entrance);
ap->set_is_exit(access_point.is_exit);
if (access_point.pathway_mode != nt::ap_default_value) {
ap->set_pathway_mode(access_point.pathway_mode);
}
if (access_point->length != nt::ap_default_value) {
ap->set_length(access_point->length);
if (access_point.length != nt::ap_default_value) {
ap->set_length(access_point.length);
}
if (access_point->traversal_time != nt::ap_default_value) {
ap->set_traversal_time(access_point->traversal_time);
if (access_point.traversal_time != nt::ap_default_value) {
ap->set_traversal_time(access_point.traversal_time);
}
if (access_point->stair_count != nt::ap_default_value) {
ap->set_stair_count(access_point->stair_count);
if (access_point.stair_count != nt::ap_default_value) {
ap->set_stair_count(access_point.stair_count);
}
if (access_point->max_slope != nt::ap_default_value) {
ap->set_max_slope(access_point->max_slope);
if (access_point.max_slope != nt::ap_default_value) {
ap->set_max_slope(access_point.max_slope);
}
if (access_point->min_width != nt::ap_default_value) {
ap->set_min_width(access_point->min_width);
if (access_point.min_width != nt::ap_default_value) {
ap->set_min_width(access_point.min_width);
}
if (!access_point->signposted_as.empty()) {
ap->set_signposted_as(access_point->signposted_as);
if (!access_point.signposted_as.empty()) {
ap->set_signposted_as(access_point.signposted_as);
}
if (!access_point->reversed_signposted_as.empty()) {
ap->set_reversed_signposted_as(access_point->reversed_signposted_as);
if (!access_point.reversed_signposted_as.empty()) {
ap->set_reversed_signposted_as(access_point.reversed_signposted_as);
}
if (access_point->parent_station != nullptr) {
if (access_point.parent_station != nullptr) {
pbnavitia::StopArea* sa = ap->mutable_parent_station();
fill_pb_object(access_point->parent_station, sa);
fill_pb_object(access_point.parent_station, sa);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion source/type/pb_converter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ struct PbCreator {
void fill_pb_object(const ng::Address*, pbnavitia::Address*);
void fill_pb_object(const nt::Comment*, pbnavitia::Note*);

void fill_access_points(const std::set<nt::AccessPoint*>& access_points, pbnavitia::StopPoint* stop_point);
void fill_access_points(const std::set<nt::AccessPoint>& access_points, pbnavitia::StopPoint* stop_point);

// Used for place
template <typename T>
Expand Down
3 changes: 2 additions & 1 deletion source/type/stop_point.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ www.navitia.io
#include "type/type_interfaces.h"
#include "type/geographical_coord.h"
#include "type/fwd_type.h"
#include "type/access_point.h"

#include <boost/container/flat_set.hpp>

Expand All @@ -53,7 +54,7 @@ struct StopPoint : public Header, Nameable, hasProperties, HasMessages {

StopArea* stop_area;
std::vector<navitia::georef::Admin*> admin_list;
std::set<navitia::type::AccessPoint*> access_points;
std::set<navitia::type::AccessPoint> access_points;
Network* network;
std::vector<StopPointConnection*> stop_point_connection_list;
std::set<Dataset*> dataset_list;
Expand Down

0 comments on commit 96e1385

Please sign in to comment.