fix(datastore): Update network connection availability checking status logic in ReachabilityMonitor of Datastore

aladine commented 5 months ago

[x] PR title and description conform to Pull Request guidelines.

Issue #, if available: https://github.com/aws-amplify/amplify-android/issues/2738

Description of changes: Enhance the capability to detect network connection availabilities in syncengine of aws datastore. This includes:

Instead of checking activeNetwork, connectivity provider could retrieve getNetworkCapabilities and check the network capability is valid.
Network callback should have an extra onCapabilitiesChanged(). Since amplify-android supports API 23 and above, onCapabilitiesChanged should be called immediately after onAvailable callback and contains accurate information to determine network connection type.
A more fine grained detail to support different Android SDK version for network checking.

How did you test these changes? (Please add a line here how the changes were tested) Add unit test to verify the new change

Documentation update required?

[x] No
[ ] Yes (Please include a PR link for the documentation update)

General Checklist

[x] Added Unit Tests
[ ] Added Integration Tests
[ ] Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

tylerjroach commented 5 months ago

Hi @aladine, thank you for your contribution submission. We will prioritize reviewing this PR and reply back!

aladine commented 3 months ago

Any update in this pull request?

tylerjroach commented 3 months ago

Hi @aladine, sorry for the delay. I will try and provide some feedback within next few days and suggest any updates if needed.

aladine commented 3 months ago

No problem, I am happy to answer all the questions or concerns for this pull request.

aladine commented 3 months ago

Overall changes look good. Can you take a look at an alternate implementation here: https://github.com/aws-amplify/amplify-android/tree/tjroach/improve-network-detection-datastore

I attempted to take in Matt's suggestions and simplify it a bit. I believe the code functions identically to yours. The tests would just need updated to use MockK

Thanks Tyler, I move the codes from NetworkUtil to extensions folder as your branch. Update test location and mock method also.

aladine commented 1 week ago

Hi all, a kind ping for the progress of this pull request. I can simplify my logic if it introduces churns to the current implementation.

aws-amplify / amplify-android

fix(datastore): Update network connection availability checking status logic in ReachabilityMonitor of Datastore #2854