Validate characters in changeset comments

[rails.git] / app / models / relation.rb

diff --git a/app/models/relation.rb b/app/models/relation.rb
index 9e9e7144f93f3fa4c3108d19ca30368b0fa79d6b..d244a6d6cf2cb23214d7a89960fb6b2d7b66d06a 100644 (file)
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -33,7 +33,7 @@ class Relation < ActiveRecord::Base
   scope :ways, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Way", :member_id => ids.flatten }) }
   scope :relations, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Relation", :member_id => ids.flatten }) }
 
   scope :ways, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Way", :member_id => ids.flatten }) }
   scope :relations, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Relation", :member_id => ids.flatten }) }
 

-  TYPES = %w(node way relation)
+  TYPES = %w(node way relation).freeze

 
   def self.from_xml(xml, create = false)
     p = XML::Parser.string(xml)
 
   def self.from_xml(xml, create = false)
     p = XML::Parser.string(xml)


@@ -86,13 +86,10 @@ class Relation < ActiveRecord::Base
 
     pt.find("member").each do |member|
       # member_type =
 
     pt.find("member").each do |member|
       # member_type =

-      logger.debug "each member"

       fail OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"]
       fail OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"]

-      logger.debug "after raise"

       # member_ref = member['ref']
       # member_role
       member["role"] ||= "" # Allow  the upload to not include this, in which case we default to an empty string.
       # member_ref = member['ref']
       # member_role
       member["role"] ||= "" # Allow  the upload to not include this, in which case we default to an empty string.

-      logger.debug member["role"]

       relation.add_member(member["type"].classify, member["ref"], member["role"])
     end
     fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
       relation.add_member(member["type"].classify, member["ref"], member["role"])
     end
     fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?


@@ -186,10 +183,10 @@ class Relation < ActiveRecord::Base
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Relation.transaction do
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Relation.transaction do

-      self.lock!
+      lock!

       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation

-      rel = RelationMember.joins(:relation).where("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id).first
+      rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)

       fail OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil?
 
       self.changeset_id = new_relation.changeset_id
       fail OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil?
 
       self.changeset_id = new_relation.changeset_id


@@ -202,7 +199,7 @@ class Relation < ActiveRecord::Base
 
   def update_from(new_relation, user)
     Relation.transaction do
 
   def update_from(new_relation, user)
     Relation.transaction do

-      self.lock!
+      lock!

       check_consistency(self, new_relation, user)
       unless new_relation.preconditions_ok?(members)
         fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
       check_consistency(self, new_relation, user)
       unless new_relation.preconditions_ok?(members)
         fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")


@@ -218,7 +215,7 @@ class Relation < ActiveRecord::Base
 
   def create_with_history(user)
     check_create_consistency(self, user)
 
   def create_with_history(user)
     check_create_consistency(self, user)

-    unless self.preconditions_ok?
+    unless preconditions_ok?

       fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
     end
     self.version = 0
       fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
     end
     self.version = 0


@@ -249,8 +246,10 @@ class Relation < ActiveRecord::Base
 
       # use reflection to look up the appropriate class
       model = Kernel.const_get(m[0].capitalize)
 
       # use reflection to look up the appropriate class
       model = Kernel.const_get(m[0].capitalize)

-      # get the element with that ID
-      element = model.where(:id => m[1]).first
+      # get the element with that ID. and, if found, lock the element to
+      # ensure it can't be deleted until after the current transaction
+      # commits.
+      element = model.lock("for share").find_by(:id => m[1])

 
       # and check that it is OK to use.
       unless element && element.visible? && element.preconditions_ok?
 
       # and check that it is OK to use.
       unless element && element.visible? && element.preconditions_ok?


@@ -290,7 +289,7 @@ class Relation < ActiveRecord::Base
       t = Time.now.getutc
       self.version += 1
       self.timestamp = t
       t = Time.now.getutc
       self.version += 1
       self.timestamp = t

-      self.save!
+      save!

 
       tags = self.tags.clone
       relation_tags.each do |old_tag|
 
       tags = self.tags.clone
       relation_tags.each do |old_tag|


@@ -313,7 +312,7 @@ class Relation < ActiveRecord::Base
       end
       # if there are left-over tags then they are new and will have to
       # be added.
       end
       # if there are left-over tags then they are new and will have to
       # be added.

-      tags_changed |= (!tags.empty?)
+      tags_changed |= !tags.empty?

       RelationTag.delete_all(:relation_id => id)
       self.tags.each do |k, v|
         tag = RelationTag.new
       RelationTag.delete_all(:relation_id => id)
       self.tags.each do |k, v|
         tag = RelationTag.new


@@ -371,7 +370,7 @@ class Relation < ActiveRecord::Base
       # materially change the rest of the relation.
       any_relations =
         changed_members.collect { |_id, type| type == "relation" }
       # materially change the rest of the relation.
       any_relations =
         changed_members.collect { |_id, type| type == "relation" }

-        .inject(false) { |a, e| a || e }
+                       .inject(false) { |a, e| a || e }

 
       update_members = if tags_changed || any_relations
                          # add all non-relation bounding boxes to the changeset
 
       update_members = if tags_changed || any_relations
                          # add all non-relation bounding boxes to the changeset