From 18a83028e81028ddb852725a4a09606c62e6f415 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Mon, 31 Aug 2015 17:55:43 +0100 Subject: [PATCH 01/17] WIP: getting data structures right for changesets. --- Makefile.am | 2 +- include/cgimap/api06/changeset_handler.hpp | 34 ++++++ .../apidb/writeable_pgsql_selection.hpp | 6 + include/cgimap/data_selection.hpp | 16 +++ include/cgimap/handler.hpp | 4 +- include/cgimap/json_formatter.hpp | 3 + include/cgimap/osm_current_responder.hpp | 3 +- include/cgimap/output_formatter.hpp | 39 +++++++ include/cgimap/router.hpp | 3 + include/cgimap/xml_formatter.hpp | 3 + src/Makefile.am | 5 +- src/api06/changeset_handler.cpp | 37 +++++++ .../apidb/writeable_pgsql_selection.cpp | 99 +++++++++++++++++ src/backend/staticxml/staticxml.cpp | 103 +++++++++++++++++- src/data_selection.cpp | 13 +++ src/json_formatter.cpp | 19 +++- src/osm_current_responder.cpp | 9 +- src/output_formatter.cpp | 27 +++++ src/process_request.cpp | 7 +- src/routes.cpp | 5 + src/xml_formatter.cpp | 26 +++++ test/changesets.testcore/data.osm | 11 ++ test/changesets.testcore/read.case | 13 +++ test/json.testcore/node_1.case | 1 + test/test_apidb_backend.cpp | 1 + test/test_core.cpp | 3 + 26 files changed, 474 insertions(+), 18 deletions(-) create mode 100644 include/cgimap/api06/changeset_handler.hpp create mode 100644 src/api06/changeset_handler.cpp create mode 100644 test/changesets.testcore/data.osm create mode 100644 test/changesets.testcore/read.case diff --git a/Makefile.am b/Makefile.am index f19b2d9c3..1e8f01679 100644 --- a/Makefile.am +++ b/Makefile.am @@ -75,7 +75,7 @@ TESTS = test/map.testcore test/node.testcore test/anon.testcore test/way.testcor TESTS += test/empty.testcore test/way_full.testcore test/relation_full.testcore TESTS += test/test_parse_id_list test/test_oauth test/test_http if ENABLE_EXPERIMENTAL -TESTS += test/node_ways.testcore +TESTS += test/node_ways.testcore test/changesets.testcore endif if HAVE_YAJL TESTS += test/json.testcore diff --git a/include/cgimap/api06/changeset_handler.hpp b/include/cgimap/api06/changeset_handler.hpp new file mode 100644 index 000000000..ae803d9ef --- /dev/null +++ b/include/cgimap/api06/changeset_handler.hpp @@ -0,0 +1,34 @@ +#ifndef API06_CHANGESET_HANDLER_HPP +#define API06_CHANGESET_HANDLER_HPP + +#include "cgimap/handler.hpp" +#include "cgimap/osm_current_responder.hpp" +#include "cgimap/request.hpp" +#include + +namespace api06 { + +class changeset_responder : public osm_current_responder { +public: + changeset_responder(mime::type, osm_changeset_id_t, factory_ptr &); + ~changeset_responder(); + +private: + osm_changeset_id_t id; +}; + +class changeset_handler : public handler { +public: + changeset_handler(request &req, osm_changeset_id_t id); + ~changeset_handler(); + + std::string log_name() const; + responder_ptr_t responder(factory_ptr &x) const; + +private: + osm_changeset_id_t id; +}; + +} // namespace api06 + +#endif /* API06_CHANGESET_HANDLER_HPP */ diff --git a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp index 7e4d23610..e63de12ba 100644 --- a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp +++ b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp @@ -21,6 +21,7 @@ class writeable_pgsql_selection : public data_selection { void write_nodes(output_formatter &formatter); void write_ways(output_formatter &formatter); void write_relations(output_formatter &formatter); + void write_changesets(output_formatter &formatter, const boost::posix_time::ptime &now); visibility_t check_node_visibility(osm_nwr_id_t id); visibility_t check_way_visibility(osm_nwr_id_t id); @@ -39,6 +40,11 @@ class writeable_pgsql_selection : public data_selection { void select_relations_from_relations(); void select_relations_members_of_relations(); +#ifdef ENABLE_EXPERIMENTAL + bool supports_changesets(); + int select_changesets(const std::vector &); +#endif /* ENABLE_EXPERIMENTAL */ + /** * abstracts the creation of transactions for the writeable * data selection. diff --git a/include/cgimap/data_selection.hpp b/include/cgimap/data_selection.hpp index 30d020522..8a48a36ce 100644 --- a/include/cgimap/data_selection.hpp +++ b/include/cgimap/data_selection.hpp @@ -6,6 +6,7 @@ #include #include +#include /** * represents a selected set of data which can be written out to @@ -29,6 +30,12 @@ class data_selection { /// write the relations to an output formatter virtual void write_relations(output_formatter &formatter) = 0; +#ifdef ENABLE_EXPERIMENTAL + /// does this data selection support changesets? + virtual void write_changesets(output_formatter &formatter, + const boost::posix_time::ptime &now); +#endif /* ENABLE_EXPERIMENTAL */ + /******************* information functions *******************/ // check if the node is visible, deleted or has never existed @@ -82,6 +89,15 @@ class data_selection { /// select relations which are members of selected relations virtual void select_relations_members_of_relations() = 0; +#ifdef ENABLE_EXPERIMENTAL + /// does this data selection support changesets? + virtual bool supports_changesets(); + + /// select specified changesets, returning the number of + /// changesets selected. + virtual int select_changesets(const std::vector &); +#endif /* ENABLE_EXPERIMENTAL */ + /** * factory for the creation of data selections. this abstracts away * the creation process of transactions, and allows some up-front diff --git a/include/cgimap/handler.hpp b/include/cgimap/handler.hpp index 5759c9d35..fab72d6d4 100644 --- a/include/cgimap/handler.hpp +++ b/include/cgimap/handler.hpp @@ -10,6 +10,7 @@ #include #include #include +#include /** * object which is able to respond to an already-setup request. @@ -19,7 +20,8 @@ class responder { responder(mime::type); virtual ~responder(); virtual void write(boost::shared_ptr f, - const std::string &generator) = 0; + const std::string &generator, + const boost::posix_time::ptime &now) = 0; mime::type resource_type() const; diff --git a/include/cgimap/json_formatter.hpp b/include/cgimap/json_formatter.hpp index efb2c9505..8704739ca 100644 --- a/include/cgimap/json_formatter.hpp +++ b/include/cgimap/json_formatter.hpp @@ -37,6 +37,9 @@ class json_formatter : public output_formatter { void write_relation(const element_info &elem, const members_t &members, const tags_t &tags); + void write_changeset(const changeset_info &elem, + const tags_t &tags); + void flush(); void error(const std::string &); }; diff --git a/include/cgimap/osm_current_responder.hpp b/include/cgimap/osm_current_responder.hpp index e0dd8f9d6..9ca0c632f 100644 --- a/include/cgimap/osm_current_responder.hpp +++ b/include/cgimap/osm_current_responder.hpp @@ -19,7 +19,8 @@ class osm_current_responder : public osm_responder { // writes whatever is in the tmp_nodes/ways/relations tables to the given // formatter. void write(boost::shared_ptr f, - const std::string &generator); + const std::string &generator, + const boost::posix_time::ptime &now); protected: // current selection of elements to be written out diff --git a/include/cgimap/output_formatter.hpp b/include/cgimap/output_formatter.hpp index 28eed6d67..c3496786d 100644 --- a/include/cgimap/output_formatter.hpp +++ b/include/cgimap/output_formatter.hpp @@ -12,6 +12,7 @@ * What type of element the formatter is starting to write. */ enum element_type { + element_type_changeset, element_type_node, element_type_way, element_type_relation @@ -37,6 +38,40 @@ struct element_info { bool visible; }; +struct changeset_info { + changeset_info(); + changeset_info(const changeset_info &); + changeset_info(osm_changeset_id_t id_, + const std::string &created_at_, + const std::string &closed_at_, + const boost::optional &uid_, + const boost::optional &display_name_, + const boost::optional &bounding_box_, + size_t num_changes_, + size_t comments_count_, + bool open_); + // standard meaning of ID + osm_changeset_id_t id; + // changesets are created at a certain time and may be either + // closed explicitly with a closing time, or close implicitly + // an hour after the last update to the changeset. + std::string created_at, closed_at; + // anonymous objects don't have UIDs or display names + boost::optional uid; + boost::optional display_name; + // changesets with edits will have a bounding box containing + // the extent of all the changes. + boost::optional bounding_box; + // the number of changes (new element versions) associated + // with this changeset. + size_t num_changes; + // if the changeset has a discussion attached, then this will + // be the number of comments. + size_t comments_count; + // is the changeset open (true) or closed (false) + bool open; +}; + struct member_info { element_type type; osm_nwr_id_t ref; @@ -96,6 +131,10 @@ struct output_formatter { virtual void write_relation(const element_info &elem, const members_t &members, const tags_t &tags) = 0; + // output a single changeset. + virtual void write_changeset(const changeset_info &elem, + const tags_t &tags) = 0; + // flush the current state virtual void flush() = 0; diff --git a/include/cgimap/router.hpp b/include/cgimap/router.hpp index 64272e26f..d95977bbc 100644 --- a/include/cgimap/router.hpp +++ b/include/cgimap/router.hpp @@ -113,6 +113,9 @@ struct match_string : public ops { match_string(const std::string &s); match_string(const char *s); + // copy just copies the held string + inline match_string(const match_string &m) : str(m.str) {} + match_type match(part_iterator &begin, const part_iterator &end) const; private: diff --git a/include/cgimap/xml_formatter.hpp b/include/cgimap/xml_formatter.hpp index f83afc596..aab9e948e 100644 --- a/include/cgimap/xml_formatter.hpp +++ b/include/cgimap/xml_formatter.hpp @@ -37,6 +37,9 @@ class xml_formatter : public output_formatter { void write_relation(const element_info &elem, const members_t &members, const tags_t &tags); + void write_changeset(const changeset_info &elem, + const tags_t &tags); + void flush(); void error(const std::string &); }; diff --git a/src/Makefile.am b/src/Makefile.am index 70f8d8746..bab728e5c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -105,7 +105,8 @@ libcgimap_core_la_SOURCES+=\ api06/way_handler.cpp \ api06/ways_handler.cpp \ api06/relation_full_handler.cpp \ - api06/way_full_handler.cpp + api06/way_full_handler.cpp \ + api06/changeset_handler.cpp if ENABLE_EXPERIMENTAL libcgimap_core_la_SOURCES+=\ @@ -129,7 +130,7 @@ libcgimap_apidb_la_SOURCES=\ backend/apidb/changeset.cpp \ backend/apidb/quad_tile.cpp \ backend/apidb/oauth_store.cpp -libcgimap_apidb_la_LIBADD=libcgimap_core.la @LIBPQXX_LIBS@ +libcgimap_apidb_la_LIBADD=libcgimap_core.la @BOOST_DATE_TIME_LIB@ @LIBPQXX_LIBS@ endif if ENABLE_PGSNAPSHOT diff --git a/src/api06/changeset_handler.cpp b/src/api06/changeset_handler.cpp new file mode 100644 index 000000000..359c989ab --- /dev/null +++ b/src/api06/changeset_handler.cpp @@ -0,0 +1,37 @@ +#include "cgimap/api06/changeset_handler.hpp" +#include "cgimap/http.hpp" + +#include + +using std::stringstream; +using std::vector; + +namespace api06 { + +changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, factory_ptr &w_) + : osm_current_responder(mt, w_), id(id_) { + vector ids; + ids.push_back(id); + + if (!sel->supports_changesets()) { + throw http::server_error("Data source does not support changesets."); + } + + if (sel->select_changesets(ids) == 0) { + throw http::not_found(""); + } +} + +changeset_responder::~changeset_responder() {} + +changeset_handler::changeset_handler(request &, osm_changeset_id_t id_) : id(id_) {} + +changeset_handler::~changeset_handler() {} + +std::string changeset_handler::log_name() const { return "changeset"; } + +responder_ptr_t changeset_handler::responder(factory_ptr &w) const { + return responder_ptr_t(new changeset_responder(mime_type, id, w)); +} + +} // namespace api06 diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index 0de571fa6..2a8305c92 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -19,7 +19,10 @@ #define PREPARE_ARGS(args) args #endif +#define MAX_ELEMENTS (50000) + namespace po = boost::program_options; +namespace pt = boost::posix_time; using std::set; using std::stringstream; using std::list; @@ -68,6 +71,27 @@ template <> struct string_traits > { return ostr.str(); } }; + +// and again for osm_changeset_id_t +template <> struct string_traits > { + static const char *name() { return "vector"; } + static bool has_null() { return false; } + static bool is_null(const vector &) { return false; } + static stringstream null() { + internal::throw_null_conversion(name()); + // No, dear compiler, we don't need a return here. + throw 0; + } + static void from_string(const char[], vector &) {} + static std::string to_string(const vector &ids) { + stringstream ostr; + ostr << "{"; + std::copy(ids.begin(), ids.end(), + infix_ostream_iterator(ostr, ",")); + ostr << "}"; + return ostr.str(); + } +}; } namespace { @@ -124,6 +148,51 @@ void extract_elem(const pqxx::result::tuple &row, element_info &elem, } } +template +boost::optional extract_optional(const pqxx::result::field &f) { + if (f.is_null()) { + return boost::none; + } else { + return f.as(); + } +} + +void extract_changeset(const pqxx::result::tuple &row, + changeset_info &elem, + cache &changeset_cache, + const pt::ptime &now) { + elem.id = row["id"].as(); + elem.created_at = row["created_at"].c_str(); + elem.closed_at = row["closed_at"].c_str(); + + shared_ptr cs = changeset_cache.get(elem.id); + if (cs->data_public) { + elem.uid = cs->user_id; + elem.display_name = cs->display_name; + } else { + elem.uid = boost::none; + elem.display_name = boost::none; + } + + boost::optional min_lat = extract_optional(row["min_lat"]); + boost::optional max_lat = extract_optional(row["max_lat"]); + boost::optional min_lon = extract_optional(row["min_lon"]); + boost::optional max_lon = extract_optional(row["max_lon"]); + + if (bool(min_lat) && bool(min_lon) && bool(max_lat) && bool(max_lon)) { + elem.bounding_box = bbox(double(*min_lat) / SCALE, + double(*min_lon) / SCALE, + double(*max_lat) / SCALE, + double(*max_lon) / SCALE); + } else { + elem.bounding_box = boost::none; + } + + const size_t num_changes = row["num_changes"].as(); + const pt::ptime closed_at_time = pt::time_from_string(elem.closed_at); + elem.open = (closed_at_time > now) && (num_changes <= MAX_ELEMENTS); +} + void extract_tags(const pqxx::result &res, tags_t &tags) { tags.clear(); for (pqxx::result::const_iterator itr = res.begin(); itr != res.end(); @@ -189,6 +258,7 @@ writeable_pgsql_selection::writeable_pgsql_selection( w.exec("CREATE TEMPORARY TABLE tmp_nodes (id bigint PRIMARY KEY)"); w.exec("CREATE TEMPORARY TABLE tmp_ways (id bigint PRIMARY KEY)"); w.exec("CREATE TEMPORARY TABLE tmp_relations (id bigint PRIMARY KEY)"); + w.exec("CREATE TEMPORARY TABLE tmp_changesets (id bigint PRIMARY KEY)"); m_tables_empty = true; } @@ -249,6 +319,20 @@ void writeable_pgsql_selection::write_relations(output_formatter &formatter) { } } +void writeable_pgsql_selection::write_changesets(output_formatter &formatter, + const pt::ptime &now) { + changeset_info elem; + tags_t tags; + + pqxx::result changesets = w.prepared("extract_changesets").exec(); + for (pqxx::result::const_iterator itr = changesets.begin(); + itr != changesets.end(); ++itr) { + extract_changeset(*itr, elem, cc, now); + extract_tags(w.prepared("extract_changeset_tags")(elem.id).exec(), tags); + formatter.write_changeset(elem, tags); + } +} + data_selection::visibility_t writeable_pgsql_selection::check_node_visibility(osm_nwr_id_t id) { return check_table_visibility(w, id, "visible_node"); @@ -346,6 +430,16 @@ void writeable_pgsql_selection::select_relations_members_of_relations() { w.prepared("relation_members_of_relations").exec(); } +#ifdef ENABLE_EXPERIMENTAL +bool writeable_pgsql_selection::supports_changesets() { + return true; +} + +int writeable_pgsql_selection::select_changesets(const std::vector &ids) { + return w.prepared("add_changesets_list")(ids).exec().affected_rows(); +} +#endif /* ENABLE_EXPERIMENTAL */ + namespace { /* this exists solely because converting boost::any seems to just * do type equality, with no fall-back to boost::lexical_cast or @@ -502,6 +596,11 @@ writeable_pgsql_selection::factory::factory(const po::variables_map &opts) "WHERE r.id = ANY($1) " "AND tr.id IS NULL") PREPARE_ARGS(("bigint[]")); + m_connection.prepare("add_changesets_list", + "INSERT INTO tmp_changesets " + "SELECT c.id from changesets " + "WHERE c.id = ANY($1)") + PREPARE_ARGS(("bigint[]")); // queries for filling elements which are used as members in relations m_connection.prepare("nodes_from_relations", diff --git a/src/backend/staticxml/staticxml.cpp b/src/backend/staticxml/staticxml.cpp index 0fdd325e6..aaf54afbd 100644 --- a/src/backend/staticxml/staticxml.cpp +++ b/src/backend/staticxml/staticxml.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -11,10 +12,12 @@ #include namespace po = boost::program_options; +namespace pt = boost::posix_time; using boost::shared_ptr; using std::string; -#define CACHE_SIZE 1000 +#define CACHE_SIZE (1000) +#define MAX_ELEMENTS (50000) namespace { @@ -54,7 +57,13 @@ struct relation { tags_t m_tags; }; +struct changeset { + changeset_info m_info; + tags_t m_tags; +}; + struct database { + std::map m_changesets; std::map m_nodes; std::map m_ways; std::map m_relations; @@ -85,17 +94,48 @@ boost::optional opt_attribute(const char *name, size_t len, } void parse_info(element_info &info, const xmlChar **attributes) { - info.id = get_attribute("id", 2, attributes); + info.id = get_attribute("id", 3, attributes); info.version = get_attribute("version", 8, attributes); - info.changeset = get_attribute("changeset", 2, attributes); + info.changeset = get_attribute("changeset", 10, attributes); info.timestamp = get_attribute("timestamp", 10, attributes); info.uid = opt_attribute("uid", 4, attributes); info.display_name = opt_attribute("user", 5, attributes); info.visible = get_attribute("visible", 8, attributes); } +void parse_changeset_info(changeset_info &info, const xmlChar **attributes) { + info.id = get_attribute("id", 3, attributes); + info.created_at = get_attribute("created_at", 11, attributes); + info.closed_at = get_attribute("closed_at", 10, attributes); + info.uid = opt_attribute("uid", 4, attributes); + info.display_name = opt_attribute("user", 5, attributes); + + const boost::optional + min_lat = opt_attribute("min_lat", 8, attributes), + min_lon = opt_attribute("min_lon", 8, attributes), + max_lat = opt_attribute("max_lat", 8, attributes), + max_lon = opt_attribute("max_lon", 8, attributes); + + if (bool(min_lat) && bool(min_lon) && bool(max_lat) && + bool(max_lon)) { + info.bounding_box = bbox(*min_lat, *min_lon, *max_lat, *max_lon); + } else { + info.bounding_box = boost::none; + } + + info.num_changes = get_attribute("num_changes", 11, attributes); + info.comments_count = 0; +} + struct xml_parser { - xml_parser(database *db) : m_db(db) {} + xml_parser(database *db) + : m_db(db), + m_cur_node(NULL), + m_cur_way(NULL), + m_cur_rel(NULL), + m_cur_tags(NULL), + m_cur_changeset(NULL) + {} static void start_element(void *ctx, const xmlChar *name, const xmlChar **attributes) { @@ -112,6 +152,7 @@ struct xml_parser { parser->m_cur_tags = &(parser->m_cur_node->m_tags); parser->m_cur_way = NULL; parser->m_cur_rel = NULL; + parser->m_cur_changeset = NULL; } else if (strncmp((const char *)name, "way", 4) == 0) { way w; @@ -122,6 +163,7 @@ struct xml_parser { parser->m_cur_tags = &(parser->m_cur_way->m_tags); parser->m_cur_node = NULL; parser->m_cur_rel = NULL; + parser->m_cur_changeset = NULL; } else if (strncmp((const char *)name, "relation", 9) == 0) { relation r; @@ -132,6 +174,20 @@ struct xml_parser { parser->m_cur_tags = &(parser->m_cur_rel->m_tags); parser->m_cur_node = NULL; parser->m_cur_way = NULL; + parser->m_cur_changeset = NULL; + + } else if (strncmp((const char *)name, "changeset", 10) == 0) { + changeset c; + + parse_changeset_info(c.m_info, attributes); + std::pair::iterator, bool> status = + parser->m_db->m_changesets.insert(std::make_pair(c.m_info.id, c)); + + parser->m_cur_changeset = &(status.first->second); + parser->m_cur_tags = &(parser->m_cur_changeset->m_tags); + parser->m_cur_node = NULL; + parser->m_cur_way = NULL; + parser->m_cur_rel = NULL; } else if (strncmp((const char *)name, "tag", 4) == 0) { if (parser->m_cur_tags != NULL) { @@ -191,6 +247,7 @@ struct xml_parser { way *m_cur_way; relation *m_cur_rel; tags_t *m_cur_tags; + changeset *m_cur_changeset; }; boost::shared_ptr parse_xml(const char *filename) { @@ -217,7 +274,7 @@ boost::shared_ptr parse_xml(const char *filename) { } struct static_data_selection : public data_selection { - static_data_selection(boost::shared_ptr db) : m_db(db) {} + explicit static_data_selection(boost::shared_ptr db) : m_db(db) {} virtual ~static_data_selection() {} virtual void write_nodes(output_formatter &formatter) { @@ -250,6 +307,18 @@ struct static_data_selection : public data_selection { } } +#ifdef ENABLE_EXPERIMENTAL + virtual void write_changesets(output_formatter &formatter) { + BOOST_FOREACH(osm_changeset_id_t id, m_changesets) { + std::map::iterator itr = m_db->m_changesets.find(id); + if (itr != m_db->m_changesets.end()) { + const changeset &c = itr->second; + formatter.write_changeset(c.m_info, c.m_tags); + } + } + } +#endif /* ENABLE_EXPERIMENTAL */ + virtual visibility_t check_node_visibility(osm_nwr_id_t id) { std::map::iterator itr = m_db->m_nodes.find(id); if (itr != m_db->m_nodes.end()) { @@ -447,13 +516,32 @@ struct static_data_selection : public data_selection { } } +#ifdef ENABLE_EXPERIMENTAL + virtual bool supports_changesets() { return true; } + + virtual int select_changesets(const std::vector &ids) { + int selected = 0; + BOOST_FOREACH(osm_changeset_id_t id, ids) { + std::map::iterator itr = m_db->m_changesets.find(id); + if (itr != m_db->m_changesets.end()) { + m_changesets.insert(id); + ++selected; + } + } + return selected; + } +#endif /* ENABLE_EXPERIMENTAL */ + + private: boost::shared_ptr m_db; + std::set m_changesets; std::set m_nodes, m_ways, m_relations; }; struct factory : public data_selection::factory { - factory(const std::string &file) : m_database(parse_xml(file.c_str())) {} + factory(const std::string &file) + : m_database(parse_xml(file.c_str())) {} virtual ~factory() {} @@ -478,6 +566,9 @@ struct staticxml_backend : public backend { shared_ptr create(const po::variables_map &opts) { std::string file = opts["file"].as(); + // TODO: now should be a variable on the *request* not in the data store. + // TODO: the changeset data structure parsed here can/should contain the + // number of changes read from the OSM file, not open/closed status. return boost::make_shared(file); } diff --git a/src/data_selection.cpp b/src/data_selection.cpp index 4734c4f3e..8cd245338 100644 --- a/src/data_selection.cpp +++ b/src/data_selection.cpp @@ -2,4 +2,17 @@ data_selection::~data_selection() {} +#ifdef ENABLE_EXPERIMENTAL +void data_selection::write_changesets(output_formatter &, const boost::posix_time::ptime &) { +} + +bool data_selection::supports_changesets() { + return false; +} + +int data_selection::select_changesets(const std::vector &) { + return 0; +} +#endif /* ENABLE_EXPERIMENTAL */ + data_selection::factory::~factory() {} diff --git a/src/json_formatter.cpp b/src/json_formatter.cpp index 9190624c3..845d91a0c 100644 --- a/src/json_formatter.cpp +++ b/src/json_formatter.cpp @@ -43,11 +43,14 @@ void json_formatter::write_tags(const tags_t &tags) { writer->end_object(); } +#define WRITE_KV(k,vt,v) \ + writer->object_key(k); \ + writer->entry_##vt(v); + void json_formatter::start_document(const std::string &generator) { writer->start_object(); - writer->object_key("version"); - writer->entry_string("0.6"); + WRITE_KV("version", string, "0.6"); writer->object_key("generator"); writer->entry_string(generator); writer->object_key("copyright"); @@ -75,7 +78,9 @@ void json_formatter::write_bounds(const bbox &bounds) { void json_formatter::end_document() { writer->end_object(); } void json_formatter::start_element_type(element_type type) { - if (type == element_type_node) { + if (type == element_type_changeset) { + writer->object_key("changesets"); + } else if (type == element_type_node) { writer->object_key("nodes"); } else if (type == element_type_way) { writer->object_key("ways"); @@ -172,6 +177,14 @@ void json_formatter::write_relation(const element_info &elem, writer->end_object(); } +void json_formatter::write_changeset(const changeset_info &elem, const tags_t &tags) { + writer->start_object(); + + + write_tags(tags); + writer->end_object(); +} + void json_formatter::flush() { writer->flush(); } void json_formatter::error(const std::string &s) { writer->error(s); } diff --git a/src/osm_current_responder.cpp b/src/osm_current_responder.cpp index 6f04363ef..7d0b48389 100644 --- a/src/osm_current_responder.cpp +++ b/src/osm_current_responder.cpp @@ -3,6 +3,7 @@ using std::list; using boost::shared_ptr; +namespace pt = boost::posix_time; osm_current_responder::osm_current_responder(mime::type mt, factory_ptr &f, boost::optional b) @@ -11,7 +12,8 @@ osm_current_responder::osm_current_responder(mime::type mt, factory_ptr &f, osm_current_responder::~osm_current_responder() {} void osm_current_responder::write(shared_ptr formatter, - const std::string &generator) { + const std::string &generator, + const pt::ptime &now) { // TODO: is it possible that formatter can be null? output_formatter &fmt = *formatter; @@ -21,6 +23,11 @@ void osm_current_responder::write(shared_ptr formatter, fmt.write_bounds(bounds.get()); } + // write all selected changesets + fmt.start_element_type(element_type_changeset); + sel->write_changesets(fmt, now); + fmt.end_element_type(element_type_changeset); + // write all selected nodes fmt.start_element_type(element_type_node); sel->write_nodes(fmt); diff --git a/src/output_formatter.cpp b/src/output_formatter.cpp index 2c589b0af..631a235c0 100644 --- a/src/output_formatter.cpp +++ b/src/output_formatter.cpp @@ -20,4 +20,31 @@ element_info::element_info(osm_nwr_id_t id_, osm_nwr_id_t version_, timestamp(timestamp_), uid(uid_), display_name(display_name_), visible(visible_) {} +changeset_info::changeset_info() + : id(0), created_at(""), closed_at(""), uid(), + display_name(), bounding_box(), num_changes(0), + comments_count(0), open(false) {} + +changeset_info::changeset_info(const changeset_info &other) + : id(other.id), created_at(other.created_at), + closed_at(other.closed_at), uid(other.uid), + display_name(other.display_name), bounding_box(other.bounding_box), + num_changes(other.num_changes), comments_count(other.comments_count), + open(other.open) {} + +changeset_info::changeset_info( + osm_changeset_id_t id_, + const std::string &created_at_, + const std::string &closed_at_, + const boost::optional &uid_, + const boost::optional &display_name_, + const boost::optional &bounding_box_, + size_t num_changes_, + size_t comments_count_, + bool open_) + : id(id_), created_at(created_at_), closed_at(closed_at_), + uid(uid_), display_name(display_name_), + bounding_box(bounding_box_), num_changes(num_changes_), + comments_count(comments_count_), open(open_) {} + output_formatter::~output_formatter() {} diff --git a/src/process_request.cpp b/src/process_request.cpp index 4f3fadc3d..acb5b766a 100644 --- a/src/process_request.cpp +++ b/src/process_request.cpp @@ -76,7 +76,8 @@ void process_not_allowed(request &req) { boost::tuple process_get_request(request &req, routes &route, boost::shared_ptr factory, - const string &ip, const string &generator) { + const string &ip, const string &generator, + const pt::ptime &now) { // figure how to handle the request handler_ptr_t handler = route(req); @@ -111,7 +112,7 @@ process_get_request(request &req, routes &route, try { // call to write the response - responder->write(o_formatter, generator); + responder->write(o_formatter, generator, now); // ensure the request is finished req.finish(); @@ -316,7 +317,7 @@ void process_request(request &req, rate_limiter &limiter, // process request if (method == "GET") { boost::tie(request_name, bytes_written) = - process_get_request(req, route, factory, ip, generator); + process_get_request(req, route, factory, ip, generator, start_time); } else if (method == "HEAD") { boost::tie(request_name, bytes_written) = diff --git a/src/routes.cpp b/src/routes.cpp index e5ac15d44..e84bb5ee0 100644 --- a/src/routes.cpp +++ b/src/routes.cpp @@ -18,6 +18,7 @@ #ifdef ENABLE_EXPERIMENTAL #include "cgimap/api06/node_ways_handler.hpp" +#include "cgimap/api06/changeset_handler.hpp" #endif /* ENABLE_EXPERIMENTAL */ #ifdef ENABLE_API07 @@ -152,6 +153,10 @@ routes::routes() r->add(root_ / "relation" / osm_id_ / "full"); r->add(root_ / "relation" / osm_id_); r->add(root_ / "relations"); + +#ifdef ENABLE_EXPERIMENTAL + r->add(root_ / "changeset" / osm_id_); +#endif /* ENABLE_EXPERIMENTAL */ } #ifdef ENABLE_API07 diff --git a/src/xml_formatter.cpp b/src/xml_formatter.cpp index ad2cdd549..db01315c2 100644 --- a/src/xml_formatter.cpp +++ b/src/xml_formatter.cpp @@ -148,6 +148,32 @@ void xml_formatter::write_relation(const element_info &elem, writer->end(); } +void xml_formatter::write_changeset(const changeset_info &elem, const tags_t &tags) { + writer->start("changeset"); + + writer->attribute("id", elem.id); + writer->attribute("created_at", elem.created_at); + writer->attribute("closed_at", elem.closed_at); + writer->attribute("open", elem.open); + + if (bool(elem.display_name) && bool(elem.uid)) { + writer->attribute("user", elem.display_name.get()); + writer->attribute("uid", elem.uid.get()); + } + + if (elem.bounding_box) { + writer->attribute("min_lat", elem.bounding_box->minlat); + writer->attribute("min_lon", elem.bounding_box->minlon); + writer->attribute("max_lat", elem.bounding_box->maxlat); + writer->attribute("max_lon", elem.bounding_box->maxlon); + } + + writer->attribute("comments_count", elem.comments_count); + + write_tags(tags); + writer->end(); +} + void xml_formatter::flush() { writer->flush(); } void xml_formatter::error(const std::string &s) { writer->error(s); } diff --git a/test/changesets.testcore/data.osm b/test/changesets.testcore/data.osm new file mode 100644 index 000000000..8b2b87f3c --- /dev/null +++ b/test/changesets.testcore/data.osm @@ -0,0 +1,11 @@ + + + + + + First post!!!! + + + + + diff --git a/test/changesets.testcore/read.case b/test/changesets.testcore/read.case new file mode 100644 index 000000000..199cec857 --- /dev/null +++ b/test/changesets.testcore/read.case @@ -0,0 +1,13 @@ +Request-Method: GET +Request-URI: /api/0.6/changeset/1 +HTTP-X-Error-Format: xml +--- +Status: 200 OK +Content-Type: text/xml; charset=utf-8 +!Content-Disposition: +--- + + + + + diff --git a/test/json.testcore/node_1.case b/test/json.testcore/node_1.case index 0ba62ed82..01065cc3d 100644 --- a/test/json.testcore/node_1.case +++ b/test/json.testcore/node_1.case @@ -11,6 +11,7 @@ Status: 200 OK "copyright": "***", "attribution": "***", "license": "***", + "changesets": [], "nodes": [ { "id": 1, "visible": true, diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index b79cd198d..5ed08508b 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -285,6 +285,7 @@ struct test_formatter : public output_formatter { const tags_t &tags) {} void write_relation(const element_info &elem, const members_t &members, const tags_t &tags) {} + void write_changeset(const changeset_info &elem, const tags_t &tags) {} void flush() {} void error(const std::exception &e) { diff --git a/test/test_core.cpp b/test/test_core.cpp index d3d553a2f..3797839db 100644 --- a/test/test_core.cpp +++ b/test/test_core.cpp @@ -566,6 +566,9 @@ int main(int argc, char *argv[]) { po::variables_map vm; vm.insert(std::make_pair(std::string("file"), po::variable_value(data_file.native(), false))); + vm.insert(std::make_pair(std::string("time"), + po::variable_value( + boost::posix_time::to_simple_string(now)))); boost::shared_ptr data_backend = make_staticxml_backend(); boost::shared_ptr factory = From c0336699ead8baec4bc0603cccfbb26b37141de0 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Tue, 1 Sep 2015 00:23:07 +0100 Subject: [PATCH 02/17] Added time to request object, added test for parsing ISO 8601 format times, made tests pass. Still WIP for getting discussions working. --- .gitignore | 1 + Makefile.am | 2 +- .../apidb/writeable_pgsql_selection.hpp | 2 + include/cgimap/fcgi_request.hpp | 9 +++- include/cgimap/json_formatter.hpp | 3 +- include/cgimap/output_formatter.hpp | 20 ++++--- include/cgimap/request.hpp | 4 ++ include/cgimap/time.hpp | 9 ++++ include/cgimap/xml_formatter.hpp | 3 +- src/Makefile.am | 9 +++- .../apidb/writeable_pgsql_selection.cpp | 13 +++-- src/backend/staticxml/staticxml.cpp | 9 +++- src/fcgi_request.cpp | 12 ++++- src/json_formatter.cpp | 6 +-- src/main.cpp | 4 +- src/osm_current_responder.cpp | 2 + src/output_formatter.cpp | 25 ++++++--- src/process_request.cpp | 7 ++- src/time.cpp | 53 +++++++++++++++++++ src/xml_formatter.cpp | 6 ++- test/changesets.testcore/data.osm | 2 +- test/changesets.testcore/read.case | 2 +- test/test_apidb_backend.cpp | 3 +- test/test_core.cpp | 16 ++++-- test/test_oauth.cpp | 6 +++ test/test_parse_id_list.cpp | 4 ++ test/test_parse_time.cpp | 35 ++++++++++++ 27 files changed, 222 insertions(+), 45 deletions(-) create mode 100644 include/cgimap/time.hpp create mode 100644 src/time.cpp create mode 100644 test/test_parse_time.cpp diff --git a/.gitignore b/.gitignore index 4abcaf7c9..f6718af19 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,7 @@ test/test_apidb_backend test/test_parse_id_list test/test_http test/test_oauth +test/test_parse_time test/*.trs include/cgimap/config.hpp.in include/cgimap/config.hpp diff --git a/Makefile.am b/Makefile.am index 1e8f01679..23d6c8b82 100644 --- a/Makefile.am +++ b/Makefile.am @@ -73,7 +73,7 @@ endif TESTS = test/map.testcore test/node.testcore test/anon.testcore test/way.testcore test/relation.testcore TESTS += test/empty.testcore test/way_full.testcore test/relation_full.testcore -TESTS += test/test_parse_id_list test/test_oauth test/test_http +TESTS += test/test_parse_id_list test/test_oauth test/test_http test/test_parse_time if ENABLE_EXPERIMENTAL TESTS += test/node_ways.testcore test/changesets.testcore endif diff --git a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp index e63de12ba..c8e481643 100644 --- a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp +++ b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp @@ -21,7 +21,9 @@ class writeable_pgsql_selection : public data_selection { void write_nodes(output_formatter &formatter); void write_ways(output_formatter &formatter); void write_relations(output_formatter &formatter); +#ifdef ENABLE_EXPERIMENTAL void write_changesets(output_formatter &formatter, const boost::posix_time::ptime &now); +#endif /* ENABLE_EXPERIMENTAL */ visibility_t check_node_visibility(osm_nwr_id_t id); visibility_t check_way_visibility(osm_nwr_id_t id); diff --git a/include/cgimap/fcgi_request.hpp b/include/cgimap/fcgi_request.hpp index 63d499490..0c279224c 100644 --- a/include/cgimap/fcgi_request.hpp +++ b/include/cgimap/fcgi_request.hpp @@ -5,10 +5,17 @@ #include struct fcgi_request : public request { - explicit fcgi_request(int socket); + fcgi_request(int socket, const boost::posix_time::ptime &now); virtual ~fcgi_request(); const char *get_param(const char *key); + // getting and setting the current time + boost::posix_time::ptime get_current_time() const; + // need to be able to set the time, since the fcgi_request is + // actually wrapping the whole socket and so persists over + // several calls. + void set_current_time(const boost::posix_time::ptime &now); + int accept_r(); static int open_socket(const std::string &, int); void dispose(); diff --git a/include/cgimap/json_formatter.hpp b/include/cgimap/json_formatter.hpp index 8704739ca..95f653098 100644 --- a/include/cgimap/json_formatter.hpp +++ b/include/cgimap/json_formatter.hpp @@ -38,7 +38,8 @@ class json_formatter : public output_formatter { const tags_t &tags); void write_changeset(const changeset_info &elem, - const tags_t &tags); + const tags_t &tags, + const boost::posix_time::ptime &now); void flush(); void error(const std::string &); diff --git a/include/cgimap/output_formatter.hpp b/include/cgimap/output_formatter.hpp index c3496786d..b6735653a 100644 --- a/include/cgimap/output_formatter.hpp +++ b/include/cgimap/output_formatter.hpp @@ -7,6 +7,7 @@ #include #include #include +#include /** * What type of element the formatter is starting to write. @@ -48,13 +49,21 @@ struct changeset_info { const boost::optional &display_name_, const boost::optional &bounding_box_, size_t num_changes_, - size_t comments_count_, - bool open_); + size_t comments_count_); + + // returns true if the changeset is "open" at a particular + // point in time. + // + // note that the definition of "open" is fraught with + // difficulty, and it's not wise to rely on it too much. + bool is_open_at(const boost::posix_time::ptime &) const; + // standard meaning of ID osm_changeset_id_t id; // changesets are created at a certain time and may be either // closed explicitly with a closing time, or close implicitly - // an hour after the last update to the changeset. + // an hour after the last update to the changeset. closed_at + // should have an ISO 8601 format: YYYY-MM-DDTHH:MM:SSZ std::string created_at, closed_at; // anonymous objects don't have UIDs or display names boost::optional uid; @@ -68,8 +77,6 @@ struct changeset_info { // if the changeset has a discussion attached, then this will // be the number of comments. size_t comments_count; - // is the changeset open (true) or closed (false) - bool open; }; struct member_info { @@ -133,7 +140,8 @@ struct output_formatter { // output a single changeset. virtual void write_changeset(const changeset_info &elem, - const tags_t &tags) = 0; + const tags_t &tags, + const boost::posix_time::ptime &now) = 0; // flush the current state virtual void flush() = 0; diff --git a/include/cgimap/request.hpp b/include/cgimap/request.hpp index d5e87c098..b7f4a8c4a 100644 --- a/include/cgimap/request.hpp +++ b/include/cgimap/request.hpp @@ -4,6 +4,7 @@ #include #include #include +#include // forward declaration of output_buffer, which is only needed here by // reference. @@ -29,6 +30,9 @@ struct request { // the key could not be found. this function can be called at any time. virtual const char *get_param(const char *key) = 0; + // get the current time of the request. + virtual boost::posix_time::ptime get_current_time() const = 0; + /********************** RESPONSE HEADER FUNCTIONS **************************/ // set the status for the response. by default, the status is 500 and the user diff --git a/include/cgimap/time.hpp b/include/cgimap/time.hpp new file mode 100644 index 000000000..cca26fc4d --- /dev/null +++ b/include/cgimap/time.hpp @@ -0,0 +1,9 @@ +#ifndef UTIL_TIME_HPP +#define UTIL_TIME_HPP + +#include + +// parse a time string (ISO 8601 - YYYY-MM-DDTHH:MM:SSZ) +boost::posix_time::ptime parse_time(const std::string &); + +#endif /* UTIL_TIME_HPP */ diff --git a/include/cgimap/xml_formatter.hpp b/include/cgimap/xml_formatter.hpp index aab9e948e..0704b5ab7 100644 --- a/include/cgimap/xml_formatter.hpp +++ b/include/cgimap/xml_formatter.hpp @@ -38,7 +38,8 @@ class xml_formatter : public output_formatter { const tags_t &tags); void write_changeset(const changeset_info &elem, - const tags_t &tags); + const tags_t &tags, + const boost::posix_time::ptime &now); void flush(); void error(const std::string &); diff --git a/src/Makefile.am b/src/Makefile.am index bab728e5c..a91700237 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -36,10 +36,11 @@ endif ___openstreetmap_cgimap_LDADD+=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @LIBPQXX_LIBS@ @CRYPTOPP_LIBS@ ___test_test_core_LDADD+=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @CRYPTOPP_LIBS@ -check_PROGRAMS+=../test/test_parse_id_list ../test/test_oauth ../test/test_http +check_PROGRAMS+=../test/test_parse_id_list ../test/test_oauth ../test/test_http ../test/test_parse_time ___test_test_parse_id_list_LDADD=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @LIBPQXX_LIBS@ @CRYPTOPP_LIBS@ ___test_test_oauth_LDADD=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @BOOST_LOCALE_LIB@ @LIBPQXX_LIBS@ @CRYPTOPP_LIBS@ ___test_test_http_LDADD=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @BOOST_LOCALE_LIB@ @LIBPQXX_LIBS@ @CRYPTOPP_LIBS@ +___test_test_parse_time_LDADD=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_DATE_TIME_LIB@ @BOOST_SYSTEM_LIB@ @LIBPQXX_LIBS@ if ENABLE_APIDB ___test_test_apidb_backend_LDADD+=libcgimap_core.la @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @BOOST_LOCALE_LIB@ @LIBPQXX_LIBS@ @CRYPTOPP_LIBS@ @@ -61,6 +62,9 @@ ___test_test_oauth_SOURCES=\ ___test_test_http_SOURCES=\ ../test/test_http.cpp +___test_test_parse_time_SOURCES=\ + ../test/test_parse_time.cpp + if ENABLE_APIDB ___test_test_apidb_backend_SOURCES=\ ../test/test_apidb_backend.cpp @@ -90,10 +94,11 @@ libcgimap_core_la_SOURCES=\ request_helpers.cpp \ router.cpp \ routes.cpp \ + time.cpp \ xml_formatter.cpp \ xml_writer.cpp \ zlib.cpp -libcgimap_core_la_LIBADD=@LIBXML_LIBS@ @YAJL_LIBS@ @LIBMEMCACHED_LIBS@ @BOOST_REGEX_LIB@ @BOOST_DATE_TIME_LIB@ @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @BOOST_LOCALE_LIB@ +libcgimap_core_la_LIBADD=@LIBXML_LIBS@ @YAJL_LIBS@ @LIBMEMCACHED_LIBS@ @BOOST_REGEX_LIB@ @BOOST_DATE_TIME_LIB@ @BOOST_PROGRAM_OPTIONS_LIB@ @BOOST_FILESYSTEM_LIB@ @BOOST_SYSTEM_LIB@ @BOOST_LOCALE_LIB@ @CRYPTOPP_LIBS@ libcgimap_core_la_SOURCES+=\ api06/map_handler.cpp \ diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index 2a8305c92..16c0e57fb 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -159,8 +159,7 @@ boost::optional extract_optional(const pqxx::result::field &f) { void extract_changeset(const pqxx::result::tuple &row, changeset_info &elem, - cache &changeset_cache, - const pt::ptime &now) { + cache &changeset_cache) { elem.id = row["id"].as(); elem.created_at = row["created_at"].c_str(); elem.closed_at = row["closed_at"].c_str(); @@ -188,9 +187,7 @@ void extract_changeset(const pqxx::result::tuple &row, elem.bounding_box = boost::none; } - const size_t num_changes = row["num_changes"].as(); - const pt::ptime closed_at_time = pt::time_from_string(elem.closed_at); - elem.open = (closed_at_time > now) && (num_changes <= MAX_ELEMENTS); + elem.num_changes = row["num_changes"].as(); } void extract_tags(const pqxx::result &res, tags_t &tags) { @@ -319,6 +316,7 @@ void writeable_pgsql_selection::write_relations(output_formatter &formatter) { } } +#ifdef ENABLE_EXPERIMENTAL void writeable_pgsql_selection::write_changesets(output_formatter &formatter, const pt::ptime &now) { changeset_info elem; @@ -327,11 +325,12 @@ void writeable_pgsql_selection::write_changesets(output_formatter &formatter, pqxx::result changesets = w.prepared("extract_changesets").exec(); for (pqxx::result::const_iterator itr = changesets.begin(); itr != changesets.end(); ++itr) { - extract_changeset(*itr, elem, cc, now); + extract_changeset(*itr, elem, cc); extract_tags(w.prepared("extract_changeset_tags")(elem.id).exec(), tags); - formatter.write_changeset(elem, tags); + formatter.write_changeset(elem, tags, now); } } +#endif /* ENABLE_EXPERIMENTAL */ data_selection::visibility_t writeable_pgsql_selection::check_node_visibility(osm_nwr_id_t id) { diff --git a/src/backend/staticxml/staticxml.cpp b/src/backend/staticxml/staticxml.cpp index aaf54afbd..ffceadbbe 100644 --- a/src/backend/staticxml/staticxml.cpp +++ b/src/backend/staticxml/staticxml.cpp @@ -226,6 +226,10 @@ struct xml_parser { parser->m_cur_rel->m_members.push_back(m); } + } else if (strncmp((const char *)name, "comment", 8) == 0) { + if (parser->m_cur_changeset != NULL) { + parser->m_cur_changeset->m_info.comments_count += 1; + } } } @@ -308,12 +312,13 @@ struct static_data_selection : public data_selection { } #ifdef ENABLE_EXPERIMENTAL - virtual void write_changesets(output_formatter &formatter) { + virtual void write_changesets(output_formatter &formatter, + const pt::ptime &now) { BOOST_FOREACH(osm_changeset_id_t id, m_changesets) { std::map::iterator itr = m_db->m_changesets.find(id); if (itr != m_db->m_changesets.end()) { const changeset &c = itr->second; - formatter.write_changeset(c.m_info, c.m_tags); + formatter.write_changeset(c.m_info, c.m_tags, now); } } } diff --git a/src/fcgi_request.cpp b/src/fcgi_request.cpp index a75286a4e..2848284a1 100644 --- a/src/fcgi_request.cpp +++ b/src/fcgi_request.cpp @@ -39,9 +39,10 @@ struct fcgi_buffer : public output_buffer { struct fcgi_request::pimpl { FCGX_Request req; + boost::posix_time::ptime now; }; -fcgi_request::fcgi_request(int socket) : m_impl(new pimpl) { +fcgi_request::fcgi_request(int socket, const boost::posix_time::ptime &now) : m_impl(new pimpl) { // initialise FCGI if (FCGX_Init() != 0) { throw runtime_error("Couldn't initialise FCGX library."); @@ -49,6 +50,7 @@ fcgi_request::fcgi_request(int socket) : m_impl(new pimpl) { if (FCGX_InitRequest(&m_impl->req, socket, FCGI_FAIL_ACCEPT_ON_INTR) != 0) { throw runtime_error("Couldn't initialise FCGX request structure."); } + m_impl->now = now; m_buffer = boost::shared_ptr(new fcgi_buffer(m_impl->req)); } @@ -58,6 +60,14 @@ const char *fcgi_request::get_param(const char *key) { return FCGX_GetParam(key, m_impl->req.envp); } +boost::posix_time::ptime fcgi_request::get_current_time() const { + return m_impl->now; +} + +void fcgi_request::set_current_time(const boost::posix_time::ptime &now) { + m_impl->now = now; +} + void fcgi_request::write_header_info(int status, const request::headers_t &headers) { std::ostringstream ostr; diff --git a/src/json_formatter.cpp b/src/json_formatter.cpp index 845d91a0c..a9796de86 100644 --- a/src/json_formatter.cpp +++ b/src/json_formatter.cpp @@ -6,6 +6,7 @@ using boost::shared_ptr; using std::string; using std::transform; +namespace pt = boost::posix_time; namespace { @@ -177,10 +178,9 @@ void json_formatter::write_relation(const element_info &elem, writer->end_object(); } -void json_formatter::write_changeset(const changeset_info &elem, const tags_t &tags) { +void json_formatter::write_changeset(const changeset_info &elem, const tags_t &tags, + const pt::ptime &now) { writer->start_object(); - - write_tags(tags); writer->end_object(); } diff --git a/src/main.cpp b/src/main.cpp index 1869bda3d..8e5ad2f4d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -140,7 +140,7 @@ static void process_requests(int socket, const po::variables_map &options) { routes route; // create the request object (persists over several calls) - fcgi_request req(socket); + fcgi_request req(socket, pt::ptime()); // create a factory for data selections - the mechanism for actually // getting at data. @@ -163,6 +163,8 @@ static void process_requests(int socket, const po::variables_map &options) { // get the next request if (req.accept_r() >= 0) { + pt::ptime now(pt::second_clock::local_time()); + req.set_current_time(now); process_request(req, limiter, generator, route, factory, oauth_store); } } diff --git a/src/osm_current_responder.cpp b/src/osm_current_responder.cpp index 7d0b48389..ba4a39a06 100644 --- a/src/osm_current_responder.cpp +++ b/src/osm_current_responder.cpp @@ -23,10 +23,12 @@ void osm_current_responder::write(shared_ptr formatter, fmt.write_bounds(bounds.get()); } +#ifdef ENABLE_EXPERIMENTAL // write all selected changesets fmt.start_element_type(element_type_changeset); sel->write_changesets(fmt, now); fmt.end_element_type(element_type_changeset); +#endif /* ENABLE_EXPERIMENTAL */ // write all selected nodes fmt.start_element_type(element_type_node); diff --git a/src/output_formatter.cpp b/src/output_formatter.cpp index 631a235c0..a5ed243f8 100644 --- a/src/output_formatter.cpp +++ b/src/output_formatter.cpp @@ -1,4 +1,13 @@ #include "cgimap/output_formatter.hpp" +#include "cgimap/time.hpp" +#include + +namespace pt = boost::posix_time; + +// maximum number of element versions which can be associated +// with a single changeset. this is hard-coded in the API, +// see app/models/changeset.rb for the details. +#define MAX_CHANGESET_ELEMENTS (50000) element_info::element_info() : id(0), version(0), changeset(0), @@ -23,14 +32,14 @@ element_info::element_info(osm_nwr_id_t id_, osm_nwr_id_t version_, changeset_info::changeset_info() : id(0), created_at(""), closed_at(""), uid(), display_name(), bounding_box(), num_changes(0), - comments_count(0), open(false) {} + comments_count(0) {} changeset_info::changeset_info(const changeset_info &other) : id(other.id), created_at(other.created_at), closed_at(other.closed_at), uid(other.uid), display_name(other.display_name), bounding_box(other.bounding_box), - num_changes(other.num_changes), comments_count(other.comments_count), - open(other.open) {} + num_changes(other.num_changes), + comments_count(other.comments_count) {} changeset_info::changeset_info( osm_changeset_id_t id_, @@ -40,11 +49,15 @@ changeset_info::changeset_info( const boost::optional &display_name_, const boost::optional &bounding_box_, size_t num_changes_, - size_t comments_count_, - bool open_) + size_t comments_count_) : id(id_), created_at(created_at_), closed_at(closed_at_), uid(uid_), display_name(display_name_), bounding_box(bounding_box_), num_changes(num_changes_), - comments_count(comments_count_), open(open_) {} + comments_count(comments_count_) {} + +bool changeset_info::is_open_at(const pt::ptime &now) const { + const pt::ptime closed_at_time = parse_time(closed_at); + return (closed_at_time > now) && (num_changes < MAX_CHANGESET_ELEMENTS); +} output_formatter::~output_formatter() {} diff --git a/src/process_request.cpp b/src/process_request.cpp index acb5b766a..92a098a26 100644 --- a/src/process_request.cpp +++ b/src/process_request.cpp @@ -76,8 +76,7 @@ void process_not_allowed(request &req) { boost::tuple process_get_request(request &req, routes &route, boost::shared_ptr factory, - const string &ip, const string &generator, - const pt::ptime &now) { + const string &ip, const string &generator) { // figure how to handle the request handler_ptr_t handler = route(req); @@ -112,7 +111,7 @@ process_get_request(request &req, routes &route, try { // call to write the response - responder->write(o_formatter, generator, now); + responder->write(o_formatter, generator, req.get_current_time()); // ensure the request is finished req.finish(); @@ -317,7 +316,7 @@ void process_request(request &req, rate_limiter &limiter, // process request if (method == "GET") { boost::tie(request_name, bytes_written) = - process_get_request(req, route, factory, ip, generator, start_time); + process_get_request(req, route, factory, ip, generator); } else if (method == "HEAD") { boost::tie(request_name, bytes_written) = diff --git a/src/time.cpp b/src/time.cpp new file mode 100644 index 000000000..f20cc5a19 --- /dev/null +++ b/src/time.cpp @@ -0,0 +1,53 @@ +#include "cgimap/time.hpp" +#include +#include +#include +#include + +boost::posix_time::ptime parse_time(const std::string &s) { + // parse only YYYY-MM-DDTHH:MM:SSZ + if ((s.size() == 20) && + (isdigit(s[0]) != 0) && + (isdigit(s[1]) != 0) && + (isdigit(s[2]) != 0) && + (isdigit(s[3]) != 0) && + (s[4] == '-') && + (isdigit(s[5]) != 0) && + (isdigit(s[6]) != 0) && + (s[7] == '-') && + (isdigit(s[8]) != 0) && + (isdigit(s[9]) != 0) && + (s[10] == 'T') && + (isdigit(s[11]) != 0) && + (isdigit(s[12]) != 0) && + (s[13] == ':') && + (isdigit(s[14]) != 0) && + (isdigit(s[15]) != 0) && + (s[16] == ':') && + (isdigit(s[17]) != 0) && + (isdigit(s[18]) != 0) && + (s[19] == 'Z')) { + return boost::posix_time::ptime( + boost::gregorian::date( + int(s[0] - '0') * 1000 + + int(s[1] - '0') * 100 + + int(s[2] - '0') * 10 + + int(s[3] - '0'), + int(s[5] - '0') * 10 + + int(s[6] - '0'), + int(s[8] - '0') * 10 + + int(s[9] - '0')), + boost::posix_time::time_duration( + int(s[11] - '0') * 10 + + int(s[12] - '0'), + int(s[14] - '0') * 10 + + int(s[15] - '0'), + int(s[17] - '0') * 10 + + int(s[18] - '0'))); + } + + std::ostringstream ostr; + ostr << "Unable to parse string '" << s << "' as an ISO 8601 format date time."; + throw std::runtime_error(ostr.str()); +} + diff --git a/src/xml_formatter.cpp b/src/xml_formatter.cpp index db01315c2..27e66815a 100644 --- a/src/xml_formatter.cpp +++ b/src/xml_formatter.cpp @@ -7,6 +7,7 @@ using std::string; using boost::shared_ptr; using std::transform; +namespace pt = boost::posix_time; namespace { @@ -148,13 +149,14 @@ void xml_formatter::write_relation(const element_info &elem, writer->end(); } -void xml_formatter::write_changeset(const changeset_info &elem, const tags_t &tags) { +void xml_formatter::write_changeset(const changeset_info &elem, const tags_t &tags, + const pt::ptime &now) { writer->start("changeset"); writer->attribute("id", elem.id); writer->attribute("created_at", elem.created_at); writer->attribute("closed_at", elem.closed_at); - writer->attribute("open", elem.open); + writer->attribute("open", elem.is_open_at(now)); if (bool(elem.display_name) && bool(elem.uid)) { writer->attribute("user", elem.display_name.get()); diff --git a/test/changesets.testcore/data.osm b/test/changesets.testcore/data.osm index 8b2b87f3c..1fbb6903a 100644 --- a/test/changesets.testcore/data.osm +++ b/test/changesets.testcore/data.osm @@ -1,5 +1,5 @@ - + diff --git a/test/changesets.testcore/read.case b/test/changesets.testcore/read.case index 199cec857..f0a5929ea 100644 --- a/test/changesets.testcore/read.case +++ b/test/changesets.testcore/read.case @@ -7,7 +7,7 @@ Content-Type: text/xml; charset=utf-8 !Content-Disposition: --- - + diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index 5ed08508b..f5d7fb6f6 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -285,7 +285,8 @@ struct test_formatter : public output_formatter { const tags_t &tags) {} void write_relation(const element_info &elem, const members_t &members, const tags_t &tags) {} - void write_changeset(const changeset_info &elem, const tags_t &tags) {} + void write_changeset(const changeset_info &elem, const tags_t &tags, + const boost::posix_time::ptime &) {} void flush() {} void error(const std::exception &e) { diff --git a/test/test_core.cpp b/test/test_core.cpp index 3797839db..471462280 100644 --- a/test/test_core.cpp +++ b/test/test_core.cpp @@ -3,6 +3,7 @@ #include "cgimap/backend/staticxml/staticxml.hpp" #include "cgimap/request_helpers.hpp" #include "cgimap/config.hpp" +#include "cgimap/time.hpp" #include #include @@ -22,6 +23,7 @@ namespace fs = boost::filesystem; namespace po = boost::program_options; namespace al = boost::algorithm; namespace pt = boost::property_tree; +namespace bt = boost::posix_time; /** * Mock output buffer so that we can get back an in-memory result as a string @@ -71,6 +73,9 @@ struct test_request : public request { } std::stringstream &buffer() { return m_output; } + bt::ptime get_current_time() const { return m_now; } + void set_current_time(const bt::ptime &now) { m_now = now; } + protected: virtual void write_header_info(int status, const headers_t &headers) { assert(m_output.tellp() == 0); @@ -88,6 +93,7 @@ struct test_request : public request { private: std::stringstream m_output; std::map m_params; + bt::ptime m_now; }; std::map read_headers(std::istream &in, @@ -145,7 +151,12 @@ void setup_request_headers(test_request &req, std::istream &in) { al::to_upper(key); al::replace_all(key, "-", "_"); - req.set_header(key, val.second); + if (key == "DATE") { + req.set_current_time(parse_time(val.second)); + + } else { + req.set_header(key, val.second); + } } // always set the remote addr variable @@ -566,9 +577,6 @@ int main(int argc, char *argv[]) { po::variables_map vm; vm.insert(std::make_pair(std::string("file"), po::variable_value(data_file.native(), false))); - vm.insert(std::make_pair(std::string("time"), - po::variable_value( - boost::posix_time::to_simple_string(now)))); boost::shared_ptr data_backend = make_staticxml_backend(); boost::shared_ptr factory = diff --git a/test/test_oauth.cpp b/test/test_oauth.cpp index 1539cd4aa..956a5c48a 100644 --- a/test/test_oauth.cpp +++ b/test/test_oauth.cpp @@ -53,6 +53,8 @@ struct test_request : public request { const char *get_param(const char *key); void dispose(); + boost::posix_time::ptime get_current_time() const; + protected: void write_header_info(int status, const headers_t &headers); @@ -115,6 +117,10 @@ void test_request::finish_internal() { throw std::runtime_error("test_request::finish_internal unimplemented."); } +boost::posix_time::ptime test_request::get_current_time() const { + return boost::posix_time::ptime(); +} + } // anonymous namespace void oauth_check_signature_base_string() { diff --git a/test/test_parse_id_list.cpp b/test/test_parse_id_list.cpp index fd44fff02..bf857781d 100644 --- a/test/test_parse_id_list.cpp +++ b/test/test_parse_id_list.cpp @@ -30,6 +30,10 @@ struct test_request : public request { } std::stringstream &buffer() { assert(false); } + boost::posix_time::ptime get_current_time() const { + return boost::posix_time::ptime(); + } + protected: virtual void write_header_info(int status, const headers_t &headers) {} virtual boost::shared_ptr get_buffer_internal() { diff --git a/test/test_parse_time.cpp b/test/test_parse_time.cpp new file mode 100644 index 000000000..a00009f19 --- /dev/null +++ b/test/test_parse_time.cpp @@ -0,0 +1,35 @@ +#include "cgimap/time.hpp" +#include +#include +#include +#include + +namespace pt = boost::posix_time; +namespace gg = boost::gregorian; + +void assert_eq_time(pt::ptime expected, std::string str) { + const pt::ptime tb = parse_time(str); + if (expected != tb) { + std::ostringstream ostr; + ostr << "Expected " << boost::posix_time::to_simple_string(expected) << ", but got time " + << boost::posix_time::to_simple_string(tb) << " [from '" << str << "'] instead."; + throw std::runtime_error(ostr.str()); + } +} + +int main(int argc, char *argv[]) { + try { + assert_eq_time(pt::ptime(gg::date(2015, 8, 31), pt::time_duration(23, 40, 10)), + "2015-08-31T23:40:10Z"); + + } catch (const std::exception &ex) { + std::cerr << "EXCEPTION: " << ex.what() << std::endl; + return 1; + + } catch (...) { + std::cerr << "UNKNOWN ERROR" << std::endl; + return 1; + } + + return 0; +} From c9d8dde4a317d1f7e946fc84af1592ee9adaf665 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Wed, 2 Sep 2015 00:06:14 +0100 Subject: [PATCH 03/17] Added changeset discussion dumps and test cases. --- include/cgimap/api06/changeset_handler.hpp | 4 +- .../apidb/writeable_pgsql_selection.hpp | 7 +++ include/cgimap/data_selection.hpp | 5 ++ include/cgimap/json_formatter.hpp | 2 + include/cgimap/output_formatter.hpp | 10 ++++ include/cgimap/xml_formatter.hpp | 2 + src/api06/changeset_handler.cpp | 38 ++++++++++++-- .../apidb/writeable_pgsql_selection.cpp | 14 +++++- src/backend/staticxml/staticxml.cpp | 49 +++++++++++++++++-- src/data_selection.cpp | 3 ++ src/json_formatter.cpp | 5 +- src/xml_formatter.cpp | 29 +++++++++-- test/changesets.testcore/data.osm | 2 +- test/changesets.testcore/read.case | 5 +- test/changesets.testcore/read_discussion.case | 19 +++++++ test/changesets.testcore/read_open.case | 14 ++++++ test/structure.sql | 2 + test/test_apidb_backend.cpp | 1 + test/test_core.cpp | 2 +- 19 files changed, 195 insertions(+), 18 deletions(-) create mode 100644 test/changesets.testcore/read_discussion.case create mode 100644 test/changesets.testcore/read_open.case diff --git a/include/cgimap/api06/changeset_handler.hpp b/include/cgimap/api06/changeset_handler.hpp index ae803d9ef..c2bbcda12 100644 --- a/include/cgimap/api06/changeset_handler.hpp +++ b/include/cgimap/api06/changeset_handler.hpp @@ -10,11 +10,12 @@ namespace api06 { class changeset_responder : public osm_current_responder { public: - changeset_responder(mime::type, osm_changeset_id_t, factory_ptr &); + changeset_responder(mime::type, osm_changeset_id_t, bool, factory_ptr &); ~changeset_responder(); private: osm_changeset_id_t id; + bool include_discussion; }; class changeset_handler : public handler { @@ -27,6 +28,7 @@ class changeset_handler : public handler { private: osm_changeset_id_t id; + bool include_discussion; }; } // namespace api06 diff --git a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp index c8e481643..45735123b 100644 --- a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp +++ b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp @@ -45,6 +45,7 @@ class writeable_pgsql_selection : public data_selection { #ifdef ENABLE_EXPERIMENTAL bool supports_changesets(); int select_changesets(const std::vector &); + void select_changeset_discussions(); #endif /* ENABLE_EXPERIMENTAL */ /** @@ -76,6 +77,12 @@ class writeable_pgsql_selection : public data_selection { // true if a query hasn't been run yet, i.e: it's possible to // assume that all the temporary tables are empty. bool m_tables_empty; + +#ifdef ENABLE_EXPERIMENTAL + // true if we want to include changeset discussions along with + // the changesets themselves. defaults to false. + bool include_changeset_discussions; +#endif /* ENABLE_EXPERIMENTAL */ }; #endif /* WRITEABLE_PGSQL_SELECTION_HPP */ diff --git a/include/cgimap/data_selection.hpp b/include/cgimap/data_selection.hpp index 8a48a36ce..fd5246bc8 100644 --- a/include/cgimap/data_selection.hpp +++ b/include/cgimap/data_selection.hpp @@ -96,6 +96,11 @@ class data_selection { /// select specified changesets, returning the number of /// changesets selected. virtual int select_changesets(const std::vector &); + + /// select the changeset discussions as well. this effectively + /// just sets a flag - by default, discussions are not included, + /// if this is called then discussions will be included. + virtual void select_changeset_discussions(); #endif /* ENABLE_EXPERIMENTAL */ /** diff --git a/include/cgimap/json_formatter.hpp b/include/cgimap/json_formatter.hpp index 95f653098..a31b3c1e9 100644 --- a/include/cgimap/json_formatter.hpp +++ b/include/cgimap/json_formatter.hpp @@ -39,6 +39,8 @@ class json_formatter : public output_formatter { void write_changeset(const changeset_info &elem, const tags_t &tags, + bool include_comments, + const comments_t &comments, const boost::posix_time::ptime &now); void flush(); diff --git a/include/cgimap/output_formatter.hpp b/include/cgimap/output_formatter.hpp index b6735653a..79eda046c 100644 --- a/include/cgimap/output_formatter.hpp +++ b/include/cgimap/output_formatter.hpp @@ -79,6 +79,13 @@ struct changeset_info { size_t comments_count; }; +struct changeset_comment_info { + osm_user_id_t author_id; + std::string body; + std::string created_at; + std::string author_display_name; +}; + struct member_info { element_type type; osm_nwr_id_t ref; @@ -88,6 +95,7 @@ struct member_info { typedef std::list nodes_t; typedef std::list members_t; typedef std::list > tags_t; +typedef std::vector comments_t; /** * Base type for different output formats. Hopefully this is general @@ -141,6 +149,8 @@ struct output_formatter { // output a single changeset. virtual void write_changeset(const changeset_info &elem, const tags_t &tags, + bool include_comments, + const comments_t &comments, const boost::posix_time::ptime &now) = 0; // flush the current state diff --git a/include/cgimap/xml_formatter.hpp b/include/cgimap/xml_formatter.hpp index 0704b5ab7..fae6b99aa 100644 --- a/include/cgimap/xml_formatter.hpp +++ b/include/cgimap/xml_formatter.hpp @@ -39,6 +39,8 @@ class xml_formatter : public output_formatter { void write_changeset(const changeset_info &elem, const tags_t &tags, + bool include_comments, + const comments_t &comments, const boost::posix_time::ptime &now); void flush(); diff --git a/src/api06/changeset_handler.cpp b/src/api06/changeset_handler.cpp index 359c989ab..8b70983fd 100644 --- a/src/api06/changeset_handler.cpp +++ b/src/api06/changeset_handler.cpp @@ -1,4 +1,5 @@ #include "cgimap/api06/changeset_handler.hpp" +#include "cgimap/request_helpers.hpp" #include "cgimap/http.hpp" #include @@ -8,8 +9,11 @@ using std::vector; namespace api06 { -changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, factory_ptr &w_) - : osm_current_responder(mt, w_), id(id_) { +changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, + bool include_discussion_, + factory_ptr &w_) + : osm_current_responder(mt, w_), id(id_), + include_discussion(include_discussion_) { vector ids; ids.push_back(id); @@ -20,18 +24,44 @@ changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, if (sel->select_changesets(ids) == 0) { throw http::not_found(""); } + + if (include_discussion) { + sel->select_changeset_discussions(); + } } changeset_responder::~changeset_responder() {} -changeset_handler::changeset_handler(request &, osm_changeset_id_t id_) : id(id_) {} +namespace { +// functor to use in find_if to locate the "include_discussion" header +struct discussion_header_finder { + bool operator()(const std::pair &header) const { + static const std::string target("include_discussion"); + return header.first == target; + } +}; +} + +changeset_handler::changeset_handler(request &req, osm_changeset_id_t id_) + : id(id_), include_discussion(false) { + using std::vector; + using std::pair; + using std::string; + + string decoded = http::urldecode(get_query_string(req)); + const vector > params = http::parse_params(decoded); + vector >::const_iterator itr = + std::find_if(params.begin(), params.end(), discussion_header_finder()); + + include_discussion = (itr != params.end()); +} changeset_handler::~changeset_handler() {} std::string changeset_handler::log_name() const { return "changeset"; } responder_ptr_t changeset_handler::responder(factory_ptr &w) const { - return responder_ptr_t(new changeset_responder(mime_type, id, w)); + return responder_ptr_t(new changeset_responder(mime_type, id, include_discussion, w)); } } // namespace api06 diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index 16c0e57fb..938657ff1 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -251,7 +251,11 @@ void extract_members(const pqxx::result &res, members_t &members) { writeable_pgsql_selection::writeable_pgsql_selection( pqxx::connection &conn, cache &changeset_cache) - : w(conn), cc(changeset_cache) { + : w(conn), cc(changeset_cache) +#ifdef ENABLE_EXPERIMENTAL + , include_changeset_discussions(false) +#endif /* ENABLE_EXPERIMENTAL */ +{ w.exec("CREATE TEMPORARY TABLE tmp_nodes (id bigint PRIMARY KEY)"); w.exec("CREATE TEMPORARY TABLE tmp_ways (id bigint PRIMARY KEY)"); w.exec("CREATE TEMPORARY TABLE tmp_relations (id bigint PRIMARY KEY)"); @@ -321,13 +325,15 @@ void writeable_pgsql_selection::write_changesets(output_formatter &formatter, const pt::ptime &now) { changeset_info elem; tags_t tags; + comments_t comments; pqxx::result changesets = w.prepared("extract_changesets").exec(); for (pqxx::result::const_iterator itr = changesets.begin(); itr != changesets.end(); ++itr) { extract_changeset(*itr, elem, cc); extract_tags(w.prepared("extract_changeset_tags")(elem.id).exec(), tags); - formatter.write_changeset(elem, tags, now); + // extract_comments(w.prepared("extract_changeset_comments")(elem.id).exec(), comments); + formatter.write_changeset(elem, tags, include_changeset_discussions, comments, now); } } #endif /* ENABLE_EXPERIMENTAL */ @@ -437,6 +443,10 @@ bool writeable_pgsql_selection::supports_changesets() { int writeable_pgsql_selection::select_changesets(const std::vector &ids) { return w.prepared("add_changesets_list")(ids).exec().affected_rows(); } + +void writeable_pgsql_selection::select_changeset_discussions() { + include_changeset_discussions = true; +} #endif /* ENABLE_EXPERIMENTAL */ namespace { diff --git a/src/backend/staticxml/staticxml.cpp b/src/backend/staticxml/staticxml.cpp index ffceadbbe..998fb712b 100644 --- a/src/backend/staticxml/staticxml.cpp +++ b/src/backend/staticxml/staticxml.cpp @@ -60,6 +60,7 @@ struct relation { struct changeset { changeset_info m_info; tags_t m_tags; + comments_t m_comments; }; struct database { @@ -134,7 +135,8 @@ struct xml_parser { m_cur_way(NULL), m_cur_rel(NULL), m_cur_tags(NULL), - m_cur_changeset(NULL) + m_cur_changeset(NULL), + m_in_text(false) {} static void start_element(void *ctx, const xmlChar *name, @@ -229,10 +231,34 @@ struct xml_parser { } else if (strncmp((const char *)name, "comment", 8) == 0) { if (parser->m_cur_changeset != NULL) { parser->m_cur_changeset->m_info.comments_count += 1; + + changeset_comment_info info; + info.author_id = get_attribute("uid", 4, attributes); + info.author_display_name = get_attribute("user", 5, attributes); + info.created_at = get_attribute("date", 5, attributes); + parser->m_cur_changeset->m_comments.push_back(info); + } + } else if (strncmp((const char *)name, "text", 5) == 0) { + if ((parser->m_cur_changeset != NULL) && + (parser->m_cur_changeset->m_comments.size() > 0)) { + parser->m_in_text = true; } } } + static void end_element(void *ctx, const xmlChar *) { + xml_parser *parser = static_cast(ctx); + parser->m_in_text = false; + } + + static void characters(void *ctx, const xmlChar *str, int len) { + xml_parser *parser = static_cast(ctx); + + if (parser->m_in_text) { + parser->m_cur_changeset->m_comments.back().body.append((const char *)str, len); + } + } + static void warning(void *, const char *, ...) { // TODO } @@ -252,6 +278,7 @@ struct xml_parser { relation *m_cur_rel; tags_t *m_cur_tags; changeset *m_cur_changeset; + bool m_in_text; }; boost::shared_ptr parse_xml(const char *filename) { @@ -260,8 +287,10 @@ boost::shared_ptr parse_xml(const char *filename) { handler.initialized = XML_SAX2_MAGIC; handler.startElement = &xml_parser::start_element; + handler.endElement = &xml_parser::end_element; handler.warning = &xml_parser::warning; handler.error = &xml_parser::error; + handler.characters = &xml_parser::characters; boost::shared_ptr db = boost::make_shared(); xml_parser parser(db.get()); @@ -278,7 +307,12 @@ boost::shared_ptr parse_xml(const char *filename) { } struct static_data_selection : public data_selection { - explicit static_data_selection(boost::shared_ptr db) : m_db(db) {} + explicit static_data_selection(boost::shared_ptr db) + : m_db(db) +#ifdef ENABLE_EXPERIMENTAL + , m_include_changeset_comments(false) +#endif /* ENABLE_EXPERIMENTAL */ + {} virtual ~static_data_selection() {} virtual void write_nodes(output_formatter &formatter) { @@ -318,7 +352,9 @@ struct static_data_selection : public data_selection { std::map::iterator itr = m_db->m_changesets.find(id); if (itr != m_db->m_changesets.end()) { const changeset &c = itr->second; - formatter.write_changeset(c.m_info, c.m_tags, now); + formatter.write_changeset( + c.m_info, c.m_tags, m_include_changeset_comments, + c.m_comments, now); } } } @@ -535,6 +571,10 @@ struct static_data_selection : public data_selection { } return selected; } + + virtual void select_changeset_discussions() { + m_include_changeset_comments = true; + } #endif /* ENABLE_EXPERIMENTAL */ @@ -542,6 +582,9 @@ struct static_data_selection : public data_selection { boost::shared_ptr m_db; std::set m_changesets; std::set m_nodes, m_ways, m_relations; +#ifdef ENABLE_EXPERIMENTAL + bool m_include_changeset_comments; +#endif /* ENABLE_EXPERIMENTAL */ }; struct factory : public data_selection::factory { diff --git a/src/data_selection.cpp b/src/data_selection.cpp index 8cd245338..20dd75b7f 100644 --- a/src/data_selection.cpp +++ b/src/data_selection.cpp @@ -13,6 +13,9 @@ bool data_selection::supports_changesets() { int data_selection::select_changesets(const std::vector &) { return 0; } + +void data_selection::select_changeset_discussions() { +} #endif /* ENABLE_EXPERIMENTAL */ data_selection::factory::~factory() {} diff --git a/src/json_formatter.cpp b/src/json_formatter.cpp index a9796de86..861efe4c8 100644 --- a/src/json_formatter.cpp +++ b/src/json_formatter.cpp @@ -178,7 +178,10 @@ void json_formatter::write_relation(const element_info &elem, writer->end_object(); } -void json_formatter::write_changeset(const changeset_info &elem, const tags_t &tags, +void json_formatter::write_changeset(const changeset_info &elem, + const tags_t &tags, + bool include_comments, + const comments_t &comments, const pt::ptime &now) { writer->start_object(); write_tags(tags); diff --git a/src/xml_formatter.cpp b/src/xml_formatter.cpp index 27e66815a..bc397e4ff 100644 --- a/src/xml_formatter.cpp +++ b/src/xml_formatter.cpp @@ -149,14 +149,20 @@ void xml_formatter::write_relation(const element_info &elem, writer->end(); } -void xml_formatter::write_changeset(const changeset_info &elem, const tags_t &tags, +void xml_formatter::write_changeset(const changeset_info &elem, + const tags_t &tags, + bool include_comments, + const comments_t &comments, const pt::ptime &now) { writer->start("changeset"); writer->attribute("id", elem.id); writer->attribute("created_at", elem.created_at); - writer->attribute("closed_at", elem.closed_at); - writer->attribute("open", elem.is_open_at(now)); + const bool is_open = elem.is_open_at(now); + if (!is_open) { + writer->attribute("closed_at", elem.closed_at); + } + writer->attribute("open", is_open); if (bool(elem.display_name) && bool(elem.uid)) { writer->attribute("user", elem.display_name.get()); @@ -173,6 +179,23 @@ void xml_formatter::write_changeset(const changeset_info &elem, const tags_t &ta writer->attribute("comments_count", elem.comments_count); write_tags(tags); + + if (include_comments) { + writer->start("discussion"); + for (comments_t::const_iterator itr = comments.begin(); + itr != comments.end(); ++itr) { + writer->start("comment"); + writer->attribute("date", itr->created_at); + writer->attribute("uid", itr->author_id); + writer->attribute("user", itr->author_display_name); + writer->start("text"); + writer->text(itr->body); + writer->end(); + writer->end(); + } + writer->end(); + } + writer->end(); } diff --git a/test/changesets.testcore/data.osm b/test/changesets.testcore/data.osm index 1fbb6903a..050b03cff 100644 --- a/test/changesets.testcore/data.osm +++ b/test/changesets.testcore/data.osm @@ -1,5 +1,5 @@ - + diff --git a/test/changesets.testcore/read.case b/test/changesets.testcore/read.case index f0a5929ea..4df69fabc 100644 --- a/test/changesets.testcore/read.case +++ b/test/changesets.testcore/read.case @@ -1,13 +1,14 @@ Request-Method: GET Request-URI: /api/0.6/changeset/1 HTTP-X-Error-Format: xml +Date: 2015-08-09T11:33:13Z --- Status: 200 OK Content-Type: text/xml; charset=utf-8 !Content-Disposition: --- - - + + diff --git a/test/changesets.testcore/read_discussion.case b/test/changesets.testcore/read_discussion.case new file mode 100644 index 000000000..98f375c31 --- /dev/null +++ b/test/changesets.testcore/read_discussion.case @@ -0,0 +1,19 @@ +Request-Method: GET +Request-URI: /api/0.6/changeset/1?include_discussion=1 +HTTP-X-Error-Format: xml +Date: 2015-08-09T11:33:13Z +--- +Status: 200 OK +Content-Type: text/xml; charset=utf-8 +!Content-Disposition: +--- + + + + + + First post!!! + + + + diff --git a/test/changesets.testcore/read_open.case b/test/changesets.testcore/read_open.case new file mode 100644 index 000000000..23cc0731e --- /dev/null +++ b/test/changesets.testcore/read_open.case @@ -0,0 +1,14 @@ +Request-Method: GET +Request-URI: /api/0.6/changeset/1 +HTTP-X-Error-Format: xml +Date: 2015-08-09T10:33:13Z +--- +Status: 200 OK +Content-Type: text/xml; charset=utf-8 +!Content-Disposition: +--- + + + + + diff --git a/test/structure.sql b/test/structure.sql index d15a3c2ec..c3c722270 100644 --- a/test/structure.sql +++ b/test/structure.sql @@ -2,6 +2,8 @@ -- PostgreSQL database dump -- +SET statement_timeout = 0; +SET lock_timeout = 0; SET client_encoding = 'UTF8'; SET standard_conforming_strings = on; SET check_function_bodies = false; diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index f5d7fb6f6..899c2ccf9 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -286,6 +286,7 @@ struct test_formatter : public output_formatter { void write_relation(const element_info &elem, const members_t &members, const tags_t &tags) {} void write_changeset(const changeset_info &elem, const tags_t &tags, + bool include_comments, const comments_t &comments, const boost::posix_time::ptime &) {} void flush() {} diff --git a/test/test_core.cpp b/test/test_core.cpp index 471462280..27dde0932 100644 --- a/test/test_core.cpp +++ b/test/test_core.cpp @@ -190,7 +190,7 @@ void check_xmlattr(const pt::ptree &expected, const pt::ptree &actual) { if (act_keys.size() > exp_keys.size()) { BOOST_FOREACH(const std::string &ek, exp_keys) { act_keys.erase(ek); } std::ostringstream out; - out << "Missing attributes ["; + out << "Extra attributes ["; BOOST_FOREACH(const std::string &ak, act_keys) { out << ak << " "; } out << "]"; throw std::runtime_error(out.str()); From e9ed0dfcb7814b9211ed7fe383cf8b6e4957ff25 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Wed, 2 Sep 2015 22:58:21 +0100 Subject: [PATCH 04/17] Added changesets to the apidb test backend, but not yet used in any tests. --- include/cgimap/bbox.hpp | 2 + include/cgimap/output_formatter.hpp | 2 + src/Makefile.am | 4 +- src/bbox.cpp | 7 + src/output_formatter.cpp | 7 + test/test_apidb_backend.cpp | 298 +--------------------------- test/test_database.cpp | 176 ++++++++++++++++ test/test_database.hpp | 71 +++++++ test/test_formatter.cpp | 128 ++++++++++++ test/test_formatter.hpp | 69 +++++++ 10 files changed, 468 insertions(+), 296 deletions(-) create mode 100644 test/test_database.cpp create mode 100644 test/test_database.hpp create mode 100644 test/test_formatter.cpp create mode 100644 test/test_formatter.hpp diff --git a/include/cgimap/bbox.hpp b/include/cgimap/bbox.hpp index 7b62832bb..fb162c82d 100644 --- a/include/cgimap/bbox.hpp +++ b/include/cgimap/bbox.hpp @@ -13,6 +13,8 @@ struct bbox { bbox(); + bool operator==(const bbox &) const; + /** * Attempt to parse a bounding box from a comma-separated string * of coordinates. Returns true if parsing was succesful and the diff --git a/include/cgimap/output_formatter.hpp b/include/cgimap/output_formatter.hpp index 79eda046c..d02a13ebd 100644 --- a/include/cgimap/output_formatter.hpp +++ b/include/cgimap/output_formatter.hpp @@ -84,6 +84,8 @@ struct changeset_comment_info { std::string body; std::string created_at; std::string author_display_name; + + bool operator==(const changeset_comment_info &) const; }; struct member_info { diff --git a/src/Makefile.am b/src/Makefile.am index a91700237..4e58f8a7c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -67,7 +67,9 @@ ___test_test_parse_time_SOURCES=\ if ENABLE_APIDB ___test_test_apidb_backend_SOURCES=\ - ../test/test_apidb_backend.cpp + ../test/test_apidb_backend.cpp \ + ../test/test_formatter.cpp \ + ../test/test_database.cpp endif libcgimap_fcgi_la_SOURCES=\ diff --git a/src/bbox.cpp b/src/bbox.cpp index 46df32bc1..a59741459 100644 --- a/src/bbox.cpp +++ b/src/bbox.cpp @@ -17,6 +17,13 @@ bbox::bbox(double minlat_, double minlon_, double maxlat_, double maxlon_) bbox::bbox() : minlat(0.0), minlon(0.0), maxlat(0.0), maxlon(0.0) {} +bool bbox::operator==(const bbox &other) const { + return ((minlat == other.minlat) && + (minlon == other.minlon) && + (maxlat == other.maxlat) && + (maxlon == other.maxlon)); +} + bool bbox::parse(const std::string &s) { vector strs; al::split(strs, s, al::is_any_of(",")); diff --git a/src/output_formatter.cpp b/src/output_formatter.cpp index a5ed243f8..c747b7f35 100644 --- a/src/output_formatter.cpp +++ b/src/output_formatter.cpp @@ -60,4 +60,11 @@ bool changeset_info::is_open_at(const pt::ptime &now) const { return (closed_at_time > now) && (num_changes < MAX_CHANGESET_ELEMENTS); } +bool changeset_comment_info::operator==(const changeset_comment_info &other) const { + return ((author_id == other.author_id) && + (body == other.body) && + (created_at == other.created_at) && + (author_display_name == other.author_display_name)); +} + output_formatter::~output_formatter() {} diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index 899c2ccf9..065d0996d 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -1,232 +1,19 @@ #include -#include #include #include #include #include -#include #include -#include #include #include -#include "cgimap/backend/apidb/apidb.hpp" - -namespace po = boost::program_options; -namespace fs = boost::filesystem; +#include "test_formatter.hpp" +#include "test_database.hpp" +#include "cgimap/oauth.hpp" namespace { -/** - * test_database is a RAII object to create a unique apidb format database - * populated with fake data to allow the apidb data selection process to - * be tested in isolation. - */ -struct test_database : public boost::noncopyable { - // simple error type - we distinguish this from a programming error and - // allow the test to be skipped, as people might not have or want an - // apidb database set up on their local machines. - struct setup_error : public std::exception { - setup_error(const boost::format &fmt) : m_str(fmt.str()) {} - ~setup_error() throw() {} - virtual const char *what() const throw() { return m_str.c_str(); } - - private: - const std::string m_str; - }; - - // set up a unique test database. - test_database(); - - // drop the test database. - ~test_database(); - - // create table structure and fill with fake data. - void setup(); - - // run a test. func will be called once with each of a writeable and - // readonly data selection backed by the database. the func should - // do its own testing - the run method here is just plumbing. - void run(boost::function)> func); - - // run a test. func will be called once with the OAuth store backed by - // the database. - void run(boost::function)> func); - -private: - // create a random, and hopefully unique, database name. - static std::string random_db_name(); - - // set up the schema of the database and fill it with test data. - static void fill_fake_data(pqxx::connection &w); - - // the name of the test database. - std::string m_db_name; - - // factories using the test database which produce writeable and - // read-only data selections. - boost::shared_ptr m_writeable_factory, - m_readonly_factory; - - // oauth store based on the writeable connection. - boost::shared_ptr m_oauth_store; -}; - -/** - * set up a test database, isolated from anything else, and fill it with - * fake data for testing. - */ -test_database::test_database() { - try { - std::string db_name = random_db_name(); - - pqxx::connection conn("dbname=postgres"); - pqxx::nontransaction w(conn); - - w.exec((boost::format("CREATE DATABASE %1%") % db_name).str()); - w.commit(); - m_db_name = db_name; - - } catch (const std::exception &e) { - throw setup_error(boost::format("Unable to set up test database: %1%") % - e.what()); - - } catch (...) { - throw setup_error( - boost::format("Unable to set up test database due to unknown error.")); - } -} - -/** - * set up table structure and create access resources. this isn't - * involved with the table creation per se, so can error independently - * and cause a rollback / table drop. - */ -void test_database::setup() { - pqxx::connection conn((boost::format("dbname=%1%") % m_db_name).str()); - fill_fake_data(conn); - - boost::shared_ptr apidb = make_apidb_backend(); - - { - po::options_description desc = apidb->options(); - const char *argv[] = { "", "--dbname", m_db_name.c_str() }; - int argc = sizeof(argv) / sizeof(*argv); - po::variables_map vm; - po::store(po::parse_command_line(argc, argv, desc), vm); - vm.notify(); - m_writeable_factory = apidb->create(vm); - m_oauth_store = apidb->create_oauth_store(vm); - } - - { - po::options_description desc = apidb->options(); - const char *argv[] = { "", "--dbname", m_db_name.c_str(), "--readonly" }; - int argc = sizeof(argv) / sizeof(*argv); - po::variables_map vm; - po::store(po::parse_command_line(argc, argv, desc), vm); - vm.notify(); - m_readonly_factory = apidb->create(vm); - } -} - -test_database::~test_database() { - if (m_writeable_factory) { - m_writeable_factory.reset(); - } - if (m_readonly_factory) { - m_readonly_factory.reset(); - } - if (m_oauth_store) { - m_oauth_store.reset(); - } - - if (!m_db_name.empty()) { - try { - pqxx::connection conn("dbname=postgres"); - pqxx::nontransaction w(conn); - - w.exec((boost::format("DROP DATABASE %1%") % m_db_name).str()); - w.commit(); - m_db_name.clear(); - - } catch (const std::exception &e) { - // nothing we can do here in the destructor except complain - // loudly. - std::cerr << "Unable to drop database: " << e.what() << std::endl; - - } catch (...) { - std::cerr << "Unable to drop database due to unknown exception." - << std::endl; - } - } -} - -std::string test_database::random_db_name() { - char name[20]; - - // try to make something that has a reasonable chance of being - // unique on this machine, in case we clash with anything else. - unsigned int hash = (unsigned int)getpid(); - struct timeval tv; - gettimeofday(&tv, NULL); - hash ^= (unsigned int)((tv.tv_usec & 0xffffu) << 16); - - snprintf(name, 20, "osm_test_%08x", hash); - return std::string(name); -} - -void test_database::run( - boost::function)> func) { - try { - func((*m_writeable_factory).make_selection()); - } catch (const std::exception &e) { - throw std::runtime_error( - (boost::format("%1%, in writeable selection") % e.what()).str()); - } - - try { - func((*m_readonly_factory).make_selection()); - } catch (const std::exception &e) { - throw std::runtime_error( - (boost::format("%1%, in read-only selection") % e.what()).str()); - } -} - -void test_database::run( - boost::function)> func) { - if (!m_oauth_store) { - throw std::runtime_error("OAuth store not available."); - } - - func(m_oauth_store); -} - -// reads a file of SQL statements, splits on ';' and tries to -// execute them in a transaction. -struct file_read_transactor : public pqxx::transactor { - file_read_transactor(const std::string &filename) : m_filename(filename) {} - - void operator()(pqxx::work &w) { - std::streamsize size = fs::file_size(m_filename); - std::string big_query(size, '\0'); - std::ifstream in(m_filename.c_str()); - in.read(&big_query[0], size); - if (in.fail() || (size != in.gcount())) { - throw std::runtime_error("Unable to read input SQL file."); - } - w.exec(big_query); - } - - std::string m_filename; -}; - -void test_database::fill_fake_data(pqxx::connection &conn) { - conn.perform(file_read_transactor("test/structure.sql")); - conn.perform(file_read_transactor("test/test_apidb_backend.sql")); -} - template void assert_equal(const T& a, const T&b, const std::string &message) { if (a != b) { @@ -237,85 +24,6 @@ void assert_equal(const T& a, const T&b, const std::string &message) { } } -struct test_formatter : public output_formatter { - struct node_t { - node_t(const element_info &elem_, double lon_, double lat_, - const tags_t &tags_) - : elem(elem_), lon(lon_), lat(lat_), tags(tags_) {} - - element_info elem; - double lon, lat; - tags_t tags; - - inline bool operator!=(const node_t &other) const { - return !operator==(other); - } - - bool operator==(const node_t &other) const { -#define CMP(sym) { if ((sym) != other. sym) { return false; } } - CMP(elem.id); - CMP(elem.version); - CMP(elem.changeset); - CMP(elem.timestamp); - CMP(elem.uid); - CMP(elem.display_name); - CMP(elem.visible); - CMP(lon); - CMP(lat); - CMP(tags.size()); -#undef CMP - return std::equal(tags.begin(), tags.end(), other.tags.begin()); - } - }; - - std::vector m_nodes; - - virtual ~test_formatter() {} - mime::type mime_type() const { throw std::runtime_error("Unimplemented"); } - void start_document(const std::string &generator) {} - void end_document() {} - void write_bounds(const bbox &bounds) {} - void start_element_type(element_type type) {} - void end_element_type(element_type type) {} - void write_node(const element_info &elem, double lon, double lat, - const tags_t &tags) { - m_nodes.push_back(node_t(elem, lon, lat, tags)); - } - void write_way(const element_info &elem, const nodes_t &nodes, - const tags_t &tags) {} - void write_relation(const element_info &elem, - const members_t &members, const tags_t &tags) {} - void write_changeset(const changeset_info &elem, const tags_t &tags, - bool include_comments, const comments_t &comments, - const boost::posix_time::ptime &) {} - void flush() {} - - void error(const std::exception &e) { - throw e; - } - void error(const std::string &str) { - throw std::runtime_error(str); - } -}; - -std::ostream &operator<<(std::ostream &out, const test_formatter::node_t &n) { - out << "node(element_info(" - << "id=" << n.elem.id << ", " - << "version=" << n.elem.version << ", " - << "changeset=" << n.elem.changeset << ", " - << "timestamp=" << n.elem.timestamp << ", " - << "uid=" << n.elem.uid << ", " - << "display_name=" << n.elem.display_name << ", " - << "visible=" << n.elem.visible << "), " - << "lon=" << n.lon << ", " - << "lat=" << n.lat << ", " - << "{"; - BOOST_FOREACH(const tags_t::value_type &v, n.tags) { - out << "\"" << v.first << "\" => \"" << v.second << "\", "; - } - out << "})"; -} - void test_single_nodes(boost::shared_ptr sel) { if (sel->check_node_visibility(1) != data_selection::exists) { throw std::runtime_error("Node 1 should be visible, but isn't"); diff --git a/test/test_database.cpp b/test/test_database.cpp new file mode 100644 index 000000000..47e7ff142 --- /dev/null +++ b/test/test_database.cpp @@ -0,0 +1,176 @@ +#include "cgimap/backend/apidb/apidb.hpp" +#include "test_database.hpp" + +#include + +#include + +namespace fs = boost::filesystem; +namespace po = boost::program_options; + +test_database::setup_error::setup_error(const boost::format &fmt) + : m_str(fmt.str()) { +} + +test_database::setup_error::~setup_error() throw() {} + +const char *test_database::setup_error::what() const throw() { + return m_str.c_str(); +} + +/** + * set up a test database, isolated from anything else, and fill it with + * fake data for testing. + */ +test_database::test_database() { + try { + std::string db_name = random_db_name(); + + pqxx::connection conn("dbname=postgres"); + pqxx::nontransaction w(conn); + + w.exec((boost::format("CREATE DATABASE %1%") % db_name).str()); + w.commit(); + m_db_name = db_name; + + } catch (const std::exception &e) { + throw setup_error(boost::format("Unable to set up test database: %1%") % + e.what()); + + } catch (...) { + throw setup_error( + boost::format("Unable to set up test database due to unknown error.")); + } +} + +/** + * set up table structure and create access resources. this isn't + * involved with the table creation per se, so can error independently + * and cause a rollback / table drop. + */ +void test_database::setup() { + pqxx::connection conn((boost::format("dbname=%1%") % m_db_name).str()); + fill_fake_data(conn); + + boost::shared_ptr apidb = make_apidb_backend(); + + { + po::options_description desc = apidb->options(); + const char *argv[] = { "", "--dbname", m_db_name.c_str() }; + int argc = sizeof(argv) / sizeof(*argv); + po::variables_map vm; + po::store(po::parse_command_line(argc, argv, desc), vm); + vm.notify(); + m_writeable_factory = apidb->create(vm); + m_oauth_store = apidb->create_oauth_store(vm); + } + + { + po::options_description desc = apidb->options(); + const char *argv[] = { "", "--dbname", m_db_name.c_str(), "--readonly" }; + int argc = sizeof(argv) / sizeof(*argv); + po::variables_map vm; + po::store(po::parse_command_line(argc, argv, desc), vm); + vm.notify(); + m_readonly_factory = apidb->create(vm); + } +} + +test_database::~test_database() { + if (m_writeable_factory) { + m_writeable_factory.reset(); + } + if (m_readonly_factory) { + m_readonly_factory.reset(); + } + if (m_oauth_store) { + m_oauth_store.reset(); + } + + if (!m_db_name.empty()) { + try { + pqxx::connection conn("dbname=postgres"); + pqxx::nontransaction w(conn); + + w.exec((boost::format("DROP DATABASE %1%") % m_db_name).str()); + w.commit(); + m_db_name.clear(); + + } catch (const std::exception &e) { + // nothing we can do here in the destructor except complain + // loudly. + std::cerr << "Unable to drop database: " << e.what() << std::endl; + + } catch (...) { + std::cerr << "Unable to drop database due to unknown exception." + << std::endl; + } + } +} + +std::string test_database::random_db_name() { + char name[20]; + + // try to make something that has a reasonable chance of being + // unique on this machine, in case we clash with anything else. + unsigned int hash = (unsigned int)getpid(); + struct timeval tv; + gettimeofday(&tv, NULL); + hash ^= (unsigned int)((tv.tv_usec & 0xffffu) << 16); + + snprintf(name, 20, "osm_test_%08x", hash); + return std::string(name); +} + +void test_database::run( + boost::function)> func) { + try { + func((*m_writeable_factory).make_selection()); + } catch (const std::exception &e) { + throw std::runtime_error( + (boost::format("%1%, in writeable selection") % e.what()).str()); + } + + try { + func((*m_readonly_factory).make_selection()); + } catch (const std::exception &e) { + throw std::runtime_error( + (boost::format("%1%, in read-only selection") % e.what()).str()); + } +} + +void test_database::run( + boost::function)> func) { + if (!m_oauth_store) { + throw std::runtime_error("OAuth store not available."); + } + + func(m_oauth_store); +} + +namespace { +// reads a file of SQL statements, splits on ';' and tries to +// execute them in a transaction. +struct file_read_transactor : public pqxx::transactor { + file_read_transactor(const std::string &filename) : m_filename(filename) {} + + void operator()(pqxx::work &w) { + std::streamsize size = fs::file_size(m_filename); + std::string big_query(size, '\0'); + std::ifstream in(m_filename.c_str()); + in.read(&big_query[0], size); + if (in.fail() || (size != in.gcount())) { + throw std::runtime_error("Unable to read input SQL file."); + } + w.exec(big_query); + } + + std::string m_filename; +}; + +} // anonymous namespace + +void test_database::fill_fake_data(pqxx::connection &conn) { + conn.perform(file_read_transactor("test/structure.sql")); + conn.perform(file_read_transactor("test/test_apidb_backend.sql")); +} diff --git a/test/test_database.hpp b/test/test_database.hpp new file mode 100644 index 000000000..ee7e71329 --- /dev/null +++ b/test/test_database.hpp @@ -0,0 +1,71 @@ +#ifndef TEST_TEST_DATABASE_HPP +#define TEST_TEST_DATABASE_HPP + +#include +#include + +#include +#include +#include + +#include + +#include "cgimap/data_selection.hpp" +#include "cgimap/oauth.hpp" + +/** + * test_database is a RAII object to create a unique apidb format database + * populated with fake data to allow the apidb data selection process to + * be tested in isolation. + */ +struct test_database : public boost::noncopyable { + // simple error type - we distinguish this from a programming error and + // allow the test to be skipped, as people might not have or want an + // apidb database set up on their local machines. + struct setup_error : public std::exception { + setup_error(const boost::format &fmt); + ~setup_error() throw(); + virtual const char *what() const throw(); + + private: + const std::string m_str; + }; + + // set up a unique test database. + test_database(); + + // drop the test database. + ~test_database(); + + // create table structure and fill with fake data. + void setup(); + + // run a test. func will be called once with each of a writeable and + // readonly data selection backed by the database. the func should + // do its own testing - the run method here is just plumbing. + void run(boost::function)> func); + + // run a test. func will be called once with the OAuth store backed by + // the database. + void run(boost::function)> func); + +private: + // create a random, and hopefully unique, database name. + static std::string random_db_name(); + + // set up the schema of the database and fill it with test data. + static void fill_fake_data(pqxx::connection &w); + + // the name of the test database. + std::string m_db_name; + + // factories using the test database which produce writeable and + // read-only data selections. + boost::shared_ptr m_writeable_factory, + m_readonly_factory; + + // oauth store based on the writeable connection. + boost::shared_ptr m_oauth_store; +}; + +#endif /* TEST_TEST_DATABASE_HPP */ diff --git a/test/test_formatter.cpp b/test/test_formatter.cpp new file mode 100644 index 000000000..1a52f1eb6 --- /dev/null +++ b/test/test_formatter.cpp @@ -0,0 +1,128 @@ +#include "cgimap/output_formatter.hpp" +#include "test_formatter.hpp" + +#include +#include + +test_formatter::node_t::node_t(const element_info &elem_, double lon_, double lat_, + const tags_t &tags_) + : elem(elem_), lon(lon_), lat(lat_), tags(tags_) {} + +bool test_formatter::node_t::operator==(const node_t &other) const { +#define CMP(sym) { if ((sym) != other. sym) { return false; } } + CMP(elem.id); + CMP(elem.version); + CMP(elem.changeset); + CMP(elem.timestamp); + CMP(elem.uid); + CMP(elem.display_name); + CMP(elem.visible); + CMP(lon); + CMP(lat); + CMP(tags.size()); +#undef CMP + return std::equal(tags.begin(), tags.end(), other.tags.begin()); +} + +test_formatter::changeset_t::changeset_t(const changeset_info &info, + const tags_t &tags, + bool include_comments, + const comments_t &comments, + const boost::posix_time::ptime &time) + : m_info(info) + , m_tags(tags) + , m_include_comments(include_comments) + , m_comments(comments) + , m_time(time) { +} + +bool test_formatter::changeset_t::operator==(const changeset_t &other) const { +#define CMP(sym) { if ((sym) != other. sym) { return false; } } + CMP(m_info.id); + CMP(m_info.created_at); + CMP(m_info.closed_at); + CMP(m_info.uid); + CMP(m_info.display_name); + CMP(m_info.bounding_box); + CMP(m_info.num_changes); + CMP(m_info.comments_count); + CMP(m_tags.size()); + CMP(m_include_comments); + if (m_include_comments) { + CMP(m_comments.size()); + if (!std::equal(m_comments.begin(), m_comments.end(), other.m_comments.begin())) { + return false; + } + } + CMP(m_time); +#undef CMP + return std::equal(m_tags.begin(), m_tags.end(), other.m_tags.begin()); +} + +test_formatter::~test_formatter() {} + +mime::type test_formatter::mime_type() const { + throw std::runtime_error("Unimplemented"); +} + +void test_formatter::start_document(const std::string &generator) { +} + +void test_formatter::end_document() { +} + +void test_formatter::write_bounds(const bbox &bounds) { +} + +void test_formatter::start_element_type(element_type type) { +} + +void test_formatter::end_element_type(element_type type) { +} + +void test_formatter::write_node(const element_info &elem, double lon, double lat, + const tags_t &tags) { + m_nodes.push_back(node_t(elem, lon, lat, tags)); +} +void test_formatter::write_way(const element_info &elem, const nodes_t &nodes, + const tags_t &tags) { +} + +void test_formatter::write_relation(const element_info &elem, + const members_t &members, const tags_t &tags) { +} + +void test_formatter::write_changeset(const changeset_info &elem, const tags_t &tags, + bool include_comments, const comments_t &comments, + const boost::posix_time::ptime &time) { + m_changesets.push_back(changeset_t(elem, tags, include_comments, comments, time)); +} + +void test_formatter::flush() {} + +void test_formatter::error(const std::exception &e) { + throw e; +} + +void test_formatter::error(const std::string &str) { + throw std::runtime_error(str); +} + +std::ostream &operator<<(std::ostream &out, const test_formatter::node_t &n) { + out << "node(element_info(" + << "id=" << n.elem.id << ", " + << "version=" << n.elem.version << ", " + << "changeset=" << n.elem.changeset << ", " + << "timestamp=" << n.elem.timestamp << ", " + << "uid=" << n.elem.uid << ", " + << "display_name=" << n.elem.display_name << ", " + << "visible=" << n.elem.visible << "), " + << "lon=" << n.lon << ", " + << "lat=" << n.lat << ", " + << "{"; + BOOST_FOREACH(const tags_t::value_type &v, n.tags) { + out << "\"" << v.first << "\" => \"" << v.second << "\", "; + } + out << "})"; +} + diff --git a/test/test_formatter.hpp b/test/test_formatter.hpp new file mode 100644 index 000000000..a34ec0249 --- /dev/null +++ b/test/test_formatter.hpp @@ -0,0 +1,69 @@ +#ifndef TEST_TEST_FORMATTER +#define TEST_TEST_FORMATTER + +#include "cgimap/output_formatter.hpp" + +struct test_formatter : public output_formatter { + struct node_t { + node_t(const element_info &elem_, double lon_, double lat_, + const tags_t &tags_); + + element_info elem; + double lon, lat; + tags_t tags; + + inline bool operator!=(const node_t &other) const { + return !operator==(other); + } + + bool operator==(const node_t &other) const; + }; + + struct changeset_t { + changeset_info m_info; + tags_t m_tags; + bool m_include_comments; + comments_t m_comments; + boost::posix_time::ptime m_time; + + changeset_t(const changeset_info &info, + const tags_t &tags, + bool include_comments, + const comments_t &comments, + const boost::posix_time::ptime &time); + + inline bool operator!=(const changeset_t &other) const { + return !operator==(other); + } + + bool operator==(const changeset_t &other) const; + }; + + std::vector m_changesets; + std::vector m_nodes; + + virtual ~test_formatter(); + mime::type mime_type() const; + void start_document(const std::string &generator); + void end_document(); + void write_bounds(const bbox &bounds); + void start_element_type(element_type type); + void end_element_type(element_type type); + void write_node(const element_info &elem, double lon, double lat, + const tags_t &tags); + void write_way(const element_info &elem, const nodes_t &nodes, + const tags_t &tags); + void write_relation(const element_info &elem, + const members_t &members, const tags_t &tags); + void write_changeset(const changeset_info &elem, const tags_t &tags, + bool include_comments, const comments_t &comments, + const boost::posix_time::ptime &time); + void flush(); + + void error(const std::exception &e); + void error(const std::string &str); +}; + +std::ostream &operator<<(std::ostream &out, const test_formatter::node_t &n); + +#endif /* TEST_TEST_FORMATTER */ From 8aa7fe8bfda1c213a752c50126a7af59e87c06f3 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 19:21:34 +0100 Subject: [PATCH 05/17] Enable and implement changesets on writeable and readonly apidb backends, plus first simple test. --- .../apidb/readonly_pgsql_selection.hpp | 16 ++ .../apidb/readonly_pgsql_selection.cpp | 164 +++++++++++++++++- .../apidb/writeable_pgsql_selection.cpp | 12 +- test/test_apidb_backend.cpp | 43 +++++ test/test_apidb_backend.sql | 7 + test/test_formatter.cpp | 45 ++++- test/test_formatter.hpp | 1 + 7 files changed, 282 insertions(+), 6 deletions(-) diff --git a/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp b/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp index 44aab95a7..1f34d0142 100644 --- a/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp +++ b/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp @@ -24,6 +24,9 @@ class readonly_pgsql_selection : public data_selection { void write_nodes(output_formatter &formatter); void write_ways(output_formatter &formatter); void write_relations(output_formatter &formatter); +#ifdef ENABLE_EXPERIMENTAL + void write_changesets(output_formatter &formatter, const boost::posix_time::ptime &now); +#endif /* ENABLE_EXPERIMENTAL */ visibility_t check_node_visibility(osm_nwr_id_t id); visibility_t check_way_visibility(osm_nwr_id_t id); @@ -42,6 +45,12 @@ class readonly_pgsql_selection : public data_selection { void select_relations_from_relations(); void select_relations_members_of_relations(); +#ifdef ENABLE_EXPERIMENTAL + bool supports_changesets(); + int select_changesets(const std::vector &); + void select_changeset_discussions(); +#endif /* ENABLE_EXPERIMENTAL */ + /** * a factory for the creation of read-only selections, so it * can set up prepared statements. @@ -67,7 +76,14 @@ class readonly_pgsql_selection : public data_selection { // unlike writeable_pgsql_selection. pqxx::work w; +#ifdef ENABLE_EXPERIMENTAL + // true if we want to include changeset discussions along with + // the changesets themselves. defaults to false. + bool include_changeset_discussions; +#endif /* ENABLE_EXPERIMENTAL */ + // the set of selected nodes, ways and relations + std::set sel_changesets; std::set sel_nodes, sel_ways, sel_relations; cache &cc; }; diff --git a/src/backend/apidb/readonly_pgsql_selection.cpp b/src/backend/apidb/readonly_pgsql_selection.cpp index bfefe25fe..d8c2efebc 100644 --- a/src/backend/apidb/readonly_pgsql_selection.cpp +++ b/src/backend/apidb/readonly_pgsql_selection.cpp @@ -18,7 +18,10 @@ #define PREPARE_ARGS(args) args #endif +#define MAX_ELEMENTS (50000) + namespace po = boost::program_options; +namespace pt = boost::posix_time; using std::set; using std::stringstream; using std::list; @@ -89,6 +92,46 @@ template <> struct string_traits > { return ostr.str(); } }; + +// and again for osm_changeset_id_t +template <> struct string_traits > { + static const char *name() { return "vector"; } + static bool has_null() { return false; } + static bool is_null(const vector &) { return false; } + static stringstream null() { + internal::throw_null_conversion(name()); + // No, dear compiler, we don't need a return here. + throw 0; + } + static void from_string(const char[], vector &) {} + static std::string to_string(const vector &ids) { + stringstream ostr; + ostr << "{"; + std::copy(ids.begin(), ids.end(), + infix_ostream_iterator(ostr, ",")); + ostr << "}"; + return ostr.str(); + } +}; +template <> struct string_traits > { + static const char *name() { return "set"; } + static bool has_null() { return false; } + static bool is_null(const set &) { return false; } + static stringstream null() { + internal::throw_null_conversion(name()); + // No, dear compiler, we don't need a return here. + throw 0; + } + static void from_string(const char[], set &) {} + static std::string to_string(const set &ids) { + stringstream ostr; + ostr << "{"; + std::copy(ids.begin(), ids.end(), + infix_ostream_iterator(ostr, ",")); + ostr << "}"; + return ostr.str(); + } +}; } namespace { @@ -128,12 +171,13 @@ check_table_visibility(pqxx::work &w, osm_nwr_id_t id, } } -inline int insert_results(const pqxx::result &res, set &elems) { +template +inline int insert_results(const pqxx::result &res, set &elems) { int num_inserted = 0; for (pqxx::result::const_iterator itr = res.begin(); itr != res.end(); ++itr) { - const osm_nwr_id_t id = (*itr)["id"].as(); + const T id = (*itr)["id"].as(); // note: only count the *new* rows inserted. if (elems.insert(id).second) { @@ -167,6 +211,48 @@ void extract_elem(const pqxx::result::tuple &row, element_info &elem, } } +template +boost::optional extract_optional(const pqxx::result::field &f) { + if (f.is_null()) { + return boost::none; + } else { + return f.as(); + } +} + +void extract_changeset(const pqxx::result::tuple &row, + changeset_info &elem, + cache &changeset_cache) { + elem.id = row["id"].as(); + elem.created_at = row["created_at"].c_str(); + elem.closed_at = row["closed_at"].c_str(); + + shared_ptr cs = changeset_cache.get(elem.id); + if (cs->data_public) { + elem.uid = cs->user_id; + elem.display_name = cs->display_name; + } else { + elem.uid = boost::none; + elem.display_name = boost::none; + } + + boost::optional min_lat = extract_optional(row["min_lat"]); + boost::optional max_lat = extract_optional(row["max_lat"]); + boost::optional min_lon = extract_optional(row["min_lon"]); + boost::optional max_lon = extract_optional(row["max_lon"]); + + if (bool(min_lat) && bool(min_lon) && bool(max_lat) && bool(max_lon)) { + elem.bounding_box = bbox(double(*min_lat) / SCALE, + double(*min_lon) / SCALE, + double(*max_lat) / SCALE, + double(*max_lon) / SCALE); + } else { + elem.bounding_box = boost::none; + } + + elem.num_changes = row["num_changes"].as(); +} + void extract_tags(const pqxx::result &res, tags_t &tags) { tags.clear(); for (pqxx::result::const_iterator itr = res.begin(); itr != res.end(); @@ -228,7 +314,11 @@ void extract_members(const pqxx::result &res, members_t &members) { readonly_pgsql_selection::readonly_pgsql_selection( pqxx::connection &conn, cache &changeset_cache) - : w(conn), cc(changeset_cache) {} + : w(conn), cc(changeset_cache) +#ifdef ENABLE_EXPERIMENTAL + , include_changeset_discussions(false) +#endif /* ENABLE_EXPERIMENTAL */ +{} readonly_pgsql_selection::~readonly_pgsql_selection() {} @@ -352,6 +442,49 @@ void readonly_pgsql_selection::write_relations(output_formatter &formatter) { } } +#ifdef ENABLE_EXPERIMENTAL +void readonly_pgsql_selection::write_changesets(output_formatter &formatter, + const pt::ptime &now) { + changeset_info elem; + tags_t tags; + comments_t comments; + + // fetch in chunks... + set::iterator prev_itr = sel_changesets.begin(); + size_t chunk_i = 0; + for (set::iterator n_itr = sel_changesets.begin();; + ++n_itr, ++chunk_i) { + bool at_end = n_itr == sel_changesets.end(); + if ((chunk_i >= STRIDE) || ((chunk_i > 0) && at_end)) { + stringstream query; + query << "SELECT id, " + << "to_char(created_at,'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS created_at, " + << "to_char(closed_at, 'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS closed_at, " + << "min_lat, max_lat, min_lon, max_lon, " + << "num_changes " + << "FROM changesets WHERE id IN ("; + std::copy(prev_itr, n_itr, infix_ostream_iterator(query, ",")); + query << ")"; + + pqxx::result changesets = w.exec(query); + for (pqxx::result::const_iterator itr = changesets.begin(); + itr != changesets.end(); ++itr) { + extract_changeset(*itr, elem, cc); + extract_tags(w.prepared("extract_changeset_tags")(elem.id).exec(), tags); + // extract_comments(w.prepared("extract_changeset_comments")(elem.id).exec(), comments); + formatter.write_changeset(elem, tags, include_changeset_discussions, comments, now); + } + + chunk_i = 0; + prev_itr = n_itr; + } + + if (at_end) + break; + } +} +#endif /* ENABLE_EXPERIMENTAL */ + data_selection::visibility_t readonly_pgsql_selection::check_node_visibility(osm_nwr_id_t id) { return check_table_visibility(w, id, "visible_node"); @@ -473,6 +606,24 @@ void readonly_pgsql_selection::select_relations_members_of_relations() { } } +#ifdef ENABLE_EXPERIMENTAL +bool readonly_pgsql_selection::supports_changesets() { + return true; +} + +int readonly_pgsql_selection::select_changesets(const std::vector &ids) { + if (!ids.empty()) { + return insert_results(w.prepared("select_changesets")(ids).exec(), sel_changesets); + } else { + return 0; + } +} + +void readonly_pgsql_selection::select_changeset_discussions() { + include_changeset_discussions = true; +} +#endif /* ENABLE_EXPERIMENTAL */ + readonly_pgsql_selection::factory::factory(const po::variables_map &opts) : m_connection(connect_db_str(opts)), m_cache_connection(connect_db_str(opts)), @@ -540,6 +691,8 @@ readonly_pgsql_selection::factory::factory(const po::variables_map &opts) "SELECT k, v FROM current_way_tags WHERE way_id=$1")PREPARE_ARGS(("bigint")); m_connection.prepare("extract_relation_tags", "SELECT k, v FROM current_relation_tags WHERE relation_id=$1")PREPARE_ARGS(("bigint")); + m_connection.prepare("extract_changeset_tags", + "SELECT k, v FROM changeset_tags WHERE changeset_id=$1")PREPARE_ARGS(("bigint")); // selecting a set of objects as a list m_connection.prepare("select_nodes", @@ -557,6 +710,11 @@ readonly_pgsql_selection::factory::factory(const po::variables_map &opts) "FROM current_relations " "WHERE id = ANY($1)") PREPARE_ARGS(("bigint[]")); + m_connection.prepare("select_changesets", + "SELECT id " + "FROM changesets " + "WHERE id = ANY($1)") + PREPARE_ARGS(("bigint[]")); // select ways used by nodes m_connection.prepare("ways_from_nodes", diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index 938657ff1..e044efa2b 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -557,6 +557,14 @@ writeable_pgsql_selection::factory::factory(const po::variables_map &opts) "to_char(r.timestamp,'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS timestamp " "FROM current_relations r " "JOIN tmp_relations tr ON tr.id=r.id"); + m_connection.prepare("extract_changesets", + "SELECT c.id, " + "to_char(c.created_at,'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS created_at, " + "to_char(c.closed_at, 'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS closed_at, " + "c.min_lat, c.max_lat, c.min_lon, c.max_lon, " + "c.num_changes " + "FROM changesets c " + "JOIN tmp_changesets tc ON tc.id=c.id"); // extraction functions for child information m_connection.prepare("extract_way_nds", @@ -579,6 +587,8 @@ writeable_pgsql_selection::factory::factory(const po::variables_map &opts) "SELECT k, v FROM current_way_tags WHERE way_id=$1")PREPARE_ARGS(("bigint")); m_connection.prepare("extract_relation_tags", "SELECT k, v FROM current_relation_tags WHERE relation_id=$1")PREPARE_ARGS(("bigint")); + m_connection.prepare("extract_changeset_tags", + "SELECT k, v FROM changeset_tags WHERE changeset_id=$1")PREPARE_ARGS(("bigint")); // selecting a set of nodes as a list m_connection.prepare("add_nodes_list", @@ -607,7 +617,7 @@ writeable_pgsql_selection::factory::factory(const po::variables_map &opts) PREPARE_ARGS(("bigint[]")); m_connection.prepare("add_changesets_list", "INSERT INTO tmp_changesets " - "SELECT c.id from changesets " + "SELECT c.id from changesets c " "WHERE c.id = ANY($1)") PREPARE_ARGS(("bigint[]")); diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index 065d0996d..c0dc112f0 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -8,6 +8,8 @@ #include #include +#include "cgimap/config.hpp" +#include "cgimap/time.hpp" #include "test_formatter.hpp" #include "test_database.hpp" #include "cgimap/oauth.hpp" @@ -209,6 +211,44 @@ void test_negative_changeset_ids(boost::shared_ptr sel) { f.m_nodes[1], "second node written"); } +void test_changeset(boost::shared_ptr sel) { +#ifdef ENABLE_EXPERIMENTAL + assert_equal(sel->supports_changesets(), true, + "apidb should support changesets."); + + std::vector ids; + ids.push_back(1); + int num = sel->select_changesets(ids); + assert_equal(num, 1, "should have selected one changeset."); + + boost::posix_time::ptime t = parse_time("2015-09-05T17:15:33Z"); + + test_formatter f; + sel->write_changesets(f, t); + assert_equal(f.m_changesets.size(), 1, + "should have written one changeset."); + + assert_equal( + f.m_changesets.front(), + test_formatter::changeset_t( + changeset_info( + 1, // ID + "2013-11-14T02:10:00Z", // created_at + "2013-11-14T03:10:00Z", // closed_at + 1, // uid + std::string("user_1"), // display_name + boost::none, // bounding box + 2, // num_changes + 0 // comments_count + ), + tags_t(), + false, + comments_t(), + t), + "changesets should be equal."); +#endif /* ENABLE_EXPERIMENTAL */ +} + } // anonymous namespace int main(int, char **) { @@ -234,6 +274,9 @@ int main(int, char **) { tdb.run(boost::function)>( &test_negative_changeset_ids)); + tdb.run(boost::function)>( + &test_changeset)); + } catch (const test_database::setup_error &e) { std::cout << "Unable to set up test database: " << e.what() << std::endl; return 77; diff --git a/test/test_apidb_backend.sql b/test/test_apidb_backend.sql index 59e9e0d7c..ca52ae9b4 100644 --- a/test/test_apidb_backend.sql +++ b/test/test_apidb_backend.sql @@ -49,3 +49,10 @@ VALUES 'Rzcm5aDiDgqgub8j96MfDaYyAc4cRwI9CmZB7HBf', '2UxsEFziZGv64hdWN3Qa90Vb6v1aovVxaTTQIn1D', '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z'); + +-- update the number of edits +UPDATE changesets + SET num_changes = ( + SELECT count(*) + FROM current_nodes + WHERE changeset_id = changesets.id); diff --git a/test/test_formatter.cpp b/test/test_formatter.cpp index 1a52f1eb6..65cd0bbfb 100644 --- a/test/test_formatter.cpp +++ b/test/test_formatter.cpp @@ -3,6 +3,7 @@ #include #include +#include test_formatter::node_t::node_t(const element_info &elem_, double lon_, double lat_, const tags_t &tags_) @@ -119,10 +120,50 @@ std::ostream &operator<<(std::ostream &out, const test_formatter::node_t &n) { << "visible=" << n.elem.visible << "), " << "lon=" << n.lon << ", " << "lat=" << n.lat << ", " - << "{"; + << "tags{"; BOOST_FOREACH(const tags_t::value_type &v, n.tags) { out << "\"" << v.first << "\" => \"" << v.second << "\", "; } out << "})"; -} + return out; +} + +std::ostream &operator<<(std::ostream &out, const bbox &b) { + out << "bbox(" + << b.minlon << ", " + << b.minlat << ", " + << b.maxlon << ", " + << b.maxlat << ")"; + return out; +} + +std::ostream &operator<<(std::ostream &out, const test_formatter::changeset_t &c) { + out << "changeset(changeset_info(" + << "id=" << c.m_info.id << ", " + << "created_at=\"" << c.m_info.created_at << "\", " + << "closed_at=\"" << c.m_info.closed_at << "\", " + << "uid=" << c.m_info.uid << ", " + << "display_name=\"" << c.m_info.display_name << "\", " + << "bounding_box=" << c.m_info.bounding_box << ", " + << "num_changes=" << c.m_info.num_changes << ", " + << "comments_count=" << c.m_info.comments_count << "), " + << "tags{"; + BOOST_FOREACH(const tags_t::value_type &v, c.m_tags) { + out << "\"" << v.first << "\" => \"" << v.second << "\", "; + } + out << "}, " + << "include_comments=" << c.m_include_comments << ", " + << "comments["; + BOOST_FOREACH(const comments_t::value_type &v, c.m_comments) { + out << "comment(author_id=" << v.author_id << ", " + << "body=\"" << v.body << "\", " + << "created_at=\"" << v.created_at << "\", " + << "author_display_name=\"" << v.author_display_name << "\"), "; + } + out << "], " + << "time=" << c.m_time + << ")"; + + return out; +} diff --git a/test/test_formatter.hpp b/test/test_formatter.hpp index a34ec0249..a4b0bf5ac 100644 --- a/test/test_formatter.hpp +++ b/test/test_formatter.hpp @@ -65,5 +65,6 @@ struct test_formatter : public output_formatter { }; std::ostream &operator<<(std::ostream &out, const test_formatter::node_t &n); +std::ostream &operator<<(std::ostream &out, const test_formatter::changeset_t &n); #endif /* TEST_TEST_FORMATTER */ From 21365272b0b74e8a3b5cc0515c325b5137742d78 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 19:24:59 +0100 Subject: [PATCH 06/17] Added missing header. --- include/cgimap/output_formatter.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/cgimap/output_formatter.hpp b/include/cgimap/output_formatter.hpp index d02a13ebd..73f2a56a2 100644 --- a/include/cgimap/output_formatter.hpp +++ b/include/cgimap/output_formatter.hpp @@ -5,6 +5,7 @@ #include "cgimap/types.hpp" #include "cgimap/mime_types.hpp" #include +#include #include #include #include From 1faf3b06725a5c37caeda3331af498fbd0d5bd74 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 19:47:46 +0100 Subject: [PATCH 07/17] Which fool decided to use pre-processor macros? Oh, yeah, that'd be me... --- src/api06/changeset_handler.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/api06/changeset_handler.cpp b/src/api06/changeset_handler.cpp index 8b70983fd..3bc45a85c 100644 --- a/src/api06/changeset_handler.cpp +++ b/src/api06/changeset_handler.cpp @@ -1,6 +1,7 @@ #include "cgimap/api06/changeset_handler.hpp" #include "cgimap/request_helpers.hpp" #include "cgimap/http.hpp" +#include "cgimap/config.hpp" #include @@ -17,8 +18,11 @@ changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, vector ids; ids.push_back(id); +#ifdef ENABLE_EXPERIMENTAL if (!sel->supports_changesets()) { +#endif /* ENABLE_EXPERIMENTAL */ throw http::server_error("Data source does not support changesets."); +#ifdef ENABLE_EXPERIMENTAL } if (sel->select_changesets(ids) == 0) { @@ -28,6 +32,7 @@ changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, if (include_discussion) { sel->select_changeset_discussions(); } +#endif /* ENABLE_EXPERIMENTAL */ } changeset_responder::~changeset_responder() {} From d8978aa148341ebfd12f3fb5d596fff573c2ee24 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 20:00:51 +0100 Subject: [PATCH 08/17] Remove `lock_timeout` parameter, apparently unsupported on the version of PostgreSQL that Travis is using. --- test/structure.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/test/structure.sql b/test/structure.sql index c3c722270..a5bc168db 100644 --- a/test/structure.sql +++ b/test/structure.sql @@ -3,7 +3,6 @@ -- SET statement_timeout = 0; -SET lock_timeout = 0; SET client_encoding = 'UTF8'; SET standard_conforming_strings = on; SET check_function_bodies = false; From 2b5da20c932971652f36da2f1caf3e27b63f51db Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 20:29:40 +0100 Subject: [PATCH 09/17] Better error reporting for attribute mismatches in test runner. --- test/test_core.cpp | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/test/test_core.cpp b/test/test_core.cpp index 27dde0932..7952ab346 100644 --- a/test/test_core.cpp +++ b/test/test_core.cpp @@ -197,13 +197,30 @@ void check_xmlattr(const pt::ptree &expected, const pt::ptree &actual) { } BOOST_FOREACH(const std::string &k, exp_keys) { - std::string exp_val = expected.get_child(k).data(); - std::string act_val = actual.get_child(k).data(); - if ((exp_val != act_val) && (exp_val != "***")) { - throw std::runtime_error( + boost::optional exp_child = expected.get_child_optional(k); + boost::optional act_child = actual.get_child_optional(k); + + if (exp_child) { + if (act_child) { + std::string exp_val = exp_child->data(); + std::string act_val = act_child->data(); + if ((exp_val != act_val) && (exp_val != "***")) { + throw std::runtime_error( + (boost::format( + "Attribute `%1%' expected value `%2%', but got `%3%'") % + k % exp_val % act_val).str()); + } + } else { + throw std::runtime_error( (boost::format( - "Attribute `%1%' expected value `%2%', but got `%3%'") % - k % exp_val % act_val).str()); + "Expected to find attribute `%1%', but it was missing.") % + k).str()); + } + } else if (act_child) { + throw std::runtime_error( + (boost::format( + "Found attribute `%1%', but it was not expected to exist.") % + k).str()); } } } From 61d024b74a1d7db36ecc643d3a0f0d6e2022eec7 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 20:30:46 +0100 Subject: [PATCH 10/17] Added tests for non-public changesets. --- test/changesets.testcore/data.osm | 1 + test/changesets.testcore/read_nonpublic.case | 12 +++++ test/test_apidb_backend.cpp | 47 ++++++++++++++++++-- 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 test/changesets.testcore/read_nonpublic.case diff --git a/test/changesets.testcore/data.osm b/test/changesets.testcore/data.osm index 050b03cff..268e9f7f5 100644 --- a/test/changesets.testcore/data.osm +++ b/test/changesets.testcore/data.osm @@ -7,5 +7,6 @@ + diff --git a/test/changesets.testcore/read_nonpublic.case b/test/changesets.testcore/read_nonpublic.case new file mode 100644 index 000000000..7e42794bd --- /dev/null +++ b/test/changesets.testcore/read_nonpublic.case @@ -0,0 +1,12 @@ +Request-Method: GET +Request-URI: /api/0.6/changeset/2 +HTTP-X-Error-Format: xml +Date: 2015-09-05T20:17:10Z +--- +Status: 200 OK +Content-Type: text/xml; charset=utf-8 +!Content-Disposition: +--- + + + diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index c0dc112f0..da944e176 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -211,8 +211,8 @@ void test_negative_changeset_ids(boost::shared_ptr sel) { f.m_nodes[1], "second node written"); } -void test_changeset(boost::shared_ptr sel) { #ifdef ENABLE_EXPERIMENTAL +void test_changeset(boost::shared_ptr sel) { assert_equal(sel->supports_changesets(), true, "apidb should support changesets."); @@ -245,10 +245,46 @@ void test_changeset(boost::shared_ptr sel) { false, comments_t(), t), - "changesets should be equal."); -#endif /* ENABLE_EXPERIMENTAL */ + "changesets"); } +void test_nonpublic_changeset(boost::shared_ptr sel) { + assert_equal(sel->supports_changesets(), true, + "apidb should support changesets."); + + std::vector ids; + ids.push_back(4); + int num = sel->select_changesets(ids); + assert_equal(num, 1, "should have selected one changeset."); + + boost::posix_time::ptime t = parse_time("2015-09-05T20:13:23Z"); + + test_formatter f; + sel->write_changesets(f, t); + assert_equal(f.m_changesets.size(), 1, + "should have written one changeset."); + + assert_equal( + f.m_changesets.front(), + test_formatter::changeset_t( + changeset_info( + 4, // ID + "2013-11-14T02:10:00Z", // created_at + "2013-11-14T03:10:00Z", // closed_at + boost::none, // uid + boost::none, // display_name + boost::none, // bounding box + 1, // num_changes + 0 // comments_count + ), + tags_t(), + false, + comments_t(), + t), + "changesets"); +} +#endif /* ENABLE_EXPERIMENTAL */ + } // anonymous namespace int main(int, char **) { @@ -274,9 +310,14 @@ int main(int, char **) { tdb.run(boost::function)>( &test_negative_changeset_ids)); +#ifdef ENABLE_EXPERIMENTAL tdb.run(boost::function)>( &test_changeset)); + tdb.run(boost::function)>( + &test_nonpublic_changeset)); +#endif /* ENABLE_EXPERIMENTAL */ + } catch (const test_database::setup_error &e) { std::cout << "Unable to set up test database: " << e.what() << std::endl; return 77; From ef8f8340d2c9fc2177e16f83cf046fd8c013f23c Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 20:35:39 +0100 Subject: [PATCH 11/17] Added test for changeset with tags. --- test/test_apidb_backend.cpp | 42 +++++++++++++++++++++++++++++++++++++ test/test_apidb_backend.sql | 5 +++++ 2 files changed, 47 insertions(+) diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index da944e176..518ec9d4a 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -283,6 +283,45 @@ void test_nonpublic_changeset(boost::shared_ptr sel) { t), "changesets"); } + +void test_changeset_with_tags(boost::shared_ptr sel) { + assert_equal(sel->supports_changesets(), true, + "apidb should support changesets."); + + std::vector ids; + ids.push_back(2); + int num = sel->select_changesets(ids); + assert_equal(num, 1, "should have selected one changeset."); + + boost::posix_time::ptime t = parse_time("2015-09-05T20:33:00Z"); + + test_formatter f; + sel->write_changesets(f, t); + assert_equal(f.m_changesets.size(), 1, + "should have written one changeset."); + + tags_t tags; + tags.push_back(std::make_pair("test_key", "test_value")); + tags.push_back(std::make_pair("test_key2", "test_value2")); + assert_equal( + f.m_changesets.front(), + test_formatter::changeset_t( + changeset_info( + 2, // ID + "2013-11-14T02:10:00Z", // created_at + "2013-11-14T03:10:00Z", // closed_at + 1, // uid + std::string("user_1"), // display_name + boost::none, // bounding box + 0, // num_changes + 0 // comments_count + ), + tags, + false, + comments_t(), + t), + "changesets should be equal."); +} #endif /* ENABLE_EXPERIMENTAL */ } // anonymous namespace @@ -316,6 +355,9 @@ int main(int, char **) { tdb.run(boost::function)>( &test_nonpublic_changeset)); + + tdb.run(boost::function)>( + &test_changeset_with_tags)); #endif /* ENABLE_EXPERIMENTAL */ } catch (const test_database::setup_error &e) { diff --git a/test/test_apidb_backend.sql b/test/test_apidb_backend.sql index ca52ae9b4..9a58de4a0 100644 --- a/test/test_apidb_backend.sql +++ b/test/test_apidb_backend.sql @@ -17,6 +17,11 @@ VALUES (-1, -1, '2016-04-16T15:09:00Z', '2016-04-16T15:09:00Z'), (3, 1, '2013-11-14T02:10:00Z', '2013-11-14T03:10:00Z'), (4, 2, '2013-11-14T02:10:00Z', '2013-11-14T03:10:00Z'); +-- and some tags on those changesets +INSERT INTO changeset_tags (changeset_id, k, v) +VALUES (2, 'test_key', 'test_value'), + (2, 'test_key2', 'test_value2'); + -- add some nodes for each of the users INSERT INTO current_nodes (id, latitude, longitude, changeset_id, visible, "timestamp", tile, version) VALUES (1, 0, 0, 1, true, '2013-11-14T02:10:00Z', 3221225472, 1), From 7653b2022acf84f1083c1385669ffe4f5cc40dab Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 22:02:27 +0100 Subject: [PATCH 12/17] Added apidb tests for changeset comments. --- .../apidb/readonly_pgsql_selection.cpp | 23 ++++- .../apidb/writeable_pgsql_selection.cpp | 23 ++++- test/test_apidb_backend.cpp | 83 ++++++++++++++++++- test/test_apidb_backend.sql | 8 +- 4 files changed, 132 insertions(+), 5 deletions(-) diff --git a/src/backend/apidb/readonly_pgsql_selection.cpp b/src/backend/apidb/readonly_pgsql_selection.cpp index d8c2efebc..c86071760 100644 --- a/src/backend/apidb/readonly_pgsql_selection.cpp +++ b/src/backend/apidb/readonly_pgsql_selection.cpp @@ -310,6 +310,19 @@ void extract_members(const pqxx::result &res, members_t &members) { } } +void extract_comments(const pqxx::result &res, comments_t &comments) { + changeset_comment_info comment; + comments.clear(); + for (pqxx::result::const_iterator itr = res.begin(); itr != res.end(); + ++itr) { + comment.author_id = (*itr)["author_id"].as(); + comment.author_display_name = (*itr)["display_name"].c_str(); + comment.body = (*itr)["body"].c_str(); + comment.created_at = (*itr)["created_at"].c_str(); + comments.push_back(comment); + } +} + } // anonymous namespace readonly_pgsql_selection::readonly_pgsql_selection( @@ -471,7 +484,7 @@ void readonly_pgsql_selection::write_changesets(output_formatter &formatter, itr != changesets.end(); ++itr) { extract_changeset(*itr, elem, cc); extract_tags(w.prepared("extract_changeset_tags")(elem.id).exec(), tags); - // extract_comments(w.prepared("extract_changeset_comments")(elem.id).exec(), comments); + extract_comments(w.prepared("extract_changeset_comments")(elem.id).exec(), comments); formatter.write_changeset(elem, tags, include_changeset_discussions, comments, now); } @@ -683,6 +696,14 @@ readonly_pgsql_selection::factory::factory(const po::variables_map &opts) "WHERE relation_id=$1 " "ORDER BY sequence_id ASC") PREPARE_ARGS(("bigint")); + m_connection.prepare("extract_changeset_comments", + "SELECT cc.author_id, u.display_name, cc.body, " + "to_char(cc.created_at,'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS created_at " + "FROM changeset_comments cc " + "JOIN users u ON cc.author_id = u.id " + "WHERE cc.changeset_id=$1 AND cc.visible " + "ORDER BY cc.created_at ASC") + PREPARE_ARGS(("bigint")); // extraction functions for tags m_connection.prepare("extract_node_tags", diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index e044efa2b..5a2bf09ec 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -247,6 +247,19 @@ void extract_members(const pqxx::result &res, members_t &members) { } } +void extract_comments(const pqxx::result &res, comments_t &comments) { + changeset_comment_info comment; + comments.clear(); + for (pqxx::result::const_iterator itr = res.begin(); itr != res.end(); + ++itr) { + comment.author_id = (*itr)["author_id"].as(); + comment.author_display_name = (*itr)["display_name"].c_str(); + comment.body = (*itr)["body"].c_str(); + comment.created_at = (*itr)["created_at"].c_str(); + comments.push_back(comment); + } +} + } // anonymous namespace writeable_pgsql_selection::writeable_pgsql_selection( @@ -332,7 +345,7 @@ void writeable_pgsql_selection::write_changesets(output_formatter &formatter, itr != changesets.end(); ++itr) { extract_changeset(*itr, elem, cc); extract_tags(w.prepared("extract_changeset_tags")(elem.id).exec(), tags); - // extract_comments(w.prepared("extract_changeset_comments")(elem.id).exec(), comments); + extract_comments(w.prepared("extract_changeset_comments")(elem.id).exec(), comments); formatter.write_changeset(elem, tags, include_changeset_discussions, comments, now); } } @@ -579,6 +592,14 @@ writeable_pgsql_selection::factory::factory(const po::variables_map &opts) "WHERE relation_id=$1 " "ORDER BY sequence_id ASC") PREPARE_ARGS(("bigint")); + m_connection.prepare("extract_changeset_comments", + "SELECT cc.author_id, u.display_name, cc.body, " + "to_char(cc.created_at,'YYYY-MM-DD\"T\"HH24:MI:SS\"Z\"') AS created_at " + "FROM changeset_comments cc " + "JOIN users u ON cc.author_id = u.id " + "WHERE cc.changeset_id=$1 AND cc.visible " + "ORDER BY cc.created_at ASC") + PREPARE_ARGS(("bigint")); // extraction functions for tags m_connection.prepare("extract_node_tags", diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index 518ec9d4a..19e51b478 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -288,7 +288,7 @@ void test_changeset_with_tags(boost::shared_ptr sel) { assert_equal(sel->supports_changesets(), true, "apidb should support changesets."); - std::vector ids; + std::vector ids; ids.push_back(2); int num = sel->select_changesets(ids); assert_equal(num, 1, "should have selected one changeset."); @@ -313,7 +313,7 @@ void test_changeset_with_tags(boost::shared_ptr sel) { 1, // uid std::string("user_1"), // display_name boost::none, // bounding box - 0, // num_changes + 1, // num_changes 0 // comments_count ), tags, @@ -322,6 +322,79 @@ void test_changeset_with_tags(boost::shared_ptr sel) { t), "changesets should be equal."); } + +void check_changeset_with_comments(boost::shared_ptr sel, + bool include_discussion) { + assert_equal(sel->supports_changesets(), true, + "apidb should support changesets."); + + std::vector ids; + ids.push_back(3); + int num = sel->select_changesets(ids); + assert_equal(num, 1, "should have selected one changeset."); + + if (include_discussion) { + sel->select_changeset_discussions(); + } + + boost::posix_time::ptime t = parse_time("2015-09-05T20:38:00Z"); + + test_formatter f; + sel->write_changesets(f, t); + assert_equal(f.m_changesets.size(), 1, + "should have written one changeset."); + + comments_t comments; + { + changeset_comment_info comment; + comment.author_id = 3; + comment.body = "a nice comment!"; + comment.created_at = "2015-09-05T20:37:01Z"; + comment.author_display_name = "user_3"; + comments.push_back(comment); + } + // note that we don't see the non-visible one in the database. + assert_equal( + f.m_changesets.front(), + test_formatter::changeset_t( + changeset_info( + 3, // ID + "2013-11-14T02:10:00Z", // created_at + "2013-11-14T03:10:00Z", // closed_at + 1, // uid + std::string("user_1"), // display_name + boost::none, // bounding box + 0, // num_changes + 0 // comments_count + ), + tags_t(), + include_discussion, + comments, + t), + "changesets should be equal."); +} + +void test_changeset_with_comments_not_including_discussions(boost::shared_ptr sel) { + try { + check_changeset_with_comments(sel, false); + + } catch (const std::exception &e) { + std::ostringstream ostr; + ostr << e.what() << ", while include_discussion was false"; + throw std::runtime_error(ostr.str()); + } +} + +void test_changeset_with_comments_including_discussions(boost::shared_ptr sel) { + try { + check_changeset_with_comments(sel, true); + + } catch (const std::exception &e) { + std::ostringstream ostr; + ostr << e.what() << ", while include_discussion was true"; + throw std::runtime_error(ostr.str()); + } +} #endif /* ENABLE_EXPERIMENTAL */ } // anonymous namespace @@ -358,6 +431,12 @@ int main(int, char **) { tdb.run(boost::function)>( &test_changeset_with_tags)); + + tdb.run(boost::function)>( + &test_changeset_with_comments_not_including_discussions)); + + tdb.run(boost::function)>( + &test_changeset_with_comments_including_discussions)); #endif /* ENABLE_EXPERIMENTAL */ } catch (const test_database::setup_error &e) { diff --git a/test/test_apidb_backend.sql b/test/test_apidb_backend.sql index 9a58de4a0..410f4b466 100644 --- a/test/test_apidb_backend.sql +++ b/test/test_apidb_backend.sql @@ -4,7 +4,8 @@ INSERT INTO users (id, email, pass_crypt, creation_time, display_name, data_publ -- writing an apidb from a planet file or extract. VALUES (-1, 'osmosis@osmosis.com', '', '2016-04-16T15:09:00Z', 'osmosis', false), (1, 'user_1@example.com', '', '2013-11-14T02:10:00Z', 'user_1', true), - (2, 'user_2@example.com', '', '2013-11-14T02:10:00Z', 'user_2', false); + (2, 'user_2@example.com', '', '2013-11-14T02:10:00Z', 'user_2', false), + (3, 'user_3@example.com', '', '2015-09-05T20:37:00Z', 'user_3', true); -- give the users some changesets INSERT INTO changesets (id, user_id, created_at, closed_at) @@ -22,6 +23,11 @@ INSERT INTO changeset_tags (changeset_id, k, v) VALUES (2, 'test_key', 'test_value'), (2, 'test_key2', 'test_value2'); +-- and a discussion on one of those changesets +INSERT INTO changeset_comments (id, changeset_id, author_id, body, created_at, visible) +VALUES (1, 3, 3, 'a nice comment!', '2015-09-05T20:37:01Z', true), + (2, 3, 3, 'a nasty comment', '2015-09-05T20:37:10Z', false); + -- add some nodes for each of the users INSERT INTO current_nodes (id, latitude, longitude, changeset_id, visible, "timestamp", tile, version) VALUES (1, 0, 0, 1, true, '2013-11-14T02:10:00Z', 3221225472, 1), From 0086c7db063b7f66d3f9f53bfa3f09e906404255 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sat, 5 Sep 2015 22:04:23 +0100 Subject: [PATCH 13/17] Removed comments referencing fixed TODOs. --- src/backend/staticxml/staticxml.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/backend/staticxml/staticxml.cpp b/src/backend/staticxml/staticxml.cpp index 998fb712b..e6f29e86c 100644 --- a/src/backend/staticxml/staticxml.cpp +++ b/src/backend/staticxml/staticxml.cpp @@ -614,9 +614,6 @@ struct staticxml_backend : public backend { shared_ptr create(const po::variables_map &opts) { std::string file = opts["file"].as(); - // TODO: now should be a variable on the *request* not in the data store. - // TODO: the changeset data structure parsed here can/should contain the - // number of changes read from the OSM file, not open/closed status. return boost::make_shared(file); } From 6b6dac07ebee2356ee0d897c563e76414734a3db Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sun, 21 Aug 2016 15:23:26 +0100 Subject: [PATCH 14/17] Remove experimental feature flag on changesets - we seem to be using branches for this anyway, so there's little need for the flag. --- Makefile.am | 3 ++- .../cgimap/backend/apidb/readonly_pgsql_selection.hpp | 6 ------ .../backend/apidb/writeable_pgsql_selection.hpp | 6 ------ include/cgimap/data_selection.hpp | 4 ---- src/api06/changeset_handler.cpp | 4 ---- src/backend/apidb/readonly_pgsql_selection.cpp | 9 +-------- src/backend/apidb/writeable_pgsql_selection.cpp | 9 +-------- src/backend/staticxml/staticxml.cpp | 11 +---------- src/data_selection.cpp | 2 -- src/osm_current_responder.cpp | 2 -- src/routes.cpp | 5 ++--- test/test_apidb_backend.cpp | 4 ---- 12 files changed, 7 insertions(+), 58 deletions(-) diff --git a/Makefile.am b/Makefile.am index 23d6c8b82..6e12e016e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -74,8 +74,9 @@ endif TESTS = test/map.testcore test/node.testcore test/anon.testcore test/way.testcore test/relation.testcore TESTS += test/empty.testcore test/way_full.testcore test/relation_full.testcore TESTS += test/test_parse_id_list test/test_oauth test/test_http test/test_parse_time +TESTS += test/changesets.testcore if ENABLE_EXPERIMENTAL -TESTS += test/node_ways.testcore test/changesets.testcore +TESTS += test/node_ways.testcore endif if HAVE_YAJL TESTS += test/json.testcore diff --git a/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp b/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp index 1f34d0142..423ecf278 100644 --- a/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp +++ b/include/cgimap/backend/apidb/readonly_pgsql_selection.hpp @@ -24,9 +24,7 @@ class readonly_pgsql_selection : public data_selection { void write_nodes(output_formatter &formatter); void write_ways(output_formatter &formatter); void write_relations(output_formatter &formatter); -#ifdef ENABLE_EXPERIMENTAL void write_changesets(output_formatter &formatter, const boost::posix_time::ptime &now); -#endif /* ENABLE_EXPERIMENTAL */ visibility_t check_node_visibility(osm_nwr_id_t id); visibility_t check_way_visibility(osm_nwr_id_t id); @@ -45,11 +43,9 @@ class readonly_pgsql_selection : public data_selection { void select_relations_from_relations(); void select_relations_members_of_relations(); -#ifdef ENABLE_EXPERIMENTAL bool supports_changesets(); int select_changesets(const std::vector &); void select_changeset_discussions(); -#endif /* ENABLE_EXPERIMENTAL */ /** * a factory for the creation of read-only selections, so it @@ -76,11 +72,9 @@ class readonly_pgsql_selection : public data_selection { // unlike writeable_pgsql_selection. pqxx::work w; -#ifdef ENABLE_EXPERIMENTAL // true if we want to include changeset discussions along with // the changesets themselves. defaults to false. bool include_changeset_discussions; -#endif /* ENABLE_EXPERIMENTAL */ // the set of selected nodes, ways and relations std::set sel_changesets; diff --git a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp index 45735123b..5785e9963 100644 --- a/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp +++ b/include/cgimap/backend/apidb/writeable_pgsql_selection.hpp @@ -21,9 +21,7 @@ class writeable_pgsql_selection : public data_selection { void write_nodes(output_formatter &formatter); void write_ways(output_formatter &formatter); void write_relations(output_formatter &formatter); -#ifdef ENABLE_EXPERIMENTAL void write_changesets(output_formatter &formatter, const boost::posix_time::ptime &now); -#endif /* ENABLE_EXPERIMENTAL */ visibility_t check_node_visibility(osm_nwr_id_t id); visibility_t check_way_visibility(osm_nwr_id_t id); @@ -42,11 +40,9 @@ class writeable_pgsql_selection : public data_selection { void select_relations_from_relations(); void select_relations_members_of_relations(); -#ifdef ENABLE_EXPERIMENTAL bool supports_changesets(); int select_changesets(const std::vector &); void select_changeset_discussions(); -#endif /* ENABLE_EXPERIMENTAL */ /** * abstracts the creation of transactions for the writeable @@ -78,11 +74,9 @@ class writeable_pgsql_selection : public data_selection { // assume that all the temporary tables are empty. bool m_tables_empty; -#ifdef ENABLE_EXPERIMENTAL // true if we want to include changeset discussions along with // the changesets themselves. defaults to false. bool include_changeset_discussions; -#endif /* ENABLE_EXPERIMENTAL */ }; #endif /* WRITEABLE_PGSQL_SELECTION_HPP */ diff --git a/include/cgimap/data_selection.hpp b/include/cgimap/data_selection.hpp index fd5246bc8..97b6f9a6c 100644 --- a/include/cgimap/data_selection.hpp +++ b/include/cgimap/data_selection.hpp @@ -30,11 +30,9 @@ class data_selection { /// write the relations to an output formatter virtual void write_relations(output_formatter &formatter) = 0; -#ifdef ENABLE_EXPERIMENTAL /// does this data selection support changesets? virtual void write_changesets(output_formatter &formatter, const boost::posix_time::ptime &now); -#endif /* ENABLE_EXPERIMENTAL */ /******************* information functions *******************/ @@ -89,7 +87,6 @@ class data_selection { /// select relations which are members of selected relations virtual void select_relations_members_of_relations() = 0; -#ifdef ENABLE_EXPERIMENTAL /// does this data selection support changesets? virtual bool supports_changesets(); @@ -101,7 +98,6 @@ class data_selection { /// just sets a flag - by default, discussions are not included, /// if this is called then discussions will be included. virtual void select_changeset_discussions(); -#endif /* ENABLE_EXPERIMENTAL */ /** * factory for the creation of data selections. this abstracts away diff --git a/src/api06/changeset_handler.cpp b/src/api06/changeset_handler.cpp index 3bc45a85c..f021f0266 100644 --- a/src/api06/changeset_handler.cpp +++ b/src/api06/changeset_handler.cpp @@ -18,11 +18,8 @@ changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, vector ids; ids.push_back(id); -#ifdef ENABLE_EXPERIMENTAL if (!sel->supports_changesets()) { -#endif /* ENABLE_EXPERIMENTAL */ throw http::server_error("Data source does not support changesets."); -#ifdef ENABLE_EXPERIMENTAL } if (sel->select_changesets(ids) == 0) { @@ -32,7 +29,6 @@ changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id_, if (include_discussion) { sel->select_changeset_discussions(); } -#endif /* ENABLE_EXPERIMENTAL */ } changeset_responder::~changeset_responder() {} diff --git a/src/backend/apidb/readonly_pgsql_selection.cpp b/src/backend/apidb/readonly_pgsql_selection.cpp index c86071760..5cae0c317 100644 --- a/src/backend/apidb/readonly_pgsql_selection.cpp +++ b/src/backend/apidb/readonly_pgsql_selection.cpp @@ -328,10 +328,7 @@ void extract_comments(const pqxx::result &res, comments_t &comments) { readonly_pgsql_selection::readonly_pgsql_selection( pqxx::connection &conn, cache &changeset_cache) : w(conn), cc(changeset_cache) -#ifdef ENABLE_EXPERIMENTAL - , include_changeset_discussions(false) -#endif /* ENABLE_EXPERIMENTAL */ -{} + , include_changeset_discussions(false) {} readonly_pgsql_selection::~readonly_pgsql_selection() {} @@ -455,7 +452,6 @@ void readonly_pgsql_selection::write_relations(output_formatter &formatter) { } } -#ifdef ENABLE_EXPERIMENTAL void readonly_pgsql_selection::write_changesets(output_formatter &formatter, const pt::ptime &now) { changeset_info elem; @@ -496,7 +492,6 @@ void readonly_pgsql_selection::write_changesets(output_formatter &formatter, break; } } -#endif /* ENABLE_EXPERIMENTAL */ data_selection::visibility_t readonly_pgsql_selection::check_node_visibility(osm_nwr_id_t id) { @@ -619,7 +614,6 @@ void readonly_pgsql_selection::select_relations_members_of_relations() { } } -#ifdef ENABLE_EXPERIMENTAL bool readonly_pgsql_selection::supports_changesets() { return true; } @@ -635,7 +629,6 @@ int readonly_pgsql_selection::select_changesets(const std::vector &changeset_cache) : w(conn), cc(changeset_cache) -#ifdef ENABLE_EXPERIMENTAL - , include_changeset_discussions(false) -#endif /* ENABLE_EXPERIMENTAL */ -{ + , include_changeset_discussions(false) { w.exec("CREATE TEMPORARY TABLE tmp_nodes (id bigint PRIMARY KEY)"); w.exec("CREATE TEMPORARY TABLE tmp_ways (id bigint PRIMARY KEY)"); w.exec("CREATE TEMPORARY TABLE tmp_relations (id bigint PRIMARY KEY)"); @@ -333,7 +330,6 @@ void writeable_pgsql_selection::write_relations(output_formatter &formatter) { } } -#ifdef ENABLE_EXPERIMENTAL void writeable_pgsql_selection::write_changesets(output_formatter &formatter, const pt::ptime &now) { changeset_info elem; @@ -349,7 +345,6 @@ void writeable_pgsql_selection::write_changesets(output_formatter &formatter, formatter.write_changeset(elem, tags, include_changeset_discussions, comments, now); } } -#endif /* ENABLE_EXPERIMENTAL */ data_selection::visibility_t writeable_pgsql_selection::check_node_visibility(osm_nwr_id_t id) { @@ -448,7 +443,6 @@ void writeable_pgsql_selection::select_relations_members_of_relations() { w.prepared("relation_members_of_relations").exec(); } -#ifdef ENABLE_EXPERIMENTAL bool writeable_pgsql_selection::supports_changesets() { return true; } @@ -460,7 +454,6 @@ int writeable_pgsql_selection::select_changesets(const std::vector parse_xml(const char *filename) { struct static_data_selection : public data_selection { explicit static_data_selection(boost::shared_ptr db) : m_db(db) -#ifdef ENABLE_EXPERIMENTAL - , m_include_changeset_comments(false) -#endif /* ENABLE_EXPERIMENTAL */ - {} + , m_include_changeset_comments(false) {} virtual ~static_data_selection() {} virtual void write_nodes(output_formatter &formatter) { @@ -345,7 +342,6 @@ struct static_data_selection : public data_selection { } } -#ifdef ENABLE_EXPERIMENTAL virtual void write_changesets(output_formatter &formatter, const pt::ptime &now) { BOOST_FOREACH(osm_changeset_id_t id, m_changesets) { @@ -358,7 +354,6 @@ struct static_data_selection : public data_selection { } } } -#endif /* ENABLE_EXPERIMENTAL */ virtual visibility_t check_node_visibility(osm_nwr_id_t id) { std::map::iterator itr = m_db->m_nodes.find(id); @@ -557,7 +552,6 @@ struct static_data_selection : public data_selection { } } -#ifdef ENABLE_EXPERIMENTAL virtual bool supports_changesets() { return true; } virtual int select_changesets(const std::vector &ids) { @@ -575,16 +569,13 @@ struct static_data_selection : public data_selection { virtual void select_changeset_discussions() { m_include_changeset_comments = true; } -#endif /* ENABLE_EXPERIMENTAL */ private: boost::shared_ptr m_db; std::set m_changesets; std::set m_nodes, m_ways, m_relations; -#ifdef ENABLE_EXPERIMENTAL bool m_include_changeset_comments; -#endif /* ENABLE_EXPERIMENTAL */ }; struct factory : public data_selection::factory { diff --git a/src/data_selection.cpp b/src/data_selection.cpp index 20dd75b7f..5fbaab5a8 100644 --- a/src/data_selection.cpp +++ b/src/data_selection.cpp @@ -2,7 +2,6 @@ data_selection::~data_selection() {} -#ifdef ENABLE_EXPERIMENTAL void data_selection::write_changesets(output_formatter &, const boost::posix_time::ptime &) { } @@ -16,6 +15,5 @@ int data_selection::select_changesets(const std::vector &) { void data_selection::select_changeset_discussions() { } -#endif /* ENABLE_EXPERIMENTAL */ data_selection::factory::~factory() {} diff --git a/src/osm_current_responder.cpp b/src/osm_current_responder.cpp index ba4a39a06..7d0b48389 100644 --- a/src/osm_current_responder.cpp +++ b/src/osm_current_responder.cpp @@ -23,12 +23,10 @@ void osm_current_responder::write(shared_ptr formatter, fmt.write_bounds(bounds.get()); } -#ifdef ENABLE_EXPERIMENTAL // write all selected changesets fmt.start_element_type(element_type_changeset); sel->write_changesets(fmt, now); fmt.end_element_type(element_type_changeset); -#endif /* ENABLE_EXPERIMENTAL */ // write all selected nodes fmt.start_element_type(element_type_node); diff --git a/src/routes.cpp b/src/routes.cpp index e84bb5ee0..b2be35491 100644 --- a/src/routes.cpp +++ b/src/routes.cpp @@ -16,9 +16,10 @@ #include "cgimap/api06/relation_full_handler.hpp" #include "cgimap/api06/relations_handler.hpp" +#include "cgimap/api06/changeset_handler.hpp" + #ifdef ENABLE_EXPERIMENTAL #include "cgimap/api06/node_ways_handler.hpp" -#include "cgimap/api06/changeset_handler.hpp" #endif /* ENABLE_EXPERIMENTAL */ #ifdef ENABLE_API07 @@ -154,9 +155,7 @@ routes::routes() r->add(root_ / "relation" / osm_id_); r->add(root_ / "relations"); -#ifdef ENABLE_EXPERIMENTAL r->add(root_ / "changeset" / osm_id_); -#endif /* ENABLE_EXPERIMENTAL */ } #ifdef ENABLE_API07 diff --git a/test/test_apidb_backend.cpp b/test/test_apidb_backend.cpp index 19e51b478..e827e470f 100644 --- a/test/test_apidb_backend.cpp +++ b/test/test_apidb_backend.cpp @@ -211,7 +211,6 @@ void test_negative_changeset_ids(boost::shared_ptr sel) { f.m_nodes[1], "second node written"); } -#ifdef ENABLE_EXPERIMENTAL void test_changeset(boost::shared_ptr sel) { assert_equal(sel->supports_changesets(), true, "apidb should support changesets."); @@ -395,7 +394,6 @@ void test_changeset_with_comments_including_discussions(boost::shared_ptr)>( &test_negative_changeset_ids)); -#ifdef ENABLE_EXPERIMENTAL tdb.run(boost::function)>( &test_changeset)); @@ -437,7 +434,6 @@ int main(int, char **) { tdb.run(boost::function)>( &test_changeset_with_comments_including_discussions)); -#endif /* ENABLE_EXPERIMENTAL */ } catch (const test_database::setup_error &e) { std::cout << "Unable to set up test database: " << e.what() << std::endl; From fb6ad2253a026baed63a4e3f53407bf5d3c9b1cf Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Mon, 22 Aug 2016 11:00:37 +0100 Subject: [PATCH 15/17] DRY: remove duplication of pqxx_string_traits template instantiation. --- .../backend/apidb/pqxx_string_traits.hpp | 52 +++++++++ .../apidb/readonly_pgsql_selection.cpp | 104 +----------------- .../apidb/writeable_pgsql_selection.cpp | 68 +----------- 3 files changed, 54 insertions(+), 170 deletions(-) create mode 100644 include/cgimap/backend/apidb/pqxx_string_traits.hpp diff --git a/include/cgimap/backend/apidb/pqxx_string_traits.hpp b/include/cgimap/backend/apidb/pqxx_string_traits.hpp new file mode 100644 index 000000000..ffb444c7f --- /dev/null +++ b/include/cgimap/backend/apidb/pqxx_string_traits.hpp @@ -0,0 +1,52 @@ +#ifndef BACKEND_APIDB_PQXX_STRING_TRAITS_HPP +#define BACKEND_APIDB_PQXX_STRING_TRAITS_HPP + +#include "cgimap/types.hpp" +#include "cgimap/infix_ostream_iterator.hpp" +#include +#include +#include +#include +#include + +namespace pqxx { + +/* + * PQXX_ARRAY_STRING_TRAITS provides an instantiation of the string_traits + * template from PQXX which is used to stringify arguments when sending them + * to Postgres. Cgimap uses several different containers across different + * integer types, all of which stringify to arrays in the same way. + * + * Note that it would be nicer to hide this in a .cpp, but it seems that the + * implementation has to be available when used in the prepared statement + * code. + */ +#define PQXX_ARRAY_STRING_TRAITS(type) \ + template <> struct string_traits { \ + static const char *name() { return #type; } \ + static bool has_null() { return false; } \ + static bool is_null(const type &) { return false; } \ + static std::stringstream null() { \ + internal::throw_null_conversion(name()); \ + throw 0; /* need this to satisfy compiler escape detection */ \ + } \ + static void from_string(const char[], type &) {} \ + static std::string to_string(const type &ids) { \ + std::stringstream ostr; \ + ostr << "{"; \ + std::copy(ids.begin(), ids.end(), \ + infix_ostream_iterator(ostr, ",")); \ + ostr << "}"; \ + return ostr.str(); \ + } \ + } + +PQXX_ARRAY_STRING_TRAITS(std::vector); +PQXX_ARRAY_STRING_TRAITS(std::set); +PQXX_ARRAY_STRING_TRAITS(std::vector); +PQXX_ARRAY_STRING_TRAITS(std::vector); +PQXX_ARRAY_STRING_TRAITS(std::set); + +} // namespace pqxx + +#endif /* BACKEND_APIDB_PQXX_STRING_TRAITS_HPP */ diff --git a/src/backend/apidb/readonly_pgsql_selection.cpp b/src/backend/apidb/readonly_pgsql_selection.cpp index 5cae0c317..fe8ae8a90 100644 --- a/src/backend/apidb/readonly_pgsql_selection.cpp +++ b/src/backend/apidb/readonly_pgsql_selection.cpp @@ -1,5 +1,6 @@ #include "cgimap/backend/apidb/readonly_pgsql_selection.hpp" #include "cgimap/backend/apidb/apidb.hpp" +#include "cgimap/backend/apidb/pqxx_string_traits.hpp" #include "cgimap/logger.hpp" #include "cgimap/backend/apidb/quad_tile.hpp" #include "cgimap/infix_ostream_iterator.hpp" @@ -31,109 +32,6 @@ using boost::shared_ptr; // number of nodes to chunk together #define STRIDE (1000) -namespace pqxx { -template <> struct string_traits > { - static const char *name() { return "vector"; } - static bool has_null() { return false; } - static bool is_null(const vector &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], vector &) {} - static std::string to_string(const vector &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; -template <> struct string_traits > { - static const char *name() { return "set"; } - static bool has_null() { return false; } - static bool is_null(const set &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], set &) {} - static std::string to_string(const set &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; - -// need this for PQXX to serialise lists of tile_id_t, which is different -// from osm_nwr_id_t -template <> struct string_traits > { - static const char *name() { return "vector"; } - static bool has_null() { return false; } - static bool is_null(const vector &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], vector &) {} - static std::string to_string(const vector &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; - -// and again for osm_changeset_id_t -template <> struct string_traits > { - static const char *name() { return "vector"; } - static bool has_null() { return false; } - static bool is_null(const vector &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], vector &) {} - static std::string to_string(const vector &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; -template <> struct string_traits > { - static const char *name() { return "set"; } - static bool has_null() { return false; } - static bool is_null(const set &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], set &) {} - static std::string to_string(const set &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; -} - namespace { std::string connect_db_str(const po::variables_map &options) { // build the connection string. diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index bc78e4419..a4bbfdc96 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -3,7 +3,7 @@ #include "cgimap/logger.hpp" #include "cgimap/backend/apidb/quad_tile.hpp" #include "cgimap/infix_ostream_iterator.hpp" - +#include "cgimap/backend/apidb/pqxx_string_traits.hpp" #include #include #include @@ -24,76 +24,10 @@ namespace po = boost::program_options; namespace pt = boost::posix_time; using std::set; -using std::stringstream; using std::list; using std::vector; using boost::shared_ptr; -namespace pqxx { -template <> struct string_traits > { - static const char *name() { return "vector"; } - static bool has_null() { return false; } - static bool is_null(const vector &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], vector &) {} - static std::string to_string(const vector &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; - -// need this for PQXX to serialise lists of tile_id_t, which is different -// from osm_nwr_id_t -template <> struct string_traits > { - static const char *name() { return "vector"; } - static bool has_null() { return false; } - static bool is_null(const vector &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], vector &) {} - static std::string to_string(const vector &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; - -// and again for osm_changeset_id_t -template <> struct string_traits > { - static const char *name() { return "vector"; } - static bool has_null() { return false; } - static bool is_null(const vector &) { return false; } - static stringstream null() { - internal::throw_null_conversion(name()); - // No, dear compiler, we don't need a return here. - throw 0; - } - static void from_string(const char[], vector &) {} - static std::string to_string(const vector &ids) { - stringstream ostr; - ostr << "{"; - std::copy(ids.begin(), ids.end(), - infix_ostream_iterator(ostr, ",")); - ostr << "}"; - return ostr.str(); - } -}; -} - namespace { std::string connect_db_str(const po::variables_map &options) { // build the connection string. From 701caa6244ad2b15760ea361817f8793c44b351c Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Mon, 22 Aug 2016 11:15:06 +0100 Subject: [PATCH 16/17] Replace hand-written parser with call to library function. --- src/time.cpp | 56 +++++++++++++++------------------------------------- 1 file changed, 16 insertions(+), 40 deletions(-) diff --git a/src/time.cpp b/src/time.cpp index f20cc5a19..ea492a910 100644 --- a/src/time.cpp +++ b/src/time.cpp @@ -4,50 +4,26 @@ #include #include -boost::posix_time::ptime parse_time(const std::string &s) { +namespace bpt = boost::posix_time; +namespace bdt = boost::date_time; + +bpt::ptime parse_time(const std::string &s) { // parse only YYYY-MM-DDTHH:MM:SSZ - if ((s.size() == 20) && - (isdigit(s[0]) != 0) && - (isdigit(s[1]) != 0) && - (isdigit(s[2]) != 0) && - (isdigit(s[3]) != 0) && - (s[4] == '-') && - (isdigit(s[5]) != 0) && - (isdigit(s[6]) != 0) && - (s[7] == '-') && - (isdigit(s[8]) != 0) && - (isdigit(s[9]) != 0) && - (s[10] == 'T') && - (isdigit(s[11]) != 0) && - (isdigit(s[12]) != 0) && - (s[13] == ':') && - (isdigit(s[14]) != 0) && - (isdigit(s[15]) != 0) && - (s[16] == ':') && - (isdigit(s[17]) != 0) && - (isdigit(s[18]) != 0) && - (s[19] == 'Z')) { - return boost::posix_time::ptime( - boost::gregorian::date( - int(s[0] - '0') * 1000 + - int(s[1] - '0') * 100 + - int(s[2] - '0') * 10 + - int(s[3] - '0'), - int(s[5] - '0') * 10 + - int(s[6] - '0'), - int(s[8] - '0') * 10 + - int(s[9] - '0')), - boost::posix_time::time_duration( - int(s[11] - '0') * 10 + - int(s[12] - '0'), - int(s[14] - '0') * 10 + - int(s[15] - '0'), - int(s[17] - '0') * 10 + - int(s[18] - '0'))); + if ((s.size() == 20) && (s[19] == 'Z')) { + // chop off the trailing 'Z' so as not to interfere with the standard + // parsing. + std::string without_tz = s.substr(0, 19); + + bpt::ptime t = bdt::parse_delimited_time(without_tz, 'T'); + + // if parsing succeeded, then return the time parsed. else fall back to + // printing an error. + if (!t.is_special()) { + return t; + } } std::ostringstream ostr; ostr << "Unable to parse string '" << s << "' as an ISO 8601 format date time."; throw std::runtime_error(ostr.str()); } - From 0697e91b69cf8585f7399e1b5a08f3fc28579570 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Mon, 22 Aug 2016 11:29:31 +0100 Subject: [PATCH 17/17] Remove unused MAX_ELEMENTS defines. --- src/backend/apidb/readonly_pgsql_selection.cpp | 2 -- src/backend/apidb/writeable_pgsql_selection.cpp | 2 -- src/backend/staticxml/staticxml.cpp | 1 - 3 files changed, 5 deletions(-) diff --git a/src/backend/apidb/readonly_pgsql_selection.cpp b/src/backend/apidb/readonly_pgsql_selection.cpp index fe8ae8a90..6d037cef5 100644 --- a/src/backend/apidb/readonly_pgsql_selection.cpp +++ b/src/backend/apidb/readonly_pgsql_selection.cpp @@ -19,8 +19,6 @@ #define PREPARE_ARGS(args) args #endif -#define MAX_ELEMENTS (50000) - namespace po = boost::program_options; namespace pt = boost::posix_time; using std::set; diff --git a/src/backend/apidb/writeable_pgsql_selection.cpp b/src/backend/apidb/writeable_pgsql_selection.cpp index a4bbfdc96..a9583df45 100644 --- a/src/backend/apidb/writeable_pgsql_selection.cpp +++ b/src/backend/apidb/writeable_pgsql_selection.cpp @@ -19,8 +19,6 @@ #define PREPARE_ARGS(args) args #endif -#define MAX_ELEMENTS (50000) - namespace po = boost::program_options; namespace pt = boost::posix_time; using std::set; diff --git a/src/backend/staticxml/staticxml.cpp b/src/backend/staticxml/staticxml.cpp index 135a0d263..54293a8e1 100644 --- a/src/backend/staticxml/staticxml.cpp +++ b/src/backend/staticxml/staticxml.cpp @@ -17,7 +17,6 @@ using boost::shared_ptr; using std::string; #define CACHE_SIZE (1000) -#define MAX_ELEMENTS (50000) namespace {