"close" method of instance from "Node.get_content()" should be invoked if the method exists

[Jun Omae]

Type	Returned value	close	__enter__, __exit__
SubversionNode	FileContentStream instance	available	n/a
GitNode	cStringIO.StringIO instance	available	n/a *1
MercurialNode	self	n/a	n/a

*1 io.BytesIO has __enter__

trac/versioncontrol/web_ui/changeset.py

@@ -549 +549 @@
                 return None
             if mview.is_binary(new_node.content_type, new_node.path):
                 return None
-            old_content = old_node.get_content().read()
+            old_content = _read_content(old_node)
             if mview.is_binary(content=old_content):
                 return None
-            new_content = new_node.get_content().read()
+            new_content = _read_content(new_node)
             if mview.is_binary(content=new_content):
                 return None
 
@@ -719 +719 @@
                     continue
                 if mimeview.is_binary(old_node.content_type, old_node.path):
                     continue
-                old_content = old_node.get_content().read()
+                old_content = _read_content(old_node)
                 if mimeview.is_binary(content=old_content):
                     continue
                 old_node_info = (old_node.path, old_node.rev)
@@ -730 +730 @@
                     continue
                 if mimeview.is_binary(new_node.content_type, new_node.path):
                     continue
-                new_content = new_node.get_content().read()
+                new_content = _read_content(new_node)
                 if mimeview.is_binary(content=new_content):
                     continue
                 new_node_info = (new_node.path, new_node.rev)
@@ -1226 +1226 @@
 
         add_script(req, 'common/js/suggest.js')
         return 'diff_form.html', data, None
+
+
+def _read_content(node):
+    content = node.get_content()
+    try:
+        return content.read()
+    finally:
+        if hasattr(content, 'close'):
+            content.close()

tracopt/versioncontrol/svn/svn_fs.py

@@ -1198 +1198 @@
                                                    node._scoped_path_utf8,
                                                    self.pool()))
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, et, ev, tb):
+        self.close()
+
     def __del__(self):
         self.close()
 

[Jun Omae]

Revised patch: [3f87f5802/jomae.git] (jomae.git@t12977)

[Ryan J Ollos]

Looks like a good change to make.

[Jun Omae]

Thanks! Committed in [16591] and merged in [16592-16593].

[Ryan J Ollos]

Minor revision on trunk in r16607.

[Jun Omae]

Hmm, I often wrongly merge such lines from 1.0-stable.

I added the following lines to my Makefile.cfg to check the cases before unit-test:

test-py26 :
        grep -q '^min_python = (2, [0-5])' setup.py || ( \
            ! grep -r --include='*.py' '\<import *with_statement\>' . && \
            ! grep -r --include='*.py' '\<except *.*, *[A-Za-z0-9_]\+:' . \
        )

unit-test : test-py26