Bug 1451733 - [mozprofile] Remove ability to download addons from AddonManager r=jmaher

This is another seemingly unused feature in mozilla-central.

Being able to download addons in AddonManager is a violation of the single
responsibility principle. If consumers *really* need to download addons, they
can easily do so with requests and then pass the file path in to AddonManager
like normal. There's no need to have this baked into AddonManager itself.

MozReview-Commit-ID: IorG0foiHfT

--HG--
extra : rebase_source : 27b4162d9adfb986c2b9822c12b6abc5a2561a9a
This commit is contained in:
Andrew Halberstadt 2018-04-05 10:39:31 -04:00
parent 7368b59d75
commit caac402c4b
2 changed files with 22 additions and 149 deletions

View File

@ -14,7 +14,6 @@ import hashlib
from xml.dom import minidom
from six import reraise, string_types
from six.moves.urllib import request
import mozfile
from mozlog.unstructured import getLogger
@ -56,9 +55,6 @@ class AddonManager(object):
# Backup folder for already existing addons
self.backup_dir = None
# Add-ons downloaded and which have to be removed from the file system
self.downloaded_addons = []
# Information needed for profile reset (see http://bit.ly/17JesUf)
self.installed_addons = []
@ -81,10 +77,6 @@ class AddonManager(object):
except IOError:
pass
# Remove all downloaded add-ons
for addon in self.downloaded_addons:
mozfile.remove(addon)
# restore backups
if self.backup_dir and os.path.isdir(self.backup_dir):
extensions_path = os.path.join(self.profile, 'extensions')
@ -99,36 +91,6 @@ class AddonManager(object):
# reset instance variables to defaults
self._internal_init()
@classmethod
def download(self, url, target_folder=None):
"""
Downloads an add-on from the specified URL to the target folder
:param url: URL of the add-on (XPI file)
:param target_folder: Folder to store the XPI file in
"""
response = request.urlopen(url)
fd, path = tempfile.mkstemp(suffix='.xpi')
os.write(fd, response.read())
os.close(fd)
if not self.is_addon(path):
mozfile.remove(path)
raise AddonFormatError('Not a valid add-on: %s' % url)
# Give the downloaded file a better name by using the add-on id
details = self.addon_details(path)
new_path = path.replace('.xpi', '_%s.xpi' % details.get('id'))
# Move the add-on to the target folder if requested
if target_folder:
new_path = os.path.join(target_folder, os.path.basename(new_path))
os.rename(path, new_path)
return new_path
def get_addon_path(self, addon_id):
"""Returns the path to the installed add-on
@ -296,17 +258,11 @@ class AddonManager(object):
def install_from_path(self, path, unpack=False):
"""
Installs addon from a filepath, url or directory of addons in the profile.
Installs addon from a filepath or directory of addons in the profile.
:param path: url, path to .xpi, or directory of addons
:param path: path to .xpi or directory of addons
:param unpack: whether to unpack unless specified otherwise in the install.rdf
"""
# if the addon is a URL, download it
# note that this won't work with protocols urllib2 doesn't support
if mozfile.is_url(path):
path = self.download(path)
self.downloaded_addons.append(path)
addons = [path]
# if path is not an add-on, try to install all contained add-ons

View File

@ -4,7 +4,7 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.
from __future__ import absolute_import
from __future__ import absolute_import, unicode_literals
import os
import tempfile
@ -14,13 +14,10 @@ import zipfile
import mozunit
import mozfile
import mozhttpd
import mozlog.unstructured as mozlog
import mozprofile
from addon_stubs import generate_addon
from six.moves.urllib import error
here = os.path.dirname(os.path.abspath(__file__))
@ -65,55 +62,6 @@ class TestAddonsManager(unittest.TestCase):
self.assertIn(addon_xpi, self.am.installed_addons)
self.am.clean()
def test_download(self):
server = mozhttpd.MozHttpd(docroot=os.path.join(here, 'addons'))
server.start()
# Download a valid add-on without a class instance to the general
# tmp folder and clean-up
try:
addon = server.get_url() + 'empty.xpi'
xpi_file = mozprofile.addons.AddonManager.download(addon)
self.assertTrue(os.path.isfile(xpi_file))
self.assertIn('test-empty@quality.mozilla.org.xpi',
os.path.basename(xpi_file))
self.assertNotIn(self.tmpdir, os.path.dirname(xpi_file))
finally:
# Given that the file is stored outside of the created tmp dir
# we have to ensure to explicitely remove it
if os.path.isfile(xpi_file):
os.remove(xpi_file)
# Download an valid add-on to a special folder
addon = server.get_url() + 'empty.xpi'
xpi_file = self.am.download(addon, self.tmpdir)
self.assertTrue(os.path.isfile(xpi_file))
self.assertIn('test-empty@quality.mozilla.org.xpi',
os.path.basename(xpi_file))
self.assertIn(self.tmpdir, os.path.dirname(xpi_file))
self.assertEqual(self.am.downloaded_addons, [])
os.remove(xpi_file)
# Download an invalid add-on to a special folder
addon = server.get_url() + 'invalid.xpi'
self.assertRaises(mozprofile.addons.AddonFormatError,
self.am.download, addon, self.tmpdir)
self.assertEqual(os.listdir(self.tmpdir), [])
# Download from an invalid URL
addon = server.get_url() + 'not_existent.xpi'
self.assertRaises(error.HTTPError,
self.am.download, addon, self.tmpdir)
self.assertEqual(os.listdir(self.tmpdir), [])
# Download from an invalid URL
addon = 'not_existent.xpi'
self.assertRaises(ValueError,
self.am.download, addon, self.tmpdir)
self.assertEqual(os.listdir(self.tmpdir), [])
server.stop()
def test_install_webextension_from_dir(self):
addon = os.path.join(here, 'addons', 'apply-css.xpi')
zipped = zipfile.ZipFile(addon)
@ -126,32 +74,25 @@ class TestAddonsManager(unittest.TestCase):
self.assertTrue(os.path.isdir(self.am.installed_addons[0]))
def test_install_webextension(self):
server = mozhttpd.MozHttpd(docroot=os.path.join(here, 'addons'))
server.start()
try:
addon = server.get_url() + 'apply-css.xpi'
self.am.install_from_path(addon)
finally:
server.stop()
addon = os.path.join(here, 'addons', 'apply-css.xpi')
self.am.install_from_path(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]))
self.assertEqual(len(self.am.downloaded_addons), 1)
self.assertTrue(os.path.isfile(self.am.downloaded_addons[0]))
self.assertIn('test-webext@quality.mozilla.org.xpi',
os.path.basename(self.am.downloaded_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):
server = mozhttpd.MozHttpd(docroot=os.path.join(here, 'addons'))
server.start()
try:
addon = server.get_url() + 'apply-css-sans-id.xpi'
self.am.install_from_path(addon)
finally:
server.stop()
addon = os.path.join(here, 'addons', 'apply-css-sans-id.xpi')
self.am.install_from_path(addon)
self.assertEqual(len(self.am.downloaded_addons), 1)
self.assertTrue(os.path.isfile(self.am.downloaded_addons[0]))
self.assertIn('temporary-addon.xpi',
os.path.basename(self.am.downloaded_addons[0]))
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):
addons_to_install = []
@ -214,20 +155,6 @@ class TestAddonsManager(unittest.TestCase):
self.assertEqual(self.am.installed_addons, [addon_no_unpack])
self.am.clean()
def test_install_from_path_url(self):
server = mozhttpd.MozHttpd(docroot=os.path.join(here, 'addons'))
server.start()
addon = server.get_url() + 'empty.xpi'
self.am.install_from_path(addon)
server.stop()
self.assertEqual(len(self.am.downloaded_addons), 1)
self.assertTrue(os.path.isfile(self.am.downloaded_addons[0]))
self.assertIn('test-empty@quality.mozilla.org.xpi',
os.path.basename(self.am.downloaded_addons[0]))
def test_install_from_path_after_reset(self):
# Installing the same add-on after a reset should not cause a failure
addon = generate_addon('test-addon-1@mozilla.org',
@ -367,10 +294,6 @@ class TestAddonsManager(unittest.TestCase):
def test_noclean(self):
"""test `restore=True/False` functionality"""
server = mozhttpd.MozHttpd(docroot=os.path.join(here, 'addons'))
server.start()
profile = tempfile.mkdtemp()
tmpdir = tempfile.mkdtemp()
@ -379,10 +302,10 @@ class TestAddonsManager(unittest.TestCase):
self.assertFalse(bool(os.listdir(profile)))
# make an addon
addons = []
addons.append(generate_addon('test-addon-1@mozilla.org',
path=tmpdir))
addons.append(server.get_url() + 'empty.xpi')
addons = [
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)
@ -396,17 +319,11 @@ class TestAddonsManager(unittest.TestCase):
self.assertTrue(os.path.exists(staging_folder))
self.assertEqual(len(os.listdir(staging_folder)), 2)
# del addons; now its gone though the directory tree exists
downloaded_addons = am.downloaded_addons
del am
self.assertEqual(os.listdir(profile), ['extensions'])
self.assertTrue(os.path.exists(staging_folder))
self.assertEqual(os.listdir(staging_folder), [])
for addon in downloaded_addons:
self.assertFalse(os.path.isfile(addon))
finally:
mozfile.rmtree(tmpdir)
mozfile.rmtree(profile)