Merge branch 'username-period-again' into 'master'

Don't allow username to end in period.

Fixes #2174 and #2249.

cc @jacobvosmaer Please review and test with the GitLab.com dump.

See merge request !1786

5 files changed, 102 insertions, 4 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0d6b0f2d942..e5c06776ecf 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,7 @@ v 7.11.0 (unreleased)
   - Ignore invalid lines in .gitmodules
   - Fix "Cannot move project" error message from popping up after a successful transfer (Stan Hu)
   - Redirect to sign in page after signing out.
+  - Fix "Hello @username." references not working by no longer allowing usernames to end in period.
   -
   - Add "Reply quoting selected text" shortcut key (`r`)
   - Fix bug causing `@whatever` inside an issue's first code block to be picked up as a user mention.
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index e1de114375e..211dfa76b81 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -60,15 +60,24 @@ class Namespace < ActiveRecord::Base
 
     def clean_path(path)
       path = path.dup
+      # Get the email username by removing everything after an `@` sign.
       path.gsub!(/@.*\z/,             "")
+      # Usernames can't end in .git, so remove it.
       path.gsub!(/\.git\z/,           "")
+      # Remove dashes at the start of the username.
       path.gsub!(/\A-+/,              "")
+      # Remove periods at the end of the username.
       path.gsub!(/\.+\z/,             "")
+      # Remove everything that's not in the list of allowed characters.
       path.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
 
+      # Users with the great usernames of "." or ".." would end up with a blank username.
+      # Work around that by setting their username to "blank", followed by a counter. 
+      path = "blank" if path.blank?
+
       counter = 0
       base = path
-      while Namespace.by_path(path).present?
+      while Namespace.find_by_path_or_name(path)
         counter += 1
         path = "#{base}#{counter}"
       end
diff --git a/db/migrate/20150421120000_remove_periods_at_ends_of_usernames.rb b/db/migrate/20150421120000_remove_periods_at_ends_of_usernames.rb
new file mode 100644
index 00000000000..3057ea3c68c
--- /dev/null
+++ b/db/migrate/20150421120000_remove_periods_at_ends_of_usernames.rb
@@ -0,0 +1,88 @@
+class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration
+  include Gitlab::ShellAdapter
+
+  class Namespace < ActiveRecord::Base
+    class << self
+      def find_by_path_or_name(path)
+        find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase)
+      end
+
+      def clean_path(path)
+        path = path.dup
+        # Get the email username by removing everything after an `@` sign.
+        path.gsub!(/@.*\z/,             "")
+        # Usernames can't end in .git, so remove it.
+        path.gsub!(/\.git\z/,           "")
+        # Remove dashes at the start of the username.
+        path.gsub!(/\A-+/,              "")
+        # Remove periods at the end of the username.
+        path.gsub!(/\.+\z/,             "")
+        # Remove everything that's not in the list of allowed characters.
+        path.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
+
+        # Users with the great usernames of "." or ".." would end up with a blank username.
+        # Work around that by setting their username to "blank", followed by a counter.
+        path = "blank" if path.blank?
+
+        counter = 0
+        base = path
+        while Namespace.find_by_path_or_name(path)
+          counter += 1
+          path = "#{base}#{counter}"
+        end
+
+        path
+      end
+    end
+  end
+
+  def up
+    changed_paths = {}
+
+    select_all("SELECT id, username FROM users WHERE username LIKE '%.'").each do |user|
+      username_was = user["username"]
+      username = Namespace.clean_path(username_was)
+      changed_paths[username_was] = username
+
+      username = quote_string(username)
+      execute "UPDATE users SET username = '#{username}' WHERE id = #{user["id"]}"
+      execute "UPDATE namespaces SET path = '#{username}', name = '#{username}' WHERE type IS NULL AND owner_id = #{user["id"]}"
+    end
+
+    select_all("SELECT id, path FROM namespaces WHERE type = 'Group' AND path LIKE '%.'").each do |group|
+      path_was = group["path"]
+      path = Namespace.clean_path(path_was)
+      changed_paths[path_was] = path
+
+      path = quote_string(path)
+      execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{group["id"]}"
+    end
+
+    changed_paths.each do |path_was, path|
+      # Don't attempt to move if original path only contains periods.
+      next if path_was =~ /\A\.+\z/
+
+      if gitlab_shell.mv_namespace(path_was, path)
+        # If repositories moved successfully we need to remove old satellites
+        # and send update instructions to users.
+        # However we cannot allow rollback since we moved namespace dir
+        # So we basically we mute exceptions in next actions
+        begin
+          gitlab_shell.rm_satellites(path_was)
+          # We cannot send update instructions since models and mailers
+          # can't safely be used from migrations as they may be written for 
+          # later versions of the database.
+          # send_update_instructions
+        rescue
+          # Returning false does not rollback after_* transaction but gives
+          # us information about failing some of tasks
+          false
+        end
+      else
+        # if we cannot move namespace directory we should rollback
+        # db changes in order to prevent out of sync between db and fs
+        raise Exception.new('namespace directory cannot be moved')
+      end
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2b7e27e3a31..6db7e386c86 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150417122318) do
+ActiveRecord::Schema.define(version: 20150421120000) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 9aeed5e6939..0571574aa4f 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -2,7 +2,7 @@ module Gitlab
   module Regex
     extend self
 
-    NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*)'.freeze
+    NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*[a-zA-Z0-9_\-]|[a-zA-Z0-9_])'.freeze
 
     def namespace_regex
       @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
@@ -10,7 +10,7 @@ module Gitlab
 
     def namespace_regex_message
       "can contain only letters, digits, '_', '-' and '.'. " \
-      "Cannot start with '-'." \
+      "Cannot start with '-' or end in '.'." \
     end