From 76a7aa9a245135d04e5d8e2dbf27b64623954b6c Mon Sep 17 00:00:00 2001 From: Duncan Sommerville Date: Thu, 18 Jun 2015 18:18:48 -0400 Subject: [PATCH] Fix SoundCloud and TaskManager bugs, switch tasks to use acks_late, and provide feedback from SoundCloud context menu items --- .../application/common/TaskManager.php | 7 +++++- .../controllers/LibraryController.php | 12 ++++++++-- .../application/models/airtime/CcFiles.php | 4 +++- .../services/ThirdPartyService.php | 22 +++++++++++++++---- .../public/js/airtime/library/library.js | 11 ++++++++++ .../airtime-celery/airtime-celery/tasks.py | 4 ++-- 6 files changed, 50 insertions(+), 10 deletions(-) diff --git a/airtime_mvc/application/common/TaskManager.php b/airtime_mvc/application/common/TaskManager.php index cec3a2f56..415afa55f 100644 --- a/airtime_mvc/application/common/TaskManager.php +++ b/airtime_mvc/application/common/TaskManager.php @@ -22,7 +22,7 @@ final class TaskManager { * @var int TASK_INTERVAL_SECONDS how often, in seconds, to run the TaskManager tasks, * if they need to be run */ - const TASK_INTERVAL_SECONDS = 60; + const TASK_INTERVAL_SECONDS = 30; /** * @var $con PDO Propel connection object @@ -67,6 +67,11 @@ final class TaskManager { try { $lock = $this->_getLock(); if ($lock && microtime(true) < $lock['valstr'] + self::TASK_INTERVAL_SECONDS) { + // Fun fact: Propel caches the database connection and uses it persistently + // (thus why calling Propel::getConnection explicitly and passing a connection + // parameter is often not necessary when making Propel queries). Long story short, + // if we don't use commit() here, we end up blocking other queries made within this request + $this->_con->commit(); return; } $this->_updateLock($lock); diff --git a/airtime_mvc/application/controllers/LibraryController.php b/airtime_mvc/application/controllers/LibraryController.php index 6b691f760..e2de84adf 100644 --- a/airtime_mvc/application/controllers/LibraryController.php +++ b/airtime_mvc/application/controllers/LibraryController.php @@ -278,9 +278,17 @@ class LibraryController extends Zend_Controller_Action $serviceId = $soundcloudService->getServiceId($id); if (!is_null($file) && $serviceId != 0) { $menu["soundcloud"]["items"]["view"] = array("name" => _("View track"), "icon" => "soundcloud", "url" => $baseUrl."soundcloud/view-on-sound-cloud/id/{$id}"); - $menu["soundcloud"]["items"]["upload"] = array("name" => _("Remove track"), "icon" => "soundcloud", "url" => $baseUrl."soundcloud/delete/id/{$id}"); + $menu["soundcloud"]["items"]["remove"] = array("name" => _("Remove track"), "icon" => "soundcloud", "url" => $baseUrl."soundcloud/delete/id/{$id}"); } else { - $menu["soundcloud"]["items"]["upload"] = array("name" => _("Upload track"), "icon" => "soundcloud", "url" => $baseUrl."soundcloud/upload/id/{$id}"); + // If a reference exists for this file ID, that means the user has uploaded the track + // but we haven't yet gotten a response from Celery, so disable the menu item + if ($soundcloudService->referenceExists($id)) { + $menu["soundcloud"]["items"]["upload"] = array("name" => _("Upload track"), "icon" => "soundcloud", + "url" => $baseUrl."soundcloud/upload/id/{$id}", "disabled" => true); + } else { + $menu["soundcloud"]["items"]["upload"] = array("name" => _("Upload track"), "icon" => "soundcloud", + "url" => $baseUrl."soundcloud/upload/id/{$id}"); + } } } diff --git a/airtime_mvc/application/models/airtime/CcFiles.php b/airtime_mvc/application/models/airtime/CcFiles.php index 4140485ea..16fb99c67 100644 --- a/airtime_mvc/application/models/airtime/CcFiles.php +++ b/airtime_mvc/application/models/airtime/CcFiles.php @@ -74,6 +74,8 @@ class CcFiles extends BaseCcFiles { /** Used to create a CcFiles object from an array containing metadata and a file uploaded by POST. * This is used by our Media REST API! * @param $fileArray An array containing metadata for a CcFiles object. + * + * @return object the sanitized response * @throws Exception */ public static function createFromUpload($fileArray) @@ -94,7 +96,7 @@ class CcFiles extends BaseCcFiles { $tempFilePath = $_FILES['file']['tmp_name']; try { - self::createAndImport($fileArray, $tempFilePath, $originalFilename); + return self::createAndImport($fileArray, $tempFilePath, $originalFilename); } catch (Exception $e) { if (file_exists($tempFilePath)) { unlink($tempFilePath); diff --git a/airtime_mvc/application/services/ThirdPartyService.php b/airtime_mvc/application/services/ThirdPartyService.php index 365002ee7..988b24588 100644 --- a/airtime_mvc/application/services/ThirdPartyService.php +++ b/airtime_mvc/application/services/ThirdPartyService.php @@ -74,7 +74,7 @@ abstract class ThirdPartyService { * Given a CcFiles identifier for a file that's been uploaded to a third-party service, * return the third-party identifier for the remote file * - * @param int $fileId the local CcFiles identifier + * @param int $fileId the cc_files identifier * * @return string the service foreign identifier */ @@ -85,11 +85,25 @@ abstract class ThirdPartyService { return empty($ref) ? '' : $ref->getDbForeignId(); } + /** + * Check if a reference exists for a given CcFiles identifier + * + * @param int $fileId the cc_files identifier + * + * @return string the service foreign identifier + */ + public function referenceExists($fileId) { + $ref = ThirdPartyTrackReferencesQuery::create() + ->filterByDbService(static::$_SERVICE_NAME) + ->findOneByDbFileId($fileId); // There shouldn't be duplicates! + return !empty($ref); + } + /** * Given a CcFiles identifier for a file that's been uploaded to a third-party service, * return a link to the remote file * - * @param int $fileId CcFiles identifier + * @param int $fileId the cc_files identifier * * @return string the link to the remote file */ @@ -101,14 +115,14 @@ abstract class ThirdPartyService { /** * Upload the file with the given identifier to a third-party service * - * @param int $fileId CcFiles identifier + * @param int $fileId the cc_files identifier */ abstract function upload($fileId); /** * Delete the file with the given identifier from a third-party service * - * @param int $fileId the local CcFiles identifier + * @param int $fileId the cc_files identifier * * @throws ServiceNotFoundException when a $fileId with no corresponding * service identifier is given diff --git a/airtime_mvc/public/js/airtime/library/library.js b/airtime_mvc/public/js/airtime/library/library.js index 445e36b5a..2f3bf87d0 100644 --- a/airtime_mvc/public/js/airtime/library/library.js +++ b/airtime_mvc/public/js/airtime/library/library.js @@ -1016,11 +1016,22 @@ var AIRTIME = (function(AIRTIME) { if (soundcloud.upload !== undefined) { callback = function() { + alert($.i18n._("Your track is being uploaded to SoundCloud")); $.post(soundcloud.upload.url, function(){}); }; soundcloud.upload.callback = callback; } + // define an upload to soundcloud callback. + if (soundcloud.remove !== undefined) { + + callback = function() { + alert($.i18n._("Your track is being deleted from SoundCloud")); + $.post(soundcloud.remove.url, function(){}); + }; + soundcloud.remove.callback = callback; + } + // define a view on soundcloud callback if (soundcloud.view !== undefined) { diff --git a/python_apps/airtime-celery/airtime-celery/tasks.py b/python_apps/airtime-celery/airtime-celery/tasks.py index bfe873609..27554241a 100644 --- a/python_apps/airtime-celery/airtime-celery/tasks.py +++ b/python_apps/airtime-celery/airtime-celery/tasks.py @@ -9,7 +9,7 @@ celery = Celery() logger = get_task_logger(__name__) -@celery.task(name='soundcloud-upload') +@celery.task(name='soundcloud-upload', acks_late=True) def soundcloud_upload(data, token, file_path): """ Upload a file to SoundCloud @@ -33,7 +33,7 @@ def soundcloud_upload(data, token, file_path): data['asset_data'].close() return json.dumps(track.fields()) -@celery.task(name='soundcloud-delete') +@celery.task(name='soundcloud-delete', acks_late=True) def soundcloud_delete(token, track_id): """ Delete a file from SoundCloud