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

@@ -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
+        }
     }
 
     /**

app/Exceptions/DropboxRetryException.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace App\Exceptions;
+
+class DropboxRetryException extends \Exception
+{
+    private $innerException;
+
+    /**
+     * @return mixed
+     */
+    public function getInnerException()
+    {
+        return $this->innerException;
+    }
+
+    public function __construct($httpCode, \Exception $innerException)
+    {
+        parent::__construct('Dropbox requested to retry the request');
+
+        $this->innerException = $innerException;
+    }
+}

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);
+                return redirect($dropbox->authoriseUrl($storage));
-                $viewData['callbackUrl'] = $dropbox->callbackUrl();
-                break;
 
             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);
     }
 
     /**

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,14 +100,19 @@ class DropboxService
         $this->accessToken = $accessToken;
     }
 
-    public function uploadFile($pathToFileToUpload, $pathToStorage)
+    public function uploadFile($pathToFileToUpload, $pathOnStorage)
     {
         $dropboxArgs = [
-            'path' => $pathToStorage,
+            'path' => $pathOnStorage,
-            'mode' => 'add',
+            'mode' => 'overwrite',
             'mute' => true
         ];
 
+        $shouldRetry = true;
+        while ($shouldRetry)
+        {
+            try
+            {
                 $uploadResult = $this->sendRequest(
                     $this->config['upload_url'],
                     'POST',
@@ -100,6 +125,22 @@ class DropboxService
                         '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);
         }
 
+        try
+        {
             if ($httpCode != 200 && $httpCode != 304)
             {
-            throw new \Exception(sprintf('Exception from Dropbox: %s', $result));
+                if ($httpCode == 429)
+                {
+                    throw new DropboxRetryException($httpCode, new \Exception(sprintf('Exception from Dropbox: %s', $result)));
                 }
 
-        curl_close($ch);
+                throw new \Exception(sprintf('Exception from Dropbox: %s', $result));
+            }
 
             return $postOptions['response_body_is_json']
                 ? json_decode($result)
                 : $result;
         }
+        finally
+        {
+            curl_close($ch);
+        }
+    }
 }

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'

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()

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);
+                            }
                         }
                     }
                 }

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'
     ],

resources/views/themes/base/admin/analyse_album.blade.php

@@ -23,7 +23,7 @@
                             <p class="text-success"><span v-text="image.name"></span> ... <i class="fa fa-fw fa-check"></i></p>
                         </div>
                         <div v-for="image in imagesInProgress.slice(0, 5)" style="margin-top: 20px;">
-                            <p><span v-text="image.name"></span> ... <i class="fa fa-fw fa-refresh"></i></p>
+                            <p><span v-text="image.name"></span> ... <i class="fa fa-fw fa-sync"></i></p>
                         </div>
 
                         <div v-if="imagesInProgress.length > 5">

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')
+                                ])
                             </div>
                         </div>
                     </div>

resources/views/themes/base/partials/navbar.blade.php

@@ -9,14 +9,14 @@
             @if (!Auth::guest() && UserConfig::get('social_user_feeds'))
                 <li class="nav-item">
                     <a class="nav-link" href="{{ route('userActivityFeed') }}">
-                        <i class="fa fa-rss"></i> @lang('navigation.navbar.activity_feed')
+                        <i class="fa fa-rss mr-1"></i> @lang('navigation.navbar.activity_feed')
                     </a>
                 </li>
             @endif
             @can('photo.quick_upload')
                 <li class="nav-item">
                     <a class="nav-link" href="#" data-toggle="modal" data-target="#quick-upload-modal">
-                        <i class="fa fa-plus"></i> @lang('navigation.navbar.quick_post')
+                        <i class="fa fa-plus mr-1"></i> @lang('navigation.navbar.quick_post')
                     </a>
                 </li>
             @endcan
@@ -24,7 +24,7 @@
             @if (count($g_albums) > 0)
                 <li class="nav-item dropdown ml-2">
                     <a class="nav-link dropdown-toggle" href="{{ url('/') }}" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
-                        <i class="fa fa-book"></i> @lang('navigation.navbar.albums')
+                        <i class="fa fa-book mr-1"></i> @lang('navigation.navbar.albums')
                     </a>
                     <div class="dropdown-menu">
                         @foreach ($g_albums_menu as $album)
@@ -41,7 +41,7 @@
             @if (count($g_labels) > 0)
                 <li class="nav-item dropdown ml-2">
                     <a class="nav-link dropdown-toggle" href="{{ url('/') }}" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
-                        <i class="fa fa-tags"></i> @lang('navigation.navbar.labels')
+                        <i class="fa fa-tags mr-1"></i> @lang('navigation.navbar.labels')
                     </a>
                     <div class="dropdown-menu">
                         @foreach ($g_labels as $label)
@@ -57,7 +57,7 @@
 
             @if (count($g_albums) > 0 && \App\User::currentOrAnonymous()->can('statistics.public-access'))
                 <li class="nav-item ml-2">
-                    <a class="nav-link" href="{{ route('statistics.index') }}"><i class="fa fa-chart-line"></i> @lang('navigation.navbar.statistics')</a>
+                    <a class="nav-link" href="{{ route('statistics.index') }}"><i class="fa fa-chart-line mr-1"></i> @lang('navigation.navbar.statistics')</a>
                 </li>
             @endif