issue #216

Author: maxtepkeev

CHANGELOG.rst

@@ -4,9 +4,16 @@ Changelog
 2.1.2 (2019-XX-XX)
 ++++++++++++++++++
 
+**Improvements**:
+
+- ``PerformanceWarning`` will be issued when Python-Redmine does some unnecessary work under the hood to fix the
+  clients code problems
+
 **Bugfixes**:
 
-- ``session()`` doesn't restore previous engine if fails (`Issue #211 <https://github.com/maxtepkeev/
+- ``Redmine.upload()`` fails under certain circumstances when used with a file-like object and it contains unicode
+  instead of bytes (`Issue #216 <https://github.com/maxtepkeev/python-redmine/issues/216>`__)
+- ``Redmine.session()`` doesn't restore previous engine if fails (`Issue #211 <https://github.com/maxtepkeev/
   python-redmine/issues/211>`__) (thanks to `Dmitry Logvinenko <https://github.com/dm-logv>`__)
 
 2.1.1 (2018-05-02)

docs/resources/issue.rst

@@ -56,7 +56,7 @@ create
 
 .. code-block:: python
 
-   >>> from io import StringIO
+   >>> from io import BytesIO
    >>> issue = redmine.issue.create(
    ...     project_id='vacation',
    ...     subject='Vacation',
@@ -72,7 +72,7 @@ create
    ...     estimated_hours=4,
    ...     done_ratio=40,
    ...     custom_fields=[{'id': 1, 'value': 'foo'}, {'id': 2, 'value': 'bar'}],
-   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': StringIO('I am content of file 2')}]
+   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': BytesIO(b'I am content of file 2')}]
    ... )
    >>> issue
    <redminelib.resources.Issue #123 "Vacation">
@@ -323,7 +323,7 @@ update
 
 .. code-block:: python
 
-   >>> from io import StringIO
+   >>> from io import BytesIO
    >>> redmine.issue.update(
    ...     1,
    ...     project_id=1,
@@ -341,7 +341,7 @@ update
    ...     estimated_hours=4,
    ...     done_ratio=40,
    ...     custom_fields=[{'id': 1, 'value': 'foo'}, {'id': 2, 'value': 'bar'}],
-   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': StringIO('I am content of file 2')}]
+   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': BytesIO(b'I am content of file 2')}]
    ... )
    True
 

docs/resources/wiki_page.rst

@@ -42,14 +42,14 @@ create
 
 .. code-block:: python
 
-   >>> from io import StringIO
+   >>> from io import BytesIO
    >>> wiki_page = redmine.wiki_page.create(
    ...     project_id='vacation',
    ...     title='FooBar',
    ...     text='foo',
    ...     parent_title='Yada',
    ...     comments='bar',
-   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': StringIO('I am content of file 2')}]
+   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': BytesIO(b'I am content of file 2')}]
    ... )
    >>> wiki_page
    <redminelib.resources.WikiPage "FooBar">
@@ -194,15 +194,15 @@ update
 
 .. code-block:: python
 
-   >>> from io import StringIO
+   >>> from io import BytesIO
    >>> redmine.wiki_page.update(
    ...     'Foo',
    ...     project_id='vacation',
    ...     title='FooBar',
    ...     text='foo',
    ...     parent_title='Yada',
    ...     comments='bar',
-   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': StringIO('I am content of file 2')}]
+   ...     uploads=[{'path': '/absolute/path/to/file'}, {'path': BytesIO(b'I am content of file 2')}]
    ... )
    True
 

redminelib/__init__.py

@@ -3,7 +3,10 @@
 """
 
 import os
+import io
+import sys
 import inspect
+import warnings
 import contextlib
 
 from distutils.version import LooseVersion
@@ -103,6 +106,19 @@ def upload(self, f):
         # stream ourselves as we have no idea of what the client is going to do with it afterwards, so we
         # leave the closing part to the client or to the garbage collector
         if hasattr(f, 'close'):
+            try:
+                c = f.read(0)
+            except (AttributeError, TypeError):
+                raise exceptions.FileObjectError
+
+            # We need to send bytes over the socket, so in case a file-like object contains a unicode
+            # object underneath, we need to convert it to bytes, otherwise we'll get an exception
+            if isinstance(c, str if sys.version_info[0] >= 3 else unicode):
+                warnings.warn("File-like object contains unicode, hence an additional step is performed to convert "
+                              "it's content to bytes, please consider switching to bytes to eliminate this warning",
+                              exceptions.PerformanceWarning)
+                f = io.BytesIO(f.read().encode('utf-8'))
+
             stream = f
             close = False
         else:

redminelib/exceptions.py

@@ -1,11 +1,24 @@
 """
 Python-Redmine tries it's best to provide human readable errors in all situations.
-This is a list of all exceptions that Python-Redmine can throw.
+This is a list of all exceptions or warnings that Python-Redmine can throw/raise.
 """
 
 from . import utilities
 
 
+@utilities.fix_unicode
+class BaseRedmineWarning(Warning):
+    """
+    Base warning class for Redmine warnings.
+    """
+
+
+class PerformanceWarning(BaseRedmineWarning):
+    """
+    Warning raised when there's a possible performance impact.
+    """
+
+
 @utilities.fix_unicode
 class BaseRedmineError(Exception):
     """
@@ -29,6 +42,14 @@ def __init__(self):
         super(NoFileError, self).__init__("Can't upload a file that doesn't exist or is empty")
 
 
+class FileObjectError(BaseRedmineError):
+    """
+    File-like object isn't supported as it doesn't support the read(size) method.
+    """
+    def __init__(self):
+        super(FileObjectError, self).__init__("File-like object doesn't support the read(size) method")
+
+
 class ResourceNotFoundError(BaseRedmineError):
     """
     Requested resource doesn't exist.

tests/test_managers.py

@@ -1,3 +1,5 @@
+import warnings
+
 from . import mock, BaseRedmineTestCase
 from .responses import responses
 
@@ -120,7 +122,11 @@ def test_create_resource_with_stream_uploads(self):
             'issue': {'subject': 'Foo', 'project_id': 1, 'id': 1}
         }
         stream = StringIO(b'\xcf\x86oo'.decode('utf8'))
-        issue = self.redmine.issue.create(project_id=1, subject='Foo', uploads=[{'path': stream}])
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            issue = self.redmine.issue.create(project_id=1, subject='Foo', uploads=[{'path': stream}])
+            self.assertEquals(len(w), 1)
+            self.assertIs(w[0].category, exceptions.PerformanceWarning)
         self.assertEqual(issue.project_id, 1)
         self.assertEqual(issue.subject, 'Foo')
 
@@ -153,7 +159,11 @@ def test_update_resource_with_stream_uploads(self):
             mock.Mock(status_code=200, history=[], content='')
         ])
         stream = StringIO(b'\xcf\x86oo'.decode('utf8'))
-        self.assertEqual(self.redmine.issue.update(1, subject='Bar', uploads=[{'path': stream}]), True)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            self.assertEqual(self.redmine.issue.update(1, subject='Bar', uploads=[{'path': stream}]), True)
+            self.assertEquals(len(w), 1)
+            self.assertIs(w[0].category, exceptions.PerformanceWarning)
 
     def test_delete_resource(self):
         self.response.content = ''

tests/test_redmine.py

@@ -1,3 +1,5 @@
+import warnings
+
 from . import mock, BaseRedmineTestCase, Redmine
 
 from redminelib import engines, resultsets, exceptions
@@ -66,7 +68,11 @@ def test_successful_filestream_upload(self):
         from io import StringIO
         self.response.status_code = 201
         self.response.json.return_value = {'upload': {'id': 1, 'token': '456789'}}
-        self.assertEqual(self.redmine.upload(StringIO(b'\xcf\x86oo'.decode('utf8')))['token'], '456789')
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            self.assertEqual(self.redmine.upload(StringIO(b'\xcf\x86oo'.decode('utf8')))['token'], '456789')
+            self.assertEquals(len(w), 1)
+            self.assertIs(w[0].category, exceptions.PerformanceWarning)
 
     @mock.patch('redminelib.open', mock.mock_open(), create=True)
     def test_successful_file_download(self):
@@ -86,6 +92,10 @@ def test_file_url_exception(self):
     def test_file_upload_no_file_exception(self):
         self.assertRaises(exceptions.NoFileError, lambda: self.redmine.upload('foo',))
 
+    def test_file_upload_file_object_exception(self):
+        f = type('FileObject', (), {'close': lambda obj: None})()
+        self.assertRaises(exceptions.FileObjectError, lambda: self.redmine.upload(f))
+
     def test_file_upload_not_supported_exception(self):
         self.redmine.ver = '1.0.0'
         self.assertRaises(exceptions.VersionMismatchError, lambda: self.redmine.upload('foo',))