From a81375e97a8c9524f785835c84772152c05b6832 Mon Sep 17 00:00:00 2001 From: mmd-osm Date: Mon, 5 Aug 2024 22:27:37 +0200 Subject: [PATCH] Rework changeset json output according to Rails implementation --- include/cgimap/api06/changeset_handler.hpp | 4 +- include/cgimap/json_formatter.hpp | 9 ++- include/cgimap/osm_changeset_responder.hpp | 38 +++++++++ include/cgimap/output_formatter.hpp | 14 ++-- include/cgimap/text_formatter.hpp | 8 +- include/cgimap/xml_formatter.hpp | 8 +- src/CMakeLists.txt | 1 + src/api06/changeset_handler.cpp | 2 +- src/json_formatter.cpp | 81 ++++++++++++------- src/osm_changeset_responder.cpp | 48 +++++++++++ src/osm_current_responder.cpp | 24 ++---- src/osmchange_responder.cpp | 16 +++- src/text_formatter.cpp | 12 ++- src/xml_formatter.cpp | 18 +++-- .../read_discussion_json.case | 24 +++--- .../read_discussion_multiple_json.case | 16 ++-- test/changesets.testcore/read_json.case | 21 +++-- test/test_formatter.cpp | 10 ++- test/test_formatter.hpp | 6 +- 19 files changed, 249 insertions(+), 111 deletions(-) create mode 100644 include/cgimap/osm_changeset_responder.hpp create mode 100644 src/osm_changeset_responder.cpp diff --git a/include/cgimap/api06/changeset_handler.hpp b/include/cgimap/api06/changeset_handler.hpp index 5dc816bf..77dbb9c3 100644 --- a/include/cgimap/api06/changeset_handler.hpp +++ b/include/cgimap/api06/changeset_handler.hpp @@ -11,13 +11,13 @@ #define API06_CHANGESET_HANDLER_HPP #include "cgimap/handler.hpp" -#include "cgimap/osm_current_responder.hpp" +#include "cgimap/osm_changeset_responder.hpp" #include "cgimap/request.hpp" #include namespace api06 { -class changeset_responder : public osm_current_responder { +class changeset_responder : public osm_changeset_responder { public: changeset_responder(mime::type, osm_changeset_id_t, bool, data_selection &); }; diff --git a/include/cgimap/json_formatter.hpp b/include/cgimap/json_formatter.hpp index fccb2097..a9ef04db 100644 --- a/include/cgimap/json_formatter.hpp +++ b/include/cgimap/json_formatter.hpp @@ -22,7 +22,6 @@ class json_formatter : public output_formatter { private: std::unique_ptr writer; - bool is_in_elements_array{false}; void write_tags(const tags_t &tags); void write_id(const element_info &elem); @@ -37,8 +36,12 @@ class json_formatter : public output_formatter { void start_document(const std::string &generator, const std::string &root_name) override; void end_document() override; void write_bounds(const bbox &bounds) override; - void start_element_type(element_type type) override; - void end_element_type(element_type type) override; + + void start_element() override; + void end_element() override; + void start_changeset(bool) override; + void end_changeset(bool) override; + void start_action(action_type type) override; void end_action(action_type type) override; void error(const std::exception &e) override; diff --git a/include/cgimap/osm_changeset_responder.hpp b/include/cgimap/osm_changeset_responder.hpp new file mode 100644 index 00000000..529528da --- /dev/null +++ b/include/cgimap/osm_changeset_responder.hpp @@ -0,0 +1,38 @@ +/** + * SPDX-License-Identifier: GPL-2.0-only + * + * This file is part of openstreetmap-cgimap (https://github.com/zerebubuth/openstreetmap-cgimap/). + * + * Copyright (C) 2009-2023 by the CGImap developer community. + * For a full list of authors see the git log. + */ + +#ifndef OSM_CHANGESET_RESPONDER_HPP +#define OSM_CHANGESET_RESPONDER_HPP + +#include "cgimap/osm_responder.hpp" + +#include + + +class osm_changeset_responder : public osm_responder { +public: + // construct, passing the mime type down to the responder. + // multi_selection flag can be set if we plan to fetch and print multiple changesets + osm_changeset_responder(mime::type, + data_selection &s, + bool multi_selection); + + // writes whatever is in the selection to the formatter + void write(output_formatter& f, + const std::string &generator, + const std::chrono::system_clock::time_point &now) override; + +protected: + // current selection of elements to be written out + data_selection& sel; + // do we want to select and print multiple changesets? + bool multi_selection{false}; +}; + +#endif /* OSM_CHANGESET_RESPONDER_HPP */ diff --git a/include/cgimap/output_formatter.hpp b/include/cgimap/output_formatter.hpp index e675a52f..f6c8438d 100644 --- a/include/cgimap/output_formatter.hpp +++ b/include/cgimap/output_formatter.hpp @@ -206,13 +206,15 @@ struct output_formatter { // to record the box used by a map call. virtual void write_bounds(const bbox &bounds) = 0; - // start a type of element. this is called once for nodes, ways or - // relations. between the start and end called for a particular element - // type only write_* functions for that type will be called. - virtual void start_element_type(element_type type) = 0; + // + virtual void start_element() = 0; + + // + virtual void end_element() = 0; + + virtual void start_changeset(bool) = 0; - // end a type of element. this is called once for nodes, ways or relations - virtual void end_element_type(element_type type) = 0; + virtual void end_changeset(bool) = 0; // TODO: document me. virtual void start_action(action_type type) = 0; diff --git a/include/cgimap/text_formatter.hpp b/include/cgimap/text_formatter.hpp index 67fc5fbe..a314f38c 100644 --- a/include/cgimap/text_formatter.hpp +++ b/include/cgimap/text_formatter.hpp @@ -32,8 +32,12 @@ class text_formatter : public output_formatter { void start_document(const std::string &generator, const std::string &root_name) override; void end_document() override; void write_bounds(const bbox &bounds) override; - void start_element_type(element_type type) override; - void end_element_type(element_type type) override; + + void start_element() override; + void end_element() override; + void start_changeset(bool) override; + void end_changeset(bool) override; + void start_action(action_type type) override; void end_action(action_type type) override; void error(const std::exception &e) override; diff --git a/include/cgimap/xml_formatter.hpp b/include/cgimap/xml_formatter.hpp index 7fe654e6..9f67ff8a 100644 --- a/include/cgimap/xml_formatter.hpp +++ b/include/cgimap/xml_formatter.hpp @@ -33,8 +33,12 @@ class xml_formatter : public output_formatter { void start_document(const std::string &generator, const std::string &root_name) override; void end_document() override; void write_bounds(const bbox &bounds) override; - void start_element_type(element_type type) override; - void end_element_type(element_type type) override; + + void start_element() override; + void end_element() override; + void start_changeset(bool) override; + void end_changeset(bool) override; + void start_action(action_type type) override; void end_action(action_type type) override; void error(const std::exception &e) override; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f359f84c..5e301c80 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -19,6 +19,7 @@ target_sources(cgimap_core PRIVATE oauth2.cpp options.cpp osm_responder.cpp + osm_changeset_responder.cpp osm_current_responder.cpp osm_diffresult_responder.cpp osmchange_responder.cpp diff --git a/src/api06/changeset_handler.cpp b/src/api06/changeset_handler.cpp index 35418dbc..d67ecaaa 100644 --- a/src/api06/changeset_handler.cpp +++ b/src/api06/changeset_handler.cpp @@ -21,7 +21,7 @@ changeset_responder::changeset_responder(mime::type mt, osm_changeset_id_t id, bool include_discussion, data_selection &w) - : osm_current_responder(mt, w) { + : osm_changeset_responder(mt, w, false) { if (sel.select_changesets({id}) == 0) { throw http::not_found(fmt::format("Changeset {:d} was not found.", id)); diff --git a/src/json_formatter.cpp b/src/json_formatter.cpp index 74c478b1..cb3f996b 100644 --- a/src/json_formatter.cpp +++ b/src/json_formatter.cpp @@ -52,24 +52,42 @@ void json_formatter::write_bounds(const bbox &bounds) { writer->end_object(); } -void json_formatter::end_document() { +void json_formatter::start_element() { + + writer->object_key("elements"); + writer->start_array(); +} + +// +void json_formatter::end_element() { writer->end_array(); // end of elements array - is_in_elements_array = false; writer->end_object(); } -void json_formatter::start_element_type(element_type type) { +void json_formatter::start_changeset(bool multi_selection) { - if (is_in_elements_array) - return; + if (multi_selection) { + writer->object_key("changesets"); + writer->start_array(); + } + else + { + writer->object_key("changeset"); + } +} - writer->object_key("elements"); - writer->start_array(); - is_in_elements_array = true; +void json_formatter::end_changeset(bool multi_selection) { + + if (multi_selection) { + writer->end_array(); + } } -void json_formatter::end_element_type(element_type type) {} + +void json_formatter::end_document() { + writer->end_object(); +} void json_formatter::start_action(action_type type) { } @@ -170,59 +188,60 @@ void json_formatter::write_relation(const element_info &elem, writer->end_object(); } -void json_formatter::write_changeset(const changeset_info &elem, +void json_formatter::write_changeset(const changeset_info &cs, const tags_t &tags, bool include_comments, const comments_t &comments, const std::chrono::system_clock::time_point &now) { writer->start_object(); - writer->property("type", "changeset"); - writer->property("id", elem.id); - writer->property("created_at", elem.created_at); + writer->property("id", cs.id); + writer->property("created_at", cs.created_at); - const bool is_open = elem.is_open_at(now); - if (!is_open) { - writer->property("closed_at", elem.closed_at); - } + const bool is_open = cs.is_open_at(now); writer->property("open", is_open); - if (elem.display_name && bool(elem.uid)) { - writer->property("user", *elem.display_name); - writer->property("uid", *elem.uid); + writer->property("comments_count", cs.comments_count); + writer->property("changes_count", cs.num_changes); + + if (!is_open) { + writer->property("closed_at", cs.closed_at); } - if (elem.bounding_box) { - writer->property("minlat", elem.bounding_box->minlat); - writer->property("minlon", elem.bounding_box->minlon); - writer->property("maxlat", elem.bounding_box->maxlat); - writer->property("maxlon", elem.bounding_box->maxlon); + if (cs.bounding_box) { + writer->property("min_lat", cs.bounding_box->minlat); + writer->property("min_lon", cs.bounding_box->minlon); + writer->property("max_lat", cs.bounding_box->maxlat); + writer->property("max_lon", cs.bounding_box->maxlon); } - writer->property("comments_count", elem.comments_count); - writer->property("changes_count", elem.num_changes); + if (cs.display_name && bool(cs.uid)) { + writer->property("uid", *cs.uid); + writer->property("user", *cs.display_name); + } write_tags(tags); if (include_comments && !comments.empty()) { - writer->object_key("discussion"); + writer->object_key("comments"); writer->start_array(); for (const auto & comment : comments) { writer->start_object(); - writer->property("id", comment.id); + writer->property("id", comment.id); + writer->property("visible", true); writer->property("date", comment.created_at); - writer->property("uid", comment.author_id); + writer->property("uid", comment.author_id); writer->property("user", comment.author_display_name); writer->property("text", comment.body); writer->end_object(); } writer->end_array(); } - writer->end_object(); } + void json_formatter::write_diffresult_create_modify(const element_type elem, const osm_nwr_signed_id_t old_id, const osm_nwr_id_t new_id, diff --git a/src/osm_changeset_responder.cpp b/src/osm_changeset_responder.cpp new file mode 100644 index 00000000..5ab7d49e --- /dev/null +++ b/src/osm_changeset_responder.cpp @@ -0,0 +1,48 @@ +/** + * SPDX-License-Identifier: GPL-2.0-only + * + * This file is part of openstreetmap-cgimap (https://github.com/zerebubuth/openstreetmap-cgimap/). + * + * Copyright (C) 2009-2023 by the CGImap developer community. + * For a full list of authors see the git log. + */ + +#include "cgimap/logger.hpp" +#include "cgimap/osm_changeset_responder.hpp" + +#include + +#include + +using std::list; + +osm_changeset_responder::osm_changeset_responder(mime::type mt, + data_selection &s, + bool multi_selection) + : osm_responder(mt), + sel(s), + multi_selection(multi_selection){} + + +void osm_changeset_responder::write(output_formatter& fmt, + const std::string &generator, + const std::chrono::system_clock::time_point &now) { + + try { + fmt.start_document(generator, "osm"); + + fmt.start_changeset(multi_selection); + + // write changeset + sel.write_changesets(fmt, now); + + fmt.end_changeset(multi_selection); + + } catch (const std::exception &e) { + logger::message(fmt::format("Caught error in osm_changeset_responder: {}", + e.what())); + fmt.error(e); + } + + fmt.end_document(); +} diff --git a/src/osm_current_responder.cpp b/src/osm_current_responder.cpp index 68023f87..325992dc 100644 --- a/src/osm_current_responder.cpp +++ b/src/osm_current_responder.cpp @@ -34,25 +34,11 @@ void osm_current_responder::write(output_formatter& fmt, fmt.write_bounds(*bounds); } - // 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); - fmt.end_element_type(element_type::node); - - // all selected ways - fmt.start_element_type(element_type::way); - sel.write_ways(fmt); - fmt.end_element_type(element_type::way); - - // all selected relations - fmt.start_element_type(element_type::relation); - sel.write_relations(fmt); - fmt.end_element_type(element_type::relation); + fmt.start_element(); + sel.write_nodes(fmt); // write all selected nodes + sel.write_ways(fmt); // all selected ways + sel.write_relations(fmt); // all selected relations + fmt.end_element(); } catch (const std::exception &e) { logger::message(fmt::format("Caught error in osm_current_responder: {}", diff --git a/src/osmchange_responder.cpp b/src/osmchange_responder.cpp index 506409d5..3cafb174 100644 --- a/src/osmchange_responder.cpp +++ b/src/osmchange_responder.cpp @@ -88,12 +88,20 @@ struct sorting_formatter : public output_formatter { throw std::runtime_error("sorting_formatter::write_bounds unimplemented"); } - void start_element_type(element_type) override { - throw std::runtime_error("sorting_formatter::start_element_type unimplemented"); + void start_element() { + throw std::runtime_error("sorting_formatter::start_element unimplemented"); } - void end_element_type(element_type) override { - throw std::runtime_error("sorting_formatter::end_element_type unimplemented"); + void end_element() { + throw std::runtime_error("sorting_formatter::end_element unimplemented"); + } + + void start_changeset(bool) { + throw std::runtime_error("sorting_formatter::start_changeset unimplemented"); + } + + void end_changeset(bool) { + throw std::runtime_error("sorting_formatter::end_changeset unimplemented"); } void write_node( diff --git a/src/text_formatter.cpp b/src/text_formatter.cpp index 7fa071c8..9a4f30bc 100644 --- a/src/text_formatter.cpp +++ b/src/text_formatter.cpp @@ -31,11 +31,19 @@ void text_formatter::write_bounds(const bbox &bounds) { // nothing needed here } -void text_formatter::start_element_type(element_type) { +void text_formatter::start_element() { // nothing needed here } -void text_formatter::end_element_type(element_type) { +void text_formatter::end_element() { + // nothing needed here +} + +void text_formatter::start_changeset(bool) { + // nothing needed here +} + +void text_formatter::end_changeset(bool) { // nothing needed here } diff --git a/src/xml_formatter.cpp b/src/xml_formatter.cpp index 14a5e827..b283e9f2 100644 --- a/src/xml_formatter.cpp +++ b/src/xml_formatter.cpp @@ -43,13 +43,21 @@ void xml_formatter::write_bounds(const bbox &bounds) { writer->end(); } -void xml_formatter::start_element_type(element_type) { - // xml documents surround each element with its type, so there's no - // need to output any information here. +// +void xml_formatter::start_element() { + // nothing to do for xml } -void xml_formatter::end_element_type(element_type) { - // ditto - nothing needed here for XML. +void xml_formatter::end_element() { + // nothing to do for xml +} + +void xml_formatter::start_changeset(bool) { + // nothing to do for xml +} + +void xml_formatter::end_changeset(bool) { + // nothing to do for xml } void xml_formatter::start_action(action_type type) { diff --git a/test/changesets.testcore/read_discussion_json.case b/test/changesets.testcore/read_discussion_json.case index e3d6b466..12ec3794 100644 --- a/test/changesets.testcore/read_discussion_json.case +++ b/test/changesets.testcore/read_discussion_json.case @@ -6,7 +6,7 @@ Status: 200 OK Content-Type: application/json; charset=utf-8 Content-Encoding: identity Cache-Control: private, max-age=0, must-revalidate -!Content-Disposition: +!Content-Disposition: --- { "version": "0.6", @@ -14,29 +14,29 @@ Cache-Control: private, max-age=0, must-revalidate "copyright": "***", "attribution": "***", "license": "***", - "elements": [{ - "type": "changeset", + "changeset": { "id": 1, "created_at": "2015-08-09T10:33:13Z", - "closed_at": "2015-08-09T11:33:13Z", "open": false, - "user": "FakeUser1", - "uid": 1, - "minlat": 51.5288506, - "minlon": -0.1465242, - "maxlat": 51.5288620, - "maxlon": -0.1464925, "comments_count": 1, "changes_count": 1, + "closed_at": "2015-08-09T11:33:13Z", + "min_lat": 51.5288506, + "min_lon": -0.1465242, + "max_lat": 51.5288620, + "max_lon": -0.1464925, + "uid": 1, + "user": "FakeUser1", "tags": { "created_by": "manually" }, - "discussion": [{ + "comments": [{ "id": 1, + "visible": true, "date": "2015-08-09T10:33:13Z", "uid": 1, "user": "FakeUser1", "text": "First post!!!!" }] - }] + } } diff --git a/test/changesets.testcore/read_discussion_multiple_json.case b/test/changesets.testcore/read_discussion_multiple_json.case index 86222fe5..93cc761a 100644 --- a/test/changesets.testcore/read_discussion_multiple_json.case +++ b/test/changesets.testcore/read_discussion_multiple_json.case @@ -14,34 +14,36 @@ Cache-Control: private, max-age=0, must-revalidate "copyright": "***", "attribution": "***", "license": "***", - "elements": [{ - "type": "changeset", + "changeset": { "id": 12, "created_at": "2020-01-01T12:34:56Z", - "closed_at": "2020-01-01T13:12:11Z", "open": false, - "user": "FakeUser1", - "uid": 1, "comments_count": 3, "changes_count": 0, - "discussion": [{ + "closed_at": "2020-01-01T13:12:11Z", + "uid": 1, + "user": "FakeUser1", + "comments": [{ "id": 2, + "visible": true, "date": "2020-01-02T12:00:00Z", "uid": 1, "user": "FakeUser1", "text": "it was empty" },{ "id": 3, + "visible": true, "date": "2020-01-03T12:00:00Z", "uid": 2, "user": "FakeUser2", "text": "why?" },{ "id": 4, + "visible": true, "date": "2020-01-04T12:00:00Z", "uid": 1, "user": "FakeUser1", "text": "because" }] - }] + } } diff --git a/test/changesets.testcore/read_json.case b/test/changesets.testcore/read_json.case index 446ad653..f57613e6 100644 --- a/test/changesets.testcore/read_json.case +++ b/test/changesets.testcore/read_json.case @@ -6,7 +6,7 @@ Status: 200 OK Content-Type: application/json; charset=utf-8 Content-Encoding: identity Cache-Control: private, max-age=0, must-revalidate -!Content-Disposition: +!Content-Disposition: --- { "version": "0.6", @@ -14,23 +14,22 @@ Cache-Control: private, max-age=0, must-revalidate "copyright": "***", "attribution": "***", "license": "***", - "elements": [{ - "type": "changeset", + "changeset": { "id": 1, "created_at": "2015-08-09T10:33:13Z", - "closed_at": "2015-08-09T11:33:13Z", "open": false, - "user": "FakeUser1", - "uid": 1, - "minlat": 51.5288506, - "minlon": -0.1465242, - "maxlat": 51.5288620, - "maxlon": -0.1464925, "comments_count": 1, "changes_count": 1, + "closed_at": "2015-08-09T11:33:13Z", + "min_lat": 51.5288506, + "min_lon": -0.1465242, + "max_lat": 51.5288620, + "max_lon": -0.1464925, + "uid": 1, + "user": "FakeUser1", "tags": { "created_by": "manually" } - }] + } } diff --git a/test/test_formatter.cpp b/test/test_formatter.cpp index c32dbae8..07098baa 100644 --- a/test/test_formatter.cpp +++ b/test/test_formatter.cpp @@ -155,10 +155,16 @@ 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::start_element() { } -void test_formatter::end_element_type(element_type type) { +void test_formatter::end_element() { +} + +void test_formatter::start_changeset(bool) { +} + +void test_formatter::end_changeset(bool) { } void test_formatter::start_action(action_type type) { diff --git a/test/test_formatter.hpp b/test/test_formatter.hpp index a6d3a502..464dc37d 100644 --- a/test/test_formatter.hpp +++ b/test/test_formatter.hpp @@ -90,8 +90,10 @@ struct test_formatter : public output_formatter { void start_document(const std::string &generator, const std::string &root_name) override; void end_document() override; void write_bounds(const bbox &bounds) override; - void start_element_type(element_type type) override; - void end_element_type(element_type type) override; + void start_element() override; + void end_element() override; + void start_changeset(bool) override; + void end_changeset(bool) override; void start_action(action_type type) override; void end_action(action_type type) override; void write_node(const element_info &elem, double lon, double lat,