You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by jo...@apache.org on 2014/01/09 22:32:11 UTC

git commit: [#6993] Fixed hidden arg error prevent GH attachments from importing

Updated Branches:
  refs/heads/cj/6993 [created] 7f9f8da1c


[#6993] Fixed hidden arg error prevent GH attachments from importing

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/7f9f8da1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/7f9f8da1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/7f9f8da1

Branch: refs/heads/cj/6993
Commit: 7f9f8da1c39bcd5169c0f9833e842eac71fe1a83
Parents: da8258d
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Thu Jan 9 21:31:48 2014 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Thu Jan 9 21:31:48 2014 +0000

----------------------------------------------------------------------
 ForgeImporters/forgeimporters/github/tracker.py | 38 ++++++++++++++------
 .../forgeimporters/tests/github/test_tracker.py | 25 +++++++++++--
 2 files changed, 49 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7f9f8da1/ForgeImporters/forgeimporters/github/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tracker.py b/ForgeImporters/forgeimporters/github/tracker.py
index 31ed344..db16c83 100644
--- a/ForgeImporters/forgeimporters/github/tracker.py
+++ b/ForgeImporters/forgeimporters/github/tracker.py
@@ -16,7 +16,9 @@
 #       under the License.
 
 import re
+import logging
 from datetime import datetime
+from urllib2 import HTTPError
 
 try:
     from cStringIO import StringIO
@@ -52,6 +54,9 @@ from forgeimporters.base import ToolImportForm
 from forgeimporters.github.utils import GitHubMarkdownConverter
 
 
+log = logging.getLogger(__name__)
+
+
 class GitHubTrackerImportForm(ToolImportForm):
     gh_project_name = fev.UnicodeString(not_empty=True)
     gh_user_name = fev.UnicodeString(not_empty=True)
@@ -127,7 +132,7 @@ class GitHubTrackerImporter(ToolImporter):
                         ticket_num=ticket_num,
                         import_id=import_id_converter.expand(ticket_num, app)
                     )
-                    self.process_fields(ticket, issue)
+                    self.process_fields(extractor, ticket, issue)
                     self.process_comments(extractor, ticket, issue)
                     self.process_events(extractor, ticket, issue)
                     self.process_milestones(ticket, issue)
@@ -153,7 +158,7 @@ class GitHubTrackerImporter(ToolImporter):
     def get_user_link(self, user):
         return u'[{0}](https://github.com/{0})'.format(user)
 
-    def process_fields(self, ticket, issue):
+    def process_fields(self, extractor, ticket, issue):
         ticket.summary = issue['title']
         ticket.status = issue['state']
         ticket.created_date = self.parse_datetime(issue['created_at'])
@@ -164,7 +169,7 @@ class GitHubTrackerImporter(ToolImporter):
         else:
             owner_line = ''
         # body processing happens here
-        body, attachments = self._get_attachments(issue['body'])
+        body, attachments = self._get_attachments(extractor, issue['body'])
         ticket.add_multiple_attachments(attachments)
         ticket.description = (
                 u'*Originally created by:* {creator}\n'
@@ -179,7 +184,7 @@ class GitHubTrackerImporter(ToolImporter):
 
     def process_comments(self, extractor, ticket, issue):
         for comment in extractor.iter_comments(issue):
-            body, attachments = self._get_attachments(comment['body'])
+            body, attachments = self._get_attachments(extractor, comment['body'])
             if comment['user']:
                 posted_by = u'*Originally posted by:* {}\n\n'.format(
                     self.get_user_link(comment['user']['login']))
@@ -240,7 +245,7 @@ class GitHubTrackerImporter(ToolImporter):
             })
         return [global_milestones]
 
-    def _get_attachments(self, body):
+    def _get_attachments(self, extractor, body):
         # at github, attachments are images only and are included into comment's body
         # usual syntax is
         # ![cdbpzjc5ex4](https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg)\r\n
@@ -258,19 +263,30 @@ class GitHubTrackerImporter(ToolImporter):
             body = body.replace(match.group(0), '')
             # stripping url and extension
             attachments.append(Attachment(
+                extractor,
                 match.group(1),  # url
                 'attach{}.{}'.format(i + 1, match.group(2)) # extension
             ))
         return (body, attachments)
 
 class Attachment(object):
-    def __init__(self, url, filename):
+    def __init__(self, extractor, url, filename):
         self.url = url
         self.filename = filename
         self.type = None
+        file = self.get_file(extractor)
+        if file:
+            # don't set unless valid (add_multiple_attachments uses hasattr)
+            self.file = file
 
-    @property
-    def file(self):
-        fp_ish = GitHubProjectExtractor.urlopen(self.url)
-        fp = StringIO(fp_ish.read())
-        return fp
+    def get_file(self, extractor):
+        try:
+            fp_ish = extractor.urlopen(self.url)
+            fp = StringIO(fp_ish.read())
+            return fp
+        except HTTPError as e:
+            if e.code == 404:
+                log.error('Unable to load attachment: %s', self.url)
+                return None
+            else:
+                raise

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7f9f8da1/ForgeImporters/forgeimporters/tests/github/test_tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/github/test_tracker.py b/ForgeImporters/forgeimporters/tests/github/test_tracker.py
index 9679c6b..b32df0a 100644
--- a/ForgeImporters/forgeimporters/tests/github/test_tracker.py
+++ b/ForgeImporters/forgeimporters/tests/github/test_tracker.py
@@ -17,6 +17,7 @@
 
 from datetime import datetime
 from unittest import TestCase
+from urllib2 import HTTPError
 import mock
 
 from ...github import tracker
@@ -80,9 +81,11 @@ class TestTrackerImporter(TestCase):
         }
         importer = tracker.GitHubTrackerImporter()
         importer.github_markdown_converter = GitHubMarkdownConverter('user', 'project')
+        extractor = mock.Mock()
+        extractor.urlopen().read.return_value = 'data'
         with mock.patch.object(tracker, 'datetime') as dt:
             dt.strptime.side_effect = lambda s,f: s
-            importer.process_fields(ticket, issue)
+            importer.process_fields(extractor, ticket, issue)
             self.assertEqual(ticket.summary, 'title')
             self.assertEqual(ticket.description, '*Originally created by:* [creator](https://github.com/creator)\n*Originally owned by:* [owner](https://github.com/owner)\n\nhello')
             self.assertEqual(ticket.status, 'New')
@@ -116,14 +119,28 @@ class TestTrackerImporter(TestCase):
 
     def test_get_attachments(self):
         importer = tracker.GitHubTrackerImporter()
+        extractor = mock.Mock()
+        extractor.urlopen().read.return_value = 'data'
         body = 'hello\n' \
         '![cdbpzjc5ex4](https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg)\r\n' \
         '![screensh0t](http://f.cl.ly/items/13453x43053r2G0d3x0v/Screen%20Shot%202012-04-28%20at%2010.48.17%20AM.png)'
-        new_body, attachments = importer._get_attachments(body)
+        new_body, attachments = importer._get_attachments(extractor, body)
         self.assertEqual(new_body, 'hello\n')
         self.assertEqual(len(attachments), 2)
         self.assertEqual(attachments[0].url, 'https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg')
         self.assertEqual(attachments[1].url, 'http://f.cl.ly/items/13453x43053r2G0d3x0v/Screen%20Shot%202012-04-28%20at%2010.48.17%20AM.png')
+        self.assertEqual(attachments[0].file.read(), 'data')
+        self.assertEqual(attachments[1].file.read(), 'data')
+
+    def test_get_attachments_404(self):
+        importer = tracker.GitHubTrackerImporter()
+        extractor = mock.Mock()
+        extractor.urlopen.side_effect = HTTPError('url', 404, 'mock', None, None)
+        body = 'hello\n' \
+            '![cdbpzjc5ex4](https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg)\r\n'
+        new_body, attachments = importer._get_attachments(extractor, body)
+        self.assertIsNotNone(attachments[0])
+        assert not hasattr(attachments[0], 'file')
 
     def test_process_comments(self):
         ticket = mock.Mock()
@@ -213,9 +230,11 @@ Hello
         }
         importer = tracker.GitHubTrackerImporter()
         importer.github_markdown_converter = GitHubMarkdownConverter('user', 'project')
+        extractor = mock.Mock()
+        extractor.urlopen().read.return_value = 'data'
         with mock.patch.object(tracker, 'datetime') as dt:
             dt.strptime.side_effect = lambda s,f: s
-            importer.process_fields(ticket, issue)
+            importer.process_fields(extractor, ticket, issue)
         self.assertEqual(ticket.description.strip(), body_converted.strip())
 
     def test_github_markdown_converted_in_comments(self):