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/30 23:56:55 UTC
[1/2] git commit: [#7118] Added validation to confirm GitHub project
exists and is readable
Updated Branches:
refs/heads/cj/7118 ca119b44e -> 1d8f6c8a0 (forced update)
[#7118] Added validation to confirm GitHub project exists and is readable
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/38c10a68
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/38c10a68
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/38c10a68
Branch: refs/heads/cj/7118
Commit: 38c10a6890a37730f98ff880193a4970e5d6919d
Parents: 5bc685d
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Thu Jan 30 21:14:26 2014 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Thu Jan 30 22:55:24 2014 +0000
----------------------------------------------------------------------
.../forgeimporters/github/__init__.py | 27 ++++++++++++++++++++
ForgeImporters/forgeimporters/github/code.py | 8 ++++--
ForgeImporters/forgeimporters/github/project.py | 15 +++++------
.../forgeimporters/github/tests/test_code.py | 18 +++++++++----
.../forgeimporters/github/tests/test_tracker.py | 9 +++++--
.../forgeimporters/github/tests/test_wiki.py | 11 ++++++--
ForgeImporters/forgeimporters/github/tracker.py | 14 +++++++---
ForgeImporters/forgeimporters/github/wiki.py | 8 ++++--
.../forgeimporters/google/__init__.py | 8 +++---
.../tests/github/test_extractor.py | 7 +++++
10 files changed, 97 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/__init__.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/__init__.py b/ForgeImporters/forgeimporters/github/__init__.py
index 2d4e85c..0f17e0b 100644
--- a/ForgeImporters/forgeimporters/github/__init__.py
+++ b/ForgeImporters/forgeimporters/github/__init__.py
@@ -26,12 +26,35 @@ from tg import config, session, redirect, request, expose
from tg.decorators import without_trailing_slash
from pylons import tmpl_context as c
from requests_oauthlib import OAuth2Session
+import requests
+from formencode import validators as fev
from forgeimporters import base
log = logging.getLogger(__name__)
+class GitHubProjectNameValidator(fev.FancyValidator):
+ not_empty = True
+ messages = {
+ 'invalid': 'Valid symbols are: letters, numbers, dashes, '
+ 'underscores and periods',
+ 'unavailable': 'This project is unavailable for import',
+ }
+
+ def _to_python(self, value, state=None):
+ user_name = state.full_dict.get('user_name', '')
+ user_name = state.full_dict.get('gh_user_name', user_name).strip()
+ project_name = value.strip()
+ full_project_name = '%s/%s' % (user_name, project_name)
+ if not re.match(r'^[a-zA-Z0-9-_.]+$', project_name):
+ raise fev.Invalid(self.message('invalid', state), value, state)
+
+ if not GitHubProjectExtractor(full_project_name).check_readable():
+ raise fev.Invalid(self.message('unavailable', state), value, state)
+ return project_name
+
+
class GitHubProjectExtractor(base.ProjectExtractor):
PAGE_MAP = {
'project_info': 'https://api.github.com/repos/{project_name}',
@@ -83,6 +106,10 @@ class GitHubProjectExtractor(base.ProjectExtractor):
return self.urlopen(url, **kw)
return resp
+ def check_readable(self):
+ resp = requests.head(self.get_page_url('project_info'))
+ return resp.status_code == 200
+
def get_next_page_url(self, link):
if not link:
return
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/code.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/code.py b/ForgeImporters/forgeimporters/github/code.py
index ede36b4..710e868 100644
--- a/ForgeImporters/forgeimporters/github/code.py
+++ b/ForgeImporters/forgeimporters/github/code.py
@@ -39,12 +39,16 @@ from forgeimporters.base import (
ToolImporter,
ToolImportForm,
)
-from forgeimporters.github import GitHubProjectExtractor, GitHubOAuthMixin
+from forgeimporters.github import (
+ GitHubProjectExtractor,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
class GitHubRepoImportForm(ToolImportForm):
- gh_project_name = fev.UnicodeString(not_empty=True)
gh_user_name = fev.UnicodeString(not_empty=True)
+ gh_project_name = GitHubProjectNameValidator()
class GitHubRepoImportController(BaseController, GitHubOAuthMixin):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/project.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/project.py b/ForgeImporters/forgeimporters/github/project.py
index fc7247b..003adae 100644
--- a/ForgeImporters/forgeimporters/github/project.py
+++ b/ForgeImporters/forgeimporters/github/project.py
@@ -24,20 +24,19 @@ from tg.decorators import with_trailing_slash
from allura.lib.decorators import require_post
-from .. import base
-from . import tasks
-from . import GitHubOAuthMixin
+from forgeimporters import base
+from forgeimporters.github import (
+ tasks,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
log = logging.getLogger(__name__)
class GitHubProjectForm(base.ProjectImportForm):
- project_name = fev.Regex(r'^[a-zA-Z0-9-_.]+$',
- not_empty=True,
- messages={
- 'invalid': 'Valid symbols are: letters, numbers, dashes, underscores and periods',
- })
+ project_name = GitHubProjectNameValidator()
class GitHubProjectImporter(base.ProjectImporter, GitHubOAuthMixin):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/tests/test_code.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tests/test_code.py b/ForgeImporters/forgeimporters/github/tests/test_code.py
index 8fa6a2f..bb8ed25 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_code.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_code.py
@@ -75,8 +75,10 @@ class TestGitHubImportController(TestController, TestCase):
self.assertIsNotNone(r.html.find(attrs=dict(name="mount_point")))
@with_git
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create(self, import_tool):
+ def test_create(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
params = dict(
gh_user_name='spooky',
gh_project_name='poop',
@@ -84,11 +86,13 @@ class TestGitHubImportController(TestController, TestCase):
mount_point='mymount',
)
r = self.app.post(
- '/p/{}/admin/ext/import/github-repo/create'.format(test_project_with_repo),
+ '/p/{}/admin/ext/import/github-repo/create'.format(
+ test_project_with_repo),
params,
status=302)
self.assertEqual(
- r.location, 'http://localhost/p/{}/admin/'.format(test_project_with_repo))
+ r.location, 'http://localhost/p/{}/admin/'.format(
+ test_project_with_repo))
self.assertEqual(
u'mymount', import_tool.post.call_args[1]['mount_point'])
self.assertEqual(
@@ -96,10 +100,13 @@ class TestGitHubImportController(TestController, TestCase):
self.assertEqual(
u'poop', import_tool.post.call_args[1]['project_name'])
self.assertEqual(u'spooky', import_tool.post.call_args[1]['user_name'])
+ self.assertEqual(requests.head.call_count, 1)
@with_git
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create_limit(self, import_tool):
+ def test_create_limit(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
project = M.Project.query.get(shortname=test_project_with_repo)
project.set_tool_data('GitHubRepoImporter', pending=1)
ThreadLocalORMSession.flush_all()
@@ -110,7 +117,8 @@ class TestGitHubImportController(TestController, TestCase):
mount_point='mymount',
)
r = self.app.post(
- '/p/{}/admin/ext/import/github-repo/create'.format(test_project_with_repo),
+ '/p/{}/admin/ext/import/github-repo/create'.format(
+ test_project_with_repo),
params,
status=302).follow()
self.assertIn('Please wait and try again', r)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/tests/test_tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tests/test_tracker.py b/ForgeImporters/forgeimporters/github/tests/test_tracker.py
index 72e1148..ba09ece 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_tracker.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_tracker.py
@@ -45,8 +45,10 @@ class TestGitHubTrackerImportController(TestController, TestCase):
self.assertIsNotNone(r.html.find(attrs=dict(name='mount_point')))
@with_tracker
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create(self, import_tool):
+ def test_create(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
params = dict(
gh_user_name='spooky',
gh_project_name='mulder',
@@ -62,10 +64,13 @@ class TestGitHubTrackerImportController(TestController, TestCase):
self.assertEqual(
u'mulder', import_tool.post.call_args[1]['project_name'])
self.assertEqual(u'spooky', import_tool.post.call_args[1]['user_name'])
+ self.assertEqual(requests.head.call_count, 1)
@with_tracker
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create_limit(self, import_tool):
+ def test_create_limit(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
p = M.Project.query.get(shortname=test_project_with_tracker)
p.set_tool_data('GitHubTrackerImporter', pending=1)
ThreadLocalORMSession.flush_all()
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/tests/test_wiki.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tests/test_wiki.py b/ForgeImporters/forgeimporters/github/tests/test_wiki.py
index f231f49..50e7278 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_wiki.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_wiki.py
@@ -538,8 +538,10 @@ class TestGitHubWikiImportController(TestController, TestCase):
r.html.find(attrs=dict(name='tool_option', value='import_history')))
@with_wiki
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create(self, import_tool):
+ def test_create(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
params = dict(
gh_user_name='spooky',
gh_project_name='mulder',
@@ -555,8 +557,10 @@ class TestGitHubWikiImportController(TestController, TestCase):
self.assertEqual(u'mulder', args['project_name'])
self.assertEqual(u'spooky', args['user_name'])
self.assertEqual(u'import_history', args['tool_option'])
+ self.assertEqual(requests.head.call_count, 1)
# without history
+ requests.head.reset_mock()
params.pop('tool_option')
r = self.app.post(self.url + 'create', params, status=302)
self.assertEqual(r.location, 'http://localhost/p/%s/admin/' %
@@ -567,10 +571,13 @@ class TestGitHubWikiImportController(TestController, TestCase):
self.assertEqual(u'mulder', args['project_name'])
self.assertEqual(u'spooky', args['user_name'])
self.assertEqual(u'', args['tool_option'])
+ self.assertEqual(requests.head.call_count, 1)
@with_wiki
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create_limit(self, import_tool):
+ def test_create_limit(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
p = M.Project.query.get(shortname=test_project_with_wiki)
p.set_tool_data('GitHubWikiImporter', pending=1)
ThreadLocalORMSession.flush_all()
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tracker.py b/ForgeImporters/forgeimporters/github/tracker.py
index 3af1517..fff88f6 100644
--- a/ForgeImporters/forgeimporters/github/tracker.py
+++ b/ForgeImporters/forgeimporters/github/tracker.py
@@ -46,11 +46,17 @@ from ming.orm import session, ThreadLocalORMSession
from pylons import tmpl_context as c
from pylons import app_globals as g
-from . import GitHubProjectExtractor, GitHubOAuthMixin
-from ..base import ToolImporter
from forgetracker.tracker_main import ForgeTrackerApp
from forgetracker import model as TM
-from forgeimporters.base import ToolImportForm
+from forgeimporters.base import (
+ ToolImportForm,
+ ToolImporter,
+)
+from forgeimporters.github import (
+ GitHubProjectExtractor,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
from forgeimporters.github.utils import GitHubMarkdownConverter
@@ -58,7 +64,7 @@ log = logging.getLogger(__name__)
class GitHubTrackerImportForm(ToolImportForm):
- gh_project_name = fev.UnicodeString(not_empty=True)
+ gh_project_name = GitHubProjectNameValidator()
gh_user_name = fev.UnicodeString(not_empty=True)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/github/wiki.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/wiki.py b/ForgeImporters/forgeimporters/github/wiki.py
index 0bd95da..99333f7 100644
--- a/ForgeImporters/forgeimporters/github/wiki.py
+++ b/ForgeImporters/forgeimporters/github/wiki.py
@@ -52,7 +52,11 @@ from forgeimporters.base import (
ToolImporter,
ToolImportForm,
)
-from forgeimporters.github import GitHubProjectExtractor, GitHubOAuthMixin
+from forgeimporters.github import (
+ GitHubProjectExtractor,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
from forgeimporters.github.utils import GitHubMarkdownConverter
from forgewiki import model as WM
from forgewiki.converters import mediawiki2markdown
@@ -71,7 +75,7 @@ except ImportError:
class GitHubWikiImportForm(ToolImportForm):
- gh_project_name = fev.UnicodeString(not_empty=True)
+ gh_project_name = GitHubProjectNameValidator()
gh_user_name = fev.UnicodeString(not_empty=True)
tool_option = fev.UnicodeString(if_missing=u'')
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/google/__init__.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/__init__.py b/ForgeImporters/forgeimporters/google/__init__.py
index 51560f9..aebc596 100644
--- a/ForgeImporters/forgeimporters/google/__init__.py
+++ b/ForgeImporters/forgeimporters/google/__init__.py
@@ -118,7 +118,9 @@ def csv_parser(page):
class GoogleCodeProjectNameValidator(fev.FancyValidator):
not_empty = True
messages = {
- 'invalid': 'Please enter a project URL, or a project name containing only letters, numbers, and dashes.',
+ 'invalid': 'Please enter a project URL, or a project name containing '
+ 'only letters, numbers, and dashes.',
+ 'unavailable': 'This project is unavailable for import',
}
def _to_python(self, value, state=None):
@@ -128,10 +130,10 @@ class GoogleCodeProjectNameValidator(fev.FancyValidator):
else:
project_name = os.path.basename(url.path.strip('/'))
if not re.match(r'^[a-z0-9][a-z0-9-]{,61}$', project_name):
- raise fev.Invalid(self.message('invalid'))
+ raise fev.Invalid(self.message('invalid', state), value, state)
if not GoogleCodeProjectExtractor(project_name).check_readable():
- raise fev.Invalid('The project "%s" is not avalible for import' % value, value, state)
+ raise fev.Invalid(self.message('unavailable', state), value, state)
return project_name
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/38c10a68/ForgeImporters/forgeimporters/tests/github/test_extractor.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/github/test_extractor.py b/ForgeImporters/forgeimporters/tests/github/test_extractor.py
index a8ae0a5..8b81f88 100644
--- a/ForgeImporters/forgeimporters/tests/github/test_extractor.py
+++ b/ForgeImporters/forgeimporters/tests/github/test_extractor.py
@@ -213,3 +213,10 @@ class TestGitHubProjectExtractor(TestCase):
'Rate limit exceeded (10 requests/hour). '
'Sleeping until 2013-10-25 09:32:02 UTC'
)
+
+ @patch.object(github.requests, 'head')
+ def test_check_readable(self, head):
+ head.return_value.status_code = 200
+ assert github.GitHubProjectExtractor('my-project').check_readable()
+ head.return_value.status_code = 404
+ assert not github.GitHubProjectExtractor('my-project').check_readable()
[2/2] git commit: [#7118] Better handling for GitHub wiki import
errors
Posted by jo...@apache.org.
[#7118] Better handling for GitHub wiki import errors
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/1d8f6c8a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/1d8f6c8a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/1d8f6c8a
Branch: refs/heads/cj/7118
Commit: 1d8f6c8a087d267ade34262445c9b06680267a46
Parents: 38c10a6
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Thu Jan 30 21:11:11 2014 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Thu Jan 30 22:55:56 2014 +0000
----------------------------------------------------------------------
ForgeImporters/forgeimporters/github/wiki.py | 22 +++++++++++++++++-----
requirements-sf.txt | 4 ++--
2 files changed, 19 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1d8f6c8a/ForgeImporters/forgeimporters/github/wiki.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/wiki.py b/ForgeImporters/forgeimporters/github/wiki.py
index 99333f7..1891da7 100644
--- a/ForgeImporters/forgeimporters/github/wiki.py
+++ b/ForgeImporters/forgeimporters/github/wiki.py
@@ -143,14 +143,15 @@ class GitHubWikiImporter(ToolImporter):
available_pages = []
def import_tool(
- self, project, user, project_name=None, mount_point=None, mount_label=None, user_name=None,
- tool_option=None, **kw):
+ self, project, user, project_name=None, mount_point=None,
+ mount_label=None, user_name=None, tool_option=None, **kw):
""" Import a GitHub wiki into a new Wiki Allura tool.
"""
project_name = "%s/%s" % (user_name, project_name)
extractor = GitHubProjectExtractor(project_name, user=user)
- if not extractor.has_wiki():
+ wiki_avail = extractor.has_wiki()
+ if not wiki_avail:
return
self.github_wiki_url = extractor.get_page_url(
@@ -171,8 +172,19 @@ class GitHubWikiImporter(ToolImporter):
try:
M.session.artifact_orm_session._get().skip_mod_date = True
with h.push_config(c, app=self.app):
- self.import_pages(
- extractor.get_page_url('wiki_url'), history=with_history)
+ try:
+ wiki_url = extractor.get_page_url('wiki_url')
+ self.import_pages(wiki_url, history=with_history)
+ except git.GitCommandError:
+ log.error(
+ 'Unable to clone GitHub wiki: '
+ 'wiki_url=%s; '
+ 'wiki_avail=%s; '
+ 'avail_url=%s',
+ wiki_url, wiki_avail,
+ extractor.get_page_url('project_info'),
+ exc_info=True)
+ raise
ThreadLocalORMSession.flush_all()
M.AuditLog.log(
'import tool %s from %s on %s' % (
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1d8f6c8a/requirements-sf.txt
----------------------------------------------------------------------
diff --git a/requirements-sf.txt b/requirements-sf.txt
index 8c6c040..b54c0c0 100644
--- a/requirements-sf.txt
+++ b/requirements-sf.txt
@@ -24,9 +24,9 @@ TracWikiImporter==0.3.6
MediawikiImporter==0.0.2
Unidecode==0.04.14
-# use version built from https://github.com/johnsca/GitPython/commits/tv/6000
+# use version built from https://github.com/johnsca/GitPython/tree/sf-master
# for unmerged fixes for [#5411], [#6000], and [#6078]
-GitPython==0.3.2.RC1-20131017
+GitPython==0.3.2.RC1-20140130
WebError==0.10.3-20130423