From 751683dd97a33893a510250c07382163997ee09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Guti=C3=A9rrez=20Alfaro?= Date: Mon, 19 Jan 2026 19:40:08 -0600 Subject: [PATCH 1/4] Refactor Note List, add unit test --- app/src/main/AndroidManifest.xml | 1 + .../net/osmtracker/activity/NoteList.java | 245 +++++++----------- .../net/osmtracker/adapter/NoteAdapter.java | 122 +++++++++ .../java/net/osmtracker/db/DataHelper.java | 4 +- .../net/osmtracker/db/NoteListAdapter.java | 100 ------- .../osmtracker/db/TrackContentProvider.java | 28 ++ app/src/main/res/layout/notelist.xml | 8 + app/src/main/res/layout/notelist_item.xml | 104 ++++---- .../net/osmtracker/db/DataHelperNoteTest.java | 68 +++++ .../db/model/OSMVisibilityTest.java | 2 +- .../{test => }/db/model/TrackTest.java | 2 +- 11 files changed, 374 insertions(+), 310 deletions(-) create mode 100644 app/src/main/java/net/osmtracker/adapter/NoteAdapter.java delete mode 100644 app/src/main/java/net/osmtracker/db/NoteListAdapter.java create mode 100644 app/src/main/res/layout/notelist.xml create mode 100644 app/src/test/java/net/osmtracker/db/DataHelperNoteTest.java rename app/src/test/java/net/osmtracker/{test => }/db/model/OSMVisibilityTest.java (98%) rename app/src/test/java/net/osmtracker/{test => }/db/model/TrackTest.java (98%) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 97f6435b6..397e7e289 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -60,6 +60,7 @@ android:label="@string/wplist" /> { + String newName = editNoteName.getText().toString(); + dataHelper.updateNote(trackId, uuid, newName); + refreshData(); + alert.dismiss(); }); - // Delete waypoint - buttonDelete.setOnClickListener(new EditNoteDialogOnClickListener(alert, cursor) { - @Override - public void onClick(View view) { - new AlertDialog.Builder(NoteList.this) - .setTitle(getString(R.string.delete_note_confirm_dialog_title)) - .setMessage(getString(R.string.delete_note_confirm_dialog_msg)) - .setPositiveButton(getString(R.string.delete_note_confirm_bt_ok), new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - dataHelper.deleteNote(uuid); - cursor.requery(); - alert.dismiss(); - dialog.dismiss(); - } - }) - .setNegativeButton(getString(R.string.delete_note_confirm_bt_cancel), new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - dialog.dismiss(); - } - }) - .show(); - } - }); + // Delete note + buttonDelete.setOnClickListener(v -> new AlertDialog.Builder(this) + .setTitle(R.string.delete_note_confirm_dialog_title) + .setMessage(R.string.delete_note_confirm_dialog_msg) + .setPositiveButton(R.string.delete_note_confirm_bt_ok, (dialog, which) -> { + dataHelper.deleteNote(uuid); + refreshData(); + alert.dismiss(); + }) + .setNegativeButton(R.string.delete_note_confirm_bt_cancel, + (dialog, which) -> dialog.dismiss()) + .show()); // Upload note text to OpenStreetMap - buttonOSMUpload.setOnClickListener(new EditNoteDialogOnClickListener(alert, null) { - @Override - public void onClick(View view) { - uploadNoteToOSM(cursor); - } + buttonOSMUpload.setOnClickListener(v -> { + uploadNoteToOSM(noteId); + alert.dismiss(); }); // Cancel button - buttonCancel.setOnClickListener(new EditWaypointDialogOnClickListener(alert, null) { - @Override - public void onClick(View view) { - alert.dismiss(); - } - }); + buttonCancel.setOnClickListener(v -> alert.dismiss()); alert.setView(editNoteDialog); alert.show(); - - super.onListItemClick(l, v, position, id); } - // Where the menu items get defined - @Override - public void onCreateContextMenu(ContextMenu menu, View v, ContextMenu.ContextMenuInfo menuInfo) { - super.onCreateContextMenu(menu, v, menuInfo); - getMenuInflater().inflate(R.menu.note_contextmenu, menu); - } - - // What happens when a menu item is selected - @Override - public boolean onContextItemSelected(MenuItem item) { - AdapterContextMenuInfo info = (AdapterContextMenuInfo) item.getMenuInfo(); - final Cursor cursor = ((CursorAdapter) getListAdapter()).getCursor(); - if (!cursor.moveToPosition(info.position)) return super.onContextItemSelected(item); - - // Menu options when you long press on a note - switch (item.getItemId()) { - case R.id.notelist_contextmenu_osm_note_upload: - uploadNoteToOSM(cursor); - return true; - } - return super.onContextItemSelected(item); - } - /** - * Extracts note data from the cursor and launches the OSM Note Upload activity. - * @param cursor The cursor positioned at the selected note. + * Extracts note data from DB and launches the OSM Note Upload activity. */ - private void uploadNoteToOSM(Cursor cursor) { - long noteId = cursor.getLong(cursor.getColumnIndex(TrackContentProvider.Schema.COL_ID)); - String noteText = cursor.getString(cursor.getColumnIndex(TrackContentProvider.Schema.COL_NAME)); - double lat = cursor.getDouble(cursor.getColumnIndex(TrackContentProvider.Schema.COL_LATITUDE)); - double lon = cursor.getDouble(cursor.getColumnIndex(TrackContentProvider.Schema.COL_LONGITUDE)); - - Intent intent = new Intent(this, OpenStreetMapNotesUpload.class); - intent.putExtra("noteId", noteId); - intent.putExtra("noteContent", noteText); - intent.putExtra("appName", getString(R.string.app_name)); - intent.putExtra("latitude", lat); - intent.putExtra("longitude", lon); - - // Retrieve app version number - try { - PackageInfo pi = getPackageManager().getPackageInfo(getPackageName(), 0); - intent.putExtra("version", pi.versionName); - } catch (PackageManager.NameNotFoundException e) { - // Log error or ignore - } + private void uploadNoteToOSM(long noteId) { + // Query the specific note to get latest Lat/Lon + Cursor cursor = getContentResolver().query( + TrackContentProvider.noteUri(noteId), + null,null,null,null); + + if (cursor != null && cursor.moveToFirst()) { + String noteText = cursor.getString( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_NAME)); + double lat = cursor.getDouble( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_LATITUDE)); + double lon = cursor.getDouble( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_LONGITUDE)); + + Intent intent = new Intent(this, OpenStreetMapNotesUpload.class); + intent.putExtra("noteId", noteId); + intent.putExtra("noteContent", noteText); + intent.putExtra("appName", getString(R.string.app_name)); + intent.putExtra("latitude", lat); + intent.putExtra("longitude", lon); + + // Retrieve app version number + try { + PackageInfo pi = getPackageManager().getPackageInfo(getPackageName(), 0); + intent.putExtra("version", pi.versionName); + } catch (PackageManager.NameNotFoundException e) { + // Ignore + Log.d(TAG, "Package name not found", e); + } - startActivity(intent); + cursor.close(); + startActivity(intent); + } } } diff --git a/app/src/main/java/net/osmtracker/adapter/NoteAdapter.java b/app/src/main/java/net/osmtracker/adapter/NoteAdapter.java new file mode 100644 index 000000000..896c8dadc --- /dev/null +++ b/app/src/main/java/net/osmtracker/adapter/NoteAdapter.java @@ -0,0 +1,122 @@ +package net.osmtracker.adapter; + +import android.content.Context; +import android.database.Cursor; +import android.view.LayoutInflater; +import android.view.View; +import android.view.ViewGroup; +import android.widget.ImageView; +import android.widget.TextView; + +import androidx.annotation.NonNull; +import androidx.recyclerview.widget.RecyclerView; + +import net.osmtracker.R; +import net.osmtracker.db.TrackContentProvider; + +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.Locale; +import java.util.TimeZone; + +public class NoteAdapter extends RecyclerView.Adapter { + + public static final SimpleDateFormat DATE_FORMATTER = + new SimpleDateFormat("HH:mm:ss 'UTC'", Locale.ROOT); + static { + DATE_FORMATTER.setTimeZone(TimeZone.getTimeZone("UTC")); + } + + private Cursor cursor; + private final OnNoteClickListener listener; + + public interface OnNoteClickListener { + void onNoteClick(long id, long noteId, String uuid, String name); + } + + public NoteAdapter(OnNoteClickListener listener) { + this.listener = listener; + } + + public void swapCursor(Cursor newCursor) { + if (cursor == newCursor) return; + if (cursor != null) cursor.close(); + cursor = newCursor; + notifyDataSetChanged(); + } + + @NonNull + @Override + public NoteViewHolder onCreateViewHolder(ViewGroup parent, int viewType) { + View v = LayoutInflater.from( + parent.getContext()).inflate(R.layout.notelist_item, parent, false); + return new NoteViewHolder(v); + } + + @Override + public void onBindViewHolder(@NonNull NoteViewHolder holder, int position) { + if (cursor.moveToPosition(position)) { + Context context = holder.itemView.getContext(); + + // Bind name + String name = cursor.getString( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_NAME)); + holder.tvName.setText(name); + + // Upload status + if (cursor.isNull( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_OSM_UPLOAD_DATE))) { + holder.ivUploadStatus.setVisibility(View.GONE); + } else { + holder.ivUploadStatus.setImageResource(android.R.drawable.stat_sys_upload_done); + holder.ivUploadStatus.setVisibility(View.VISIBLE); + } + + //Bind Location (Latitude/Longitude) + String lat = cursor.getString( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_LATITUDE)); + String lon = cursor.getString( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_LONGITUDE)); + + String locationStr = context.getString(R.string.wplist_latitude) + lat + ", " + + context.getString(R.string.wplist_longitude) + lon; + holder.tvLocation.setText(locationStr); + + // Bind Timestamp + Date ts = new Date(cursor.getLong( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_TIMESTAMP))); + holder.tvTimestamp.setText(DATE_FORMATTER.format(ts)); + + // Setup Click Listener + String uuid = cursor.getString( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_UUID)); + long trackId = cursor.getLong( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_TRACK_ID)); + long noteId = cursor.getLong( + cursor.getColumnIndexOrThrow(TrackContentProvider.Schema.COL_ID)); + holder.itemView.setOnClickListener( + v -> listener.onNoteClick(trackId, noteId, uuid, name)); + } + } + + @Override + public int getItemCount() { + return (cursor == null) ? 0 : cursor.getCount(); + } + + public static class NoteViewHolder extends RecyclerView.ViewHolder { + TextView tvName; + ImageView ivUploadStatus; + TextView tvLocation; + TextView tvTimestamp; + + + public NoteViewHolder(View v) { + super(v); + tvName = v.findViewById(R.id.notelist_item_name); + ivUploadStatus = v.findViewById(R.id.notelist_item_upload_status_icon); + tvLocation = v.findViewById(R.id.notelist_item_location); + tvTimestamp = v.findViewById(R.id.notelist_item_timestamp); + } + } +} \ No newline at end of file diff --git a/app/src/main/java/net/osmtracker/db/DataHelper.java b/app/src/main/java/net/osmtracker/db/DataHelper.java index 68f8f85fc..37935bec1 100644 --- a/app/src/main/java/net/osmtracker/db/DataHelper.java +++ b/app/src/main/java/net/osmtracker/db/DataHelper.java @@ -349,7 +349,7 @@ public void trackNote(long trackId, Location location, String name, String uuid) * Updates a note * * @param trackId Id of the track - * @param uuid Unique ID of the target waypoint + * @param uuid Unique ID of the target note * @param name New text value for the note */ public void updateNote(long trackId, String uuid, String name) { @@ -374,7 +374,7 @@ public void updateNote(long trackId, String uuid, String name) { public void deleteNote(String uuid) { Log.v(TAG, "Deleting note with uuid '" + uuid); if (uuid != null) { - contentResolver.delete(Uri.withAppendedPath(TrackContentProvider.CONTENT_URI_WAYPOINT_UUID, uuid), null, null); + contentResolver.delete(Uri.withAppendedPath(TrackContentProvider.CONTENT_URI_NOTE_UUID, uuid), null, null); } } diff --git a/app/src/main/java/net/osmtracker/db/NoteListAdapter.java b/app/src/main/java/net/osmtracker/db/NoteListAdapter.java deleted file mode 100644 index 17e83713a..000000000 --- a/app/src/main/java/net/osmtracker/db/NoteListAdapter.java +++ /dev/null @@ -1,100 +0,0 @@ -package net.osmtracker.db; - -import android.content.Context; -import android.database.Cursor; -import android.view.LayoutInflater; -import android.view.View; -import android.view.ViewGroup; -import android.widget.CursorAdapter; -import android.widget.ImageView; -import android.widget.TableLayout; -import android.widget.TextView; - -import net.osmtracker.R; - -import java.text.SimpleDateFormat; -import java.util.Date; -import java.util.TimeZone; - -/** - * Adapter for the note list. Gets notes from database. - * - */ -public class NoteListAdapter extends CursorAdapter { - - /** - * Date formatter - */ - public static final SimpleDateFormat DATE_FORMATTER = new SimpleDateFormat("HH:mm:ss 'UTC'"); - static { - DATE_FORMATTER.setTimeZone(TimeZone.getTimeZone("UTC")); - } - - /** - * Constructor. - * - * @param context - * Application context - * @param c - * {@link Cursor} to data - */ - public NoteListAdapter(Context context, Cursor c) { - super(context, c); - } - - @Override - public void bindView(View view, Context context, Cursor cursor) { - TableLayout tl = (TableLayout) view; - bind(cursor, tl, context); - } - - @Override - public View newView(Context context, Cursor cursor, ViewGroup vg) { - TableLayout tl = (TableLayout) LayoutInflater.from(vg.getContext()).inflate(R.layout.notelist_item, - vg, false); - return bind(cursor, tl, context); - } - - /** - * Do the binding between data and item view. - * - * @param cursor Cursor to pull data - * @param tl RelativeView representing one item - * @param context Context, to get resources - * @return The relative view with data bound. - */ - private View bind(Cursor cursor, TableLayout tl, Context context) { - TextView vName = tl.findViewById(R.id.notelist_item_name); - ImageView vUploadStatus = tl.findViewById(R.id.notelist_item_upload_status_icon); - TextView vLocation = tl.findViewById(R.id.notelist_item_location); - TextView vTimestamp = tl.findViewById(R.id.notelist_item_timestamp); - - // Bind name - String name = cursor.getString(cursor.getColumnIndex(TrackContentProvider.Schema.COL_NAME)); - vName.setText(name); - - // Upload status - if (cursor.isNull(cursor.getColumnIndex(TrackContentProvider.Schema.COL_OSM_UPLOAD_DATE))) { - vUploadStatus.setVisibility(View.GONE); - } - else{ - vUploadStatus.setImageResource(android.R.drawable.stat_sys_upload_done); - vUploadStatus.setVisibility(View.VISIBLE); - } - - // Bind location - StringBuffer locationAsString = new StringBuffer(); - locationAsString.append(context.getResources().getString(R.string.wplist_latitude) - + cursor.getString(cursor.getColumnIndex(TrackContentProvider.Schema.COL_LATITUDE))); - locationAsString.append(", " + context.getResources().getString(R.string.wplist_longitude) - + cursor.getString(cursor.getColumnIndex(TrackContentProvider.Schema.COL_LONGITUDE))); - - vLocation.setText(locationAsString.toString()); - - // Bind timestamp - Date ts = new Date(cursor.getLong(cursor.getColumnIndex(TrackContentProvider.Schema.COL_TIMESTAMP))); - vTimestamp.setText(DATE_FORMATTER.format(ts)); - return tl; - } - -} diff --git a/app/src/main/java/net/osmtracker/db/TrackContentProvider.java b/app/src/main/java/net/osmtracker/db/TrackContentProvider.java index 8b9467370..a5129c8a4 100644 --- a/app/src/main/java/net/osmtracker/db/TrackContentProvider.java +++ b/app/src/main/java/net/osmtracker/db/TrackContentProvider.java @@ -119,6 +119,7 @@ public class TrackContentProvider extends ContentProvider { uriMatcher.addURI(AUTHORITY, Schema.TBL_WAYPOINT + "/uuid/*", Schema.URI_CODE_WAYPOINT_UUID); uriMatcher.addURI(AUTHORITY, Schema.TBL_TRACKPOINT + "/#", Schema.URI_CODE_TRACKPOINT_ID); uriMatcher.addURI(AUTHORITY, Schema.TBL_NOTE + "/#", Schema.URI_CODE_NOTE_ID); + uriMatcher.addURI(AUTHORITY, Schema.TBL_NOTE + "/uuid/*", Schema.URI_CODE_NOTE_UUID); } /** @@ -139,6 +140,14 @@ public static final Uri waypointUri(long waypointId) { return ContentUris.withAppendedId(CONTENT_URI_WAYPOINT, waypointId); } + /** + * @param noteId target note id + * @return Uri for the note + */ + public static final Uri noteUri(long noteId) { + return ContentUris.withAppendedId(CONTENT_URI_NOTE, noteId); + } + /** * @param trackId target track id * @return Uri for the notes of the track @@ -223,6 +232,14 @@ public int delete(Uri uri, String selection, String[] selectionArgs) { count = 0; } break; + case Schema.URI_CODE_NOTE_UUID: + String noteUUID = uri.getLastPathSegment(); + if(noteUUID != null){ + count = dbHelper.getWritableDatabase().delete(Schema.TBL_NOTE, Schema.COL_UUID + " = ?", new String[]{noteUUID}); + }else{ + count = 0; + } + break; default: throw new IllegalArgumentException("Unknown URI: " + uri); } @@ -386,6 +403,16 @@ public Cursor query(Uri uri, String[] projection, String selectionIn, String[] s selection = Schema.COL_TRACK_ID + " = ?"; selectionArgs = new String[] {trackId}; break; + case Schema.URI_CODE_NOTE_ID: + if (selectionIn != null || selectionArgsIn != null) { + // Any selection/selectionArgs will be ignored + throw new UnsupportedOperationException(); + } + String noteId = uri.getPathSegments().get(1); + qb.setTables(Schema.TBL_NOTE); + selection = Schema.COL_ID + " = ?"; + selectionArgs = new String[] {noteId}; + break; case Schema.URI_CODE_TRACK_START: if (selectionIn != null || selectionArgsIn != null) { // Any selection/selectionArgs will be ignored @@ -585,6 +612,7 @@ public static final class Schema { public static final int URI_CODE_TRACKPOINT_ID = 12; public static final int URI_CODE_TRACK_NOTES = 13; public static final int URI_CODE_NOTE_ID = 14; + public static final int URI_CODE_NOTE_UUID = 15; public static final int VAL_TRACK_ACTIVE = 1; diff --git a/app/src/main/res/layout/notelist.xml b/app/src/main/res/layout/notelist.xml new file mode 100644 index 000000000..15b60f4ce --- /dev/null +++ b/app/src/main/res/layout/notelist.xml @@ -0,0 +1,8 @@ + + + + \ No newline at end of file diff --git a/app/src/main/res/layout/notelist_item.xml b/app/src/main/res/layout/notelist_item.xml index de51a0e5d..1fef1e588 100644 --- a/app/src/main/res/layout/notelist_item.xml +++ b/app/src/main/res/layout/notelist_item.xml @@ -1,58 +1,58 @@ - + - + - + - - + - + - - - - - - \ No newline at end of file + \ No newline at end of file diff --git a/app/src/test/java/net/osmtracker/db/DataHelperNoteTest.java b/app/src/test/java/net/osmtracker/db/DataHelperNoteTest.java new file mode 100644 index 000000000..b243fc9f4 --- /dev/null +++ b/app/src/test/java/net/osmtracker/db/DataHelperNoteTest.java @@ -0,0 +1,68 @@ +package net.osmtracker.db; + +import android.database.Cursor; +import android.location.Location; +import android.os.Bundle; + +import androidx.test.core.app.ApplicationProvider; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import java.util.UUID; + +@RunWith(RobolectricTestRunner.class) +@Config(sdk = 25) +public class DataHelperNoteTest { + + private DataHelper dataHelper; + + @Before + public void setup() { + // Initialize DataHelper with the Robolectric application context + dataHelper = new DataHelper(ApplicationProvider.getApplicationContext()); + } + + @Test + public void testDeleteNote_RemovesFromDatabase() { + String noteUUID = UUID.randomUUID().toString(); + long trackId = 1L; + + // 1. Insert a note + Location loc = new Location("gps"); + loc.setLatitude(1.23); + loc.setLongitude(4.56); + loc.setTime(System.currentTimeMillis()); + + // Initialize extras to avoid NullPointerException if logic accesses location extras + loc.setExtras(new Bundle()); + + dataHelper.trackNote(trackId, loc, "Note to delete", noteUUID); + + // 2. Verify it exists before deletion + Assert.assertTrue("Note should exist after insertion", noteExists(trackId)); + + // 3. Delete note + dataHelper.deleteNote(noteUUID); + + // 4. Verify it is gone + Assert.assertFalse("Note should have been deleted from DB", noteExists(trackId)); + } + + private boolean noteExists(long TrackId) { + // Query using the content resolver provided by the Robolectric environment + Cursor c = ApplicationProvider.getApplicationContext().getContentResolver().query( + TrackContentProvider.notesUri(TrackId), + null,null,null,null); + + boolean exists = (c != null && c.getCount() > 0); + if (c != null) { + c.close(); + } + return exists; + } +} \ No newline at end of file diff --git a/app/src/test/java/net/osmtracker/test/db/model/OSMVisibilityTest.java b/app/src/test/java/net/osmtracker/db/model/OSMVisibilityTest.java similarity index 98% rename from app/src/test/java/net/osmtracker/test/db/model/OSMVisibilityTest.java rename to app/src/test/java/net/osmtracker/db/model/OSMVisibilityTest.java index 36e2fdde4..39ea4af0e 100644 --- a/app/src/test/java/net/osmtracker/test/db/model/OSMVisibilityTest.java +++ b/app/src/test/java/net/osmtracker/db/model/OSMVisibilityTest.java @@ -1,4 +1,4 @@ -package net.osmtracker.test.db.model; +package net.osmtracker.db.model; import android.content.Context; diff --git a/app/src/test/java/net/osmtracker/test/db/model/TrackTest.java b/app/src/test/java/net/osmtracker/db/model/TrackTest.java similarity index 98% rename from app/src/test/java/net/osmtracker/test/db/model/TrackTest.java rename to app/src/test/java/net/osmtracker/db/model/TrackTest.java index 04c524fbc..fa02164cb 100644 --- a/app/src/test/java/net/osmtracker/test/db/model/TrackTest.java +++ b/app/src/test/java/net/osmtracker/db/model/TrackTest.java @@ -1,4 +1,4 @@ -package net.osmtracker.test.db.model; +package net.osmtracker.db.model; import android.content.ContentResolver; import android.database.Cursor; From 9beed2dd28111e11f9f22bc14f2ee87537162b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Guti=C3=A9rrez=20Alfaro?= Date: Tue, 20 Jan 2026 18:41:07 -0600 Subject: [PATCH 2/4] Open Street Map Notes Upload clases and add unit test --- app/src/main/AndroidManifest.xml | 3 +- .../activity/OpenStreetMapNotesUpload.java | 227 +++++++------- .../osm/UploadToOpenStreetMapNotesTask.java | 283 +++++++++--------- app/src/main/res/layout/osm_note_upload.xml | 36 ++- .../OpenStreetMapNotesUploadTest.java | 107 +++++++ 5 files changed, 382 insertions(+), 274 deletions(-) create mode 100644 app/src/test/java/net/osmtracker/activity/OpenStreetMapNotesUploadTest.java diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 397e7e289..071c23b80 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -70,6 +70,8 @@ android:label="@string/osm_upload" android:exported="true"> + @@ -114,7 +116,6 @@ - Uploads a note on OSM using the API and - * OAuth authentication.

+ *

Uploads a note on OSM using the API and OAuth authentication.

* *

This activity may be called twice during a single * upload cycle: First to start the upload, then a second @@ -37,7 +38,7 @@ * * @author Most of the code was made by Nicolas Guillaumin, adapted by Jose Andrés Vargas Serrano */ -public class OpenStreetMapNotesUpload extends Activity { +public class OpenStreetMapNotesUpload extends AppCompatActivity { private static final String TAG = OpenStreetMapNotesUpload.class.getSimpleName(); @@ -51,19 +52,39 @@ public class OpenStreetMapNotesUpload extends Activity { /** URL that the browser will call once the user is authenticated */ public final static String OAUTH2_CALLBACK_URL = "osmtracker://osm-upload/oath2-completed/"; - public final static int RC_AUTH = 7; - private AuthorizationService authService; + private ActivityResultLauncher authLauncher; @Override protected void onCreate(Bundle savedInstanceState) { - super.onCreate(savedInstanceState); - View uploadNoteView = getLayoutInflater().inflate(R.layout.osm_note_upload, null); - setContentView(uploadNoteView); - setTitle(R.string.osm_note_upload); - - noteContentView = uploadNoteView.findViewById(R.id.wplist_item_name); - noteFooterView = uploadNoteView.findViewById(R.id.osm_note_footer); + super.onCreate(savedInstanceState); + + // Register the launcher + authLauncher = registerForActivityResult( + new ActivityResultContracts.StartActivityForResult(), + result -> { + // This replaces the logic previously in onActivityResult + Intent data = result.getData(); + // RC_AUTH logic + if (data != null) { + AuthorizationResponse resp = AuthorizationResponse.fromIntent(data); + AuthorizationException ex = AuthorizationException.fromIntent(data); + + if (resp != null) { + exchangeAuthorizationCode(resp); + } else { + Log.e(TAG, "Authorization failed: " + (ex != null ? ex.getMessage() : "Unknown error")); + Toast.makeText(this, R.string.osm_upload_oauth_failed, Toast.LENGTH_SHORT).show(); + } + } + } + ); + + + setContentView(R.layout.osm_note_upload); + setTitle(R.string.osm_note_upload); + noteContentView = findViewById(R.id.wplist_item_name); + noteFooterView = findViewById(R.id.osm_note_footer); // Read and cache extras Bundle extras = getIntent().getExtras(); @@ -85,20 +106,10 @@ protected void onCreate(Bundle savedInstanceState) { noteContentView.setText(initialNoteText); noteFooterView.setText(getString(R.string.osm_note_footer, appName, version)); - final Button btnOk = (Button) findViewById(R.id.osm_note_upload_button_ok); - btnOk.setOnClickListener(new OnClickListener() { - @Override - public void onClick(View v) { - startUpload(noteId); - } - }); - final Button btnCancel = (Button) findViewById(R.id.osm_note_upload_button_cancel); - btnCancel.setOnClickListener(new OnClickListener() { - @Override - public void onClick(View v) { - finish(); - } - }); + final Button btnOk = findViewById(R.id.osm_note_upload_button_ok); + btnOk.setOnClickListener(v -> startUpload(noteId)); + final Button btnCancel = findViewById(R.id.osm_note_upload_button_cancel); + btnCancel.setOnClickListener(v -> finish()); } @@ -109,100 +120,96 @@ public void onClick(View v) { */ private void startUpload(long noteId) { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this); - if ( prefs.contains(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN) ) { - // Re-use saved token - uploadToOsm(prefs.getString(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN, ""), noteId); - } else { - // Open browser and request token + String accessToken = prefs.getString(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN, null); + + if (accessToken != null && !accessToken.isEmpty()) { + // STATE: AUTHORIZED. Re-use saved token + Log.d(TAG, "Token found, proceeding to upload note to OSM."); + uploadToOsm(accessToken, noteId); + } else { + // STATE: UNAUTHORIZED. Open browser and request token + Log.d(TAG, "No token found, requesting authorization."); requestOsmAuth(); } } /* - * Init Authorization request workflow. + * Init Authorization request workflow. Launches browser to request authorization. */ public void requestOsmAuth() { // Authorization service configuration - AuthorizationServiceConfiguration serviceConfig = - new AuthorizationServiceConfiguration( + AuthorizationServiceConfiguration serviceConfig = new AuthorizationServiceConfiguration( Uri.parse(OpenStreetMapConstants.OAuth2.Urls.AUTHORIZATION_ENDPOINT), Uri.parse(OpenStreetMapConstants.OAuth2.Urls.TOKEN_ENDPOINT)); - // Obtaining an authorization code - Uri redirectURI = Uri.parse(OAUTH2_CALLBACK_URL); - AuthorizationRequest.Builder authRequestBuilder = - new AuthorizationRequest.Builder( - serviceConfig, OpenStreetMapConstants.OAuth2.CLIENT_ID, - ResponseTypeValues.CODE, redirectURI); - AuthorizationRequest authRequest = authRequestBuilder - .setScope(OpenStreetMapConstants.OAuth2.SCOPE) - .build(); - - // Start activity. + // Obtaining an authorization code + AuthorizationRequest authRequest = new AuthorizationRequest.Builder( + serviceConfig, + OpenStreetMapConstants.OAuth2.CLIENT_ID, + ResponseTypeValues.CODE, + Uri.parse(OAUTH2_CALLBACK_URL)) + .setScope(OpenStreetMapConstants.OAuth2.SCOPE) + .build(); + + // Start activity. authService = new AuthorizationService(this); Intent authIntent = authService.getAuthorizationRequestIntent(authRequest); - startActivityForResult(authIntent, RC_AUTH); //when done onActivityResult will be called. + //when done onActivityResult will be called. + // Use the launcher instead of startActivityForResult + authLauncher.launch(authIntent); } - - protected void onActivityResult(int requestCode, int resultCode, Intent data) { - super.onActivityResult(requestCode, resultCode, data); - // User is returning from authentication - if (requestCode == RC_AUTH) { - // Handling the authorization response - AuthorizationResponse resp = AuthorizationResponse.fromIntent(data); - AuthorizationException ex = AuthorizationException.fromIntent(data); - // ... process the response or exception ... - if (ex != null) { - Log.e(TAG, "Authorization Error. Exception received from server."); - Log.e(TAG, ex.getMessage()); - } else if (resp == null) { - Log.e(TAG, "Authorization Error. Null response from server."); - } else { - SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this); - - //Exchanging the authorization code - authService.performTokenRequest( - resp.createTokenExchangeRequest(), - new AuthorizationService.TokenResponseCallback() { - @Override public void onTokenRequestCompleted( - TokenResponse resp, AuthorizationException ex) { - if (resp != null) { - // exchange succeeded - SharedPreferences.Editor editor = prefs.edit(); - editor.putString(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN, resp.accessToken); - editor.apply(); - //continue with the note Upload. - uploadToOsm(resp.accessToken, noteId); - } else { - // authorization failed, check ex for more details - Log.e(TAG, "OAuth failed."); - } - } - }); - } - } else { - Log.e(TAG, "Unexpected requestCode:" + requestCode + "."); - } - } + private void exchangeAuthorizationCode(AuthorizationResponse resp) { + authService.performTokenRequest(resp.createTokenExchangeRequest(), (tokenResp, tokenEx) -> { + if (tokenResp != null && tokenResp.accessToken != null) { + // STATE: TRANSITION TO AUTHORIZED + persistToken(tokenResp.accessToken); + uploadToOsm(tokenResp.accessToken, noteId); + } else { + Log.e(TAG, "Token exchange failed"); + } + }); + } + + private void persistToken(String token) { + PreferenceManager.getDefaultSharedPreferences(this).edit() + .putString(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN, token) + .apply(); + } /** * Uploads notes to OSM. */ public void uploadToOsm(String accessToken, long noteId) { - String noteText = noteContentView.getText().toString(); - String footer = noteFooterView.getText().toString(); - if (!footer.isEmpty()) { - noteText = noteText + "\n\n" + footer; - } - new UploadToOpenStreetMapNotesTask( - OpenStreetMapNotesUpload.this, - accessToken, - noteId, - noteText, - latitude, - longitude - ).execute(); - } - + String noteText = noteContentView.getText().toString(); + String footer = noteFooterView.getText().toString(); + if (!footer.isEmpty()) { + noteText = noteText + "\n\n" + footer; + } + + // Final variables for the background thread + final String finalNoteText = noteText; + + // This replaces the deprecated AsyncTask.execute() + ExecutorService executor = Executors.newSingleThreadExecutor(); + executor.execute(() -> { + try { + new UploadToOpenStreetMapNotesTask( + OpenStreetMapNotesUpload.this, + accessToken, + noteId, + finalNoteText, + latitude, + longitude + ).run(); + } catch (Exception e) { + Log.e(TAG, "Error during OSM Note upload", e); + runOnUiThread(() -> + Toast.makeText(this, R.string.osm_upload_error, Toast.LENGTH_SHORT).show() + ); + } finally { + executor.shutdown(); + } + }); + } } diff --git a/app/src/main/java/net/osmtracker/osm/UploadToOpenStreetMapNotesTask.java b/app/src/main/java/net/osmtracker/osm/UploadToOpenStreetMapNotesTask.java index 55d75c749..28c6c560b 100644 --- a/app/src/main/java/net/osmtracker/osm/UploadToOpenStreetMapNotesTask.java +++ b/app/src/main/java/net/osmtracker/osm/UploadToOpenStreetMapNotesTask.java @@ -1,184 +1,173 @@ package net.osmtracker.osm; import android.app.Activity; -import android.app.AlertDialog; -import android.app.ProgressDialog; -import android.content.DialogInterface; -import android.content.SharedPreferences.Editor; -import android.os.AsyncTask; -import android.preference.PreferenceManager; import android.util.Log; +import android.view.Gravity; +import android.widget.LinearLayout; +import android.widget.ProgressBar; +import android.widget.TextView; + +import androidx.appcompat.app.AlertDialog; +import androidx.preference.PreferenceManager; import net.osmtracker.OSMTracker; import net.osmtracker.R; import net.osmtracker.db.DataHelper; import net.osmtracker.util.DialogUtils; +import java.lang.ref.WeakReference; + import de.westnordost.osmapi.OsmConnection; import de.westnordost.osmapi.common.errors.OsmAuthorizationException; import de.westnordost.osmapi.common.errors.OsmBadUserInputException; import de.westnordost.osmapi.map.data.OsmLatLon; -import de.westnordost.osmapi.notes.Note; // Note object -import de.westnordost.osmapi.notes.NotesApi; // Api for uploading notes to OSM -import de.westnordost.osmapi.map.data.LatLon; // Data type for location points, maybe I'll put it in the dialog file +import de.westnordost.osmapi.notes.NotesApi; /** * Uploads a note to OpenStreetMap * * @author Most of the code was made by Nicolas Guillaumin, adapted by Jose Andrés Vargas Serrano */ -public class UploadToOpenStreetMapNotesTask extends AsyncTask { +public class UploadToOpenStreetMapNotesTask { private static final String TAG = UploadToOpenStreetMapNotesTask.class.getSimpleName(); - /** Upload progress dialog */ - private ProgressDialog dialog; + // Result constants + private static final int RESULT_OK = 1; + private static final int RESULT_ERROR_INTERNAL = -1; + private static final int RESULT_ERROR_AUTH = -2; + private static final int RESULT_ERROR_OSM_USER = -3; - private final Activity activity; + private final WeakReference activityRef; private final String accessToken; + //OSM Note data private final long noteId; - - /** Note text */ private final String noteText; - - /** Note longitude */ private final double longitude; - - /** Note latitude */ private final double latitude; - /** - * Error message, or text of the response returned by OSM - * if the request completed - */ + private AlertDialog progressDialog; + // Error message returned by OSM if the request completed private String errorMsg; + private int resultCode = RESULT_ERROR_INTERNAL; - /** - * Either the HTTP result code, or -1 for an internal error - */ - private int resultCode = -1; - private final int authorizationErrorResultCode = -2; - private final int anotherErrorResultCode = -3; - private final int okResultCode = 1; - - // Not using an activity yet - public UploadToOpenStreetMapNotesTask(Activity activity, String accessToken, long noteId, + public UploadToOpenStreetMapNotesTask(Activity activity, String accessToken, long noteId, String noteText, double latitude, double longitude) { - this.activity = activity; - this.accessToken = accessToken; + this.activityRef = new WeakReference<>(activity); + this.accessToken = accessToken; this.noteId = noteId; - this.noteText = noteText; - this.longitude = longitude; - this.latitude = latitude; - } - - @Override - protected void onPreExecute() { - try { - // Display progress dialog - dialog = new ProgressDialog(activity); - dialog.setProgressStyle(ProgressDialog.STYLE_HORIZONTAL); - dialog.setIndeterminate(true); - dialog.setTitle(R.string.osm_note_upload); - - dialog.setCancelable(false); - dialog.show(); - - } catch (Exception e) { - Log.e(TAG, "onPreExecute() failed", e); - errorMsg = e.getLocalizedMessage(); - cancel(true); - } - } - - @Override - protected void onPostExecute(Void result) { - switch (resultCode) { - case -1: - dialog.dismiss(); - // Internal error, the request didn't start at all - DialogUtils.showErrorDialog(activity, - activity.getResources().getString(R.string.osm_note_upload_error) - + ": " + errorMsg); - break; - case okResultCode: - dialog.dismiss(); - // Success ! Update database and close activity - DataHelper.setNoteUploadDate(noteId, System.currentTimeMillis(), activity.getContentResolver()); - - new AlertDialog.Builder(activity) - .setTitle(android.R.string.dialog_alert_title) - .setIcon(android.R.drawable.ic_dialog_info) - .setMessage(R.string.osm_upload_sucess) - .setCancelable(true) - .setNeutralButton(android.R.string.ok, new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - dialog.dismiss(); - activity.finish(); - } - }).create().show(); - - break; - case authorizationErrorResultCode: - dialog.dismiss(); - Log.e(TAG, "onPostExecute() authorization failed: " + errorMsg + " (" + resultCode + ")"); - // Authorization issue. Provide a way to clear credentials - new AlertDialog.Builder(activity) - .setTitle(android.R.string.dialog_alert_title) - .setIcon(android.R.drawable.ic_dialog_alert) - .setMessage(R.string.osm_note_upload_unauthorized) - .setCancelable(true) - .setNegativeButton(android.R.string.no, new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - dialog.dismiss(); - } - }) - .setPositiveButton(android.R.string.yes, new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - Editor editor = PreferenceManager.getDefaultSharedPreferences(activity).edit(); - editor.remove(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN); - editor.commit(); - - dialog.dismiss(); - } - }).create().show(); - break; - - default: - // Another error. Display OSM response - dialog.dismiss(); - // Internal error, the request didn't start at all - Log.e(TAG, "onPostExecute() default failed: " + errorMsg + " (" + resultCode + ")"); - DialogUtils.showErrorDialog(activity, - activity.getResources().getString(R.string.osm_note_upload_error) - + ": " + errorMsg); - } - } - - @Override - protected Void doInBackground(Void... params) { - OsmConnection osm = new OsmConnection(OpenStreetMapConstants.Api.OSM_API_URL_PATH, - OpenStreetMapConstants.OAuth2.USER_AGENT, accessToken); - - try { - LatLon point = new OsmLatLon(latitude, longitude); - Note note = new NotesApi(osm).create(point, noteText); - resultCode = okResultCode; - } catch (/*IOException |*/ IllegalArgumentException | OsmBadUserInputException e) { - Log.d(TAG, e.getMessage()); - resultCode = -1; //internal error. - } catch (OsmAuthorizationException oae) { - Log.d(TAG, "OsmAuthorizationException"); - resultCode = authorizationErrorResultCode; - } catch (Exception e) { - Log.e(TAG, e.getMessage()); - resultCode = anotherErrorResultCode; - } - return null; - } + this.noteText = noteText; + this.longitude = longitude; + this.latitude = latitude; + } + + /** + * Synchronous execution for use with ExecutorService. + * Replaces the lifecycle of AsyncTask. */ + public void run() { + final Activity activity = activityRef.get(); + if (activity == null || activity.isFinishing()) return; + + // 1. Prepare UI (Equivalent to onPreExecute) + activity.runOnUiThread(() -> progressDialog = createProgressDialog(activity)); + + // 2. Execute Network Logic (Equivalent to doInBackground) + OsmConnection osm = new OsmConnection( + OpenStreetMapConstants.Api.OSM_API_URL_PATH, + OpenStreetMapConstants.OAuth2.USER_AGENT, + accessToken); + + try { + new NotesApi(osm).create(new OsmLatLon(latitude, longitude), noteText); + resultCode = RESULT_OK; + } catch (OsmBadUserInputException e) { + Log.e(TAG, "Bad OSM user input or illegal argument", e); + errorMsg = e.getLocalizedMessage(); + resultCode = RESULT_ERROR_OSM_USER; + } catch (OsmAuthorizationException oae) { + Log.e(TAG, "OSM Authorization error", oae); + errorMsg = oae.getLocalizedMessage(); + resultCode = RESULT_ERROR_AUTH; + } catch (Exception e) { + Log.e(TAG, "Upload error", e); + errorMsg = e.getLocalizedMessage(); + resultCode = RESULT_ERROR_INTERNAL; + } + + // 3. Handle Results (Equivalent to onPostExecute) + activity.runOnUiThread(() -> { + Activity act = activityRef.get(); + if (act == null || act.isFinishing()) return; + + if (progressDialog != null && progressDialog.isShowing()) { + progressDialog.dismiss(); + } + + handleResult(act); + }); + } + + private void handleResult(Activity activity) { + switch (resultCode) { + case RESULT_OK: + DataHelper.setNoteUploadDate(noteId, + System.currentTimeMillis(), activity.getContentResolver()); + new AlertDialog.Builder(activity) + .setIcon(android.R.drawable.ic_dialog_info) + .setMessage(R.string.osm_upload_sucess) + .setPositiveButton(android.R.string.ok, + (d, w) -> activity.finish()) + .show(); + break; + + case RESULT_ERROR_AUTH: + showAuthErrorDialog(activity); + break; + + default: + DialogUtils.showErrorDialog(activity, + activity.getString(R.string.osm_note_upload_error) + ": " + errorMsg); + break; + } + } + + private AlertDialog createProgressDialog(Activity activity) { + LinearLayout layout = new LinearLayout(activity); + layout.setOrientation(LinearLayout.HORIZONTAL); + layout.setPadding(50, 50, 50, 50); + layout.setGravity(Gravity.CENTER_VERTICAL); + + ProgressBar pb = new ProgressBar(activity); + pb.setIndeterminate(true); + layout.addView(pb); + + TextView tv = new TextView(activity); + tv.setText(R.string.osm_note_upload); + tv.setPadding(40, 0, 0, 0); + layout.addView(tv); + + AlertDialog progressDialog = new AlertDialog.Builder(activity) + .setView(layout).setCancelable(false).create(); + progressDialog.show(); + return progressDialog; + } + + /** + * Helper to show the specific authorization error dialog + */ + private void showAuthErrorDialog(Activity activity) { + new AlertDialog.Builder(activity) + .setTitle(android.R.string.dialog_alert_title) + .setIcon(android.R.drawable.ic_dialog_alert) + .setMessage(R.string.osm_note_upload_unauthorized) + .setCancelable(true) + .setNegativeButton(android.R.string.no, (d, w) -> d.dismiss()) + .setPositiveButton(android.R.string.yes, (d, w) -> { + PreferenceManager.getDefaultSharedPreferences(activity).edit() + .remove(OSMTracker.Preferences.KEY_OSM_OAUTH2_ACCESSTOKEN).apply(); + d.dismiss(); + }).show(); + } } diff --git a/app/src/main/res/layout/osm_note_upload.xml b/app/src/main/res/layout/osm_note_upload.xml index 268319633..17de9d4e7 100644 --- a/app/src/main/res/layout/osm_note_upload.xml +++ b/app/src/main/res/layout/osm_note_upload.xml @@ -5,45 +5,50 @@ android:padding="16dp"> + android:gravity="center" + android:orientation="vertical"> + android:textAppearance="?android:attr/textAppearanceLarge" + android:textStyle="bold" /> + android:textAppearance="?android:attr/textAppearanceMedium" + android:textColor="@android:color/holo_green_light" /> + android:id="@+id/osm_note_footer" + android:layout_width="fill_parent" + android:layout_height="wrap_content" + android:gravity="center_horizontal" + android:textAppearance="?android:attr/textAppearanceSmall" + android:textColor="@android:color/holo_green_light" /> + android:orientation="horizontal"> +