New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for diff uploads (III) #152

Merged
merged 110 commits into from Jan 18, 2019

Conversation

Projects
None yet
2 participants
@mmd-osm
Copy link
Collaborator

mmd-osm commented Jul 22, 2018

Follow up for #143, so that all changes appear in the same pull request for easier review.

Fixes #162, closes #140

mmd-osm and others added some commits Jan 26, 2018

Abstract interface for node, way & relation updaters.
This makes it possible for cgimap core not to depend on the apidb and pqxx libraries, which means that the tests can compile.
Start adding upload support via routes.
* Add proper support for OPTIONS, and handlers declaring their methods.
* Add route for changeset upload.
* Add simple tests.
@zerebubuth

This comment has been minimized.

Copy link
Owner Author

zerebubuth commented on 08e5482 Feb 23, 2018

No, this is something I added. But now I can't remember why I added it. I can't get Travis to accept C++14, but it fails while my local build doesn't. So I'm shaving that yak trying to get a trusty VM up and working again. 🤞

@mmd-osm

This comment has been minimized.

Copy link
Collaborator Author

mmd-osm commented Aug 15, 2018

Latest code coverage analysis results: cgimap_cov.zip

mmd-osm added some commits Aug 15, 2018

Merge pull request #158 from mmd-osm/patch/time
High precision time measurement

@mmd-osm mmd-osm force-pushed the feature/bulk_upload branch 2 times, most recently from e2e7f8c to e118adf Sep 2, 2018

@mmd-osm mmd-osm force-pushed the feature/bulk_upload branch from e118adf to 25f138d Sep 2, 2018

mmd-osm added some commits Sep 6, 2018

@mmd-osm mmd-osm force-pushed the feature/bulk_upload branch from 4d33f81 to ab96ca6 Sep 27, 2018

@mmd-osm mmd-osm force-pushed the feature/bulk_upload branch from ab96ca6 to 17416bb Sep 27, 2018

mmd-osm added some commits Sep 29, 2018

Simplify reference check SQL statements
Invisible objects don't have any members
Restrict XML location info to low level XML parsing issues
Some clients are using regex for higher level message parsing
and can't cope with the additional location information

@mmd-osm mmd-osm force-pushed the feature/bulk_upload branch from 5c5e17e to 584195d Oct 18, 2018

@mmd-osm

This comment has been minimized.

Copy link
Collaborator Author

mmd-osm commented Dec 13, 2018

@zerebubuth : I'm planning to merge this PR some time in January 2019, and release it as 0.7.0 version. Hopefully this would leave a bit of time for a high level code review. We could also schedule some session to go through the code on a high level, if that would be helpful in any way.

Some of the major building blocks:

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/src/api06/changeset_upload_handler.cpp

  • Main entry point to respond to an OSMChange message
  • Instantiates all required objects:
    • XML parser
    • Node/Way/Relation updater
    • Change tracker to keep information as to what kind of changes have been applied (needed for diffResult response message)

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/include/cgimap/api06/changeset_upload/osmchange_input_format.hpp

  • XML parser, populates Node, Way and Relation objects, triggers parser callback for each object
  • The parser it very similar to the one in libosmium
  • All static XML format checks are included in the XML parser itself, or the respective OSMObject/Node/way/Relation instances

https://github.com/zerebubuth/openstreetmap-cgimap/tree/feature/bulk_upload/src/backend/apidb/changeset_upload

  • This is where the real work is done, it includes almost all relevant SQL statements, except for the creation of temporary tables
  • Each class for nodes/ways/relations has methods to populate a local buffer (e.g. add_node, modify_node, delete_node), as well as
    process all objects in the buffer in a bulk operation (e.g. process_new_nodes, process_modify_nodes, process_delete_nodes).

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/src/api06/changeset_upload/osmchange_handler.cpp

  • This is a callback class for the XML parser that is getting called for each OSM object.
  • As long as the overall state doesn't change, the respective node/way/relation gets added to a buffer in their
    node_/way_/relation_updater instance
  • If the state changes (e.g. creating new relations -> modifying existing changes), all objects in the buffer are being processed (-> database updates)

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/src/backend/apidb/pgsql_update.cpp

  • Creates dedicated connection for database update purposes
  • Creates required temporary tables
  • This would be the right place to add further DB connection specific settings, or adding global database hints

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/src/process_request.cpp#L525-L545

  • Processing of HTTP POST requests
  • Checks if user hasn't been blocked, and has API write permission

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/src/process_request.cpp#L214-L281

  • Determine proper handler to process the POST request (currently there's only 1 available)

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/include/cgimap/api06/changeset_upload/osmchange_tracking.hpp

  • Keeps track of which objects have been created/modified/deleted, so we can send back a diffResult message at the end of the processing

https://github.com/zerebubuth/openstreetmap-cgimap/blob/feature/bulk_upload/src/osm_diffresult_responder.cpp

  • Logic to generate a diffResult XML response message based on the information in osmchange_tracking
@mmd-osm

This comment has been minimized.

Copy link
Collaborator Author

mmd-osm commented Jan 5, 2019

@pnorman & @tomhughes : as you're both familiar with the code base as well, you're of course also invited to add comments, time permitting. In case you have some very specific questions, I'll be glad to provide a high level summary or point you to the relevant parts of the code. This could also include topics that are relevant for operations or troubleshooting.

@mmd-osm mmd-osm force-pushed the feature/bulk_upload branch from 584195d to ba6a7bb Jan 16, 2019

@mmd-osm mmd-osm merged commit 2521507 into master Jan 18, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment