Modify

Opened 13 years ago

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

Download all attachments as: .zip

Change History (22)

by brycenesbitt, 13 years ago

Attachment: import_changeset_tags.diff added

svn diff against trunk for this feature.

comment:1 by anonymous, 13 years ago

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

comment:2 by bastiK, 13 years ago

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 by stoecker, 13 years ago

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

comment:4 by anonymous, 13 years ago

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

by brycenesbitt, 13 years ago

Try 2 at patch

in reply to:  4 comment:5 by bastiK, 13 years ago

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...

by brycenesbitt, 13 years ago

Attachment: josm_try_3.diff added

Ok, fine. Try 3 with <changeset> syntax

comment:6 by bastiK, 13 years ago

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 by bastiK, 13 years ago

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 by brycenesbitt, 13 years ago

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

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

comment:9 by bastiK, 13 years ago

Works for me.

comment:10 by brycenesbitt, 13 years ago

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.

by brycenesbitt, 13 years ago

Attachment: changeset_try4.diff added

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

comment:11 by brycenesbitt, 13 years ago

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 by bastiK, 13 years ago

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 by brycenesbitt, 13 years ago

Ok, are things ready to accept this now?

comment:14 by bastiK, 13 years ago

In [4413/josm]:

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

in reply to:  13 comment:15 by bastiK, 13 years ago

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 by stoecker, 13 years ago

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

comment:17 by brycenesbitt, 13 years ago

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 by bastiK, 13 years ago

Resolution: fixed
Status: newclosed

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>

Modify Ticket

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

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.