Move savedNetworkConfig into HotspotManager and use constructor injection

bramble-api/src/main/java/org/briarproject/bramble/api/settings/SettingsManager.java

@@ -22,11 +22,4 @@ public interface SettingsManager {
 	 * namespace.
 	 */
 	void mergeSettings(Settings s, String namespace) throws DbException;
-
-	/**
-	 * Merges the given settings with any existing settings in the given
-	 * namespace.
-	 */
-	void mergeSettings(Transaction txn, Settings s, String namespace)
-			throws DbException;
 }

bramble-core/src/main/java/org/briarproject/bramble/settings/SettingsManagerImpl.java

@@ -37,10 +37,4 @@ class SettingsManagerImpl implements SettingsManager {
 	public void mergeSettings(Settings s, String namespace) throws DbException {
 		db.transaction(false, txn -> db.mergeSettings(txn, s, namespace));
 	}
-
-	@Override
-	public void mergeSettings(Transaction txn, Settings s, String namespace)
-			throws DbException {
-		db.mergeSettings(txn, s, namespace);
-	}
 }

briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java

@@ -1,5 +1,6 @@
 package org.briarproject.briar.android.hotspot;
 
+import android.app.Application;
 import android.content.Context;
 import android.graphics.Bitmap;
 import android.net.wifi.WifiManager;
@@ -11,7 +12,14 @@ import android.net.wifi.p2p.WifiP2pManager.GroupInfoListener;
 import android.os.Handler;
 import android.util.DisplayMetrics;
 
+import org.briarproject.bramble.api.db.DatabaseExecutor;
+import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
+import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
+import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
+import org.briarproject.bramble.api.settings.Settings;
+import org.briarproject.bramble.api.settings.SettingsManager;
+import org.briarproject.bramble.api.system.AndroidExecutor;
 import org.briarproject.briar.R;
 import org.briarproject.briar.android.hotspot.HotspotState.NetworkConfig;
 import org.briarproject.briar.android.util.QrCodeUtils;
@@ -20,9 +28,11 @@ import java.security.SecureRandom;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
+import javax.inject.Inject;
+
 import androidx.annotation.Nullable;
+import androidx.annotation.RequiresApi;
 import androidx.annotation.UiThread;
-import androidx.lifecycle.LiveData;
 
 import static android.content.Context.WIFI_P2P_SERVICE;
 import static android.content.Context.WIFI_SERVICE;
@@ -34,11 +44,12 @@ import static android.net.wifi.p2p.WifiP2pManager.ERROR;
 import static android.net.wifi.p2p.WifiP2pManager.NO_SERVICE_REQUESTS;
 import static android.net.wifi.p2p.WifiP2pManager.P2P_UNSUPPORTED;
 import static android.os.Build.VERSION.SDK_INT;
-import static java.util.Objects.requireNonNull;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Logger.getLogger;
-import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce;
+import static org.briarproject.briar.android.util.UiUtils.handleException;
 
+@MethodsNotNullByDefault
+@ParametersNotNullByDefault
 class HotspotManager implements ActionListener {
 
 	interface HotspotListener {
@@ -59,27 +70,42 @@ class HotspotManager implements ActionListener {
 
 	private static final int MAX_GROUP_INFO_ATTEMPTS = 5;
 	private static final int RETRY_DELAY_MILLIS = 1000;
+	private static final String HOTSPOT_NAMESPACE = "hotspot";
+	private static final String HOTSPOT_KEY_SSID = "ssid";
+	private static final String HOTSPOT_KEY_PASS = "pass";
 
 	private final Context ctx;
+	@DatabaseExecutor
+	private final Executor dbExecutor;
 	@IoExecutor
 	private final Executor ioExecutor;
-	private final LiveData<NetworkConfig> savedNetworkConfig;
+	private final AndroidExecutor androidExecutor;
-	private final HotspotListener listener;
+	private final SettingsManager settingsManager;
+	private final SecureRandom random;
 	private final WifiManager wifiManager;
 	private final WifiP2pManager wifiP2pManager;
 	private final Handler handler;
 	private final String lockTag;
 
+	private HotspotListener listener;
 	private WifiManager.WifiLock wifiLock;
 	private WifiP2pManager.Channel channel;
+	@RequiresApi(29)
+	private volatile NetworkConfig savedNetworkConfig;
 
-	HotspotManager(Context ctx, @IoExecutor Executor ioExecutor,
+	@Inject
-			LiveData<NetworkConfig> savedNetworkConfig,
+	HotspotManager(Application ctx,
-			HotspotListener listener) {
+			@DatabaseExecutor Executor dbExecutor,
-		this.ctx = ctx;
+			@IoExecutor Executor ioExecutor,
+			AndroidExecutor androidExecutor,
+			SettingsManager settingsManager,
+			SecureRandom random) {
+		this.ctx = ctx.getApplicationContext();
+		this.dbExecutor = dbExecutor;
 		this.ioExecutor = ioExecutor;
-		this.savedNetworkConfig = savedNetworkConfig;
+		this.androidExecutor = androidExecutor;
-		this.listener = listener;
+		this.settingsManager = settingsManager;
+		this.random = random;
 		wifiManager = (WifiManager) ctx.getApplicationContext()
 				.getSystemService(WIFI_SERVICE);
 		wifiP2pManager =
@@ -88,6 +114,10 @@ class HotspotManager implements ActionListener {
 		lockTag = ctx.getPackageName() + ":app-sharing-hotspot";
 	}
 
+	void setHotspotListener(HotspotListener listener) {
+		this.listener = listener;
+	}
+
 	@UiThread
 	void startWifiP2pHotspot() {
 		if (wifiP2pManager == null) {
@@ -102,18 +132,23 @@ class HotspotManager implements ActionListener {
 					ctx.getString(R.string.hotspot_error_no_wifi_direct));
 			return;
 		}
-		acquireLock();
 		try {
 			if (SDK_INT >= 29) {
-				observeForeverOnce(savedNetworkConfig, c -> {
+				dbExecutor.execute(() -> {
-					WifiP2pConfig config = new WifiP2pConfig.Builder()
+					// load savedNetworkConfig before starting hotspot
-							.setGroupOperatingBand(GROUP_OWNER_BAND_2GHZ)
+					loadSavedNetworkConfig();
-							.setNetworkName(c.ssid)
+					androidExecutor.runOnUiThread(() -> {
-							.setPassphrase(c.password)
+						WifiP2pConfig config = new WifiP2pConfig.Builder()
-							.build();
+								.setGroupOperatingBand(GROUP_OWNER_BAND_2GHZ)
-					wifiP2pManager.createGroup(channel, config, this);
+								.setNetworkName(savedNetworkConfig.ssid)
+								.setPassphrase(savedNetworkConfig.password)
+								.build();
+						acquireLock();
+						wifiP2pManager.createGroup(channel, config, this);
+					});
 				});
 			} else {
+				acquireLock();
 				wifiP2pManager.createGroup(channel, this);
 			}
 		} catch (SecurityException e) {
@@ -242,8 +277,7 @@ class HotspotManager implements ActionListener {
 			return false;
 		} else if (SDK_INT >= 29) {
 			// if we get here, the savedNetworkConfig must have a value
-			String networkName =
+			String networkName = savedNetworkConfig.ssid;
-					requireNonNull(savedNetworkConfig.getValue()).ssid;
 			if (!networkName.equals(group.getNetworkName())) {
 				if (LOG.isLoggable(INFO)) {
 					LOG.info("expected networkName: " + networkName);
@@ -291,37 +325,72 @@ class HotspotManager implements ActionListener {
 		}
 	}
 
+	/**
+	 * Store persistent Wi-Fi SSID and passphrase in Settings to improve UX
+	 * so that users don't have to change them when attempting to connect.
+	 * Works only on API 29 and above.
+	 */
+	@RequiresApi(29)
+	@DatabaseExecutor
+	private void loadSavedNetworkConfig() {
+		try {
+			Settings settings = settingsManager.getSettings(HOTSPOT_NAMESPACE);
+			String ssid = settings.get(HOTSPOT_KEY_SSID);
+			String pass = settings.get(HOTSPOT_KEY_PASS);
+			if (ssid == null || pass == null) {
+				ssid = getSsid();
+				pass = getPassword();
+				settings.put(HOTSPOT_KEY_SSID, ssid);
+				settings.put(HOTSPOT_KEY_PASS, pass);
+				settingsManager.mergeSettings(settings, HOTSPOT_NAMESPACE);
+			}
+			savedNetworkConfig = new NetworkConfig(ssid, pass, null);
+		} catch (DbException e) {
+			handleException(ctx, androidExecutor, LOG, e);
+			// probably never happens, but if lets use non-persistent data
+			String ssid = getSsid();
+			String pass = getPassword();
+			savedNetworkConfig = new NetworkConfig(ssid, pass, null);
+		}
+	}
+
+	@RequiresApi(29)
+	private String getSsid() {
+		return "DIRECT-" + getRandomString(2) + "-" +
+				getRandomString(10);
+	}
+
+	@RequiresApi(29)
+	private String getPassword() {
+		return getRandomString(8);
+	}
+
 	private static String createWifiLoginString(String ssid, String password) {
 		// https://en.wikipedia.org/wiki/QR_code#WiFi_network_login
 		// do not remove the dangling ';', it can cause problems to omit it
 		return "WIFI:S:" + ssid + ";T:WPA;P:" + password + ";;";
 	}
 
-	static String getSsid(SecureRandom random) {
-		return "DIRECT-" + getRandomString(random, 2) + "-" +
-				getRandomString(random, 10);
-	}
-
-	static String getPassword(SecureRandom random) {
-		return getRandomString(random, 8);
-	}
-
 	private static final String digits = "123456789"; // avoid 0
 	private static final String letters = "abcdefghijkmnopqrstuvwxyz"; // no l
 	private static final String LETTERS = "ABCDEFGHJKLMNPQRSTUVWXYZ"; // no I, O
 
-	private static String getRandomString(SecureRandom random, int length) {
+	private String getRandomString(int length) {
 		char[] c = new char[length];
 		for (int i = 0; i < length; i++) {
 			if (random.nextBoolean()) {
-				c[i] = digits.charAt(random.nextInt(digits.length()));
+				c[i] = random(digits);
 			} else if (random.nextBoolean()) {
-				c[i] = letters.charAt(random.nextInt(letters.length()));
+				c[i] = random(letters);
 			} else {
-				c[i] = LETTERS.charAt(random.nextInt(LETTERS.length()));
+				c[i] = random(LETTERS);
 			}
 		}
 		return new String(c);
 	}
 
+	private char random(String universe) {
+		return universe.charAt(random.nextInt(universe.length()));
+	}
+
 }

briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java

@@ -7,8 +7,6 @@ import org.briarproject.bramble.api.db.TransactionManager;
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.lifecycle.LifecycleManager;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
-import org.briarproject.bramble.api.settings.Settings;
-import org.briarproject.bramble.api.settings.SettingsManager;
 import org.briarproject.bramble.api.system.AndroidExecutor;
 import org.briarproject.briar.R;
 import org.briarproject.briar.android.hotspot.HotspotManager.HotspotListener;
@@ -23,19 +21,16 @@ import org.briarproject.briar.android.viewmodel.LiveEvent;
 import org.briarproject.briar.android.viewmodel.MutableLiveEvent;
 import org.briarproject.briar.api.android.AndroidNotificationManager;
 
-import java.security.SecureRandom;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
 import javax.inject.Inject;
 
 import androidx.annotation.Nullable;
-import androidx.annotation.RequiresApi;
 import androidx.annotation.UiThread;
 import androidx.lifecycle.LiveData;
 import androidx.lifecycle.MutableLiveData;
 
-import static android.os.Build.VERSION.SDK_INT;
 import static java.util.logging.Logger.getLogger;
 
 @NotNullByDefault
@@ -44,14 +39,9 @@ class HotspotViewModel extends DbViewModel
 
 	private static final Logger LOG =
 			getLogger(HotspotViewModel.class.getName());
-	private static final String HOTSPOT_NAMESPACE = "hotspot";
-	private static final String HOTSPOT_KEY_SSID = "ssid";
-	private static final String HOTSPOT_KEY_PASS = "pass";
 
 	@IoExecutor
 	private final Executor ioExecutor;
-	private final SettingsManager settingsManager;
-	private final SecureRandom random;
 	private final AndroidNotificationManager notificationManager;
 	private final HotspotManager hotspotManager;
 	private final WebServerManager webServerManager;
@@ -60,8 +50,6 @@ class HotspotViewModel extends DbViewModel
 			new MutableLiveData<>();
 	private final MutableLiveEvent<Boolean> peerConnected =
 			new MutableLiveEvent<>();
-	private final MutableLiveData<NetworkConfig> savedNetworkConfig =
-			new MutableLiveData<>();
 
 	@Nullable
 	// Field to temporarily store the network config received via onHotspotStarted()
@@ -75,48 +63,16 @@ class HotspotViewModel extends DbViewModel
 			TransactionManager db,
 			AndroidExecutor androidExecutor,
 			@IoExecutor Executor ioExecutor,
-			SettingsManager settingsManager,
+			HotspotManager hotspotManager,
-			SecureRandom secureRandom,
+			WebServerManager webServerManager,
 			AndroidNotificationManager notificationManager) {
 		super(app, dbExecutor, lifecycleManager, db, androidExecutor);
 		this.ioExecutor = ioExecutor;
-		this.settingsManager = settingsManager;
-		this.random = secureRandom;
 		this.notificationManager = notificationManager;
-		hotspotManager =
+		this.hotspotManager = hotspotManager;
-				new HotspotManager(app, ioExecutor, savedNetworkConfig, this);
+		this.hotspotManager.setHotspotListener(this);
-		webServerManager = new WebServerManager(app, this);
+		this.webServerManager = webServerManager;
-		// get or set persistent SSID and password
+		this.webServerManager.setListener(this);
-		if (SDK_INT >= 29) getOrSetNetworkConfig();
-	}
-
-	/**
-	 * Store persistent Wi-Fi SSID and passphrase in Settings to improve UX
-	 * so that users don't have to change them when attempting to connect.
-	 * Works only on API 29 and above.
-	 */
-	@RequiresApi(29)
-	private void getOrSetNetworkConfig() {
-		runOnDbThread(false, txn -> {
-			Settings settings =
-					settingsManager.getSettings(txn, HOTSPOT_NAMESPACE);
-			String ssid = settings.get(HOTSPOT_KEY_SSID);
-			String pass = settings.get(HOTSPOT_KEY_PASS);
-			if (ssid == null || pass == null) {
-				ssid = HotspotManager.getSsid(random);
-				pass = HotspotManager.getPassword(random);
-				settings.put(HOTSPOT_KEY_SSID, ssid);
-				settings.put(HOTSPOT_KEY_PASS, pass);
-				settingsManager.mergeSettings(txn, settings, HOTSPOT_NAMESPACE);
-			}
-			savedNetworkConfig.postValue(new NetworkConfig(ssid, pass, null));
-		}, error -> {
-			handleException(error);
-			// probably never happens, but if lets use non-persistent data
-			String ssid = HotspotManager.getSsid(random);
-			String pass = HotspotManager.getPassword(random);
-			savedNetworkConfig.postValue(new NetworkConfig(ssid, pass, null));
-		});
 	}
 
 	@UiThread

briar-android/src/main/java/org/briarproject/briar/android/hotspot/WebServer.java

@@ -2,6 +2,7 @@ package org.briarproject.briar.android.hotspot;
 
 import android.content.Context;
 
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.briar.BuildConfig;
 import org.briarproject.briar.R;
 import org.jsoup.Jsoup;
@@ -29,7 +30,8 @@ import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.briar.BuildConfig.VERSION_NAME;
 
-public class WebServer extends NanoHTTPD {
+@NotNullByDefault
+class WebServer extends NanoHTTPD {
 
 	final static int PORT = 9999;
 
@@ -98,7 +100,7 @@ public class WebServer extends NanoHTTPD {
 		return doc.outerHtml();
 	}
 
-	private String getUnknownSourcesString(String userAgent) {
+	private String getUnknownSourcesString(@Nullable String userAgent) {
 		boolean is8OrHigher = false;
 		if (userAgent != null) {
 			Matcher matcher = REGEX_AGENT.matcher(userAgent);

briar-android/src/main/java/org/briarproject/briar/android/hotspot/WebServerManager.java

@@ -1,10 +1,12 @@
 package org.briarproject.briar.android.hotspot;
 
-import android.content.Context;
+import android.app.Application;
 import android.graphics.Bitmap;
 import android.util.DisplayMetrics;
 
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
+import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
+import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.briar.android.hotspot.HotspotState.WebsiteConfig;
 import org.briarproject.briar.android.util.QrCodeUtils;
 
@@ -17,6 +19,8 @@ import java.util.Enumeration;
 import java.util.List;
 import java.util.logging.Logger;
 
+import javax.inject.Inject;
+
 import androidx.annotation.Nullable;
 
 import static java.util.Collections.emptyList;
@@ -27,6 +31,8 @@ import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.briar.android.hotspot.WebServer.PORT;
 
+@MethodsNotNullByDefault
+@ParametersNotNullByDefault
 class WebServerManager {
 
 	interface WebServerListener {
@@ -41,15 +47,20 @@ class WebServerManager {
 			getLogger(WebServerManager.class.getName());
 
 	private final WebServer webServer;
-	private final WebServerListener listener;
 	private final DisplayMetrics dm;
 
-	WebServerManager(Context ctx, WebServerListener listener) {
+	private WebServerListener listener;
-		this.listener = listener;
+
+	@Inject
+	WebServerManager(Application ctx) {
 		webServer = new WebServer(ctx);
 		dm = ctx.getResources().getDisplayMetrics();
 	}
 
+	void setListener(WebServerListener listener) {
+		this.listener = listener;
+	}
+
 	@IoExecutor
 	void startWebServer() {
 		try {