fix: connectivity service #16232
fix: connectivity service #16232
Conversation
alperozturk96
force-pushed
the
fix/connectivity-serv
branch
from
deba220 to
c424058
Compare
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
alperozturk96
force-pushed
the
fix/connectivity-serv
branch
from
0261ff4 to
f2a1b3a
Compare
|
test-Unit test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/16232-Unit-test-07-36 |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/16232.apk |
ZetaTom
left a comment
ZetaTom
left a comment
There was a problem hiding this comment.
Code-wise this looks good to me. I've added a few minor suggestions about the comments.
I'm currently testing this pull request but this will take a bit longer due to the shear amount of scenarios considering the following:
- Aeroplane Mode
- WiFi, Mobile, or both
- In-app manual upload
- 'Share to Nextcloud' manual upload
- Auto Upload
- Auto Upload (only on unmetered WiFi)
- VPN connections
I came up with a Matrix to test most of these combinations and will share my findings tomorrow, hopefully.
| * | ||
| * <p>The implementation performs the following steps:</p> | ||
| * <ul> | ||
| * <li>Uses cached results from {@link WalledCheckCache} when available to avoid | ||
| * redundant network calls.</li> | ||
| * <li>Retrieves the active {@link Server} from {@link UserAccountManager}.</li> | ||
| * <li>If connected issues a lightweight | ||
| * HTTP {@code GET} request to the server’s <code>/index.php/204</code> endpoint | ||
| * (which should respond with HTTP 204 No Content when connectivity is healthy).</li> | ||
| * <li>If the response differs from the expected 204 No Content, the connection is | ||
| * assumed to be behind a captive portal or otherwise restricted.</li> | ||
| * <li>If no active network or server is detected, the method assumes the Internet | ||
| * is walled.</li> | ||
| * </ul> |
There was a problem hiding this comment.
| * | |
| * <p>The implementation performs the following steps:</p> | |
| * <ul> | |
| * <li>Uses cached results from {@link WalledCheckCache} when available to avoid | |
| * redundant network calls.</li> | |
| * <li>Retrieves the active {@link Server} from {@link UserAccountManager}.</li> | |
| * <li>If connected issues a lightweight | |
| * HTTP {@code GET} request to the server’s <code>/index.php/204</code> endpoint | |
| * (which should respond with HTTP 204 No Content when connectivity is healthy).</li> | |
| * <li>If the response differs from the expected 204 No Content, the connection is | |
| * assumed to be behind a captive portal or otherwise restricted.</li> | |
| * <li>If no active network or server is detected, the method assumes the Internet | |
| * is walled.</li> | |
| * </ul> |
As this is an interface it should only serve as a general description of the function and not describe the implementation.
| * <p>The check is based on {@link #isInternetWalled()} — if the Internet is not | ||
| * walled (i.e., the server is reachable and not restricted by a captive portal), | ||
| * this method reports {@code true}. Otherwise, it reports {@code false}.</p> | ||
| * | ||
| * @param callback A callback to handle the result of the network and server availability check. | ||
| * @param callback a callback that receives {@code true} when the network and | ||
| * Nextcloud server are reachable, or {@code false} otherwise. |
There was a problem hiding this comment.
I think this should only reference isInternetWalled() and not describe what that function returns as this is documented in the function itself.
2 participants
Fixes: #14681
Deduplicates
isNetworkAndServerAvailablelogicRemoves
c.isWifi() && !c.isMetered()from internet walled logic sinceisConnectedalready covers local internet and internet accessImplements
ConnectivityManager.NetworkCallbackfor better network state listeningRemoves deprecated logics
Adds better documentation