Bug fixing and refactoring:

1) Make sure no one but service changes recordingTrackId (which is owned by it)
2) Cache sharedPreferences
3) Clear recordingTrackId if it doesn't correspond to a valid track or the service is not recording
4) More unit tests
This commit is contained in:
Bartlomiej Niechwiej
2010-10-28 22:49:01 -07:00
parent dc8ee5943c
commit a0f090a83a
4 changed files with 49 additions and 69 deletions
@@ -97,9 +97,10 @@ public class MyTracks extends TabActivity implements OnTouchListener,
/** /**
* Singleton instance * Singleton instance
*/ */
private static MyTracks instance = null; private static MyTracks instance;
private ChartActivity chartActivity = null;
private ChartActivity chartActivity;
public ChartActivity getChartActivity() { public ChartActivity getChartActivity() {
return chartActivity; return chartActivity;
} }
@@ -178,6 +179,8 @@ public class MyTracks extends TabActivity implements OnTouchListener,
* Utilities to deal with the database. * Utilities to deal with the database.
*/ */
private MyTracksProviderUtils providerUtils; private MyTracksProviderUtils providerUtils;
private SharedPreferences sharedPreferences;
/** /**
* The connection to the track recording service. * The connection to the track recording service.
@@ -275,6 +278,7 @@ public class MyTracks extends TabActivity implements OnTouchListener,
instance = this; instance = this;
providerUtils = MyTracksProviderUtils.Factory.get(this); providerUtils = MyTracksProviderUtils.Factory.get(this);
menuManager = new MenuManager(this); menuManager = new MenuManager(this);
sharedPreferences = getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
// The volume we want to control is the Text-To-Speech volume // The volume we want to control is the Text-To-Speech volume
int volumeStream = int volumeStream =
@@ -310,14 +314,12 @@ public class MyTracks extends TabActivity implements OnTouchListener,
tabHost.addView(layout); tabHost.addView(layout);
layout.setOnTouchListener(this); layout.setOnTouchListener(this);
SharedPreferences prefs = if (sharedPreferences != null) {
getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
if (prefs != null) {
selectedTrackId = selectedTrackId =
prefs.getLong(getString(R.string.selected_track_key), -1); sharedPreferences.getLong(getString(R.string.selected_track_key), -1);
recordingTrackId = recordingTrackId = sharedPreferences.getLong(
prefs.getLong(getString(R.string.recording_track_key), -1); getString(R.string.recording_track_key), -1);
prefs.registerOnSharedPreferenceChangeListener(this); sharedPreferences.registerOnSharedPreferenceChangeListener(this);
Log.d(MyTracksConstants.TAG, "recordingTrackId: " + recordingTrackId Log.d(MyTracksConstants.TAG, "recordingTrackId: " + recordingTrackId
+ ", selectedTrackId: " + selectedTrackId); + ", selectedTrackId: " + selectedTrackId);
if (recordingTrackId > 0) { if (recordingTrackId > 0) {
@@ -1029,20 +1031,15 @@ public class MyTracks extends TabActivity implements OnTouchListener,
shareIntent.putExtra(Intent.EXTRA_SUBJECT, shareIntent.putExtra(Intent.EXTRA_SUBJECT,
getResources().getText(R.string.share_map_subject).toString()); getResources().getText(R.string.share_map_subject).toString());
SharedPreferences prefs =
getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
boolean shareUrlOnly = true; boolean shareUrlOnly = true;
if (prefs != null) { if (sharedPreferences != null) {
shareUrlOnly = shareUrlOnly = sharedPreferences.getBoolean(
prefs.getBoolean(getString(R.string.share_url_only_key), false); getString(R.string.share_url_only_key), false);
} }
String url = MyMapsConstants.MAPSHOP_BASE_URL + "?msa=0&msid=" + mapId; String url = MyMapsConstants.MAPSHOP_BASE_URL + "?msa=0&msid=" + mapId;
String msg = (shareUrlOnly String msg = shareUrlOnly ? url : String.format(
? url getResources().getText(R.string.share_map_body_format).toString(), url);
: String.format(
getResources().getText(R.string.share_map_body_format).toString(),
url));
shareIntent.putExtra(Intent.EXTRA_TEXT, msg); shareIntent.putExtra(Intent.EXTRA_TEXT, msg);
startActivity(Intent.createChooser(shareIntent, startActivity(Intent.createChooser(shareIntent,
getResources().getText(R.string.share_map).toString())); getResources().getText(R.string.share_map).toString()));
@@ -1256,9 +1253,10 @@ public class MyTracks extends TabActivity implements OnTouchListener,
private void startRecordingNewTrack() { private void startRecordingNewTrack() {
try { try {
recordingTrackId = trackRecordingService.startNewTrack(); recordingTrackId = trackRecordingService.startNewTrack();
// Select the recording track.
setSelectedTrack(recordingTrackId);
Toast.makeText(this, getString(R.string.status_now_recording), Toast.makeText(this, getString(R.string.status_now_recording),
Toast.LENGTH_SHORT).show(); Toast.LENGTH_SHORT).show();
setSelectedAndRecordingTrack(recordingTrackId, recordingTrackId);
} catch (RemoteException e) { } catch (RemoteException e) {
Toast.makeText(this, Toast.makeText(this,
getString(R.string.error_unable_to_start_recording), getString(R.string.error_unable_to_start_recording),
@@ -1293,7 +1291,6 @@ public class MyTracks extends TabActivity implements OnTouchListener,
} catch (RemoteException e) { } catch (RemoteException e) {
Log.e(MyTracksConstants.TAG, "Unable to stop recording.", e); Log.e(MyTracksConstants.TAG, "Unable to stop recording.", e);
} }
setRecordingTrack(-1);
Intent intent = new Intent(MyTracks.this, MyTracksDetails.class); Intent intent = new Intent(MyTracks.this, MyTracksDetails.class);
intent.putExtra("trackid", recordingTrackId); intent.putExtra("trackid", recordingTrackId);
intent.putExtra("hasCancelButton", false); intent.putExtra("hasCancelButton", false);
@@ -1348,9 +1345,7 @@ public class MyTracks extends TabActivity implements OnTouchListener,
public void setSelectedTrack(final long trackId) { public void setSelectedTrack(final long trackId) {
runOnUiThread(new Runnable() { runOnUiThread(new Runnable() {
public void run() { public void run() {
SharedPreferences prefs = SharedPreferences.Editor editor = sharedPreferences.edit();
getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
SharedPreferences.Editor editor = prefs.edit();
editor.putLong(getString(R.string.selected_track_key), trackId); editor.putLong(getString(R.string.selected_track_key), trackId);
editor.commit(); editor.commit();
} }
@@ -1361,44 +1356,6 @@ public class MyTracks extends TabActivity implements OnTouchListener,
return selectedTrackId; return selectedTrackId;
} }
/**
* Writes the recording track id to the shared preferences.
* Executed on the UI thread.
*
* @param trackId the id of the track
*/
private void setRecordingTrack(final long trackId) {
runOnUiThread(new Runnable() {
public void run() {
SharedPreferences prefs =
getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
SharedPreferences.Editor editor = prefs.edit();
editor.putLong(getString(R.string.recording_track_key), trackId);
editor.commit();
}
});
}
/**
* Writes the selected and the recording track id to the shared preferences.
* Executed on UI thread.
*/
private void setSelectedAndRecordingTrack(final long theSelectedTrackId,
final long theRecordingTrackId) {
runOnUiThread(new Runnable() {
public void run() {
SharedPreferences prefs =
getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
if (prefs != null) {
SharedPreferences.Editor editor = prefs.edit();
editor.putLong(getString(R.string.selected_track_key), theSelectedTrackId);
editor.putLong(getString(R.string.recording_track_key), theRecordingTrackId);
editor.commit();
}
}
});
}
/** /**
* Binds to track recording service if it is running. * Binds to track recording service if it is running.
*/ */
@@ -702,7 +702,7 @@ public class MyTracksMap extends MapActivity
*/ */
public void setSelectedTrack(final long trackId) { public void setSelectedTrack(final long trackId) {
Log.d(MyTracksConstants.TAG, "MyTracksMap.setSelectedTrack: " Log.d(MyTracksConstants.TAG, "MyTracksMap.setSelectedTrack: "
+ "id = " + trackId + ", selectedTrackId = " + selectedTrackId); + selectedTrackId + " -> " + trackId);
if (selectedTrackId == trackId) { if (selectedTrackId == trackId) {
// Selected track did not change, nothing to do. // Selected track did not change, nothing to do.
@@ -1000,15 +1000,18 @@ public class MyTracksMap extends MapActivity
if (key == null || key.equals(getString(R.string.recording_track_key))) { if (key == null || key.equals(getString(R.string.recording_track_key))) {
recordingTrackId = sharedPreferences.getLong( recordingTrackId = sharedPreferences.getLong(
getString(R.string.recording_track_key), -1); getString(R.string.recording_track_key), -1);
if (isATrackSelected()) {
mapOverlay.setShowEndMarker(!isRecordingSelected());
mapView.postInvalidate();
}
} }
if (key == null || key.equals(getString(R.string.selected_track_key))) { if (key == null || key.equals(getString(R.string.selected_track_key))) {
setSelectedTrack(sharedPreferences.getLong( setSelectedTrack(sharedPreferences.getLong(
getString(R.string.selected_track_key), -1)); getString(R.string.selected_track_key), -1));
} }
// Show end marker if the track has been selected and is no recording.
// Note: This check must be *after* a call to setSelectedTrack(...) above.
if (isATrackSelected()) {
mapOverlay.setShowEndMarker(!isRecordingSelected());
mapView.postInvalidate();
}
} }
private void readAllNewTrackPoints() { private void readAllNewTrackPoints() {
@@ -662,6 +662,11 @@ public class TrackRecordingService extends Service implements LocationListener {
if (recordingTrack != null) { if (recordingTrack != null) {
restoreStats(recordingTrack); restoreStats(recordingTrack);
isRecording = true; isRecording = true;
} else {
// Make sure we have consistent state in shared preferences.
Log.w(MyTracksConstants.TAG, "TrackRecordingService.onCreate: Resetting "
+ "an orphaned recording track: " + recordingTrackId);
prefManager.setRecordingTrack(recordingTrackId = -1);
} }
showNotification(); showNotification();
} }
@@ -257,6 +257,18 @@ public class TrackRecordingServiceTest
assertEquals(-1, service.getRecordingTrackId()); assertEquals(-1, service.getRecordingTrackId());
} }
@MediumTest
public void testRecording_orphanedRecordingTrack() throws Exception {
// Just set recording track to a bogus value.
setRecordingTrack(256);
// Make sure that the service will not start recording and will clear
// the bogus track.
ITrackRecordingService service = bindAndGetService(createStartIntent());
assertFalse(service.isRecording());
assertEquals(-1, service.getRecordingTrackId());
}
@MediumTest @MediumTest
public void testStartNewTrack_noRecording() throws Exception { public void testStartNewTrack_noRecording() throws Exception {
List<Track> tracks = providerUtils.getAllTracks(); List<Track> tracks = providerUtils.getAllTracks();
@@ -538,9 +550,12 @@ public class TrackRecordingServiceTest
assertTrue(track.getId() >= 0); assertTrue(track.getId() >= 0);
providerUtils.insertTrack(track); providerUtils.insertTrack(track);
assertEquals(track.getId(), providerUtils.getTrack(track.getId()).getId()); assertEquals(track.getId(), providerUtils.getTrack(track.getId()).getId());
setRecordingTrack(isRecording ? track.getId() : -1);
}
private void setRecordingTrack(long id) {
Editor editor = sharedPreferences.edit(); Editor editor = sharedPreferences.edit();
editor.putLong(context.getString(R.string.recording_track_key), editor.putLong(context.getString(R.string.recording_track_key), id);
isRecording ? track.getId() : -1);
editor.commit(); editor.commit();
} }
} }