Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small improvements for Android backup + custom behavior when quota exceeded #882

Merged
merged 9 commits into from
Jul 26, 2023
36 changes: 36 additions & 0 deletions patches/react-native-background-upload+6.6.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
diff --git a/node_modules/react-native-background-upload/android/src/main/java/com/vydia/RNUploader/GlobalRequestObserverDelegate.kt b/node_modules/react-native-background-upload/android/src/main/java/com/vydia/RNUploader/GlobalRequestObserverDelegate.kt
index c89d495..5da9d14 100644
--- a/node_modules/react-native-background-upload/android/src/main/java/com/vydia/RNUploader/GlobalRequestObserverDelegate.kt
+++ b/node_modules/react-native-background-upload/android/src/main/java/com/vydia/RNUploader/GlobalRequestObserverDelegate.kt
@@ -9,6 +9,7 @@ import com.facebook.react.modules.core.DeviceEventManagerModule.RCTDeviceEventEm
import net.gotev.uploadservice.data.UploadInfo
import net.gotev.uploadservice.network.ServerResponse
import net.gotev.uploadservice.observer.request.RequestObserverDelegate
+import net.gotev.uploadservice.exceptions.UploadError

class GlobalRequestObserverDelegate(reactContext: ReactApplicationContext) : RequestObserverDelegate {
private val TAG = "UploadReceiver"
@@ -28,6 +29,10 @@ class GlobalRequestObserverDelegate(reactContext: ReactApplicationContext) : Req
// Make sure we do not try to call getMessage() on a null object
if (exception != null) {
params.putString("error", exception.message)
+ if (exception is UploadError) {
+ params.putInt("responseCode", exception.serverResponse.code)
+ params.putString("responseBody", String(exception.serverResponse.body, Charsets.US_ASCII))
+ }
} else {
params.putString("error", "Unknown exception")
}
diff --git a/node_modules/react-native-background-upload/index.d.ts b/node_modules/react-native-background-upload/index.d.ts
index 8b2a07c..80cff93 100644
--- a/node_modules/react-native-background-upload/index.d.ts
+++ b/node_modules/react-native-background-upload/index.d.ts
@@ -11,6 +11,8 @@ declare module "react-native-background-upload" {

export interface ErrorData extends EventData {
error: string
+ responseCode: number
+ responseBody: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you create an upstream PR? can you link it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

export interface CompletedData extends EventData {
18 changes: 18 additions & 0 deletions src/app/domain/backup/helpers/error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export class BackupError extends Error {
statusCode: number | undefined

constructor(message: string, statusCode: number | undefined) {
const stringifiedMessage = JSON.stringify({
message,
statusCode
})

super(stringifiedMessage)
this.name = 'BackupError'
this.statusCode = statusCode
}
}

export const STACK_ERRORS_INTERRUPTING_BACKUP = [
413 // quota exceeded
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true all the time. You can have a 413 error when the file is too big (5 GB) for our storage service (swift).

If you want to handle that correctly, see desktop:

https://github.com/cozy-labs/cozy-desktop/blob/eb1d431d382b9e7dcb93c49421e0555c60085951/core/remote/errors.js#L274-L287

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be done later, not a blocker ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not know about this! I will update with this new PR : https://github.com/cozy/cozy-stack/pull/4069/files

]
37 changes: 37 additions & 0 deletions src/app/domain/backup/helpers/file.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {
getPathExtension,
getPathWithoutExtension,
getPathWithoutFilename
} from '/app/domain/backup/helpers'

describe('file helper', () => {
describe('getPathExtension', () => {
it.each([
['IMG_001.heic', 'heic'],
['/Folder/IMG_001.heic', 'heic'],
['IMG_001.heic.mov', 'mov']
])('with %p should return %p', (path, result) => {
expect(getPathExtension(path)).toBe(result)
})
})

describe('getPathWithoutExtension', () => {
it.each([
['IMG_001.heic', 'IMG_001'],
['/Folder/IMG_001.heic', '/Folder/IMG_001'],
['IMG_001.heic.mov', 'IMG_001.heic']
])('with %p should return %p', (path, result) => {
expect(getPathWithoutExtension(path)).toBe(result)
})
})

describe('getPathWithoutFilename', () => {
it.each([
['IMG_001.heic', ''],
['/Folder/IMG_001.heic', '/Folder'],
['/Folder/Directory/IMG_001.heic', '/Folder/Directory']
])('with %p should return %p', (path, result) => {
expect(getPathWithoutFilename(path)).toBe(result)
})
})
})
11 changes: 11 additions & 0 deletions src/app/domain/backup/helpers/file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export const getPathExtension = (path: string): string => {
return path.substring(path.lastIndexOf('.') + 1)
}

export const getPathWithoutExtension = (path: string): string => {
return path.substring(0, path.lastIndexOf('.'))
}

export const getPathWithoutFilename = (path: string): string => {
return path.substring(0, path.lastIndexOf('/'))
}
2 changes: 2 additions & 0 deletions src/app/domain/backup/helpers/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from '/app/domain/backup/helpers/error'
export * from '/app/domain/backup/helpers/file'
19 changes: 11 additions & 8 deletions src/app/domain/backup/services/getMedias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import RNFS from 'react-native-fs'

import { Media, BackupedMedia, Album } from '/app/domain/backup/models'
import { getLocalBackupConfig } from '/app/domain/backup/services/manageLocalBackupConfig'
import {
getPathWithoutExtension,
getPathWithoutFilename
} from '/app/domain/backup/helpers'
import { t } from '/locales/i18n'

import type CozyClient from 'cozy-client'

Expand Down Expand Up @@ -36,7 +41,7 @@ export const getMimeType = (media: Media): string => {
const guessedMimeType = mime.getType(media.name)

if (guessedMimeType === null) {
throw new Error('File is not supported for backup')
throw new Error(t('services.backup.errors.fileNotSupported'))
}

return guessedMimeType
Expand All @@ -52,16 +57,14 @@ export const getRemotePath = (uri: string): string => {
const uriWithExternalStorageDirectoryPathRemoved =
uriWithProtocolRemoved.replace(RNFS.ExternalStorageDirectoryPath, '')

const uriWithFilenameRemoved =
uriWithExternalStorageDirectoryPathRemoved.substring(
0,
uriWithExternalStorageDirectoryPathRemoved.lastIndexOf('/')
)
const uriWithFilenameRemoved = getPathWithoutFilename(
uriWithExternalStorageDirectoryPathRemoved
)

return uriWithFilenameRemoved
}

throw new Error('Platform is not supported for backup')
throw new Error(t('services.backup.errors.platformNotSupported'))
}

export const formatMediasFromPhotoIdentifier = (
Expand All @@ -83,7 +86,7 @@ export const formatMediasFromPhotoIdentifier = (
if (subTypes.includes('PhotoLive')) {
return [
{
name: filename.substring(0, filename.lastIndexOf('.')) + '.MOV',
name: getPathWithoutExtension(filename) + '.MOV',
path: uri,
remotePath: getRemotePath(uri),
type: 'video',
Expand Down
6 changes: 3 additions & 3 deletions src/app/domain/backup/services/manageLocalBackupConfig.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type CozyClient from 'cozy-client'
import Minilog from 'cozy-minilog'

import {
Expand All @@ -12,8 +13,7 @@ import {
storeUserPersistedData,
UserPersistedStorageKeys
} from '/libs/localStore'

import type CozyClient from 'cozy-client'
import { t } from '/locales/i18n'

const log = Minilog('💿 Backup')

Expand Down Expand Up @@ -45,7 +45,7 @@ export const getLocalBackupConfig = async (
)

if (backupConfig === null) {
throw new Error('Local backup config has not been initialized')
throw new Error(t('services.backup.errors.configNotInitialized'))
}

return backupConfig
Expand Down
5 changes: 3 additions & 2 deletions src/app/domain/backup/services/managePermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
NATIVE_PERMISSIONS,
NativePermissionStatus
} from '/app/domain/nativePermissions'
import { t } from '/locales/i18n'

export const checkBackupPermissions =
async (): Promise<NativePermissionStatus> => {
Expand Down Expand Up @@ -33,7 +34,7 @@ export const checkBackupPermissions =
)
}

throw new Error('Platform is not supported for backup')
throw new Error(t('services.backup.errors.platformNotSupported'))
}

export const requestBackupPermissions =
Expand Down Expand Up @@ -62,5 +63,5 @@ export const requestBackupPermissions =
)
}

throw new Error('Platform is not supported for backup')
throw new Error(t('services.backup.errors.platformNotSupported'))
}
10 changes: 5 additions & 5 deletions src/app/domain/backup/services/manageRemoteBackupConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ import {
File,
FilesQueryAllResult
} from '/app/domain/backup/queries'
import { getPathWithoutFilename } from '/app/domain/backup/helpers'
import { t } from '/locales/i18n'

import { IOCozyFile } from 'cozy-client/types/types'

const DOCTYPE_APPS = 'io.cozy.apps'
const DOCTYPE_FILES = 'io.cozy.files'
const BACKUP_REF = `io.cozy.apps/photos/mobile`
const BACKUP_ROOT_PATH = '/Sauvegardé depuis mon mobile'

interface BackupFolder {
_id: string
Expand Down Expand Up @@ -128,7 +129,7 @@ export const createRemoteBackupFolder = async (
data: { _id: backupRootId }
} = await client
.collection(DOCTYPE_FILES)
.createDirectoryByPath(BACKUP_ROOT_PATH)
.createDirectoryByPath(t('services.backup.backupRootPath'))

const backupFolderAttributes = {
type: 'directory',
Expand Down Expand Up @@ -184,9 +185,8 @@ const formatBackupedMedia = (
''
)

const pathWithFilenameRemoved = pathWithBackupFolderRemoved.substring(
0,
pathWithBackupFolderRemoved.lastIndexOf('/')
const pathWithFilenameRemoved = getPathWithoutFilename(
pathWithBackupFolderRemoved
)

return {
Expand Down
43 changes: 32 additions & 11 deletions src/app/domain/backup/services/uploadMedia.android.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import RNBackgroundUpload, {

import { getMimeType } from '/app/domain/backup/services/getMedias'
import { Media, UploadMediaResult } from '/app/domain/backup/models/Media'
import { t } from '/locales/i18n'

import CozyClient, { IOCozyFile } from 'cozy-client'
import CozyClient, { StackErrors, IOCozyFile } from 'cozy-client'

let currentUploadId: string | undefined

Expand Down Expand Up @@ -40,7 +41,33 @@ export const uploadMedia = async (
}`
},
notification: {
enabled: false
enabled: true,
autoClear: true,
onProgressTitle: t('services.backup.notifications.onProgressTitle'),
onProgressMessage: t(
'services.backup.notifications.onProgressMessage',
{
filename: media.name
}
),
onCompleteTitle: t('services.backup.notifications.onCompleteTitle'),
onCompleteMessage: t(
'services.backup.notifications.onCompleteMessage',
{
filename: media.name
}
),
onErrorTitle: t('services.backup.notifications.onErrorTitle'),
onErrorMessage: t('services.backup.notifications.onErrorMessage', {
filename: media.name
}),
onCancelledTitle: t('services.backup.notifications.onCancelledTitle'),
onCancelledMessage: t(
'services.backup.notifications.onCancelledMessage',
{
filename: media.name
}
)
}
} as UploadOptions

Expand All @@ -49,16 +76,10 @@ export const uploadMedia = async (
setCurrentUploadId(uploadId)

RNBackgroundUpload.addListener('error', uploadId, error => {
// RNBackgroundUpload does not return status code and response body...
const { errors } = JSON.parse(error.responseBody) as StackErrors
reject({
statusCode: -1,
errors: [
{
status: -1,
title: error.error,
detail: error.error
}
]
statusCode: error.responseCode,
errors
})
})
RNBackgroundUpload.addListener('cancelled', uploadId, data => {
Expand Down
14 changes: 14 additions & 0 deletions src/app/domain/backup/services/uploadMedia.ios.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { getVideoPathFromLivePhoto } from '/app/domain/backup/services/uploadMedia.ios'

describe('uploadMedia.ios', () => {
describe('getVideoPathFromLivePhoto', () => {
it.each([
['IMG_001.HEIC', 'IMG_001.MOV'],
['FullSizeRender.heic', 'FullSizeRender.mov'],
['/Folder/IMG_001.HEIC', '/Folder/IMG_001.MOV'],
['/Folder/FullSizeRender.heic', '/Folder/FullSizeRender.mov']
])('with %p should return %p', (path, result) => {
expect(getVideoPathFromLivePhoto(path)).toBe(result)
})
})
})
19 changes: 16 additions & 3 deletions src/app/domain/backup/services/uploadMedia.ios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,24 @@ import RNFileSystem from 'react-native-fs'

import { getMimeType } from '/app/domain/backup/services/getMedias'
import { Media, UploadMediaResult } from '/app/domain/backup/models/Media'
import {
getPathExtension,
getPathWithoutExtension
} from '/app/domain/backup/helpers'
import { t } from '/locales/i18n'

import CozyClient, { StackErrors, IOCozyFile } from 'cozy-client'

const getVideoPathFromLivePhoto = (photoPath: string): string => {
return photoPath.substring(0, photoPath.lastIndexOf('.')) + '.MOV'
export const getVideoPathFromLivePhoto = (photoPath: string): string => {
const extension = getPathExtension(photoPath)

if (extension === extension.toUpperCase()) {
// Path is something like IMG_001.MOV
return getPathWithoutExtension(photoPath) + '.MOV'
} else {
// When Live Photo has been modified, path is something like FullSizeRender.mov
return getPathWithoutExtension(photoPath) + '.mov'
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seriously? 😮‍💨

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

One day we will update react-native-camera-roll to handle this correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? New version of camera-roll handle that? Why can't we upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No no I wanted to say "we will make a PR to add the feature directly on react-native-camera-roll"

}

const getRealFilepath = async (media: Media): Promise<string> => {
Expand All @@ -17,7 +30,7 @@ const getRealFilepath = async (media: Media): Promise<string> => {
let filepath = data.node.image.filepath

if (filepath === null) {
throw new Error('Impossible to get a real filepath')
throw new Error(t('services.backup.errors.fileNotFound'))
}

filepath = filepath.replace('file://', '')
Expand Down
Loading
Loading