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 2013/03/07 16:30:54 UTC

[2/5] git commit: [#5896] Improved error handling of missing trees and blob prev_commit context

[#5896] Improved error handling of missing trees and blob prev_commit context

Signed-off-by: Cory Johns <jo...@geek.net>


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

Branch: refs/heads/cj/5854
Commit: 8802894c9a7138f605416f4fe2bfc58556c69e12
Parents: c5f397a
Author: Cory Johns <jo...@geek.net>
Authored: Fri Mar 1 19:45:29 2013 +0000
Committer: Cory Johns <jo...@geek.net>
Committed: Wed Mar 6 22:06:22 2013 +0000

----------------------------------------------------------------------
 Allura/allura/model/repo.py           |   30 ++++-----
 Allura/allura/tests/unit/test_repo.py |   94 +++++++---------------------
 2 files changed, 35 insertions(+), 89 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/8802894c/Allura/allura/model/repo.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repo.py b/Allura/allura/model/repo.py
index 1510de6..668f00e 100644
--- a/Allura/allura/model/repo.py
+++ b/Allura/allura/model/repo.py
@@ -665,21 +665,9 @@ class Blob(object):
 
     @LazyProperty
     def prev_commit(self):
-        lc = LastCommit.get(self.tree, create=False)
-        if lc:
-            last_commit = self.repo.commit(lc.by_name[self.name])
-            prev_commit = last_commit.get_parent()
-            try:
-                tree = prev_commit and prev_commit.get_path(self.tree.path().rstrip('/'), create=False)
-            except KeyError:
-                return None  # parent tree added this commit
-            if not tree or self.name not in tree.by_name:
-                return None  # tree or file added this commit
-            lc = LastCommit.get(tree, create=False)
-            commit_id = lc and lc.by_name.get(self.name)
-            if commit_id:
-                prev_commit = self.repo.commit(commit_id)
-                return prev_commit
+        pcid = LastCommit._prev_commit_id(self.commit, self.path().strip('/'))
+        if pcid:
+            return self.repo.commit(pcid)
         return None
 
     @LazyProperty
@@ -733,8 +721,16 @@ class Blob(object):
         path = self.path()
         prev = self.prev_commit
         next = self.next_commit
-        if prev is not None: prev = prev.get_path(path, create=False)
-        if next is not None: next = next.get_path(path, create=False)
+        if prev is not None:
+            try:
+                prev = prev.get_path(path, create=False)
+            except KeyError as e:
+                prev = None
+        if next is not None:
+            try:
+                next = next.get_path(path, create=False)
+            except KeyError as e:
+                next = None
         return dict(
             prev=prev,
             next=next)

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/8802894c/Allura/allura/tests/unit/test_repo.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/test_repo.py b/Allura/allura/tests/unit/test_repo.py
index 5ec219f..9f9f302 100644
--- a/Allura/allura/tests/unit/test_repo.py
+++ b/Allura/allura/tests/unit/test_repo.py
@@ -129,87 +129,37 @@ class TestBlob(unittest.TestCase):
         blob.path = Mock(return_value='path')
         blob.prev_commit = Mock()
         blob.next_commit = Mock()
+        blob.prev_commit.get_path.return_value = '_prev'
+        blob.next_commit.get_path.return_value = '_next'
         context = blob.context()
+        assert_equal(context, {'prev': '_prev', 'next': '_next'})
         blob.prev_commit.get_path.assert_called_with('path', create=False)
         blob.next_commit.get_path.assert_called_with('path', create=False)
 
-    @patch.object(M.repo.LastCommit, 'get')
-    def test_prev_commit_no_create(self, lc_get):
-        lc_get.return_value = None
-        blob = M.repo.Blob(Mock(), Mock(), Mock())
-        pc = blob.prev_commit
-        lc_get.assert_called_once_with(blob.tree, create=False)
-        assert not blob.repo.commit.called
-        assert_equal(pc, None)
-
-        lc_get.reset_mock()
-        _lcd = Mock()
-        _lcd.by_name = {'blob': 'cid'}
-        lc_get.return_value = _lcd
-        _lc = Mock()
-        _pc = _lc.get_parent()
-        _pc.get_path.side_effect = KeyError
-        blob = M.repo.Blob(Mock(), Mock(), Mock())
-        blob.name = 'blob'
-        blob.tree.path.return_value = 'path/'
-        blob.repo.commit.return_value = _lc
-        pc = blob.prev_commit
-        blob.repo.commit.assert_called_once_with('cid')
-        lc_get.assert_called_once_with(blob.tree, create=False)
-        _pc.get_path.assert_called_once_with('path', create=False)
-        assert_equal(pc, None)
-
-        lc_get.reset_mock()
-        _lcd = Mock()
-        _lcd.by_name = {'blob': 'cid'}
-        lc_get.return_value = _lcd
-        _lc = Mock()
-        _pc = _lc.get_parent()
-        _pc.get_path.return_value = None
-        blob = M.repo.Blob(Mock(), Mock(), Mock())
-        blob.name = 'blob'
-        blob.tree.path.return_value = 'path/'
-        blob.repo.commit.return_value = _lc
-        pc = blob.prev_commit
-        blob.repo.commit.assert_called_once_with('cid')
-        lc_get.assert_called_once_with(blob.tree, create=False)
-        _pc.get_path.assert_called_once_with('path', create=False)
-        assert_equal(pc, None)
+        blob.prev_commit.get_path.side_effect = KeyError
+        blob.next_commit.get_path.side_effect = KeyError
+        context = blob.context()
+        assert_equal(context, {'prev': None, 'next': None})
 
-        lc_get.reset_mock()
-        _lcd = Mock()
-        _lcd.by_name = {'blob': 'cid'}
-        lc_get.return_value = _lcd
-        _lc = Mock()
-        _pc = _lc.get_parent()
-        _pc.get_path.return_value = Mock(by_name=['foo'])
-        blob = M.repo.Blob(Mock(), Mock(), Mock())
-        blob.name = 'blob'
-        blob.tree.path.return_value = 'path/'
-        blob.repo.commit.return_value = _lc
+    @patch.object(M.repo.LastCommit, '_prev_commit_id')
+    def test_prev_commit_no_create(self, lc_pcid):
+        lc_pcid.return_value = None
+        blob = M.repo.Blob(Mock(), 'foo', 'bid')
+        blob.tree.path.return_value = '/path/'
         pc = blob.prev_commit
-        blob.repo.commit.assert_called_once_with('cid')
-        lc_get.assert_called_once_with(blob.tree, create=False)
-        _pc.get_path.assert_called_once_with('path', create=False)
+        lc_pcid.assert_called_once_with(blob.commit, 'path/foo')
+        assert not blob.repo.commit.called
         assert_equal(pc, None)
 
-        lc_get.reset_mock()
-        _lcd = Mock()
-        _lcd.by_name = {'blob': 'cid'}
-        lc_get.return_value = _lcd
-        _lc = Mock()
-        _pc = _lc.get_parent()
-        _tree = Mock(by_name=['blob'])
-        _pc.get_path.return_value = _tree
-        blob = M.repo.Blob(Mock(), Mock(), Mock())
-        blob.name = 'blob'
-        blob.tree.path.return_value = 'path/'
-        blob.repo.commit.return_value = _lc
+        lc_pcid.reset_mock()
+        lc_pcid.return_value = 'pcid'
+        blob = M.repo.Blob(Mock(), 'foo', 'bid')
+        blob.tree.path.return_value = '/path/'
+        blob.repo.commit.return_value = 'commit'
         pc = blob.prev_commit
-        assert_equal(lc_get.call_args_list, [call(blob.tree, create=False), call(_tree, create=False)])
-        _pc.get_path.assert_called_once_with('path', create=False)
-        assert_equal(blob.repo.commit.call_args_list, [call('cid'), call('cid')])
-        assert_equal(pc, _lc)
+        lc_pcid.assert_called_once_with(blob.commit, 'path/foo')
+        blob.repo.commit.assert_called_once_with('pcid')
+        assert_equal(pc, 'commit')
 
     def test_next_commit_no_create(self):
         blob = M.repo.Blob(MagicMock(), MagicMock(), MagicMock())