Merge lp:~nataliabidart/ubuntuone-client/fix-803984 into lp:ubuntuone-client

[Natalia Bidart]

- Hasher class should open files with the 'rb' mode when hashing (LP: #803984).

- Upload command should open temp files using os_helper.open_file and 'rb' mode (LP: #803984).

[lp:~nataliabidart/ubuntuone-client/fix-803984]

1061. By Natalia Bidart on 2011-07-11

Attaching bug #803984.

[Facundo Batista (facundo)]

Like it

review: Approve

[Manuel de la Peña (mandel)]

review: Approve

=== modified file 'tests/syncdaemon/test_action_queue.py'
--- tests/syncdaemon/test_action_queue.py	2011-06-30 15:01:42 +0000
+++ tests/syncdaemon/test_action_queue.py	2011-07-11 20:34:03 +0000
@@ -3642,6 +3642,31 @@
         finally:
             self.command.action_queue.client = None
 
+    def test_uses_rb_flags_when_creating_temp_file(self):
+        """When creating the temp file, the open_file helper is used.
+
+        Also, check that the 'rb' flag is passed.
+
+        """
+        mh = content_hash.magic_hash_factory()
+        self.command.magic_hash = mh.content_hash()
+
+        called = []
+        self.patch(action_queue, 'open_file', lambda *a: called.append(a))
+
+        self.command.tempfile = NamedTemporaryFile()
+        expected_name = self.command.tempfile.name
+
+        # fake the self.command.client so put_content_request does not fail
+        self.command.action_queue.client = FakeClient()
+        self.command._run()
+
+        self.assertEqual(called, [(expected_name, 'rb')])
+
+        # unset self.command.client so the client is not tried to be disconnected
+        self.command.action_queue.client = None
+
+
 class CreateShareTestCase(ConnectedBaseTestCase):
     """Test for CreateShare ActionQueueCommand."""
 
@@ -5660,4 +5685,4 @@
         tmp.write(data)
         tmp.seek(0)
         self.assertEqual(data, tmp.read(len(data)))
-        tmp.close()
 \ No newline at end of file
+        tmp.close()
 
=== modified file 'tests/syncdaemon/test_hashqueue.py'
--- tests/syncdaemon/test_hashqueue.py	2011-02-28 15:53:29 +0000
+++ tests/syncdaemon/test_hashqueue.py	2011-07-11 20:34:03 +0000
@@ -32,8 +32,7 @@
 from contrib.testing import testcase
 from ubuntuone.devtools.handlers import MementoHandler
 from ubuntuone.syncdaemon import hash_queue
-from ubuntuone.storageprotocol.content_hash import \
-            content_hash_factory, crc32
+from ubuntuone.storageprotocol.content_hash import content_hash_factory, crc32
 
 
 # this is a unit test we need to access protected members
@@ -41,16 +40,13 @@
 class HasherTests(testcase.BaseTwistedTestCase):
     """Test the whole stuff to receive signals."""
 
+    @defer.inlineCallbacks
     def setUp(self):
         """Setup the test."""
-        testcase.BaseTwistedTestCase.setUp(self)
+        yield testcase.BaseTwistedTestCase.setUp(self)
         self.test_dir = self.mktemp('test_dir')
         self.timeout = 2
 
-    def tearDown(self):
-        """Clean up the tests."""
-        testcase.BaseTwistedTestCase.tearDown(self)
-
     def test_live_process(self):
         """Check that the hasher lives and dies."""
         # create the hasher
@@ -276,6 +272,42 @@
         queue.put((testfile, "mdid"))
         return d
 
+    @defer.inlineCallbacks
+    def test_open_file_with_rb(self):
+        """Check that the file to hash is opened with 'rb' mode."""
+        called = []
+
+        orig = hash_queue.open_file
+        def faked_open_file(*a):
+            called.append(a)
+            return orig(*a)
+
+        self.patch(hash_queue, 'open_file', faked_open_file)
+
+        queue = hash_queue.UniqueQueue()
+        testfile = os.path.join(self.test_dir, "testfile")
+        with open(testfile, "w") as fh:
+            fh.write("foobar")
+        queue.put((testfile, "mdid"))
+
+        d = defer.Deferred()
+
+        class FakeEventQueue(object):
+            """Helper class."""
+            def push(inner_self, event, **kwargs):
+                """Callback."""
+                d.callback(event)
+
+        eq = FakeEventQueue()
+
+        hasher = hash_queue._Hasher(queue=queue, end_mark='end-mark', event_queue=eq)
+        hasher.start()
+
+        yield d
+        hasher.stop()
+
+        self.assertEqual(called, [(testfile, 'rb')])
+
 
 class HashQueueTests(testcase.BaseTwistedTestCase):
     """Test the whole stuff to receive signals."""
@@ -713,6 +745,7 @@
         reactor.callLater(0.1, queue.clear)
         return d
 
+
 def test_suite():
     # pylint: disable-msg=C0111
     return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'ubuntuone/syncdaemon/action_queue.py'
--- ubuntuone/syncdaemon/action_queue.py	2011-06-30 13:55:22 +0000
+++ ubuntuone/syncdaemon/action_queue.py	2011-07-11 20:34:03 +0000
@@ -49,6 +49,7 @@
 from twisted.python.failure import Failure, DefaultException
 
 from oauth import oauth
+from ubuntuone.platform import open_file
 from ubuntuone.storageprotocol import protocol_pb2, content_hash
 from ubuntuone.storageprotocol import errors as protocol_errors
 from ubuntuone.storageprotocol.client import (
@@ -250,7 +251,8 @@
     else:
         return ('start - processMessage: %s',
                 str(message).replace("\n", " "))
-        
+
+
 class LoggingStorageClient(ThrottlingStorageClient):
     """A subclass of StorageClient that logs.
 
@@ -2470,7 +2472,7 @@
                                            hash=self.hash)
 
         if getattr(self.tempfile, 'name', None) is not None:
-            self.tempfile = open(self.tempfile.name)
+            self.tempfile = open_file(self.tempfile.name, 'rb')
         f = UploadProgressWrapper(self.tempfile, self)
 
         # access here the magic hash value, don't log anywhere, and
 
=== modified file 'ubuntuone/syncdaemon/hash_queue.py'
--- ubuntuone/syncdaemon/hash_queue.py	2011-02-28 15:53:29 +0000
+++ ubuntuone/syncdaemon/hash_queue.py	2011-07-11 20:34:03 +0000
@@ -115,7 +115,7 @@
         size = 0
         try:
             initial_stat = stat_path(path)
-            with open_file(path) as fh:
+            with open_file(path, 'rb') as fh:
                 while True:
                     # stop hashing if path_to_cancel == path or _stopped is True
                     with self.mutex:
@@ -196,8 +196,7 @@
 
 
 class UniqueQueue(Queue.Queue):
-    """Variant of Queue that only inserts unique items in the Queue
-    """
+    """Variant of Queue that only inserts unique items in the Queue."""
 
     def __init__(self, *args, **kwargs):
         """create the instance"""