Clean up _translate_msgid logic a bit

Streamline the logic in _translate_msgid to a series of cascading
if/elif statements to avoid having several different return points. Add
some test coverage for situations not handled by existing test code.

Change-Id: I9e0343757f515116c07f7f3fe1eab078046555e0

oslo_i18n/_message.py

@@ -110,15 +110,17 @@ class Message(six.text_type):
                                    languages=[desired_locale],
                                    fallback=True)
 
-        # Primary translation function
         if not has_contextual_form and not has_plural_form:
+            # This is the most common case, so check it first.
             translator = lang.gettext if six.PY3 else lang.ugettext
-
             translated_message = translator(msgid)
-            return translated_message
 
-        # Contextual translation function
-        if has_contextual_form and not has_plural_form:
+        elif has_contextual_form and has_plural_form:
+            # Reserved for contextual and plural translation function,
+            # which is not yet implemented.
+            raise ValueError("Unimplemented.")
+
+        elif has_contextual_form:
             (msgctx, msgtxt) = msgid
             translator = lang.gettext if six.PY3 else lang.ugettext
 
@@ -126,24 +128,15 @@ class Message(six.text_type):
             translated_message = translator(msg_with_ctx)
 
             if CONTEXT_SEPARATOR in translated_message:
-                # Translation not found
+                # Translation not found, use the original text
                 translated_message = msgtxt
 
-            return translated_message
-
-        # Plural translation function
-        if not has_contextual_form and has_plural_form:
+        elif has_plural_form:
             (msgsingle, msgplural, msgcount) = msgid
             translator = lang.ngettext if six.PY3 else lang.ungettext
-
             translated_message = translator(msgsingle, msgplural, msgcount)
-            return translated_message
 
-        # Reserved for contextual and plural translation function
-        if has_contextual_form and has_plural_form:
-            raise ValueError("Unimplemented.")
-
-        raise TypeError("Unknown msgid type.")
+        return translated_message
 
     def __mod__(self, other):
         # When we mod a Message we want the actual operation to be performed

oslo_i18n/tests/test_message.py

@@ -516,3 +516,73 @@ class MessageTestCase(test_base.BaseTestCase):
         # Translation into other locales still works
         self.assertEqual(zh_translation, msg.translate('zh'))
         self.assertEqual(fr_translation, msg.translate('fr'))
+
+
+class TranslateMsgidTest(test_base.BaseTestCase):
+
+    @mock.patch('gettext.translation')
+    def test_contextual(self, translation):
+        lang = mock.Mock()
+        translation.return_value = lang
+        trans = mock.Mock()
+        trans.return_value = 'translated'
+        lang.gettext = trans
+        lang.ugettext = trans
+        result = _message.Message._translate_msgid(
+            ('context', 'message'),
+            domain='domain',
+            has_contextual_form=True,
+            has_plural_form=False,
+        )
+        self.assertEqual('translated', result)
+        trans.assert_called_with(
+            'context' + _message.CONTEXT_SEPARATOR + 'message'
+        )
+
+    @mock.patch('gettext.translation')
+    def test_contextual_untranslatable(self, translation):
+        msg_with_context = 'context' + _message.CONTEXT_SEPARATOR + 'message'
+        lang = mock.Mock()
+        translation.return_value = lang
+        trans = mock.Mock()
+        trans.return_value = msg_with_context
+        lang.gettext = trans
+        lang.ugettext = trans
+        result = _message.Message._translate_msgid(
+            ('context', 'message'),
+            domain='domain',
+            has_contextual_form=True,
+            has_plural_form=False,
+        )
+        self.assertEqual('message', result)
+        trans.assert_called_with(msg_with_context)
+
+    @mock.patch('gettext.translation')
+    def test_plural(self, translation):
+        lang = mock.Mock()
+        translation.return_value = lang
+        trans = mock.Mock()
+        trans.return_value = 'translated'
+        lang.ngettext = trans
+        lang.ungettext = trans
+        result = _message.Message._translate_msgid(
+            ('single', 'plural', -1),
+            domain='domain',
+            has_contextual_form=False,
+            has_plural_form=True,
+        )
+        self.assertEqual('translated', result)
+        trans.assert_called_with(
+            'single', 'plural', -1,
+        )
+
+    @mock.patch('gettext.translation')
+    def test_contextual_and_plural(self, translation):
+        self.assertRaises(
+            ValueError,
+            _message.Message._translate_msgid,
+            'nothing',
+            domain='domain',
+            has_contextual_form=True,
+            has_plural_form=True,
+        )