On Wed, Aug 27, 2014 at 4:51 PM, Raphael Hertzog <hert...@debian.org> wrote: > PackageExtractedInfo.objects.filter(key='screenshots').delete()
Oh! For some reason I had miss-conceptualized PackageExtractedInfo objects. I thought that the object contained multiple key/values, and that that would select and delete the entire object including the 'general' key. Update patch attached, which is now much quicker! Thanks, -- Andrew Starr-Bochicchio Ubuntu Developer <https://launchpad.net/~andrewsomething> Debian Developer <http://qa.debian.org/developer.php?login=asb> PGP/GPG Key ID: D53FDCB1
From c55d99f72a61bb175613fbf544b9db3fb8edb53e Mon Sep 17 00:00:00 2001 From: Andrew Starr-Bochicchio <a.star...@gmail.com> Date: Mon, 25 Aug 2014 11:18:15 -0700 Subject: [PATCH] Add link to screenshots if they exist on screenshots.debian.net MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated based on Raphaƫl's suggestions. --- distro_tracker/vendor/debian/tests.py | 172 +++++++++++++++++++++++++ distro_tracker/vendor/debian/tracker_panels.py | 25 ++++ distro_tracker/vendor/debian/tracker_tasks.py | 57 ++++++++ 3 files changed, 254 insertions(+) diff --git a/distro_tracker/vendor/debian/tests.py b/distro_tracker/vendor/debian/tests.py index c98fe7f..6c3764d 100644 --- a/distro_tracker/vendor/debian/tests.py +++ b/distro_tracker/vendor/debian/tests.py @@ -63,6 +63,7 @@ from distro_tracker.vendor.debian.tracker_tasks import UpdateExcusesTask from distro_tracker.vendor.debian.tracker_tasks import UpdateDebciStatusTask from distro_tracker.vendor.debian.tracker_tasks import UpdateDebianDuckTask from distro_tracker.vendor.debian.tracker_tasks import UpdateAutoRemovalsStatsTask +from distro_tracker.vendor.debian.tracker_tasks import UpdatePackageScreenshotsTask from distro_tracker.vendor.debian.models import DebianContributor from distro_tracker.vendor.debian.models import UbuntuPackage from distro_tracker.vendor.debian.tracker_tasks import UpdateLintianStatsTask @@ -2579,6 +2580,52 @@ class DebtagsLinkTest(TestCase): self.assertNotIn('edit tags', response_content) +class ScreenshotsLinkTest(TestCase): + """ + Tests that the screenshots link is added to source package pages. + """ + def setUp(self): + self.package_name = SourcePackageName.objects.create(name='dummy') + self.package = SourcePackage.objects.create( + source_package_name=self.package_name, + version='1.0.0') + PackageExtractedInfo.objects.create( + package=self.package.source_package_name, + key='screenshots', + value={'screenshots': 'true'} + ) + + def get_package_page_response(self, package_name): + package_page_url = reverse('dtracker-package-page', kwargs={ + 'package_name': package_name, + }) + return self.client.get(package_page_url) + + def test_source_package_with_screenshot(self): + response = self.get_package_page_response(self.package.name) + + response_content = response.content.decode('utf8') + self.assertIn('screenshots', response_content) + + def test_source_package_without_screenshot(self): + package_name = SourcePackageName.objects.create(name='other') + package = SourcePackage.objects.create( + source_package_name=package_name, + version='1.0.0') + response = self.get_package_page_response(package.name) + + response_content = response.content.decode('utf8') + self.assertNotIn('screenshots', response_content) + + def test_pseudo_package(self): + package = PseudoPackageName.objects.create(name='somepackage') + + response = self.get_package_page_response(package.name) + + response_content = response.content.decode('utf-8') + self.assertNotIn('screenshots', response_content) + + class UpdatePiupartsTaskTests(TestCase): suites = [] @@ -4781,3 +4828,128 @@ class UpdateAutoRemovalsStatsTaskTest(TestCase): self.run_task() self.assertEqual(0, self.dummy_package.action_items.count()) + + +@mock.patch('distro_tracker.core.utils.http.requests') +class UpdatePackageScreenshotsTaskTest(TestCase): + """ + Tests for the:class:`distro_tracker.vendor.debian.tracker_tasks. + UpdatePackageScreenshotsTask` task. + """ + def setUp(self): + self.dummy_package = SourcePackageName.objects.create(name='dummy') + self.json_data = """{ + "packages": [{ + "maintainer": "Jane Doe", + "name": "dummy", + "url": "http://screenshots.debian.net/package/dummy", + "section": "universe/games", + "maintainer_email": "j...@example.com", + "homepage": "http://example.com/packages/dummy", + "description": "a game that you can play" + }]} + """ + PackageExtractedInfo.objects.create( + package=self.dummy_package, + key='general', + value={ + 'name': 'dummy', + 'maintainer': { + 'email': 'j...@example.com', + } + } + ) + self.other_json_data = """{ + "packages": [{ + "maintainer": "John Doe", + "name": "other", + "url": "http://screenshots.debian.net/package/other", + "section": "universe/games", + "maintainer_email": "j...@example.com", + "homepage": "http://example.com/packages/other", + "description": "yet another game that you can play" + }]} + """ + + def run_task(self): + """ + Runs the debci status update task. + """ + task = UpdatePackageScreenshotsTask() + task.execute() + + def test_extractedinfo_item_for_without_screenshot(self, mock_requests): + """ + Tests that packages without screenshots don't claim to have them. + """ + set_mock_response(mock_requests, text=self.json_data) + other_package = SourcePackageName.objects.create(name='other-package') + + self.run_task() + + with self.assertRaises(PackageExtractedInfo.DoesNotExist): + info = other_package.packageextractedinfo_set.get(key='screenshots') + + def test_no_extractedinfo_for_unknown_package(self, mock_requests): + """ + Tests that UpdatePackageScreenshotsTask doesn't fail with an unknown package. + """ + data = """{ + "packages": [{ + "maintainer": "John Doe", + "name": "other", + "url": "http://screenshots.debian.net/package/other", + "section": "universe/games", + "maintainer_email": "j...@example.com", + "homepage": "http://example.com/packages/other", + "description": "yet another game that you can play" + }]} + """ + set_mock_response(mock_requests, text=data) + + self.run_task() + + self.assertEqual(0, + PackageExtractedInfo.objects.filter(key='screenshots').count()) + + def test_extractedinfo_for_package_with_screenshots(self, mock_requests): + """ + Tests that PackageExtractedInfo for a package with a screenshot is + correct. + """ + set_mock_response(mock_requests, text=self.json_data) + + self.run_task() + + info = self.dummy_package.packageextractedinfo_set.get(key='screenshots') + + self.assertEqual(info.value['screenshots'], 'true') + + def test_extractedinfo_is_dropped_when_no_more_screenshot(self, mock_requests): + """ + Tests that PackageExtractedInfo is dropped if screenshot goes away. + """ + set_mock_response(mock_requests, text=self.json_data) + self.run_task() + + set_mock_response(mock_requests, text=self.other_json_data) + self.run_task() + + with self.assertRaises(PackageExtractedInfo.DoesNotExist): + info = self.dummy_package.packageextractedinfo_set.get( + key='screenshots') + + def test_other_extractedinfo_keys_not_dropped(self, mock_requests): + """ + Ensure that other PackageExtractedInfo keys are not dropped when + deleting the screenshot key. + """ + set_mock_response(mock_requests, text=self.json_data) + self.run_task() + + set_mock_response(mock_requests, text=self.other_json_data) + self.run_task() + + info = self.dummy_package.packageextractedinfo_set.get(key='general') + + self.assertEqual(info.value['name'], 'dummy') diff --git a/distro_tracker/vendor/debian/tracker_panels.py b/distro_tracker/vendor/debian/tracker_panels.py index d995f16..7080116 100644 --- a/distro_tracker/vendor/debian/tracker_panels.py +++ b/distro_tracker/vendor/debian/tracker_panels.py @@ -170,6 +170,31 @@ class DebtagsLink(LinksPanel.ItemProvider): ] +class ScreenshotsLink(LinksPanel.ItemProvider): + """ + Add a link to screenshots.debian.net + """ + SOURCES_URL_TEMPLATE = \ + 'http://screenshots.debian.net/package/{package}' + + def get_panel_items(self): + if not isinstance(self.package, SourcePackageName): + return + try: + infos = self.package.packageextractedinfo_set.get(key='screenshots') + except PackageExtractedInfo.DoesNotExist: + return + if infos.value['screenshots'] == 'true': + return [ + LinksPanel.SimpleLinkItem( + 'screenshots', + self.SOURCES_URL_TEMPLATE.format(package=self.package.name) + ) + ] + else: + return + + class TransitionsPanel(BasePanel): template_name = 'debian/transitions-panel.html' panel_importance = 2 diff --git a/distro_tracker/vendor/debian/tracker_tasks.py b/distro_tracker/vendor/debian/tracker_tasks.py index 7376cb3..a423ba8 100644 --- a/distro_tracker/vendor/debian/tracker_tasks.py +++ b/distro_tracker/vendor/debian/tracker_tasks.py @@ -2206,3 +2206,60 @@ class UpdateAutoRemovalsStatsTask(BaseTask): for package in packages: self.update_action_item(package, autoremovals_stats[package.name]) + + +class UpdatePackageScreenshotsTask(BaseTask): + """ + Check if a screenshot exists on screenshots.debian.net, and add a + key to PackageExtractedInfo if it does. + """ + EXTRACTED_INFO_KEY = 'screenshots' + + def __init__(self, force_update=False, *args, **kwargs): + super(UpdatePackageScreenshotsTask, self).__init__(*args, **kwargs) + self.force_update = force_update + + def set_parameters(self, parameters): + if 'force_update' in parameters: + self.force_update = parameters['force_update'] + + def _get_screenshots(self): + url = 'https://screenshots.debian.net/json/packages' + cache = HttpCache(settings.DISTRO_TRACKER_CACHE_DIRECTORY) + response, updated = cache.update(url, force=self.force_update) + response.raise_for_status() + if not updated: + return + + data = json.loads(response.text) + return data + + def execute(self): + content = self._get_screenshots() + + packages_with_screenshots = [] + for item in content['packages']: + try: + package = SourcePackageName.objects.get(name=item['name']) + packages_with_screenshots.append(package) + except SourcePackageName.DoesNotExist: + pass + + with transaction.atomic(): + PackageExtractedInfo.objects.filter(key='screenshots').delete() + + extracted_info = [] + for package in packages_with_screenshots: + try: + screenshot_info = package.packageextractedinfo_set.get( + key=self.EXTRACTED_INFO_KEY) + screenshot_info.value['screenshots'] = 'true' + except PackageExtractedInfo.DoesNotExist: + screenshot_info = PackageExtractedInfo( + key=self.EXTRACTED_INFO_KEY, + package=package, + value={'screenshots': 'true'}) + + extracted_info.append(screenshot_info) + + PackageExtractedInfo.objects.bulk_create(extracted_info) -- 1.9.1