Android - Fixes https://github.com/apache/cordova-plugin-network-information#110

[PieterVanPoyer]

Platforms affected

Android

Motivation and Context

Fixes #110 .

Description

The JSONObject of Android does not have an equals implementation. So the equals between previous networkinfo and new networkinfo state always returned false.
I've explicitly tested all properties of the JSONObject for equality.

Testing

I did test it on an Android Nexus 5X Emulator.

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

I didn't prefix the commit, but I did prefix the PR.

[PieterVanPoyer]

Hey

Can someone review this, please.
During the startup procedure, there are also events dispatched, but they are not checked with the equals mechanism (Same as before).
But IMO sometimes it fires 2 times when starting up the app.
I think that's more a job for a Cordova specialist, or we could wait for an issue to be filed.

In the startup routine it is just fired, no check has been done.

cordova-plugin-network-information/src/android/NetworkManager.java

Lines 114 to 127 in 9f85270

	public boolean execute(String action, JSONArray args, CallbackContext callbackContext) {
	if (action.equals("getConnectionInfo")) {
	this.connectionCallbackContext = callbackContext;
	NetworkInfo info = sockMan.getActiveNetworkInfo();
	String connectionType = "";
	try {
	connectionType = this.getConnectionInfo(info).get("type").toString();
	} catch (JSONException e) {
	LOG.d(LOG_TAG, e.getLocalizedMessage());
	}
	PluginResult pluginResult = new PluginResult(PluginResult.Status.OK, connectionType);
	pluginResult.setKeepCallback(true);
	callbackContext.sendPluginResult(pluginResult);

Maybe we should change the documentation and mention that no checks on Android are done when the app is in the background/idle.

Kind regards
Pieter

[breautek]

Thank you for your PR 🙇

This looks good to me. While this is fixing a bug, it is changing behaviour (which is to stop event firing on resume, if there was no change in state), which is a behaviour that some people may be dependent on. Because of this, I think we need to treat this as a breaking change.

Currently the plugin is targeting a patch release (2.0.3-dev) so it may be some time before this actually gets merged. I noticed that the PR is sourced from your fork's master branch. I think you should move the 2 commits into a new branch and recreate this PR. (You can literally copy & paste your description). This is to avoid having accidental commits pushed to this PR.

When you do this I'll give my explicit approval.

Testing notes

I've tested in an API 29 emulator and went through the following paths:

Path 1

1. Put app into background
2. Put app into foreground

Saw no event fire as expected, because there was no network change. ✔️

Path 2

1. App in foreground.
2. Turned off wifi
3. Observed "4g" event
4. Turned on wifi.
5. Observed "wifi" event.

Events fired as expected. ✔️

Path 3

1. App in foreground
2. App in background
3. Turned off wifi
4. App in foreground

"4g" event fired as expected on resume. ✔️

[PieterVanPoyer]

I've made a new PR, starting from another branche.
This one may be closed.

[breautek]

Thanks. The new PR that pulls from a sourced branch is located at #114.