Merge lp:~renamer-developers/renamer/842236-unicode-filenames into lp:renamer

[Jonathan Jacobs]

[Tristan Seligmann (mithrandi)]

26 + # If we put non-ASCII unicode in here, then log.msg just neglects to emit
27 + # it for whatever bizarre reason.

I don't think it so much "neglects to emit it" as "blows up with a UnicodeDecodeError" due to unicode->str coercion. Either way, calling log.msg with a unicode object is not support, so we definitely shouldn't be doing it.

30 + getattr(sys.stdin, 'encoding', None) or sys.getdefaultencoding())

Shouldn't this be sys.stdout? This is kinda awful anyway, but I suppose there's no better way to do it.

107 - Look up TV episode metadata on TV Rage.
108 + Construct the TV Rage URL to the quickinfo page for the seriesName,
109 + season and episode.

The site seems to refer to itself as "TVRage" not "TV Rage", this needs to be fixed in a few places.

237 +class BodyReceiver(Protocol):

_buffer needs to be documented.

261 + def connectionLost(self, reason):
262 + data = self._buffer.getvalue().decode(self.encoding)
263 + self.finished.callback(data)

This needs to do error handling. Something like:

def connectionLost(self, reason):
    if reason.check(PotentialDataLoss, RequestDone) is None:
        self.finished.errback(reason)
    else:
        data = self._buffer.getvalue().decode(self.encoding)
        self.finished.callback(data)

review: Needs Fixing

[lp:~renamer-developers/renamer/842236-unicode-filenames]

91. By Jonathan Jacobs on 2011-09-14

Address review commentary.

[Jonathan Jacobs (jjacobs)]

Addressed review commentary.

[Tristan Seligmann (mithrandi)]

227 + lines = self.dataPath.child('tvrage').open().read()

Instead of this, do:

    lines = self.dataPath.child('tvrage').getContent()

Aside from reading more nicely, that will also ensure that the file handle is closed.

Other than that, this looks good to merge.

review: Needs Fixing

[lp:~renamer-developers/renamer/842236-unicode-filenames]

92. By Jonathan Jacobs on 2011-09-16

Review commentary.

=== modified file 'renamer/application.py'
--- renamer/application.py	2010-11-01 08:48:08 +0000
+++ renamer/application.py	2011-09-16 07:58:24 +0000
@@ -65,6 +65,13 @@
             os.path.basename(sys.argv[0]),)
 
 
+    def postOptions(self):
+        if self['name'] is not None:
+            self['name'] = self.decodeCommandLine(self['name'])
+        if self['prefix'] is not None:
+            self['prefix'] = self.decodeCommandLine(self['prefix'])
+
+
     def opt_verbose(self):
         """
         Increase output, use more times for greater effect.

=== modified file 'renamer/logging.py'
--- renamer/logging.py	2010-10-04 14:18:07 +0000
+++ renamer/logging.py	2011-09-16 07:58:24 +0000
@@ -42,6 +42,11 @@
 
 
 def msg(message, **kw):
+    # Passing unicode to log.msg is not supported, don't do it.
+    if isinstance(message, unicode):
+        codec = (
+            getattr(sys.stdout, 'encoding', None) or sys.getdefaultencoding())
+        message = message.encode(codec, 'replace')
     log.msg(message, source='renamer', **kw)
 
 

=== modified file 'renamer/plugins/audio.py'
--- renamer/plugins/audio.py	2010-10-15 08:51:27 +0000
+++ renamer/plugins/audio.py	2011-09-16 07:58:24 +0000
@@ -30,11 +30,11 @@
 
 
     defaultPrefixTemplate = string.Template(
-        '${artist}/${album} (${date})')
+        u'${artist}/${album} (${date})')
 
 
     defaultNameTemplate = string.Template(
-        '${tracknumber}. ${title}')
+        u'${tracknumber}. ${title}')
 
 
     def postOptions(self):

=== modified file 'renamer/plugins/tv.py'
--- renamer/plugins/tv.py	2010-10-27 00:48:13 +0000
+++ renamer/plugins/tv.py	2011-09-16 07:58:24 +0000
@@ -9,11 +9,13 @@
 except ImportError:
     pymeta = None
 
-from twisted.web.client import getPage
+from twisted.internet import reactor
+from twisted.web.client import Agent
 
 from renamer import logging
 from renamer.plugin import RenamingCommand
 from renamer.errors import PluginError
+from renamer.util import deliverBody, BodyReceiver
 try:
     from renamer._compiled_grammar.tv import Parser as FilenameGrammar
     FilenameGrammar # Ssssh, Pyflakes.
@@ -69,7 +71,7 @@
 
 
 
-if FilenameGrammar is None:
+if pymeta is not None and FilenameGrammar is None:
     class FilenameGrammar(OMeta.makeGrammar(filenameGrammar, globals())):
         pass
 
@@ -79,12 +81,12 @@
     name = 'tvrage'
 
 
-    description = 'Rename TV episodes with TV Rage metadata.'
+    description = 'Rename TV episodes with TVRage metadata.'
 
 
     longdesc = """
     Extract TV episode information from filenames and rename them based on the
-    correct information from TV Rage <http://tvrage.com/>.
+    correct information from TVRage <http://tvrage.com/>.
 
     Available placeholders for templates are:
 
@@ -93,7 +95,7 @@
 
 
     defaultNameTemplate = string.Template(
-        '$series [${season}x${padded_episode}] - $title')
+        u'$series [${season}x${padded_episode}] - $title')
 
 
     optParameters = [
@@ -106,6 +108,9 @@
         if pymeta is None:
             raise PluginError(
                 'The "pymeta" package is required for this command')
+        if self['series'] is not None:
+            self['series'] = self.decodeCommandLine(self['series'])
+        self.agent = Agent(reactor)
 
 
     def buildMapping(self, (seriesName, season, episode, episodeName)):
@@ -165,7 +170,7 @@
 
     def extractMetadata(self, pageData):
         """
-        Extract TV episode metadata from a TV Rage response.
+        Extract TV episode metadata from a TVRage response.
         """
         data = {}
         for line in pageData.splitlines():
@@ -178,16 +183,28 @@
         return series, season, episode, title
 
 
-    def lookupMetadata(self, seriesName, season, episode, fetcher=getPage):
+    def buildURL(self, seriesName, season, episode):
         """
-        Look up TV episode metadata on TV Rage.
+        Construct the TVRage URL to the quickinfo page for the seriesName,
+        season and episode.
         """
         ep = '%dx%02d' % (int(season), int(episode))
         qs = urllib.urlencode({'show': seriesName, 'ep': ep})
-        url = 'http://services.tvrage.com/tools/quickinfo.php?%s' % (qs,)
-        logging.msg('Looking up TV Rage metadata at %s' % (url,),
+        return 'http://services.tvrage.com/tools/quickinfo.php?%s' % (qs,)
+
+
+    def lookupMetadata(self, seriesName, season, episode):
+        """
+        Look up TV episode metadata on TVRage.
+        """
+        url = self.buildURL(seriesName, season, episode)
+        logging.msg('Looking up TVRage metadata at %s' % (url,),
                     verbosity=4)
-        return fetcher(url).addCallback(self.extractMetadata)
+
+        d = self.agent.request('GET', url)
+        d.addCallback(deliverBody, BodyReceiver)
+        d.addCallback(self.extractMetadata)
+        return d
 
 
     # IRenamerCommand

=== modified file 'renamer/test/test_tvrage.py'
--- renamer/test/test_tvrage.py	2010-10-26 22:39:58 +0000
+++ renamer/test/test_tvrage.py	2011-09-16 07:58:24 +0000
@@ -1,7 +1,6 @@
 import cgi
 import urllib
 
-from twisted.internet.defer import succeed
 from twisted.python.filepath import FilePath
 from twisted.trial.unittest import TestCase
 
@@ -50,14 +49,6 @@
         self.plugin.postOptions()
 
 
-    def fetcher(self, url):
-        """
-        "Fetch" TV rage data.
-        """
-        data = self.dataPath.child('tvrage').open().read()
-        return succeed(data)
-
-
     def test_extractParts(self):
         """
         Extracting TV show information from filenames works correctly.
@@ -120,7 +111,7 @@
 
     def test_missingPyMeta(self):
         """
-        Attempting to use the TV Rage plugin without PyMeta installed raises a
+        Attempting to use the TVRage plugin without PyMeta installed raises a
         L{renamer.errors.PluginError}.
         """
         self.patch(tv, 'pymeta', None)
@@ -134,31 +125,24 @@
     def test_extractMetadata(self):
         """
         L{renamer.plugins.tv.TVRage.extractMetadata} extracts structured TV
-        episode information from a TV Rage response.
-        """
-        d = self.plugin.lookupMetadata('Dexter', 1, 2, fetcher=self.fetcher)
-
-        @d.addCallback
-        def checkMetadata((series, season, episode, title)):
-            self.assertEquals(series, u'Dexter')
-            self.assertEquals(season, 1)
-            self.assertEquals(episode, 2)
-            self.assertEquals(title, u'Crocodile')
-
-        return d
-
-
-    def test_lookupMetadata(self):
-        """
-        L{renamer.plugins.tv.TVRage.lookupMetadata} requests structured TV
-        episode information from TV Rage.
-        """
-        def fetcher(url):
-            path, query = urllib.splitquery(url)
-            query = cgi.parse_qs(query)
-            self.assertEquals(
-                query,
-                dict(show=['Dexter'], ep=['1x02']))
-            return self.fetcher(url)
-
-        return self.plugin.lookupMetadata('Dexter', 1, 2, fetcher=fetcher)
+        episode information from a TVRage response.
+        """
+        lines = self.dataPath.child('tvrage').getContent()
+        series, season, episode, title = self.plugin.extractMetadata(lines)
+        self.assertEquals(series, u'Dexter')
+        self.assertEquals(season, 1)
+        self.assertEquals(episode, 2)
+        self.assertEquals(title, u'Crocodile')
+
+
+    def test_buildURL(self):
+        """
+        L{renamer.plugins.tv.TVRage.buildURL} constructs a URL for the TVRage
+        quickinfo API to a specific series, season and episode.
+        """
+        url = self.plugin.buildURL('Dexter', 1, 2)
+        path, query = urllib.splitquery(url)
+        query = cgi.parse_qs(query)
+        self.assertEquals(
+            query,
+            dict(show=['Dexter'], ep=['1x02']))

=== modified file 'renamer/util.py'
--- renamer/util.py	2010-10-31 19:34:03 +0000
+++ renamer/util.py	2011-09-16 07:58:24 +0000
@@ -1,12 +1,17 @@
+import cgi
 import errno
 import glob
 import itertools
 import os
 import sys
+from StringIO import StringIO
 from zope.interface import alsoProvides
 
-from twisted.internet.defer import DeferredList
+from twisted.internet.defer import DeferredList, Deferred
 from twisted.internet.task import Cooperator
+from twisted.internet.protocol import Protocol
+from twisted.web.client import ResponseDone
+from twisted.web.http import PotentialDataLoss
 
 from renamer import errors
 
@@ -117,3 +122,52 @@
     """
     return itertools.islice(
         itertools.chain(iterable, itertools.repeat(padding)), count)
+
+
+
+class BodyReceiver(Protocol):
+    """
+    Body receiver, suitable for use with L{IResponse.deliverBody}.
+
+    @type finished: C{Deferred<unicode>}
+    @ivar finished: User-specified deferred that is fired with the decoded body
+        text, stored in L{_buffer}, when the body has been entirely delivered.
+
+    @type encoding: C{str}
+    @ivar encoding: Body text encoding, as specified in the I{'Content-Type'}
+        header, defaults to C{'UTF-8'}.
+
+    @type _buffer: C{StringIO}
+    @ivar _buffer: Delivered body content buffer.
+    """
+    def __init__(self, response, finished):
+        header, args = cgi.parse_header(
+            response.headers.getRawHeaders('Content-Type', default=[''])[0])
+        self.encoding = args.get('charset', 'utf-8')
+        self.finished = finished
+        self._buffer = StringIO()
+
+
+    def dataReceived(self, data):
+        self._buffer.write(data)
+
+
+    def connectionLost(self, reason):
+        if reason.check(PotentialDataLoss, ResponseDone) is None:
+            self.finished.errback(reason)
+        else:
+            data = self._buffer.getvalue().decode(self.encoding)
+            self.finished.callback(data)
+
+
+
+def deliverBody(response, cls):
+    """
+    Invoke C{response.deliverBody} with C{cls(response, deferred)}.
+
+    @rtype: C{Deferred}
+    @return: A deferred that fires when the instance of C{cls} callbacks it.
+    """
+    finished = Deferred()
+    response.deliverBody(cls(response, finished))
+    return finished