tree: now behaves like a list with string indexing functionality - using a dict as cache is a problem as the tree is ordered, added blobs, trees and traverse method

repo: remove blob function as blobs are created directly or iterated - primitve types should not clutter the repo interface

6 files changed, 192 insertions, 74 deletions

diff --git a/CHANGES b/CHANGES
index 597d529c..ea98b587 100644
--- a/CHANGES
+++ b/CHANGES
@@ -47,7 +47,11 @@ Blob
 Tree
 ----
 * former 'name' member renamed to path as it suits the actual data better
-
+* added traverse method allowing to recursively traverse tree items
+* deleted blob method
+* added blobs and trees properties allowing to query the respective items in the 
+  tree
+* now mimics behaviour of a read-only list instead of a dict to maintain order.
 
 0.1.6
 =====
diff --git a/lib/git/objects/tree.py b/lib/git/objects/tree.py
index 273384a3..707cebaa 100644
--- a/lib/git/objects/tree.py
+++ b/lib/git/objects/tree.py
@@ -9,26 +9,57 @@ import blob
 import base
 
 class Tree(base.IndexObject):
+	"""
+	Tress represent a ordered list of Blobs and other Trees. Hence it can be 
+	accessed like a list.
+	
+	Tree's will cache their contents after first retrieval to improve efficiency.
+	
+	``Tree as a list``::
+		
+		Access a specific blob using the  
+		tree['filename'] notation.
+		
+		You may as well access by index
+		blob = tree[0]
+		
+		
+	"""
 	
 	type = "tree"
-	__slots__ = "_contents"
+	__slots__ = "_cache"
 	
 	def __init__(self, repo, id, mode=None, path=None):
 		super(Tree, self).__init__(repo, id, mode, path)
 
 	def _set_cache_(self, attr):
-		if attr == "_contents":
-			# Read the tree contents.
-			self._contents = {}
-			for line in self.repo.git.ls_tree(self.id).splitlines():
-				obj = self.content__from_string(self.repo, line)
-				if obj is not None:
-					self._contents[obj.path] = obj
+		if attr == "_cache":
+			# Set the data when we need it
+			self._cache = self._get_tree_cache(self.repo, self.id)
 		else:
 			super(Tree, self)._set_cache_(attr)
 
-	@staticmethod
-	def content__from_string(repo, text):
+	@classmethod
+	def _get_tree_cache(cls, repo, treeish):
+		"""
+		Return
+			list(object_instance, ...)
+		
+		``treeish``
+			sha or ref identifying a tree  
+		"""
+		out = list()
+		for line in repo.git.ls_tree(treeish).splitlines():
+			obj = cls.content_from_string(repo, line)
+			if obj is not None:
+				out.append(obj)
+			# END if object was handled
+		# END for each line from ls-tree
+		return out
+		
+
+	@classmethod
+	def content_from_string(cls, repo, text):
 		"""
 		Parse a content item and create the appropriate object
 
@@ -40,6 +71,8 @@ class Tree(base.IndexObject):
 
 		Returns
 			``git.Blob`` or ``git.Tree``
+			
+		NOTE: Currently sub-modules are ignored !
 		"""
 		try:
 			mode, typ, id, path = text.expandtabs(1).split(" ", 3)
@@ -51,6 +84,7 @@ class Tree(base.IndexObject):
 		elif typ == "blob":
 			return blob.Blob(repo, id, mode, path)
 		elif typ == "commit":
+			# TODO: Return a submodule
 			return None 
 		else:
 		  raise(TypeError, "Invalid type: %s" % typ)
@@ -67,36 +101,104 @@ class Tree(base.IndexObject):
 			<git.Blob "8b1e02c0fb554eed2ce2ef737a68bb369d7527df">
 
 		Returns
-			``git.Blob`` or ``git.Tree`` or ``None`` if not found
+			``git.Blob`` or ``git.Tree``
+		
+		Raise 
+			KeyError if given file or tree does not exist in tree
 		"""
-		return self.get(file)
+		return self[file]
 
 
 	def __repr__(self):
 		return '<git.Tree "%s">' % self.id
+		
+	@classmethod
+	def _iter_recursive(cls, repo, tree, cur_depth, max_depth, predicate ):
+		
+		for obj in tree:
+			# adjust path to be complete
+			obj.path = os.path.join(tree.path, obj.path)
+			if not predicate(obj):
+				continue
+			yield obj
+			if obj.type == "tree" and ( max_depth < 0 or cur_depth+1 <= max_depth ):
+				for recursive_obj in cls._iter_recursive( repo, obj, cur_depth+1, max_depth, predicate ):
+					yield recursive_obj
+				# END for each recursive object
+			# END if we may enter recursion
+		# END for each object
+		
+	def traverse(self, max_depth=-1, predicate = lambda i: True):
+		"""
+		Returns
+			Iterator to traverse the tree recursively up to the given level.
+			The iterator returns Blob and Tree objects
+		
+		``max_depth``
+		
+			if -1, the whole tree will be traversed
+			if 0, only the first level will be traversed which is the same as 
+			the default non-recursive iterator
+			
+		``predicate``
+		
+			If predicate(item) returns True, item will be returned by iterator
+		"""
+		return self._iter_recursive( self.repo, self, 0, max_depth, predicate )
+		
+	@property
+	def trees(self):
+		"""
+		Returns
+			list(Tree, ...) list of trees directly below this tree
+		"""
+		return [ i for i in self if i.type == "tree" ]
+		
+	@property
+	def blobs(self):
+		"""
+		Returns
+			list(Blob, ...) list of blobs directly below this tree
+		"""
+		return [ i for i in self if i.type == "blob" ]
 
-	# Implement the basics of the dict protocol:
-	# directories/trees can be seen as object dicts.
-	def __getitem__(self, key):
-		return self._contents[key]
 
+	# List protocol
+	def __getslice__(self,i,j):
+		return self._cache[i:j]
+		
 	def __iter__(self):
-		return iter(self._contents)
-
+		return iter(self._cache)
+		
 	def __len__(self):
-		return len(self._contents)
-
-	def __contains__(self, key):
-		return key in self._contents
-
-	def get(self, key):
-		return self._contents.get(key)
-
-	def items(self):
-		return self._contents.items()
-
-	def keys(self):
-		return self._contents.keys()
-
-	def values(self):
-		return self._contents.values()
+		return len(self._cache)
+		
+	def __getitem__(self,item):
+		if isinstance(item, int):
+			return self._cache[item]
+		
+		if isinstance(item, basestring):
+			# compatability
+			for obj in self._cache:
+				if obj.path == item:
+					return obj
+			# END for each obj
+			raise KeyError( "Blob or Tree named %s not found" % item )
+		# END index is basestring 
+		
+		raise TypeError( "Invalid index type: %r" % item )
+		
+		
+	def __contains__(self,item):
+		if isinstance(item, base.IndexObject):
+			return item in self._cache
+		
+		# compatability
+		for obj in self._cache:
+			if item == obj.path:
+				return True
+		# END for each item
+		return False
+	
+	def __reversed__(self):
+		return reversed(self._cache)
diff --git a/lib/git/repo.py b/lib/git/repo.py
index 39e84088..c1387870 100644
--- a/lib/git/repo.py
+++ b/lib/git/repo.py
@@ -342,19 +342,10 @@ class Repo(object):
 		
 		# we should also check whether the ref has a valid commit ... but lets n
 		# not be over-critical
-		return Tree(self, treeish)
+		# the root has an empty relative path and the default mode
+		root = Tree(self, treeish, 0, '')
+		return root
 
-	def blob(self, id):
-		"""
-		The Blob object for the given id
-
-		``id``
-			is the SHA1 id of the blob
-
-		Returns
-			``git.Blob``
-		"""
-		return Blob(self, id=id)
 
 	def log(self, commit='master', path=None, **kwargs):
 		"""
diff --git a/test/git/test_repo.py b/test/git/test_repo.py
index e999ffe8..7f87f78b 100644
--- a/test/git/test_repo.py
+++ b/test/git/test_repo.py
@@ -93,8 +93,8 @@ class TestRepo(object):
 
 		tree = self.repo.tree(Head(self.repo, 'master'))
 
-		assert_equal(4, len([c for c in tree.values() if isinstance(c, Blob)]))
-		assert_equal(3, len([c for c in tree.values() if isinstance(c, Tree)]))
+		assert_equal(4, len([c for c in tree if isinstance(c, Blob)]))
+		assert_equal(3, len([c for c in tree if isinstance(c, Tree)]))
 
 		assert_true(git.called)
 
@@ -102,7 +102,7 @@ class TestRepo(object):
 	def test_blob(self, git):
 		git.return_value = fixture('cat_file_blob')
 
-		blob = self.repo.blob("abc")
+		blob = Blob(self.repo,"abc")
 		assert_equal("Hello world", blob.data)
 
 		assert_true(git.called)
diff --git a/test/git/test_tree.py b/test/git/test_tree.py
index cb8ebb04..e0429db1 100644
--- a/test/git/test_tree.py
+++ b/test/git/test_tree.py
@@ -7,19 +7,20 @@
 from test.testlib import *
 from git import *
 
-class TestTree(object):
-	def setup(self):
+class TestTree(TestCase):
+	
+	def setUp(self):
 		self.repo = Repo(GIT_REPO)
 
 	@patch_object(Git, '_call_process')
 	def test_contents_should_cache(self, git):
 		git.return_value = fixture('ls_tree_a') + fixture('ls_tree_b')
 	
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 
 		child = tree['grit']
-		child.items()
-		child.items()
+		len(child)
+		len(child)
 		
 		assert_true(git.called)
 		assert_equal(2, git.call_count)
@@ -27,7 +28,7 @@ class TestTree(object):
   
 	def test_content_from_string_tree_should_return_tree(self):
 		text = fixture('ls_tree_a').splitlines()[-1]
-		tree = Tree.content__from_string(None, text)
+		tree = Tree.content_from_string(None, text)
 
 		assert_equal(Tree, tree.__class__)
 		assert_equal("650fa3f0c17f1edb4ae53d8dcca4ac59d86e6c44", tree.id)
@@ -37,7 +38,7 @@ class TestTree(object):
 	def test_content_from_string_tree_should_return_blob(self):
 		text = fixture('ls_tree_b').split("\n")[0]
 		
-		tree = Tree.content__from_string(None, text)
+		tree = Tree.content_from_string(None, text)
 
 		assert_equal(Blob, tree.__class__)
 		assert_equal("aa94e396335d2957ca92606f909e53e7beaf3fbb", tree.id)
@@ -47,12 +48,12 @@ class TestTree(object):
 	def test_content_from_string_tree_should_return_commit(self):
 		text = fixture('ls_tree_commit').split("\n")[1]
 	
-		tree = Tree.content__from_string(None, text)
+		tree = Tree.content_from_string(None, text)
 		assert_none(tree)
 	
 	@raises(TypeError)
 	def test_content_from_string_invalid_type_should_raise(self):
-		Tree.content__from_string(None, "040000 bogus 650fa3f0c17f1edb4ae53d8dcca4ac59d86e6c44	test")
+		Tree.content_from_string(None, "040000 bogus 650fa3f0c17f1edb4ae53d8dcca4ac59d86e6c44	test")
 
 	@patch_object(Blob, 'size')
 	@patch_object(Git, '_call_process')
@@ -60,13 +61,37 @@ class TestTree(object):
 		git.return_value = fixture('ls_tree_a')
 		blob.return_value = 1
 		
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 		
 		assert_equal('aa06ba24b4e3f463b3c4a85469d0fb9e5b421cf8', (tree/'lib').id)
 		assert_equal('8b1e02c0fb554eed2ce2ef737a68bb369d7527df', (tree/'README.txt').id)
 		
 		assert_true(git.called)
-		assert_equal(git.call_args, (('ls_tree', 'master'), {}))
+		
+	def test_traverse(self):
+		root = self.repo.tree()
+		num_recursive = 0
+		all_items = list()
+		for obj in root.traverse():
+			if "/" in obj.path:
+				num_recursive += 1
+				
+			assert isinstance(obj, (Blob, Tree))
+			all_items.append(obj)
+		# END for each object
+		# limit recursion level to 0 - should be same as default iteration
+		assert all_items
+		assert 'CHANGES' in root
+		assert len(list(root)) == len(list(root.traverse(max_depth=0)))
+		
+		# only choose trees
+		trees_only = lambda i: i.type == "tree"
+		trees = list(root.traverse(predicate = trees_only))
+		assert len(trees) == len(list( i for i in root.traverse() if trees_only(i) ))
+		
+		# trees and blobs
+		assert len(set(trees)|set(root.trees)) == len(trees)
+		assert len(set(b for b in root if isinstance(b, Blob)) | set(root.blobs)) == len( root.blobs )
   
 	@patch_object(Blob, 'size')
 	@patch_object(Git, '_call_process')
@@ -74,26 +99,24 @@ class TestTree(object):
 		git.return_value = fixture('ls_tree_a')
 		blob.return_value = 0
 		
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 		
 		assert_not_none(tree/'README.txt')
 		assert_equal('8b1e02c0fb554eed2ce2ef737a68bb369d7527df', (tree/'README.txt').id)
 		
 		assert_true(git.called)
-		assert_equal(git.call_args, (('ls_tree', 'master'), {}))
   
 	@patch_object(Git, '_call_process')
 	def test_slash_with_commits(self, git):
 		git.return_value = fixture('ls_tree_commit')
 
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 	
-		assert_none(tree/'bar')
+		self.failUnlessRaises(KeyError, tree.__div__, 'bar')
 		assert_equal('2afb47bcedf21663580d5e6d2f406f08f3f65f19', (tree/'foo').id)
 		assert_equal('f623ee576a09ca491c4a27e48c0dfe04be5f4a2e', (tree/'baz').id)
 
 		assert_true(git.called)
-		assert_equal(git.call_args, (('ls_tree', 'master'), {}))
 
 	@patch_object(Blob, 'size')
 	@patch_object(Git, '_call_process')
@@ -101,13 +124,12 @@ class TestTree(object):
 		git.return_value = fixture('ls_tree_a')
 		blob.return_value = 1
 
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 
 		assert_equal('aa06ba24b4e3f463b3c4a85469d0fb9e5b421cf8', tree['lib'].id)
 		assert_equal('8b1e02c0fb554eed2ce2ef737a68bb369d7527df', tree['README.txt'].id)
 
 		assert_true(git.called)
-		assert_equal(git.call_args, (('ls_tree', 'master'), {}))
 
 	@patch_object(Blob, 'size')
 	@patch_object(Git, '_call_process')
@@ -115,33 +137,31 @@ class TestTree(object):
 		git.return_value = fixture('ls_tree_a')
 		blob.return_value = 0
 
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 
 		assert_not_none(tree['README.txt'])
 		assert_equal('8b1e02c0fb554eed2ce2ef737a68bb369d7527df', tree['README.txt'].id)
 
 		assert_true(git.called)
-		assert_equal(git.call_args, (('ls_tree', 'master'), {}))
 
 	@patch_object(Git, '_call_process')
 	def test_dict_with_commits(self, git):
 		git.return_value = fixture('ls_tree_commit')
 
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 
-		assert_none(tree.get('bar'))
+		self.failUnlessRaises(KeyError, tree.__getitem__, 'bar')
 		assert_equal('2afb47bcedf21663580d5e6d2f406f08f3f65f19', tree['foo'].id)
 		assert_equal('f623ee576a09ca491c4a27e48c0dfe04be5f4a2e', tree['baz'].id)
 
 		assert_true(git.called)
-		assert_equal(git.call_args, (('ls_tree', 'master'), {}))
 
 	@patch_object(Git, '_call_process')
 	@raises(KeyError)
 	def test_dict_with_non_existant_file(self, git):
 		git.return_value = fixture('ls_tree_commit')
 
-		tree = self.repo.tree('master')
+		tree = self.repo.tree(Head(self.repo,'master'))
 		tree['bar']
 
 	def test_repr(self):
diff --git a/test/testlib/__init__.py b/test/testlib/__init__.py
index 2133eb8c..f364171b 100644
--- a/test/testlib/__init__.py
+++ b/test/testlib/__init__.py
@@ -8,6 +8,7 @@ import inspect
 from mock import *
 from asserts import *
 from helper import *
+from unittest import TestCase
 
 __all__ = [ name for name, obj in locals().items()
 			if not (name.startswith('_') or inspect.ismodule(obj)) ]