Skip to content

Commit

Permalink
Merge remote-tracking branch 'zere/master'
Browse files Browse the repository at this point in the history
  • Loading branch information
@Gnonthgol
Gnonthgol committed Apr 25, 2012
2 parents 732a643 + cc7ad9b commit c7c17ba
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 178 deletions.
37 changes: 26 additions & 11 deletions change_bot.rb
View file
Expand Up @@ -14,6 +14,25 @@ def initialize(versions, db)
@versions.sort_by! {|obj| obj.version}
end

def odbl_reset?
# we need to track odbl clean-ness. if the tag is set and
# later unset, then we consider it a mistake.
odbl_set = false
odbl_reset = false

@versions.each do |obj|
odbl = Tags.odbl_clean?(obj.tags)
if odbl
odbl_set = true
odbl_reset = false
elsif odbl_set
odbl_reset = true
end
end

return odbl_reset
end

def actions
prev_obj = @versions.first.version_zero

Expand All @@ -22,6 +41,12 @@ def actions

tainted_tags = Array.new

# ignore odbl flags if the odbl=clean tag ends up being reset
# at some later point in history - we consider this case to
# be an indicator that the original setting of odbl=clean was
# in error.
ignore_odbl = odbl_reset?

@versions.each do |obj|
# deletions are always "clean", and we consider them to
# have no tags and the "version zero" geometry. what
Expand All @@ -39,7 +64,7 @@ def actions

# is this version clean? there are many ways to be
# clean, and we try to enumerate them here.
status = if Tags.odbl_clean?(obj.tags)
status = if Tags.odbl_clean?(obj.tags) and not ignore_odbl
:odbl_clean
elsif changeset_is_accepted?(obj.changeset_id)
:acceptor_edit
Expand All @@ -51,11 +76,6 @@ def actions
:unclean
end

#puts
#puts "[#{obj.version}] status:#{status}"
#puts "[#{obj.version}] geom:#{obj.geom}"
#puts "[#{obj.version}] geom_patch:#{geom_patch}"

# if this is not a clean version, then the only part
# of the patch we can apply is the deletions, by the
# 'deletions are always OK' rule.
Expand Down Expand Up @@ -94,9 +114,6 @@ def actions
new_tags.delete(k) if new_tags[k] == v
end

#puts "[#{obj.version}] new_geom:#{new_geom}"
#puts "[#{obj.version}] obj_geom:#{obj.geom}"

# if the result of applying the patches is any different
# from the version we actually have, then the object is
# in a state that we can't display, so redact it.
Expand All @@ -112,8 +129,6 @@ def actions
base_obj.geom = new_geom
base_obj.tags = new_tags

#puts "[#{obj.version}] base_obj:#{base_obj}"

prev_obj = obj
end

Expand Down
2 changes: 1 addition & 1 deletion geom.rb
View file
Expand Up @@ -39,7 +39,7 @@ def apply!(obj, options = {})
end

def to_s
"NodeDiff[#{@null_move},#{@position}]"
@null_move ? "NodeDiff[]" : "NodeDiff#{@position.inspect}"
end

private
Expand Down
8 changes: 6 additions & 2 deletions test_auto_fail.rb
View file
Expand Up @@ -3570,7 +3570,8 @@ def test_automatic_node296700000
bot = ChangeBot.new(@db)
actions = bot.action_for(history)
assert_equal([Delete[OSM::Node, 296700000],
Redact[OSM::Node, 296700000, 1, :hidden]
Redact[OSM::Node, 296700000, 1, :hidden],
Redact[OSM::Node, 296700000, 2, :visible] # some tags changed, but most still tainted
], actions)
end

Expand All @@ -3584,7 +3585,10 @@ def test_automatic_node296800000
]
bot = ChangeBot.new(@db)
actions = bot.action_for(history)
assert_equal([Edit[OSM::Node[[50.2204098, 15.8607321], :id => 296800000, :version => 3, :visible => true, :changeset => -1 ]],
assert_equal([Edit[OSM::Node[[50.2204098, 15.8607321], :id => 296800000, :version => 3, :visible => true, :changeset => -1,
"addr:conscriptionnumber" => "483", # significant move in v2
"addr:housenumber" => "483/5", # significant edit in v2
"addr:streetnumber" => "5"]], # added in v2, agreed
Redact[OSM::Node, 296800000, 1, :hidden], # this is an ugly test and it is never going to work. all tags in the v2 and v3
Redact[OSM::Node, 296800000, 2, :visible], # are obviously derived from v1 and therefore need to be removed, but it is
Redact[OSM::Node, 296800000, 3, :visible] # virtually impossible to determine that automatically.
Expand Down
130 changes: 0 additions & 130 deletions test_needs_clarity.rb
View file
Expand Up @@ -26,136 +26,6 @@ def setup
3 => Changeset[User[false]]
})
end

# this is a node with some early bad content all of which has been eradicated many versions ago
# It also has an old tag mapped by a problem mapper reintroduced later by an agreeing mapper.
# LWG has clarified that such reintroductions are clean _if_ they happen in a separate changeset
# to the removal of the tag. (that is, tax is put back in a separate context, we apply good faith in the agreeing mapper)
#
# NOTE: this needs some thought.
# the issue is that the "foo=bar" tag re-added in v9 is the same as a tag added by
# a decliner in v1. on the basis of identity it might be hard to tell whether this
# is newly-surveyed data, or added by looking at the object's history.
#
# so the question is: can a tag added by an agreer in a later version of an element,
# even though it may be similar to a previously-removed tag added by a decliner, be
# considered clean?
#
def test_node_reformed_ccoholic
history = [OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 1, "foo" => "bar"], # created by decliner
OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 2 ], # tag removed by decliner
OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 3, "sugar" => "sweet" ], # tag added by decliner
OSM::Node[[1,1], :id => 1, :changeset => 2, :version => 4, "sugar" => "sweet", "bar" => "baz"], # other tag added, node moved by agreer
OSM::Node[[1,1], :id => 1, :changeset => 3, :version => 5, "sugar" => "sweet", "rose" => "red", "bar" => "baz" ], # tag added by decliner
OSM::Node[[1,1], :id => 1, :changeset => 2, :version => 6, "sugar" => "sweet", "bar" => "baz", "dapper" => "mapper" ], # tag added by agreer, dirty tag removed
OSM::Node[[2,2], :id => 1, :changeset => 1, :version => 7, "bar" => "baz", "dapper" => "mapper" ], # moved by agreer
OSM::Node[[2,2], :id => 1, :changeset => 2, :version => 8, "bar" => "baz", "dapper" => "mapper", "e" => "mc**2" ], # tag added by agreer
OSM::Node[[2,2], :id => 1, :changeset => 2, :version => 9, "bar" => "baz", "dapper" => "mapper", "e" => "mc**2", "foo" => "bar" ]] # tag re-added by agreer
bot = ChangeBot.new(@db)
actions = bot.action_for(history)
# this is effectively a revert back to version 8 (because v9 re-adds the tainted "foo=bar" tag)
# ^^^^^^^^^^^^^^^^^^^^^^ -- this needs more thought
# and then hides version 6 and before because v6-v3 have the "sugar=sweet" tag which was added
# by a decliner and is therefore tainted, and v1 & v2 are decliner edits.
assert_equal([Redact[OSM::Node, 1, 1, :hidden],
Redact[OSM::Node, 1, 2, :hidden],
Redact[OSM::Node, 1, 3, :hidden],
Redact[OSM::Node, 1, 4, :visible],
Redact[OSM::Node, 1, 5, :hidden],
Redact[OSM::Node, 1, 6, :visible],
], actions)
end

# Identical to test_node_reformed_ccoholic but the tag is reintroduced in the same change set as it is deleted
# LWG has concluded that this may be risky and would prefer to see odbl=clean used in such cases with no tag deletion and replacement

def test_node_reformed_ccoholic_too_hasty
history = [OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 1, "foo" => "bar"], # created by decliner
OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 2, "foo" => "bar", "diddle" => "dum" ], # tag added by decliner
OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 3, "foo" => "bar", "diddle" => "dum", "sugar" => "sweet" ], # tag added by decliner
OSM::Node[[1,1], :id => 1, :changeset => 2, :version => 4, "foo" => "bar", "diddle" => "dum", "sugar" => "sweet", "bar" => "baz"], # other tag added, node moved by agreer
OSM::Node[[1,1], :id => 1, :changeset => 3, :version => 5, "foo" => "bar", "diddle" => "dum", "sugar" => "sweet", "bar" => "baz", "sugar" => "sweet", "rose" => "red"], # tag added by decliner
OSM::Node[[1,1], :id => 1, :changeset => 2, :version => 6, "bar" => "baz", "dapper" => "mapper" ], # tag added by agreer, dirty tags removed
OSM::Node[[2,2], :id => 1, :changeset => 2, :version => 7, "bar" => "baz", "dapper" => "mapper", "foo" => "bar"], # Previously dirty tag added back in **same changeset as deletion**
OSM::Node[[2,2], :id => 1, :changeset => 2, :version => 8, "bar" => "baz", "dapper" => "mapper", "e" => "mc**2", "foo" => "bar" ], # tag added by agreer
OSM::Node[[2,2], :id => 1, :changeset => 2, :version => 9, "bar" => "baz", "dapper" => "mapper", "e" => "mc**2", "foo" => "bar", "bored" => "yet?" ]] # new tag added by agreer
bot = ChangeBot.new(@db)
actions = bot.action_for(history)

assert_equal([Edit[OSM::Node[[2,2], :id => 1, :changeset => -1, :version => 9, "bar" => "baz", "dapper" => "mapper", "e" => "mc**2", "bored" => "yet?" ]],
Redact[OSM::Node, 1, 1, :hidden],
Redact[OSM::Node, 1, 2, :hidden],
Redact[OSM::Node, 1, 3, :hidden],
Redact[OSM::Node, 1, 4, :visible],
Redact[OSM::Node, 1, 5, :hidden],
# Redact[OSM::Node, 1, 6, :visible], # Surely this version is clean and needs no redaction?
Redact[OSM::Node, 1, 7, :visible],
Redact[OSM::Node, 1, 8, :visible],
Redact[OSM::Node, 1, 9, :visible],
], actions)
end

# We can even keep (some...) changes to a tag created by a non-agreeing mapper
#
# NOTE: needs some thought.
# the issue here is whether the *keys* of tags contain any copyright status.
# here, the key is changed from "foo"="bar" to "foo"="feefie", which is a
# Significant Change to the value (see tests for that in test_tags.rb), but
# is no change to the key.
#
# if we assume that keys are potentially copyrightable then we must reject
# the following test case, and potentially leave a lot of "highway"= and
# "name"= tags in an old state (or remove them). on the other hand, some
# tags may well have copyright-worthy information in the keys, given that
# they're free-form strings just like the values.
def test_simple_node_unclean_edited_clean_later_position_bad_tag_changed
history = [OSM::Node[[0,0], :id => 1, :changeset => 3, :version => 1, "wibble" => "wobble", "foo" => "bar"],
OSM::Node[[1,1], :id => 1, :changeset => 1, :version => 2, "wibble" => "wobble", "foo" => "feefie"]]
bot = ChangeBot.new(@db)
actions = bot.action_for(history)
assert_equal([Edit[OSM::Node[[1,1], :id => 1, :changeset => -1, :version => 2, "foo" => "feefie"]],
Redact[OSM::Node, 1, 1, :hidden],
Redact[OSM::Node, 1, 2, :visible]
], actions)
end

# relation created by agreer, then order changed by decliner
#
# this is tricky for the same reason as the way-node ordering stuff above.
# the change of order can equally be interpreted as a deletion and
# addition, which trigger a completely different set of rules.
#
def test_relation_order_changed
history = [OSM::Relation[[ [OSM::Way,1] , [OSM::Way,4], [OSM::Way,2], [OSM::Way,3] ], :id => 1, :changeset => 1, :version => 1, "type" => "route" ],
OSM::Relation[[ [OSM::Way,1] , [OSM::Way,2], [OSM::Way,3], [OSM::Way,4] ], :id => 1, :changeset => 3, :version => 2, "type" => "route" ]]
bot = ChangeBot.new(@db)
actions = bot.action_for(history)
assert_equal([Edit[OSM::Relation[[ [OSM::Way,1], [OSM::Way,4], [OSM::Way,2], [OSM::Way,3] ], :id=>1, :changeset=>-1, :version=>3, "type" => "route"]],
Redact[OSM::Relation,1,2,:hidden]
], actions)
end

# relation created by agreer, then order changed by decliner, then extra members added by agreer
# (we can't preserve order in this case. Difficult to know what to do - fall back to the last
# good order, with new members appended? Or try and insert at the same index, even if it doesn't
# make any sense without the previous order?)
#
# this is tricky for the same reason as the way-node ordering stuff above.
# the change of order can equally be interpreted as a deletion and
# addition, which trigger a completely different set of rules.
#
def test_relation_order_changed_then_member_appended
history = [OSM::Relation[[ [OSM::Way,1] , [OSM::Way,4], [OSM::Way,2], [OSM::Way,3] ], :id => 1, :changeset => 1, :version => 1, "type" => "route" ],
OSM::Relation[[ [OSM::Way,1] , [OSM::Way,2], [OSM::Way,3], [OSM::Way,4] ], :id => 1, :changeset => 3, :version => 2, "type" => "route" ],
OSM::Relation[[ [OSM::Way,1] , [OSM::Way,2], [OSM::Way,3], [OSM::Way,4], [OSM::Way,5] ], :id => 1, :changeset => 2, :version => 3, "type" => "route" ]]
bot = ChangeBot.new(@db)
actions = bot.action_for(history)
assert_equal([Edit[OSM::Relation[[ [OSM::Way,1], [OSM::Way,4], [OSM::Way,2], [OSM::Way,3], [OSM::Way,5] ], :id=>1, :changeset=>-1, :version=>4, "type" => "route"]],
Redact[OSM::Relation,1,2,:hidden],
Redact[OSM::Relation,1,3,:visible]
], actions)
end

end

if __FILE__ == $0
Expand Down

0 comments on commit c7c17ba

Please sign in to comment.