#105 fix a bug with any errors during drive space refresh were hidden

ylexus · Jun 28, 2021 · fc8cce2 · fc8cce2
1 parent 013390b
commit fc8cce2
Show file tree

Hide file tree

Showing 13 changed files with 88 additions and 10 deletions.
diff --git a/app/build.gradle b/app/build.gradle
@@ -49,7 +49,7 @@ dependencies {
     ext.orgJunitJupiterVersion = '5.7.2'
     ext.orgMockitoVersion = '3.10.0'
     ext.orgImmutablesVersion = '2.8.8'
-    ext.netYudichevJiottyVersion = '2.0.4'
+    ext.netYudichevJiottyVersion = '2.1-SNAPSHOT'
     ext.orgApacheLoggingLog4jVersion = '2.14.1'
 
     annotationProcessor "org.immutables:value:$orgImmutablesVersion"

diff --git a/app/src/main/java/net/yudichev/googlephotosupload/core/DriveSpaceTrackerImpl.java b/app/src/main/java/net/yudichev/googlephotosupload/core/DriveSpaceTrackerImpl.java
@@ -17,7 +17,6 @@
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
-import static java.util.concurrent.CompletableFuture.supplyAsync;
 import static net.yudichev.jiotty.common.lang.Locks.inLock;
 import static net.yudichev.jiotty.common.lang.MoreThrowables.getAsUnchecked;
 
@@ -55,13 +54,17 @@ final class DriveSpaceTrackerImpl implements DriveSpaceTracker {
 
     @Override
     public CompletableFuture<Void> reset() {
-        return supplyAsync(() -> {
+        return CompletableFuture.<Void>supplyAsync(() -> {
             inLock(lock, () -> {
                 //noinspection AssignmentToNull assigned in the method called next
                 driveSpaceStatus = null;
                 refreshDriveQuota();
             });
             return null;
+        }).whenComplete((ignored, e) -> {
+            if (driveSpaceStatus != null && e != null) {
+                driveSpaceStatus.close(false);
+            }
         });
     }
 
@@ -125,7 +128,8 @@ private void refreshDriveQuota() {
                         driveSpaceStatus.updateSuccess((int) toMegabytes(usage));
                         refreshStatusDescription();
                     }
-                });
+                })
+                .getNow(null); // otherwise any exceptions will be silently swallowed;
     }
 
     private void validateUsage() {

diff --git a/app/src/main/java/net/yudichev/googlephotosupload/ui/UploadPaneControllerImpl.java b/app/src/main/java/net/yudichev/googlephotosupload/ui/UploadPaneControllerImpl.java
@@ -1,5 +1,6 @@
 package net.yudichev.googlephotosupload.ui;
 
+import com.google.api.client.googleapis.json.GoogleJsonResponseException;
 import javafx.event.ActionEvent;
 import javafx.scene.control.Button;
 import javafx.scene.layout.VBox;
@@ -21,7 +22,9 @@
 import java.util.ResourceBundle;
 import java.util.concurrent.CompletableFuture;
 
+import static com.google.api.client.http.HttpStatusCodes.STATUS_CODE_FORBIDDEN;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Throwables.getCausalChain;
 import static javafx.application.Platform.runLater;
 import static net.yudichev.jiotty.common.lang.HumanReadableExceptionMessage.humanReadableMessage;
 
@@ -119,7 +122,7 @@ private void onUploadComplete(@Nullable Throwable exception) {
                     logger.error("Upload failed", exception);
                     logArea.getStyleClass().add("failed-background");
                     logAreaChildren.add(new Text(resourceBundle.getString("uploadPaneLogAreaFailurePrefix") + " "));
-                    var failureText = new Text(humanReadableMessage(exception));
+                    var failureText = new Text(toHumanReadableMessage(exception));
                     failureText.getStyleClass().add("failed-text");
                     logAreaChildren.add(failureText);
                 }
@@ -129,6 +132,26 @@ private void onUploadComplete(@Nullable Throwable exception) {
         });
     }
 
+    private String toHumanReadableMessage(Throwable exception) {
+        return getCausalChain(exception).stream()
+                .filter(throwable -> throwable instanceof GoogleJsonResponseException)
+                .findFirst()
+                .map(throwable -> (GoogleJsonResponseException) throwable)
+                .map(jsonResponseException -> {
+                    // better error for GoogleJsonResponseException, otherwise there's too much technical details.
+                    var details = jsonResponseException.getDetails();
+                    if (details != null && details.getMessage() != null) {
+                        if (details.getCode() == STATUS_CODE_FORBIDDEN) {
+                            return details.getMessage() + ' ' + resourceBundle.getString("uploadPanePermissionErrorSuffix");
+                        } else {
+                            return details.getMessage();
+                        }
+                    }
+                    return null;
+                })
+                .orElseGet(() -> humanReadableMessage(exception));
+    }
+
     @Override
     public void stopUpload() {
         stopButton.setDisable(true);

diff --git a/app/src/main/resources/i18n/Resources.properties b/app/src/main/resources/i18n/Resources.properties
@@ -85,6 +85,7 @@ supportPaneReportIssueSentence=Problems? Suggestions?
 supportPaneReportIssueLinkSentence=Raise an issue here.
 uploadPaneStopButtonText=Stop
 uploadPaneUploadMoreButtonText=Upload more...
+uploadPanePermissionErrorSuffix=Have you denied the app any requested permissions during login?
 directoryStructureSupplierProgressTitle=Looking for files
 cloudAlbumsProviderProgressTitle=Loading albums in Google Photos
 uploaderAlbumProgressTitle=Adding to albums

diff --git a/app/src/main/resources/i18n/Resources_es.properties b/app/src/main/resources/i18n/Resources_es.properties
@@ -98,6 +98,8 @@ supportPaneReportIssueSentence=Problemas? Sugerencias?
 supportPaneReportIssueLinkSentence=Plantea un problema aquí.
 uploadPaneStopButtonText=Detener
 uploadPaneUploadMoreButtonText=Subir más...
+#TODO translate
+uploadPanePermissionErrorSuffix=Have you denied the app any requested permissions during login?
 directoryStructureSupplierProgressTitle=Buscando archivos
 cloudAlbumsProviderProgressTitle=Cargando albumes en Google Photos
 uploaderAlbumProgressTitle=Añadiendo a los álbumes

diff --git a/app/src/main/resources/i18n/Resources_nl.properties b/app/src/main/resources/i18n/Resources_nl.properties
@@ -91,6 +91,8 @@ uiAuthorisationBrowserTitle=Login in Google
 uploadPaneLogAreaSuccessLabel=Success damen en heren
 uploadPaneLogAreaFailurePrefix=Er is iets fout gegaan:
 uploadPaneLogAreaPartialSuccessLabel=Klaar met enkele fouten, Om de fouten te bekijken klik op de "%s" links
+#TODO translate
+uploadPanePermissionErrorSuffix=Have you denied the app any requested permissions during login?
 folderSelectorDragHereLabel=Sleep je folder met media hierheen of
 folderSelectorBrowseButtonLabel=Bladeren...
 folderSelectorResumeCheckboxLabel=Ga door vanaf je laatste poging

diff --git a/app/src/main/resources/i18n/Resources_ru.properties b/app/src/main/resources/i18n/Resources_ru.properties
@@ -69,6 +69,7 @@ uiAuthorisationBrowserTitle=Войдите в Google
 uploadPaneLogAreaSuccessLabel=Дамы и господа, полёт завершён успешно!
 uploadPaneLogAreaFailurePrefix=Что-то пошло не так:
 uploadPaneLogAreaPartialSuccessLabel=Завершено, но с нескольмими ошибками. Ссылки "%s" выше покажут, что случилось.
+uploadPanePermissionErrorSuffix=Возможно, приложению не дан доступ к некоторым запрошенным ресурсам во время входа в аккаунт?
 folderSelectorDragHereLabel=Перетащите сюда папку с медиа-файлами или
 folderSelectorBrowseButtonLabel=Выберите на диске...
 folderSelectorResumeCheckboxLabel=Продолжить с места последней попытки

diff --git a/app/src/main/resources/i18n/Resources_zh_Hans.properties b/app/src/main/resources/i18n/Resources_zh_Hans.properties
@@ -73,6 +73,8 @@ uiAuthorisationBrowserTitle=登录Google
 uploadPaneLogAreaSuccessLabel=全部成功上传!
 uploadPaneLogAreaFailurePrefix=有错误产生:
 uploadPaneLogAreaPartialSuccessLabel=已完成，有部分失败，检视请按"%s"。
+#TODO translate
+uploadPanePermissionErrorSuffix=Have you denied the app any requested permissions during login?
 folderSelectorDragHereLabel=拖曳文件匣或文檔到这里，或
 folderSelectorBrowseButtonLabel=导航...
 folderSelectorResumeCheckboxLabel=从上次继续

diff --git a/app/src/main/resources/i18n/Resources_zh_Hant.properties b/app/src/main/resources/i18n/Resources_zh_Hant.properties
@@ -73,6 +73,8 @@ uiAuthorisationBrowserTitle=登入Google
 uploadPaneLogAreaSuccessLabel=全部成功上傳!
 uploadPaneLogAreaFailurePrefix=有錯誤產生:
 uploadPaneLogAreaPartialSuccessLabel=已完成，有部分失敗，檢視請按"%s"。
+#TODO translate
+uploadPanePermissionErrorSuffix=Have you denied the app any requested permissions during login?
 folderSelectorDragHereLabel=拖曳資料匣或檔案到這裡，或
 folderSelectorBrowseButtonLabel=瀏覽...
 folderSelectorResumeCheckboxLabel=從上次繼續

diff --git a/app/src/main/resources/log4j2-ui.yaml b/app/src/main/resources/log4j2-ui.yaml
@@ -21,7 +21,7 @@ Configuration:
 
   Loggers:
     logger:
-      - name: net.yudichev.jiotty.connector.google.photos
+      - name: net.yudichev.jiotty.connector.google
         level: debug
         includeLocation: true
       - name: net.yudichev.googlephotosupload

diff --git a/app/src/test/java/net/yudichev/googlephotosupload/core/IntegrationTest.java b/app/src/test/java/net/yudichev/googlephotosupload/core/IntegrationTest.java
@@ -1,11 +1,17 @@
 package net.yudichev.googlephotosupload.core;
 
+import com.google.api.client.googleapis.json.GoogleJsonError;
+import com.google.api.client.googleapis.json.GoogleJsonResponseException;
+import com.google.api.client.http.HttpHeaders;
+import com.google.api.client.http.HttpResponseException;
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import net.yudichev.googlephotosupload.core.RecordingGooglePhotosClient.Album;
 import net.yudichev.googlephotosupload.core.RecordingGooglePhotosClient.MediaItem;
 import net.yudichev.jiotty.common.app.Application;
 import net.yudichev.jiotty.common.lang.Json;
 import net.yudichev.jiotty.common.varstore.VarStore;
+import net.yudichev.jiotty.connector.google.drive.InMemoryGoogleDriveClient;
 import net.yudichev.jiotty.connector.google.photos.GoogleMediaItem;
 import net.yudichev.jiotty.connector.google.photos.GooglePhotosAlbum;
 import org.apache.commons.cli.CommandLineParser;
@@ -67,6 +73,7 @@ final class IntegrationTest {
     private RecordingGooglePhotosClient googlePhotosClient;
     private RecordingProgressStatusFactory progressStatusFactory;
     private Path settingsRootPath;
+    private InMemoryGoogleDriveClient googleDriveClient;
 
     @BeforeEach
     void setUp() throws IOException {
@@ -81,6 +88,7 @@ void setUp() throws IOException {
         TestTimeModule.resetTime();
 
         setDefaultPreferences();
+        googleDriveClient = new InMemoryGoogleDriveClient();
     }
 
     @AfterEach
@@ -773,6 +781,22 @@ void customisedRelevantFolderDepth1() throws Exception {
         ));
     }
 
+    @Test
+    void failsUploadIfDriveSpaceRefreshFails() throws InterruptedException {
+        var details = new GoogleJsonError();
+        details.setCode(403);
+        details.setMessage("Nope");
+        var rootCause = new GoogleJsonResponseException(
+                new HttpResponseException.Builder(403, "Nope", new HttpHeaders()),
+                details);
+        googleDriveClient.getBehaviour().failOnCreateFileWith(new RuntimeException(rootCause));
+
+        doExecuteUpload();
+        assertThat(getLastFailure().map(Throwables::getRootCause), optionalWithValue(equalTo(rootCause)));
+        var spaceUsedProgress = progressStatusFactory.getStatusByName().get("Google Account Space Used (currently unreliable!)");
+        assertThat(spaceUsedProgress.getClosedWithSuccess(), is(optionalWithValue(equalTo(false))));
+    }
+
     private void createStandardTestFiles() throws IOException {
         /*
         outerAlbum
@@ -877,6 +901,7 @@ private void doVerifyGoogleClientAlbumState() {
     }
 
     private void doVerifyGoogleClientItemState() {
+
         assertThat(googlePhotosClient.getAllItems(), containsInAnyOrder(
                 allOf(
                         itemForFile(rootPhoto),
@@ -908,7 +933,7 @@ private void doExecuteUpload(String... additionalCommandLineOptions) throws Inte
                     .addModule(TestTimeModule::new)
                     .addModule(() -> new CoreDependenciesModule(settingsModule.getAuthDataStoreRootPath()))
                     .addModule(() -> new MockGooglePhotosModule(googlePhotosClient))
-                    .addModule(MockGoogleDriveModule::new)
+                    .addModule(() -> new MockGoogleDriveModule(googleDriveClient))
                     .addModule(ResourceBundleModule::new)
                     .addModule(() -> new UploadPhotosModule(Duration.ofMillis(1)))
                     .addModule(() -> new IntegrationTestUploadStarterModule(commandLine, progressStatusFactory))
@@ -948,8 +973,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
                 return delete(dir);
             }
 
-            @SuppressWarnings("SameReturnValue")
-            private FileVisitResult delete(Path path) throws IOException {
+            private static FileVisitResult delete(Path path) throws IOException {
                 Files.delete(path);
                 return CONTINUE;
             }

diff --git a/app/src/test/java/net/yudichev/googlephotosupload/core/MockGoogleDriveModule.java b/app/src/test/java/net/yudichev/googlephotosupload/core/MockGoogleDriveModule.java
@@ -4,9 +4,18 @@
 import net.yudichev.jiotty.connector.google.drive.GoogleDriveClient;
 import net.yudichev.jiotty.connector.google.drive.InMemoryGoogleDriveClient;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 final class MockGoogleDriveModule extends AbstractModule {
+
+    private final InMemoryGoogleDriveClient googleDriveClient;
+
+    MockGoogleDriveModule(InMemoryGoogleDriveClient googleDriveClient) {
+        this.googleDriveClient = checkNotNull(googleDriveClient);
+    }
+
     @Override
     protected void configure() {
-        bind(GoogleDriveClient.class).toInstance(new InMemoryGoogleDriveClient());
+        bind(GoogleDriveClient.class).toInstance(googleDriveClient);
     }
 }
diff --git a/app/src/test/java/net/yudichev/googlephotosupload/core/RecordingProgressStatusFactory.java b/app/src/test/java/net/yudichev/googlephotosupload/core/RecordingProgressStatusFactory.java
@@ -44,6 +44,7 @@ static final class RecordingProgressStatus implements ProgressStatus {
         private Optional<Integer> totalCount;
         private int successCount;
         private String description;
+        private Optional<Boolean> closedWithSuccess = Optional.empty();
 
         private RecordingProgressStatus(Optional<Integer> totalCount) {
             this.totalCount = checkNotNull(totalCount);
@@ -83,6 +84,9 @@ public void onBackoffDelay(long backoffDelayMs) {
 
         @Override
         public void close(boolean success) {
+            inLock(lock, () -> {
+                closedWithSuccess = Optional.of(success);
+            });
         }
 
         @Override
@@ -105,5 +109,9 @@ public int getSuccessCount() {
         public String getDescription() {
             return inLock(lock, () -> description);
         }
+
+        public Optional<Boolean> getClosedWithSuccess() {
+            return inLock(lock, () -> closedWithSuccess);
+        }
     }
 }