diff --git a/plone/namedfile/field.py b/plone/namedfile/field.py index 70b2c5c..770642e 100644 --- a/plone/namedfile/field.py +++ b/plone/namedfile/field.py @@ -10,6 +10,7 @@ from plone.namedfile.interfaces import INamedFileField from plone.namedfile.interfaces import INamedImage from plone.namedfile.interfaces import INamedImageField +from plone.namedfile.interfaces import INamedTyped from plone.namedfile.interfaces import IPluggableFileFieldValidation from plone.namedfile.interfaces import IPluggableImageFieldValidation from plone.namedfile.utils import get_contenttype @@ -138,7 +139,17 @@ class NamedBlobFile(NamedField): accept = () def validate(self, value): - super().validate(value, IPluggableFileFieldValidation) + # Bit of a hack but we avoid loading the .data into memory + # because schema validation checks the property exists + # which loads the entire file into memory for no reaon. + # This can slow down imports and uploads a lot. + # TODO: better fix might be get DX to check for a decorator first + # - https://stackoverflow.com/questions/16169948/check-if-something-is-an-attribute-or-decorator-in-python + self.schema = INamedTyped + try: + super().validate(value, IPluggableFileFieldValidation) + finally: + self.schema = INamedBlobFile @implementer(INamedBlobImageField) @@ -150,4 +161,9 @@ class NamedBlobImage(NamedField): accept = ("image/*",) def validate(self, value): - super().validate(value, IPluggableImageFieldValidation) + # see NamedBlobFile comment + self.schema = INamedTyped + try: + super().validate(value, IPluggableImageFieldValidation) + finally: + self.schema = INamedBlobImage diff --git a/plone/namedfile/interfaces.py b/plone/namedfile/interfaces.py index f9a22e6..9d7719e 100644 --- a/plone/namedfile/interfaces.py +++ b/plone/namedfile/interfaces.py @@ -10,8 +10,7 @@ HAVE_BLOBS = True - -class IFile(Interface): +class ITyped(Interface): contentType = schema.NativeStringLine( title="Content Type", @@ -21,6 +20,8 @@ class IFile(Interface): missing_value="", ) +class IFile(Interface): + data = schema.Bytes( title="Data", description="The actual content of the object.", @@ -83,12 +84,15 @@ class INamed(Interface): filename = schema.TextLine(title="Filename", required=False, default=None) +class INamedTyped(INamed, ITyped): + """An item with a filename and contentType""" + -class INamedFile(INamed, IFile): +class INamedFile(INamedTyped, IFile): """A non-BLOB file with a filename""" -class INamedImage(INamed, IImage): +class INamedImage(INamedTyped, IImage): """A non-BLOB image with a filename""" diff --git a/plone/namedfile/tests/test_storable.py b/plone/namedfile/tests/test_storable.py index 8e8309c..7e4f214 100644 --- a/plone/namedfile/tests/test_storable.py +++ b/plone/namedfile/tests/test_storable.py @@ -18,6 +18,8 @@ from OFS.Image import Pdata from plone.namedfile.file import FileChunk from plone.namedfile.file import NamedBlobImage +from plone.namedfile.file import NamedBlobFile +from plone.namedfile import field from plone.namedfile.testing import PLONE_NAMEDFILE_FUNCTIONAL_TESTING from plone.namedfile.tests import getFile @@ -56,3 +58,44 @@ def test_opened_file_storable(self): if os.path.exists(path): os.remove(path) self.assertEqual(303, fi.getSize()) + + def test_upload_no_read(self): + # ensure we don't read the whole file into memory + + import ZODB.blob + + old_open = ZODB.blob.Blob.open + blob_read = 0 + blob_write = 0 + + def count_open(self, mode="r"): + nonlocal blob_read, blob_write + blob_read += 1 if "r" in mode else 0 + blob_write += 1 if "w" in mode else 0 + return old_open(self, mode) + + with unittest.mock.patch.object(ZODB.blob.Blob, 'open', count_open): + data = getFile("image.gif") + f = tempfile.NamedTemporaryFile(delete=False) + try: + path = f.name + f.write(data) + f.close() + with open(path, "rb") as f: + fi = NamedBlobFile(f, filename="image.gif") + finally: + if os.path.exists(path): + os.remove(path) + self.assertEqual(303, fi.getSize()) + self.assertEqual(blob_read, 1, "blob should have only been opened to get size") + self.assertEqual( + blob_write, + 1, + "Slow write to blob instead of os rename. Should be only 1 on init", + ) + blob_read = 0 + + blob_field = field.NamedBlobFile() + blob_field.validate(fi) + + self.assertEqual(blob_read, 0, "Validation is reading the whole blob in memory")