amplify-android icon indicating copy to clipboard operation
amplify-android copied to clipboard

Add Boolean for Android File Transfer Notification, allowing Dev to Not Show It

Open djsjr opened this issue 2 years ago • 5 comments

Before opening, please confirm:

Language and Async Model

Kotlin

Amplify Categories

Storage

Gradle script dependencies

I am using Flutter with version 0.6.5 dependencies
// Put output below this line

Environment information

``` Error: Could not find or load main class org.gradle.wrapper..GradleWrapperMain ``` Not sure why ATM but N/A anyway

Please include any relevant guides or documentation you're referencing

https://developer.android.com/guide/components/foreground-services

Describe the feature request

Android docs specifically state that: > You should only use a foreground service when your app needs to perform a task that is noticeable by the user even when they're not directly interacting with the app. If the action is of low enough importance that you want to use a minimum-priority notification, create a background task instead.

Right now, Amplify has decided to use foreground service notifications to to ensure transfers are completed. However this goes against Android's own guidelines on foreground services, since it really isn't something that should be noticeable by the user in many cases. Many transfers are small enough that they are almost certainly completed before the user can put the app into the background anyway.

My suggestion for the immediate future (before getting into customizing notifications) is to allow the dev to decide whether or not a notification should be shown with an input variable for each transfer -- and warn the dev in the docs that the process could be killed should the app go into the background without a notification.

This way, larger transfers that the user is likely aware of (like media downloads) can be ensured while smaller ones that should not notify the user still go undetected.

Initialization steps (if applicable)

  1. initialize amplify
  2. make a call using the Amplify Storage to download a file, setting notification to false
  3. do not receive unpleasant notification

Code Snippet

same as current without notification, adding variable for notification

// Put your code below this line.
val file = File("${applicationContext.filesDir}/download.txt")
Amplify.Storage.downloadFile("ExampleKey", file, false
    { Log.i("MyAmplifyApp", "Successfully downloaded: ${it.file.name}") },
    { Log.e("MyAmplifyApp",  "Download Failure", it) }
)

amplifyconfiguration.json

No response

GraphQL Schema

N/A ```graphql // Put your schema below this line

</details>


### Additional information and screenshots

_No response_

djsjr avatar Aug 15 '22 19:08 djsjr

Thank you for feedback. We are still researching the optimal way to provide flexibility on when to show the Foreground notification, or if we can remove it entirely in the future. We will be replacing a lot of our internal implementation with WorkManager soon, which will give us more flexibility.

I do have a solution that may be work for you, but please see the caveats.

Possible Workaround

To prevent the notification from displaying, try adding these lines in the <application> block of your manifest:

<service 
    android:name="com.amplifyframework.storage.s3.service.AmplifyTransferService" 
    tools:replace="android:enabled" 
    android:enabled="false" />

This will prevent AmplifyTransferService from starting, which prvents the Foreground Notification from displaying.

Caveats

  • This will only work on the latest version of Amplify Android (1.37.2).
  • Disabling AmplifyTransferService will also disable the network detection handler that will restart transfers in changing network conditions. This would cause transfers to fail on network disconnections, rather than gracefully resume when connection is restored. This functionality could be restored by manually registering TranferNetworkLossHandler in a service of your own. See AmplifyTransferService source code as an example.
  • You will not be able to start a transfer from the background, unless you create your own foreground service.
  • Long running transfers may be killed if the application is backgrounded. In my testing, transfers will continue upon the app being backgrounded and the screen turned-off, however, there would be less guarantee the process does not get killed.
  • The code will still attempt to show AmplifyTransferService. In my testing, the failure will silently occur with a Warn log. I have not seen any crashes, but multiple versions of Android and devices should be tested on your application to verify this behavior.
    • W/ActivityManager: Unable to start service Intent { cmp=app.amplify.amplifyapplication/com.amplifyframework.storage.s3.service.AmplifyTransferService } U=0: not found

Once again, we understand the desire to initiate transfers without showing a foreground notification. We will continue researching the best approach internally, but in the meantime, this workaround may provide the result you are looking for.

tylerjroach avatar Aug 17 '22 18:08 tylerjroach

@tylerjroach

Thanks for the workaround but it will not work for us as we have a background service kicking off downloads and we cannot create a foreground service to do this.

We are looking to revert back to an older version of the framework where this notification is not present. It is not ideal of course but this is causing a lot of problems with our users.

Do you know at what version the notification was introduced so we can get the latest version without it? I have been looking back through the versions and it seems it came in version 1.35.6.

Also, I have been testing this notification and there a couple of problems with it we are seeing.

  1. The notification once there, stays forever.
  2. As a user I cannot swipe it away.

So once this notification comes, you cannot get rid of it without uninstalling the app. Is this a bug or are we doing something wrong? We are just downloading S3 files.

Amplify.Storage.downloadFile( .... )

This was on a pixel 4a, targeting 30 and running Android 12.

tracedmark avatar Aug 30 '22 07:08 tracedmark

Correct, the notification was introduced in 1.35.6, so 1.35.5 would be the last version that didn't show the notification. The Storage library has always used a Service to manage transfers. The notification introduction was a result of starting to bind the transfer service to the foreground so that Transfers could be initiated while the app was backgrounded on versions of Android < SDK 31. Recent versions of Android have placed heavy restrictions on what can/can't be done while the app is in the background. We will begin using WorkManager soon which will provide us with more flexibility on initiating transfers.

"You will not be able to start a transfer from the background, unless you create your own foreground service." This was meant as a suggestion on how to enable transfers from the background if you have removed. AmplifyTransferService. You may be able to come up with another solution that allows transfers to be initiated in the background. Transfers will report an onError immediately if the Android OS did not allow the transfer to proceed.

The latest releases fixed the bug where notifications were sticking to the drawer, and on the current version, the notification should only show while a transfer is in progress.

tylerjroach avatar Aug 30 '22 14:08 tylerjroach

I see what you are saying with the foreground service now. If we create our own version of AmplifyTransferService we can do what we like and remove/change the notification?

How would we register our service so the amplify api would use it instead of the disabled AmplifyTransferService?

tracedmark avatar Aug 30 '22 15:08 tracedmark

Correct, there isn't too much to it.

Check out: https://github.com/aws-amplify/amplify-android/blob/main/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.java. You can see that we just startTransferService when a transfer start/resume is initiated. Just start your own service before calling any Amplify transfer operation.

tylerjroach avatar Aug 30 '22 15:08 tylerjroach

Hi guys! Is there any news regarding this topic?

yaroslav-v avatar Nov 14 '22 10:11 yaroslav-v

Hi @yaroslav-v, we have removed the foreground notification from our upcoming v2 release. I can't confirm the exact release date but it is coming very soon. I will provide an update on this thread when v2 is available.

tylerjroach avatar Nov 14 '22 14:11 tylerjroach

@tylerjroach Thx!

yaroslav-v avatar Nov 14 '22 16:11 yaroslav-v

@djsjr, @tracedmark, @yaroslav-v Amplify Library for Android v2.0.0 has been released and has removed the Foreground Transfer Notification.

tylerjroach avatar Nov 18 '22 16:11 tylerjroach

@tylerjroach Hi! Thx for the update.

As far as I can see, it has breaking changes for the minimal OS version. So, it supports now Android 7.0 (Nougat) and higher.

Do you have any plans to port those changes that remove the Foreground Transfer Notification to v1.x? It'd be great because we still support Android 5+.

yaroslav-v avatar Nov 18 '22 17:11 yaroslav-v

We do not have plans to port the v2 work into v1 as it was almost a complete re-write from the v1 implementation. If using v2 is not possible at this time, the suggested workaround for v1 will still be required.

tylerjroach avatar Nov 18 '22 17:11 tylerjroach

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Dec 08 '22 19:12 github-actions[bot]