Add a more complecated test case for odbl=clean wierdness and fix

change_bot.rb

@@ -14,23 +14,26 @@ def initialize(versions, db)
     @versions.sort_by! {|obj| obj.version}
   end
 
-  def odbl_reset?
+  def odbl_clean? (o)
     # 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
+    # early exit if this is not taged clean
+    if not Tags.odbl_clean?(o.tags)
+      return false
+    end
+
+    # see if it is part of the last set of objects with the odbl=clean tag
+    @versions.reverse.each do |obj|
+      if obj == o
+        return true
+      elsif not Tags.odbl_clean?(obj.tags)
+        # there is a version later then this that does not have the odbl=clean tag
+        return false
       end
     end
 
-    return odbl_reset
+    return false
   end
 
   def actions
@@ -41,12 +44,6 @@ 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
@@ -64,7 +61,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) and not ignore_odbl
+      status = if odbl_clean?(obj)
                  :odbl_clean
                elsif changeset_is_accepted?(obj.changeset_id)
                  :acceptor_edit

test_odbl_tag.rb

@@ -124,6 +124,21 @@ def test_node_odbl_clean_removed
                   Redact[OSM::Node, 1, 4, :visible]
                  ], actions)
   end
+
+  # What if someone was adding and removing the odbl=clean tag
+  def test_node_odbl_clean_removed
+    history = [OSM::Node[[0,0], :id=>1, :changeset => 1, :version => 1], # created by agreer
+               OSM::Node[[0,0], :id=>1, :changeset => 3, :version => 2, "foo" => "bar"], # edited by decliner
+               OSM::Node[[0,0], :id=>1, :changeset => 2, :version => 3, "foo" => "bar", "odbl" => "clean"], # odbl=clean added by agreer
+               OSM::Node[[0,0], :id=>1, :changeset => 2, :version => 4, "foo" => "bar"], # odbl tag removed by agreer
+               OSM::Node[[0,0], :id=>1, :changeset => 2, :version => 5, "odbl" => "clean"]] # object cleaned and odbl tag reintroduced by agreer
+    bot = ChangeBot.new(@db)
+    actions = bot.action_for(history)
+    assert_equal([Redact[OSM::Node, 1, 2, :hidden],
+                  Redact[OSM::Node, 1, 3, :visible],
+                  Redact[OSM::Node, 1, 4, :visible]
+                 ], actions)
+  end
 end
 
 if __FILE__ == $0