OTWO-1048 - Captures committer/author name/email correctly.

Author: amujumdar

lib/scm/parsers/bzr_xml_parser.rb

@@ -46,10 +46,12 @@ def tag_end(name)
       when 'message'
         @commit.message = @cdata
 			when 'committer'
-				@commit.committer_name = @text[/(.+?)(\s+<(.+)>)/, 1]
-				@commit.committer_email = @text[/(.+?)(\s+<(.+)>)/, 3]
+        committer = BzrXmlParser.capture_name(@text)
+				@commit.committer_name = committer[0]
+				@commit.committer_email = committer[1]
 			when 'author'
-				@authors << {:author_name => @text[/(.+?)(\s+<(.+)>)/, 1], :author_email => @text[/(.+?)(\s+<(.+)>)/, 3] }
+        author = BzrXmlParser.capture_name(@text)
+				@authors << {:author_name => author[0], :author_email => author[1]}
 			when 'timestamp'
 				@commit.committer_date = Time.parse(@text)
       when 'file'
@@ -111,22 +113,12 @@ def strip_trailing_asterisk(path)
 
     def remove_dupes(diffs)
       BzrXmlParser.remove_dupes(diffs)
-      # Bazaar may report that a file was added and modified in a single commit.
-      # Reduce these cases to a single 'A' action.
-      #diffs.delete_if do |d| 
-      #  d.action == 'M' && diffs.select { |x| x.path == d.path && x.action == 'A' }.any?
-      #end 
-
-      # Bazaar may report that a file was both deleted and added in a single commit.
-      # Reduce these cases to a single 'M' action.
-      #diffs.each do |d| 
-      #  d.action = 'M' if diffs.select { |x| x.path == d.path }.size > 1 
-      #end.uniq
     end 
 
 	end
 
 	class BzrXmlParser < Parser
+    NAME_REGEX = /^(.+?)(\s+<(.+)>\s*)?$/
 		def self.internal_parse(buffer, opts)
 			buffer = '<?xml?>' if buffer.is_a?(StringIO) and buffer.length < 2
 			begin
@@ -152,5 +144,16 @@ def self.remove_dupes(diffs)
         d.action = 'M' if diffs.select { |x| x.path == d.path }.size > 1 
       end.uniq
     end
+
+    # Bazaar expects committer/author to be specified in this format
+    # Name <email>, or John Doe <jdoe@example.com>
+    # However, we find many variations in the real world including
+    # ones where only email is specified as name.
+    def self.capture_name(text)
+      parts = text.match(NAME_REGEX).to_a
+      name = parts[1] || parts[0]
+      email = parts[3]
+      [name, email]
+    end
 	end
 end

test/unit/bzr_xml_parser_test.rb

@@ -362,5 +362,48 @@ def test_strip_trailing_asterisk_from_executables
       assert_equal 'D', diffs[1].action
       assert_equal 'arch', diffs[1].path
     end
+
+    def test_committer_name_capture
+      name, email = BzrXmlParser.capture_name('John')
+      assert_equal name, 'John'
+      assert_equal email, nil
+      assert_equal ['John Doe', nil], BzrXmlParser.capture_name('John Doe')
+      assert_equal ['John Doe jdoe@example.com', nil], BzrXmlParser.capture_name('John Doe jdoe@example.com')
+      assert_equal ['John Doe <jdoe@example.com', nil], BzrXmlParser.capture_name('John Doe <jdoe@example.com')
+      assert_equal ['John Doe jdoe@example.com>', nil], BzrXmlParser.capture_name('John Doe jdoe@example.com>')
+      assert_equal ['John Doe', 'jdoe@example.com'], BzrXmlParser.capture_name('John Doe <jdoe@example.com>')
+      assert_equal ['John Doe', 'jdoe@example.com'], BzrXmlParser.capture_name('John Doe     <jdoe@example.com>   ')
+      assert_equal ['jdoe@example.com', nil], BzrXmlParser.capture_name('jdoe@example.com')
+      xml = <<-XML
+<logs>
+  <log>
+    <revno>15</revno>
+    <revisionid>a@b.com-20111013152207-q8uec9pp1330gfbh</revisionid>
+    <parents>
+      <parent>test@example.com-20111012195747-seei62z2wmefjhmo</parent>
+    </parents>
+    <committer>a@b.com</committer>
+    <authors>
+      <author>author@c.com</author>
+    </authors>
+    <branch-nick>myproject</branch-nick>
+    <timestamp>Thu 2011-10-13 11:22:07 -0400</timestamp>
+    <message><![CDATA[Updated with only email as committer name.]]></message>
+    <affected-files>
+      <modified>
+        <file fid="test3.txt-20110722163813-257mjqqrvw3mav0f-4">test_a.txt</file>
+      </modified>
+    </affected-files>
+  </log>
+</logs>
+      XML
+      c = BzrXmlParser.parse(xml).first
+      assert_equal 'M', c.diffs.first.action
+      assert_equal 'test_a.txt', c.diffs.first.path
+      assert_equal 'a@b.com', c.committer_name
+      assert_equal nil, c.committer_email
+      assert_equal 'author@c.com', c.author_name
+      assert_equal nil, c.author_email
+    end
   end
 end