Modify

Opened 5 years ago

Closed 5 years ago

#6742 closed enhancement (fixed)

[PATCH] to allow script generated files to set default changeset info

Reported by: brycenesbitt Owned by: team
Priority: minor Milestone:
Component: Core Version:
Keywords: changeset Cc:

Description

This patch extends the file format to allow changeset tags:

<?xml version="1.0" encoding="UTF-8"?>
<osm version="0.6" generator="osmfetch">
    <changeset_tag k="source:website" v="http://www.citycarshare.org/"/>
    <changeset_tag k="source" v="osmfetch:ccs"/>
    <changeset_tag k="note" v="Prepared for human review by osmfetch: reads the car share reservation system and suggests matching updates for osm."/>
    <changeset_tag k="conflation_key" v="source:pkey"/>
    <node action='modify' lat='37.9024917' lon='-122.2992889' id='1409407225' version='2' >
        <tag k='amenity' v='car_sharing'/>
        <tag k='source:pkey' v='81'/>
        <tag k='source' v='osmfetch:ccs'/>
        <tag k='operator' v='City CarShare'/>
        <tag k='name' v='El Cerrito/Fairmont: El Cerrito Plaza BART'/>
    </node>
</osm>

It works fine, but needs a tiny bit of help from a more experienced developer: "comment" does not work, but all other tags (including "created_by") work fine.

Attachments (4)

import_changeset_tags.diff (7.8 KB) - added by brycenesbitt 5 years ago.
svn diff against trunk for this feature.
import_changeset_tags.2.diff (7.9 KB) - added by brycenesbitt 5 years ago.
Try 2 at patch
josm_try_3.diff (8.8 KB) - added by brycenesbitt 5 years ago.
Ok, fine. Try 3 with <changeset> syntax
changeset_try4.diff (9.0 KB) - added by brycenesbitt 5 years ago.
Patch to prevent node/way/relation damage in case of extra tags.

Download all attachments as: .zip

Change History (22)

Changed 5 years ago by brycenesbitt

svn diff against trunk for this feature.

comment:1 Changed 5 years ago by anonymous

  • Summary changed from Patch to allow script generated files to set default changeset info to [PATCH] to allow script generated files to set default changeset info

comment:2 Changed 5 years ago by bastiK

Changesets are objects similar to nodes, ways or relations. They have tags, user, timestamp (created & closed). We may decide to save other changesets in a josm file some day, so the following syntax seems more consistent to me:

<osm upload-changeset="-1">
<changeset id='-1'>
  <tag k="source" v="osmfetch:ccs"/>
  <tag k="comment" v="Prepared for ..."/>
</changeset>
<node .../>
...
</osm>

(Also don't compare strings with ==)

comment:3 Changed 5 years ago by stoecker

Also please no remaining debug stuff (logger) in patches.

comment:4 follow-up: Changed 5 years ago by anonymous

bastiK: I'd be happy to see the <changeset> syntax you propose. I was uncomfortable modifying the sax parser handler to that extent. Would you feel comfortable taking that part of the task? I feel JOSM should adopt the same schema as API 0.6 for changeset.

stoecker: fixed

Changed 5 years ago by brycenesbitt

Try 2 at patch

comment:5 in reply to: ↑ 4 Changed 5 years ago by bastiK

Replying to anonymous:

bastiK: I'd be happy to see the <changeset> syntax you propose. I was uncomfortable modifying the sax parser handler to that extent. Would you feel comfortable taking that part of the task? I feel JOSM should adopt the same schema as API 0.6 for changeset.

I'm confident, that you will manage...

Changed 5 years ago by brycenesbitt

Ok, fine. Try 3 with <changeset> syntax

comment:6 Changed 5 years ago by bastiK

Seems to work (despite the comment in the patch). We are currently in stabilization, so this won't be applied before next release.

comment:7 Changed 5 years ago by bastiK

In [4389/josm]:

don't throw NPE when there is <changeset> object with <tag> elements at the top of the file (see #6742)

comment:8 Changed 5 years ago by brycenesbitt

The patch works overall, only the specific tag comment gets ignored:

<osm>
  <changeset>
    <tag k="comment" v="my comment">
  </changeset>
   ....

comment:9 Changed 5 years ago by bastiK

Works for me.

comment:10 Changed 5 years ago by brycenesbitt

Definitely bad here:

<?xml version="1.0" encoding="UTF-8"?>
<osm version="0.6" generator="osmfetch">
    <changeset>
        <tag k="comment" v="NEW COMMENT"/>
        <tag k="source " v="NEW SOURCE"/>
        <tag k="note "   v="NEW NOTE"/>
        <tag k="conflation_key" v="source:pkey"/>
    </changeset>
    <node action='modify' version='1' lon='-122.496' user='h4ck3rm1k3' lat='48.194' id='586625159' >

Load that file, press "upload". The comment shows as the comment from the prior upload.
The source, note and conflation_key all show up as expected.

Changed 5 years ago by brycenesbitt

Patch to prevent node/way/relation damage in case of extra tags.

comment:11 Changed 5 years ago by brycenesbitt

The 4389/josm patch does not prevent this from going badly:

<?xml version="1.0" encoding="UTF-8"?>
<osm version="0.6" generator="osmfetch">
	<node action='modify' version='1' lon='-122.496' user='h4ck3rm1k3' lat='48.194' id='586625159' >
		<tag k='tag2' v='tag2'/>
		<tag k='tag3' v='tag3'/>
	</node>
    <changeset>
		<tag k='tag1' v='tag1'/>
    </changeset>
    <foo>
		<tag k='i_do_not_belong' v='tag4'/>
    </foo>
</osm>

It places the rogue tag i_do_not_belong on the node.

comment:12 Changed 5 years ago by bastiK

I know, [4389] just covers the case, where <changeset> is on top, followed by <node>, <way> and <relation> elements.

OsmReader need rework to handle unknown elements better. (Basically fix the problem that you described.) But I don't like to do it before next release on Monday.

comment:13 follow-up: Changed 5 years ago by brycenesbitt

Ok, are things ready to accept this now?

comment:14 Changed 5 years ago by bastiK

In [4413/josm]:

make osm reader more robust - don't mess up xml structure when there are unknown elements (see #6742)

comment:15 in reply to: ↑ 13 Changed 5 years ago by bastiK

Replying to brycenesbitt:

Ok, are things ready to accept this now?

About 90% of all users still have JOSM < 4389 (which throws a NullPointerException when opening a file with changeset hint). This means we may get a lot of bug reports, when files in this format are distributed right now.

@stoecker: What do you think?

comment:16 Changed 5 years ago by stoecker

As wide distribution of new formats will not happen I see no issue here.

comment:17 Changed 5 years ago by brycenesbitt

I agree with stoecker... the few who build this style changeset can be instructed to upgrade.
Switching to a top level tag ( <changeset_tag k= v= /> ) would avoid the bug: but I don't think it is worth it.

comment:18 Changed 5 years ago by bastiK

  • Resolution set to fixed
  • Status changed from new to closed

In [4414/josm]:

applied #6742 - allow script generated files to set default changeset info (based on patch by brycenesbitt)

To set a default upload comment and other tags, include a changeset element in the osm file like this:
<osm version='0.6' upload-changeset='-1'>

<changeset id='-1'>

<tag k='comment' v='suggested upload comment'/>
<tag k='source' v='the source'/>

</changeset>
<node .../>
...

</osm>

Add Comment

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.