openstack
/
swift
Code Issues Proposed changes

Fix bug in mem_diskfile write_metadata method 

The mem_diskfile DiskFile.write_metadata method was
apparently never called in any existing test, as if it
were it would blow up as reported in the bug.

This patch fixes the method and adds a test that
exercises it. The test addition itself should be useful
since it verifies the behaviour of Last-Modified after
POSTs to an object.

Drive-by fixes for bad docstring and undefined references
in the _quarantine method.

Change-Id: I17fd62e5f02be5b48bfd9ba7fa25315e30a0a4bf
Closes-Bug: #1536037
This commit is contained in:
Alistair Coles 2016-01-20 18:14:16 +00:00
parent 6a473e3d7b
commit 2f8105e5fc
2 changed files with 107 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
swift/obj/mem_diskfile.py
View File

@ -25,7 +25,9 @@ from six import moves
from swift.common.utils import Timestamp
from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
DiskFileCollision, DiskFileDeleted, DiskFileNotOpen
from swift.common.request_helpers import is_sys_meta
from swift.common.swob import multi_range_iterator
from swift.obj.diskfile import DATAFILE_SYSTEM_META
class InMemoryFileSystem(object):
@ -99,7 +101,6 @@ class DiskFileWriter(object):
with the `StringIO` object.
:param metadata: dictionary of metadata to be written
:param extension: extension to be used when making the file
"""
metadata['name'] = self._name
self._filesystem.put_object(self._name, self._fp, metadata)
@ -209,7 +210,7 @@ class DiskFileReader(object):
if self._bytes_read != self._obj_size:
self._quarantine(
"Bytes read: %s, does not match metadata: %s" % (
self.bytes_read, self._obj_size))
self._bytes_read, self._obj_size))
elif self._iter_etag and \
self._etag != self._iter_etag.hexdigest():
self._quarantine(
@ -239,14 +240,10 @@ class DiskFile(object):
Manage object files in-memory.
:param mgr: DiskFileManager
:param device_path: path to the target device or drive
:param threadpool: thread pool to use for blocking operations
:param partition: partition on the device in which the object lives
:param fs: an instance of InMemoryFileSystem
:param account: account name for the object
:param container: container name for the object
:param obj: object name for the object
:param keep_cache: caller's preference for keeping data read in the cache
"""
def __init__(self, fs, account, container, obj):
@ -283,6 +280,19 @@ class DiskFile(object):
if self._fp is not None:
self._fp = None
def _quarantine(self, name, msg):
"""
Quarantine a file; responsible for incrementing the associated logger's
count of quarantines.
:param name: name of object to quarantine
:param msg: reason for quarantining to be included in the exception
:returns: DiskFileQuarantined exception object
"""
# for this implementation we simply delete the bad object
self._filesystem.del_object(name)
return DiskFileQuarantined(msg)
def _verify_data_file(self, fp):
"""
Verify the metadata's name value matches what we think the object is
@ -396,9 +406,18 @@ class DiskFile(object):
"""
Write a block of metadata to an object.
"""
cur_fp = self._filesystem.get(self._name)
if cur_fp is not None:
self._filesystem[self._name] = (cur_fp, metadata)
data, cur_mdata = self._filesystem.get_object(self._name)
if data is not None:
# The object exists. Update the new metadata with the object's
# immutable metadata (e.g. name, size, etag, sysmeta) and store it
# with the object data.
immutable_metadata = dict(
[(key, val) for key, val in cur_mdata.items()
if key.lower() in DATAFILE_SYSTEM_META
or is_sys_meta('object', key)])
metadata.update(immutable_metadata)
metadata['name'] = self._name
self._filesystem.put_object(self._name, data, metadata)
def delete(self, timestamp):
"""

104
test/unit/proxy/test_server.py
View File

@ -2861,8 +2861,50 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(headers[:len(exp)], exp)
@unpatch_policies
def test_PUT_last_modified(self):
def test_PUT_POST_last_modified(self):
prolis = _test_sockets[0]
prosrv = _test_servers[0]
def _do_HEAD():
# do a HEAD to get reported last modified time
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('HEAD /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'X-Storage-Token: t\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 200'
self.assertEqual(headers[:len(exp)], exp)
last_modified_head = [line for line in headers.split('\r\n')
if lm_hdr in line][0][len(lm_hdr):]
return last_modified_head
def _do_conditional_GET_checks(last_modified_time):
# check If-(Un)Modified-Since GETs
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('GET /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'If-Modified-Since: %s\r\n'
'X-Storage-Token: t\r\n\r\n' % last_modified_time)
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 304'
self.assertEqual(headers[:len(exp)], exp)
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('GET /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'If-Unmodified-Since: %s\r\n'
'X-Storage-Token: t\r\n\r\n' % last_modified_time)
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 200'
self.assertEqual(headers[:len(exp)], exp)
# PUT the object
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('PUT /v1/a/c/o.last_modified HTTP/1.1\r\n'
@ -2876,40 +2918,50 @@ class TestObjectController(unittest.TestCase):
last_modified_put = [line for line in headers.split('\r\n')
if lm_hdr in line][0][len(lm_hdr):]
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('HEAD /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'X-Storage-Token: t\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 200'
self.assertEqual(headers[:len(exp)], exp)
last_modified_head = [line for line in headers.split('\r\n')
if lm_hdr in line][0][len(lm_hdr):]
last_modified_head = _do_HEAD()
self.assertEqual(last_modified_put, last_modified_head)
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('GET /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'If-Modified-Since: %s\r\n'
'X-Storage-Token: t\r\n\r\n' % last_modified_put)
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 304'
self.assertEqual(headers[:len(exp)], exp)
_do_conditional_GET_checks(last_modified_put)
# now POST to the object using default object_post_as_copy setting
orig_post_as_copy = prosrv.object_post_as_copy
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('GET /v1/a/c/o.last_modified HTTP/1.1\r\n'
fd.write('POST /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'If-Unmodified-Since: %s\r\n'
'X-Storage-Token: t\r\n\r\n' % last_modified_put)
'X-Storage-Token: t\r\nContent-Length: 0\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 200'
exp = 'HTTP/1.1 202'
self.assertEqual(headers[:len(exp)], exp)
for line in headers.split('\r\n'):
self.assertFalse(line.startswith(lm_hdr))
# last modified time will have changed due to POST
last_modified_head = _do_HEAD()
_do_conditional_GET_checks(last_modified_head)
# now POST using non-default object_post_as_copy setting
try:
prosrv.object_post_as_copy = not orig_post_as_copy
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('POST /v1/a/c/o.last_modified HTTP/1.1\r\n'
'Host: localhost\r\nConnection: close\r\n'
'X-Storage-Token: t\r\nContent-Length: 0\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 202'
self.assertEqual(headers[:len(exp)], exp)
for line in headers.split('\r\n'):
self.assertFalse(line.startswith(lm_hdr))
finally:
prosrv.object_post_as_copy = orig_post_as_copy
# last modified time will have changed due to POST
last_modified_head = _do_HEAD()
_do_conditional_GET_checks(last_modified_head)
def test_PUT_auto_content_type(self):
with save_globals():