Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[Connectivity] migrate to the new android embedding #2142

Merged
cyanglaz merged 26 commits into from
Oct 15, 2019
Merged

[Connectivity] migrate to the new android embedding #2142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ae42d12
draft
Oct 3, 2019
67f4f70
migrate connectivity
Oct 3, 2019
6aebaf7
revert some changes
Oct 3, 2019
f682eb7
revert testing code
Oct 3, 2019
8922d5b
add
Oct 3, 2019
b24cfc9
revert
Oct 3, 2019
7267b11
remove deps
Oct 3, 2019
4926557
javadoc
Oct 4, 2019
60dde9e
refacotring and testing
Oct 4, 2019
24d949a
add nonnull notation
Oct 4, 2019
13c60dd
remove empty line
Oct 4, 2019
1516b81
formatting
Oct 4, 2019
b3abbe4
review fixes
Oct 4, 2019
a212fc9
some javadoc rewrites and update broadcast receiver related structure
Oct 5, 2019
dfcd8c9
more javadoc fixes
Oct 5, 2019
98c2d09
add lisence header
Oct 7, 2019
a58e305
merge new and old plugins
Oct 7, 2019
ba9a2fb
review fixes
Oct 7, 2019
43bfaea
refactor
Oct 7, 2019
ba0bb22
rename activities
Oct 8, 2019
ef9762e
non public
Oct 8, 2019
e118ec6
fix gradle script
Oct 11, 2019
5194df3
version, e2e and more
Oct 14, 2019
17213fd
merge
Oct 14, 2019
1ee15d8
formatting
Oct 14, 2019
114435d
Update pubspec.yaml
Oct 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
migrate connectivity
  • Loading branch information
Chris Yang committed Oct 3, 2019
commit 67f4f705f3602d128d57688789dc5bd504f154d1
54 changes: 24 additions & 30 deletions ...nnectivity/android/src/main/java/dev/flutter/plugins/connectivity/ConnectivityPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,35 @@

package dev.flutter.plugins.connectivity;

import android.hardware.camera2.CameraAccessException;
import android.os.Build;

//import androidx.annotation.NonNull;

import io.flutter.embedding.engine.plugins.FlutterPlugin;
import io.flutter.embedding.engine.plugins.activity.ActivityAware;
import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding;
import io.flutter.plugin.common.EventChannel;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.MethodChannel.MethodCallHandler;
import io.flutter.plugin.common.MethodChannel.Result;
import io.flutter.plugin.common.PluginRegistry;
import io.flutter.plugins.connectivity.ConnectivityMethodChannelHandler;

public class ConnectivityPlugin implements FlutterPlugin {
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

Would you mind add Javadocs to all public, non-trivial classes/methods/fields?

Copy link
Contributor

@amirh amirh Oct 4, 2019

Choose a reason for hiding this comment

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

New classes members/methods added should definitely be added with Javadocs.

Regarding all existing code - this is definitely tech debt we should be paying, and is welcomed if the author is interested in doing it as part of this PR, though it shouldn't block it. For the purpose of supporting the v2 embedder in all plugins I suggest we focus PRs and reviews on that goal. Please do leave any improvement suggestions of existing code as non-blocking nits.

I think this fits with the spirit of the Flutter code review guidelines:

In general, reviewers should favor approving a PR once it is in a state where it definitely improves the overall code health of the system being worked on, even if the PR isn't perfect.


private FlutterPluginBinding pluginBinding;

@Override
public void onAttachedToEngine(FlutterPluginBinding binding) {
this.pluginBinding = binding;
final MethodChannel channel =
new MethodChannel(pluginBinding.getFlutterEngine().getDartExecutor(), "plugins.flutter.io/connectivity");
final EventChannel eventChannel = new EventChannel(pluginBinding.getFlutterEngine().getDartExecutor(), "plugins.flutter.io/connectivity_status");

ConnectivityMethodChannelHandler handler = new ConnectivityMethodChannelHandler(pluginBinding.getApplicationContext());

channel.setMethodCallHandler(handler);
eventChannel.setStreamHandler(handler);
}

@Override
public void onDetachedFromEngine(FlutterPluginBinding binding) {
this.pluginBinding = null;
}
private FlutterPluginBinding pluginBinding;

@Override
public void onAttachedToEngine(FlutterPluginBinding binding) {
this.pluginBinding = binding;
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

You don't need to cache the binding if you only use it within these 2 methods.

final MethodChannel channel =
new MethodChannel(
pluginBinding.getFlutterEngine().getDartExecutor(), "plugins.flutter.io/connectivity");
final EventChannel eventChannel =
new EventChannel(
pluginBinding.getFlutterEngine().getDartExecutor(),
"plugins.flutter.io/connectivity_status");

ConnectivityMethodChannelHandler handler =
new ConnectivityMethodChannelHandler(pluginBinding.getApplicationContext());

channel.setMethodCallHandler(handler);
eventChannel.setStreamHandler(handler);
}

@Override
public void onDetachedFromEngine(FlutterPluginBinding binding) {
this.pluginBinding = null;
}
}
280 changes: 139 additions & 141 deletions ...droid/src/main/java/io/flutter/plugins/connectivity/ConnectivityMethodChannelHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,155 +11,153 @@
import android.net.wifi.WifiInfo;
import android.net.wifi.WifiManager;
import android.os.Build;

import io.flutter.plugin.common.EventChannel;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.PluginRegistry;
import io.flutter.view.TextureRegistry;

public class ConnectivityMethodChannelHandler implements MethodChannel.MethodCallHandler, EventChannel.StreamHandler {

private final Context context;
private ConnectivityManager manager;
private BroadcastReceiver receiver;

public ConnectivityMethodChannelHandler(Context context) {
assert(context != null);
this.context = context;
this.manager = (ConnectivityManager)context.getApplicationContext().getSystemService(Context.CONNECTIVITY_SERVICE);
}

@Override
public void onListen(Object arguments, EventChannel.EventSink events) {
context.registerReceiver(receiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
}

@Override
public void onCancel(Object arguments) {
context.unregisterReceiver(receiver);
receiver = null;
}

@Override
public void onMethodCall(MethodCall call, MethodChannel.Result result) {
switch (call.method) {
case "check":
handleCheck(call, result);
break;
case "wifiName":
handleWifiName(call, result);
break;
case "wifiBSSID":
handleBSSID(call, result);
break;
case "wifiIPAddress":
handleWifiIPAddress(call, result);
break;
default:
result.notImplemented();
break;
}
}

private String getNetworkType(ConnectivityManager manager) {
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Network network = manager.getActiveNetwork();
NetworkCapabilities capabilities = manager.getNetworkCapabilities(network);
if (capabilities == null) {
return "none";
}
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
|| capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)) {
return "wifi";
}
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
return "mobile";
}
}

return getNetworkTypeLegacy(manager);
}

@SuppressWarnings("deprecation")
private String getNetworkTypeLegacy(ConnectivityManager manager) {
// handle type for Android versions less than Android 9
NetworkInfo info = manager.getActiveNetworkInfo();
if (info == null || !info.isConnected()) {
return "none";
}
int type = info.getType();
switch (type) {
case ConnectivityManager.TYPE_ETHERNET:
case ConnectivityManager.TYPE_WIFI:
case ConnectivityManager.TYPE_WIMAX:
return "wifi";
case ConnectivityManager.TYPE_MOBILE:
case ConnectivityManager.TYPE_MOBILE_DUN:
case ConnectivityManager.TYPE_MOBILE_HIPRI:
return "mobile";
default:
return "none";
}
public class ConnectivityMethodChannelHandler
implements MethodChannel.MethodCallHandler, EventChannel.StreamHandler {

private final Context context;
private ConnectivityManager manager;
private BroadcastReceiver receiver;

public ConnectivityMethodChannelHandler(Context context) {
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

Recommendation: If you explicitly pass a ConnectivityManager instance, and if you create an interface around broadcast registration, passing an instance of that interface, instead of passing a Context, then you'll be a much better position to test interactions in this class.

assert (context != null);
this.context = context;
this.manager =
(ConnectivityManager)
context.getApplicationContext().getSystemService(Context.CONNECTIVITY_SERVICE);
}

@Override
public void onListen(Object arguments, EventChannel.EventSink events) {
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

Please add nullability annotations to all input parameters and return values for each public and private method.

context.registerReceiver(receiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
}

@Override
public void onCancel(Object arguments) {
context.unregisterReceiver(receiver);
receiver = null;
}

@Override
public void onMethodCall(MethodCall call, MethodChannel.Result result) {
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

Would you mind adding unit tests for this class to ensure that incoming messages are correctly parsed, and outgoing messages are correctly serialized?

Copy link
Contributor

@collinjackson collinjackson Oct 15, 2019

Choose a reason for hiding this comment

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

I think that Dart e2e integration tests would be acceptable as an alternative to unit tests.

switch (call.method) {
case "check":
handleCheck(call, result);
break;
case "wifiName":
handleWifiName(call, result);
break;
case "wifiBSSID":
handleBSSID(call, result);
break;
case "wifiIPAddress":
handleWifiIPAddress(call, result);
break;
default:
result.notImplemented();
break;
}

private BroadcastReceiver createReceiver(final EventChannel.EventSink events) {
return new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
events.success(checkNetworkType());
}
};
}

private String checkNetworkType() {
return getNetworkType(manager);
}

private String getNetworkType(ConnectivityManager manager) {
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Network network = manager.getActiveNetwork();
NetworkCapabilities capabilities = manager.getNetworkCapabilities(network);
if (capabilities == null) {
return "none";
}
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
|| capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)) {
return "wifi";
}
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
return "mobile";
}
}

private void handleCheck(MethodCall call, final MethodChannel.Result result) {
result.success(checkNetworkType());
}
return getNetworkTypeLegacy(manager);
}

private WifiInfo getWifiInfo() {
WifiManager wifiManager =
(WifiManager)
context.getApplicationContext().getSystemService(Context.WIFI_SERVICE);
return wifiManager == null ? null : wifiManager.getConnectionInfo();
@SuppressWarnings("deprecation")
private String getNetworkTypeLegacy(ConnectivityManager manager) {
// handle type for Android versions less than Android 9
NetworkInfo info = manager.getActiveNetworkInfo();
if (info == null || !info.isConnected()) {
return "none";
}

private void handleWifiName(MethodCall call, final MethodChannel.Result result) {
WifiInfo wifiInfo = getWifiInfo();
String ssid = null;
if (wifiInfo != null) ssid = wifiInfo.getSSID();
if (ssid != null) ssid = ssid.replaceAll("\"", ""); // Android returns "SSID"
result.success(ssid);
}

private void handleBSSID(MethodCall call, MethodChannel.Result result) {
WifiInfo wifiInfo = getWifiInfo();
String bssid = null;
if (wifiInfo != null) bssid = wifiInfo.getBSSID();
result.success(bssid);
}

private void handleWifiIPAddress(MethodCall call, final MethodChannel.Result result) {
WifiManager wifiManager =
(WifiManager)
context.getApplicationContext().getSystemService(Context.WIFI_SERVICE);

WifiInfo wifiInfo = null;
if (wifiManager != null) wifiInfo = wifiManager.getConnectionInfo();

String ip = null;
int i_ip = 0;
if (wifiInfo != null) i_ip = wifiInfo.getIpAddress();

if (i_ip != 0)
ip =
String.format(
"%d.%d.%d.%d",
(i_ip & 0xff), (i_ip >> 8 & 0xff), (i_ip >> 16 & 0xff), (i_ip >> 24 & 0xff));

result.success(ip);
int type = info.getType();
switch (type) {
case ConnectivityManager.TYPE_ETHERNET:
case ConnectivityManager.TYPE_WIFI:
case ConnectivityManager.TYPE_WIMAX:
return "wifi";
case ConnectivityManager.TYPE_MOBILE:
case ConnectivityManager.TYPE_MOBILE_DUN:
case ConnectivityManager.TYPE_MOBILE_HIPRI:
return "mobile";
default:
return "none";
}
}

private BroadcastReceiver createReceiver(final EventChannel.EventSink events) {
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

Recommendation: If I were migrating this, I would probably restructure such that this BroadcastReceiver is separated from this MethodCallHandler and then bring them together in a third class.

return new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
events.success(checkNetworkType());
}
};
}

private String checkNetworkType() {
return getNetworkType(manager);
}

private void handleCheck(MethodCall call, final MethodChannel.Result result) {
result.success(checkNetworkType());
}

private WifiInfo getWifiInfo() {
WifiManager wifiManager =
(WifiManager) context.getApplicationContext().getSystemService(Context.WIFI_SERVICE);
return wifiManager == null ? null : wifiManager.getConnectionInfo();
}

private void handleWifiName(MethodCall call, final MethodChannel.Result result) {
WifiInfo wifiInfo = getWifiInfo();
String ssid = null;
if (wifiInfo != null) ssid = wifiInfo.getSSID();
if (ssid != null) ssid = ssid.replaceAll("\"", ""); // Android returns "SSID"
result.success(ssid);
}

private void handleBSSID(MethodCall call, MethodChannel.Result result) {
WifiInfo wifiInfo = getWifiInfo();
String bssid = null;
if (wifiInfo != null) bssid = wifiInfo.getBSSID();
result.success(bssid);
}

private void handleWifiIPAddress(MethodCall call, final MethodChannel.Result result) {
WifiManager wifiManager =
Copy link
Contributor

@matthew-carroll matthew-carroll Oct 3, 2019

Choose a reason for hiding this comment

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

Recommendation: passing this as a constructor argument allows you to mock for tests.

(WifiManager) context.getApplicationContext().getSystemService(Context.WIFI_SERVICE);

WifiInfo wifiInfo = null;
if (wifiManager != null) wifiInfo = wifiManager.getConnectionInfo();

String ip = null;
int i_ip = 0;
if (wifiInfo != null) i_ip = wifiInfo.getIpAddress();

if (i_ip != 0)
ip =
String.format(
"%d.%d.%d.%d",
(i_ip & 0xff), (i_ip >> 8 & 0xff), (i_ip >> 16 & 0xff), (i_ip >> 24 & 0xff));

result.success(ip);
}
}
Loading