Bug 1451733 - [mozprofile] Clean up the public facing addons API a bit r=jmaher
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Thu, 05 Apr 2018 12:04:21 -0400
changeset 778785 5a16ea729deb5ff9d89b9fd5575d23c57783b9fc
parent 778784 538664e0c82226505628be47a04ff475cdd7f893
child 778786 688370d3982d69df54ffa07aabfdc158e40f2b17
push id105582
push userkgupta@mozilla.com
push dateFri, 06 Apr 2018 21:31:13 +0000
reviewersjmaher
bugs1451733
milestone61.0a1
Bug 1451733 - [mozprofile] Clean up the public facing addons API a bit r=jmaher While we are removing a bunch of stuff and breaking backwards compatibility, I figured this would be a good time to also change some of the APIs. These APIs aren't used much in mozilla-central (and this patch updates the few places that do). This rolls the 'install_addons()' and 'install_addon_from_path' method into a single 'install' method. This install method can accept a string or list of paths to an individual addon (directory or .xpi), or a directory containing addons. This also renames Profile.addon_manager to Profile.addons, which reads better. MozReview-Commit-ID: 7vDPnG4cKqu
testing/mozbase/mozprofile/mozprofile/addons.py
testing/mozbase/mozprofile/mozprofile/profile.py
testing/mozbase/mozprofile/tests/test_addons.py
testing/talos/talos/ffsetup.py
--- a/testing/mozbase/mozprofile/mozprofile/addons.py
+++ b/testing/mozbase/mozprofile/mozprofile/addons.py
@@ -115,30 +115,95 @@ class AddonManager(object):
         :param addon_path: path to the add-on directory or XPI
         """
         try:
             self.addon_details(addon_path)
             return True
         except AddonFormatError:
             return False
 
-    def install_addons(self, addons):
+    def _install_addon(self, path, unpack=False):
+        addons = [path]
+
+        # if path is not an add-on, try to install all contained add-ons
+        try:
+            self.addon_details(path)
+        except AddonFormatError as e:
+            module_logger.warning('Could not install %s: %s' % (path, str(e)))
+
+            # If the path doesn't exist, then we don't really care, just return
+            if not os.path.isdir(path):
+                return
+
+            addons = [os.path.join(path, x) for x in os.listdir(path) if
+                      self.is_addon(os.path.join(path, x))]
+            addons.sort()
+
+        # install each addon
+        for addon in addons:
+            # determine the addon id
+            addon_details = self.addon_details(addon)
+            addon_id = addon_details.get('id')
+
+            # if the add-on has to be unpacked force it now
+            # note: we might want to let Firefox do it in case of addon details
+            orig_path = None
+            if os.path.isfile(addon) and (unpack or addon_details['unpack']):
+                orig_path = addon
+                addon = tempfile.mkdtemp()
+                mozfile.extract(orig_path, addon)
+
+            # copy the addon to the profile
+            extensions_path = os.path.join(self.profile, 'extensions')
+            addon_path = os.path.join(extensions_path, addon_id)
+
+            if os.path.isfile(addon):
+                addon_path += '.xpi'
+
+                # move existing xpi file to backup location to restore later
+                if os.path.exists(addon_path):
+                    self.backup_dir = self.backup_dir or tempfile.mkdtemp()
+                    shutil.move(addon_path, self.backup_dir)
+
+                # copy new add-on to the extension folder
+                if not os.path.exists(extensions_path):
+                    os.makedirs(extensions_path)
+                shutil.copy(addon, addon_path)
+            else:
+                # move existing folder to backup location to restore later
+                if os.path.exists(addon_path):
+                    self.backup_dir = self.backup_dir or tempfile.mkdtemp()
+                    shutil.move(addon_path, self.backup_dir)
+
+                # copy new add-on to the extension folder
+                shutil.copytree(addon, addon_path, symlinks=True)
+
+            # if we had to extract the addon, remove the temporary directory
+            if orig_path:
+                mozfile.remove(addon)
+                addon = orig_path
+
+            self._addons.append(addon_id)
+            self.installed_addons.append(addon)
+
+    def install(self, addons, **kwargs):
         """
-        Installs all types of addons
+        Installs addons from a filepath or directory of addons in the profile.
 
-        :param addons: a list of addon paths to install
+        :param addons: paths to .xpi or addon directories
+        :param unpack: whether to unpack unless specified otherwise in the install.rdf
         """
         if not addons:
             return
 
         # install addon paths
         if isinstance(addons, string_types):
             addons = [addons]
         for addon in set(addons):
-            self.install_from_path(addon)
+            self._install_addon(addon, **kwargs)
 
     @classmethod
     def _gen_iid(cls, addon_path):
         hash = hashlib.sha1(_SALT)
         hash.update(addon_path)
         return hash.hexdigest() + _TEMPORARY_ADDON_SUFFIX
 
     @classmethod
@@ -251,85 +316,15 @@ class AddonManager(object):
             details['unpack'] = details['unpack'].lower() == 'true'
 
         # If no ID is set, the add-on is invalid
         if details.get('id') is None and not is_webext:
             raise AddonFormatError('Add-on id could not be found.')
 
         return details
 
-    def install_from_path(self, path, unpack=False):
-        """
-        Installs addon from a filepath or directory of addons in the profile.
-
-        :param path: path to .xpi or directory of addons
-        :param unpack: whether to unpack unless specified otherwise in the install.rdf
-        """
-        addons = [path]
-
-        # if path is not an add-on, try to install all contained add-ons
-        try:
-            self.addon_details(path)
-        except AddonFormatError as e:
-            module_logger.warning('Could not install %s: %s' % (path, str(e)))
-
-            # If the path doesn't exist, then we don't really care, just return
-            if not os.path.isdir(path):
-                return
-
-            addons = [os.path.join(path, x) for x in os.listdir(path) if
-                      self.is_addon(os.path.join(path, x))]
-            addons.sort()
-
-        # install each addon
-        for addon in addons:
-            # determine the addon id
-            addon_details = self.addon_details(addon)
-            addon_id = addon_details.get('id')
-
-            # if the add-on has to be unpacked force it now
-            # note: we might want to let Firefox do it in case of addon details
-            orig_path = None
-            if os.path.isfile(addon) and (unpack or addon_details['unpack']):
-                orig_path = addon
-                addon = tempfile.mkdtemp()
-                mozfile.extract(orig_path, addon)
-
-            # copy the addon to the profile
-            extensions_path = os.path.join(self.profile, 'extensions')
-            addon_path = os.path.join(extensions_path, addon_id)
-
-            if os.path.isfile(addon):
-                addon_path += '.xpi'
-
-                # move existing xpi file to backup location to restore later
-                if os.path.exists(addon_path):
-                    self.backup_dir = self.backup_dir or tempfile.mkdtemp()
-                    shutil.move(addon_path, self.backup_dir)
-
-                # copy new add-on to the extension folder
-                if not os.path.exists(extensions_path):
-                    os.makedirs(extensions_path)
-                shutil.copy(addon, addon_path)
-            else:
-                # move existing folder to backup location to restore later
-                if os.path.exists(addon_path):
-                    self.backup_dir = self.backup_dir or tempfile.mkdtemp()
-                    shutil.move(addon_path, self.backup_dir)
-
-                # copy new add-on to the extension folder
-                shutil.copytree(addon, addon_path, symlinks=True)
-
-            # if we had to extract the addon, remove the temporary directory
-            if orig_path:
-                mozfile.remove(addon)
-                addon = orig_path
-
-            self._addons.append(addon_id)
-            self.installed_addons.append(addon)
-
     def remove_addon(self, addon_id):
         """Remove the add-on as specified by the id
 
         :param addon_id: id of the add-on to be removed
         """
         path = self.get_addon_path(addon_id)
         mozfile.remove(path)
--- a/testing/mozbase/mozprofile/mozprofile/profile.py
+++ b/testing/mozbase/mozprofile/mozprofile/profile.py
@@ -127,18 +127,18 @@ class Profile(object):
                 ))
             else:
                 prefs_js.append(("security.sandbox.content.read_path_whitelist",
                                  ",".join(self._whitelistpaths)))
         self.set_preferences(prefs_js, 'prefs.js')
         self.set_preferences(user_js)
 
         # handle add-on installation
-        self.addon_manager = AddonManager(self.profile, restore=self.restore)
-        self.addon_manager.install_addons(self._addons)
+        self.addons = AddonManager(self.profile, restore=self.restore)
+        self.addons.install(self._addons)
 
     def __enter__(self):
         return self
 
     def __exit__(self, type, value, traceback):
         self.cleanup()
 
     def __del__(self):
--- a/testing/mozbase/mozprofile/tests/test_addons.py
+++ b/testing/mozbase/mozprofile/tests/test_addons.py
@@ -25,229 +25,229 @@ here = os.path.dirname(os.path.abspath(_
 class TestAddonsManager(unittest.TestCase):
     """ Class to test mozprofile.addons.AddonManager """
 
     def setUp(self):
         self.logger = mozlog.getLogger('mozprofile.addons')
         self.logger.setLevel(mozlog.ERROR)
 
         self.profile = mozprofile.profile.Profile()
-        self.am = self.profile.addon_manager
+        self.am = self.profile.addons
 
         self.profile_path = self.profile.profile
         self.tmpdir = tempfile.mkdtemp()
         self.addCleanup(mozfile.remove, self.tmpdir)
 
-    def test_install_addons_multiple_same_source(self):
+    def test_install_multiple_same_source(self):
         # Generate installer stubs for all possible types of addons
         addon_xpi = generate_addon('test-addon-1@mozilla.org',
                                    path=self.tmpdir)
         addon_folder = generate_addon('test-addon-1@mozilla.org',
                                       path=self.tmpdir,
                                       xpi=False)
 
         # The same folder should not be installed twice
-        self.am.install_addons([addon_folder, addon_folder])
+        self.am.install([addon_folder, addon_folder])
         self.assertEqual(self.am.installed_addons, [addon_folder])
         self.am.clean()
 
         # The same XPI file should not be installed twice
-        self.am.install_addons([addon_xpi, addon_xpi])
+        self.am.install([addon_xpi, addon_xpi])
         self.assertEqual(self.am.installed_addons, [addon_xpi])
         self.am.clean()
 
         # Even if it is the same id the add-on should be installed twice, if
         # specified via XPI and folder
-        self.am.install_addons([addon_folder, addon_xpi])
+        self.am.install([addon_folder, addon_xpi])
         self.assertEqual(len(self.am.installed_addons), 2)
         self.assertIn(addon_folder, self.am.installed_addons)
         self.assertIn(addon_xpi, self.am.installed_addons)
         self.am.clean()
 
     def test_install_webextension_from_dir(self):
         addon = os.path.join(here, 'addons', 'apply-css.xpi')
         zipped = zipfile.ZipFile(addon)
         try:
             zipped.extractall(self.tmpdir)
         finally:
             zipped.close()
-        self.am.install_from_path(self.tmpdir)
+        self.am.install(self.tmpdir)
         self.assertEqual(len(self.am.installed_addons), 1)
         self.assertTrue(os.path.isdir(self.am.installed_addons[0]))
 
     def test_install_webextension(self):
         addon = os.path.join(here, 'addons', 'apply-css.xpi')
-        self.am.install_from_path(addon)
+        self.am.install(addon)
         self.assertEqual(len(self.am.installed_addons), 1)
         self.assertTrue(os.path.isfile(self.am.installed_addons[0]))
         self.assertEqual('apply-css.xpi', os.path.basename(self.am.installed_addons[0]))
 
         details = self.am.addon_details(self.am.installed_addons[0])
         self.assertEqual('test-webext@quality.mozilla.org', details['id'])
 
     def test_install_webextension_sans_id(self):
         addon = os.path.join(here, 'addons', 'apply-css-sans-id.xpi')
-        self.am.install_from_path(addon)
+        self.am.install(addon)
 
         self.assertEqual(len(self.am.installed_addons), 1)
         self.assertTrue(os.path.isfile(self.am.installed_addons[0]))
         self.assertEqual('apply-css-sans-id.xpi', os.path.basename(self.am.installed_addons[0]))
 
         details = self.am.addon_details(self.am.installed_addons[0])
         self.assertIn('@temporary-addon', details['id'])
 
-    def test_install_from_path_xpi(self):
+    def test_install_xpi(self):
         addons_to_install = []
         addons_installed = []
 
         # Generate installer stubs and install them
         for ext in ['test-addon-1@mozilla.org', 'test-addon-2@mozilla.org']:
             temp_addon = generate_addon(ext, path=self.tmpdir)
             addons_to_install.append(self.am.addon_details(temp_addon)['id'])
-            self.am.install_from_path(temp_addon)
+            self.am.install(temp_addon)
 
         # Generate a list of addons installed in the profile
         addons_installed = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
                             self.profile.profile, 'extensions'))]
         self.assertEqual(addons_to_install.sort(), addons_installed.sort())
 
-    def test_install_from_path_folder(self):
+    def test_install_folder(self):
         # Generate installer stubs for all possible types of addons
         addons = []
         addons.append(generate_addon('test-addon-1@mozilla.org',
                                      path=self.tmpdir))
         addons.append(generate_addon('test-addon-2@mozilla.org',
                                      path=self.tmpdir,
                                      xpi=False))
         addons.append(generate_addon('test-addon-3@mozilla.org',
                                      path=self.tmpdir,
                                      name='addon-3'))
         addons.append(generate_addon('test-addon-4@mozilla.org',
                                      path=self.tmpdir,
                                      name='addon-4',
                                      xpi=False))
         addons.sort()
 
-        self.am.install_from_path(self.tmpdir)
+        self.am.install(self.tmpdir)
 
         self.assertEqual(self.am.installed_addons, addons)
 
-    def test_install_from_path_unpack(self):
+    def test_install_unpack(self):
         # Generate installer stubs for all possible types of addons
         addon_xpi = generate_addon('test-addon-unpack@mozilla.org',
                                    path=self.tmpdir)
         addon_folder = generate_addon('test-addon-unpack@mozilla.org',
                                       path=self.tmpdir,
                                       xpi=False)
         addon_no_unpack = generate_addon('test-addon-1@mozilla.org',
                                          path=self.tmpdir)
 
         # Test unpack flag for add-on as XPI
-        self.am.install_from_path(addon_xpi)
+        self.am.install(addon_xpi)
         self.assertEqual(self.am.installed_addons, [addon_xpi])
         self.am.clean()
 
         # Test unpack flag for add-on as folder
-        self.am.install_from_path(addon_folder)
+        self.am.install(addon_folder)
         self.assertEqual(self.am.installed_addons, [addon_folder])
         self.am.clean()
 
         # Test forcing unpack an add-on
-        self.am.install_from_path(addon_no_unpack, unpack=True)
+        self.am.install(addon_no_unpack, unpack=True)
         self.assertEqual(self.am.installed_addons, [addon_no_unpack])
         self.am.clean()
 
-    def test_install_from_path_after_reset(self):
+    def test_install_after_reset(self):
         # Installing the same add-on after a reset should not cause a failure
         addon = generate_addon('test-addon-1@mozilla.org',
                                path=self.tmpdir, xpi=False)
 
         # We cannot use self.am because profile.reset() creates a new instance
-        self.profile.addon_manager.install_from_path(addon)
+        self.profile.addons.install(addon)
 
         self.profile.reset()
 
-        self.profile.addon_manager.install_from_path(addon)
-        self.assertEqual(self.profile.addon_manager.installed_addons, [addon])
+        self.profile.addons.install(addon)
+        self.assertEqual(self.profile.addons.installed_addons, [addon])
 
-    def test_install_from_path_backup(self):
+    def test_install_backup(self):
         staged_path = os.path.join(self.profile_path, 'extensions')
 
         # Generate installer stubs for all possible types of addons
         addon_xpi = generate_addon('test-addon-1@mozilla.org',
                                    path=self.tmpdir)
         addon_folder = generate_addon('test-addon-1@mozilla.org',
                                       path=self.tmpdir,
                                       xpi=False)
         addon_name = generate_addon('test-addon-1@mozilla.org',
                                     path=self.tmpdir,
                                     name='test-addon-1-dupe@mozilla.org')
 
         # Test backup of xpi files
-        self.am.install_from_path(addon_xpi)
+        self.am.install(addon_xpi)
         self.assertIsNone(self.am.backup_dir)
 
-        self.am.install_from_path(addon_xpi)
+        self.am.install(addon_xpi)
         self.assertIsNotNone(self.am.backup_dir)
         self.assertEqual(os.listdir(self.am.backup_dir),
                          ['test-addon-1@mozilla.org.xpi'])
 
         self.am.clean()
         self.assertEqual(os.listdir(staged_path),
                          ['test-addon-1@mozilla.org.xpi'])
         self.am.clean()
 
         # Test backup of folders
-        self.am.install_from_path(addon_folder)
+        self.am.install(addon_folder)
         self.assertIsNone(self.am.backup_dir)
 
-        self.am.install_from_path(addon_folder)
+        self.am.install(addon_folder)
         self.assertIsNotNone(self.am.backup_dir)
         self.assertEqual(os.listdir(self.am.backup_dir),
                          ['test-addon-1@mozilla.org'])
 
         self.am.clean()
         self.assertEqual(os.listdir(staged_path),
                          ['test-addon-1@mozilla.org'])
         self.am.clean()
 
         # Test backup of xpi files with another file name
-        self.am.install_from_path(addon_name)
+        self.am.install(addon_name)
         self.assertIsNone(self.am.backup_dir)
 
-        self.am.install_from_path(addon_xpi)
+        self.am.install(addon_xpi)
         self.assertIsNotNone(self.am.backup_dir)
         self.assertEqual(os.listdir(self.am.backup_dir),
                          ['test-addon-1@mozilla.org.xpi'])
 
         self.am.clean()
         self.assertEqual(os.listdir(staged_path),
                          ['test-addon-1@mozilla.org.xpi'])
         self.am.clean()
 
-    def test_install_from_path_invalid_addons(self):
+    def test_install_invalid_addons(self):
         # Generate installer stubs for all possible types of addons
         addons = []
         addons.append(generate_addon('test-addon-invalid-no-manifest@mozilla.org',
                                      path=self.tmpdir,
                                      xpi=False))
         addons.append(generate_addon('test-addon-invalid-no-id@mozilla.org',
                                      path=self.tmpdir))
 
-        self.am.install_from_path(self.tmpdir)
+        self.am.install(self.tmpdir)
 
         self.assertEqual(self.am.installed_addons, [])
 
     @unittest.skip("Feature not implemented as part of AddonManger")
-    def test_install_from_path_error(self):
-        """ Check install_from_path raises an error with an invalid addon"""
+    def test_install_error(self):
+        """ Check install raises an error with an invalid addon"""
 
         temp_addon = generate_addon('test-addon-invalid-version@mozilla.org')
         # This should raise an error here
-        self.am.install_from_path(temp_addon)
+        self.am.install(temp_addon)
 
     def test_addon_details(self):
         # Generate installer stubs for a valid and invalid add-on manifest
         valid_addon = generate_addon('test-addon-1@mozilla.org',
                                      path=self.tmpdir)
         invalid_addon = generate_addon('test-addon-invalid-not-wellformed@mozilla.org',
                                        path=self.tmpdir)
 
@@ -271,26 +271,26 @@ class TestAddonsManager(unittest.TestCas
         self.assertRaises(mozprofile.addons.AddonFormatError,
                           self.am.addon_details, addon_path)
 
     @unittest.skip("Bug 900154")
     def test_clean_addons(self):
         addon_one = generate_addon('test-addon-1@mozilla.org')
         addon_two = generate_addon('test-addon-2@mozilla.org')
 
-        self.am.install_addons(addon_one)
+        self.am.install(addon_one)
         installed_addons = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
                             self.profile.profile, 'extensions'))]
 
         # Create a new profile based on an existing profile
         # Install an extra addon in the new profile
         # Cleanup addons
         duplicate_profile = mozprofile.profile.Profile(profile=self.profile.profile,
                                                        addons=addon_two)
-        duplicate_profile.addon_manager.clean()
+        duplicate_profile.addons.clean()
 
         addons_after_cleanup = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
                                 duplicate_profile.profile, 'extensions'))]
         # New addons installed should be removed by clean_addons()
         self.assertEqual(installed_addons, addons_after_cleanup)
 
     def test_noclean(self):
         """test `restore=True/False` functionality"""
@@ -306,17 +306,17 @@ class TestAddonsManager(unittest.TestCas
                 generate_addon('test-addon-1@mozilla.org', path=tmpdir),
                 os.path.join(here, 'addons', 'empty.xpi'),
             ]
 
             # install it with a restore=True AddonManager
             am = mozprofile.addons.AddonManager(profile, restore=True)
 
             for addon in addons:
-                am.install_from_path(addon)
+                am.install(addon)
 
             # now its there
             self.assertEqual(os.listdir(profile), ['extensions'])
             staging_folder = os.path.join(profile, 'extensions')
             self.assertTrue(os.path.exists(staging_folder))
             self.assertEqual(len(os.listdir(staging_folder)), 2)
 
             del am
@@ -330,17 +330,17 @@ class TestAddonsManager(unittest.TestCas
 
     def test_remove_addon(self):
         addons = []
         addons.append(generate_addon('test-addon-1@mozilla.org',
                                      path=self.tmpdir))
         addons.append(generate_addon('test-addon-2@mozilla.org',
                                      path=self.tmpdir))
 
-        self.am.install_from_path(self.tmpdir)
+        self.am.install(self.tmpdir)
 
         extensions_path = os.path.join(self.profile_path, 'extensions')
         staging_path = os.path.join(extensions_path)
 
         for addon in self.am._addons:
             self.am.remove_addon(addon)
 
         self.assertEqual(os.listdir(staging_path), [])
--- a/testing/talos/talos/ffsetup.py
+++ b/testing/talos/talos/ffsetup.py
@@ -120,17 +120,17 @@ class FFSetup(object):
                                 ignore=_feedback,
                                 restore=False)
 
         profile.set_preferences(preferences)
 
         # installing addons
         LOG.info("Installing Add-ons:")
         LOG.info(extensions)
-        profile.addon_manager.install_addons(extensions)
+        profile.addons.install(extensions)
 
         # installing webextensions
         webextensions = self.test_config.get('webextensions', None)
         if isinstance(webextensions, basestring):
             webextensions = [webextensions]
 
         if webextensions is not None:
             LOG.info("Installing Webextensions:")
@@ -138,17 +138,17 @@ class FFSetup(object):
                 filename = utils.interpolate(webext)
                 if mozinfo.os == 'win':
                     filename = filename.replace('/', '\\')
                 if not filename.endswith('.xpi'):
                     continue
                 if not os.path.exists(filename):
                     continue
                 LOG.info(filename)
-                profile.addon_manager.install_from_path(filename)
+                profile.addons.install(filename)
 
     def _run_profile(self):
         runner_cls = mozrunner.runners.get(
             mozinfo.info.get(
                 'appname',
                 'firefox'),
             mozrunner.Runner)
         args = [self.browser_config["extra_args"], self.browser_config["init_url"]]