Cleanup misc TODO's that are floating around (swap and general f-droid)
`SwapService`: Remove unneccesary TODO's and unused constant. Also added other TODO's there which will need to be done before swap is stable. Can make them into issues in gitlab, but will leave there for now. `SwapWorkflowActivity`: Clarified TODO's, fixed issue with cancelling swap when in the "connecting" state (before it would not show the right view again, now it goes back to the first view). `WifiQrView`: Make scanning of QR code initiated by `SwapActivity`. This is because the activity is actually the one who is able to listen for a response anyway. `BluetoothFinder`: Cleaned up TODO's in but didn't actually address them. `BluetoothPeer`: Removed TODO about fingerprints, because TOFO is probably the way to go with Bluetooth anyhow. `BonjourFinder`: Remove dead code, prevent NPE by using final local variable rather than member variable (the member variable gets set to null in a different thread). `BonjourPeer`: Fix bug with prompting other device to swap back. `BluetoothDownloader`: Fix typo in comment. `Downloader`: Removed TODO that was left over from a refactor, and was never actually correct. The code after refactoring was broken because it created an `InputStream` which was never closed. Now it doesn't do that, because it was fixed in an earlier commit in this branch. `HttpDownloader`: Remove TODO, but clarify that the downloader does not follow 30x redirects.
This commit is contained in:
Peter Serwylo
parent
9180c9f5c0
commit
4d11c97d51
@ -60,6 +60,13 @@ import java.util.TimerTask;
|
||||
* Central service which manages all of the different moving parts of swap which are required
|
||||
* to enable p2p swapping of apps. Currently manages WiFi and NFC. Will manage Bluetooth in
|
||||
* the future.
|
||||
*
|
||||
* TODO: Show "Waiting for other device to finish setting up swap" when only F-Droid shown in swap
|
||||
* TODO: Handle not connected to wifi more gracefully. For example, Bonjour discovery falls over.
|
||||
* TODO: Remove peers from list of peers when no longer "visible".
|
||||
* TODO: Show feedback for "Setting up (wifi|bluetooth)" in start swap view.
|
||||
* TODO: Turn off bluetooth after cancelling/timing out if we turned it on.
|
||||
*
|
||||
*/
|
||||
public class SwapService extends Service {
|
||||
|
||||
@ -116,8 +123,6 @@ public class SwapService extends Service {
|
||||
public static final int STEP_SUCCESS = 7;
|
||||
public static final int STEP_CONFIRM_SWAP = 8;
|
||||
|
||||
public static final int STEP_BLUETOOTH = 1000; // TODO: Remove this once nathans code is merged and the UI is migrated to the nearby peers screen.
|
||||
|
||||
/**
|
||||
* Special view, that we don't really want to actually store against the
|
||||
* {@link SwapService#step}. Rather, we use it for the purpose of specifying
|
||||
@ -232,11 +237,13 @@ public class SwapService extends Service {
|
||||
if (repo == null) {
|
||||
ContentValues values = new ContentValues(6);
|
||||
|
||||
// TODO: i18n and think about most appropriate name. Although it wont be visible in
|
||||
// the "Manage repos" UI after being marked as a swap repo here...
|
||||
// The name/description is not really required, as swap repos are not shown in the
|
||||
// "Manage repos" UI on other device. Doesn't hurt to put something there though,
|
||||
// on the off chance that somebody is looking through the sqlite database which
|
||||
// contains the repos...
|
||||
values.put(RepoProvider.DataColumns.NAME, peer.getName());
|
||||
values.put(RepoProvider.DataColumns.ADDRESS, peer.getRepoAddress());
|
||||
values.put(RepoProvider.DataColumns.DESCRIPTION, ""); // TODO;
|
||||
values.put(RepoProvider.DataColumns.DESCRIPTION, "");
|
||||
values.put(RepoProvider.DataColumns.FINGERPRINT, peer.getFingerprint());
|
||||
values.put(RepoProvider.DataColumns.IN_USE, true);
|
||||
values.put(RepoProvider.DataColumns.IS_SWAP, true);
|
||||
@ -263,7 +270,7 @@ public class SwapService extends Service {
|
||||
* This is the same as, e.g. {@link Context#getSystemService(String)}
|
||||
*/
|
||||
@IntDef({STEP_INTRO, STEP_SELECT_APPS, STEP_JOIN_WIFI, STEP_SHOW_NFC, STEP_WIFI_QR,
|
||||
STEP_CONNECTING, STEP_SUCCESS, STEP_CONFIRM_SWAP, STEP_INITIAL_LOADING, STEP_BLUETOOTH})
|
||||
STEP_CONNECTING, STEP_SUCCESS, STEP_CONFIRM_SWAP, STEP_INITIAL_LOADING})
|
||||
@Retention(RetentionPolicy.SOURCE)
|
||||
public @interface SwapStep {}
|
||||
|
||||
@ -378,12 +385,7 @@ public class SwapService extends Service {
|
||||
// ==========================================
|
||||
|
||||
/**
|
||||
* Ensures that the webserver is running, as are the other services which make swap work.
|
||||
* Will only do all this if it is not already running, and will run on a background thread.'
|
||||
* TODO: What about an "enabling" status? Not sure if it will be useful or not.
|
||||
*
|
||||
*
|
||||
* TODO: Call this at the relevant time, when wifi or bluetooth is enabled.
|
||||
* Moves the service to the forground and [re]starts the timeout timer.
|
||||
*/
|
||||
private void attachService() {
|
||||
Log.d(TAG, "Moving SwapService to foreground so that it hangs around even when F-Droid is closed.");
|
||||
@ -562,6 +564,10 @@ public class SwapService extends Service {
|
||||
Log.i(TAG, "Asked to stop swapping, will stop bluetooth, wifi, and move service to BG for GC.");
|
||||
getBluetoothSwap().stopInBackground();
|
||||
getWifiSwap().stopInBackground();
|
||||
|
||||
// Ensure the user is sent back go the first screen when returning if we have just forceably
|
||||
// cancelled all swapping.
|
||||
setStep(STEP_INTRO);
|
||||
detachService();
|
||||
}
|
||||
|
||||
@ -589,12 +595,11 @@ public class SwapService extends Service {
|
||||
|
||||
private void initTimer() {
|
||||
if (timer != null) {
|
||||
Log.d(TAG, "Cancelling existing timer");
|
||||
Log.d(TAG, "Cancelling existing timeout timer so timeout can be reset.");
|
||||
timer.cancel();
|
||||
}
|
||||
|
||||
// automatically turn off after 15 minutes
|
||||
Log.d(TAG, "Initializing timer to 15 minutes");
|
||||
Log.d(TAG, "Initializing swap timeout to " + TIMEOUT + "ms minutes");
|
||||
timer = new Timer();
|
||||
timer.schedule(new TimerTask() {
|
||||
@Override
|
||||
|
||||
@ -57,6 +57,8 @@ public class BluetoothFinder extends PeerFinder<BluetoothPeer> {
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
// TODO: Unregister this receiver at the appropriate time.
|
||||
context.registerReceiver(scanCompleteReceiver, new IntentFilter(BluetoothAdapter.ACTION_DISCOVERY_FINISHED));
|
||||
}
|
||||
|
||||
@ -66,8 +68,9 @@ public class BluetoothFinder extends PeerFinder<BluetoothPeer> {
|
||||
private void startDiscovery() {
|
||||
|
||||
if (adapter.isDiscovering()) {
|
||||
// TODO: Can we reset the discovering timeout, so that it doesn't, e.g. time out
|
||||
// in 3 seconds because we had already almost completed the previous scan?
|
||||
// TODO: Can we reset the discovering timeout, so that it doesn't, e.g. time out in 3
|
||||
// seconds because we had already almost completed the previous scan? We could
|
||||
// cancelDiscovery(), but then it will probably prompt the user again.
|
||||
Log.d(TAG, "Requested bluetooth scan when already scanning, so will ignore request.");
|
||||
return;
|
||||
}
|
||||
|
||||
@ -39,9 +39,13 @@ public class BluetoothPeer implements Peer {
|
||||
return "bluetooth://" + device.getAddress().replace(':', '-') + "/fdroid/repo";
|
||||
}
|
||||
|
||||
/**
|
||||
* Bluetooth will exclusively be TOFU. Once a device is connected to a bluetooth socket,
|
||||
* if we trust it enough to accept a fingerprint from it somehow, then we may as well trust it
|
||||
* enough to receive an index from it that contains a fingerprint we can use.
|
||||
*/
|
||||
@Override
|
||||
public String getFingerprint() {
|
||||
// TODO: Get fingerprint somehow over bluetooth.
|
||||
return "";
|
||||
}
|
||||
|
||||
|
||||
@ -82,6 +82,8 @@ public class BonjourFinder extends PeerFinder<BonjourPeer> implements ServiceLis
|
||||
|
||||
private void listServices() {
|
||||
|
||||
// The member variable is likely to get set to null if a swap process starts, thus we hold
|
||||
// a reference for the benefit of the background task so it doesn't have to synchronoize on it.
|
||||
final JmDNS mdns = jmdns;
|
||||
|
||||
new AsyncTask<Void, Void, Void>() {
|
||||
@ -93,20 +95,6 @@ public class BonjourFinder extends PeerFinder<BonjourPeer> implements ServiceLis
|
||||
addFDroidServices(mdns.list(HTTPS_SERVICE_TYPE));
|
||||
return null;
|
||||
}
|
||||
|
||||
// TODO: Remove once stable, added here for testing because it is easier to see the
|
||||
// data being broadcast over mDNS.
|
||||
/*@Override
|
||||
protected void onPostExecute(Void v) {
|
||||
Log.d(TAG, "Queuing up another poll in 2 secs.");
|
||||
new Timer().schedule(new TimerTask() {
|
||||
@Override
|
||||
public void run() {
|
||||
Log.d(TAG, "Time to poll for services again.");
|
||||
listServices();
|
||||
}
|
||||
}, 2000);
|
||||
}*/
|
||||
}.execute();
|
||||
}
|
||||
|
||||
@ -125,10 +113,14 @@ public class BonjourFinder extends PeerFinder<BonjourPeer> implements ServiceLis
|
||||
//
|
||||
// If so, when is the old one removed?
|
||||
addFDroidService(event.getInfo());
|
||||
|
||||
// The member variable is likely to get set to null if a swap process starts, thus we hold
|
||||
// a reference for the benefit of the background task so it doesn't have to synchronoize on it.
|
||||
final JmDNS mdns = jmdns;
|
||||
new AsyncTask<Void, Void, Void>() {
|
||||
@Override
|
||||
protected Void doInBackground(Void... params) {
|
||||
jmdns.requestServiceInfo(event.getType(), event.getName(), true);
|
||||
mdns.requestServiceInfo(event.getType(), event.getName(), true);
|
||||
return null;
|
||||
}
|
||||
}.execute();
|
||||
|
||||
@ -14,6 +14,7 @@ public class BonjourPeer extends WifiPeer {
|
||||
this.serviceInfo = new FDroidServiceInfo(serviceInfo);
|
||||
this.name = serviceInfo.getDomain();
|
||||
this.uri = Uri.parse(this.serviceInfo.getRepoAddress());
|
||||
this.shouldPromptForSwapBack = true;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -56,7 +57,7 @@ public class BonjourPeer extends WifiPeer {
|
||||
}
|
||||
|
||||
protected BonjourPeer(Parcel in) {
|
||||
this.serviceInfo = in.readParcelable(FDroidServiceInfo.class.getClassLoader());
|
||||
this((ServiceInfo)in.readParcelable(FDroidServiceInfo.class.getClassLoader()));
|
||||
}
|
||||
|
||||
public static final Creator<BonjourPeer> CREATOR = new Creator<BonjourPeer>() {
|
||||
|
||||
@ -56,7 +56,7 @@ public class BluetoothDownloader extends Downloader {
|
||||
|
||||
/**
|
||||
* May return null if an error occurred while getting file details.
|
||||
* TODO: Should we throw an exception? Everywhere else in this blue package throws IO exceptions weelx`x`xy-neely.
|
||||
* TODO: Should we throw an exception? Everywhere else in this blue package throws IO exceptions weely-neely.
|
||||
* Will probably require some thought as to how the API looks, with regards to all of the public methods
|
||||
* and their signatures.
|
||||
*/
|
||||
|
||||
@ -127,7 +127,6 @@ public abstract class Downloader {
|
||||
// we were interrupted before proceeding to the download.
|
||||
throwExceptionIfInterrupted();
|
||||
|
||||
// TODO: Check side effects of changing this second getInputStream() to input.
|
||||
copyInputToOutputStream(input);
|
||||
} finally {
|
||||
Utils.closeQuietly(outputStream);
|
||||
|
||||
@ -2,6 +2,9 @@ package org.fdroid.fdroid.net;
|
||||
|
||||
import android.content.Context;
|
||||
import android.util.Log;
|
||||
|
||||
import com.nostra13.universalimageloader.core.download.BaseImageDownloader;
|
||||
|
||||
import org.fdroid.fdroid.Preferences;
|
||||
|
||||
import javax.net.ssl.SSLHandshakeException;
|
||||
@ -54,10 +57,17 @@ public class HttpDownloader extends Downloader {
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Note: Doesn't follow redirects (as far as I'm aware).
|
||||
* {@link BaseImageDownloader#getStreamFromNetwork(String, Object)} has an implementation worth
|
||||
* checking out that follows redirects up to a certain point. I guess though the correct way
|
||||
* is probably to check for a loop (keep a list of all URLs redirected to and if you hit the
|
||||
* same one twice, bail with an exception).
|
||||
* @throws IOException
|
||||
*/
|
||||
@Override
|
||||
public InputStream getInputStream() throws IOException {
|
||||
setupConnection();
|
||||
// TODO check out BaseImageDownloader.getStreamFromNetwork() for optims
|
||||
return connection.getInputStream();
|
||||
}
|
||||
|
||||
|
||||
@ -9,8 +9,10 @@ public class BluetoothConstants {
|
||||
|
||||
public static UUID fdroidUuid() {
|
||||
// TODO: Generate a UUID deterministically from, e.g. "org.fdroid.fdroid.net.Bluetooth";
|
||||
// This UUID is just from the first example at http://www.ietf.org/rfc/rfc4122.txt
|
||||
return UUID.fromString("f81d4fae-7dec-11d0-a765-00a0c91e6bf6");
|
||||
// This can be an offline process, as long as it can be reproduced by other people who
|
||||
// want to do so.
|
||||
// This UUID is just from mashing random hex characters on the keyboard.
|
||||
return UUID.fromString("cd59ba31-5729-b3bb-cb29-732b59eu6gaa");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@ -150,7 +150,7 @@ public class BluetoothServer extends Thread {
|
||||
if (request.getMethod().equals(Request.Methods.HEAD)) {
|
||||
builder = new Response.Builder();
|
||||
} else {
|
||||
HashMap<String, String> headers = new HashMap<String, String>();
|
||||
HashMap<String, String> headers = new HashMap<>();
|
||||
Response resp = respond(headers, "/" + request.getPath());
|
||||
|
||||
builder = new Response.Builder(resp.toContentStream());
|
||||
|
||||
@ -119,7 +119,7 @@ public class Request {
|
||||
}
|
||||
|
||||
/**
|
||||
* First line of a HTTP response is the status line:
|
||||
* First line of a HTTP 1.1 response is the status line:
|
||||
* http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1
|
||||
* The first part is the HTTP version, followed by a space, then the status code, then
|
||||
* a space, and then the status label (which may contain spaces).
|
||||
|
||||
@ -64,7 +64,11 @@ public class Response {
|
||||
if (headers != null) {
|
||||
for (Map.Entry<String, String> entry : headers.entrySet()) {
|
||||
if (entry.getKey().toLowerCase().equals("content-length")) {
|
||||
return Integer.parseInt( entry.getValue()); // TODO: error handling.
|
||||
try {
|
||||
return Integer.parseInt(entry.getValue());
|
||||
} catch (NumberFormatException e) {
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -398,20 +398,39 @@ public class SwapWorkflowActivity extends AppCompatActivity {
|
||||
}
|
||||
|
||||
/**
|
||||
* When swapping with a peer that is identified by a NewRepoConfig, that means that they
|
||||
* came from a QR Code scan. In this situation, we should already have a full swap repo
|
||||
* ready to go.
|
||||
* TODO: What if we scanned the repo but do not have a repo running yet?
|
||||
* This is for when we initiate a swap by viewing the "Are you sure you want to swap with" view
|
||||
* This can arise either:
|
||||
* * As a result of scanning a QR code (in which case we likely already have a repo setup) or
|
||||
* * As a result of the other device selecting our device in the "start swap" screen, in which
|
||||
* case we are likely just sitting on the start swap screen also, and haven't configured
|
||||
* anything yet.
|
||||
*/
|
||||
public void swapWith(NewRepoConfig repoConfig) {
|
||||
getService().swapWith(repoConfig.toPeer());
|
||||
startSwappingWithPeer();
|
||||
Peer peer = repoConfig.toPeer();
|
||||
if (getService().getStep() == SwapService.STEP_INTRO || getService().getStep() == SwapService.STEP_CONFIRM_SWAP) {
|
||||
// This will force the "Select apps to swap" workflow to begin.
|
||||
// TODO: Find a better way to decide whether we need to select the apps. Not sure if we
|
||||
// can or cannot be in STEP_INTRO with a full blown repo ready to swap.
|
||||
swapWith(peer);
|
||||
} else {
|
||||
getService().swapWith(repoConfig.toPeer());
|
||||
startSwappingWithPeer();
|
||||
}
|
||||
}
|
||||
|
||||
public void denySwap() {
|
||||
showIntro();
|
||||
}
|
||||
|
||||
/**
|
||||
* Attempts to open a QR code scanner, in the hope a user will then scan the QR code of another
|
||||
* device configured to swapp apps with us. Delegates to the zxing library to do so.
|
||||
*/
|
||||
public void initiateQrScan() {
|
||||
IntentIntegrator integrator = new IntentIntegrator(this);
|
||||
integrator.initiateScan();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onActivityResult(int requestCode, int resultCode, Intent intent) {
|
||||
IntentResult scanResult = IntentIntegrator.parseActivityResult(requestCode, resultCode, intent);
|
||||
@ -642,10 +661,10 @@ public class SwapWorkflowActivity extends AppCompatActivity {
|
||||
message = "No swap service";
|
||||
} else {
|
||||
{
|
||||
String bluetooth = service.getBluetoothSwap().isConnected() ? "Yes" : " No";
|
||||
String wifi = service.getWifiSwap().isConnected() ? "Yes" : " No";
|
||||
String mdns = service.getWifiSwap().getBonjour().isConnected() ? "Yes" : " No";
|
||||
message += "Broadcast { BT: " + bluetooth + ", WiFi: " + wifi + ", mDNS: " + mdns + "}, ";
|
||||
String bluetooth = service.getBluetoothSwap().isConnected() ? "Y" : " N";
|
||||
String wifi = service.getWifiSwap().isConnected() ? "Y" : " N";
|
||||
String mdns = service.getWifiSwap().getBonjour().isConnected() ? "Y" : " N";
|
||||
message += "Swap { BT: " + bluetooth + ", WiFi: " + wifi + ", mDNS: " + mdns + "}, ";
|
||||
}
|
||||
|
||||
{
|
||||
@ -653,18 +672,19 @@ public class SwapWorkflowActivity extends AppCompatActivity {
|
||||
String bluetooth = "N/A";
|
||||
if (adapter != null) {
|
||||
Map<Integer, String> scanModes = new HashMap<>(3);
|
||||
scanModes.put(BluetoothAdapter.SCAN_MODE_CONNECTABLE, "CONNECTABLE");
|
||||
scanModes.put(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE, "CONNECTABLE_DISCOVERABLE");
|
||||
scanModes.put(BluetoothAdapter.SCAN_MODE_CONNECTABLE, "CON");
|
||||
scanModes.put(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE, "CON_DISC");
|
||||
scanModes.put(BluetoothAdapter.SCAN_MODE_NONE, "NONE");
|
||||
bluetooth = "\"" + adapter.getName() + "\" - " + scanModes.get(adapter.getScanMode());
|
||||
}
|
||||
|
||||
String wifi = service.getBonjourFinder().isScanning() ? "Yes" : " No";
|
||||
message += "Discover { BT: " + bluetooth + ", WiFi: " + wifi + "}";
|
||||
String wifi = service.getBonjourFinder().isScanning() ? "Y" : " N";
|
||||
message += "Find { BT: " + bluetooth + ", WiFi: " + wifi + "}";
|
||||
}
|
||||
}
|
||||
|
||||
Log.d("Swap Status", new Date().toLocaleString() + " " + message);
|
||||
Date now = new Date();
|
||||
Log.d("Swap Status", now.getHours() + ":" + now.getMinutes() + ":" + now.getSeconds() + " " + message);
|
||||
|
||||
new Timer().schedule(new TimerTask() {
|
||||
@Override
|
||||
|
||||
@ -77,17 +77,11 @@ public class WifiQrView extends ScrollView implements SwapWorkflowActivity.Inner
|
||||
openQr.setOnClickListener(new Button.OnClickListener() {
|
||||
@Override
|
||||
public void onClick(View v) {
|
||||
// TODO: Should probably ask the activity or some other class to do this for us.
|
||||
// The view should be dumb and only know how to show things and delegate things to
|
||||
// other classes that know how to do things.
|
||||
IntentIntegrator integrator = new IntentIntegrator(getActivity());
|
||||
integrator.initiateScan();
|
||||
getActivity().initiateQrScan();
|
||||
}
|
||||
});
|
||||
|
||||
// TODO: As with the JoinWifiView, this should be refactored to be part of the SwapState.
|
||||
// Otherwise, we are left with SwapState, LocalRepoService, WifiStateChangeService, and
|
||||
// some static variables in FDroidApp all which manage the state for swap.
|
||||
// TODO: Unregister this receiver properly.
|
||||
LocalBroadcastManager.getInstance(getActivity()).registerReceiver(
|
||||
new BroadcastReceiver() {
|
||||
@Override
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user