Skip to content

Commit

Permalink
Break dependency of DeviceTempFile from DeviceUtils
Browse files Browse the repository at this point in the history
Instead of accessing DeviceUtils methods (which tend to cause a circular
dependency with that module), we access directly the lower level
AdbWrapper to create temp files on the device.

Interface changes:
- DeviceTempFile expects an AdbWrapper object (instead of a DeviceUtils
  object).

Implementation changes:
- by default, write temp files to /data/local/tmp (c.f. BUG 436133)
- use 'test' to check for existence of files and directories
- use 'touch' to reserve the name of a temp file
- provide a |name_quoted| property
- ignore errors when removing temp file

The first patch set comes from the reverted CL:
https://codereview.chromium.org/751063002/

BUG=440703,436133

Review URL: https://codereview.chromium.org/793653002

Cr-Commit-Position: refs/heads/master@{#307705}
  • Loading branch information
perezju authored and Commit bot committed Dec 10, 2014
1 parent f93aa7a commit 4d3b5dc
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 38 deletions.
2 changes: 1 addition & 1 deletion build/android/pylib/device/device_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@ def WriteFile(self, device_path, contents, as_root=False, force_push=False,
host_temp.write(contents)
host_temp.flush()
if as_root and self.NeedsSU():
with device_temp_file.DeviceTempFile(self) as device_temp:
with device_temp_file.DeviceTempFile(self.adb) as device_temp:
self.adb.Push(host_temp.name, device_temp.name)
# Here we need 'cp' rather than 'mv' because the temp and
# destination files might be on different file systems (e.g.
Expand Down
2 changes: 1 addition & 1 deletion build/android/pylib/device/device_utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ def testWriteFile_withPushAndSU(self):
with self.assertCalls(
(mock.call.tempfile.NamedTemporaryFile(), tmp_host),
(self.call.device.NeedsSU(), True),
(mock.call.pylib.utils.device_temp_file.DeviceTempFile(self.device),
(mock.call.pylib.utils.device_temp_file.DeviceTempFile(self.adb),
MockTempFile('/external/path/tmp/on.device')),
self.call.adb.Push('/tmp/file/on.host', '/external/path/tmp/on.device'),
self.call.device.RunShellCommand(
Expand Down
40 changes: 24 additions & 16 deletions build/android/pylib/utils/device_temp_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,42 @@
import random
import time

from pylib import cmd_helper
from pylib.device import device_errors


class DeviceTempFile(object):
def __init__(self, device, prefix='temp_file', suffix=''):
def __init__(self, adb, suffix='', prefix='temp_file', dir='/data/local/tmp'):
"""Find an unused temporary file path in the devices external directory.
When this object is closed, the file will be deleted on the device.
Args:
device: An instance of DeviceUtils
prefix: The prefix of the name of the temp file.
adb: An instance of AdbWrapper
suffix: The suffix of the name of the temp file.
prefix: The prefix of the name of the temp file.
dir: The directory on the device where to place the temp file.
"""
self._device = device
self._adb = adb
# make sure that the temp dir is writable
self._adb.Shell('test -d %s' % cmd_helper.SingleQuote(dir))
while True:
i = random.randint(0, 1000000)
self.name = '%s/%s-%d-%010d%s' % (
self._device.GetExternalStoragePath(),
prefix, int(time.time()), i, suffix)
if not self._device.FileExists(self.name):
break
# Immediately create an empty file so that other temp files can't
# be given the same name.
# |as_root| must be set to False due to the implementation of |WriteFile|.
# Having |as_root| be True may cause infinite recursion.
self._device.WriteFile(self.name, '', as_root=False)
self.name = '{dir}/{prefix}-{time:d}-{nonce:d}{suffix}'.format(
dir=dir, prefix=prefix, time=int(time.time()),
nonce=random.randint(0, 1000000), suffix=suffix)
self.name_quoted = cmd_helper.SingleQuote(self.name)
try:
self._adb.Shell('test -e %s' % self.name_quoted)
except device_errors.AdbCommandFailedError:
break # file does not exist

# Immediately touch the file, so other temp files can't get the same name.
self._adb.Shell('touch %s' % self.name_quoted)

def close(self):
"""Deletes the temporary file from the device."""
self._device.RunShellCommand(['rm', self.name])
# we use -f to ignore errors if the file is already gone
self._adb.Shell('rm -f %s' % self.name_quoted)

def __enter__(self):
return self
Expand Down
78 changes: 59 additions & 19 deletions build/android/pylib/utils/device_temp_file_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,78 @@
import unittest

from pylib import constants
from pylib.device import adb_wrapper
from pylib.device import device_errors
from pylib.utils import device_temp_file
from pylib.utils import mock_calls

sys.path.append(os.path.join(
constants.DIR_SOURCE_ROOT, 'third_party', 'pymock'))
import mock # pylint: disable=F0401

class DeviceTempFileTest(unittest.TestCase):
class DeviceTempFileTest(mock_calls.TestCase):

def setUp(self):
self.device_utils = mock.MagicMock()
test_serial = '0123456789abcdef'
self.adb = mock.Mock(spec=adb_wrapper.AdbWrapper)
self.adb.__str__ = mock.Mock(return_value=test_serial)
self.watchMethodCalls(self.call.adb)

def testTempFileNameAlreadyExists(self):
self.device_utils.GetExternalStoragePath.return_value = '/sdcard'
self.device_utils.FileExists.side_effect = [True, True, True, False]
def mockShellCall(self, cmd_prefix, action=''):
"""Expect an adb.Shell(cmd) call with cmd_prefix and do some action
tmpfile = device_temp_file.DeviceTempFile(self.device_utils)
logging.debug('Temp file name: %s' % tmpfile.name)
self.assertEqual(self.device_utils.FileExists.call_count, 4)
self.device_utils.WriteFile.assert_called_with(tmpfile.name, '')
Args:
cmd_prefix: A string, the cmd of the received call is expected to have
this as a prefix.
action: If callable, an action to perform when the expected call is
received, otherwise a return value.
Returns:
An (expected_call, action) pair suitable for use in assertCalls.
"""
def check_and_return(cmd):
self.assertTrue(
cmd.startswith(cmd_prefix),
'command %r does not start with prefix %r' % (cmd, cmd_prefix))
if callable(action):
return action(cmd)
else:
return action
return (self.call.adb.Shell(mock.ANY), check_and_return)

def testTempFileLifecycle(self):
self.device_utils.GetExternalStoragePath.return_value = '/sdcard'
self.device_utils.FileExists.return_value = False
def mockExistsTest(self, exists_result):
def action(cmd):
if exists_result:
return ''
else:
raise device_errors.AdbCommandFailedError(
cmd, 'File not found', 1, str(self.adb))
return self.mockShellCall('test -e ', action)

with device_temp_file.DeviceTempFile(self.device_utils) as tmpfile:
filename = tmpfile.name
self.assertEqual(self.device_utils.WriteFile.call_count, 1)
self.assertNotEqual(self.device_utils.RunShellCommand.call_args,
mock.call(['rm', mock.ANY]))
def testTempFileNameAlreadyExists(self):
with self.assertCalls(
self.mockShellCall('test -d /data/local/tmp'),
self.mockExistsTest(True),
self.mockExistsTest(True),
self.mockExistsTest(True),
self.mockExistsTest(False),
self.mockShellCall('touch '),
self.mockShellCall('rm -f ')):
with device_temp_file.DeviceTempFile(self.adb) as tmpfile:
logging.debug('Temp file name: %s' % tmpfile.name)

self.assertEqual(self.device_utils.RunShellCommand.call_args,
mock.call(['rm', filename]))
def testTempFileLifecycle(self):
with self.assertCalls(
self.mockShellCall('test -d /data/local/tmp'),
self.mockExistsTest(False),
self.mockShellCall('touch ')):
tempFileContextManager = device_temp_file.DeviceTempFile(self.adb)
with mock.patch.object(self.adb, 'Shell'):
with tempFileContextManager as tmpfile:
logging.debug('Temp file name: %s' % tmpfile.name)
self.assertEquals(0, self.adb.Shell.call_count)
self.assertEquals(1, self.adb.Shell.call_count)
args, _ = self.adb.Shell.call_args
self.assertTrue(args[0].startswith('rm -f '))

if __name__ == '__main__':
logging.getLogger().setLevel(logging.DEBUG)
Expand Down
3 changes: 2 additions & 1 deletion build/android/pylib/utils/md5sum.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ def CalculateDeviceMd5Sums(paths, device):

out = []
with tempfile.NamedTemporaryFile() as md5sum_script_file:
with device_temp_file.DeviceTempFile(device) as md5sum_device_script_file:
with device_temp_file.DeviceTempFile(
device.adb) as md5sum_device_script_file:
md5sum_script = (
MD5SUM_DEVICE_SCRIPT_FORMAT.format(
path=p, md5sum_lib=MD5SUM_DEVICE_LIB_PATH,
Expand Down

0 comments on commit 4d3b5dc

Please sign in to comment.