Fix most auto-correctable rubocop issues
authorTom Hughes <tom@compton.nu>
Mon, 16 Feb 2015 19:02:59 +0000 (19:02 +0000)
committerTom Hughes <tom@compton.nu>
Fri, 20 Feb 2015 08:56:16 +0000 (08:56 +0000)
206 files changed:
.rubocop.yml
.rubocop_todo.yml
app/controllers/amf_controller.rb
app/controllers/api_controller.rb
app/controllers/application_controller.rb
app/controllers/browse_controller.rb
app/controllers/changeset_controller.rb
app/controllers/diary_entry_controller.rb
app/controllers/export_controller.rb
app/controllers/geocoder_controller.rb
app/controllers/message_controller.rb
app/controllers/node_controller.rb
app/controllers/notes_controller.rb
app/controllers/oauth_clients_controller.rb
app/controllers/oauth_controller.rb
app/controllers/old_controller.rb
app/controllers/old_node_controller.rb
app/controllers/old_relation_controller.rb
app/controllers/old_way_controller.rb
app/controllers/redactions_controller.rb
app/controllers/relation_controller.rb
app/controllers/search_controller.rb
app/controllers/site_controller.rb
app/controllers/swf_controller.rb
app/controllers/trace_controller.rb
app/controllers/user_blocks_controller.rb
app/controllers/user_controller.rb
app/controllers/user_preference_controller.rb
app/controllers/user_roles_controller.rb
app/controllers/way_controller.rb
app/helpers/application_helper.rb
app/helpers/browse_helper.rb
app/helpers/geocoder_helper.rb
app/helpers/note_helper.rb
app/helpers/user_helper.rb
app/helpers/user_roles_helper.rb
app/models/access_token.rb
app/models/acl.rb
app/models/changeset.rb
app/models/changeset_comment.rb
app/models/client_application.rb
app/models/diary_comment.rb
app/models/diary_entry.rb
app/models/language.rb
app/models/message.rb
app/models/node.rb
app/models/note.rb
app/models/note_comment.rb
app/models/notifier.rb
app/models/oauth2_token.rb
app/models/oauth2_verifier.rb
app/models/oauth_token.rb
app/models/old_node.rb
app/models/old_relation.rb
app/models/old_way.rb
app/models/relation.rb
app/models/request_token.rb
app/models/trace.rb
app/models/tracepoint.rb
app/models/user.rb
app/models/user_block.rb
app/models/user_preference.rb
app/models/user_role.rb
app/models/user_token.rb
app/models/way.rb
config/application.rb
config/initializers/abstract_adapter.rb
config/initializers/field_error.rb
config/initializers/i18n.rb
config/initializers/libxml.rb
config/initializers/memory_limits.rb
config/initializers/paperclip.rb
config/initializers/piwik.rb
config/initializers/potlatch.rb
config/initializers/r2.rb
config/initializers/router.rb
config/initializers/sanitize.rb
config/initializers/streaming.rb
config/initializers/tempfile.rb
config/preinitializer.rb
config/routes.rb
db/migrate/001_create_osm_db.rb
db/migrate/002_cleanup_osm_db.rb
db/migrate/004_user_enhancements.rb
db/migrate/005_tile_tracepoints.rb
db/migrate/006_tile_nodes.rb
db/migrate/007_add_relations.rb
db/migrate/008_remove_segments.rb
db/migrate/010_diary_comments.rb
db/migrate/013_add_email_valid.rb
db/migrate/020_populate_node_tags_and_remove.rb
db/migrate/021_move_to_innodb.rb
db/migrate/022_key_constraints.rb
db/migrate/023_add_changesets.rb
db/migrate/028_add_more_changeset_indexes.rb
db/migrate/030_add_foreign_keys.rb
db/migrate/034_create_languages.rb
db/migrate/038_add_message_sender_index.rb
db/migrate/039_add_more_controls_to_gpx_files.rb
db/migrate/040_create_oauth_tables.rb
db/migrate/041_add_fine_o_auth_permissions.rb
db/migrate/044_create_user_roles.rb
db/migrate/051_add_status_to_user.rb
db/migrate/053_add_map_bug_tables.rb
db/migrate/057_add_map_bug_comment_event.rb
db/migrate/20120208122334_merge_acl_address_and_mask.rb
db/migrate/20120214210114_add_text_format.rb
lib/bounding_box.rb
lib/classic_pagination/pagination.rb
lib/classic_pagination/pagination_helper.rb
lib/consistency_validations.rb
lib/country.rb
lib/daemons/gpx_import.rb
lib/daemons/gpx_import_ctl
lib/diff_reader.rb
lib/editors.rb
lib/geo_record.rb
lib/gpx.rb
lib/id.rb
lib/migrate.rb
lib/nominatim.rb
lib/not_redactable.rb
lib/osm.rb
lib/output_compression/output_compression.rb
lib/password_hash.rb
lib/potlatch.rb
lib/quad_tile.rb
lib/quova.rb
lib/redactable.rb
lib/rich_text.rb
lib/short_link.rb
lib/tasks/add_version_to_nodes.rake
lib/utf8.rb
lib/validators.rb
script/daemons
script/deliver-message
script/locale/po2yaml
script/locale/yaml2po
script/statistics
script/update-spam-blocks
test/controllers/amf_controller_test.rb
test/controllers/api_controller_test.rb
test/controllers/browse_controller_test.rb
test/controllers/changeset_controller_test.rb
test/controllers/diary_entry_controller_test.rb
test/controllers/directions_controller_test.rb
test/controllers/export_controller_test.rb
test/controllers/geocoder_controller_test.rb
test/controllers/message_controller_test.rb
test/controllers/node_controller_test.rb
test/controllers/notes_controller_test.rb
test/controllers/oauth_clients_controller_test.rb
test/controllers/old_node_controller_test.rb
test/controllers/old_relation_controller_test.rb
test/controllers/old_way_controller_test.rb
test/controllers/relation_controller_test.rb
test/controllers/site_controller_test.rb
test/controllers/trace_controller_test.rb
test/controllers/user_blocks_controller_test.rb
test/controllers/user_controller_test.rb
test/controllers/user_roles_controller_test.rb
test/controllers/way_controller_test.rb
test/helpers/asset_helper_test.rb
test/helpers/browse_helper_test.rb
test/integration/client_application_test.rb
test/integration/oauth_test.rb
test/integration/short_links_test.rb
test/integration/user_blocks_test.rb
test/integration/user_changeset_comments_test.rb
test/integration/user_creation_test.rb
test/integration/user_diaries_test.rb
test/integration/user_login_test.rb
test/integration/user_roles_test.rb
test/integration/user_terms_seen_test.rb
test/lib/bounding_box_test.rb
test/lib/i18n_test.rb
test/lib/rich_text_test.rb
test/lib/short_link_test.rb
test/models/changeset_tag_test.rb
test/models/changeset_test.rb
test/models/diary_entry_test.rb
test/models/friend_test.rb
test/models/message_test.rb
test/models/node_tag_test.rb
test/models/node_test.rb
test/models/note_comment_test.rb
test/models/note_test.rb
test/models/oauth_nonce_test.rb
test/models/oauth_token_test.rb
test/models/old_node_tag_test.rb
test/models/old_node_test.rb
test/models/old_relation_tag_test.rb
test/models/old_way_tag_test.rb
test/models/old_way_test.rb
test/models/redaction_test.rb
test/models/relation_tag_test.rb
test/models/relation_test.rb
test/models/trace_test.rb
test/models/tracepoint_test.rb
test/models/tracetag_test.rb
test/models/user_preference_test.rb
test/models/user_test.rb
test/models/user_token_test.rb
test/models/way_tag_test.rb
test/models/way_test.rb
test/test_helper.rb

index cc32da4..8187b26 100644 (file)
@@ -1 +1,4 @@
 inherit_from: .rubocop_todo.yml
+
+Style/BracesAroundHashParameters:
+  EnforcedStyle: context_dependent
index 0c09cb1..75f088e 100644 (file)
@@ -1,5 +1,5 @@
 # This configuration was generated by `rubocop --auto-gen-config`
-# on 2015-02-16 18:44:13 +0000 using RuboCop version 0.29.1.
+# on 2015-02-16 19:20:52 +0000 using RuboCop version 0.29.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -18,21 +18,11 @@ Lint/AmbiguousRegexpLiteral:
 Lint/AssignmentInCondition:
   Enabled: false
 
-# Offense count: 4
-# Cop supports --auto-correct.
-Lint/BlockAlignment:
-  Enabled: false
-
 # Offense count: 1
 # Configuration parameters: AlignWith, SupportedStyles.
 Lint/DefEndAlignment:
   Enabled: false
 
-# Offense count: 4
-# Cop supports --auto-correct.
-Lint/DeprecatedClassMethods:
-  Enabled: false
-
 # Offense count: 2
 Lint/DuplicateMethods:
   Enabled: false
@@ -62,26 +52,6 @@ Lint/RescueException:
 Lint/ShadowingOuterLocalVariable:
   Enabled: false
 
-# Offense count: 9
-# Cop supports --auto-correct.
-Lint/SpaceBeforeFirstArg:
-  Enabled: false
-
-# Offense count: 21
-# Cop supports --auto-correct.
-Lint/StringConversionInInterpolation:
-  Enabled: false
-
-# Offense count: 17
-# Cop supports --auto-correct.
-Lint/UnusedBlockArgument:
-  Enabled: false
-
-# Offense count: 16
-# Cop supports --auto-correct.
-Lint/UnusedMethodArgument:
-  Enabled: false
-
 # Offense count: 2
 Lint/UselessAccessModifier:
   Enabled: false
@@ -111,10 +81,10 @@ Metrics/ClassLength:
 Metrics/CyclomaticComplexity:
   Max: 21
 
-# Offense count: 2076
+# Offense count: 2112
 # Configuration parameters: AllowURI, URISchemes.
 Metrics/LineLength:
-  Max: 253
+  Max: 520
 
 # Offense count: 520
 # Configuration parameters: CountComments.
@@ -130,59 +100,14 @@ Metrics/ParameterLists:
 Metrics/PerceivedComplexity:
   Max: 24
 
-# Offense count: 46
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/AccessModifierIndentation:
-  Enabled: false
-
 # Offense count: 5
 Style/AccessorMethodName:
   Enabled: false
 
-# Offense count: 6
-# Cop supports --auto-correct.
-Style/Alias:
-  Enabled: false
-
-# Offense count: 9
-# Cop supports --auto-correct.
-Style/AlignArray:
-  Enabled: false
-
-# Offense count: 6
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle, SupportedLastArgumentHashStyles.
-Style/AlignHash:
-  Enabled: false
-
-# Offense count: 159
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/AlignParameters:
-  Enabled: false
-
-# Offense count: 217
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/AndOr:
-  Enabled: false
-
 # Offense count: 1
 Style/AsciiComments:
   Enabled: false
 
-# Offense count: 87
-# Cop supports --auto-correct.
-Style/Blocks:
-  Enabled: false
-
-# Offense count: 636
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/BracesAroundHashParameters:
-  Enabled: false
-
 # Offense count: 21
 # Configuration parameters: IndentWhenRelativeTo, SupportedStyles, IndentOneStep.
 Style/CaseIndentation:
@@ -193,118 +118,36 @@ Style/CaseIndentation:
 Style/ClassAndModuleChildren:
   Enabled: false
 
-# Offense count: 3
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/ClassCheck:
-  Enabled: false
-
 # Offense count: 9
 Style/ClassVars:
   Enabled: false
 
-# Offense count: 12
-# Cop supports --auto-correct.
-Style/ColonMethodCall:
-  Enabled: false
-
 # Offense count: 12
 # Configuration parameters: Keywords.
 Style/CommentAnnotation:
   Enabled: false
 
-# Offense count: 25
-# Cop supports --auto-correct.
-Style/CommentIndentation:
-  Enabled: false
-
 # Offense count: 9
 Style/ConstantName:
   Enabled: false
 
-# Offense count: 17
-# Cop supports --auto-correct.
-Style/DeprecatedHashMethods:
-  Enabled: false
-
 # Offense count: 306
 Style/Documentation:
   Enabled: false
 
-# Offense count: 14
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/DotPosition:
-  Enabled: false
-
 # Offense count: 2
 Style/EachWithObject:
   Enabled: false
 
-# Offense count: 5
-# Cop supports --auto-correct.
-Style/ElseAlignment:
-  Enabled: false
-
 # Offense count: 3
 Style/EmptyElse:
   Enabled: false
 
-# Offense count: 8
-# Cop supports --auto-correct.
-# Configuration parameters: AllowAdjacentOneLineDefs.
-Style/EmptyLineBetweenDefs:
-  Enabled: false
-
-# Offense count: 16
-# Cop supports --auto-correct.
-Style/EmptyLines:
-  Enabled: false
-
-# Offense count: 15
-# Cop supports --auto-correct.
-Style/EmptyLinesAroundAccessModifier:
-  Enabled: false
-
-# Offense count: 9
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/EmptyLinesAroundBlockBody:
-  Enabled: false
-
-# Offense count: 28
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/EmptyLinesAroundClassBody:
-  Enabled: false
-
-# Offense count: 8
-# Cop supports --auto-correct.
-Style/EmptyLinesAroundMethodBody:
-  Enabled: false
-
-# Offense count: 8
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/EmptyLinesAroundModuleBody:
-  Enabled: false
-
-# Offense count: 45
-# Cop supports --auto-correct.
-Style/EmptyLiteral:
-  Enabled: false
-
 # Offense count: 3
 # Configuration parameters: Exclude.
 Style/FileName:
   Enabled: false
 
-# Offense count: 4
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/FirstParameterIndentation:
-  Enabled: false
-
 # Offense count: 3
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 Style/For:
@@ -331,79 +174,25 @@ Style/GuardClause:
 Style/HashSyntax:
   Enabled: false
 
-# Offense count: 42
+# Offense count: 41
 # Configuration parameters: MaxLineLength.
 Style/IfUnlessModifier:
   Enabled: false
 
-# Offense count: 18
-# Cop supports --auto-correct.
-Style/IndentArray:
-  Enabled: false
-
-# Offense count: 15
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/IndentHash:
-  Enabled: false
-
-# Offense count: 22
-# Cop supports --auto-correct.
-Style/IndentationConsistency:
-  Enabled: false
-
-# Offense count: 40
-# Cop supports --auto-correct.
-# Configuration parameters: Width.
-Style/IndentationWidth:
-  Enabled: false
-
-# Offense count: 67
-# Cop supports --auto-correct.
-Style/LeadingCommentSpace:
-  Enabled: false
-
-# Offense count: 74
+# Offense count: 60
 # Cop supports --auto-correct.
 Style/LineEndConcatenation:
   Enabled: false
 
-# Offense count: 17
-# Cop supports --auto-correct.
-Style/MethodCallParentheses:
-  Enabled: false
-
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/MethodDefParentheses:
-  Enabled: false
-
 # Offense count: 12
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 Style/MethodName:
   Enabled: false
 
-# Offense count: 28
-# Cop supports --auto-correct.
-Style/MultilineIfThen:
-  Enabled: false
-
-# Offense count: 34
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/MultilineOperationIndentation:
-  Enabled: false
-
 # Offense count: 3
 Style/MultilineTernaryOperator:
   Enabled: false
 
-# Offense count: 19
-# Cop supports --auto-correct.
-Style/NegatedIf:
-  Enabled: false
-
 # Offense count: 1
 Style/NestedTernaryOperator:
   Enabled: false
@@ -413,17 +202,6 @@ Style/NestedTernaryOperator:
 Style/Next:
   Enabled: false
 
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: IncludeSemanticChanges.
-Style/NonNilCheck:
-  Enabled: false
-
-# Offense count: 32
-# Cop supports --auto-correct.
-Style/Not:
-  Enabled: false
-
 # Offense count: 53
 # Cop supports --auto-correct.
 Style/NumericLiterals:
@@ -437,18 +215,6 @@ Style/OneLineConditional:
 Style/OpMethod:
   Enabled: false
 
-# Offense count: 7
-# Cop supports --auto-correct.
-# Configuration parameters: AllowSafeAssignment.
-Style/ParenthesesAroundCondition:
-  Enabled: false
-
-# Offense count: 21
-# Cop supports --auto-correct.
-# Configuration parameters: PreferredDelimiters.
-Style/PercentLiteralDelimiters:
-  Enabled: false
-
 # Offense count: 44
 # Cop supports --auto-correct.
 Style/PerlBackrefs:
@@ -459,32 +225,11 @@ Style/PerlBackrefs:
 Style/PredicateName:
   Enabled: false
 
-# Offense count: 8
-# Cop supports --auto-correct.
-Style/Proc:
-  Enabled: false
-
 # Offense count: 95
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 Style/RaiseArgs:
   Enabled: false
 
-# Offense count: 11
-# Cop supports --auto-correct.
-Style/RedundantBegin:
-  Enabled: false
-
-# Offense count: 113
-# Cop supports --auto-correct.
-# Configuration parameters: AllowMultipleReturnValues.
-Style/RedundantReturn:
-  Enabled: false
-
-# Offense count: 200
-# Cop supports --auto-correct.
-Style/RedundantSelf:
-  Enabled: false
-
 # Offense count: 11
 Style/RegexpLiteral:
   MaxSlashes: 5
@@ -493,112 +238,16 @@ Style/RegexpLiteral:
 Style/RescueModifier:
   Enabled: false
 
-# Offense count: 7
-# Cop supports --auto-correct.
-Style/SelfAssignment:
-  Enabled: false
-
-# Offense count: 60
-# Cop supports --auto-correct.
+# Offense count: 25
 # Configuration parameters: AllowAsExpressionSeparator.
 Style/Semicolon:
   Enabled: false
 
-# Offense count: 156
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/SignalException:
-  Enabled: false
-
 # Offense count: 4
 # Configuration parameters: Methods.
 Style/SingleLineBlockParams:
   Enabled: false
 
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/SingleSpaceBeforeFirstArg:
-  Enabled: false
-
-# Offense count: 2
-# Cop supports --auto-correct.
-Style/SpaceAfterColon:
-  Enabled: false
-
-# Offense count: 409
-# Cop supports --auto-correct.
-Style/SpaceAfterComma:
-  Enabled: false
-
-# Offense count: 1
-# Cop supports --auto-correct.
-Style/SpaceAfterControlKeyword:
-  Enabled: false
-
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/SpaceAfterMethodName:
-  Enabled: false
-
-# Offense count: 34
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/SpaceAroundEqualsInParameterDefault:
-  Enabled: false
-
-# Offense count: 493
-# Cop supports --auto-correct.
-Style/SpaceAroundOperators:
-  Enabled: false
-
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/SpaceBeforeBlockBraces:
-  Enabled: false
-
-# Offense count: 13
-# Cop supports --auto-correct.
-Style/SpaceBeforeComma:
-  Enabled: false
-
-# Offense count: 2
-# Cop supports --auto-correct.
-Style/SpaceBeforeComment:
-  Enabled: false
-
-# Offense count: 76
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters.
-Style/SpaceInsideBlockBraces:
-  Enabled: false
-
-# Offense count: 133
-# Cop supports --auto-correct.
-Style/SpaceInsideBrackets:
-  Enabled: false
-
-# Offense count: 778
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces, SupportedStyles.
-Style/SpaceInsideHashLiteralBraces:
-  Enabled: false
-
-# Offense count: 9
-# Cop supports --auto-correct.
-Style/SpaceInsideParens:
-  Enabled: false
-
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/SpaceInsideRangeLiteral:
-  Enabled: false
-
-# Offense count: 2
-# Cop supports --auto-correct.
-Style/SpecialGlobalVars:
-  Enabled: false
-
 # Offense count: 6639
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
@@ -615,25 +264,7 @@ Style/StringLiteralsInInterpolation:
 Style/StructInheritance:
   Enabled: false
 
-# Offense count: 38
-# Cop supports --auto-correct.
-# Configuration parameters: IgnoredMethods.
-Style/SymbolProc:
-  Enabled: false
-
-# Offense count: 218
-# Cop supports --auto-correct.
-Style/Tab:
-  Enabled: false
-
-# Offense count: 4
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyleForMultiline, SupportedStyles.
-Style/TrailingComma:
-  Enabled: false
-
-# Offense count: 13
-# Cop supports --auto-correct.
+# Offense count: 3
 # Configuration parameters: ExactNameMatch, AllowPredicates, AllowDSLWriters, Whitelist.
 Style/TrivialAccessors:
   Enabled: false
@@ -647,23 +278,7 @@ Style/UnlessElse:
 Style/VariableName:
   Enabled: false
 
-# Offense count: 27
-# Cop supports --auto-correct.
-Style/WhenThen:
-  Enabled: false
-
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/WhileUntilDo:
-  Enabled: false
-
 # Offense count: 1
 # Configuration parameters: MaxLineLength.
 Style/WhileUntilModifier:
   Enabled: false
-
-# Offense count: 85
-# Cop supports --auto-correct.
-# Configuration parameters: WordRegex.
-Style/WordArray:
-  MinSize: 14
index 19b7d5d..75dc164 100644 (file)
@@ -47,21 +47,21 @@ class AmfController < ApplicationController
   def amf_read
     self.status = :ok
     self.content_type = Mime::AMF
-    self.response_body = Dispatcher.new(request.raw_post) do |message,*args|
+    self.response_body = Dispatcher.new(request.raw_post) do |message, *args|
       logger.info("Executing AMF #{message}(#{args.join(',')})")
 
       case message
-        when 'getpresets';        result = getpresets(*args)
-        when 'whichways';         result = whichways(*args)
-        when 'whichways_deleted'; result = whichways_deleted(*args)
-        when 'getway';            result = getway(args[0].to_i)
-        when 'getrelation';       result = getrelation(args[0].to_i)
-        when 'getway_old';        result = getway_old(args[0].to_i,args[1])
-        when 'getway_history';    result = getway_history(args[0].to_i)
-        when 'getnode_history';   result = getnode_history(args[0].to_i)
-        when 'findgpx';           result = findgpx(*args)
-        when 'findrelations';     result = findrelations(*args)
-        when 'getpoi';            result = getpoi(*args)
+        when 'getpresets' then        result = getpresets(*args)
+        when 'whichways' then         result = whichways(*args)
+        when 'whichways_deleted' then result = whichways_deleted(*args)
+        when 'getway' then            result = getway(args[0].to_i)
+        when 'getrelation' then       result = getrelation(args[0].to_i)
+        when 'getway_old' then        result = getway_old(args[0].to_i, args[1])
+        when 'getway_history' then    result = getway_history(args[0].to_i)
+        when 'getnode_history' then   result = getnode_history(args[0].to_i)
+        when 'findgpx' then           result = findgpx(*args)
+        when 'findrelations' then     result = findrelations(*args)
+        when 'getpoi' then            result = getpoi(*args)
       end
 
       result
@@ -75,22 +75,22 @@ class AmfController < ApplicationController
 
     self.status = :ok
     self.content_type = Mime::AMF
-    self.response_body = Dispatcher.new(request.raw_post) do |message,*args|
+    self.response_body = Dispatcher.new(request.raw_post) do |message, *args|
       logger.info("Executing AMF #{message}")
 
       if err
         result = [-5, nil]
       else
         case message
-          when 'putway';         orn = renumberednodes.dup
-                                 result = putway(renumberednodes, *args)
-                                 result[4] = renumberednodes.reject { |k,v| orn.has_key?(k) }
-                                 if result[0] == 0 and result[2] != result[3] then renumberedways[result[2]] = result[3] end
-          when 'putrelation';    result = putrelation(renumberednodes, renumberedways, *args)
-          when 'deleteway';      result = deleteway(*args)
-          when 'putpoi';         result = putpoi(*args)
-                                 if result[0] == 0 and result[2] != result[3] then renumberednodes[result[2]] = result[3] end
-          when 'startchangeset'; result = startchangeset(*args)
+          when 'putway' then         orn = renumberednodes.dup
+                                     result = putway(renumberednodes, *args)
+                                     result[4] = renumberednodes.reject { |k, _v| orn.key?(k) }
+                                     if result[0] == 0 && result[2] != result[3] then renumberedways[result[2]] = result[3] end
+          when 'putrelation' then    result = putrelation(renumberednodes, renumberedways, *args)
+          when 'deleteway' then      result = deleteway(*args)
+          when 'putpoi' then         result = putpoi(*args)
+                                     if result[0] == 0 && result[2] != result[3] then renumberednodes[result[2]] = result[3] end
+          when 'startchangeset' then result = startchangeset(*args)
         end
 
         err = true if result[0] == -3  # If a conflict is detected, don't execute any more writes
@@ -102,7 +102,7 @@ class AmfController < ApplicationController
 
   private
 
-  def amf_handle_error(call,rootobj,rootid)
+  def amf_handle_error(call, rootobj, rootid)
     yield
   rescue OSM::APIAlreadyDeletedError => ex
     return [-4, ex.object, ex.object_id]
@@ -111,15 +111,15 @@ class AmfController < ApplicationController
   rescue OSM::APIUserChangesetMismatchError => ex
     return [-2, ex.to_s]
   rescue OSM::APIBadBoundingBox => ex
-    return [-2, "Sorry - I can't get the map for that area. The server said: #{ex.to_s}"]
+    return [-2, "Sorry - I can't get the map for that area. The server said: #{ex}"]
   rescue OSM::APIError => ex
     return [-1, ex.to_s]
   rescue Exception => ex
-    return [-2, "An unusual error happened (in #{call}). The server said: #{ex.to_s}"]
+    return [-2, "An unusual error happened (in #{call}). The server said: #{ex}"]
   end
 
-  def amf_handle_error_with_timeout(call,rootobj,rootid)
-    amf_handle_error(call,rootobj,rootid) do
+  def amf_handle_error_with_timeout(call, rootobj, rootid)
+    amf_handle_error(call, rootobj, rootid) do
       OSM::Timer.timeout(API_TIMEOUT, OSM::APITimeoutError) do
         yield
       end
@@ -130,14 +130,14 @@ class AmfController < ApplicationController
   # Returns success_code,success_message,changeset id
 
   def startchangeset(usertoken, cstags, closeid, closecomment, opennew)
-    amf_handle_error("'startchangeset'",nil,nil) do
+    amf_handle_error("'startchangeset'", nil, nil) do
       user = getuser(usertoken)
-      if !user then return -1,"You are not logged in, so Potlatch can't write any changes to the database." end
-      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
-      if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
+      unless user then return -1, "You are not logged in, so Potlatch can't write any changes to the database." end
+      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
+      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
 
       if cstags
-        if !tags_ok(cstags) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+        unless tags_ok(cstags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
         cstags = strip_non_xml_chars cstags
       end
 
@@ -145,12 +145,12 @@ class AmfController < ApplicationController
       if closeid
         cs = Changeset.find(closeid.to_i)
         cs.set_closed_time_now
-        if cs.user_id!=user.id
-          raise OSM::APIUserChangesetMismatchError.new
+        if cs.user_id != user.id
+          fail OSM::APIUserChangesetMismatchError.new
         elsif closecomment.empty?
           cs.save!
         else
-          cs.tags['comment']=closecomment
+          cs.tags['comment'] = closecomment
           # in case closecomment has chars not allowed in xml
           cs.tags = strip_non_xml_chars cs.tags
           cs.save_with_tags!
@@ -158,12 +158,12 @@ class AmfController < ApplicationController
       end
 
       # open a new changeset
-      if opennew!=0
+      if opennew != 0
         cs = Changeset.new
         cs.tags = cstags
         cs.user_id = user.id
-        if !closecomment.empty?
-          cs.tags['comment']=closecomment
+        unless closecomment.empty?
+          cs.tags['comment'] = closecomment
           # in case closecomment has chars not allowed in xml
           cs.tags = strip_non_xml_chars cs.tags
         end
@@ -171,9 +171,9 @@ class AmfController < ApplicationController
         cs.created_at = Time.now.getutc
         cs.closed_at = cs.created_at + Changeset::IDLE_TIMEOUT
         cs.save_with_tags!
-        return [0,'',cs.id]
+        return [0, '', cs.id]
       else
-        return [0,'',nil]
+        return [0, '', nil]
       end
     end
   end
@@ -181,7 +181,7 @@ class AmfController < ApplicationController
   # Return presets (default tags, localisation etc.):
   # uses POTLATCH_PRESETS global, set up in OSM::Potlatch.
 
-  def getpresets(usertoken,lang) #:doc:
+  def getpresets(usertoken, lang) #:doc:
     user = getuser(usertoken)
 
     if user && !user.languages.empty?
@@ -200,12 +200,12 @@ class AmfController < ApplicationController
     localised.delete("help_html")
 
     # Populate icon names
-    POTLATCH_PRESETS[10].each { |id|
+    POTLATCH_PRESETS[10].each do |id|
       POTLATCH_PRESETS[11][id] = localised["preset_icon_#{id}"]
       localised.delete("preset_icon_#{id}")
-    }
+    end
 
-    return POTLATCH_PRESETS+[localised,help]
+    POTLATCH_PRESETS + [localised, help]
   end
 
   def getlocalized(lang)
@@ -213,14 +213,14 @@ class AmfController < ApplicationController
     loaded_lang = 'en'
 
     # Load English defaults
-    en = YAML::load(File.open("#{Rails.root}/config/potlatch/locales/en.yml"))["en"]
+    en = YAML.load(File.open("#{Rails.root}/config/potlatch/locales/en.yml"))["en"]
 
     if lang == 'en'
       return [loaded_lang, en]
     else
       # Use English as a fallback
       begin
-        other = YAML::load(File.open("#{Rails.root}/config/potlatch/locales/#{lang}.yml"))[lang]
+        other = YAML.load(File.open("#{Rails.root}/config/potlatch/locales/#{lang}.yml"))[lang]
         loaded_lang = lang
       rescue
         other = en
@@ -247,8 +247,8 @@ class AmfController < ApplicationController
   # used in any way, rel is any relation which refers to either a way
   # or node that we're returning.
   def whichways(xmin, ymin, xmax, ymax) #:doc:
-    amf_handle_error_with_timeout("'whichways'",nil,nil) do
-      enlarge = [(xmax-xmin)/8,0.01].min
+    amf_handle_error_with_timeout("'whichways'", nil, nil) do
+      enlarge = [(xmax - xmin) / 8, 0.01].min
       xmin -= enlarge; ymin -= enlarge
       xmax += enlarge; ymax += enlarge
 
@@ -258,17 +258,17 @@ class AmfController < ApplicationController
       bbox.check_boundaries
       bbox.check_size
 
-      if POTLATCH_USE_SQL then
+      if POTLATCH_USE_SQL
         ways = sql_find_ways_in_area(bbox)
         points = sql_find_pois_in_area(bbox)
-        relations = sql_find_relations_in_area_and_ways(bbox, ways.collect {|x| x[0]})
+        relations = sql_find_relations_in_area_and_ways(bbox, ways.collect { |x| x[0] })
       else
         # find the way ids in an area
         nodes_in_area = Node.bbox(bbox).visible.includes(:ways)
-        ways = nodes_in_area.inject([]) { |sum, node|
-          visible_ways = node.ways.select { |w| w.visible? }
-          sum + visible_ways.collect { |w| [w.id,w.version] }
-        }.uniq
+        ways = nodes_in_area.inject([]) do |sum, node|
+          visible_ways = node.ways.select(&:visible?)
+          sum + visible_ways.collect { |w| [w.id, w.version] }
+        end.uniq
         ways.delete([])
 
         # find the node ids in an area that aren't part of ways
@@ -276,9 +276,9 @@ class AmfController < ApplicationController
         points = nodes_not_used_in_area.collect { |n| [n.id, n.lon, n.lat, n.tags, n.version] }.uniq
 
         # find the relations used by those nodes and ways
-        relations = Relation.nodes(nodes_in_area.collect { |n| n.id }).visible +
+        relations = Relation.nodes(nodes_in_area.collect(&:id)).visible +
                     Relation.ways(ways.collect { |w| w[0] }).visible
-        relations = relations.collect { |relation| [relation.id,relation.version] }.uniq
+        relations = relations.collect { |relation| [relation.id, relation.version] }.uniq
       end
 
       [0, '', ways, points, relations]
@@ -289,8 +289,8 @@ class AmfController < ApplicationController
   # with a deleted node only - not POIs or relations).
 
   def whichways_deleted(xmin, ymin, xmax, ymax) #:doc:
-    amf_handle_error_with_timeout("'whichways_deleted'",nil,nil) do
-      enlarge = [(xmax-xmin)/8,0.01].min
+    amf_handle_error_with_timeout("'whichways_deleted'", nil, nil) do
+      enlarge = [(xmax - xmin) / 8, 0.01].min
       xmin -= enlarge; ymin -= enlarge
       xmax += enlarge; ymax += enlarge
 
@@ -301,9 +301,9 @@ class AmfController < ApplicationController
       bbox.check_size
 
       nodes_in_area = Node.bbox(bbox).joins(:ways_via_history).where(:current_ways => { :visible => false })
-      way_ids = nodes_in_area.collect { |node| node.ways_via_history.invisible.collect { |way| way.id } }.flatten.uniq
+      way_ids = nodes_in_area.collect { |node| node.ways_via_history.invisible.collect(&:id) }.flatten.uniq
 
-      [0,'',way_ids]
+      [0, '', way_ids]
     end
   end
 
@@ -311,8 +311,8 @@ class AmfController < ApplicationController
   # Returns the way id, a Potlatch-style array of points, a hash of tags, the version number, and the user ID.
 
   def getway(wayid) #:doc:
-    amf_handle_error_with_timeout("'getway' #{wayid}" ,'way',wayid) do
-      if POTLATCH_USE_SQL then
+    amf_handle_error_with_timeout("'getway' #{wayid}", 'way', wayid) do
+      if POTLATCH_USE_SQL
         points = sql_get_nodes_in_way(wayid)
         tags = sql_get_tags_in_way(wayid)
         version = sql_get_way_version(wayid)
@@ -324,10 +324,10 @@ class AmfController < ApplicationController
         way = Way.where(:id => wayid).first
 
         # check case where way has been deleted or doesn't exist
-        return [-4, 'way', wayid] if way.nil? or !way.visible
+        return [-4, 'way', wayid] if way.nil? || !way.visible
 
         points = way.nodes.preload(:node_tags).collect do |node|
-          nodetags=node.tags
+          nodetags = node.tags
           nodetags.delete('created_by')
           [node.lon, node.lat, node.id, nodetags, node.version]
         end
@@ -356,7 +356,7 @@ class AmfController < ApplicationController
   # 5. is this the current, visible version? (boolean)
 
   def getway_old(id, timestamp) #:doc:
-    amf_handle_error_with_timeout("'getway_old' #{id}, #{timestamp}", 'way',id) do
+    amf_handle_error_with_timeout("'getway_old' #{id}, #{timestamp}", 'way', id) do
       if timestamp == ''
         # undelete
         old_way = OldWay.where(:visible => true, :way_id => id).unredacted.order("version DESC").first
@@ -368,7 +368,7 @@ class AmfController < ApplicationController
           old_way = OldWay.where("way_id = ? AND timestamp <= ?", id, timestamp).unredacted.order("timestamp DESC").first
           unless old_way.nil?
             points = old_way.get_nodes_revert(timestamp)
-            if !old_way.visible
+            unless old_way.visible
               return [-1, "Sorry, the way was deleted at that time - please revert to a previous version.", id]
             end
           end
@@ -381,9 +381,9 @@ class AmfController < ApplicationController
       if old_way.nil?
         return [-1, "Sorry, the server could not find a way at that time.", id]
       else
-        curway=Way.find(id)
+        curway = Way.find(id)
         old_way.tags['history'] = "Retrieved from v#{old_way.version}"
-        return [0, '', id, points, old_way.tags, curway.version, (curway.version==old_way.version and curway.visible)]
+        return [0, '', id, points, old_way.tags, curway.version, (curway.version == old_way.version && curway.visible)]
       end
     end
   end
@@ -399,70 +399,65 @@ class AmfController < ApplicationController
   # start date of the way.
 
   def getway_history(wayid) #:doc:
-    begin
-      # Find list of revision dates for way and all constituent nodes
-      revdates=[]
-      revusers={}
-      Way.find(wayid).old_ways.unredacted.collect do |a|
-        revdates.push(a.timestamp)
-        unless revusers.has_key?(a.timestamp.to_i) then revusers[a.timestamp.to_i]=change_user(a) end
-        a.nds.each do |n|
-          Node.find(n).old_nodes.unredacted.collect do |o|
-            revdates.push(o.timestamp)
-            unless revusers.has_key?(o.timestamp.to_i) then revusers[o.timestamp.to_i]=change_user(o) end
-          end
+    revdates = []
+    revusers = {}
+    Way.find(wayid).old_ways.unredacted.collect do |a|
+      revdates.push(a.timestamp)
+      unless revusers.key?(a.timestamp.to_i) then revusers[a.timestamp.to_i] = change_user(a) end
+      a.nds.each do |n|
+        Node.find(n).old_nodes.unredacted.collect do |o|
+          revdates.push(o.timestamp)
+          unless revusers.key?(o.timestamp.to_i) then revusers[o.timestamp.to_i] = change_user(o) end
         end
       end
-      waycreated=revdates[0]
-      revdates.uniq!
-      revdates.sort!
-      revdates.reverse!
-
-      # Remove any dates (from nodes) before first revision date of way
-      revdates.delete_if { |d| d<waycreated }
-      # Remove any elements where 2 seconds doesn't elapse before next one
-      revdates.delete_if { |d| revdates.include?(d+1) or revdates.include?(d+2) }
-      # Collect all in one nested array
-      revdates.collect! {|d| [(d + 1).strftime("%d %b %Y, %H:%M:%S")] + revusers[d.to_i] }
-      revdates.uniq!
-
-      return ['way', wayid, revdates]
-    rescue ActiveRecord::RecordNotFound
-      return ['way', wayid, []]
     end
+    waycreated = revdates[0]
+    revdates.uniq!
+    revdates.sort!
+    revdates.reverse!
+
+    # Remove any dates (from nodes) before first revision date of way
+    revdates.delete_if { |d| d < waycreated }
+    # Remove any elements where 2 seconds doesn't elapse before next one
+    revdates.delete_if { |d| revdates.include?(d + 1) || revdates.include?(d + 2) }
+    # Collect all in one nested array
+    revdates.collect! { |d| [(d + 1).strftime("%d %b %Y, %H:%M:%S")] + revusers[d.to_i] }
+    revdates.uniq!
+
+    return ['way', wayid, revdates]
+  rescue ActiveRecord::RecordNotFound
+    return ['way', wayid, []]
   end
 
   # Find history of a node. Returns 'node', id, and an array of previous versions as above.
 
   def getnode_history(nodeid) #:doc:
-    begin
-      history = Node.find(nodeid).old_nodes.unredacted.reverse.collect do |old_node|
-        [(old_node.timestamp + 1).strftime("%d %b %Y, %H:%M:%S")] + change_user(old_node)
-      end
-      return ['node', nodeid, history]
-    rescue ActiveRecord::RecordNotFound
-      return ['node', nodeid, []]
+    history = Node.find(nodeid).old_nodes.unredacted.reverse.collect do |old_node|
+      [(old_node.timestamp + 1).strftime("%d %b %Y, %H:%M:%S")] + change_user(old_node)
     end
+    return ['node', nodeid, history]
+  rescue ActiveRecord::RecordNotFound
+    return ['node', nodeid, []]
   end
 
   def change_user(obj)
     user_object = obj.changeset.user
     user = user_object.data_public? ? user_object.display_name : 'anonymous'
     uid  = user_object.data_public? ? user_object.id : 0
-    [user,uid]
+    [user, uid]
   end
 
   # Find GPS traces with specified name/id.
   # Returns array listing GPXs, each one comprising id, name and description.
 
   def findgpx(searchterm, usertoken)
-    amf_handle_error_with_timeout("'findgpx'" ,nil,nil) do
+    amf_handle_error_with_timeout("'findgpx'", nil, nil) do
       user = getuser(usertoken)
-      if !user then return -1,"You must be logged in to search for GPX traces." end
-      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
+      unless user then return -1, "You must be logged in to search for GPX traces." end
+      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
 
       query = Trace.visible_to(user)
-      if searchterm.to_i > 0 then
+      if searchterm.to_i > 0
         query = query.where(:id => searchterm.to_i)
       else
         query = query.where("MATCH(name) AGAINST (?)", searchterm).limit(21)
@@ -470,7 +465,7 @@ class AmfController < ApplicationController
       gpxs = query.collect do |gpx|
         [gpx.id, gpx.name, gpx.description]
       end
-      [0,'',gpxs]
+      [0, '', gpxs]
     end
   end
 
@@ -484,10 +479,10 @@ class AmfController < ApplicationController
   # 5. version.
 
   def getrelation(relid) #:doc:
-    amf_handle_error("'getrelation' #{relid}" ,'relation',relid) do
+    amf_handle_error("'getrelation' #{relid}", 'relation', relid) do
       rel = Relation.where(:id => relid).first
 
-      return [-4, 'relation', relid] if rel.nil? or !rel.visible
+      return [-4, 'relation', relid] if rel.nil? || !rel.visible
       [0, '', relid, rel.tags, rel.members, rel.version]
     end
   end
@@ -497,14 +492,14 @@ class AmfController < ApplicationController
 
   def findrelations(searchterm)
     rels = []
-    if searchterm.to_i>0 then
+    if searchterm.to_i > 0
       rel = Relation.where(:id => searchterm.to_i).first
-      if rel and rel.visible then
+      if rel && rel.visible
         rels.push([rel.id, rel.tags, rel.members, rel.version])
       end
     else
       RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t|
-        if t.relation.visible then
+        if t.relation.visible
           rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version])
         end
       end
@@ -520,13 +515,13 @@ class AmfController < ApplicationController
   # 3. version.
 
   def putrelation(renumberednodes, renumberedways, usertoken, changeset_id, version, relid, tags, members, visible) #:doc:
-    amf_handle_error("'putrelation' #{relid}" ,'relation',relid)  do
+    amf_handle_error("'putrelation' #{relid}", 'relation', relid)  do
       user = getuser(usertoken)
-      if !user then return -1,"You are not logged in, so the relation could not be saved." end
-      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
-      if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
+      unless user then return -1, "You are not logged in, so the relation could not be saved." end
+      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
+      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
 
-      if !tags_ok(tags) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+      unless tags_ok(tags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
       tags = strip_non_xml_chars tags
 
       relid = relid.to_i
@@ -581,7 +576,7 @@ class AmfController < ApplicationController
       else
         return [0, '', relid, relid, relation.version]
       end
-   end
+    end
   end
 
   # Save a way to the database, including all nodes. Any nodes in the previous
@@ -608,26 +603,25 @@ class AmfController < ApplicationController
   # 6. hash of node versions (node=>version)
 
   def putway(renumberednodes, usertoken, changeset_id, wayversion, originalway, pointlist, attributes, nodes, deletednodes) #:doc:
-    amf_handle_error("'putway' #{originalway}" ,'way',originalway) do
+    amf_handle_error("'putway' #{originalway}", 'way', originalway) do
       # -- Initialise
 
       user = getuser(usertoken)
-      if !user then return -1,"You are not logged in, so the way could not be saved." end
-      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
-      if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
+      unless user then return -1, "You are not logged in, so the way could not be saved." end
+      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
+      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
 
-      if pointlist.length < 2 then return -2,"Server error - way is only #{points.length} points long." end
+      if pointlist.length < 2 then return -2, "Server error - way is only #{points.length} points long." end
 
-      if !tags_ok(attributes) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+      unless tags_ok(attributes) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
       attributes = strip_non_xml_chars attributes
 
       originalway = originalway.to_i
-      pointlist.collect! {|a| a.to_i }
+      pointlist.collect!(&:to_i)
 
-      way=nil # this is returned, so scope it outside the transaction
+      way = nil # this is returned, so scope it outside the transaction
       nodeversions = {}
       Way.transaction do
-
         # -- Update each changed node
 
         nodes.each do |a|
@@ -636,8 +630,8 @@ class AmfController < ApplicationController
           id = a[2].to_i
           version = a[3].to_i
 
-          if id == 0  then return -2,"Server error - node with id 0 found in way #{originalway}." end
-          if lat== 90 then return -2,"Server error - node with latitude -90 found in way #{originalway}." end
+          if id == 0  then return -2, "Server error - node with id 0 found in way #{originalway}." end
+          if lat == 90 then return -2, "Server error - node with latitude -90 found in way #{originalway}." end
           if renumberednodes[id] then id = renumberednodes[id] end
 
           node = Node.new
@@ -647,7 +641,7 @@ class AmfController < ApplicationController
           node.tags = a[4]
 
           # fixup node tags in a way as well
-          if !tags_ok(node.tags) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+          unless tags_ok(node.tags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
           node.tags = strip_non_xml_chars node.tags
 
           node.tags.delete('created_by')
@@ -659,8 +653,8 @@ class AmfController < ApplicationController
             nodeversions[node.id] = node.version
           else
             # We're updating an existing node
-            previous=Node.find(id)
-            node.id=id
+            previous = Node.find(id)
+            node.id = id
             previous.update_from(node, user)
             nodeversions[previous.id] = previous.version
           end
@@ -668,9 +662,9 @@ class AmfController < ApplicationController
 
         # -- Save revised way
 
-        pointlist.collect! {|a|
-          renumberednodes[a] ? renumberednodes[a]:a
-        } # renumber nodes
+        pointlist.collect! do|a|
+          renumberednodes[a] ? renumberednodes[a] : a
+        end # renumber nodes
         new_way = Way.new
         new_way.tags = attributes
         new_way.nds = pointlist
@@ -678,18 +672,18 @@ class AmfController < ApplicationController
         new_way.version = wayversion
         if originalway <= 0
           new_way.create_with_history(user)
-          way=new_way # so we can get way.id and way.version
+          way = new_way # so we can get way.id and way.version
         else
           way = Way.find(originalway)
-          if way.tags!=attributes or way.nds!=pointlist or !way.visible?
-            new_way.id=originalway
-          way.update_from(new_way, user)
+          if way.tags != attributes || way.nds != pointlist || !way.visible?
+            new_way.id = originalway
+            way.update_from(new_way, user)
           end
         end
 
         # -- Delete unwanted nodes
 
-        deletednodes.each do |id,v|
+        deletednodes.each do |id, v|
           node = Node.find(id.to_i)
           new_node = Node.new
           new_node.changeset_id = changeset_id
@@ -702,7 +696,6 @@ class AmfController < ApplicationController
             # and we don't want to delete it
           end
         end
-
       end # transaction
 
       [0, '', originalway, way.id, renumberednodes, way.version, nodeversions, deletednodes]
@@ -719,13 +712,13 @@ class AmfController < ApplicationController
   # 4. version.
 
   def putpoi(usertoken, changeset_id, version, id, lon, lat, tags, visible) #:doc:
-    amf_handle_error("'putpoi' #{id}", 'node',id) do
+    amf_handle_error("'putpoi' #{id}", 'node', id) do
       user = getuser(usertoken)
-      if !user then return -1,"You are not logged in, so the point could not be saved." end
-      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
-      if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
+      unless user then return -1, "You are not logged in, so the point could not be saved." end
+      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
+      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
 
-      if !tags_ok(tags) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+      unless tags_ok(tags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
       tags = strip_non_xml_chars tags
 
       id = id.to_i
@@ -733,11 +726,11 @@ class AmfController < ApplicationController
       node = nil
       new_node = nil
       Node.transaction do
-        if id > 0 then
+        if id > 0
           node = Node.find(id)
 
-          if !visible then
-            unless node.ways.empty? then return -1,"Point #{id} has since become part of a way, so you cannot save it as a POI.",id,id,version end
+          unless visible
+            unless node.ways.empty? then return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version end
           end
         end
         # We always need a new node, based on the data that has been sent to us
@@ -753,14 +746,13 @@ class AmfController < ApplicationController
           new_node.create_with_history(user)
         elsif visible
           # We're updating the node
-          new_node.id=id
+          new_node.id = id
           node.update_from(new_node, user)
         else
           # We're deleting the node
-          new_node.id=id
+          new_node.id = id
           node.delete_with_history!(new_node, user)
         end
-
       end # transaction
 
       if id <= 0
@@ -776,8 +768,8 @@ class AmfController < ApplicationController
   #
   # Returns array of id, long, lat, hash of tags, (current) version.
 
-  def getpoi(id,timestamp) #:doc:
-    amf_handle_error("'getpoi' #{id}" ,'node',id) do
+  def getpoi(id, timestamp) #:doc:
+    amf_handle_error("'getpoi' #{id}", 'node', id) do
       id = id.to_i
       n = Node.find(id)
       v = n.version
@@ -804,18 +796,17 @@ class AmfController < ApplicationController
   # Returns 0 (success), unchanged way id, new way version, new node versions.
 
   def deleteway(usertoken, changeset_id, way_id, way_version, deletednodes) #:doc:
-    amf_handle_error("'deleteway' #{way_id}" ,'way', way_id) do
+    amf_handle_error("'deleteway' #{way_id}"'way', way_id) do
       user = getuser(usertoken)
-      unless user then return -1,"You are not logged in, so the way could not be deleted." end
-      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
-      if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
+      unless user then return -1, "You are not logged in, so the way could not be deleted." end
+      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
+      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
 
       way_id = way_id.to_i
       nodeversions = {}
-      old_way=nil # returned, so scope it outside the transaction
+      old_way = nil # returned, so scope it outside the transaction
       # Need a transaction so that if one item fails to delete, the whole delete fails.
       Way.transaction do
-
         # -- Delete the way
 
         old_way = Way.find(way_id)
@@ -827,7 +818,7 @@ class AmfController < ApplicationController
 
         # -- Delete unwanted nodes
 
-        deletednodes.each do |id,v|
+        deletednodes.each do |id, v|
           node = Node.find(id.to_i)
           new_node = Node.new
           new_node.changeset_id = changeset_id
@@ -835,19 +826,17 @@ class AmfController < ApplicationController
           new_node.id = id.to_i
           begin
             node.delete_with_history!(new_node, user)
-            nodeversions[node.id]=node.version
+            nodeversions[node.id] = node.version
           rescue OSM::APIPreconditionFailedError => ex
             # We don't do anything with the exception as the node is in use
             # elsewhere and we don't want to delete it
           end
         end
-
       end # transaction
       [0, '', way_id, old_way.version, nodeversions]
     end
   end
 
-
   # ====================================================================
   # Support functions
 
@@ -857,12 +846,12 @@ class AmfController < ApplicationController
   # not just the id, hence this abstraction
 
   def getuser(token) #:doc:
-    if (token =~ /^(.+)\:(.+)$/) then
+    if token =~ /^(.+)\:(.+)$/
       user = User.authenticate(:username => $1, :password => $2)
     else
       user = User.authenticate(:token => token)
     end
-    return user
+    user
   end
 
   def getlocales
@@ -876,14 +865,14 @@ class AmfController < ApplicationController
       return false unless UTF8.valid? k
       return false unless UTF8.valid? v
     end
-    return true
+    true
   end
 
   ##
   # strip characters which are invalid in XML documents from the strings
   # in the +tags+ hash.
   def strip_non_xml_chars(tags)
-    new_tags = Hash.new
+    new_tags = {}
     unless tags.nil?
       tags.each do |k, v|
         new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015"
@@ -891,14 +880,14 @@ class AmfController < ApplicationController
         new_tags[new_k] = new_v
       end
     end
-    return new_tags
+    new_tags
   end
 
   # ====================================================================
   # Alternative SQL queries for getway/whichways
 
   def sql_find_ways_in_area(bbox)
-    sql=<<-EOF
+    sql = <<-EOF
     SELECT DISTINCT current_ways.id AS wayid,current_ways.version AS version
       FROM current_way_nodes
     INNER JOIN current_nodes ON current_nodes.id=current_way_nodes.node_id
@@ -907,12 +896,12 @@ class AmfController < ApplicationController
        AND current_ways.visible=TRUE
        AND #{OSM.sql_for_area(bbox, "current_nodes.")}
     EOF
-    return ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['wayid'].to_i,a['version'].to_i] }
+    ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['wayid'].to_i, a['version'].to_i] }
   end
 
   def sql_find_pois_in_area(bbox)
-    pois=[]
-    sql=<<-EOF
+    pois = []
+    sql = <<-EOF
       SELECT current_nodes.id,current_nodes.latitude*0.0000001 AS lat,current_nodes.longitude*0.0000001 AS lon,current_nodes.version
       FROM current_nodes
        LEFT OUTER JOIN current_way_nodes cwn ON cwn.node_id=current_nodes.id
@@ -921,19 +910,19 @@ class AmfController < ApplicationController
        AND #{OSM.sql_for_area(bbox, "current_nodes.")}
     EOF
     ActiveRecord::Base.connection.select_all(sql).each do |row|
-      poitags={}
+      poitags = {}
       ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n|
-        poitags[n['k']]=n['v']
+        poitags[n['k']] = n['v']
       end
       pois << [row['id'].to_i, row['lon'].to_f, row['lat'].to_f, poitags, row['version'].to_i]
     end
     pois
   end
 
-  def sql_find_relations_in_area_and_ways(bbox,way_ids)
+  def sql_find_relations_in_area_and_ways(bbox, way_ids)
     # ** It would be more Potlatchy to get relations for nodes within ways
     #    during 'getway', not here
-    sql=<<-EOF
+    sql = <<-EOF
       SELECT DISTINCT cr.id AS relid,cr.version AS version
       FROM current_relations cr
       INNER JOIN current_relation_members crm ON crm.id=cr.id
@@ -941,7 +930,7 @@ class AmfController < ApplicationController
        WHERE #{OSM.sql_for_area(bbox, "cn.")}
       EOF
     unless way_ids.empty?
-      sql+=<<-EOF
+      sql += <<-EOF
        UNION
         SELECT DISTINCT cr.id AS relid,cr.version AS version
         FROM current_relations cr
@@ -950,12 +939,12 @@ class AmfController < ApplicationController
          AND crm.member_id IN (#{way_ids.join(',')})
         EOF
     end
-    ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['relid'].to_i,a['version'].to_i] }
+    ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['relid'].to_i, a['version'].to_i] }
   end
 
   def sql_get_nodes_in_way(wayid)
-    points=[]
-    sql=<<-EOF
+    points = []
+    sql = <<-EOF
       SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,current_nodes.id,current_nodes.version
       FROM current_way_nodes,current_nodes
        WHERE current_way_nodes.id=#{wayid.to_i}
@@ -964,20 +953,20 @@ class AmfController < ApplicationController
       ORDER BY sequence_id
     EOF
     ActiveRecord::Base.connection.select_all(sql).each do |row|
-      nodetags={}
+      nodetags = {}
       ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n|
-        nodetags[n['k']]=n['v']
+        nodetags[n['k']] = n['v']
       end
       nodetags.delete('created_by')
-      points << [row['lon'].to_f,row['lat'].to_f,row['id'].to_i,nodetags,row['version'].to_i]
+      points << [row['lon'].to_f, row['lat'].to_f, row['id'].to_i, nodetags, row['version'].to_i]
     end
     points
   end
 
   def sql_get_tags_in_way(wayid)
-    tags={}
+    tags = {}
     ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_way_tags WHERE id=#{wayid.to_i}").each do |row|
-      tags[row['k']]=row['v']
+      tags[row['k']] = row['v']
     end
     tags
   end
index 36bb0bf..1b9a1ad 100644 (file)
@@ -1,5 +1,4 @@
 class ApiController < ApplicationController
-
   skip_before_filter :verify_authenticity_token
   before_filter :check_api_readable, :except => [:capabilities]
   before_filter :setup_user_auth, :only => [:permissions]
@@ -9,12 +8,12 @@ class ApiController < ApplicationController
   # Get an XML response containing a list of tracepoints that have been uploaded
   # within the specified bounding box, and in the specified page.
   def trackpoints
-    #retrieve the page number
+    # retrieve the page number
     page = params['page'].to_s.to_i
 
     unless page >= 0
-        report_error("Page number must be greater than or equal to 0")
-        return
+      report_error("Page number must be greater than or equal to 0")
+      return
     end
 
     offset = page * TRACEPOINTS_PER_PAGE
@@ -127,7 +126,7 @@ class ApiController < ApplicationController
       return
     end
 
-    @nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES+1)
+    @nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES + 1)
 
     node_ids = @nodes.collect(&:id)
     if node_ids.length > MAX_NUMBER_OF_NODES
@@ -146,19 +145,19 @@ class ApiController < ApplicationController
 
     # get ways
     # find which ways are needed
-    ways = Array.new
+    ways = []
     if node_ids.length > 0
       way_nodes = WayNode.where(:node_id => node_ids)
       way_ids = way_nodes.collect { |way_node| way_node.id[0] }
       ways = Way.preload(:way_nodes, :way_tags).find(way_ids)
 
-      list_of_way_nodes = ways.collect { |way|
-        way.way_nodes.collect { |way_node| way_node.node_id }
-      }
+      list_of_way_nodes = ways.collect do |way|
+        way.way_nodes.collect(&:node_id)
+      end
       list_of_way_nodes.flatten!
 
     else
-      list_of_way_nodes = Array.new
+      list_of_way_nodes = []
     end
 
     # - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this
@@ -179,7 +178,7 @@ class ApiController < ApplicationController
       end
     end
 
-    way_ids = Array.new
+    way_ids = []
     ways.each do |way|
       if way.visible?
         doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
@@ -195,7 +194,7 @@ class ApiController < ApplicationController
     # another way B also referenced, that is not returned. But we do make
     # an exception for cases where an relation references another *relation*;
     # in that case we return that as well (but we don't go recursive here)
-    relations += Relation.relations(relations.collect { |r| r.id }).visible
+    relations += Relation.relations(relations.collect(&:id)).visible
 
     # this "uniq" may be slightly inefficient; it may be better to first collect and output
     # all node-related relations, then find the *not yet covered* way-related ones etc.
@@ -213,7 +212,7 @@ class ApiController < ApplicationController
   def changes
     zoom = (params[:zoom] || '12').to_i
 
-    if params.include?(:start) and params.include?(:end)
+    if params.include?(:start) && params.include?(:end)
       starttime = Time.parse(params[:start])
       endtime = Time.parse(params[:end])
     else
@@ -222,11 +221,11 @@ class ApiController < ApplicationController
       starttime = endtime - hours
     end
 
-    if zoom >= 1 and zoom <= 16 and
-       endtime > starttime and endtime - starttime <= 24.hours
+    if zoom >= 1 && zoom <= 16 &&
+       endtime > starttime && endtime - starttime <= 24.hours
       mask = (1 << zoom) - 1
 
-      tiles = Node.where(:timestamp => starttime .. endtime).group("maptile_for_point(latitude, longitude, #{zoom})").count
+      tiles = Node.where(:timestamp => starttime..endtime).group("maptile_for_point(latitude, longitude, #{zoom})").count
 
       doc = OSM::API.new.get_xml_doc
       changes = XML::Node.new 'changes'
@@ -264,11 +263,11 @@ class ApiController < ApplicationController
 
     api = XML::Node.new 'api'
     version = XML::Node.new 'version'
-    version['minimum'] = "#{API_VERSION}";
-    version['maximum'] = "#{API_VERSION}";
+    version['minimum'] = "#{API_VERSION}"
+    version['maximum'] = "#{API_VERSION}"
     api << version
     area = XML::Node.new 'area'
-    area['maximum'] = MAX_REQUEST_AREA.to_s;
+    area['maximum'] = MAX_REQUEST_AREA.to_s
     api << area
     tracepoints = XML::Node.new 'tracepoints'
     tracepoints['per_page'] = TRACEPOINTS_PER_PAGE.to_s
index c335dca..e3f0391 100644 (file)
@@ -9,29 +9,29 @@ class ApplicationController < ActionController::Base
     if session[:user]
       @user = User.where(:id => session[:user]).where("status IN ('active', 'confirmed', 'suspended')").first
 
-    if @user.status == "suspended"
+      if @user.status == "suspended"
         session.delete(:user)
         session_expires_automatically
 
         redirect_to :controller => "user", :action => "suspended"
 
-        # don't allow access to any auth-requiring part of the site unless
-        # the new CTs have been seen (and accept/decline chosen).
-      elsif !@user.terms_seen and flash[:skip_terms].nil?
+      # don't allow access to any auth-requiring part of the site unless
+      # the new CTs have been seen (and accept/decline chosen).
+      elsif !@user.terms_seen && flash[:skip_terms].nil?
         flash[:notice] = t 'user.terms.you need to accept or decline'
         if params[:referer]
           redirect_to :controller => "user", :action => "terms", :referer => params[:referer]
         else
           redirect_to :controller => "user", :action => "terms", :referer => request.fullpath
         end
-      end
+        end
     elsif session[:token]
       if @user = User.authenticate(:token => session[:token])
         session[:user] = @user.id
       end
     end
   rescue Exception => ex
-    logger.info("Exception authorizing user: #{ex.to_s}")
+    logger.info("Exception authorizing user: #{ex}")
     reset_session
     @user = nil
   end
@@ -47,7 +47,7 @@ class ApplicationController < ActionController::Base
   end
 
   def require_oauth
-    @oauth = @user.access_token(OAUTH_KEY) if @user and defined? OAUTH_KEY
+    @oauth = @user.access_token(OAUTH_KEY) if @user && defined? OAUTH_KEY
   end
 
   ##
@@ -87,26 +87,32 @@ class ApplicationController < ActionController::Base
   def require_allow_read_prefs
     require_capability(:allow_read_prefs)
   end
+
   def require_allow_write_prefs
     require_capability(:allow_write_prefs)
   end
+
   def require_allow_write_diary
     require_capability(:allow_write_diary)
   end
+
   def require_allow_write_api
     require_capability(:allow_write_api)
 
-    if REQUIRE_TERMS_AGREED and @user.terms_agreed.nil?
+    if REQUIRE_TERMS_AGREED && @user.terms_agreed.nil?
       report_error "You must accept the contributor terms before you can edit.", :forbidden
       return false
     end
   end
+
   def require_allow_read_gpx
     require_capability(:allow_read_gpx)
   end
+
   def require_allow_write_gpx
     require_capability(:allow_write_gpx)
   end
+
   def require_allow_write_notes
     require_capability(:allow_write_notes)
   end
@@ -131,7 +137,7 @@ class ApplicationController < ActionController::Base
   # is optional.
   def setup_user_auth
     # try and setup using OAuth
-    if not Authenticator.new(self, [:token]).allow?
+    unless Authenticator.new(self, [:token]).allow?
       username, passwd = get_auth_data # parse from headers
       # authenticate per-scheme
       if username.nil?
@@ -154,14 +160,14 @@ class ApplicationController < ActionController::Base
       # if the user hasn't seen the contributor terms then don't
       # allow editing - they have to go to the web site and see
       # (but can decline) the CTs to continue.
-      if REQUIRE_TERMS_SEEN and not @user.terms_seen and flash[:skip_terms].nil?
+      if REQUIRE_TERMS_SEEN && !@user.terms_seen && flash[:skip_terms].nil?
         set_locale
         report_error t('application.setup_user_auth.need_to_see_terms'), :forbidden
       end
     end
   end
 
-  def authorize(realm='Web Password', errormessage="Couldn't authenticate you")
+  def authorize(realm = 'Web Password', errormessage = "Couldn't authenticate you")
     # make the @user object from any auth sources we have
     setup_user_auth
 
@@ -182,7 +188,7 @@ class ApplicationController < ActionController::Base
   # from require_moderator - but what we really need to do is a fairly
   # drastic refactoring based on :format and respond_to? but not a
   # good idea to do that in this branch.
-  def authorize_moderator(errormessage="Access restricted to moderators")
+  def authorize_moderator(errormessage = "Access restricted to moderators")
     # check user is a moderator
     unless @user.moderator?
       render :text => errormessage, :status => :forbidden
@@ -191,7 +197,7 @@ class ApplicationController < ActionController::Base
   end
 
   def check_database_readable(need_api = false)
-    if STATUS == :database_offline or (need_api and STATUS == :api_offline)
+    if STATUS == :database_offline || (need_api && STATUS == :api_offline)
       if request.xhr?
         report_error "Database offline for maintenance", :service_unavailable
       else
@@ -201,8 +207,8 @@ class ApplicationController < ActionController::Base
   end
 
   def check_database_writable(need_api = false)
-    if STATUS == :database_offline or STATUS == :database_readonly or
-       (need_api and (STATUS == :api_offline or STATUS == :api_readonly))
+    if STATUS == :database_offline || STATUS == :database_readonly ||
+       (need_api && (STATUS == :api_offline || STATUS == :api_readonly))
       if request.xhr?
         report_error "Database offline for maintenance", :service_unavailable
       else
@@ -244,7 +250,7 @@ class ApplicationController < ActionController::Base
         status = :readonly
       end
     end
-    return status
+    status
   end
 
   def gpx_status
@@ -252,7 +258,7 @@ class ApplicationController < ActionController::Base
     if status == :online
       status = :offline if STATUS == :gpx_offline
     end
-    return status
+    status
   end
 
   def require_public_data
@@ -271,7 +277,7 @@ class ApplicationController < ActionController::Base
     # Todo: some sort of escaping of problem characters in the message
     response.headers['Error'] = message
 
-    if request.headers['X-Error-Format'] and
+    if request.headers['X-Error-Format'] &&
        request.headers['X-Error-Format'].downcase == "xml"
       result = OSM::API.new.get_xml_doc
       result.root.name = "osmError"
@@ -304,15 +310,15 @@ class ApplicationController < ActionController::Base
 
   def select_locale(locales = I18n.available_locales)
     if params[:locale]
-      http_accept_language.user_preferred_languages = [ params[:locale] ]
+      http_accept_language.user_preferred_languages = [params[:locale]]
     end
 
     if http_accept_language.compatible_language_from(locales).nil?
       http_accept_language.user_preferred_languages = http_accept_language.user_preferred_languages.collect do |pl|
-        pls = [ pl ]
+        pls = [pl]
 
         while pl.match(/^(.*)-[^-]+$/)
-          pls.push($1) if locales.include?($1) or locales.include?($1.to_sym)
+          pls.push($1) if locales.include?($1) || locales.include?($1.to_sym)
           pl = $1
         end
 
@@ -326,25 +332,23 @@ class ApplicationController < ActionController::Base
   helper_method :select_locale
 
   def api_call_handle_error
-    begin
-      yield
-    rescue ActiveRecord::RecordNotFound => ex
-      render :text => "", :status => :not_found
-    rescue LibXML::XML::Error, ArgumentError => ex
-      report_error ex.message, :bad_request
-    rescue ActiveRecord::RecordInvalid => ex
-      message = "#{ex.record.class} #{ex.record.id}: "
-      ex.record.errors.each { |attr,msg| message << "#{attr}: #{msg} (#{ex.record[attr].inspect})" }
-      report_error message, :bad_request
-    rescue OSM::APIError => ex
-      report_error ex.message, ex.status
-    rescue AbstractController::ActionNotFound => ex
-      raise
-    rescue Exception => ex
-      logger.info("API threw unexpected #{ex.class} exception: #{ex.message}")
-      ex.backtrace.each { |l| logger.info(l) }
-      report_error "#{ex.class}: #{ex.message}", :internal_server_error
-    end
+    yield
+  rescue ActiveRecord::RecordNotFound => ex
+    render :text => "", :status => :not_found
+  rescue LibXML::XML::Error, ArgumentError => ex
+    report_error ex.message, :bad_request
+  rescue ActiveRecord::RecordInvalid => ex
+    message = "#{ex.record.class} #{ex.record.id}: "
+    ex.record.errors.each { |attr, msg| message << "#{attr}: #{msg} (#{ex.record[attr].inspect})" }
+    report_error message, :bad_request
+  rescue OSM::APIError => ex
+    report_error ex.message, ex.status
+  rescue AbstractController::ActionNotFound => ex
+    raise
+  rescue Exception => ex
+    logger.info("API threw unexpected #{ex.class} exception: #{ex.message}")
+    ex.backtrace.each { |l| logger.info(l) }
+    report_error "#{ex.class}: #{ex.message}", :internal_server_error
   end
 
   ##
@@ -352,7 +356,7 @@ class ApplicationController < ActionController::Base
   # or raises a suitable error. +method+ should be a symbol, e.g: :put or :get.
   def assert_method(method)
     ok = request.send((method.to_s.downcase + "?").to_sym)
-    raise OSM::APIBadMethodError.new(method) unless ok
+    fail OSM::APIBadMethodError.new(method) unless ok
   end
 
   ##
@@ -374,7 +378,7 @@ class ApplicationController < ActionController::Base
   rescue ActionView::Template::Error => ex
     ex = ex.original_exception
 
-    if ex.is_a?(ActiveRecord::StatementInvalid) and ex.message =~ /execution expired/
+    if ex.is_a?(ActiveRecord::StatementInvalid) && ex.message =~ /execution expired/
       ex = Timeout::Error.new
     end
 
@@ -432,14 +436,14 @@ class ApplicationController < ActionController::Base
 
   def preferred_editor
     editor = if params[:editor]
-      params[:editor]
-    elsif @user and @user.preferred_editor
-      @user.preferred_editor
-    else
-      DEFAULT_EDITOR
+               params[:editor]
+             elsif @user && @user.preferred_editor
+               @user.preferred_editor
+             else
+               DEFAULT_EDITOR
     end
 
-    if request.env['HTTP_USER_AGENT'] =~ /MSIE|Trident/ and editor == 'id'
+    if request.env['HTTP_USER_AGENT'] =~ /MSIE|Trident/ && editor == 'id'
       editor = 'potlatch2'
     end
 
@@ -448,22 +452,22 @@ class ApplicationController < ActionController::Base
 
   helper_method :preferred_editor
 
-private
+  private
 
   # extract authorisation credentials from headers, returns user = nil if none
   def get_auth_data
-    if request.env.has_key? 'X-HTTP_AUTHORIZATION'          # where mod_rewrite might have put it
+    if request.env.key? 'X-HTTP_AUTHORIZATION'          # where mod_rewrite might have put it
       authdata = request.env['X-HTTP_AUTHORIZATION'].to_s.split
-    elsif request.env.has_key? 'REDIRECT_X_HTTP_AUTHORIZATION'          # mod_fcgi
+    elsif request.env.key? 'REDIRECT_X_HTTP_AUTHORIZATION'          # mod_fcgi
       authdata = request.env['REDIRECT_X_HTTP_AUTHORIZATION'].to_s.split
-    elsif request.env.has_key? 'HTTP_AUTHORIZATION'         # regular location
+    elsif request.env.key? 'HTTP_AUTHORIZATION'         # regular location
       authdata = request.env['HTTP_AUTHORIZATION'].to_s.split
     end
     # only basic authentication supported
-    if authdata and authdata[0] == 'Basic'
-      user, pass = Base64.decode64(authdata[1]).split(':',2)
+    if authdata && authdata[0] == 'Basic'
+      user, pass = Base64.decode64(authdata[1]).split(':', 2)
     end
-    return [user, pass]
+    [user, pass]
   end
 
   # used by oauth plugin to get the current user
@@ -473,11 +477,10 @@ private
 
   # used by oauth plugin to set the current user
   def current_user=(user)
-    @user=user
+    @user = user
   end
 
   # override to stop oauth plugin sending errors
   def invalid_oauth_response
   end
-
 end
index d3a474c..3dfcd18 100644 (file)
@@ -3,7 +3,7 @@ class BrowseController < ApplicationController
 
   before_filter :authorize_web
   before_filter :set_locale
-  before_filter :except => [ :query ] { |c| c.check_database_readable(true) }
+  before_filter :except => [:query] { |c| c.check_database_readable(true) }
   before_filter :require_oauth
   around_filter :web_timeout
 
@@ -58,14 +58,14 @@ class BrowseController < ApplicationController
   def changeset
     @type = "changeset"
     @changeset = Changeset.find(params[:id])
-    if @user and @user.moderator?
+    if @user && @user.moderator?
       @comments = @changeset.comments.unscope(:where => :visible).includes(:author)
     else
       @comments = @changeset.comments.includes(:author)
     end
-    @node_pages, @nodes = paginate(:old_nodes, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'node_page')
-    @way_pages, @ways = paginate(:old_ways, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'way_page')
-    @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page')
+    @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => 'node_page')
+    @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => 'way_page')
+    @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => 'relation_page')
     if @changeset.user.data_public?
       @next_by_user = @changeset.user.changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first
       @prev_by_user = @changeset.user.changesets.where("id < ?", @changeset.id).reorder(:id => :desc).first
index eaa87df..3f693a9 100644 (file)
@@ -78,8 +78,8 @@ class ChangesetController < ApplicationController
     check_changeset_consistency(cs, @user)
 
     # keep an array of lons and lats
-    lon = Array.new
-    lat = Array.new
+    lon = []
+    lat = []
 
     # the request is in pseudo-osm format... this is kind-of an
     # abuse, maybe should change to some other format?
@@ -257,8 +257,8 @@ class ChangesetController < ApplicationController
   ##
   # list edits (open changesets) in reverse chronological order
   def list
-    if request.format == :atom and params[:max_id]
-      redirect_to params.merge({ :max_id => nil }), :status => :moved_permanently
+    if request.format == :atom && params[:max_id]
+      redirect_to params.merge(:max_id => nil), :status => :moved_permanently
       return
     end
 
@@ -275,14 +275,14 @@ class ChangesetController < ApplicationController
       return
     end
 
-    if request.format == :html and !params[:list]
+    if request.format == :html && !params[:list]
       require_oauth
       render :action => :history, :layout => map_layout
     else
       changesets = conditions_nonempty(Changeset.all)
 
       if params[:display_name]
-        if user.data_public? or user == @user
+        if user.data_public? || user == @user
           changesets = changesets.where(:user_id => user.id)
         else
           changesets = changesets.where("false")
@@ -315,8 +315,8 @@ class ChangesetController < ApplicationController
   # Add a comment to a changeset
   def comment
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
-    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
 
     # Extract the arguments
     id = params[:id].to_i
@@ -324,14 +324,12 @@ class ChangesetController < ApplicationController
 
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
-    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    fail OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
 
     # Add a comment to the changeset
-    comment = changeset.comments.create({
-      :changeset => changeset,
-      :body => body,
-      :author => @user
-    })
+    comment = changeset.comments.create(:changeset => changeset,
+                                        :body => body,
+                                        :author => @user)
 
     # Notify current subscribers of the new comment
     changeset.subscribers.each do |user|
@@ -351,15 +349,15 @@ class ChangesetController < ApplicationController
   # Adds a subscriber to the changeset
   def subscribe
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
 
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
-    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
-    raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user.id)
+    fail OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    fail OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user.id)
 
     # Add the subscriber
     changeset.subscribers << @user
@@ -372,15 +370,15 @@ class ChangesetController < ApplicationController
   # Removes a subscriber from the changeset
   def unsubscribe
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
 
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
-    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
-    raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user.id)
+    fail OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    fail OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user.id)
 
     # Remove the subscriber
     changeset.subscribers.delete(@user)
@@ -393,7 +391,7 @@ class ChangesetController < ApplicationController
   # Sets visible flag on comment to false
   def hide_comment
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -412,7 +410,7 @@ class ChangesetController < ApplicationController
   # Sets visible flag on comment to true
   def unhide_comment
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -450,7 +448,8 @@ class ChangesetController < ApplicationController
     end
   end
 
-private
+  private
+
   #------------------------------------------------------------
   # utility functions below.
   #------------------------------------------------------------
@@ -474,21 +473,21 @@ private
   ##
   # restrict changesets to those by a particular user
   def conditions_user(changesets, user, name)
-    unless user.nil? and name.nil?
+    unless user.nil? && name.nil?
       # shouldn't provide both name and UID
-      raise OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user and name
+      fail OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name
 
       # use either the name or the UID to find the user which we're selecting on.
       u = if name.nil?
             # user input checking, we don't have any UIDs < 1
-            raise OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1
+            fail OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1
             u = User.find(user.to_i)
           else
             u = User.find_by_display_name(name)
           end
 
       # make sure we found a user
-      raise OSM::APINotFoundError.new if u.nil?
+      fail OSM::APINotFoundError.new if u.nil?
 
       # should be able to get changesets of public users only, or
       # our own changesets regardless of public-ness.
@@ -497,7 +496,7 @@ private
         # changesets if they're non-public
         setup_user_auth
 
-        raise OSM::APINotFoundError if @user.nil? or @user.id != u.id
+        fail OSM::APINotFoundError if @user.nil? || @user.id != u.id
       end
       return changesets.where(:user_id => u.id)
     else
@@ -514,7 +513,7 @@ private
       if time.count(',') == 1
         # check that we actually have 2 elements in the array
         times = time.split(/,/)
-        raise OSM::APIBadUserInput.new("bad time range") if times.size != 2
+        fail OSM::APIBadUserInput.new("bad time range") if times.size != 2
 
         from, to = times.collect { |t| DateTime.parse(t) }
         return changesets.where("closed_at >= ? and created_at <= ?", from, to)
@@ -566,9 +565,9 @@ private
     if ids.nil?
       return changesets
     elsif ids.empty?
-      raise OSM::APIBadUserInput.new("No changesets were given to search for")
+      fail OSM::APIBadUserInput.new("No changesets were given to search for")
     else
-      ids = ids.split(',').collect { |n| n.to_i }
+      ids = ids.split(',').collect(&:to_i)
       return changesets.where(:id => ids)
     end
   end
@@ -577,17 +576,17 @@ private
   # eliminate empty changesets (where the bbox has not been set)
   # this should be applied to all changeset list displays
   def conditions_nonempty(changesets)
-    return changesets.where("num_changes > 0")
+    changesets.where("num_changes > 0")
   end
 
   ##
   # Get the maximum number of comments to return
   def comments_limit
     if params[:limit]
-      if params[:limit].to_i > 0 and params[:limit].to_i <= 10000
+      if params[:limit].to_i > 0 && params[:limit].to_i <= 10000
         params[:limit].to_i
       else
-        raise OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000")
+        fail OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000")
       end
     else
       100
index a611c19..5129963 100644 (file)
@@ -38,12 +38,12 @@ class DiaryEntryController < ApplicationController
   end
 
   def edit
-    @title= t 'diary_entry.edit.title'
+    @title = t 'diary_entry.edit.title'
     @diary_entry = DiaryEntry.find(params[:id])
 
     if @user != @diary_entry.user
       redirect_to :controller => 'diary_entry', :action => 'view', :id => params[:id]
-    elsif params[:diary_entry] and @diary_entry.update_attributes(entry_params)
+    elsif params[:diary_entry] && @diary_entry.update_attributes(entry_params)
       redirect_to :controller => 'diary_entry', :action => 'view', :id => params[:id]
     end
 
@@ -85,19 +85,19 @@ class DiaryEntryController < ApplicationController
         @title = t 'diary_entry.list.title_friends'
         @entries = DiaryEntry.where(:user_id => @user.friend_users)
       else
-          require_user
-          return
+        require_user
+        return
       end
     elsif params[:nearby]
       if @user
         @title = t 'diary_entry.list.title_nearby'
         @entries = DiaryEntry.where(:user_id => @user.nearby)
       else
-          require_user
-          return
+        require_user
+        return
       end
     else
-      @entries = DiaryEntry.joins(:user).where(:users => { :status => ["active", "confirmed"] })
+      @entries = DiaryEntry.joins(:user).where(:users => { :status => %w(active confirmed) })
 
       if params[:language]
         @title = t 'diary_entry.list.in_language_title', :language => Language.find(params[:language]).english_name
@@ -131,7 +131,7 @@ class DiaryEntryController < ApplicationController
         return
       end
     else
-      @entries = DiaryEntry.joins(:user).where(:users => { :status => ["active", "confirmed"] })
+      @entries = DiaryEntry.joins(:user).where(:users => { :status => %w(active confirmed) })
 
       if params[:language]
         @entries = @entries.where(:language_code => params[:language])
@@ -180,7 +180,9 @@ class DiaryEntryController < ApplicationController
                                          :per_page => 20)
     @page = (params[:page] || 1).to_i
   end
-private
+
+  private
+
   ##
   # return permitted diary entry parameters
   def entry_params
@@ -206,17 +208,17 @@ private
   ##
   # is this list user specific?
   def user_specific_list?
-    params[:friends] or params[:nearby]
+    params[:friends] || params[:nearby]
   end
 
   ##
   # decide on a location for the diary entry map
   def set_map_location
-    if @diary_entry.latitude and @diary_entry.longitude
+    if @diary_entry.latitude && @diary_entry.longitude
       @lon = @diary_entry.longitude
       @lat = @diary_entry.latitude
       @zoom = 12
-    elsif @user.home_lat.nil? or @user.home_lon.nil?
+    elsif @user.home_lat.nil? || @user.home_lon.nil?
       @lon = params[:lon] || -0.1
       @lat = params[:lat] || 51.5
       @zoom = params[:zoom] || 4
index 00eba74..6a8348f 100644 (file)
@@ -1,21 +1,20 @@
 class ExportController < ApplicationController
-
   before_filter :authorize_web
   before_filter :set_locale
 
   caches_page :embed
 
-  #When the user clicks 'Export' we redirect to a URL which generates the export download
+  # When the user clicks 'Export' we redirect to a URL which generates the export download
   def finish
     bbox = BoundingBox.from_lon_lat_params(params)
     format = params[:format]
 
     if format == "osm"
-      #redirect to API map get
+      # redirect to API map get
       redirect_to "http://api.openstreetmap.org/api/#{API_VERSION}/map?bbox=#{bbox}"
 
     elsif format == "mapnik"
-      #redirect to a special 'export' cgi script
+      # redirect to a special 'export' cgi script
       format = params[:mapnik_format]
       scale = params[:mapnik_scale]
 
index 5a3cbeb..25042c4 100644 (file)
@@ -38,16 +38,16 @@ class GeocoderController < ApplicationController
   def search_latlon
     lat = params[:lat].to_f
     lon = params[:lon].to_f
-    if lat < -90 or lat > 90
+    if lat < -90 || lat > 90
       @error = "Latitude #{lat} out of range"
       render :action => "error"
-    elsif lon < -180 or lon > 180
+    elsif lon < -180 || lon > 180
       @error = "Longitude #{lon} out of range"
       render :action => "error"
     else
-      @results = [{:lat => lat, :lon => lon,
-                   :zoom => params[:zoom],
-                   :name => "#{lat}, #{lon}"}]
+      @results = [{ :lat => lat, :lon => lon,
+                    :zoom => params[:zoom],
+                    :name => "#{lat}, #{lon}" }]
 
       render :action => "results"
     end
@@ -58,7 +58,7 @@ class GeocoderController < ApplicationController
     query = params[:query]
 
     # create result array
-    @results = Array.new
+    @results = []
 
     # ask geocoder.us (they have a non-commercial use api)
     response = fetch_text("http://rpc.geocoder.us/service/csv?zip=#{escape_query(query)}")
@@ -66,15 +66,15 @@ class GeocoderController < ApplicationController
     # parse the response
     unless response.match(/couldn't find this zip/)
       data = response.split(/\s*,\s+/) # lat,long,town,state,zip
-      @results.push({:lat => data[0], :lon => data[1],
-                     :zoom => POSTCODE_ZOOM,
-                     :prefix => "#{data[2]}, #{data[3]},",
-                     :name => data[4]})
+      @results.push(:lat => data[0], :lon => data[1],
+                    :zoom => POSTCODE_ZOOM,
+                    :prefix => "#{data[2]}, #{data[3]},",
+                    :name => data[4])
     end
 
     render :action => "results"
   rescue Exception => ex
-    @error = "Error contacting rpc.geocoder.us: #{ex.to_s}"
+    @error = "Error contacting rpc.geocoder.us: #{ex}"
     render :action => "error"
   end
 
@@ -83,7 +83,7 @@ class GeocoderController < ApplicationController
     query = params[:query]
 
     # create result array
-    @results = Array.new
+    @results = []
 
     # ask npemap.org.uk to do a combined npemap + freethepostcode search
     response = fetch_text("http://www.npemap.org.uk/cgi/geocoder.fcgi?format=text&postcode=#{escape_query(query)}")
@@ -94,35 +94,35 @@ class GeocoderController < ApplicationController
       data = dataline.split(/,/) # easting,northing,postcode,lat,long
       postcode = data[2].gsub(/'/, "")
       zoom = POSTCODE_ZOOM - postcode.count("#")
-      @results.push({:lat => data[3], :lon => data[4], :zoom => zoom,
-                     :name => postcode})
+      @results.push(:lat => data[3], :lon => data[4], :zoom => zoom,
+                    :name => postcode)
     end
 
     render :action => "results"
   rescue Exception => ex
-    @error = "Error contacting www.npemap.org.uk: #{ex.to_s}"
+    @error = "Error contacting www.npemap.org.uk: #{ex}"
     render :action => "error"
   end
 
   def search_ca_postcode
     # get query parameters
     query = params[:query]
-    @results = Array.new
+    @results = []
 
     # ask geocoder.ca (note - they have a per-day limit)
     response = fetch_xml("http://geocoder.ca/?geoit=XML&postal=#{escape_query(query)}")
 
     # parse the response
     if response.get_elements("geodata/error").empty?
-      @results.push({:lat => response.get_text("geodata/latt").to_s,
-                     :lon => response.get_text("geodata/longt").to_s,
-                     :zoom => POSTCODE_ZOOM,
-                     :name => query.upcase})
+      @results.push(:lat => response.get_text("geodata/latt").to_s,
+                    :lon => response.get_text("geodata/longt").to_s,
+                    :zoom => POSTCODE_ZOOM,
+                    :name => query.upcase)
     end
 
     render :action => "results"
   rescue Exception => ex
-    @error = "Error contacting geocoder.ca: #{ex.to_s}"
+    @error = "Error contacting geocoder.ca: #{ex}"
     render :action => "error"
   end
 
@@ -154,12 +154,10 @@ class GeocoderController < ApplicationController
     more_url_params = CGI.parse(URI.parse(results.attributes["more_url"]).query)
 
     # create result array
-    @results = Array.new
+    @results = []
 
     # create parameter hash for "more results" link
-    @more_params = params.merge({
-      :exclude => more_url_params["exclude_place_ids"].first
-    })
+    @more_params = params.merge(:exclude => more_url_params["exclude_place_ids"].first)
 
     # parse the response
     results.elements.each("place") do |place|
@@ -168,13 +166,13 @@ class GeocoderController < ApplicationController
       klass = place.attributes["class"].to_s
       type = place.attributes["type"].to_s
       name = place.attributes["display_name"].to_s
-      min_lat,max_lat,min_lon,max_lon = place.attributes["boundingbox"].to_s.split(",")
+      min_lat, max_lat, min_lon, max_lon = place.attributes["boundingbox"].to_s.split(",")
       if type.empty?
         prefix_name = ""
       else
         prefix_name = t "geocoder.search_osm_nominatim.prefix.#{klass}.#{type}", :default => type.gsub("_", " ").capitalize
       end
-      if klass == 'boundary' and type == 'administrative'
+      if klass == 'boundary' && type == 'administrative'
         rank = (place.attributes["place_rank"].to_i + 1) / 2
         prefix_name = t "geocoder.search_osm_nominatim.admin_levels.level#{rank}", :default => prefix_name
       end
@@ -182,17 +180,17 @@ class GeocoderController < ApplicationController
       object_type = place.attributes["osm_type"]
       object_id = place.attributes["osm_id"]
 
-      @results.push({:lat => lat, :lon => lon,
-                     :min_lat => min_lat, :max_lat => max_lat,
-                     :min_lon => min_lon, :max_lon => max_lon,
-                     :prefix => prefix, :name => name,
-                     :type => object_type, :id => object_id})
+      @results.push(:lat => lat, :lon => lon,
+                    :min_lat => min_lat, :max_lat => max_lat,
+                    :min_lon => min_lon, :max_lon => max_lon,
+                    :prefix => prefix, :name => name,
+                    :type => object_type, :id => object_id)
     end
 
     render :action => "results"
-#  rescue Exception => ex
-#    @error = "Error contacting nominatim.openstreetmap.org: #{ex.to_s}"
-#    render :action => "error"
+    #  rescue Exception => ex
+    #    @error = "Error contacting nominatim.openstreetmap.org: #{ex.to_s}"
+    #    render :action => "error"
   end
 
   def search_geonames
@@ -203,7 +201,7 @@ class GeocoderController < ApplicationController
     lang = I18n.locale.to_s.split("-").first
 
     # create result array
-    @results = Array.new
+    @results = []
 
     # ask geonames.org
     response = fetch_xml("http://api.geonames.org/search?q=#{escape_query(query)}&lang=#{lang}&maxRows=20&username=#{GEONAMES_USERNAME}")
@@ -214,15 +212,15 @@ class GeocoderController < ApplicationController
       lon = geoname.get_text("lng").to_s
       name = geoname.get_text("name").to_s
       country = geoname.get_text("countryName").to_s
-      @results.push({:lat => lat, :lon => lon,
-                     :zoom => GEONAMES_ZOOM,
-                     :name => name,
-                     :suffix => ", #{country}"})
+      @results.push(:lat => lat, :lon => lon,
+                    :zoom => GEONAMES_ZOOM,
+                    :name => name,
+                    :suffix => ", #{country}")
     end
 
     render :action => "results"
   rescue Exception => ex
-    @error = "Error contacting ws.geonames.org: #{ex.to_s}"
+    @error = "Error contacting ws.geonames.org: #{ex}"
     render :action => "error"
   end
 
@@ -233,7 +231,7 @@ class GeocoderController < ApplicationController
     zoom = params[:zoom]
 
     # create result array
-    @results = Array.new
+    @results = []
 
     # ask nominatim
     response = fetch_xml("http:#{NOMINATIM_URL}reverse?lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}")
@@ -246,15 +244,15 @@ class GeocoderController < ApplicationController
       object_id = result.attributes["osm_id"]
       description = result.get_text.to_s
 
-      @results.push({:lat => lat, :lon => lon,
-                     :zoom => zoom,
-                     :name => description,
-                     :type => object_type, :id => object_id})
+      @results.push(:lat => lat, :lon => lon,
+                    :zoom => zoom,
+                    :name => description,
+                    :type => object_type, :id => object_id)
     end
 
     render :action => "results"
   rescue Exception => ex
-    @error = "Error contacting nominatim.openstreetmap.org: #{ex.to_s}"
+    @error = "Error contacting nominatim.openstreetmap.org: #{ex}"
     render :action => "error"
   end
 
@@ -267,7 +265,7 @@ class GeocoderController < ApplicationController
     lang = I18n.locale.to_s.split("-").first
 
     # create result array
-    @results = Array.new
+    @results = []
 
     # ask geonames.org
     response = fetch_xml("http://api.geonames.org/countrySubdivision?lat=#{lat}&lng=#{lon}&lang=#{lang}&username=#{GEONAMES_USERNAME}")
@@ -276,45 +274,45 @@ class GeocoderController < ApplicationController
     response.elements.each("geonames/countrySubdivision") do |geoname|
       name = geoname.get_text("adminName1").to_s
       country = geoname.get_text("countryName").to_s
-      @results.push({:lat => lat, :lon => lon,
-                     :zoom => GEONAMES_ZOOM,
-                     :name => name,
-                     :suffix => ", #{country}"})
+      @results.push(:lat => lat, :lon => lon,
+                    :zoom => GEONAMES_ZOOM,
+                    :name => name,
+                    :suffix => ", #{country}")
     end
 
     render :action => "results"
   rescue Exception => ex
-    @error = "Error contacting ws.geonames.org: #{ex.to_s}"
+    @error = "Error contacting ws.geonames.org: #{ex}"
     render :action => "error"
   end
 
-private
+  private
 
   def fetch_text(url)
-    return Net::HTTP.get(URI.parse(url))
+    Net::HTTP.get(URI.parse(url))
   end
 
   def fetch_xml(url)
-    return REXML::Document.new(fetch_text(url))
+    REXML::Document.new(fetch_text(url))
   end
 
   def format_distance(distance)
-    return t("geocoder.distance", :count => distance)
+    t("geocoder.distance", :count => distance)
   end
 
   def format_direction(bearing)
-    return t("geocoder.direction.south_west") if bearing >= 22.5 and bearing < 67.5
-    return t("geocoder.direction.south") if bearing >= 67.5 and bearing < 112.5
-    return t("geocoder.direction.south_east") if bearing >= 112.5 and bearing < 157.5
-    return t("geocoder.direction.east") if bearing >= 157.5 and bearing < 202.5
-    return t("geocoder.direction.north_east") if bearing >= 202.5 and bearing < 247.5
-    return t("geocoder.direction.north") if bearing >= 247.5 and bearing < 292.5
-    return t("geocoder.direction.north_west") if bearing >= 292.5 and bearing < 337.5
-    return t("geocoder.direction.west")
+    return t("geocoder.direction.south_west") if bearing >= 22.5 && bearing < 67.5
+    return t("geocoder.direction.south") if bearing >= 67.5 && bearing < 112.5
+    return t("geocoder.direction.south_east") if bearing >= 112.5 && bearing < 157.5
+    return t("geocoder.direction.east") if bearing >= 157.5 && bearing < 202.5
+    return t("geocoder.direction.north_east") if bearing >= 202.5 && bearing < 247.5
+    return t("geocoder.direction.north") if bearing >= 247.5 && bearing < 292.5
+    return t("geocoder.direction.north_west") if bearing >= 292.5 && bearing < 337.5
+    t("geocoder.direction.west")
   end
 
   def format_name(name)
-    return name.gsub(/( *\[[^\]]*\])*$/, "")
+    name.gsub(/( *\[[^\]]*\])*$/, "")
   end
 
   def count_results(results)
@@ -324,11 +322,11 @@ private
       count += source[:results].length if source[:results]
     end
 
-    return count
+    count
   end
 
   def escape_query(query)
-    return URI.escape(query, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]", false, 'N'))
+    URI.escape(query, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]", false, 'N'))
   end
 
   def normalize_params
@@ -353,7 +351,7 @@ private
       params.merge!(dms_to_decdeg(latlon)).delete(:query)
 
     elsif latlon = query.match(/^\s*([+-]?\d+(\.\d*)?)\s*[\s,]\s*([+-]?\d+(\.\d*)?)\s*$/)
-      params.merge!({:lat => latlon[1].to_f, :lon => latlon[3].to_f}).delete(:query)
+      params.merge!(:lat => latlon[1].to_f, :lon => latlon[3].to_f).delete(:query)
     end
   end
 
@@ -366,31 +364,30 @@ private
       captures[0].downcase != 's' ? lat = captures[1].to_f : lat = -(captures[1].to_f)
       captures[3].downcase != 'w' ? lon = captures[4].to_f : lon = -(captures[4].to_f)
     end
-    {:lat => lat, :lon => lon}
+    { :lat => lat, :lon => lon }
   end
 
   def ddm_to_decdeg(captures)
     begin
       Float(captures[0])
-      captures[3].downcase != 's' ? lat = captures[0].to_f + captures[1].to_f/60 : lat = -(captures[0].to_f + captures[1].to_f/60)
-      captures[7].downcase != 'w' ? lon = captures[4].to_f + captures[5].to_f/60 : lon = -(captures[4].to_f + captures[5].to_f/60)
+      captures[3].downcase != 's' ? lat = captures[0].to_f + captures[1].to_f / 60 : lat = -(captures[0].to_f + captures[1].to_f / 60)
+      captures[7].downcase != 'w' ? lon = captures[4].to_f + captures[5].to_f / 60 : lon = -(captures[4].to_f + captures[5].to_f / 60)
     rescue
-      captures[0].downcase != 's' ? lat = captures[1].to_f + captures[2].to_f/60 : lat = -(captures[1].to_f + captures[2].to_f/60)
-      captures[4].downcase != 'w' ? lon = captures[5].to_f + captures[6].to_f/60 : lon = -(captures[5].to_f + captures[6].to_f/60)
+      captures[0].downcase != 's' ? lat = captures[1].to_f + captures[2].to_f / 60 : lat = -(captures[1].to_f + captures[2].to_f / 60)
+      captures[4].downcase != 'w' ? lon = captures[5].to_f + captures[6].to_f / 60 : lon = -(captures[5].to_f + captures[6].to_f / 60)
     end
-    {:lat => lat, :lon => lon}
+    { :lat => lat, :lon => lon }
   end
 
   def dms_to_decdeg(captures)
     begin
       Float(captures[0])
-      captures[4].downcase != 's' ? lat = captures[0].to_f + (captures[1].to_f + captures[2].to_f/60)/60 : lat = -(captures[0].to_f + (captures[1].to_f + captures[2].to_f/60)/60)
-      captures[9].downcase != 'w' ? lon = captures[5].to_f + (captures[6].to_f + captures[7].to_f/60)/60 : lon = -(captures[5].to_f + (captures[6].to_f + captures[7].to_f/60)/60)
+      captures[4].downcase != 's' ? lat = captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : lat = -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60)
+      captures[9].downcase != 'w' ? lon = captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : lon = -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60)
     rescue
-      captures[0].downcase != 's' ? lat = captures[1].to_f + (captures[2].to_f + captures[3].to_f/60)/60 : lat = -(captures[1].to_f + (captures[2].to_f + captures[3].to_f/60)/60)
-      captures[5].downcase != 'w' ? lon = captures[6].to_f + (captures[7].to_f + captures[8].to_f/60)/60 : lon = -(captures[6].to_f + (captures[7].to_f + captures[8].to_f/60)/60)
+      captures[0].downcase != 's' ? lat = captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : lat = -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60)
+      captures[5].downcase != 'w' ? lon = captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : lon = -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60)
     end
-    {:lat => lat, :lon => lon}
+    { :lat => lat, :lon => lon }
   end
-
 end
index c7acc90..3cc80c1 100644 (file)
@@ -38,13 +38,13 @@ class MessageController < ApplicationController
   def reply
     message = Message.find(params[:message_id])
 
-    if message.to_user_id == @user.id then
+    if message.to_user_id == @user.id
       message.update_attribute(:message_read, true)
 
       @message = Message.new(
         :recipient => message.sender,
         :title => "Re: #{message.title.sub(/^Re:\s*/, '')}",
-        :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}",
+        :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}"
       )
 
       @title = @message.title
@@ -55,7 +55,7 @@ class MessageController < ApplicationController
       redirect_to :controller => "user", :action => "login", :referer => request.fullpath
     end
   rescue ActiveRecord::RecordNotFound
-    @title = t'message.no_such_message.title'
+    @title = t 'message.no_such_message.title'
     render :action => 'no_such_message', :status => :not_found
   end
 
@@ -64,7 +64,7 @@ class MessageController < ApplicationController
     @title = t 'message.read.title'
     @message = Message.find(params[:message_id])
 
-    if @message.to_user_id == @user.id or @message.from_user_id == @user.id then
+    if @message.to_user_id == @user.id || @message.from_user_id == @user.id
       @message.message_read = true if @message.to_user_id == @user.id
       @message.save
     else
@@ -72,14 +72,14 @@ class MessageController < ApplicationController
       redirect_to :controller => "user", :action => "login", :referer => request.fullpath
     end
   rescue ActiveRecord::RecordNotFound
-    @title = t'message.no_such_message.title'
+    @title = t 'message.no_such_message.title'
     render :action => 'no_such_message', :status => :not_found
   end
 
   # Display the list of messages that have been sent to the user.
   def inbox
     @title = t 'message.inbox.title'
-    if @user and params[:display_name] == @user.display_name
+    if @user && params[:display_name] == @user.display_name
     else
       redirect_to :controller => 'message', :action => 'inbox', :display_name => @user.display_name
     end
@@ -88,7 +88,7 @@ class MessageController < ApplicationController
   # Display the list of messages that the user has sent to other users.
   def outbox
     @title = t 'message.outbox.title'
-    if @user and params[:display_name] == @user.display_name
+    if @user && params[:display_name] == @user.display_name
     else
       redirect_to :controller => 'message', :action => 'outbox', :display_name => @user.display_name
     end
@@ -105,12 +105,12 @@ class MessageController < ApplicationController
       notice = t 'message.mark.as_read'
     end
     @message.message_read = message_read
-    if @message.save and not request.xhr?
+    if @message.save && !request.xhr?
       flash[:notice] = notice
       redirect_to :controller => 'message', :action => 'inbox', :display_name => @user.display_name
     end
   rescue ActiveRecord::RecordNotFound
-    @title = t'message.no_such_message.title'
+    @title = t 'message.no_such_message.title'
     render :action => 'no_such_message', :status => :not_found
   end
 
@@ -119,7 +119,7 @@ class MessageController < ApplicationController
     @message = Message.where("to_user_id = ? OR from_user_id = ?", @user.id, @user.id).find(params[:message_id])
     @message.from_user_visible = false if @message.sender == @user
     @message.to_user_visible = false if @message.recipient == @user
-    if @message.save and not request.xhr?
+    if @message.save && !request.xhr?
       flash[:notice] = t 'message.delete.deleted'
 
       if params[:referer]
@@ -129,10 +129,12 @@ class MessageController < ApplicationController
       end
     end
   rescue ActiveRecord::RecordNotFound
-    @title = t'message.no_such_message.title'
+    @title = t 'message.no_such_message.title'
     render :action => 'no_such_message', :status => :not_found
   end
-private
+
+  private
+
   ##
   # return permitted message parameters
   def message_params
index 597aa4a..cd5b893 100644 (file)
@@ -41,8 +41,8 @@ class NodeController < ApplicationController
     node = Node.find(params[:id])
     new_node = Node.from_xml(request.raw_post)
 
-    unless new_node and new_node.id == node.id
-      raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
+    unless new_node && new_node.id == node.id
+      fail OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
     end
     node.update_from(new_node, @user)
     render :text => node.version.to_s, :content_type => "text/plain"
@@ -55,8 +55,8 @@ class NodeController < ApplicationController
     node = Node.find(params[:id])
     new_node = Node.from_xml(request.raw_post)
 
-    unless new_node and new_node.id == node.id
-      raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
+    unless new_node && new_node.id == node.id
+      fail OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
     end
     node.delete_with_history!(new_node, @user)
     render :text => node.version.to_s, :content_type => "text/plain"
@@ -64,14 +64,14 @@ class NodeController < ApplicationController
 
   # Dump the details on many nodes whose ids are given in the "nodes" parameter.
   def nodes
-    if not params['nodes']
-      raise OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]")
+    unless params['nodes']
+      fail OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]")
     end
 
-    ids = params['nodes'].split(',').collect { |n| n.to_i }
+    ids = params['nodes'].split(',').collect(&:to_i)
 
     if ids.length == 0
-      raise OSM::APIBadUserInput.new("No nodes were given to search for")
+      fail OSM::APIBadUserInput.new("No nodes were given to search for")
     end
     doc = OSM::API.new.get_xml_doc
 
index e478244..5fdda44 100644 (file)
@@ -1,5 +1,4 @@
 class NotesController < ApplicationController
-
   layout 'site', :only => [:mine]
 
   before_filter :check_api_readable
@@ -21,10 +20,10 @@ class NotesController < ApplicationController
     if params[:bbox]
       bbox = BoundingBox.from_bbox_params(params)
     else
-      raise OSM::APIBadUserInput.new("No l was given") unless params[:l]
-      raise OSM::APIBadUserInput.new("No r was given") unless params[:r]
-      raise OSM::APIBadUserInput.new("No b was given") unless params[:b]
-      raise OSM::APIBadUserInput.new("No t was given") unless params[:t]
+      fail OSM::APIBadUserInput.new("No l was given") unless params[:l]
+      fail OSM::APIBadUserInput.new("No r was given") unless params[:r]
+      fail OSM::APIBadUserInput.new("No b was given") unless params[:b]
+      fail OSM::APIBadUserInput.new("No t was given") unless params[:t]
 
       bbox = BoundingBox.from_lrbt_params(params)
     end
@@ -54,12 +53,12 @@ class NotesController < ApplicationController
   # Create a new note
   def create
     # Check the ACLs
-    raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
+    fail OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
 
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No lat was given") unless params[:lat]
-    raise OSM::APIBadUserInput.new("No lon was given") unless params[:lon]
-    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+    fail OSM::APIBadUserInput.new("No lat was given") unless params[:lat]
+    fail OSM::APIBadUserInput.new("No lon was given") unless params[:lon]
+    fail OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
 
     # Extract the arguments
     lon = OSM.parse_float(params[:lon], OSM::APIBadUserInput, "lon was not a number")
@@ -70,7 +69,7 @@ class NotesController < ApplicationController
     Note.transaction do
       # Create the note
       @note = Note.create(:lat => lat, :lon => lon)
-      raise OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world?
+      fail OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world?
 
       # Save the note
       @note.save!
@@ -90,11 +89,11 @@ class NotesController < ApplicationController
   # Add a comment to an existing note
   def comment
     # Check the ACLs
-    raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
+    fail OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
 
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
-    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
 
     # Extract the arguments
     id = params[:id].to_i
@@ -102,9 +101,9 @@ class NotesController < ApplicationController
 
     # Find the note and check it is valid
     @note = Note.find(id)
-    raise OSM::APINotFoundError unless @note
-    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
-    raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
+    fail OSM::APINotFoundError unless @note
+    fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
+    fail OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
 
     # Add a comment to the note
     Note.transaction do
@@ -122,7 +121,7 @@ class NotesController < ApplicationController
   # Close a note
   def close
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -130,9 +129,9 @@ class NotesController < ApplicationController
 
     # Find the note and check it is valid
     @note = Note.find_by_id(id)
-    raise OSM::APINotFoundError unless @note
-    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
-    raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
+    fail OSM::APINotFoundError unless @note
+    fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
+    fail OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
 
     # Close the note and add a comment
     Note.transaction do
@@ -152,7 +151,7 @@ class NotesController < ApplicationController
   # Reopen a note
   def reopen
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -160,9 +159,9 @@ class NotesController < ApplicationController
 
     # Find the note and check it is valid
     @note = Note.find_by_id(id)
-    raise OSM::APINotFoundError unless @note
-    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? or @user.moderator?
-    raise OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? or not @note.visible?
+    fail OSM::APINotFoundError unless @note
+    fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || @user.moderator?
+    fail OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? || !@note.visible?
 
     # Reopen the note and add a comment
     Note.transaction do
@@ -207,12 +206,12 @@ class NotesController < ApplicationController
   # Read a note
   def show
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Find the note and check it is valid
     @note = Note.find(params[:id])
-    raise OSM::APINotFoundError unless @note
-    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
+    fail OSM::APINotFoundError unless @note
+    fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
 
     # Render the result
     respond_to do |format|
@@ -227,7 +226,7 @@ class NotesController < ApplicationController
   # Delete (hide) a note
   def destroy
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    fail OSM::APIBadUserInput.new("No id was given") unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -235,8 +234,8 @@ class NotesController < ApplicationController
 
     # Find the note and check it is valid
     @note = Note.find(id)
-    raise OSM::APINotFoundError unless @note
-    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
+    fail OSM::APINotFoundError unless @note
+    fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
 
     # Mark the note as hidden
     Note.transaction do
@@ -257,7 +256,7 @@ class NotesController < ApplicationController
   # Return a list of notes matching a given string
   def search
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No query string was given") unless params[:q]
+    fail OSM::APIBadUserInput.new("No query string was given") unless params[:q]
 
     # Get any conditions that need to be applied
     @notes = closed_condition(Note.all)
@@ -295,7 +294,8 @@ class NotesController < ApplicationController
     end
   end
 
-private
+  private
+
   #------------------------------------------------------------
   # utility functions below.
   #------------------------------------------------------------
@@ -315,10 +315,10 @@ private
   # Get the maximum number of results to return
   def result_limit
     if params[:limit]
-      if params[:limit].to_i > 0 and params[:limit].to_i <= 10000
+      if params[:limit].to_i > 0 && params[:limit].to_i <= 10000
         params[:limit].to_i
       else
-        raise OSM::APIBadUserInput.new("Note limit must be between 1 and 10000")
+        fail OSM::APIBadUserInput.new("Note limit must be between 1 and 10000")
       end
     else
       100
@@ -343,7 +343,7 @@ private
       notes = notes.where("status = 'open'")
     end
 
-    return notes
+    notes
   end
 
   ##
@@ -359,8 +359,8 @@ private
 
     comment = note.comments.create(attributes)
 
-    note.comments.map { |c| c.author }.uniq.each do |user|
-      if notify and user and user != @user
+    note.comments.map(&:author).uniq.each do |user|
+      if notify && user && user != @user
         Notifier.note_comment_notification(comment, user).deliver_now
       end
     end
index 320dab1..48074dd 100644 (file)
@@ -17,7 +17,7 @@ class OauthClientsController < ApplicationController
   def create
     @client_application = @user.client_applications.build(application_params)
     if @client_application.save
-      flash[:notice] = t'oauth_clients.create.flash'
+      flash[:notice] = t 'oauth_clients.create.flash'
       redirect_to :action => "show", :id => @client_application.id
     else
       render :action => "new"
@@ -41,7 +41,7 @@ class OauthClientsController < ApplicationController
   def update
     @client_application = @user.client_applications.find(params[:id])
     if @client_application.update_attributes(application_params)
-      flash[:notice] = t'oauth_clients.update.flash'
+      flash[:notice] = t 'oauth_clients.update.flash'
       redirect_to :action => "show", :id => @client_application.id
     else
       render :action => "edit"
@@ -54,13 +54,15 @@ class OauthClientsController < ApplicationController
   def destroy
     @client_application = @user.client_applications.find(params[:id])
     @client_application.destroy
-    flash[:notice] = t'oauth_clients.destroy.flash'
+    flash[:notice] = t 'oauth_clients.destroy.flash'
     redirect_to :action => "index"
   rescue ActiveRecord::RecordNotFound
     @type = "client application"
     render :action => "not_found", :status => :not_found
   end
-private
+
+  private
+
   def application_params
     params.require(:client_application).permit(:name, :url, :callback_url, :support_url, ClientApplication.all_permissions)
   end
index c7251f0..4f094b9 100644 (file)
@@ -35,11 +35,11 @@ class OauthController < ApplicationController
     redirect_to oauth_clients_url(:display_name => @token.user.display_name)
   end
 
-protected
+  protected
 
   def oauth1_authorize
     unless @token
-      render :action=>"authorize_failure"
+      render :action => "authorize_failure"
       return
     end
 
index 82669f5..002da67 100644 (file)
@@ -5,22 +5,22 @@ class OldController < ApplicationController
   require 'xml/libxml'
 
   skip_before_filter :verify_authenticity_token
-  before_filter :setup_user_auth, :only => [ :history, :version ]
-  before_filter :authorize, :only => [ :redact ]
-  before_filter :authorize_moderator, :only => [ :redact ]
-  before_filter :require_allow_write_api, :only => [ :redact ]
+  before_filter :setup_user_auth, :only => [:history, :version]
+  before_filter :authorize, :only => [:redact]
+  before_filter :authorize_moderator, :only => [:redact]
+  before_filter :require_allow_write_api, :only => [:redact]
   before_filter :check_api_readable
-  before_filter :check_api_writable, :only => [ :redact ]
+  before_filter :check_api_writable, :only => [:redact]
   after_filter :compress_output
   around_filter :api_call_handle_error, :api_call_timeout
-  before_filter :lookup_old_element, :except => [ :history ]
-  before_filter :lookup_old_element_versions, :only => [ :history ]
+  before_filter :lookup_old_element, :except => [:history]
+  before_filter :lookup_old_element_versions, :only => [:history]
 
   def history
     # the .where() method used in the lookup_old_element_versions
     # call won't throw an error if no records are found, so we have
     # to do that ourselves.
-    raise OSM::APINotFoundError.new if @elements.empty?
+    fail OSM::APINotFoundError.new if @elements.empty?
 
     doc = OSM::API.new.get_xml_doc
 
@@ -38,7 +38,7 @@ class OldController < ApplicationController
   end
 
   def version
-    if @old_element.redacted? and not show_redactions?
+    if @old_element.redacted? && !show_redactions?
       render :text => "", :status => :forbidden
 
     else
@@ -72,6 +72,6 @@ class OldController < ApplicationController
   private
 
   def show_redactions?
-    @user and @user.moderator? and params[:show_redactions] == "true"
+    @user && @user.moderator? && params[:show_redactions] == "true"
   end
 end
index 75be576..a32e299 100644 (file)
@@ -1,5 +1,4 @@
 class OldNodeController < OldController
-
   private
 
   def lookup_old_element
index abf3745..78eca32 100644 (file)
@@ -1,5 +1,4 @@
 class OldRelationController < OldController
-
   private
 
   def lookup_old_element
index 64fe5b6..1daab99 100644 (file)
@@ -1,5 +1,4 @@
 class OldWayController < OldController
-
   private
 
   def lookup_old_element
index 6dbe539..b40c83e 100644 (file)
@@ -52,9 +52,9 @@ class RedactionsController < ApplicationController
   end
 
   def destroy
-    unless @redaction.old_nodes.empty? and
-        @redaction.old_ways.empty? and
-        @redaction.old_relations.empty?
+    unless @redaction.old_nodes.empty? &&
+           @redaction.old_ways.empty? &&
+           @redaction.old_relations.empty?
       flash[:error] = t('redaction.destroy.not_empty')
       redirect_to @redaction
     else
index 454e0ef..f77f722 100644 (file)
@@ -17,12 +17,12 @@ class RelationController < ApplicationController
 
     # We assume that an exception has been thrown if there was an error
     # generating the relation
-    #if relation
+    # if relation
     relation.create_with_history @user
     render :text => relation.id.to_s, :content_type => "text/plain"
-    #else
+    # else
     # render :text => "Couldn't get turn the input into a relation.", :status => :bad_request
-    #end
+    # end
   end
 
   def read
@@ -41,7 +41,7 @@ class RelationController < ApplicationController
     relation = Relation.find(params[:id])
     new_relation = Relation.from_xml(request.raw_post)
 
-    if new_relation and new_relation.id == relation.id
+    if new_relation && new_relation.id == relation.id
       relation.update_from new_relation, @user
       render :text => relation.version.to_s, :content_type => "text/plain"
     else
@@ -52,7 +52,7 @@ class RelationController < ApplicationController
   def delete
     relation = Relation.find(params[:id])
     new_relation = Relation.from_xml(request.raw_post)
-    if new_relation and new_relation.id == relation.id
+    if new_relation && new_relation.id == relation.id
       relation.delete_with_history!(new_relation, @user)
       render :text => relation.version.to_s, :content_type => "text/plain"
     else
@@ -78,7 +78,7 @@ class RelationController < ApplicationController
 
       node_ids = relation.members.select { |m| m[0] == 'Node' }.map { |m| m[1] }
       way_ids = relation.members.select { |m| m[0] == 'Way' }.map { |m| m[1] }
-      relation_ids = relation.members.select { |m| m[0] == 'Relation' and m[1] != relation.id }.map { |m| m[1] }
+      relation_ids = relation.members.select { |m| m[0] == 'Relation' && m[1] != relation.id }.map { |m| m[1] }
 
       # next load the relations and the ways.
 
@@ -88,9 +88,9 @@ class RelationController < ApplicationController
       # now additionally collect nodes referenced by ways. Note how we
       # recursively evaluate ways but NOT relations.
 
-      way_node_ids = ways.collect { |way|
-        way.way_nodes.collect { |way_node| way_node.node_id }
-      }
+      way_node_ids = ways.collect do |way|
+        way.way_nodes.collect(&:node_id)
+      end
       node_ids += way_node_ids.flatten
       nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)
 
@@ -130,14 +130,14 @@ class RelationController < ApplicationController
   end
 
   def relations
-    if not params['relations']
-      raise OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]")
+    unless params['relations']
+      fail OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]")
     end
 
-    ids = params['relations'].split(',').collect { |w| w.to_i }
+    ids = params['relations'].split(',').collect(&:to_i)
 
     if ids.length == 0
-      raise OSM::APIBadUserInput.new("No relations were given to search for")
+      fail OSM::APIBadUserInput.new("No relations were given to search for")
     end
 
     doc = OSM::API.new.get_xml_doc
@@ -162,7 +162,7 @@ class RelationController < ApplicationController
   end
 
   def relations_for_object(objtype)
-    relationids = RelationMember.where(:member_type => objtype, :member_id => params[:id]).collect { |ws| ws.relation_id }.uniq
+    relationids = RelationMember.where(:member_type => objtype, :member_id => params[:id]).collect(&:relation_id).uniq
 
     doc = OSM::API.new.get_xml_doc
 
index dd7c2fe..7cd6f4c 100644 (file)
@@ -6,23 +6,25 @@ class SearchController < ApplicationController
   after_filter :compress_output
 
   def search_all
-    do_search(true,true,true)
+    do_search(true, true, true)
   end
 
   def search_ways
-    do_search(true,false,false)
+    do_search(true, false, false)
   end
+
   def search_nodes
-    do_search(false,true,false)
+    do_search(false, true, false)
   end
+
   def search_relations
-    do_search(false,false,true)
+    do_search(false, false, true)
   end
 
-  def do_search(do_ways,do_nodes,do_relations)
+  def do_search(do_ways, do_nodes, do_relations)
     type = params['type']
     value = params['value']
-    unless type or value
+    unless type || value
       name = params['name']
       if name
         type = 'name'
@@ -49,7 +51,7 @@ class SearchController < ApplicationController
       nodes = nodes.where(:current_node_tags => { :v => value }) if value
       nodes = nodes.limit(100)
     else
-      nodes = Array.new
+      nodes = []
     end
 
     # Matching for way tags table
@@ -59,7 +61,7 @@ class SearchController < ApplicationController
       ways = ways.where(:current_way_tags => { :v => value }) if value
       ways = ways.limit(100)
     else
-      ways = Array.new
+      ways = []
     end
 
     # Matching for relation tags table
@@ -69,11 +71,11 @@ class SearchController < ApplicationController
       relations = relations.where(:current_relation_tags => { :v => value }) if value
       relations = relations.limit(2000)
     else
-      relations = Array.new
+      relations = []
     end
 
     # Fetch any node needed for our ways (only have matching nodes so far)
-    nodes += Node.find(ways.collect { |w| w.nds }.uniq)
+    nodes += Node.find(ways.collect(&:nds).uniq)
 
     # Print
     visible_nodes = {}
index e76ee97..a133e7b 100644 (file)
@@ -10,34 +10,34 @@ class SiteController < ApplicationController
   before_filter :require_oauth, :only => [:index]
 
   def index
-    unless STATUS == :database_readonly or STATUS == :database_offline
+    unless STATUS == :database_readonly || STATUS == :database_offline
       session[:location] ||= OSM::IPLocation(request.env['REMOTE_ADDR'])
     end
   end
 
   def permalink
-    lon, lat, zoom = ShortLink::decode(params[:code])
+    lon, lat, zoom = ShortLink.decode(params[:code])
     new_params = params.except(:code, :lon, :lat, :zoom, :layers, :node, :way, :relation, :changeset)
 
-    if new_params.has_key? :m
+    if new_params.key? :m
       new_params.delete :m
       new_params[:mlat] = lat
       new_params[:mlon] = lon
     end
 
-    if params.has_key? :node
+    if params.key? :node
       new_params[:controller] = 'browse'
       new_params[:action] = 'node'
       new_params[:id] = params[:node]
-    elsif params.has_key? :way
+    elsif params.key? :way
       new_params[:controller] = 'browse'
       new_params[:action] = 'way'
       new_params[:id] = params[:way]
-    elsif params.has_key? :relation
+    elsif params.key? :relation
       new_params[:controller] = 'browse'
       new_params[:action] = 'relation'
       new_params[:id] = params[:relation]
-    elsif params.has_key? :changeset
+    elsif params.key? :changeset
       new_params[:controller] = 'browse'
       new_params[:action] = 'changeset'
       new_params[:id] = params[:changeset]
@@ -48,7 +48,7 @@ class SiteController < ApplicationController
 
     new_params[:anchor] = "map=#{zoom}/#{lat}/#{lon}"
 
-    if params.has_key? :layers
+    if params.key? :layers
       new_params[:anchor] += "&layers=#{params[:layers]}"
     end
 
index 02e43a8..541185d 100644 (file)
 class SwfController < ApplicationController
-       skip_before_filter :verify_authenticity_token
-       before_filter :check_api_readable
-
-# to log:
-# RAILS_DEFAULT_LOGGER.error("Args: #{args[0]}, #{args[1]}, #{args[2]}, #{args[3]}")
-# $log.puts Time.new.to_s+','+Time.new.usec.to_s+": started GPS script"
-# http://localhost:3000/api/0.4/swf/trackpoints?xmin=-2.32402605810577&xmax=-2.18386309423859&ymin=52.1546608755772&ymax=52.2272777906895&baselong=-2.25325793066437&basey=61.3948537948532&masterscale=5825.4222222222
-
-
-       # ====================================================================
-       # Public methods
-
-       # ---- trackpoints      compile SWF of trackpoints
-
-       def trackpoints
-
-               # -     Initialise
-
-               baselong        =params['baselong'].to_f
-               basey           =params['basey'].to_f
-               masterscale     =params['masterscale'].to_f
-
-               bbox = BoundingBox.new(params['xmin'], params['ymin'],
-                                      params['xmax'], params['ymax'])
-               start=params['start'].to_i;
-
-               # -     Begin movie
-
-               bounds_left  =0
-               bounds_right =320*20
-               bounds_bottom=0
-               bounds_top   =240*20
-
-               m =''
-               m+=swfRecord(9,255.chr + 155.chr + 155.chr)                     # Background
-               absx=0
-               absy=0
-               xl=yb= 9999999
-               xr=yt=-9999999
-
-               # -     Send SQL for GPS tracks
-
-               b=''
-               lasttime=0
-               lasttrack=lastfile='-1'
-
-               if params['token']
-                 user=User.authenticate(:token => params[:token])
-                 sql="SELECT gps_points.latitude*0.0000001 AS lat,gps_points.longitude*0.0000001 AS lon,gpx_files.id AS fileid,"+
-                      "      EXTRACT(EPOCH FROM gps_points.timestamp) AS ts, gps_points.trackid AS trackid "+
-                          " FROM gpx_files,gps_points "+
-                          "WHERE gpx_files.id=gpx_id "+
-                          "  AND gpx_files.user_id=#{user.id} "+
-                          "  AND "+OSM.sql_for_area(bbox,"gps_points.")+
-                          "  AND (gps_points.timestamp IS NOT NULL) "+
-                          "ORDER BY fileid DESC,ts "+
-                          "LIMIT 10000 OFFSET #{start}"
-                 else
-                       sql="SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,gpx_id AS fileid,"+
-                            "      EXTRACT(EPOCH FROM timestamp) AS ts, gps_points.trackid AS trackid "+
-                                " FROM gps_points "+
-                                "WHERE "+OSM.sql_for_area(bbox,"gps_points.")+
-                                "  AND (gps_points.timestamp IS NOT NULL) "+
-                                "ORDER BY fileid DESC,ts "+
-                                "LIMIT 10000 OFFSET #{start}"
-               end
-               gpslist=ActiveRecord::Base.connection.select_all sql
-
-               # - Draw GPS trace lines
-
-               r=startShape()
-               gpslist.each do |row|
-                       xs=(long2coord(row['lon'].to_f,baselong,masterscale)*20).floor
-                       ys=(lat2coord(row['lat'].to_f ,basey   ,masterscale)*20).floor
-                       xl=[xs,xl].min; xr=[xs,xr].max
-                       yb=[ys,yb].min; yt=[ys,yt].max
-                       if row['ts'].to_i-lasttime>180 or row['fileid']!=lastfile or row['trackid']!=lasttrack #or row['ts'].to_i==lasttime
-                         b+=startAndMove(xs,ys,'01')
-                         absx=xs.floor; absy=ys.floor
+  skip_before_filter :verify_authenticity_token
+  before_filter :check_api_readable
+
+  # to log:
+  # RAILS_DEFAULT_LOGGER.error("Args: #{args[0]}, #{args[1]}, #{args[2]}, #{args[3]}")
+  # $log.puts Time.new.to_s+','+Time.new.usec.to_s+": started GPS script"
+  # http://localhost:3000/api/0.4/swf/trackpoints?xmin=-2.32402605810577&xmax=-2.18386309423859&ymin=52.1546608755772&ymax=52.2272777906895&baselong=-2.25325793066437&basey=61.3948537948532&masterscale=5825.4222222222
+
+  # ====================================================================
+  # Public methods
+
+  # ---- trackpoints   compile SWF of trackpoints
+
+  def trackpoints
+    # -        Initialise
+
+    baselong = params['baselong'].to_f
+    basey = params['basey'].to_f
+    masterscale = params['masterscale'].to_f
+
+    bbox = BoundingBox.new(params['xmin'], params['ymin'],
+                           params['xmax'], params['ymax'])
+    start = params['start'].to_i
+
+    # -        Begin movie
+
+    bounds_left = 0
+    bounds_right = 320 * 20
+    bounds_bottom = 0
+    bounds_top = 240 * 20
+
+    m = ''
+    m += swfRecord(9, 255.chr + 155.chr + 155.chr)                     # Background
+    absx = 0
+    absy = 0
+    xl = yb = 9999999
+    xr = yt = -9999999
+
+    # -        Send SQL for GPS tracks
+
+    b = ''
+    lasttime = 0
+    lasttrack = lastfile = '-1'
+
+    if params['token']
+      user = User.authenticate(:token => params[:token])
+      sql = "SELECT gps_points.latitude*0.0000001 AS lat,gps_points.longitude*0.0000001 AS lon,gpx_files.id AS fileid," +                     "      EXTRACT(EPOCH FROM gps_points.timestamp) AS ts, gps_points.trackid AS trackid " +                            " FROM gpx_files,gps_points " +                         "WHERE gpx_files.id=gpx_id " +                          "  AND gpx_files.user_id=#{user.id} " +                         "  AND " + OSM.sql_for_area(bbox, "gps_points.") +                      "  AND (gps_points.timestamp IS NOT NULL) " +                           "ORDER BY fileid DESC,ts " +                            "LIMIT 10000 OFFSET #{start}"
+    else
+      sql = "SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,gpx_id AS fileid," +                       "      EXTRACT(EPOCH FROM timestamp) AS ts, gps_points.trackid AS trackid " +                               " FROM gps_points " +                           "WHERE " + OSM.sql_for_area(bbox, "gps_points.") +                              "  AND (gps_points.timestamp IS NOT NULL) " +                           "ORDER BY fileid DESC,ts " +                            "LIMIT 10000 OFFSET #{start}"
+    end
+    gpslist = ActiveRecord::Base.connection.select_all sql
+
+    # - Draw GPS trace lines
+
+    r = startShape
+    gpslist.each do |row|
+      xs = (long2coord(row['lon'].to_f, baselong, masterscale) * 20).floor
+      ys = (lat2coord(row['lat'].to_f, basey, masterscale) * 20).floor
+      xl = [xs, xl].min; xr = [xs, xr].max
+      yb = [ys, yb].min; yt = [ys, yt].max
+      if row['ts'].to_i - lasttime > 180 || row['fileid'] != lastfile || row['trackid'] != lasttrack # or row['ts'].to_i==lasttime
+        b += startAndMove(xs, ys, '01')
+        absx = xs.floor; absy = ys.floor
+      end
+      b += drawTo(absx, absy, xs, ys)
+      absx = xs.floor; absy = ys.floor
+      lasttime = row['ts'].to_i
+      lastfile = row['fileid']
+      lasttrack = row['trackid']
+      while b.length > 80
+        r += [b.slice!(0...80)].pack("B*")
       end
-                 b+=drawTo(absx,absy,xs,ys)
-                       absx=xs.floor; absy=ys.floor
-                       lasttime=row['ts'].to_i
-                       lastfile=row['fileid']
-                       lasttrack=row['trackid']
-                       while b.length>80 do
-                               r+=[b.slice!(0...80)].pack("B*")
-                       end
-               end
+    end
 
-               #   (Unwayed segments removed)
+    #   (Unwayed segments removed)
 
-               # - Write shape
+    # - Write shape
 
-               b+=endShape()
-               r+=[b].pack("B*")
-               m+=swfRecord(2,packUI16(1) + packRect(xl,xr,yb,yt) + r)
-               m+=swfRecord(4,packUI16(1) + packUI16(1))
+    b += endShape
+    r += [b].pack("B*")
+    m += swfRecord(2, packUI16(1) + packRect(xl, xr, yb, yt) + r)
+    m += swfRecord(4, packUI16(1) + packUI16(1))
 
-               # -     Create Flash header and write to browser
+    # -        Create Flash header and write to browser
 
-               m+=swfRecord(1,'')                                                                      # Show frame
-               m+=swfRecord(0,'')                                                                      # End
+    m += swfRecord(1, '')                                                                      # Show frame
+    m += swfRecord(0, '')                                                                      # End
 
-               m=packRect(bounds_left,bounds_right,bounds_bottom,bounds_top) + 0.chr + 12.chr + packUI16(1) + m
-               m='FWS' + 6.chr + packUI32(m.length+8) + m
+    m = packRect(bounds_left, bounds_right, bounds_bottom, bounds_top) + 0.chr + 12.chr + packUI16(1) + m
+    m = 'FWS' + 6.chr + packUI32(m.length + 8) + m
 
-               render :text => m, :content_type => "application/x-shockwave-flash"
-       end
+    render :text => m, :content_type => "application/x-shockwave-flash"
+  end
 
-       private
+  private
 
-       # =======================================================================
-       # SWF functions
+  # =======================================================================
+  # SWF functions
 
-       # -----------------------------------------------------------------------
-       # Line-drawing
+  # -----------------------------------------------------------------------
+  # Line-drawing
 
-       def startShape
-               s =0.chr                                                                                # No fill styles
-               s+=2.chr                                                                                # Two line styles
-               s+=packUI16(0) + 0.chr + 255.chr + 255.chr              # Width 5, RGB #00FFFF
-               s+=packUI16(0) + 255.chr + 0.chr + 255.chr              # Width 5, RGB #FF00FF
-               s+=34.chr                                                                               # 2 fill, 2 line index bits
-               s
-       end
+  def startShape
+    s = 0.chr                                                                          # No fill styles
+    s += 2.chr                                                                         # Two line styles
+    s += packUI16(0) + 0.chr + 255.chr + 255.chr               # Width 5, RGB #00FFFF
+    s += packUI16(0) + 255.chr + 0.chr + 255.chr               # Width 5, RGB #FF00FF
+    s += 34.chr                                                                                # 2 fill, 2 line index bits
+    s
+  end
 
-       def endShape
-               '000000'
-       end
+  def endShape
+    '000000'
+  end
 
-       def startAndMove(x,y,col)
-               d='001001'                                                                              # Line style change, moveTo
-               l =[lengthSB(x),lengthSB(y)].max
-               d+=sprintf("%05b%0#{l}b%0#{l}b",l,x,y)
-               d+=col                                                                                  # Select line style
-       end
+  def startAndMove(x, y, col)
+    d = '001001'                                                                               # Line style change, moveTo
+    l = [lengthSB(x), lengthSB(y)].max
+    d += sprintf("%05b%0#{l}b%0#{l}b", l, x, y)
+    d += col                                                                                   # Select line style
+  end
 
-       def drawTo(absx,absy,x,y)
-               dx=x-absx
-               dy=y-absy
+  def drawTo(absx, absy, x, y)
+    dx = x - absx
+    dy = y - absy
 
     # Split the line up if there's anything>16383, because
     # that would overflow the 4 bits allowed for length
-    mstep=[dx.abs/16383,dy.abs/16383,1].max.ceil
-    xstep=dx/mstep
-    ystep=dy/mstep
-    d=''
+    mstep = [dx.abs / 16383, dy.abs / 16383, 1].max.ceil
+    xstep = dx / mstep
+    ystep = dy / mstep
+    d = ''
     for i in (1..mstep)
-      d+=drawSection(x,y,x+xstep,y+ystep)
-      x+=xstep
-      y+=ystep
+      d += drawSection(x, y, x + xstep, y + ystep)
+      x += xstep
+      y += ystep
     end
     d
-       end
-
-       def drawSection(x1,y1,x2,y2)
-               d='11'                                                                                  # TypeFlag, EdgeFlag
-         dx=x2-x1
-         dy=y2-y1
-               l =[lengthSB(dx),lengthSB(dy)].max
-               d+=sprintf("%04b",l-2)
-               d+='1'                                                                                  # GeneralLine
-               d+=sprintf("%0#{l}b%0#{l}b",dx,dy)
   end
 
-       # -----------------------------------------------------------------------
-       # Specific data types
+  def drawSection(x1, y1, x2, y2)
+    d = '11'                                                                                   # TypeFlag, EdgeFlag
+    dx = x2 - x1
+    dy = y2 - y1
+    l = [lengthSB(dx), lengthSB(dy)].max
+    d += sprintf("%04b", l - 2)
+    d += '1'                                                                                   # GeneralLine
+    d += sprintf("%0#{l}b%0#{l}b", dx, dy)
+  end
+
+  # -----------------------------------------------------------------------
+  # Specific data types
 
   # SWF data block type
 
-       def swfRecord(id,r)
-               if r.length>62
+  def swfRecord(id, r)
+    if r.length > 62
       # Long header: tag id, 0x3F, length
-                       return packUI16((id<<6)+0x3F) + packUI32(r.length) + r
-               else
+      return packUI16((id << 6) + 0x3F) + packUI32(r.length) + r
+    else
       # Short header: tag id, length
-                       return packUI16((id<<6)+r.length) + r
-               end
-       end
+      return packUI16((id << 6) + r.length) + r
+    end
+  end
 
   # SWF RECT type
 
-       def packRect(a,b,c,d)
-               l=[lengthSB(a),
-                  lengthSB(b),
-                  lengthSB(c),
-                  lengthSB(d)].max
+  def packRect(a, b, c, d)
+    l = [lengthSB(a),
+         lengthSB(b),
+         lengthSB(c),
+         lengthSB(d)].max
     # create binary string (00111001 etc.) - 5-byte length, then bbox
-               n=sprintf("%05b%0#{l}b%0#{l}b%0#{l}b%0#{l}b",l,a,b,c,d)
+    n = sprintf("%05b%0#{l}b%0#{l}b%0#{l}b%0#{l}b", l, a, b, c, d)
     # pack into byte string
-               [n].pack("B*")
-       end
-
-       # -----------------------------------------------------------------------
-       # Generic pack functions
+    [n].pack("B*")
+  end
 
-       def packUI16(n)
-               [n.floor].pack("v")
-       end
+  # -----------------------------------------------------------------------
+  # Generic pack functions
 
-       def packUI32(n)
-               [n.floor].pack("V")
-       end
+  def packUI16(n)
+    [n.floor].pack("v")
+  end
 
-       # Find number of bits required to store arbitrary-length binary
+  def packUI32(n)
+    [n.floor].pack("V")
+  end
 
-       def lengthSB(n)
-               Math.frexp(n+ (n==0?1:0) )[1]+1
-       end
+  # Find number of bits required to store arbitrary-length binary
 
-       # ====================================================================
-       # Co-ordinate conversion
-       # (this is duplicated from amf_controller, should probably share)
+  def lengthSB(n)
+    Math.frexp(n + (n == 0 ? 1 : 0))[1] + 1
+  end
 
-       def lat2coord(a,basey,masterscale)
-               -(lat2y(a)-basey)*masterscale
-       end
+  # ====================================================================
+  # Co-ordinate conversion
+  # (this is duplicated from amf_controller, should probably share)
 
-       def long2coord(a,baselong,masterscale)
-               (a-baselong)*masterscale
-       end
+  def lat2coord(a, basey, masterscale)
+    -(lat2y(a) - basey) * masterscale
+  end
 
-       def lat2y(a)
-               180/Math::PI * Math.log(Math.tan(Math::PI/4+a*(Math::PI/180)/2))
-       end
+  def long2coord(a, baselong, masterscale)
+    (a - baselong) * masterscale
+  end
 
-       def sqlescape(a)
-               a.gsub("'","''").gsub(92.chr,92.chr+92.chr)
-       end
+  def lat2y(a)
+    180 / Math::PI * Math.log(Math.tan(Math::PI / 4 + a * (Math::PI / 180) / 2))
+  end
 
+  def sqlescape(a)
+    a.gsub("'", "''").gsub(92.chr, 92.chr + 92.chr)
+  end
 end
index e1553cb..251c92a 100644 (file)
@@ -21,7 +21,7 @@ class TraceController < ApplicationController
   def list
     # from display name, pick up user id if one user's traces only
     display_name = params[:display_name]
-    if !display_name.blank?
+    unless display_name.blank?
       target_user = User.active.where(:display_name => display_name).first
       if target_user.nil?
         render_unknown_user display_name
@@ -32,7 +32,7 @@ class TraceController < ApplicationController
     # set title
     if target_user.nil?
       @title = t 'trace.list.public_traces'
-    elsif @user and @user == target_user
+    elsif @user && @user == target_user
       @title = t 'trace.list.your_traces'
     else
       @title = t 'trace.list.public_traces_from', :user => target_user.display_name
@@ -47,15 +47,15 @@ class TraceController < ApplicationController
     # 4 - user's traces, not logged in as that user = all user's public traces
     if target_user.nil? # all traces
       if @user
-        @traces = Trace.visible_to(@user) #1
+        @traces = Trace.visible_to(@user) # 1
       else
-        @traces = Trace.visible_to_all #2
+        @traces = Trace.visible_to_all # 2
       end
     else
-      if @user and @user == target_user
-        @traces = @user.traces #3 (check vs user id, so no join + can't pick up non-public traces by changing name)
+      if @user && @user == target_user
+        @traces = @user.traces # 3 (check vs user id, so no join + can't pick up non-public traces by changing name)
       else
-        @traces = target_user.traces.visible_to_all #4
+        @traces = target_user.traces.visible_to_all # 4
       end
     end
 
@@ -73,7 +73,7 @@ class TraceController < ApplicationController
     @traces = @traces.includes(:user, :tags)
 
     # put together SET of tags across traces, for related links
-    tagset = Hash.new
+    tagset = {}
     @traces.each do |trace|
       trace.tags.reload if params[:tag] # if searched by tag, ActiveRecord won't bring back other tags, so do explicitly here
       trace.tags.each do |tag|
@@ -94,8 +94,8 @@ class TraceController < ApplicationController
   def view
     @trace = Trace.find(params[:id])
 
-    if @trace and @trace.visible? and
-       (@trace.public? or @trace.user == @user)
+    if @trace && @trace.visible? &&
+       (@trace.public? || @trace.user == @user)
       @title = t 'trace.view.title', :name => @trace.name
     else
       flash[:error] = t 'trace.view.trace_not_found'
@@ -129,12 +129,12 @@ class TraceController < ApplicationController
           redirect_to :action => :list, :display_name => @user.display_name
         end
       else
-        @trace = Trace.new({:name => "Dummy",
-                            :tagstring => params[:trace][:tagstring],
-                            :description => params[:trace][:description],
-                            :visibility => params[:trace][:visibility],
-                            :inserted => false, :user => @user,
-                            :timestamp => Time.now.getutc})
+        @trace = Trace.new(:name => "Dummy",
+                           :tagstring => params[:trace][:tagstring],
+                           :description => params[:trace][:description],
+                           :visibility => params[:trace][:visibility],
+                           :inserted => false, :user => @user,
+                           :timestamp => Time.now.getutc)
         @trace.valid?
         @trace.errors.add(:gpx_file, "can't be blank")
       end
@@ -148,7 +148,7 @@ class TraceController < ApplicationController
   def data
     trace = Trace.find(params[:id])
 
-    if trace.visible? and (trace.public? or (@user and @user == trace.user))
+    if trace.visible? && (trace.public? || (@user && @user == trace.user))
       if Acl.no_trace_download(request.remote_ip)
         render :text => "", :status => :forbidden
       elsif request.format == Mime::XML
@@ -168,9 +168,9 @@ class TraceController < ApplicationController
   def edit
     @trace = Trace.find(params[:id])
 
-    if not @trace.visible?
+    if !@trace.visible?
       render :text => "", :status => :not_found
-    elsif @user.nil? or @trace.user != @user
+    elsif @user.nil? || @trace.user != @user
       render :text => "", :status => :forbidden
     else
       @title = t 'trace.edit.title', :name => @trace.name
@@ -191,9 +191,9 @@ class TraceController < ApplicationController
   def delete
     trace = Trace.find(params[:id])
 
-    if not trace.visible?
+    if !trace.visible?
       render :text => "", :status => :not_found
-    elsif @user.nil? or trace.user != @user
+    elsif @user.nil? || trace.user != @user
       render :text => "", :status => :forbidden
     else
       trace.visible = false
@@ -209,7 +209,7 @@ class TraceController < ApplicationController
     @traces = Trace.visible_to_all.visible
 
     if params[:display_name]
-      @traces = @traces.joins(:user).where(:users => {:display_name => params[:display_name]})
+      @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] })
     end
 
     if params[:tag]
@@ -225,7 +225,7 @@ class TraceController < ApplicationController
     trace = Trace.find(params[:id])
 
     if trace.inserted?
-      if trace.public? or (@user and @user == trace.user)
+      if trace.public? || (@user && @user == trace.user)
         expires_in 7.days, :private => !trace.public?, :public => trace.public?
         send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline')
       else
@@ -242,7 +242,7 @@ class TraceController < ApplicationController
     trace = Trace.find(params[:id])
 
     if trace.inserted?
-      if trace.public? or (@user and @user == trace.user)
+      if trace.public? || (@user && @user == trace.user)
         expires_in 7.days, :private => !trace.public?, :public => trace.public?
         send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline')
       else
@@ -258,7 +258,7 @@ class TraceController < ApplicationController
   def api_read
     trace = Trace.visible.find(params[:id])
 
-    if trace.public? or trace.user == @user
+    if trace.public? || trace.user == @user
       render :text => trace.to_xml.to_s, :content_type => "text/xml"
     else
       render :text => "", :status => :forbidden
@@ -271,8 +271,8 @@ class TraceController < ApplicationController
     if trace.user == @user
       new_trace = Trace.from_xml(request.raw_post)
 
-      unless new_trace and new_trace.id == trace.id
-        raise OSM::APIBadUserInput.new("The id in the url (#{trace.id}) is not the same as provided in the xml (#{new_trace.id})")
+      unless new_trace && new_trace.id == trace.id
+        fail OSM::APIBadUserInput.new("The id in the url (#{trace.id}) is not the same as provided in the xml (#{new_trace.id})")
       end
 
       trace.description = new_trace.description
@@ -302,8 +302,8 @@ class TraceController < ApplicationController
   def api_data
     trace = Trace.find(params[:id])
 
-    if trace.public? or trace.user == @user
-      if request.format == Mime::XML or request.format == Mime::GPX
+    if trace.public? || trace.user == @user
+      if request.format == Mime::XML || request.format == Mime::GPX
         send_file(trace.xml_file, :filename => "#{trace.id}.xml", :type => request.format.to_s, :disposition => 'attachment')
       else
         send_file(trace.trace_name, :filename => "#{trace.id}#{trace.extension_name}", :type => trace.mime_type, :disposition => 'attachment')
@@ -341,7 +341,7 @@ class TraceController < ApplicationController
     end
   end
 
-private
+  private
 
   def do_create(file, tags, description, visibility)
     # Sanitise the user's filename
@@ -400,7 +400,6 @@ private
     else
       @user.preferences.create(:k => "gps.trace.visibility", :v => visibility)
     end
-
   end
 
   def offline_warning
@@ -422,5 +421,4 @@ private
       "public"
     end
   end
-
 end
index f530eec..1b2ff89 100644 (file)
@@ -19,7 +19,7 @@ class UserBlocksController < ApplicationController
   end
 
   def show
-    if @user and @user.id == @user_block.user_id
+    if @user && @user.id == @user_block.user_id
       @user_block.needs_view = false
       @user_block.save!
     end
@@ -39,7 +39,7 @@ class UserBlocksController < ApplicationController
         :user_id => @this_user.id,
         :creator_id => @user.id,
         :reason => params[:user_block][:reason],
-        :ends_at => Time.now.getutc() + @block_period.hours,
+        :ends_at => Time.now.getutc + @block_period.hours,
         :needs_view => params[:user_block][:needs_view]
       )
 
@@ -60,9 +60,9 @@ class UserBlocksController < ApplicationController
         flash[:error] = t('user_block.update.only_creator_can_edit')
         redirect_to :action => "edit"
       elsif @user_block.update_attributes(
-              :ends_at => Time.now.getutc() + @block_period.hours,
-              :reason => params[:user_block][:reason],
-              :needs_view => params[:user_block][:needs_view]
+        :ends_at => Time.now.getutc + @block_period.hours,
+        :reason => params[:user_block][:reason],
+        :needs_view => params[:user_block][:needs_view]
             )
         flash[:notice] = t('user_block.update.success')
         redirect_to(@user_block)
@@ -79,7 +79,7 @@ class UserBlocksController < ApplicationController
   def revoke
     if params[:confirm]
       if @user_block.revoke! @user
-        flash[:notice] = t'user_block.revoke.flash'
+        flash[:notice] = t 'user_block.revoke.flash'
         redirect_to(@user_block)
       end
     end
@@ -90,7 +90,7 @@ class UserBlocksController < ApplicationController
   def blocks_on
     @user_blocks_pages, @user_blocks = paginate(:user_blocks,
                                                 :include => [:user, :creator, :revoker],
-                                                :conditions => {:user_id => @this_user.id},
+                                                :conditions => { :user_id => @this_user.id },
                                                 :order => "user_blocks.ends_at DESC",
                                                 :per_page => 20)
   end
@@ -100,12 +100,13 @@ class UserBlocksController < ApplicationController
   def blocks_by
     @user_blocks_pages, @user_blocks = paginate(:user_blocks,
                                                 :include => [:user, :creator, :revoker],
-                                                :conditions => {:creator_id => @this_user.id},
+                                                :conditions => { :creator_id => @this_user.id },
                                                 :order => "user_blocks.ends_at DESC",
                                                 :per_page => 20)
   end
 
   private
+
   ##
   # ensure that there is a "user_block" instance variable
   def lookup_user_block
@@ -126,12 +127,11 @@ class UserBlocksController < ApplicationController
     if !UserBlock::PERIODS.include?(@block_period)
       flash[:error] = t('user_block.filter.block_period')
 
-    elsif @user_block and !@user_block.active?
+    elsif @user_block && !@user_block.active?
       flash[:error] = t('user_block.filter.block_expired')
 
     else
       @valid_params = true
     end
   end
-
 end
index ea50113..a8bbdee 100644 (file)
@@ -28,10 +28,10 @@ class UserController < ApplicationController
     else
       @title = t 'user.terms.title'
 
-      if @user and @user.terms_agreed?
+      if @user && @user.terms_agreed?
         # Already agreed to terms, so just show settings
         redirect_to :action => :account, :display_name => @user.display_name
-      elsif @user.nil? and session[:new_user].nil?
+      elsif @user.nil? && session[:new_user].nil?
         redirect_to :action => :login, :referer => request.fullpath
       end
     end
@@ -57,7 +57,7 @@ class UserController < ApplicationController
         redirect_to t('user.terms.declined')
       end
     elsif @user
-      if !@user.terms_agreed?
+      unless @user.terms_agreed?
         @user.consider_pd = params[:user][:consider_pd]
         @user.terms_agreed = Time.now.getutc
         @user.terms_seen = true
@@ -81,7 +81,7 @@ class UserController < ApplicationController
         @user.languages = http_accept_language.user_preferred_languages
         @user.terms_agreed = Time.now.getutc
         @user.terms_seen = true
-        @user.openid_url = nil if @user.openid_url and @user.openid_url.empty?
+        @user.openid_url = nil if @user.openid_url && @user.openid_url.empty?
 
         if @user.save
           flash[:piwik_goal] = PIWIK["goals"]["signup"] if defined?(PIWIK)
@@ -92,9 +92,9 @@ class UserController < ApplicationController
             uri = URI(session[:referer])
             /map=(.*)\/(.*)\/(.*)/.match(uri.fragment) do |m|
               editor = Rack::Utils.parse_query(uri.query).slice('editor')
-              referer = welcome_path({'zoom' => m[1],
-                                      'lat' => m[2],
-                                      'lon' => m[3]}.merge(editor))
+              referer = welcome_path({ 'zoom' => m[1],
+                                       'lat' => m[2],
+                                       'lon' => m[3] }.merge(editor))
             end
           rescue
             # Use default
@@ -119,9 +119,9 @@ class UserController < ApplicationController
     @title = t 'user.account.title'
     @tokens = @user.oauth_tokens.authorized
 
-    if params[:user] and params[:user][:display_name] and params[:user][:description]
-      if params[:user][:openid_url] and
-         params[:user][:openid_url].length > 0 and
+    if params[:user] && params[:user][:display_name] && params[:user][:description]
+      if params[:user][:openid_url] &&
+         params[:user][:openid_url].length > 0 &&
          params[:user][:openid_url] != @user.openid_url
         # If the OpenID has changed, we want to check that it is a
         # valid OpenID and one the user has control over before saving
@@ -152,7 +152,7 @@ class UserController < ApplicationController
   def lost_password
     @title = t 'user.lost_password.title'
 
-    if params[:user] and params[:user][:email]
+    if params[:user] && params[:user][:email]
       user = User.visible.find_by_email(params[:user][:email])
 
       if user.nil?
@@ -218,7 +218,7 @@ class UserController < ApplicationController
         user.status = "active" if user.email == verified_email
       end
 
-      if @user.openid_url.nil? or @user.invalid?
+      if @user.openid_url.nil? || @user.invalid?
         render :action => 'new'
       else
         session[:new_user] = @user
@@ -275,7 +275,7 @@ class UserController < ApplicationController
   end
 
   def login
-    if params[:username] or using_open_id?
+    if params[:username] || using_open_id?
       session[:referer] ||= params[:referer]
 
       if using_open_id?
@@ -333,7 +333,7 @@ class UserController < ApplicationController
           token = nil
         end
 
-        if token.nil? or token.user != user
+        if token.nil? || token.user != user
           flash[:notice] = t('user.confirm.success')
           redirect_to :action => :login, :referer => referer
         else
@@ -366,7 +366,7 @@ class UserController < ApplicationController
   def confirm_email
     if request.post?
       token = UserToken.find_by_token(params[:confirm_string])
-      if token and token.user.new_email?
+      if token && token.user.new_email?
         @user = token.user
         @user.email = @user.new_email
         @user.new_email = nil
@@ -398,7 +398,7 @@ class UserController < ApplicationController
   def api_gpx_files
     doc = OSM::API.new.get_xml_doc
     @user.traces.each do |trace|
-      doc.root << trace.to_xml_node() if trace.public? or trace.user == @user
+      doc.root << trace.to_xml_node if trace.public? || trace.user == @user
     end
     render :text => doc.to_s, :content_type => "text/xml"
   end
@@ -406,8 +406,8 @@ class UserController < ApplicationController
   def view
     @this_user = User.find_by_display_name(params[:display_name])
 
-    if @this_user and
-       (@this_user.visible? or (@user and @user.administrator?))
+    if @this_user &&
+       (@this_user.visible? || (@user && @user.administrator?))
       @title = @this_user.display_name
     else
       render_unknown_user params[:display_name]
@@ -486,14 +486,14 @@ class UserController < ApplicationController
   # display a list of users matching specified criteria
   def list
     if request.post?
-      ids = params[:user].keys.collect { |id| id.to_i }
+      ids = params[:user].keys.collect(&:to_i)
 
       User.update_all("status = 'confirmed'", :id => ids) if params[:confirm]
       User.update_all("status = 'deleted'", :id => ids) if params[:hide]
 
       redirect_to url_for(:status => params[:status], :ip => params[:ip], :page => params[:page])
     else
-      conditions = Hash.new
+      conditions = {}
       conditions[:status] = params[:status] if params[:status]
       conditions[:creation_ip] = params[:ip] if params[:ip]
 
@@ -504,7 +504,7 @@ class UserController < ApplicationController
     end
   end
 
-private
+  private
 
   ##
   # handle password authentication
@@ -525,7 +525,7 @@ private
   def openid_authentication(openid_url)
     # If we don't appear to have a user for this URL then ask the
     # provider for some extra information to help with signup
-    if openid_url and User.find_by_openid_url(openid_url)
+    if openid_url && User.find_by_openid_url(openid_url)
       required = nil
     else
       required = [:nickname, :email, "http://axschema.org/namePerson/friendly", "http://axschema.org/contact/email"]
@@ -553,8 +553,8 @@ private
           end
         else
           # Guard against not getting any extension data
-          sreg = Hash.new if sreg.nil?
-          ax = Hash.new if ax.nil?
+          sreg = {} if sreg.nil?
+          ax = {} if ax.nil?
 
           # We don't have a user registered to this OpenID, so redirect
           # to the create account page with username and email filled
@@ -585,8 +585,8 @@ private
         # Do we trust the emails this provider returns?
         if openid_email_verified(identity_url)
           # Guard against not getting any extension data
-          sreg = Hash.new if sreg.nil?
-          ax = Hash.new if ax.nil?
+          sreg = {} if sreg.nil?
+          ax = {} if ax.nil?
 
           # Get the verified email
           verified_email = sreg["email"] || ax["http://axschema.org/contact/email"].first
@@ -616,7 +616,7 @@ private
   def openid_expand_url(openid_url)
     if openid_url.nil?
       return nil
-    elsif openid_url.match(/(.*)gmail.com(\/?)$/) or openid_url.match(/(.*)googlemail.com(\/?)$/)
+    elsif openid_url.match(/(.*)gmail.com(\/?)$/) || openid_url.match(/(.*)googlemail.com(\/?)$/)
       # Special case gmail.com as it is potentially a popular OpenID
       # provider and, unlike yahoo.com, where it works automatically, Google
       # have hidden their OpenID endpoint somewhere obscure this making it
@@ -631,8 +631,8 @@ private
   # check if we trust an OpenID provider to return a verified
   # email, so that we can skpi verifying it ourselves
   def openid_email_verified(openid_url)
-    openid_url.match(/https:\/\/www.google.com\/accounts\/o8\/id?(.*)/) or
-    openid_url.match(/https:\/\/me.yahoo.com\/(.*)/)
+    openid_url.match(/https:\/\/www.google.com\/accounts\/o8\/id?(.*)/) ||
+      openid_url.match(/https:\/\/me.yahoo.com\/(.*)/)
   end
 
   ##
@@ -649,7 +649,7 @@ private
     # - If they have a block on them, show them that.
     # - If they were referred to the login, send them back there.
     # - Otherwise, send them to the home page.
-    if REQUIRE_TERMS_SEEN and not user.terms_seen
+    if REQUIRE_TERMS_SEEN && !user.terms_seen
       redirect_to :controller => :user, :action => :terms, :referer => target
     elsif user.blocked_on_view
       redirect_to user.blocked_on_view, :referer => target
@@ -687,7 +687,7 @@ private
     user.display_name = params[:user][:display_name]
     user.new_email = params[:user][:new_email]
 
-    if params[:user][:pass_crypt].length > 0 or params[:user][:pass_crypt_confirmation].length > 0
+    if params[:user][:pass_crypt].length > 0 || params[:user][:pass_crypt_confirmation].length > 0
       user.pass_crypt = params[:user][:pass_crypt]
       user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
     end
@@ -725,7 +725,7 @@ private
     if user.save
       set_locale
 
-      if user.new_email.blank? or user.new_email == user.email
+      if user.new_email.blank? || user.new_email == user.email
         flash.now[:notice] = t 'user.account.flash update success'
       else
         user.email = user.new_email
@@ -752,7 +752,7 @@ private
   # require that the user is a administrator, or fill out a helpful error message
   # and return them to the user page.
   def require_administrator
-    if @user and not @user.administrator?
+    if @user && !@user.administrator?
       flash[:error] = t('user.filter.not_an_administrator')
 
       if params[:display_name]
@@ -760,7 +760,7 @@ private
       else
         redirect_to :controller => 'user', :action => 'login', :referer => request.fullpath
       end
-    elsif not @user
+    elsif !@user
       redirect_to :controller => 'user', :action => 'login', :referer => request.fullpath
     end
   end
@@ -817,6 +817,6 @@ private
       render :action => 'blocked'
     end
 
-    not blocked
+    !blocked
   end
 end
index 7d6fc8e..03cb8f1 100644 (file)
@@ -33,7 +33,7 @@ class UserPreferenceController < ApplicationController
 
   # update the entire set of preferences
   def update
-    old_preferences = @user.preferences.reduce({}) do |preferences,preference|
+    old_preferences = @user.preferences.reduce({}) do |preferences, preference|
       preferences[preference.k] = preference
       preferences
     end
@@ -46,7 +46,7 @@ class UserPreferenceController < ApplicationController
       if preference = old_preferences.delete(pt["k"])
         preference.v = pt["v"]
       elsif new_preferences.include?(pt["k"])
-        raise OSM::APIDuplicatePreferenceError.new(pt["k"])
+        fail OSM::APIDuplicatePreferenceError.new(pt["k"])
       else
         preference = @user.preferences.build(:k => pt["k"], :v => pt["v"])
       end
@@ -54,13 +54,9 @@ class UserPreferenceController < ApplicationController
       new_preferences[preference.k] = preference
     end
 
-    old_preferences.each_value do |preference|
-      preference.delete
-    end
+    old_preferences.each_value(&:delete)
 
-    new_preferences.each_value do |preference|
-      preference.save!
-    end
+    new_preferences.each_value(&:save!)
 
     render :text => "", :content_type => "text/plain"
   end
index 2c83971..4316c5e 100644 (file)
@@ -15,17 +15,18 @@ class UserRolesController < ApplicationController
   end
 
   def revoke
-    UserRole.delete_all({:user_id => @this_user.id, :role => @role})
+    UserRole.delete_all(:user_id => @this_user.id, :role => @role)
     redirect_to :controller => 'user', :action => 'view', :display_name => @this_user.display_name
   end
 
   private
+
   ##
   # require that the user is an administrator, or fill out a helpful error message
   # and return them to theuser page.
   def require_administrator
     unless @user.administrator?
-      flash[:error] = t'user_role.filter.not_an_administrator'
+      flash[:error] = t 'user_role.filter.not_an_administrator'
       redirect_to :controller => 'user', :action => 'view', :display_name => @this_user.display_name
     end
   end
index 3291ce8..0c83b5b 100644 (file)
@@ -39,7 +39,7 @@ class WayController < ApplicationController
     way = Way.find(params[:id])
     new_way = Way.from_xml(request.raw_post)
 
-    if new_way and new_way.id == way.id
+    if new_way && new_way.id == way.id
       way.update_from(new_way, @user)
       render :text => way.version.to_s, :content_type => "text/plain"
     else
@@ -52,7 +52,7 @@ class WayController < ApplicationController
     way = Way.find(params[:id])
     new_way = Way.from_xml(request.raw_post)
 
-    if new_way and new_way.id == way.id
+    if new_way && new_way.id == way.id
       way.delete_with_history!(new_way, @user)
       render :text => way.version.to_s, :content_type => "text/plain"
     else
@@ -84,14 +84,14 @@ class WayController < ApplicationController
   end
 
   def ways
-    if not params['ways']
-      raise OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]")
+    unless params['ways']
+      fail OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]")
     end
 
-    ids = params['ways'].split(',').collect { |w| w.to_i }
+    ids = params['ways'].split(',').collect(&:to_i)
 
     if ids.length == 0
-      raise OSM::APIBadUserInput.new("No ways were given to search for")
+      fail OSM::APIBadUserInput.new("No ways were given to search for")
     end
 
     doc = OSM::API.new.get_xml_doc
index 5266bf6..b4a0ad1 100644 (file)
@@ -10,11 +10,11 @@ module ApplicationHelper
   end
 
   def rss_link_to(*args)
-    return link_to(image_tag("RSS.png", :size => "16x16", :border => 0), Hash[*args], { :class => "rsssmall" });
+    link_to(image_tag("RSS.png", :size => "16x16", :border => 0), Hash[*args], :class => "rsssmall")
   end
 
   def atom_link_to(*args)
-    return link_to(image_tag("RSS.png", :size => "16x16", :border => 0), Hash[*args], { :class => "rsssmall" });
+    link_to(image_tag("RSS.png", :size => "16x16", :border => 0), Hash[*args], :class => "rsssmall")
   end
 
   def style_rules
@@ -25,10 +25,10 @@ module ApplicationHelper
     css << ".hide_if_logged_in { display: none !important }" if @user
     css << ".hide_if_user_#{@user.id} { display: none !important }" if @user
     css << ".show_if_user_#{@user.id} { display: inline !important }" if @user
-    css << ".hide_unless_administrator { display: none !important }" unless @user and @user.administrator?
-    css << ".hide_unless_moderator { display: none !important }" unless @user and @user.moderator?
+    css << ".hide_unless_administrator { display: none !important }" unless @user && @user.administrator?
+    css << ".hide_unless_moderator { display: none !important }" unless @user && @user.moderator?
 
-    return content_tag(:style, css, :type => "text/css")
+    content_tag(:style, css, :type => "text/css")
   end
 
   def if_logged_in(tag = :div, &block)
@@ -58,7 +58,7 @@ module ApplicationHelper
   end
 
   def richtext_area(object_name, method, options = {})
-    id = "#{object_name.to_s}_#{method.to_s}"
+    id = "#{object_name}_#{method}"
     format = options.delete(:format) || "markdown"
 
     content_tag(:div, :id => "#{id}_container", :class => "richtext_container") do
index f4b332f..45240e0 100644 (file)
@@ -1,5 +1,5 @@
 module BrowseHelper
-  def printable_name(object, version=false)
+  def printable_name(object, version = false)
     if object.id.is_a?(Array)
       id = object.id[0]
     else
@@ -15,14 +15,14 @@ module BrowseHelper
     unless object.redacted?
       locale = I18n.locale.to_s
 
-      while locale =~ /-[^-]+/ and not object.tags.include? "name:#{I18n.locale}"
+      while locale =~ /-[^-]+/ && !object.tags.include?("name:#{I18n.locale}")
         locale = locale.sub(/-[^-]+/, "")
       end
 
       if object.tags.include? "name:#{locale}"
-        name = t 'printable_name.with_name_html', :name => content_tag(:bdi, object.tags["name:#{locale}"].to_s ), :id => content_tag(:bdi, name)
+        name = t 'printable_name.with_name_html', :name => content_tag(:bdi, object.tags["name:#{locale}"].to_s), :id => content_tag(:bdi, name)
       elsif object.tags.include? 'name'
-        name = t 'printable_name.with_name_html', :name => content_tag(:bdi, object.tags['name'].to_s ), :id => content_tag(:bdi, name)
+        name = t 'printable_name.with_name_html', :name => content_tag(:bdi, object.tags['name'].to_s), :id => content_tag(:bdi, name)
       end
     end
 
@@ -30,7 +30,7 @@ module BrowseHelper
   end
 
   def link_class(type, object)
-    classes = [ type ]
+    classes = [type]
 
     if object.redacted?
       classes << "deleted"
@@ -46,7 +46,7 @@ module BrowseHelper
     if object.redacted?
       ""
     else
-      h(icon_tags(object).map { |k,v| k + '=' + v }.to_sentence)
+      h(icon_tags(object).map { |k, v| k + '=' + v }.to_sentence)
     end
   end
 
@@ -84,15 +84,12 @@ module BrowseHelper
     end
   end
 
-private
+  private
 
-  ICON_TAGS = [
-    "aeroway", "amenity", "barrier", "building", "highway", "historic", "landuse",
-    "leisure", "man_made", "natural", "railway", "shop", "tourism", "waterway"
-  ]
+  ICON_TAGS = %w(aeroway amenity barrier building highway historic landuse leisure man_made natural railway shop tourism waterway)
 
   def icon_tags(object)
-    object.tags.find_all { |k,v| ICON_TAGS.include? k }.sort
+    object.tags.find_all { |k, _v| ICON_TAGS.include? k }.sort
   end
 
   def wiki_link(type, lookup)
@@ -110,7 +107,7 @@ private
       url = "http://wiki.openstreetmap.org/wiki/#{page}?uselang=#{locale}"
     end
 
-    return url
+    url
   end
 
   def wikipedia_link(key, value)
@@ -136,7 +133,7 @@ private
       return nil
     end
 
-    if value =~ /^([^#]*)(#.*)/ then
+    if value =~ /^([^#]*)(#.*)/
       # Contains a reference to a section of the wikipedia article
       # Must break it up to correctly build the url
       value = $1
@@ -145,23 +142,23 @@ private
       section = ""
     end
 
-    return {
+    {
       :url => "http://#{lang}.wikipedia.org/wiki/#{value}?uselang=#{I18n.locale}#{section}",
       :title => value + section
     }
   end
 
   def wikidata_link(key, value)
-    if key == "wikidata" and value =~ /^[Qq][1-9][0-9]*$/
+    if key == "wikidata" && value =~ /^[Qq][1-9][0-9]*$/
       return {
         :url => "//www.wikidata.org/wiki/#{value}?uselang=#{I18n.locale}",
         :title => value
       }
     end
-    return nil
+    nil
   end
 
-  def telephone_link(key, value)
+  def telephone_link(_key, value)
     # does it look like a phone number? eg "+1 (234) 567-8901 " ?
     return nil unless value =~ /^\s*\+[\d\s\(\)\/\.-]{6,25}\s*$/
 
@@ -169,6 +166,6 @@ private
     # "+1 (234) 567-8901 " -> "+1(234)567-8901"
     valueNoWhitespace = value.gsub(/\s+/, '')
 
-    return "tel:#{valueNoWhitespace}"
+    "tel:#{valueNoWhitespace}"
   end
 end
index 04f437a..f066310 100644 (file)
@@ -2,21 +2,21 @@ module GeocoderHelper
   def result_to_html(result)
     html_options = { :class => "set_position", :data => {} }
 
-    if result[:type] and result[:id]
+    if result[:type] && result[:id]
       url = url_for(:controller => :browse, :action => result[:type], :id => result[:id])
-    elsif result[:min_lon] and result[:min_lat] and result[:max_lon] and result[:max_lat]
+    elsif result[:min_lon] && result[:min_lat] && result[:max_lon] && result[:max_lat]
       url = "/?bbox=#{result[:min_lon]},#{result[:min_lat]},#{result[:max_lon]},#{result[:max_lat]}"
     else
       url = "/#map=#{result[:zoom]}/#{result[:lat]}/#{result[:lon]}"
     end
 
-    result.each do |key,value|
+    result.each do |key, value|
       html_options[:data][key.to_s.tr('_', '-')] = value
     end
 
     html = ""
     html << result[:prefix] if result[:prefix]
-    html << " " if result[:prefix] and result[:name]
+    html << " " if result[:prefix] && result[:name]
     html << link_to(result[:name], url, html_options) if result[:name]
     html << result[:suffix] if result[:suffix]
     html.html_safe
index e0eef26..baf5a27 100644 (file)
@@ -2,14 +2,14 @@ module NoteHelper
   def note_event(event, at, by)
     if by.nil?
       I18n.t("browse.note." + event + "_by_anonymous",
-        :when => friendly_date(at),
-        :exact_time => l(at)
+             :when => friendly_date(at),
+             :exact_time => l(at)
       ).html_safe
     else
       I18n.t("browse.note." + event + "_by",
-        :when => friendly_date(at),
-        :exact_time => l(at),
-        :user => note_author(by)
+             :when => friendly_date(at),
+             :exact_time => l(at),
+             :user => note_author(by)
       ).html_safe
     end
   end
@@ -18,8 +18,7 @@ module NoteHelper
     if author.nil?
       ""
     else
-      link_to h(author.display_name), link_options.merge({:controller => "user", :action => "view", :display_name => author.display_name})
+      link_to h(author.display_name), link_options.merge(:controller => "user", :action => "view", :display_name => author.display_name)
     end
   end
-
 end
index ba37a13..b5bf306 100644 (file)
@@ -62,7 +62,7 @@ module UserHelper
   # See http://en.gravatar.com/site/implement/images/ for details.
   def user_gravatar_url(user, options = {})
     size = options[:size] || 100
-    hash = Digest::MD5::hexdigest(user.email.downcase)
+    hash = Digest::MD5.hexdigest(user.email.downcase)
     default_image_url = image_url("users/images/large.png")
     url = "#{request.protocol}www.gravatar.com/avatar/#{hash}.jpg?s=#{size}&d=#{u(default_image_url)}"
   end
index 37fbf86..adc5b22 100644 (file)
@@ -1,10 +1,10 @@
 module UserRolesHelper
   def role_icons(user)
-    UserRole::ALL_ROLES.reduce("".html_safe) { |s,r| s + " " + role_icon(user, r) }
+    UserRole::ALL_ROLES.reduce("".html_safe) { |s, r| s + " " + role_icon(user, r) }
   end
 
   def role_icon(user, role)
-    if @user and @user.administrator?
+    if @user && @user.administrator?
       if user.has_role?(role)
         image = "roles/#{role}.png"
         alt = t("user.view.role.revoke.#{role}")
index 6e48c99..2bf2267 100644 (file)
@@ -8,7 +8,7 @@ class AccessToken < OauthToken
 
   before_create :set_authorized_at
 
-protected
+  protected
 
   def set_authorized_at
     self.authorized_at = Time.now
index e2f163e..0c89968 100644 (file)
@@ -8,14 +8,14 @@ class Acl < ActiveRecord::Base
   end
 
   def self.no_account_creation(address, domain = nil)
-    self.match(address, domain).where(:k => "no_account_creation").exists?
+    match(address, domain).where(:k => "no_account_creation").exists?
   end
 
   def self.no_note_comment(address, domain = nil)
-    self.match(address, domain).where(:k => "no_note_comment").exists?
+    match(address, domain).where(:k => "no_note_comment").exists?
   end
 
   def self.no_trace_download(address, domain = nil)
-    self.match(address, domain).where(:k => "no_trace_download").exists?
+    match(address, domain).where(:k => "no_trace_download").exists?
   end
 end
index 926e82a..2001c70 100644 (file)
@@ -46,7 +46,7 @@ class Changeset < ActiveRecord::Base
     # note that this may not be a hard limit - due to timing changes and
     # concurrency it is possible that some changesets may be slightly
     # longer than strictly allowed or have slightly more changes in them.
-    return ((closed_at > Time.now.getutc) and (num_changes <= MAX_ELEMENTS))
+    ((closed_at > Time.now.getutc) && (num_changes <= MAX_ELEMENTS))
   end
 
   def set_closed_time_now
@@ -55,21 +55,19 @@ class Changeset < ActiveRecord::Base
     end
   end
 
-  def self.from_xml(xml, create=false)
-    begin
-      p = XML::Parser.string(xml, :options => XML::Parser::Options::NOERROR)
-      doc = p.parse
+  def self.from_xml(xml, create = false)
+    p = XML::Parser.string(xml, :options => XML::Parser::Options::NOERROR)
+    doc = p.parse
 
-      doc.find('//osm/changeset').each do |pt|
-        return Changeset.from_xml_node(pt, create)
-      end
-      raise OSM::APIBadXMLError.new("changeset", xml, "XML doesn't contain an osm/changeset element.")
-    rescue LibXML::XML::Error, ArgumentError => ex
-      raise OSM::APIBadXMLError.new("changeset", xml, ex.message)
+    doc.find('//osm/changeset').each do |pt|
+      return Changeset.from_xml_node(pt, create)
     end
+    fail OSM::APIBadXMLError.new("changeset", xml, "XML doesn't contain an osm/changeset element.")
+  rescue LibXML::XML::Error, ArgumentError => ex
+    raise OSM::APIBadXMLError.new("changeset", xml, ex.message)
   end
 
-  def self.from_xml_node(pt, create=false)
+  def self.from_xml_node(pt, create = false)
     cs = Changeset.new
     if create
       cs.created_at = Time.now.getutc
@@ -81,12 +79,12 @@ class Changeset < ActiveRecord::Base
     end
 
     pt.find('tag').each do |tag|
-      raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag['k'].nil?
-      raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag['v'].nil?
+      fail OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag['k'].nil?
+      fail OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag['v'].nil?
       cs.add_tag_keyval(tag['k'], tag['v'])
     end
 
-    return cs
+    cs
   end
 
   ##
@@ -122,29 +120,27 @@ class Changeset < ActiveRecord::Base
   end
 
   def tags_as_hash
-    return tags
+    tags
   end
 
   def tags
     unless @tags
       @tags = {}
-      self.changeset_tags.each do |tag|
+      changeset_tags.each do |tag|
         @tags[tag.k] = tag.v
       end
     end
     @tags
   end
 
-  def tags=(t)
-    @tags = t
-  end
+  attr_writer :tags
 
   def add_tag_keyval(k, v)
-    @tags = Hash.new unless @tags
+    @tags = {} unless @tags
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.
-    raise OSM::APIDuplicateTagsError.new("changeset", self.id, k) if @tags.include? k
+    fail OSM::APIDuplicateTagsError.new("changeset", id, k) if @tags.include? k
 
     @tags[k] = v
   end
@@ -156,11 +152,11 @@ class Changeset < ActiveRecord::Base
       self.save!
 
       tags = self.tags
-      ChangesetTag.delete_all(:changeset_id => self.id)
+      ChangesetTag.delete_all(:changeset_id => id)
 
-      tags.each do |k,v|
+      tags.each do |k, v|
         tag = ChangesetTag.new
-        tag.changeset_id = self.id
+        tag.changeset_id = id
         tag.k = k
         tag.v = v
         tag.save!
@@ -185,46 +181,46 @@ class Changeset < ActiveRecord::Base
   def to_xml(include_discussion = false)
     doc = OSM::API.new.get_xml_doc
     doc.root << to_xml_node(nil, include_discussion)
-    return doc
+    doc
   end
 
   def to_xml_node(user_display_name_cache = nil, include_discussion = false)
     el1 = XML::Node.new 'changeset'
-    el1['id'] = self.id.to_s
+    el1['id'] = id.to_s
 
     user_display_name_cache = {} if user_display_name_cache.nil?
 
-    if user_display_name_cache and user_display_name_cache.key?(self.user_id)
+    if user_display_name_cache && user_display_name_cache.key?(user_id)
       # use the cache if available
-    elsif self.user.data_public?
-      user_display_name_cache[self.user_id] = self.user.display_name
+    elsif user.data_public?
+      user_display_name_cache[user_id] = user.display_name
     else
-      user_display_name_cache[self.user_id] = nil
+      user_display_name_cache[user_id] = nil
     end
 
-    el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil?
-    el1['uid'] = self.user_id.to_s if self.user.data_public?
+    el1['user'] = user_display_name_cache[user_id] unless user_display_name_cache[user_id].nil?
+    el1['uid'] = user_id.to_s if user.data_public?
 
-    self.tags.each do |k,v|
+    tags.each do |k, v|
       el2 = XML::Node.new('tag')
       el2['k'] = k.to_s
       el2['v'] = v.to_s
       el1 << el2
     end
 
-    el1['created_at'] = self.created_at.xmlschema
-    el1['closed_at'] = self.closed_at.xmlschema unless is_open?
+    el1['created_at'] = created_at.xmlschema
+    el1['closed_at'] = closed_at.xmlschema unless is_open?
     el1['open'] = is_open?.to_s
 
     if bbox.complete?
       bbox.to_unscaled.add_bounds_to(el1, '_')
     end
 
-    el1['comments_count'] = self.comments.count.to_s
+    el1['comments_count'] = comments.count.to_s
 
     if include_discussion
       el2 = XML::Node.new('discussion')
-      self.comments.includes(:author).each do |comment|
+      comments.includes(:author).each do |comment|
         el3 = XML::Node.new('comment')
         el3['date'] = comment.created_at.xmlschema
         el3['uid'] = comment.author.id.to_s if comment.author.data_public?
@@ -241,7 +237,7 @@ class Changeset < ActiveRecord::Base
     # they are just structures for tagging. to get the osmChange of a
     # changeset, see the download method of the controller.
 
-    return el1
+    el1
   end
 
   ##
@@ -250,13 +246,13 @@ class Changeset < ActiveRecord::Base
   # bounding box, only the tags of the changeset.
   def update_from(other, user)
     # ensure that only the user who opened the changeset may modify it.
-    unless user.id == self.user_id
-      raise OSM::APIUserChangesetMismatchError.new
+    unless user.id == user_id
+      fail OSM::APIUserChangesetMismatchError.new
     end
 
     # can't change a closed changeset
     unless is_open?
-      raise OSM::APIChangesetAlreadyClosedError.new(self)
+      fail OSM::APIChangesetAlreadyClosedError.new(self)
     end
 
     # copy the other's tags
index 1f037d6..02f68a2 100644 (file)
@@ -8,7 +8,7 @@ class ChangesetComment < ActiveRecord::Base
   validates_associated :changeset
   validates_presence_of :author
   validates_associated :author
-  validates :visible, :inclusion => { :in => [true,false] }
+  validates :visible, :inclusion => { :in => [true, false] }
 
   # Return the comment text
   def body
index fd38262..6195dfc 100644 (file)
@@ -10,8 +10,8 @@ class ClientApplication < ActiveRecord::Base
   validates_presence_of :name, :url, :key, :secret
   validates_uniqueness_of :key
   validates_format_of :url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i
-  validates_format_of :support_url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank=>true
-  validates_format_of :callback_url, :with => /\A[a-z][a-z0-9.+-]*:\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank=>true
+  validates_format_of :support_url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank => true
+  validates_format_of :callback_url, :with => /\A[a-z][a-z0-9.+-]*:\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank => true
 
   before_validation :generate_keys, :on => :create
 
@@ -27,14 +27,12 @@ class ClientApplication < ActiveRecord::Base
   end
 
   def self.verify_request(request, options = {}, &block)
-    begin
-      signature = OAuth::Signature.build(request, options, &block)
-      return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp)
-      value = signature.verify
-      value
-    rescue OAuth::Signature::UnknownSignatureMethod => e
-      false
-    end
+    signature = OAuth::Signature.build(request, options, &block)
+    return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp)
+    value = signature.verify
+    value
+  rescue OAuth::Signature::UnknownSignatureMethod => e
+    false
   end
 
   def self.all_permissions
@@ -49,8 +47,8 @@ class ClientApplication < ActiveRecord::Base
     @oauth_client ||= OAuth::Consumer.new(key, secret)
   end
 
-  def create_request_token(params={})
-    params = { :client_application => self, :callback_url => self.token_callback_url }
+  def create_request_token(params = {})
+    params = { :client_application => self, :callback_url => token_callback_url }
     permissions.each do |p|
       params[p] = true
     end
@@ -76,7 +74,7 @@ class ClientApplication < ActiveRecord::Base
     ClientApplication.all_permissions.select { |p| self[p] }
   end
 
-protected
+  protected
 
   # this is the set of permissions that the client can ask for. clients
   # have to say up-front what permissions they want and when users sign up they
@@ -86,7 +84,7 @@ protected
                  :allow_write_notes]
 
   def generate_keys
-    self.key = OAuth::Helper.generate_key(40)[0,40]
-    self.secret = OAuth::Helper.generate_key(40)[0,40]
+    self.key = OAuth::Helper.generate_key(40)[0, 40]
+    self.secret = OAuth::Helper.generate_key(40)[0, 40]
   end
 end
index 5ace3d1..b38240d 100644 (file)
@@ -20,7 +20,7 @@ class DiaryComment < ActiveRecord::Base
     md5.hexdigest
   end
 
-private
+  private
 
   def spam_check
     user.spam_check
index c0fbac5..a66eb59 100644 (file)
@@ -3,17 +3,17 @@ class DiaryEntry < ActiveRecord::Base
   belongs_to :language, :foreign_key => 'language_code'
 
   has_many :comments, -> { order(:id).preload(:user) }, :class_name => "DiaryComment"
-  has_many :visible_comments, -> { joins(:user).where(:visible => true, :users => { :status => ["active", "confirmed"] }).order(:id) }, :class_name => "DiaryComment"
+  has_many :visible_comments, -> { joins(:user).where(:visible => true, :users => { :status => %w(active confirmed) }).order(:id) }, :class_name => "DiaryComment"
 
   scope :visible, -> { where(:visible => true) }
 
   validates_presence_of :title, :body
   validates_length_of :title, :within => 1..255
-  #validates_length_of :language, :within => 2..5, :allow_nil => false
+  # validates_length_of :language, :within => 2..5, :allow_nil => false
   validates_numericality_of :latitude, :allow_nil => true,
-                            :greater_than_or_equal_to => -90, :less_than_or_equal_to => 90
+                                       :greater_than_or_equal_to => -90, :less_than_or_equal_to => 90
   validates_numericality_of :longitude, :allow_nil => true,
-                            :greater_than_or_equal_to => -180, :less_than_or_equal_to => 180
+                                        :greater_than_or_equal_to => -180, :less_than_or_equal_to => 180
   validates_associated :language
 
   after_save :spam_check
@@ -22,7 +22,7 @@ class DiaryEntry < ActiveRecord::Base
     RichText.new(read_attribute(:body_format), read_attribute(:body))
   end
 
-private
+  private
 
   def spam_check
     user.spam_check
index fdcb7ba..8ccb00a 100644 (file)
@@ -5,7 +5,7 @@ class Language < ActiveRecord::Base
 
   def self.load(file)
     Language.transaction do
-      YAML.load(File.read(file)).each do |k,v|
+      YAML.load(File.read(file)).each do |k, v|
         begin
           Language.update(k, :english_name => v["english"], :native_name => v["native"])
         rescue ActiveRecord::RecordNotFound
index b05d005..8ca1dc5 100644 (file)
@@ -6,7 +6,7 @@ class Message < ActiveRecord::Base
 
   validates_presence_of :title, :body, :sent_on, :sender, :recipient
   validates_length_of :title, :within => 1..255
-  validates_inclusion_of :message_read, :in => [ true, false ]
+  validates_inclusion_of :message_read, :in => [true, false]
   validates_as_utf8 :title
 
   def self.from_mail(mail, from, to)
@@ -16,7 +16,7 @@ class Message < ActiveRecord::Base
       elsif mail.html_part
         body = HTMLEntities.new.decode(Sanitize.clean(mail.html_part.decoded))
       end
-    elsif mail.text? and mail.sub_type == "html"
+    elsif mail.text? && mail.sub_type == "html"
       body = HTMLEntities.new.decode(Sanitize.clean(mail.decoded))
     else
       body = mail.decoded
index 755dc82..7d2219d 100644 (file)
@@ -18,15 +18,15 @@ class Node < ActiveRecord::Base
   has_many :node_tags
 
   has_many :old_way_nodes
-  has_many :ways_via_history, :class_name=> "Way", :through => :old_way_nodes, :source => :way
+  has_many :ways_via_history, :class_name => "Way", :through => :old_way_nodes, :source => :way
 
   has_many :containing_relation_members, :class_name => "RelationMember", :as => :member
   has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
 
   validates_presence_of :id, :on => :update
-  validates_presence_of :timestamp,:version,  :changeset_id
+  validates_presence_of :timestamp, :version,  :changeset_id
   validates_uniqueness_of :id
-  validates_inclusion_of :visible, :in => [ true, false ]
+  validates_inclusion_of :visible, :in => [true, false]
   validates_numericality_of :latitude, :longitude, :changeset_id, :version, :integer_only => true
   validates_numericality_of :id, :on => :update, :integer_only => true
   validate :validate_position
@@ -41,42 +41,40 @@ class Node < ActiveRecord::Base
   end
 
   # Read in xml as text and return it's Node object representation
-  def self.from_xml(xml, create=false)
-    begin
-      p = XML::Parser.string(xml)
-      doc = p.parse
+  def self.from_xml(xml, create = false)
+    p = XML::Parser.string(xml)
+    doc = p.parse
 
-      doc.find('//osm/node').each do |pt|
-        return Node.from_xml_node(pt, create)
-      end
-      raise OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/node element.")
-    rescue LibXML::XML::Error, ArgumentError => ex
-      raise OSM::APIBadXMLError.new("node", xml, ex.message)
+    doc.find('//osm/node').each do |pt|
+      return Node.from_xml_node(pt, create)
     end
+    fail OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/node element.")
+  rescue LibXML::XML::Error, ArgumentError => ex
+    raise OSM::APIBadXMLError.new("node", xml, ex.message)
   end
 
-  def self.from_xml_node(pt, create=false)
+  def self.from_xml_node(pt, create = false)
     node = Node.new
 
-    raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt['lat'].nil?
-    raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt['lon'].nil?
+    fail OSM::APIBadXMLError.new("node", pt, "lat missing") if pt['lat'].nil?
+    fail OSM::APIBadXMLError.new("node", pt, "lon missing") if pt['lon'].nil?
     node.lat = OSM.parse_float(pt['lat'], OSM::APIBadXMLError, "node", pt, "lat not a number")
     node.lon = OSM.parse_float(pt['lon'], OSM::APIBadXMLError, "node", pt, "lon not a number")
-    raise OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt['changeset'].nil?
+    fail OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt['changeset'].nil?
     node.changeset_id = pt['changeset'].to_i
 
-    raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world?
+    fail OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world?
 
     # version must be present unless creating
-    raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create or not pt['version'].nil?
+    fail OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt['version'].nil?
     node.version = create ? 0 : pt['version'].to_i
 
     unless create
-      raise OSM::APIBadXMLError.new("node", pt, "ID is required when updating.") if pt['id'].nil?
+      fail OSM::APIBadXMLError.new("node", pt, "ID is required when updating.") if pt['id'].nil?
       node.id = pt['id'].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of node cannot be zero when updating.") if node.id == 0
+      fail OSM::APIBadUserInput.new("ID of node cannot be zero when updating.") if node.id == 0
     end
 
     # We don't care about the time, as it is explicitly set on create/update/delete
@@ -85,16 +83,16 @@ class Node < ActiveRecord::Base
     node.visible = true
 
     # Start with no tags
-    node.tags = Hash.new
+    node.tags = {}
 
     # Add in any tags from the XML
     pt.find('tag').each do |tag|
-      raise OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag['k'].nil?
-      raise OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag['v'].nil?
-      node.add_tag_key_val(tag['k'],tag['v'])
+      fail OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag['k'].nil?
+      fail OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag['v'].nil?
+      node.add_tag_key_val(tag['k'], tag['v'])
     end
 
-    return node
+    node
   end
 
   ##
@@ -106,8 +104,8 @@ class Node < ActiveRecord::Base
 
   # Should probably be renamed delete_from to come in line with update
   def delete_with_history!(new_node, user)
-    unless self.visible
-      raise OSM::APIAlreadyDeletedError.new("node", new_node.id)
+    unless visible
+      fail OSM::APIAlreadyDeletedError.new("node", new_node.id)
     end
 
     # need to start the transaction here, so that the database can
@@ -117,10 +115,10 @@ class Node < ActiveRecord::Base
       self.lock!
       check_consistency(self, new_node, user)
       ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id)
-      raise OSM::APIPreconditionFailedError.new("Node #{self.id} is still used by ways #{ways.collect { |w| w.id }.join(",")}.") unless ways.empty?
+      fail OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(",")}.") unless ways.empty?
 
       rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Node", :member_id => id }).order(:id)
-      raise OSM::APIPreconditionFailedError.new("Node #{self.id} is still used by relations #{rels.collect { |r| r.id }.join(",")}.") unless rels.empty?
+      fail OSM::APIPreconditionFailedError.new("Node #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty?
 
       self.changeset_id = new_node.changeset_id
       self.tags = {}
@@ -143,7 +141,7 @@ class Node < ActiveRecord::Base
       self.changeset = new_node.changeset
 
       # update changeset bbox with *old* position first
-      changeset.update_bbox!(bbox);
+      changeset.update_bbox!(bbox)
 
       # FIXME logic needs to be double checked
       self.latitude = new_node.latitude
@@ -152,7 +150,7 @@ class Node < ActiveRecord::Base
       self.visible = true
 
       # update changeset bbox with *new* position
-      changeset.update_bbox!(bbox);
+      changeset.update_bbox!(bbox)
 
       save_with_history!
     end
@@ -171,44 +169,42 @@ class Node < ActiveRecord::Base
 
   def to_xml
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
-    return doc
+    doc.root << to_xml_node
+    doc
   end
 
   def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
     el = XML::Node.new 'node'
-    el['id'] = self.id.to_s
+    el['id'] = id.to_s
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
     if self.visible?
-      el['lat'] = self.lat.to_s
-      el['lon'] = self.lon.to_s
+      el['lat'] = lat.to_s
+      el['lon'] = lon.to_s
     end
 
-    add_tags_to_xml_node(el, self.node_tags)
+    add_tags_to_xml_node(el, node_tags)
 
-    return el
+    el
   end
 
   def tags_as_hash
-    return tags
+    tags
   end
 
   def tags
-    @tags ||= Hash[self.node_tags.collect { |t| [t.k, t.v] }]
+    @tags ||= Hash[node_tags.collect { |t| [t.k, t.v] }]
   end
 
-  def tags=(t)
-    @tags = t
-  end
+  attr_writer :tags
 
-  def add_tag_key_val(k,v)
-    @tags = Hash.new unless @tags
+  def add_tag_key_val(k, v)
+    @tags = {} unless @tags
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.
-    raise OSM::APIDuplicateTagsError.new("node", self.id, k) if @tags.include? k
+    fail OSM::APIDuplicateTagsError.new("node", id, k) if @tags.include? k
 
     @tags[k] = v
   end
@@ -223,7 +219,7 @@ class Node < ActiveRecord::Base
   ##
   # dummy method to make the interfaces of node, way and relation
   # more consistent.
-  def fix_placeholders!(id_map, placeholder_id = nil)
+  def fix_placeholders!(_id_map, _placeholder_id = nil)
     # nodes don't refer to anything, so there is nothing to do here
   end
 
@@ -238,10 +234,10 @@ class Node < ActiveRecord::Base
 
       # Create a NodeTag
       tags = self.tags
-      NodeTag.delete_all(:node_id => self.id)
-      tags.each do |k,v|
+      NodeTag.delete_all(:node_id => id)
+      tags.each do |k, v|
         tag = NodeTag.new
-        tag.node_id = self.id
+        tag.node_id = id
         tag.k = k
         tag.v = v
         tag.save!
@@ -259,5 +255,4 @@ class Node < ActiveRecord::Base
       changeset.save!
     end
   end
-
 end
index 27357e5..f70e4f3 100644 (file)
@@ -8,7 +8,7 @@ class Note < ActiveRecord::Base
   validates_numericality_of :latitude, :only_integer => true
   validates_numericality_of :longitude, :only_integer => true
   validates_presence_of :closed_at if :status == "closed"
-  validates_inclusion_of :status, :in => ["open", "closed", "hidden"]
+  validates_inclusion_of :status, :in => %w(open closed hidden)
   validate :validate_position
 
   scope :visible, -> { where("status != 'hidden'") }
@@ -25,14 +25,14 @@ class Note < ActiveRecord::Base
   def close
     self.status = "closed"
     self.closed_at = Time.now.getutc
-    self.save
+    save
   end
 
   # Reopen a note
   def reopen
     self.status = "open"
     self.closed_at = nil
-    self.save
+    save
   end
 
   # Check if a note is visible
@@ -42,20 +42,20 @@ class Note < ActiveRecord::Base
 
   # Check if a note is closed
   def closed?
-    not closed_at.nil?
+    !closed_at.nil?
   end
 
   # Return the author object, derived from the first comment
   def author
-    self.comments.first.author
+    comments.first.author
   end
 
   # Return the author IP address, derived from the first comment
   def author_ip
-    self.comments.first.author_ip
+    comments.first.author_ip
   end
 
-private
+  private
 
   # Fill in default values for new notes
   def set_defaults
index dd91a95..1920648 100644 (file)
@@ -8,7 +8,7 @@ class NoteComment < ActiveRecord::Base
   validates_associated :note
   validates_presence_of :visible
   validates_associated :author
-  validates_inclusion_of :event, :in => [ "opened", "closed", "reopened", "commented", "hidden" ]
+  validates_inclusion_of :event, :in => %w(opened closed reopened commented hidden)
   validates_format_of :body, :with => /\A[^\x00-\x08\x0b-\x0c\x0e-\x1f\x7f\ufffe\uffff]*\z/
 
   # Return the comment text
index b1a94a7..be7b8d1 100644 (file)
@@ -166,7 +166,7 @@ class Notifier < ActionMailer::Base
     end
   end
 
-private
+  private
 
   def with_recipient_locale(recipient)
     old_locale = I18n.locale
@@ -181,8 +181,8 @@ private
   end
 
   def from_address(name, type, id, digest)
-    if Object.const_defined?(:MESSAGES_DOMAIN) and domain = MESSAGES_DOMAIN
-      "#{name} <#{type}-#{id}-#{digest[0,6]}@#{domain}>"
+    if Object.const_defined?(:MESSAGES_DOMAIN) && domain = MESSAGES_DOMAIN
+      "#{name} <#{type}-#{id}-#{digest[0, 6]}@#{domain}>"
     else
       EMAIL_FROM
     end
index 9c28d88..6dd421a 100644 (file)
@@ -1,8 +1,8 @@
 class Oauth2Token < AccessToken
   attr_accessor :state
 
-  def as_json(options={})
-    d = {:access_token=>token, :token_type => 'bearer'}
+  def as_json(_options = {})
+    d = { :access_token => token, :token_type => 'bearer' }
     d[:expires_in] = expires_in if expires_at
     d
   end
index 94856d0..4dcce55 100644 (file)
@@ -2,9 +2,9 @@ class Oauth2Verifier < OauthToken
   validates_presence_of :user
   attr_accessor :state
 
-  def exchange!(params={})
+  def exchange!(_params = {})
     OauthToken.transaction do
-      token = Oauth2Token.create! :user=>user,:client_application=>client_application, :scope => scope
+      token = Oauth2Token.create! :user => user, :client_application => client_application, :scope => scope
       invalidate!
       token
     end
@@ -27,7 +27,7 @@ class Oauth2Verifier < OauthToken
   protected
 
   def generate_keys
-    self.token = OAuth::Helper.generate_key(20)[0,20]
+    self.token = OAuth::Helper.generate_key(20)[0, 20]
     self.expires_at = 10.minutes.from_now
     self.authorized_at = Time.now
   end
index bd3d574..f2dd31d 100644 (file)
@@ -18,17 +18,17 @@ class OauthToken < ActiveRecord::Base
   end
 
   def authorized?
-    authorized_at != nil && !invalidated?
+    !authorized_at.nil? && !invalidated?
   end
 
   def to_query
     "oauth_token=#{token}&oauth_token_secret=#{secret}"
   end
 
-protected
+  protected
 
   def generate_keys
-    self.token = OAuth::Helper.generate_key(40)[0,40]
-    self.secret = OAuth::Helper.generate_key(40)[0,40]
+    self.token = OAuth::Helper.generate_key(40)[0, 40]
+    self.secret = OAuth::Helper.generate_key(40)[0, 40]
   end
 end
index b3ed96c..00f46db 100644 (file)
@@ -11,7 +11,7 @@ class OldNode < ActiveRecord::Base
   include Redactable
 
   validates_presence_of :changeset_id, :timestamp
-  validates_inclusion_of :visible, :in => [ true, false ]
+  validates_inclusion_of :visible, :in => [true, false]
   validates_numericality_of :latitude, :longitude
   validate :validate_position
   validates_associated :changeset
@@ -36,69 +36,67 @@ class OldNode < ActiveRecord::Base
     old_node.changeset_id = node.changeset_id
     old_node.node_id = node.id
     old_node.version = node.version
-    return old_node
+    old_node
   end
 
   def to_xml
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
-    return doc
+    doc.root << to_xml_node
+    doc
   end
 
   def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
     el = XML::Node.new 'node'
-    el['id'] = self.node_id.to_s
+    el['id'] = node_id.to_s
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
     if self.visible?
-      el['lat'] = self.lat.to_s
-      el['lon'] = self.lon.to_s
+      el['lat'] = lat.to_s
+      el['lon'] = lon.to_s
     end
 
-    add_tags_to_xml_node(el, self.old_tags)
+    add_tags_to_xml_node(el, old_tags)
 
-    return el
+    el
   end
 
   def save_with_dependencies!
     save!
 
-    self.tags.each do |k,v|
+    tags.each do |k, v|
       tag = OldNodeTag.new
       tag.k = k
       tag.v = v
-      tag.node_id = self.node_id
-      tag.version = self.version
+      tag.node_id = node_id
+      tag.version = version
       tag.save!
     end
   end
 
   def tags
-    @tags ||= Hash[self.old_tags.collect { |t| [t.k, t.v] }]
+    @tags ||= Hash[old_tags.collect { |t| [t.k, t.v] }]
   end
 
-  def tags=(t)
-    @tags = t
-  end
+  attr_writer :tags
 
   def tags_as_hash
-    return self.tags
+    tags
   end
 
   # Pretend we're not in any ways
   def ways
-    return []
+    []
   end
 
   # Pretend we're not in any relations
   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
+    current_node.version == version
   end
 end
index 86c4784..8e07277 100644 (file)
@@ -27,24 +27,24 @@ class OldRelation < ActiveRecord::Base
     old_relation.version = relation.version
     old_relation.members = relation.members
     old_relation.tags = relation.tags
-    return old_relation
+    old_relation
   end
 
   def save_with_dependencies!
     save!
 
-    self.tags.each do |k,v|
+    tags.each do |k, v|
       tag = OldRelationTag.new
       tag.k = k
       tag.v = v
-      tag.relation_id = self.relation_id
-      tag.version = self.version
+      tag.relation_id = relation_id
+      tag.version = version
       tag.save!
     end
 
-    self.members.each_with_index do |m,i|
+    members.each_with_index do |m, i|
       member = OldRelationMember.new
-      member.id = [self.relation_id, self.version, i]
+      member.id = [relation_id, version, i]
       member.member_type = m[0].classify
       member.member_id = m[1]
       member.member_role = m[2]
@@ -53,36 +53,32 @@ class OldRelation < ActiveRecord::Base
   end
 
   def members
-    @members ||= self.old_members.collect do |member|
+    @members ||= old_members.collect do |member|
       [member.member_type, member.member_id, member.member_role]
     end
   end
 
   def tags
-    @tags ||= Hash[self.old_tags.collect { |t| [t.k, t.v] }]
+    @tags ||= Hash[old_tags.collect { |t| [t.k, t.v] }]
   end
 
-  def members=(s)
-    @members = s
-  end
+  attr_writer :members
 
-  def tags=(t)
-    @tags = t
-  end
+  attr_writer :tags
 
   def to_xml
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
-    return doc
+    doc.root << to_xml_node
+    doc
   end
 
   def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
     el = XML::Node.new 'relation'
-    el['id'] = self.relation_id.to_s
+    el['id'] = relation_id.to_s
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
-    self.old_members.each do |member|
+    old_members.each do |member|
       member_el = XML::Node.new 'member'
       member_el['type'] = member.member_type.to_s.downcase
       member_el['ref'] = member.member_id.to_s # "id" is considered uncool here as it should be unique in XML
@@ -90,29 +86,29 @@ class OldRelation < ActiveRecord::Base
       el << member_el
     end
 
-    add_tags_to_xml_node(el, self.old_tags)
+    add_tags_to_xml_node(el, old_tags)
 
-    return el
+    el
   end
 
   # Temporary method to match interface to nodes
   def tags_as_hash
-    return self.tags
+    tags
   end
 
   # Temporary method to match interface to relations
   def relation_members
-    return self.old_members
+    old_members
   end
 
   # Pretend we're not in any relations
   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
+    current_relation.version == version
   end
 end
index 167ed30..c088ee1 100644 (file)
@@ -27,25 +27,25 @@ class OldWay < ActiveRecord::Base
     old_way.version = way.version
     old_way.nds = way.nds
     old_way.tags = way.tags
-    return old_way
+    old_way
   end
 
   def save_with_dependencies!
     save!
 
-    self.tags.each do |k,v|
+    tags.each do |k, v|
       tag = OldWayTag.new
       tag.k = k
       tag.v = v
-      tag.way_id = self.way_id
-      tag.version = self.version
+      tag.way_id = way_id
+      tag.version = version
       tag.save!
     end
 
     sequence = 1
-    self.nds.each do |n|
+    nds.each do |n|
       nd = OldWayNode.new
-      nd.id = [self.way_id, self.version, sequence]
+      nd.id = [way_id, version, sequence]
       nd.node_id = n
       nd.save!
       sequence += 1
@@ -53,36 +53,32 @@ class OldWay < ActiveRecord::Base
   end
 
   def nds
-    @nds ||= self.old_nodes.order(:sequence_id).collect { |n| n.node_id }
+    @nds ||= old_nodes.order(:sequence_id).collect(&:node_id)
   end
 
   def tags
-    @tags ||= Hash[self.old_tags.collect { |t| [t.k, t.v] }]
+    @tags ||= Hash[old_tags.collect { |t| [t.k, t.v] }]
   end
 
-  def nds=(s)
-    @nds = s
-  end
+  attr_writer :nds
 
-  def tags=(t)
-    @tags = t
-  end
+  attr_writer :tags
 
   def to_xml_node(changeset_cache = {}, user_display_name_cache = {})
     el = XML::Node.new 'way'
-    el['id'] = self.way_id.to_s
+    el['id'] = way_id.to_s
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
-    self.old_nodes.each do |nd| # FIXME need to make sure they come back in the right order
+    old_nodes.each do |nd| # FIXME need to make sure they come back in the right order
       node_el = XML::Node.new 'nd'
       node_el['ref'] = nd.node_id.to_s
       el << node_el
     end
 
-    add_tags_to_xml_node(el, self.old_tags)
+    add_tags_to_xml_node(el, old_tags)
 
-    return el
+    el
   end
 
   # Read full version of old way
@@ -93,21 +89,21 @@ class OldWay < ActiveRecord::Base
   # (i.e. is it visible? are we actually reverting to an earlier version?)
 
   def get_nodes_undelete
-    self.nds.collect do |n|
+    nds.collect do |n|
       node = Node.find(n)
       [node.lon, node.lat, n, node.version, node.tags_as_hash, node.visible]
     end
   end
 
   def get_nodes_revert(timestamp)
-    points=[]
-    self.nds.each do |n|
+    points = []
+    nds.each do |n|
       oldnode = OldNode.where('node_id = ? AND timestamp <= ?', n, timestamp).unredacted.order("timestamp DESC").first
       curnode = Node.find(n)
       id = n; reuse = curnode.visible
-      if oldnode.lat != curnode.lat or oldnode.lon != curnode.lon or oldnode.tags != curnode.tags then
+      if oldnode.lat != curnode.lat || oldnode.lon != curnode.lon || oldnode.tags != curnode.tags
         # node has changed: if it's in other ways, give it a new id
-        if curnode.ways-[self.way_id] then id=-1; reuse=false end
+        if curnode.ways - [way_id] then id = -1; reuse = false end
       end
       points << [oldnode.lon, oldnode.lat, id, curnode.version, oldnode.tags_as_hash, reuse]
     end
@@ -116,22 +112,22 @@ class OldWay < ActiveRecord::Base
 
   # Temporary method to match interface to nodes
   def tags_as_hash
-    return self.tags
+    tags
   end
 
   # Temporary method to match interface to ways
   def way_nodes
-    return self.old_nodes
+    old_nodes
   end
 
   # Pretend we're not in any relations
   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
+    current_way.version == version
   end
 end
index 4647668..c59ae47 100644 (file)
@@ -18,9 +18,9 @@ class Relation < ActiveRecord::Base
   has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
 
   validates_presence_of :id, :on => :update
-  validates_presence_of :timestamp,:version,  :changeset_id
+  validates_presence_of :timestamp, :version,  :changeset_id
   validates_uniqueness_of :id
-  validates_inclusion_of :visible, :in => [ true, false ]
+  validates_inclusion_of :visible, :in => [true, false]
   validates_numericality_of :id, :on => :update, :integer_only => true
   validates_numericality_of :changeset_id, :version, :integer_only => true
   validates_associated :changeset
@@ -31,36 +31,34 @@ 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 }) }
 
-  TYPES = ["node", "way", "relation"]
+  TYPES = %w(node way relation)
 
-  def self.from_xml(xml, create=false)
-    begin
-      p = XML::Parser.string(xml)
-      doc = p.parse
+  def self.from_xml(xml, create = false)
+    p = XML::Parser.string(xml)
+    doc = p.parse
 
-      doc.find('//osm/relation').each do |pt|
-        return Relation.from_xml_node(pt, create)
-      end
-      raise OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/relation element.")
-    rescue LibXML::XML::Error, ArgumentError => ex
-      raise OSM::APIBadXMLError.new("relation", xml, ex.message)
+    doc.find('//osm/relation').each do |pt|
+      return Relation.from_xml_node(pt, create)
     end
+    fail OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/relation element.")
+  rescue LibXML::XML::Error, ArgumentError => ex
+    raise OSM::APIBadXMLError.new("relation", xml, ex.message)
   end
 
-  def self.from_xml_node(pt, create=false)
+  def self.from_xml_node(pt, create = false)
     relation = Relation.new
 
-    raise OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create or not pt['version'].nil?
+    fail OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create || !pt['version'].nil?
     relation.version = pt['version']
-    raise OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt['changeset'].nil?
+    fail OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt['changeset'].nil?
     relation.changeset_id = pt['changeset']
 
     unless create
-      raise OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt['id'].nil?
+      fail OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt['id'].nil?
       relation.id = pt['id'].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id == 0
+      fail OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id == 0
     end
 
     # We don't care about the timestamp nor the visibility as these are either
@@ -69,12 +67,12 @@ class Relation < ActiveRecord::Base
     relation.visible = true
 
     # Start with no tags
-    relation.tags = Hash.new
+    relation.tags = {}
 
     # Add in any tags from the XML
     pt.find('tag').each do |tag|
-      raise OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag['k'].nil?
-      raise OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag['v'].nil?
+      fail OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag['k'].nil?
+      fail OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag['v'].nil?
       relation.add_tag_keyval(tag['k'], tag['v'])
     end
 
@@ -82,47 +80,47 @@ class Relation < ActiveRecord::Base
     # isn't done for a new relation then @members attribute will be nil,
     # and the members will be loaded from the database instead of being
     # empty, as intended.
-    relation.members = Array.new
+    relation.members = []
 
     pt.find('member').each do |member|
-      #member_type =
+      # member_type =
       logger.debug "each member"
-      raise 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_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
-    raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
+    fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
 
-    return relation
+    relation
   end
 
   def to_xml
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
-    return doc
+    doc.root << to_xml_node
+    doc
   end
 
   def to_xml_node(visible_members = nil, changeset_cache = {}, user_display_name_cache = {})
     el = XML::Node.new 'relation'
-    el['id'] = self.id.to_s
+    el['id'] = id.to_s
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
-    self.relation_members.each do |member|
-      p=0
+    relation_members.each do |member|
+      p = 0
       if visible_members
         # if there is a list of visible members then use that to weed out deleted segments
         if visible_members[member.member_type][member.member_id]
-          p=1
+          p = 1
         end
       else
         # otherwise, manually go to the db to check things
         if member.member.visible?
-          p=1
+          p = 1
         end
       end
       if p
@@ -134,29 +132,25 @@ class Relation < ActiveRecord::Base
        end
     end
 
-    add_tags_to_xml_node(el, self.relation_tags)
+    add_tags_to_xml_node(el, relation_tags)
 
-    return el
+    el
   end
 
   # FIXME is this really needed?
   def members
-    @members ||= self.relation_members.map do |member|
+    @members ||= relation_members.map do |member|
       [member.member_type, member.member_id, member.member_role]
     end
   end
 
   def tags
-    @tags ||= Hash[self.relation_tags.collect { |t| [t.k, t.v] }]
+    @tags ||= Hash[relation_tags.collect { |t| [t.k, t.v] }]
   end
 
-  def members=(m)
-    @members = m
-  end
+  attr_writer :members
 
-  def tags=(t)
-    @tags = t
-  end
+  attr_writer :tags
 
   def add_member(type, id, role)
     @members ||= []
@@ -164,11 +158,11 @@ class Relation < ActiveRecord::Base
   end
 
   def add_tag_keyval(k, v)
-    @tags = Hash.new unless @tags
+    @tags = {} unless @tags
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.
-    raise OSM::APIDuplicateTagsError.new("relation", self.id, k) if @tags.include? k
+    fail OSM::APIDuplicateTagsError.new("relation", id, k) if @tags.include? k
 
     @tags[k] = v
   end
@@ -184,8 +178,8 @@ class Relation < ActiveRecord::Base
   end
 
   def delete_with_history!(new_relation, user)
-    unless self.visible
-      raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id)
+    unless visible
+      fail OSM::APIAlreadyDeletedError.new("relation", new_relation.id)
     end
 
     # need to start the transaction here, so that the database can
@@ -195,8 +189,8 @@ class Relation < ActiveRecord::Base
       self.lock!
       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, self.id).first
-      raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil?
+      rel = RelationMember.joins(:relation).where("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id).first
+      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
       self.tags = {}
@@ -210,8 +204,8 @@ class Relation < ActiveRecord::Base
     Relation.transaction do
       self.lock!
       check_consistency(self, new_relation, user)
-      unless new_relation.preconditions_ok?(self.members)
-        raise OSM::APIPreconditionFailedError.new("Cannot update relation #{self.id}: data or member data is invalid.")
+      unless new_relation.preconditions_ok?(members)
+        fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
       end
       self.changeset_id = new_relation.changeset_id
       self.changeset = new_relation.changeset
@@ -225,7 +219,7 @@ class Relation < ActiveRecord::Base
   def create_with_history(user)
     check_create_consistency(self, user)
     unless self.preconditions_ok?
-      raise OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
+      fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
     end
     self.version = 0
     self.visible = true
@@ -240,14 +234,15 @@ class Relation < ActiveRecord::Base
     # Thus if you have nodes with the ids of 50 and 1 already in the
     # relation, then the hash table nodes would contain:
     # => {50=>true, 1=>true}
-    elements = { :node => Hash.new, :way => Hash.new, :relation => Hash.new }
+    elements = { :node => {}, :way => {}, :relation => {} }
 
     # pre-set all existing members to good
     good_members.each { |m| elements[m[0].downcase.to_sym][m[1]] = true }
 
-    self.members.each do |m|
+    members.each do |m|
       # find the hash for the element type or die
-      hash = elements[m[0].downcase.to_sym] or return false
+      hash = elements[m[0].downcase.to_sym]
+      return false unless hash
       # unless its in the cache already
       unless hash.key? m[1]
         # use reflection to look up the appropriate class
@@ -256,19 +251,19 @@ class Relation < ActiveRecord::Base
         element = model.where(:id => m[1]).first
 
         # and check that it is OK to use.
-        unless element and element.visible? and element.preconditions_ok?
-          raise OSM::APIPreconditionFailedError.new("Relation with id #{self.id} cannot be saved due to #{m[0]} with id #{m[1]}")
+        unless element && element.visible? && element.preconditions_ok?
+          fail OSM::APIPreconditionFailedError.new("Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}")
         end
         hash[m[1]] = true
       end
     end
 
-    return true
+    true
   end
 
   # Temporary method to match interface to nodes
   def tags_as_hash
-    return self.tags
+    tags
   end
 
   ##
@@ -276,11 +271,11 @@ class Relation < ActiveRecord::Base
   # this calling this method will fix them using the map from placeholders
   # to IDs +id_map+.
   def fix_placeholders!(id_map, placeholder_id = nil)
-    self.members.map! do |type, id, role|
+    members.map! do |type, id, role|
       old_id = id.to_i
       if old_id < 0
         new_id = id_map[type.downcase.to_sym][old_id]
-        raise OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil?
+        fail OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil?
         [type, new_id, role]
       else
         [type, id, role]
@@ -302,13 +297,13 @@ class Relation < ActiveRecord::Base
       self.save!
 
       tags = self.tags.clone
-      self.relation_tags.each do |old_tag|
+      relation_tags.each do |old_tag|
         key = old_tag.k
         # if we can match the tags we currently have to the list
         # of old tags, then we never set the tags_changed flag. but
         # if any are different then set the flag and do the DB
         # update.
-        if tags.has_key? key
+        if tags.key? key
           tags_changed |= (old_tag.v != tags[key])
 
           # remove from the map, so that we can expect an empty map
@@ -322,11 +317,11 @@ class Relation < ActiveRecord::Base
       end
       # if there are left-over tags then they are new and will have to
       # be added.
-      tags_changed |= (not tags.empty?)
-      RelationTag.delete_all(:relation_id => self.id)
-      self.tags.each do |k,v|
+      tags_changed |= (!tags.empty?)
+      RelationTag.delete_all(:relation_id => id)
+      self.tags.each do |k, v|
         tag = RelationTag.new
-        tag.relation_id = self.id
+        tag.relation_id = id
         tag.k = k
         tag.v = v
         tag.save!
@@ -335,9 +330,9 @@ class Relation < ActiveRecord::Base
       # same pattern as before, but this time we're collecting the
       # changed members in an array, as the bounding box updates for
       # elements are per-element, not blanked on/off like for tags.
-      changed_members = Array.new
+      changed_members = []
       members = self.members.clone
-      self.relation_members.each do |old_member|
+      relation_members.each do |old_member|
         key = [old_member.member_type, old_member.member_id, old_member.member_role]
         i = members.index key
         if i.nil?
@@ -353,10 +348,10 @@ class Relation < ActiveRecord::Base
       # members may be in a different order and i don't feel like implementing
       # a longest common subsequence algorithm to optimise this.
       members = self.members
-      RelationMember.delete_all(:relation_id => self.id)
-      members.each_with_index do |m,i|
+      RelationMember.delete_all(:relation_id => id)
+      members.each_with_index do |m, i|
         mem = RelationMember.new
-        mem.relation_id = self.id
+        mem.relation_id = id
         mem.sequence_id = i
         mem.member_type = m[0]
         mem.member_id = m[1]
@@ -379,10 +374,10 @@ class Relation < ActiveRecord::Base
       # reasonable on the assumption that adding or removing members doesn't
       # materially change the rest of the relation.
       any_relations =
-        changed_members.collect { |id,type| type == "relation" }.
-        inject(false) { |b,s| b or s }
+        changed_members.collect { |_id, type| type == "relation" }
+        .inject(false) { |b, s| b || s }
 
-      update_members = if tags_changed or any_relations
+      update_members = if tags_changed || any_relations
                          # add all non-relation bounding boxes to the changeset
                          # FIXME: check for tag changes along with element deletions and
                          # make sure that the deleted element's bounding box is hit.
@@ -390,7 +385,7 @@ class Relation < ActiveRecord::Base
                        else
                          changed_members
                        end
-      update_members.each do |type, id, role|
+      update_members.each do |type, id, _role|
         if type != "Relation"
           update_changeset_element(type, id)
         end
@@ -403,5 +398,4 @@ class Relation < ActiveRecord::Base
       changeset.save!
     end
   end
-
 end
index 1ac502b..0d55375 100644 (file)
@@ -1,13 +1,12 @@
 class RequestToken < OauthToken
-
   attr_accessor :provided_oauth_verifier
 
   def authorize!(user)
     return false if authorized?
     self.user = user
     self.authorized_at = Time.now
-    self.verifier = OAuth::Helper.generate_key(20)[0,20] unless oauth10?
-    self.save
+    self.verifier = OAuth::Helper.generate_key(20)[0, 20] unless oauth10?
+    save
   end
 
   def exchange!
@@ -17,9 +16,9 @@ class RequestToken < OauthToken
     RequestToken.transaction do
       params = { :user => user, :client_application => client_application }
       # copy the permissions from the authorised request token to the access token
-      client_application.permissions.each { |p|
+      client_application.permissions.each do |p|
         params[p] = read_attribute(p)
-      }
+      end
 
       access_token = AccessToken.create(params)
       invalidate!
@@ -40,7 +39,6 @@ class RequestToken < OauthToken
   end
 
   def oauth10?
-    (defined? OAUTH_10_SUPPORT) && OAUTH_10_SUPPORT && self.callback_url.blank?
+    (defined? OAUTH_10_SUPPORT) && OAUTH_10_SUPPORT && callback_url.blank?
   end
-
 end
index bf3e4ef..d8cc604 100644 (file)
@@ -7,16 +7,16 @@ class Trace < ActiveRecord::Base
 
   scope :visible, -> { where(:visible => true) }
   scope :visible_to, ->(u) { visible.where("visibility IN ('public', 'identifiable') OR user_id = ?", u) }
-  scope :visible_to_all, -> { where(:visibility => ["public", "identifiable"]) }
+  scope :visible_to_all, -> { where(:visibility => %w(public identifiable)) }
   scope :tagged, ->(t) { joins(:tags).where(:gpx_file_tags => { :tag => t }) }
 
   validates_presence_of :user_id, :name, :timestamp
   validates_presence_of :description, :on => :create
   validates_length_of :name, :maximum => 255
   validates_length_of :description, :maximum => 255
-#  validates_numericality_of :latitude, :longitude
-  validates_inclusion_of :inserted, :in => [ true, false ]
-  validates_inclusion_of :visibility, :in => ["private", "public", "trackable", "identifiable"]
+  #  validates_numericality_of :latitude, :longitude
+  validates_inclusion_of :inserted, :in => [true, false]
+  validates_inclusion_of :visibility, :in => %w(private public trackable identifiable)
 
   def destroy
     super
@@ -26,19 +26,19 @@ class Trace < ActiveRecord::Base
   end
 
   def tagstring
-    return tags.collect {|tt| tt.tag}.join(", ")
+    tags.collect(&:tag).join(", ")
   end
 
   def tagstring=(s)
     if s.include? ','
-      self.tags = s.split(/\s*,\s*/).select {|tag| tag !~ /^\s*$/}.collect {|tag|
+      self.tags = s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect {|tag|
         tt = Tracetag.new
         tt.tag = tag
         tt
       }
     else
-      #do as before for backwards compatibility:
-      self.tags = s.split().collect {|tag|
+      # do as before for backwards compatibility:
+      self.tags = s.split.collect {|tag|
         tt = Tracetag.new
         tt.tag = tag
         tt
@@ -58,13 +58,13 @@ class Trace < ActiveRecord::Base
     visibility == "identifiable"
   end
 
-  def large_picture= (data)
+  def large_picture=(data)
     f = File.new(large_picture_name, "wb")
     f.syswrite(data)
     f.close
   end
 
-  def icon_picture= (data)
+  def icon_picture=(data)
     f = File.new(icon_picture_name, "wb")
     f.syswrite(data)
     f.close
@@ -105,9 +105,9 @@ class Trace < ActiveRecord::Base
     bzipped = filetype =~ /bzip2 compressed/
     zipped = filetype =~ /Zip archive/
 
-    if gzipped then
+    if gzipped
       mimetype = "application/x-gzip"
-    elsif bzipped then
+    elsif bzipped
       mimetype = "application/x-bzip2"
     elsif zipped
       mimetype = "application/x-zip"
@@ -115,7 +115,7 @@ class Trace < ActiveRecord::Base
       mimetype = "application/gpx+xml"
     end
 
-    return mimetype
+    mimetype
   end
 
   def extension_name
@@ -125,9 +125,9 @@ class Trace < ActiveRecord::Base
     zipped = filetype =~ /Zip archive/
     tarred = filetype =~ /tar archive/
 
-    if tarred and gzipped then
+    if tarred && gzipped
       extension = ".tar.gz"
-    elsif tarred and bzipped then
+    elsif tarred && bzipped
       extension = ".tar.bz2"
     elsif tarred
       extension = ".tar"
@@ -141,67 +141,65 @@ class Trace < ActiveRecord::Base
       extension = ".gpx"
     end
 
-    return extension
+    extension
   end
 
   def to_xml
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
-    return doc
+    doc.root << to_xml_node
+    doc
   end
 
   def to_xml_node
     el1 = XML::Node.new 'gpx_file'
-    el1['id'] = self.id.to_s
-    el1['name'] = self.name.to_s
-    el1['lat'] = self.latitude.to_s if self.inserted
-    el1['lon'] = self.longitude.to_s if self.inserted
-    el1['user'] = self.user.display_name
-    el1['visibility'] = self.visibility
-    el1['pending'] = (!self.inserted).to_s
-    el1['timestamp'] = self.timestamp.xmlschema
+    el1['id'] = id.to_s
+    el1['name'] = name.to_s
+    el1['lat'] = latitude.to_s if inserted
+    el1['lon'] = longitude.to_s if inserted
+    el1['user'] = user.display_name
+    el1['visibility'] = visibility
+    el1['pending'] = (!inserted).to_s
+    el1['timestamp'] = timestamp.xmlschema
 
     el2 = XML::Node.new 'description'
-    el2 << self.description
+    el2 << description
     el1 << el2
 
-    self.tags.each do |tag|
+    tags.each do |tag|
       el2 = XML::Node.new('tag')
       el2 << tag.tag
       el1 << el2
     end
 
-    return el1
+    el1
   end
 
   # Read in xml as text and return it's Node object representation
-  def self.from_xml(xml, create=false)
-    begin
-      p = XML::Parser.string(xml)
-      doc = p.parse
-
-      doc.find('//osm/gpx_file').each do |pt|
-        return Trace.from_xml_node(pt, create)
-      end
+  def self.from_xml(xml, create = false)
+    p = XML::Parser.string(xml)
+    doc = p.parse
 
-      raise OSM::APIBadXMLError.new("trace", xml, "XML doesn't contain an osm/gpx_file element.")
-    rescue LibXML::XML::Error, ArgumentError => ex
-      raise OSM::APIBadXMLError.new("trace", xml, ex.message)
+    doc.find('//osm/gpx_file').each do |pt|
+      return Trace.from_xml_node(pt, create)
     end
+
+    fail OSM::APIBadXMLError.new("trace", xml, "XML doesn't contain an osm/gpx_file element.")
+  rescue LibXML::XML::Error, ArgumentError => ex
+    raise OSM::APIBadXMLError.new("trace", xml, ex.message)
   end
 
-  def self.from_xml_node(pt, create=false)
+  def self.from_xml_node(pt, create = false)
     trace = Trace.new
 
-    raise OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt['visibility'].nil?
+    fail OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt['visibility'].nil?
     trace.visibility = pt['visibility']
 
     unless create
-      raise OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt['id'].nil?
+      fail OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt['id'].nil?
       trace.id = pt['id'].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of trace cannot be zero when updating.") if trace.id == 0
+      fail OSM::APIBadUserInput.new("ID of trace cannot be zero when updating.") if trace.id == 0
     end
 
     # We don't care about the time, as it is explicitly set on create/update/delete
@@ -210,14 +208,14 @@ class Trace < ActiveRecord::Base
     trace.visible = true
 
     description = pt.find('description').first
-    raise OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil?
+    fail OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil?
     trace.description = description.content
 
     pt.find('tag').each do |tag|
       trace.tags.build(:tag => tag.content)
     end
 
-    return trace
+    trace
   end
 
   def xml_file
@@ -228,12 +226,12 @@ class Trace < ActiveRecord::Base
     zipped = filetype =~ /Zip archive/
     tarred = filetype =~ /tar archive/
 
-    if gzipped or bzipped or zipped or tarred then
-      tmpfile = Tempfile.new("trace.#{id}");
+    if gzipped || bzipped || zipped || tarred
+      tmpfile = Tempfile.new("trace.#{id}")
 
-      if tarred and gzipped then
+      if tarred && gzipped
         system("tar -zxOf #{trace_name} > #{tmpfile.path}")
-      elsif tarred and bzipped then
+      elsif tarred && bzipped
         system("tar -jxOf #{trace_name} > #{tmpfile.path}")
       elsif tarred
         system("tar -xOf #{trace_name} > #{tmpfile.path}")
@@ -252,13 +250,13 @@ class Trace < ActiveRecord::Base
       file = File.open(trace_name)
     end
 
-    return file
+    file
   end
 
   def import
     logger.info("GPX Import importing #{name} (#{id}) from #{user.email}")
 
-    gpx = GPX::File.new(self.xml_file)
+    gpx = GPX::File.new(xml_file)
 
     f_lat = 0
     f_lon = 0
@@ -267,8 +265,8 @@ class Trace < ActiveRecord::Base
     # If there are any existing points for this trace then delete
     # them - we check for existing points first to avoid locking
     # the table in the common case where there aren't any.
-    if Tracepoint.where(:gpx_id => self.id).exists?
-      Tracepoint.delete_all(:gpx_id => self.id)
+    if Tracepoint.where(:gpx_id => id).exists?
+      Tracepoint.delete_all(:gpx_id => id)
     end
 
     gpx.points do |point|
@@ -310,6 +308,6 @@ class Trace < ActiveRecord::Base
 
     logger.info "done trace #{id}"
 
-    return gpx
+    gpx
   end
 end
index c4ad8e1..8797028 100644 (file)
@@ -13,9 +13,9 @@ class Tracepoint < ActiveRecord::Base
 
   def to_xml_node(print_timestamp = false)
     el1 = XML::Node.new 'trkpt'
-    el1['lat'] = self.lat.to_s
-    el1['lon'] = self.lon.to_s
-    el1 << (XML::Node.new("time") << self.timestamp.xmlschema) if print_timestamp
-    return el1
+    el1['lat'] = lat.to_s
+    el1['lon'] = lon.to_s
+    el1 << (XML::Node.new("time") << timestamp.xmlschema) if print_timestamp
+    el1
   end
 end
index 7ace4bc..12afbb2 100644 (file)
@@ -7,7 +7,7 @@ class User < ActiveRecord::Base
   has_many :messages, -> { where(:to_user_visible => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :foreign_key => :to_user_id
   has_many :new_messages, -> { where(:to_user_visible => true, :message_read => false).order(:sent_on => :desc) }, :class_name => "Message", :foreign_key => :to_user_id
   has_many :sent_messages, -> { where(:from_user_visible => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :class_name => "Message", :foreign_key => :from_user_id
-  has_many :friends, -> { joins(:befriendee).where(:users => { :status => ["active", "confirmed"] }) }
+  has_many :friends, -> { joins(:befriendee).where(:users => { :status => %w(active confirmed) }) }
   has_many :friend_users, :through => :friends, :source => :befriendee
   has_many :tokens, :class_name => "UserToken"
   has_many :preferences, :class_name => "UserPreference"
@@ -26,27 +26,27 @@ class User < ActiveRecord::Base
 
   has_many :roles, :class_name => "UserRole"
 
-  scope :visible, -> { where(:status => ["pending", "active", "confirmed"]) }
-  scope :active, -> { where(:status => ["active", "confirmed"]) }
+  scope :visible, -> { where(:status => %w(pending active confirmed)) }
+  scope :active, -> { where(:status => %w(active confirmed)) }
   scope :identifiable, -> { where(:data_public => true) }
 
   has_attached_file :image,
-    :default_url => "/assets/:class/:attachment/:style.png",
-    :styles => { :large => "100x100>", :small => "50x50>" }
+                    :default_url => "/assets/:class/:attachment/:style.png",
+                    :styles => { :large => "100x100>", :small => "50x50>" }
 
   validates_presence_of :email, :display_name
-  validates_confirmation_of :email#, :message => ' addresses must match'
-  validates_confirmation_of :pass_crypt#, :message => ' must match the confirmation password'
-  validates_uniqueness_of :display_name, :allow_nil => true, :case_sensitive => false, :if => Proc.new { |u| u.display_name_changed? }
-  validates_uniqueness_of :email, :case_sensitive => false, :if => Proc.new { |u| u.email_changed? }
+  validates_confirmation_of :email # , :message => ' addresses must match'
+  validates_confirmation_of :pass_crypt # , :message => ' must match the confirmation password'
+  validates_uniqueness_of :display_name, :allow_nil => true, :case_sensitive => false, :if => proc { |u| u.display_name_changed? }
+  validates_uniqueness_of :email, :case_sensitive => false, :if => proc { |u| u.email_changed? }
   validates_uniqueness_of :openid_url, :allow_nil => true
   validates_length_of :pass_crypt, :within => 8..255
   validates_length_of :display_name, :within => 3..255, :allow_nil => true
-  validates_email_format_of :email, :if => Proc.new { |u| u.email_changed? }
-  validates_email_format_of :new_email, :allow_blank => true, :if => Proc.new { |u| u.new_email_changed? }
-  validates_format_of :display_name, :with => /\A[^\x00-\x1f\x7f\ufffe\uffff\/;.,?%#]*\z/, :if => Proc.new { |u| u.display_name_changed? }
-  validates_format_of :display_name, :with => /\A\S/, :message => "has leading whitespace", :if => Proc.new { |u| u.display_name_changed? }
-  validates_format_of :display_name, :with => /\S\z/, :message => "has trailing whitespace", :if => Proc.new { |u| u.display_name_changed? }
+  validates_email_format_of :email, :if => proc { |u| u.email_changed? }
+  validates_email_format_of :new_email, :allow_blank => true, :if => proc { |u| u.new_email_changed? }
+  validates_format_of :display_name, :with => /\A[^\x00-\x1f\x7f\ufffe\uffff\/;.,?%#]*\z/, :if => proc { |u| u.display_name_changed? }
+  validates_format_of :display_name, :with => /\A\S/, :message => "has leading whitespace", :if => proc { |u| u.display_name_changed? }
+  validates_format_of :display_name, :with => /\S\z/, :message => "has trailing whitespace", :if => proc { |u| u.display_name_changed? }
   validates_exclusion_of :display_name, :in => %w(new terms save confirm confirm-email go_public reset-password forgot-password suspended)
   validates_numericality_of :home_lat, :allow_nil => true
   validates_numericality_of :home_lon, :allow_nil => true
@@ -59,7 +59,7 @@ class User < ActiveRecord::Base
   after_save :spam_check
 
   def self.authenticate(options)
-    if options[:username] and options[:password]
+    if options[:username] && options[:password]
       user = where("email = ? OR display_name = ?", options[:username], options[:username]).first
 
       if user.nil?
@@ -70,7 +70,7 @@ class User < ActiveRecord::Base
         end
       end
 
-      if user and PasswordHash.check(user.pass_crypt, user.pass_salt, options[:password])
+      if user && PasswordHash.check(user.pass_crypt, user.pass_salt, options[:password])
         if PasswordHash.upgrade?(user.pass_crypt, user.pass_salt)
           user.pass_crypt, user.pass_salt = PasswordHash.create(options[:password])
           user.save
@@ -83,36 +83,36 @@ class User < ActiveRecord::Base
       user = token.user if token
     end
 
-    if user and
-      ( user.status == "deleted" or
-        ( user.status == "pending" and not options[:pending] ) or
-        ( user.status == "suspended" and not options[:suspended] ) )
+    if user &&
+       (user.status == "deleted" ||
+         (user.status == "pending" && !options[:pending]) ||
+         (user.status == "suspended" && !options[:suspended]))
       user = nil
     end
 
-    token.update_column(:expiry, 1.week.from_now) if token and user
+    token.update_column(:expiry, 1.week.from_now) if token && user
 
-    return user
+    user
   end
 
   def to_xml
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
-    return doc
+    doc.root << to_xml_node
+    doc
   end
 
   def to_xml_node
     el1 = XML::Node.new 'user'
-    el1['display_name'] = self.display_name.to_s
-    el1['account_created'] = self.creation_time.xmlschema
-    if self.home_lat and self.home_lon
+    el1['display_name'] = display_name.to_s
+    el1['account_created'] = creation_time.xmlschema
+    if home_lat && home_lon
       home = XML::Node.new 'home'
-      home['lat'] = self.home_lat.to_s
-      home['lon'] = self.home_lon.to_s
-      home['zoom'] = self.home_zoom.to_s
+      home['lat'] = home_lat.to_s
+      home['lon'] = home_lon.to_s
+      home['zoom'] = home_zoom.to_s
       el1 << home
     end
-    return el1
+    el1
   end
 
   def description
@@ -132,39 +132,39 @@ class User < ActiveRecord::Base
   end
 
   def preferred_language_from(array)
-    (languages & array.collect { |i| i.to_s }).first
+    (languages & array.collect(&:to_s)).first
   end
 
   def nearby(radius = NEARBY_RADIUS, num = NEARBY_USERS)
-    if self.home_lon and self.home_lat
-      gc = OSM::GreatCircle.new(self.home_lat, self.home_lon)
+    if home_lon && home_lat
+      gc = OSM::GreatCircle.new(home_lat, home_lon)
       bounds = gc.bounds(radius)
       sql_for_distance = gc.sql_for_distance("home_lat", "home_lon")
       nearby = User.where("id != ? AND status IN (\'active\', \'confirmed\') AND data_public = ? AND #{sql_for_distance} <= ?", id, true, radius).order(sql_for_distance).limit(num)
     else
       nearby = []
     end
-    return nearby
+    nearby
   end
 
   def distance(nearby_user)
-    return OSM::GreatCircle.new(self.home_lat, self.home_lon).distance(nearby_user.home_lat, nearby_user.home_lon)
+    OSM::GreatCircle.new(home_lat, home_lon).distance(nearby_user.home_lat, nearby_user.home_lon)
   end
 
   def is_friends_with?(new_friend)
-    self.friends.where(:friend_user_id => new_friend.id).exists?
+    friends.where(:friend_user_id => new_friend.id).exists?
   end
 
   ##
   # returns true if a user is visible
   def visible?
-    ["pending","active","confirmed"].include? self.status
+    %w(pending active confirmed).include? status
   end
 
   ##
   # returns true if a user is active
   def active?
-    ["active","confirmed"].include? self.status
+    %w(active confirmed).include? status
   end
 
   ##
@@ -189,13 +189,13 @@ class User < ActiveRecord::Base
   # returns the first active block which would require users to view
   # a message, or nil if there are none.
   def blocked_on_view
-    blocks.active.detect { |b| b.needs_view? }
+    blocks.active.detect(&:needs_view?)
   end
 
   ##
   # delete a user - leave the account but purge most personal data
   def delete
-    self.display_name = "user_#{self.id}"
+    self.display_name = "user_#{id}"
     self.description = ""
     self.home_lat = nil
     self.home_lon = nil
@@ -204,31 +204,31 @@ class User < ActiveRecord::Base
     self.new_email = nil
     self.openid_url = nil
     self.status = "deleted"
-    self.save
+    save
   end
 
   ##
   # return a spam score for a user
   def spam_score
-    changeset_score = self.changesets.size * 50
-    trace_score = self.traces.size * 50
-    diary_entry_score = self.diary_entries.inject(0) { |s,e| s += e.body.spam_score }
-    diary_comment_score = self.diary_comments.inject(0) { |s,c| s += c.body.spam_score }
-
-    score = self.description.spam_score / 4.0
-    score += self.diary_entries.where("created_at > ?", 1.day.ago).count * 10
-    score += diary_entry_score / self.diary_entries.length if self.diary_entries.length > 0
-    score += diary_comment_score / self.diary_comments.length if self.diary_comments.length > 0
+    changeset_score = changesets.size * 50
+    trace_score = traces.size * 50
+    diary_entry_score = diary_entries.inject(0) { |s, e| s += e.body.spam_score }
+    diary_comment_score = diary_comments.inject(0) { |s, c| s += c.body.spam_score }
+
+    score = description.spam_score / 4.0
+    score += diary_entries.where("created_at > ?", 1.day.ago).count * 10
+    score += diary_entry_score / diary_entries.length if diary_entries.length > 0
+    score += diary_comment_score / diary_comments.length if diary_comments.length > 0
     score -= changeset_score
     score -= trace_score
 
-    return score.to_i
+    score.to_i
   end
 
   ##
   # perform a spam check on a user
   def spam_check
-    if status == "active" and spam_score > SPAM_THRESHOLD
+    if status == "active" && spam_score > SPAM_THRESHOLD
       update_column(:status, "suspended")
     end
   end
@@ -236,10 +236,10 @@ class User < ActiveRecord::Base
   ##
   # return an oauth access token for a specified application
   def access_token(application_key)
-    return ClientApplication.find_by_key(application_key).access_token_for_user(self)
+    ClientApplication.find_by_key(application_key).access_token_for_user(self)
   end
 
-private
+  private
 
   def set_defaults
     self.creation_time = Time.now.getutc unless self.attribute_present?(:creation_time)
index d8fa95c..6ebdeee 100644 (file)
@@ -10,7 +10,7 @@ class UserBlock < ActiveRecord::Base
   ##
   # scope to match active blocks
   def self.active
-    self.where("needs_view or ends_at > ?", Time.now.getutc)
+    where("needs_view or ends_at > ?", Time.now.getutc)
   end
 
   ##
@@ -23,7 +23,7 @@ class UserBlock < ActiveRecord::Base
   # returns true if the block is currently active (i.e: the user can't
   # use the API).
   def active?
-    needs_view or ends_at > Time.now.getutc
+    needs_view || ends_at > Time.now.getutc
   end
 
   ##
@@ -31,20 +31,20 @@ class UserBlock < ActiveRecord::Base
   # is the user object who is revoking the ban.
   def revoke!(revoker)
     update_attributes(
-      :ends_at => Time.now.getutc(),
+      :ends_at => Time.now.getutc,
       :revoker_id => revoker.id,
       :needs_view => false
     )
   end
 
-private
+  private
 
   ##
   # validate that only moderators are allowed to change the
   # block. this should be caught and dealt with in the controller,
   # but i've also included it here just in case.
   def moderator_permissions
-    errors.add(:base, I18n.t('user_block.model.non_moderator_update')) if creator_id_changed? and !creator.moderator?
-    errors.add(:base, I18n.t('user_block.model.non_moderator_revoke')) unless revoker_id.nil? or revoker.moderator?
+    errors.add(:base, I18n.t('user_block.model.non_moderator_update')) if creator_id_changed? && !creator.moderator?
+    errors.add(:base, I18n.t('user_block.model.non_moderator_revoke')) unless revoker_id.nil? || revoker.moderator?
   end
 end
index 7faec97..969a368 100644 (file)
@@ -9,10 +9,9 @@ class UserPreference < ActiveRecord::Base
   # Turn this Node in to an XML Node without the <osm> wrapper.
   def to_xml_node
     el1 = XML::Node.new 'preference'
-    el1['k'] = self.k
-    el1['v'] = self.v
+    el1['k'] = k
+    el1['v'] = v
 
-    return el1
+    el1
   end
-
 end
index fb78328..c4d7035 100644 (file)
@@ -1,7 +1,7 @@
 class UserRole < ActiveRecord::Base
   belongs_to :user
 
-  ALL_ROLES = ['administrator', 'moderator']
+  ALL_ROLES = %w(administrator moderator)
 
   validates_inclusion_of :role, :in => ALL_ROLES
   validates_uniqueness_of :role, :scope => :user_id