From f80b80540f7039cfd2d0af069113f61e646cadb8 Mon Sep 17 00:00:00 2001 From: Andy Heathershaw Date: Wed, 22 Apr 2020 06:56:15 +0100 Subject: [PATCH] Files are now removed from Dropbox when a photo/album is deleted. Added handling for Dropbox's 429 (retry) error. Added a new admin permission for restricing access to the new services area. Corrected a logic issue with failing images during the analysis process. #106 --- app/AlbumSources/DropboxSource.php | 22 +++- app/Exceptions/DropboxRetryException.php | 23 ++++ .../Controllers/Admin/StorageController.php | 13 +- app/Services/DropboxService.php | 113 +++++++++++++----- config/services.php | 1 + database/seeds/PermissionsSeeder.php | 8 ++ resources/js/albums.js | 6 + resources/lang/en/permissions.php | 1 + .../themes/base/admin/analyse_album.blade.php | 2 +- .../themes/base/admin/edit_group.blade.php | 4 + .../themes/base/partials/navbar.blade.php | 10 +- 11 files changed, 151 insertions(+), 52 deletions(-) create mode 100644 app/Exceptions/DropboxRetryException.php diff --git a/app/AlbumSources/DropboxSource.php b/app/AlbumSources/DropboxSource.php index 6098826..62aca35 100644 --- a/app/AlbumSources/DropboxSource.php +++ b/app/AlbumSources/DropboxSource.php @@ -20,7 +20,16 @@ class DropboxSource extends AlbumSourceBase implements IAlbumSource */ public function deleteAlbumContents() { - // TODO: Implement deleteAlbumContents() method. + try + { + $albumPathOnStorage = sprintf('/%s', $this->album->url_alias); + + $this->getClient()->deleteFile($albumPathOnStorage); + } + catch (\Exception $ex) + { + // Don't worry too much if the delete fails - the file may have been removed on Dropbox itself + } } /** @@ -31,7 +40,16 @@ class DropboxSource extends AlbumSourceBase implements IAlbumSource */ public function deleteThumbnail(Photo $photo, $thumbnail = null) { - // TODO: Implement deleteThumbnail() method. + try + { + $pathOnStorage = $this->getPathToPhoto($photo, $thumbnail); + + $this->getClient()->deleteFile($pathOnStorage); + } + catch (\Exception $ex) + { + // Don't worry too much if the delete fails - the file may have been removed on Dropbox itself + } } /** diff --git a/app/Exceptions/DropboxRetryException.php b/app/Exceptions/DropboxRetryException.php new file mode 100644 index 0000000..238e9fb --- /dev/null +++ b/app/Exceptions/DropboxRetryException.php @@ -0,0 +1,23 @@ +innerException; + } + + public function __construct($httpCode, \Exception $innerException) + { + parent::__construct('Dropbox requested to retry the request'); + + $this->innerException = $innerException; + } +} \ No newline at end of file diff --git a/app/Http/Controllers/Admin/StorageController.php b/app/Http/Controllers/Admin/StorageController.php index 9c41c28..afa518a 100644 --- a/app/Http/Controllers/Admin/StorageController.php +++ b/app/Http/Controllers/Admin/StorageController.php @@ -35,7 +35,7 @@ class StorageController extends Controller $storage = Storage::where('id', intval($id))->first(); if (is_null($storage)) { - App::abort(404); + return redirect(route('storages.index')); } $externalServiceType = $this->getExternalServiceType($storage); @@ -47,26 +47,17 @@ class StorageController extends Controller } $serviceTypeName = trans(sprintf('services.%s', $externalServiceType)); - $viewData = [ - 'service' => $storage->externalService, - 'serviceName' => $serviceTypeName, - 'storage' => $storage - ]; switch ($externalServiceType) { case ExternalService::DROPBOX: $dropbox = new DropboxService(); - $viewData['authoriseUrl'] = $dropbox->authoriseUrl($storage); - $viewData['callbackUrl'] = $dropbox->callbackUrl(); - break; + return redirect($dropbox->authoriseUrl($storage)); default: $request->session()->flash('error', trans('admin.storage_external_service_no_authorisation', ['service_name' => $serviceTypeName])); return redirect(route('storages.index')); } - - return Theme::render('admin.authorise_external_service', $viewData); } /** diff --git a/app/Services/DropboxService.php b/app/Services/DropboxService.php index 8b22db3..1ecce92 100644 --- a/app/Services/DropboxService.php +++ b/app/Services/DropboxService.php @@ -2,6 +2,7 @@ namespace App\Services; +use App\Exceptions\DropboxRetryException; use App\Storage; use Illuminate\Http\Request; use Illuminate\Support\Facades\Log; @@ -43,6 +44,25 @@ class DropboxService return route('services.authoriseDropbox'); } + public function deleteFile($pathOnStorage) + { + $dropboxData = ['path' => $pathOnStorage]; + + $deleteResult = $this->sendRequest( + $this->config['delete_url'], + 'POST', + $dropboxData, + [ + 'http_headers' => [ + 'Content-Type: application/json' + ], + 'post_body_is_json' => true + ] + ); + + Log::debug('DropboxService - response to deleteFile.', ['response' => $deleteResult, 'path' => $pathOnStorage]); + } + public function downloadFile($pathOnStorage) { $dropboxArgs = ['path' => $pathOnStorage]; @@ -80,26 +100,47 @@ class DropboxService $this->accessToken = $accessToken; } - public function uploadFile($pathToFileToUpload, $pathToStorage) + public function uploadFile($pathToFileToUpload, $pathOnStorage) { $dropboxArgs = [ - 'path' => $pathToStorage, - 'mode' => 'add', + 'path' => $pathOnStorage, + 'mode' => 'overwrite', 'mute' => true ]; - $uploadResult = $this->sendRequest( - $this->config['upload_url'], - 'POST', - file_get_contents($pathToFileToUpload), - [ - 'http_headers' => [ - sprintf('Dropbox-API-Arg: %s', json_encode($dropboxArgs)), - 'Content-Type: application/octet-stream' - ], - 'post_body_is_json' => false - ] - ); + $shouldRetry = true; + while ($shouldRetry) + { + try + { + $uploadResult = $this->sendRequest( + $this->config['upload_url'], + 'POST', + file_get_contents($pathToFileToUpload), + [ + 'http_headers' => [ + sprintf('Dropbox-API-Arg: %s', json_encode($dropboxArgs)), + 'Content-Type: application/octet-stream' + ], + 'post_body_is_json' => false + ] + ); + + $shouldRetry = false; + Log::debug('DropboxService - response to uploadFile.', ['response' => $uploadResult, 'path' => $pathOnStorage]); + } + catch (DropboxRetryException $dre) + { + // Retry - leave shouldRetry as true + Log::debug('DropboxService - Dropbox reported a lock/rate limit and requested to retry'); + sleep(2); + } + catch (\Exception $ex) + { + $shouldRetry = false; + Log::debug('DropboxService - exception in uploadFile.', ['exception' => $ex->getMessage()]); + } + } } private function convertAuthorisationCodeToToken($authorisationCode, Storage $storage) @@ -178,29 +219,25 @@ class DropboxService $ch = $this->getBasicHttpClient($url, $method, $httpHeaders); + Log::info(sprintf('DropboxService - %s: %s', strtoupper($method), $url)); + Log::debug('DropboxService - HTTP headers:', $httpHeaders); + if (!is_null($postData)) { if ($postOptions['post_body_is_json']) { + // Only log a post body if we have one and it's in JSON format (i.e. not a file upload) + Log::debug('DropboxService - Body: ', $postData); $postData = json_encode($postData); } curl_setopt($ch, CURLOPT_POSTFIELDS, $postData); } - Log::info(sprintf('%s: %s', strtoupper($method), $url)); - Log::debug('HTTP headers:', $httpHeaders); - - // Only log a post body if we have one and it's in JSON format (i.e. not a file upload) - if (!is_null($postData) && $postOptions['post_body_is_json']) - { - Log::debug($postData); - } - $result = curl_exec($ch); $httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE); - Log::info(sprintf('Received HTTP code %d', $httpCode)); + Log::info(sprintf('DropboxService - Received HTTP code %d', $httpCode)); // Only log a result if we have one and it's in JSON format (i.e. not a file download) if (!is_null($result) && $result !== false && $postOptions['response_body_is_json']) @@ -208,15 +245,25 @@ class DropboxService Log::debug($result); } - if ($httpCode != 200 && $httpCode != 304) + try { - throw new \Exception(sprintf('Exception from Dropbox: %s', $result)); + if ($httpCode != 200 && $httpCode != 304) + { + if ($httpCode == 429) + { + throw new DropboxRetryException($httpCode, new \Exception(sprintf('Exception from Dropbox: %s', $result))); + } + + throw new \Exception(sprintf('Exception from Dropbox: %s', $result)); + } + + return $postOptions['response_body_is_json'] + ? json_decode($result) + : $result; + } + finally + { + curl_close($ch); } - - curl_close($ch); - - return $postOptions['response_body_is_json'] - ? json_decode($result) - : $result; } } \ No newline at end of file diff --git a/config/services.php b/config/services.php index 10e07af..7edaecd 100644 --- a/config/services.php +++ b/config/services.php @@ -21,6 +21,7 @@ return [ 'dropbox' => [ 'authorise_url' => 'https://www.dropbox.com/oauth2/authorize', + 'delete_url' => 'https://api.dropboxapi.com/2/files/delete_v2', 'download_url' => 'https://content.dropboxapi.com/2/files/download', 'token_url' => 'https://api.dropbox.com/oauth2/token', 'upload_url' => 'https://content.dropboxapi.com/2/files/upload' diff --git a/database/seeds/PermissionsSeeder.php b/database/seeds/PermissionsSeeder.php index e5e5494..a6c0cf5 100644 --- a/database/seeds/PermissionsSeeder.php +++ b/database/seeds/PermissionsSeeder.php @@ -80,6 +80,14 @@ class PermissionsSeeder extends Seeder 'is_default' => false, 'sort_order' => 0 ]); + + // admin:manage-services = controls if external servies can be managed + DatabaseSeeder::createOrUpdate('permissions', [ + 'section' => 'admin', + 'description' => 'manage-services', + 'is_default' => false, + 'sort_order' => 0 + ]); } private function seedAlbumPermissions() diff --git a/resources/js/albums.js b/resources/js/albums.js index c41b22a..6ff2160 100644 --- a/resources/js/albums.js +++ b/resources/js/albums.js @@ -105,6 +105,12 @@ function AnalyseAlbumViewModel() { }); item.isSuccessful = false; item.isPending = false; + + var indexToRemove = self.imagesInProgress.indexOf(item); + if (indexToRemove > -1) + { + self.imagesInProgress.splice(indexToRemove, 1); + } } } } diff --git a/resources/lang/en/permissions.php b/resources/lang/en/permissions.php index 9633200..9f96907 100644 --- a/resources/lang/en/permissions.php +++ b/resources/lang/en/permissions.php @@ -7,6 +7,7 @@ return [ 'manage-comments' => 'Manage comments', 'manage-groups' => 'Manage user groups', 'manage-labels' => 'Manage photo labels', + 'manage-services' => 'Manage external services', 'manage-storage' => 'Manage storage locations', 'manage-users' => 'Manage users' ], diff --git a/resources/views/themes/base/admin/analyse_album.blade.php b/resources/views/themes/base/admin/analyse_album.blade.php index 9c288c7..129c2d2 100644 --- a/resources/views/themes/base/admin/analyse_album.blade.php +++ b/resources/views/themes/base/admin/analyse_album.blade.php @@ -23,7 +23,7 @@

...

-

...

+

...

diff --git a/resources/views/themes/base/admin/edit_group.blade.php b/resources/views/themes/base/admin/edit_group.blade.php index 899c506..211ea37 100644 --- a/resources/views/themes/base/admin/edit_group.blade.php +++ b/resources/views/themes/base/admin/edit_group.blade.php @@ -102,6 +102,10 @@ @include(Theme::viewName('partials.permission_checkbox'), [ 'permission' => Theme::getPermission($all_permissions, 'admin', 'manage-storage') ]) + + @include(Theme::viewName('partials.permission_checkbox'), [ + 'permission' => Theme::getPermission($all_permissions, 'admin', 'manage-services') + ])
diff --git a/resources/views/themes/base/partials/navbar.blade.php b/resources/views/themes/base/partials/navbar.blade.php index 0cad1ed..59065c1 100644 --- a/resources/views/themes/base/partials/navbar.blade.php +++ b/resources/views/themes/base/partials/navbar.blade.php @@ -9,14 +9,14 @@ @if (!Auth::guest() && UserConfig::get('social_user_feeds'))
  • - @lang('navigation.navbar.activity_feed') + @lang('navigation.navbar.activity_feed')
    • @endif @can('photo.quick_upload')
  • - @lang('navigation.navbar.quick_post') + @lang('navigation.navbar.quick_post')
    • @endcan @@ -24,7 +24,7 @@ @if (count($g_albums) > 0)
  • @foreach ($g_albums_menu as $album) @@ -41,7 +41,7 @@ @if (count($g_labels) > 0)
  • @foreach ($g_labels as $label) @@ -57,7 +57,7 @@ @if (count($g_albums) > 0 && \App\User::currentOrAnonymous()->can('statistics.public-access'))
  • - @lang('navigation.navbar.statistics') + @lang('navigation.navbar.statistics')
    • @endif