Modify

Opened 6 years ago

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

Download all attachments as: .zip

Change History (22)

Changed 6 years ago by brycenesbitt

Attachment: import_changeset_tags.diff added

svn diff against trunk for this feature.

comment:1 Changed 6 years ago by anonymous

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 Changed 6 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 6 years ago by stoecker

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

comment:4 Changed 6 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 6 years ago by brycenesbitt

Try 2 at patch

comment:5 in reply to:  4 Changed 6 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 6 years ago by brycenesbitt

Attachment: josm_try_3.diff added

Ok, fine. Try 3 with <changeset> syntax

comment:6 Changed 6 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 6 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 6 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 6 years ago by bastiK

Works for me.

comment:10 Changed 6 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 6 years ago by brycenesbitt

Attachment: changeset_try4.diff added

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

comment:11 Changed 6 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 6 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 Changed 6 years ago by brycenesbitt

Ok, are things ready to accept this now?

comment:14 Changed 6 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 6 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 6 years ago by stoecker

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

comment:17 Changed 6 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 6 years ago by bastiK

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.

Add Comment


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

 
Note: See TracTickets for help on using tickets.