Adding first cut of Redactions support
authorMatt Amos <zerebubuth@gmail.com>
Wed, 28 Mar 2012 11:37:40 +0000 (12:37 +0100)
committerTom Hughes <tom@compton.nu>
Thu, 5 Apr 2012 12:46:36 +0000 (13:46 +0100)
Redactions support hiding historical versions of elements and
collecting meta-data about that action together.

20 files changed:
app/controllers/old_node_controller.rb
app/models/node.rb
app/models/old_node.rb
app/models/old_relation.rb
app/models/old_way.rb
app/models/redaction.rb [new file with mode: 0644]
app/models/relation.rb
app/models/way.rb
config/routes.rb
lib/not_redactable.rb [new file with mode: 0644]
lib/osm.rb
lib/redactable.rb [new file with mode: 0644]
test/fixtures/current_nodes.yml
test/fixtures/nodes.yml
test/fixtures/redactions.yml [new file with mode: 0644]
test/functional/old_node_controller_test.rb
test/test_helper.rb
test/unit/node_test.rb
test/unit/old_node_test.rb
test/unit/redaction_test.rb [new file with mode: 0644]

index 615213ac18996f3a4418cbd71fd49d1c4ea7d9c2..e6170fbda0258aad3c3f4b24bbc171693c58ed0f 100644 (file)
@@ -2,17 +2,26 @@ class OldNodeController < ApplicationController
   require 'xml/libxml'
 
   skip_before_filter :verify_authenticity_token
+  before_filter :authorize, :only => [ :redact ]
+  before_filter :require_allow_write_api, :only => [ :redact ]
   before_filter :check_api_readable
+  before_filter :check_api_writable, :only => [ :redact ]
   after_filter :compress_output
   around_filter :api_call_handle_error, :api_call_timeout
 
   def history
-    node = Node.find(params[:id])
+    # TODO - maybe a bit heavyweight to do this on every
+    # call, perhaps try lazy auth.
+    setup_user_auth
+
+    node = Node.find(params[:id].to_i)
     
     doc = OSM::API.new.get_xml_doc
     
     node.old_nodes.each do |old_node|
-      doc.root << old_node.to_xml_node
+      unless old_node.redacted? and (@user.nil? or not @user.moderator?)
+        doc.root << old_node.to_xml_node
+      end
     end
     
     render :text => doc.to_s, :content_type => "text/xml"
@@ -20,14 +29,32 @@ class OldNodeController < ApplicationController
   
   def version
     if old_node = OldNode.where(:node_id => params[:id], :version => params[:version]).first
-      response.last_modified = old_node.timestamp
-
-      doc = OSM::API.new.get_xml_doc
-      doc.root << old_node.to_xml_node
+      # TODO - maybe a bit heavyweight to do this on every
+      # call, perhaps try lazy auth.
+      setup_user_auth
+      
+      if old_node.redacted? and (@user.nil? or not @user.moderator?)
+        render :nothing => true, :status => :forbidden
+      else
 
-      render :text => doc.to_s, :content_type => "text/xml"
+        response.last_modified = old_node.timestamp
+        
+        doc = OSM::API.new.get_xml_doc
+        doc.root << old_node.to_xml_node
+        
+        render :text => doc.to_s, :content_type => "text/xml"
+      end
     else
       render :nothing => true, :status => :not_found
     end
   end
+
+  def redact
+    if @user && @user.moderator?
+      render :nothing => true
+
+    else
+      render :nothing => true, :status => :forbidden
+    end
+  end
 end
index 53787245916d5c60bca1e16060e17cd0183791e2..0953241d91f2767030f452f5caafac73c193a4ff 100644 (file)
@@ -3,6 +3,7 @@ class Node < ActiveRecord::Base
 
   include GeoRecord
   include ConsistencyValidations
+  include NotRedactable
 
   self.table_name = "current_nodes"
 
index 64ae1a70c9d8fe2050f2ccb3283bc04414dae030..5643a389bc8ad69fb83f7f886b122e97f4cbe8ff 100644 (file)
@@ -1,6 +1,7 @@
 class OldNode < ActiveRecord::Base
   include GeoRecord
   include ConsistencyValidations
+  include Redactable
 
   self.table_name = "nodes"
   self.primary_keys = "node_id", "version"
@@ -12,7 +13,9 @@ class OldNode < ActiveRecord::Base
   validates_associated :changeset
 
   belongs_to :changeset
+  belongs_to :redaction
+  belongs_to :current_node, :class_name => "Node", :foreign_key => "node_id"
+
   def validate_position
     errors.add(:base, "Node is not in the world") unless in_world?
   end
@@ -106,4 +109,10 @@ class OldNode < ActiveRecord::Base
   def containing_relation_members 
     return [] 
   end 
+
+  # check whether this element is the latest version - that is,
+  # has the same version as its "current" counterpart.
+  def is_latest_version?
+    current_node.version == self.version
+  end
 end
index a75dacd0eba9b04d28a4e6e3264bc6ea58877dae..e4be8fc7fd168c3433c44910e894662c0870e547 100644 (file)
@@ -1,10 +1,13 @@
 class OldRelation < ActiveRecord::Base
   include ConsistencyValidations
+  include Redactable
   
   self.table_name = "relations"
   self.primary_keys = "relation_id", "version"
 
   belongs_to :changeset
+  belongs_to :redaction
+  belongs_to :current_relation, :class_name => "Relation", :foreign_key => "relation_id"
 
   has_many :old_members, :class_name => 'OldRelationMember', :foreign_key => [:relation_id, :version], :order => :sequence_id
   has_many :old_tags, :class_name => 'OldRelationTag', :foreign_key => [:relation_id, :version]
@@ -130,4 +133,10 @@ class OldRelation < ActiveRecord::Base
   def containing_relation_members
     return []
   end
+
+  # check whether this element is the latest version - that is,
+  # has the same version as its "current" counterpart.
+  def is_latest_version?
+    current_relation.version == self.version
+  end
 end
index db8a6df4061ae3f8d60194c86c885f29462d95a5..3df0c2d4f3b53bc99c0c27e4c276227e59a5d582 100644 (file)
@@ -1,10 +1,13 @@
 class OldWay < ActiveRecord::Base
   include ConsistencyValidations
-  
+  include Redactable
+
   self.table_name = "ways"
   self.primary_keys = "way_id", "version"
 
   belongs_to :changeset
+  belongs_to :redaction
+  belongs_to :current_way, :class_name => "Way", :foreign_key => "way_id"
 
   has_many :old_nodes, :class_name => 'OldWayNode', :foreign_key => [:way_id, :version]
   has_many :old_tags, :class_name => 'OldWayTag', :foreign_key => [:way_id, :version]
@@ -158,4 +161,10 @@ class OldWay < ActiveRecord::Base
   def containing_relation_members
     return []
   end
+
+  # check whether this element is the latest version - that is,
+  # has the same version as its "current" counterpart.
+  def is_latest_version?
+    current_way.version == self.version
+  end
 end
diff --git a/app/models/redaction.rb b/app/models/redaction.rb
new file mode 100644 (file)
index 0000000..28a4512
--- /dev/null
@@ -0,0 +1,14 @@
+##
+# Redaction represents a record associated with a particular
+# action on the database to hide revisions from the history
+# which are not appropriate to redistribute any more. 
+#
+# The circumstances of the redaction can be recorded in the
+# record's title and description fields, which can be 
+# displayed linked from the redacted records.
+#
+class Redaction < ActiveRecord::Base
+  has_many :nodes
+  has_many :ways
+  has_many :relations
+end
index 21ee058d8ac80e26f6969c6d7c5b49052e9075f5..e5ea85d6d02788eadec62b9768e6c19120b04854 100644 (file)
@@ -2,7 +2,8 @@ class Relation < ActiveRecord::Base
   require 'xml/libxml'
   
   include ConsistencyValidations
-  
+  include NotRedactable
+
   self.table_name = "current_relations"
 
   belongs_to :changeset
index adf8b928fa7a75fe8100455d7b51373e515bee7c..9bd792ddcf56dfd128e41bc2701d75f2daa0bfa3 100644 (file)
@@ -2,6 +2,7 @@ class Way < ActiveRecord::Base
   require 'xml/libxml'
   
   include ConsistencyValidations
+  include NotRedactable
 
   self.table_name = "current_ways"
   
index 779f9b2a7f4a00945d4fe57cfe74899374044cea..823c00950c1519d64f1cbb4b544b976fb170f46b 100644 (file)
@@ -16,6 +16,7 @@ OpenStreetMap::Application.routes.draw do
   match 'api/0.6/node/:id/ways' => 'way#ways_for_node', :via => :get, :id => /\d+/
   match 'api/0.6/node/:id/relations' => 'relation#relations_for_node', :via => :get, :id => /\d+/
   match 'api/0.6/node/:id/history' => 'old_node#history', :via => :get, :id => /\d+/
+  match 'api/0.6/node/:id/:version/redact' => 'old_node#redact', :version => /\d+/, :id => /\d+/
   match 'api/0.6/node/:id/:version' => 'old_node#version', :via => :get, :id => /\d+/, :version => /\d+/
   match 'api/0.6/node/:id' => 'node#read', :via => :get, :id => /\d+/
   match 'api/0.6/node/:id' => 'node#update', :via => :put, :id => /\d+/
diff --git a/lib/not_redactable.rb b/lib/not_redactable.rb
new file mode 100644 (file)
index 0000000..d7c9524
--- /dev/null
@@ -0,0 +1,11 @@
+require 'osm'
+
+module NotRedactable
+  def redacted?
+    false
+  end
+
+  def redact!(r)
+    raise OSM::APICannotRedactError.new
+  end
+end
index 1a22af93a67b2806dbf4515f126a4e3f11fe6454..8902e6c9e25f7ecd97f50ca6c782b1ad3baf6f66 100644 (file)
@@ -268,6 +268,19 @@ module OSM
     end
   end
 
+  ##
+  # raised when someone tries to redact a current version of
+  # an element - only historical versions can be redacted.
+  class APICannotRedactError < APIError
+    def status
+      :bad_request
+    end
+
+    def to_s
+      "Cannot redact current version of element, only historical versions may be redacted."
+    end
+  end
+
   # Helper methods for going to/from mercator and lat/lng.
   class Mercator
     include Math
diff --git a/lib/redactable.rb b/lib/redactable.rb
new file mode 100644 (file)
index 0000000..b994e85
--- /dev/null
@@ -0,0 +1,15 @@
+require 'osm'
+
+module Redactable
+  def redacted?
+    not self.redaction.nil?
+  end
+
+  def redact!(redaction)
+    # check that this version isn't the current version
+    raise OSM::APICannotRedactError.new if self.is_latest_version?
+
+    # make the change
+    self.redaction = redaction
+  end
+end
index b0761a264db8c45c2433d71b56e5f6e66e4ea06f..3f34ddabefac20d98112e619f1e5c58d86da7680 100644 (file)
@@ -161,3 +161,13 @@ public_visible_node:
   tile: <%= QuadTile.tile_for_point(1,1) %>
   timestamp: 2007-01-01 00:00:00
 
+redacted_node:
+  id: 17
+  latitude: <%= 1*SCALE %>
+  longitude: <%= 1*SCALE %>
+  changeset_id: 2
+  visible: false
+  version: 2
+  tile: <%= QuadTile.tile_for_point(1,1) %>
+  timestamp: 2007-01-01 00:00:00
+
index 5125e7146b6f78be20e627899a14d0200f617613..5de3d94af0115072107ff0e333c6945109ff013b 100644 (file)
@@ -191,3 +191,23 @@ public_visible_node:
   tile: <%= QuadTile.tile_for_point(1,1) %>
   timestamp: 2007-01-01 00:00:00
 
+redacted_node_redacted_version:
+  node_id: 17
+  latitude: <%= 1*SCALE %>
+  longitude: <%= 1*SCALE %>
+  changeset_id: 2
+  visible: true
+  version: 1
+  tile: <%= QuadTile.tile_for_point(1,1) %>
+  timestamp: 2007-01-01 00:00:00
+  redaction_id: 1
+
+redacted_node_current_version:
+  node_id: 17
+  latitude: <%= 1*SCALE %>
+  longitude: <%= 1*SCALE %>
+  changeset_id: 2
+  visible: false
+  version: 2
+  tile: <%= QuadTile.tile_for_point(1,1) %>
+  timestamp: 2007-01-01 00:00:00
diff --git a/test/fixtures/redactions.yml b/test/fixtures/redactions.yml
new file mode 100644 (file)
index 0000000..6933cb2
--- /dev/null
@@ -0,0 +1,6 @@
+# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/Fixtures.html
+
+example:
+  id: 1
+  title: Example redaction
+  description: Usually some text would go here to say something about why the redaction happened.
index 3c0358c2b9a7465704c953390ea017d9e1aba0d2..efef38ad18c19fb9c808f29db202cf12e2ca3b01 100644 (file)
@@ -164,7 +164,121 @@ class OldNodeControllerTest < ActionController::TestCase
     check_current_version(current_nodes(:node_used_by_relationship))
     check_current_version(current_nodes(:node_with_versions))
   end
-  
+
+  ##
+  # test the redaction of an old version of a node, while not being
+  # authorised.
+  def test_redact_node_unauthorised
+    do_redact_node(nodes(:node_with_versions_v3),
+                   redactions(:example))
+    assert_response :unauthorized, "should need to be authenticated to redact."
+  end
+
+  ##
+  # test the redaction of an old version of a node, while being 
+  # authorised as a normal user.
+  def test_redact_node_normal_user
+    basic_authorization(users(:public_user).email, "test")
+
+    do_redact_node(nodes(:node_with_versions_v3),
+                   redactions(:example))
+    assert_response :forbidden, "should need to be moderator to redact."
+  end
+
+  ##
+  # test that, even as moderator, the current version of a node
+  # can't be redacted.
+  def test_redact_node_current_version
+    basic_authorization(users(:moderator_user).email, "test")
+
+    do_redact_node(nodes(:node_with_versions_v4),
+                   redactions(:example))
+    assert_response :forbidden, "shouldn't be OK to redact current version as moderator."
+  end    
+
+  ##
+  # test that redacted nodes aren't visible, regardless of 
+  # authorisation except as moderator...
+  def test_version_redacted
+    node = nodes(:redacted_node_redacted_version)
+
+    get :version, :id => node.node_id, :version => node.version
+    assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
+
+    # not even to a logged-in user
+    basic_authorization(users(:public_user).email, "test")
+    get :version, :id => node.node_id, :version => node.version
+    assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in."
+  end
+
+  ##
+  # test that redacted nodes aren't visible in the history
+  def test_history_redacted
+    node = nodes(:redacted_node_redacted_version)
+
+    get :history, :id => node.node_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm node[id=#{node.node_id}][version=#{node.version}]", 0, "redacted node #{node.node_id} version #{node.version} shouldn't be present in the history."
+
+    # not even to a logged-in user
+    basic_authorization(users(:public_user).email, "test")
+    get :version, :id => node.node_id, :version => node.version
+    get :history, :id => node.node_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm node[id=#{node.node_id}][version=#{node.version}]", 0, "redacted node #{node.node_id} version #{node.version} shouldn't be present in the history, even when logged in."
+  end
+
+  ##
+  # test the redaction of an old version of a node, while being 
+  # authorised as a moderator.
+  def test_redact_node_moderator
+    node = nodes(:node_with_versions_v3)
+    basic_authorization(users(:moderator_user).email, "test")
+
+    do_redact_node(node, redactions(:example))
+    assert_response :success, "should be OK to redact old version as moderator."
+
+    # check moderator can still see the redacted data
+    get :version, :id => node.node_id, :version => node.version
+    assert_response :success, "After redaction, node should not be gone for moderator."
+    
+    # and when accessed via history
+    get :history, :id => node.node_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm node[id=#{node.node_id}][version=#{node.version}]", 1, "node #{node.node_id} version #{node.version} should still be present in the history for moderators."
+  end
+
+  # testing that if the moderator drops auth, he can't see the
+  # redacted stuff any more.
+  def test_redact_node_is_redacted
+    node = nodes(:node_with_versions_v3)
+    basic_authorization(users(:moderator_user).email, "test")
+
+    do_redact_node(node, redactions(:example))
+    assert_response :success, "should be OK to redact old version as moderator."
+
+    # re-auth as non-moderator
+    basic_authorization(users(:public_user).email, "test")
+
+    # check can't see the redacted data
+    get :version, :id => node.node_id, :version => node.version
+    assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
+    
+    # and when accessed via history
+    get :version, :id => node.node_id, :version => node.version
+    get :history, :id => node.node_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm node[id=#{node.node_id}][version=#{node.version}]", 0, "redacted node #{node.node_id} version #{node.version} shouldn't be present in the history."
+  end
+
+  def do_redact_node(node, redaction)
+    get :version, :id => node.node_id, :version => node.version
+    assert_response :success, "should be able to get version #{node.version} of node #{node.node_id}."
+    
+    # now redact it
+    post :redact, :id => node.node_id, :version => node.version, :redaction => redaction.id
+  end
+
   def check_current_version(node_id)
     # get the current version of the node
     current_node = with_controller(NodeController.new) do
index 8fcc7a94f33a8933de766cda5a52f250ddeb3bcc..59b87da847ab8e835836f68d6944eecdca1ec179 100644 (file)
@@ -51,6 +51,8 @@ class ActiveSupport::TestCase
     set_fixture_class :gpx_file_tags => 'Tracetag'
 
     fixtures :client_applications
+    
+    fixtures :redactions
   end
 
   ##
index 78f060678ecdf542cd18b3ee77d23a6f2be02a11..753e6a95c06fe9836a7fbd92de166b15b7098f4d 100644 (file)
@@ -4,7 +4,7 @@ class NodeTest < ActiveSupport::TestCase
   api_fixtures
   
   def test_node_count
-    assert_equal 16, Node.count
+    assert_equal 17, Node.count
   end
 
   def test_node_too_far_north
index e6f3d80356adafe3d659cd704999ba53b54978b1..84ea65b68efb762c352923f95e2172d242006862 100644 (file)
@@ -4,7 +4,7 @@ class OldNodeTest < ActiveSupport::TestCase
   api_fixtures
   
   def test_old_node_count
-    assert_equal 19, OldNode.count
+    assert_equal 21, OldNode.count
   end
 
   def test_node_too_far_north
diff --git a/test/unit/redaction_test.rb b/test/unit/redaction_test.rb
new file mode 100644 (file)
index 0000000..c19570f
--- /dev/null
@@ -0,0 +1,36 @@
+require File.dirname(__FILE__) + '/../test_helper'
+require 'osm'
+
+class RedactionTest < ActiveSupport::TestCase
+  api_fixtures
+  fixtures :redactions
+
+  def test_cannot_redact_current
+    n = current_nodes(:node_with_versions)
+    r = redactions(:example)
+    assert_equal(false, n.redacted?, "Expected node to not be redacted already.")
+    assert_raise(OSM::APICannotRedactError) do
+      n.redact!(r)
+    end
+  end
+
+  def test_cannot_redact_current_via_old
+    n = nodes(:node_with_versions_v4)
+    r = redactions(:example)
+    assert_equal(false, n.redacted?, "Expected node to not be redacted already.")
+    assert_raise(OSM::APICannotRedactError) do
+      n.redact!(r)
+    end
+  end
+
+  def test_can_redact_old
+    n = nodes(:node_with_versions_v3)
+    r = redactions(:example)
+    assert_equal(false, n.redacted?, "Expected node to not be redacted already.")
+    assert_nothing_raised(OSM::APICannotRedactError) do
+      n.redact!(r)
+    end
+    assert_equal(true, n.redacted?, "Expected node to be redacted after redact! call.")
+  end
+
+end