From 1b33184835cb15ba2b598116ce8589ed320dd42b Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Fri, 21 Feb 2025 22:50:24 +0100 Subject: [PATCH] Cleanup: Marker stores URI to photo (instead of String). --- .../opentracks/content/data/TestDataUtil.java | 3 +- .../data/CustomContentProviderUtilsTest.java | 34 ------------------- .../io/file/importer/ExportImportTest.java | 7 ++-- .../opentracks/data/ContentProviderUtils.java | 8 +++-- .../opentracks/data/models/Marker.java | 16 +++------ .../io/file/exporter/KMLTrackExporter.java | 2 +- .../io/file/exporter/KmzTrackExporter.java | 2 +- .../io/file/importer/GpxTrackImporter.java | 3 +- .../io/file/importer/KmlTrackImporter.java | 5 +-- .../io/file/importer/KmzTrackImporter.java | 2 +- .../io/file/importer/TrackImporter.java | 7 ++-- .../opentracks/share/ShareUtils.java | 6 ++-- .../ui/markers/MarkerDetailFragment.java | 2 +- .../ui/markers/MarkerEditActivity.java | 2 +- .../ui/markers/MarkerEditViewModel.java | 18 +++++----- .../ui/markers/MarkerListAdapter.java | 2 +- .../dennisguse/opentracks/util/FileUtils.java | 4 +++ 17 files changed, 45 insertions(+), 78 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/content/data/TestDataUtil.java b/src/androidTest/java/de/dennisguse/opentracks/content/data/TestDataUtil.java index afd3ebc76..fcad0e4b1 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/content/data/TestDataUtil.java +++ b/src/androidTest/java/de/dennisguse/opentracks/content/data/TestDataUtil.java @@ -145,14 +145,13 @@ public class TestDataUtil { File dstFile = new File(MarkerUtils.getImageUrl(context, trackId)); dstFile.createNewFile(); Uri photoUri = FileUtils.getUriForFile(context, dstFile); - String photoUrl = photoUri.toString(); //TODO Use TrackStatisticsUpdater TrackStatistics stats = new TrackStatistics(); stats.setTotalDistance(Distance.of(0)); stats.setTotalTime(Duration.ofMillis(0)); - return new Marker("Marker name", "Marker description", "Marker category", "", trackId, trackPoint, photoUrl); + return new Marker("Marker name", "Marker description", "Marker category", "", trackId, trackPoint, photoUri); } public static List getTrackPoints(ContentProviderUtils contentProviderUtils, Track.Id trackId) { diff --git a/src/androidTest/java/de/dennisguse/opentracks/data/CustomContentProviderUtilsTest.java b/src/androidTest/java/de/dennisguse/opentracks/data/CustomContentProviderUtilsTest.java index 6f1cdeea9..b5bfe6ba3 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/data/CustomContentProviderUtilsTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/data/CustomContentProviderUtilsTest.java @@ -429,40 +429,6 @@ public class CustomContentProviderUtilsTest { assertEquals(TEST_DESC, contentValues.get(MarkerColumns.DESCRIPTION)); } - /** - * Tests the method {@link ContentProviderUtils#createMarker(Cursor)}. - */ - @Test - public void testCreateMarker() { - int startColumnIndex = 1; - int columnIndex = startColumnIndex; - when(cursorMock.getColumnIndexOrThrow(MarkerColumns._ID)).thenReturn(columnIndex++); - when(cursorMock.getColumnIndexOrThrow(MarkerColumns.NAME)).thenReturn(columnIndex++); - when(cursorMock.getColumnIndexOrThrow(MarkerColumns.TRACKID)).thenReturn(columnIndex++); - columnIndex = startColumnIndex; - // Id - when(cursorMock.isNull(columnIndex++)).thenReturn(false); - // Name - when(cursorMock.isNull(columnIndex++)).thenReturn(false); - // trackIdIndex - when(cursorMock.isNull(columnIndex++)).thenReturn(false); - long id = System.currentTimeMillis(); - columnIndex = startColumnIndex; - // Id - when(cursorMock.getLong(columnIndex++)).thenReturn(id); - // Name - String name = NAME_PREFIX + id; - when(cursorMock.getString(columnIndex++)).thenReturn(name); - // trackIdIndex - long trackId = 11L; - when(cursorMock.getLong(columnIndex++)).thenReturn(trackId); - - Marker marker = contentProviderUtils.createMarker(cursorMock); - assertEquals(id, marker.getId().id()); - assertEquals(name, marker.getName()); - assertEquals(trackId, marker.getTrackId().id()); - } - /** * Tests the method * {@link ContentProviderUtils#deleteMarker(Context, Marker.Id)} diff --git a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java index 6181947b4..1e22cb092 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java @@ -1,6 +1,7 @@ package de.dennisguse.opentracks.io.file.importer; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -142,7 +143,7 @@ public class ExportImportTest { Distance sensorDistance = Distance.of(10); // recording distance interval sendLocation(trackPointCreator, "2020-02-02T02:02:03Z", 3, 14, 10, 13, 15, 10, 1f); - contentProviderUtils.insertMarker(new Marker("Marker 1", "Marker 1 desc", "Marker 1 category", null, trackId, service.getLastStoredTrackPointWithLocation(), "")); + contentProviderUtils.insertMarker(new Marker("Marker 1", "Marker 1 desc", "Marker 1 category", null, trackId, service.getLastStoredTrackPointWithLocation(), null)); // A sensor-only TrackPoint trackPointCreator.setClock("2020-02-02T02:02:04Z"); @@ -157,7 +158,7 @@ public class ExportImportTest { mockSensorData(trackPointCreator, 5f, Distance.of(2), 69f, 3f, 50f, null); // Distance will be added to next TrackPoint sendLocation(trackPointCreator, "2020-02-02T02:02:17Z", 3, 14.001, 10, 13, 15, 10, 0f); - contentProviderUtils.insertMarker(new Marker("Marker 2", "Marker 2 desc", "Marker 2 category", null, trackId, service.getLastStoredTrackPointWithLocation(), "")); + contentProviderUtils.insertMarker(new Marker("Marker 2", "Marker 2 desc", "Marker 2 category", null, trackId, service.getLastStoredTrackPointWithLocation(), null)); trackPointCreator.setClock("2020-02-02T02:02:18Z"); trackPointCreator.getSensorManager().sensorDataSet = new SensorDataSet(trackPointCreator); @@ -540,7 +541,7 @@ public class ExportImportTest { assertEquals(marker.getDescription(), importMarker.getDescription()); // assertEquals(marker.getIcon(), importMarker.getIcon()); // TODO for KML assertEquals(marker.getName(), importMarker.getName()); - assertEquals("", importMarker.getPhotoUrl()); + assertFalse(importMarker.hasPhoto()); assertEquals(marker.getLocation().getLatitude(), importMarker.getLocation().getLatitude(), 0.001); assertEquals(marker.getLocation().getLongitude(), importMarker.getLocation().getLongitude(), 0.001); diff --git a/src/main/java/de/dennisguse/opentracks/data/ContentProviderUtils.java b/src/main/java/de/dennisguse/opentracks/data/ContentProviderUtils.java index d609454a5..3644760fe 100644 --- a/src/main/java/de/dennisguse/opentracks/data/ContentProviderUtils.java +++ b/src/main/java/de/dennisguse/opentracks/data/ContentProviderUtils.java @@ -418,7 +418,7 @@ public class ContentProviderUtils { marker.setIcon(cursor.getString(iconIndex)); } if (!cursor.isNull(photoUrlIndex)) { - marker.setPhotoUrl(cursor.getString(photoUrlIndex)); + marker.setPhotoUrl(Uri.parse(cursor.getString(photoUrlIndex))); } return marker; } @@ -496,7 +496,7 @@ public class ContentProviderUtils { private void deleteMarkerPhoto(Context context, Marker marker) { if (marker != null && marker.hasPhoto()) { - Uri uri = marker.getPhotoURI(); + Uri uri = marker.getPhotoUrl(); File file = MarkerUtils.buildInternalPhotoFile(context, marker.getTrackId(), uri); if (file.exists()) { File parent = file.getParentFile(); @@ -545,7 +545,9 @@ public class ContentProviderUtils { values.put(MarkerColumns.BEARING, marker.getBearing()); } - values.put(MarkerColumns.PHOTOURL, marker.getPhotoUrl()); + if (marker.hasPhoto()) { + values.put(MarkerColumns.PHOTOURL, marker.getPhotoUrl().toString()); + } return values; } diff --git a/src/main/java/de/dennisguse/opentracks/data/models/Marker.java b/src/main/java/de/dennisguse/opentracks/data/models/Marker.java index 7c328077c..4917c8204 100644 --- a/src/main/java/de/dennisguse/opentracks/data/models/Marker.java +++ b/src/main/java/de/dennisguse/opentracks/data/models/Marker.java @@ -24,7 +24,6 @@ import android.os.Parcelable; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import java.time.Duration; import java.time.Instant; /** @@ -51,8 +50,7 @@ public final class Marker { private Altitude altitude; private Float bearing; - @Deprecated //TODO Make an URI instead of String - private String photoUrl = ""; + private Uri photoUrl = null; public Marker(@Nullable Track.Id trackId, Instant time) { this.trackId = trackId; @@ -71,7 +69,7 @@ public final class Marker { } @Deprecated - public Marker(String name, String description, String category, String icon, @NonNull Track.Id trackId, @NonNull TrackPoint trackPoint, String photoUrl) { + public Marker(String name, String description, String category, String icon, @NonNull Track.Id trackId, @NonNull TrackPoint trackPoint, Uri photoUrl) { this(trackId, trackPoint); this.name = name; this.description = description; @@ -222,20 +220,16 @@ public final class Marker { this.bearing = bearing; } - public String getPhotoUrl() { + public Uri getPhotoUrl() { return photoUrl; } - public void setPhotoUrl(String photoUrl) { + public void setPhotoUrl(Uri photoUrl) { this.photoUrl = photoUrl; } - public Uri getPhotoURI() { - return Uri.parse(photoUrl); - } - public boolean hasPhoto() { - return photoUrl != null && !photoUrl.isEmpty(); + return photoUrl != null; } public record Id(long id) implements Parcelable { diff --git a/src/main/java/de/dennisguse/opentracks/io/file/exporter/KMLTrackExporter.java b/src/main/java/de/dennisguse/opentracks/io/file/exporter/KMLTrackExporter.java index 833e9592d..96e95dffa 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/exporter/KMLTrackExporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/exporter/KMLTrackExporter.java @@ -270,7 +270,7 @@ public class KMLTrackExporter implements TrackExporter { } private void writeMarker(Marker marker, ZoneOffset zoneOffset) { - boolean existsPhoto = MarkerUtils.buildInternalPhotoFile(context, marker.getTrackId(), marker.getPhotoURI()) != null; + boolean existsPhoto = MarkerUtils.buildInternalPhotoFile(context, marker.getTrackId(), marker.getPhotoUrl()) != null; if (marker.hasPhoto() && exportPhotos && existsPhoto) { float heading = getHeading(marker.getTrackId(), marker.getLocation()); writePhotoOverlay(marker, heading, zoneOffset); diff --git a/src/main/java/de/dennisguse/opentracks/io/file/exporter/KmzTrackExporter.java b/src/main/java/de/dennisguse/opentracks/io/file/exporter/KmzTrackExporter.java index a13efaa3b..43b1162ef 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/exporter/KmzTrackExporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/exporter/KmzTrackExporter.java @@ -97,7 +97,7 @@ public class KmzTrackExporter implements TrackExporter { } Marker marker = contentProviderUtils.createMarker(cursor); if (marker.hasPhoto()) { - Uri uriPhoto = marker.getPhotoURI(); + Uri uriPhoto = marker.getPhotoUrl(); boolean existsPhoto = MarkerUtils.buildInternalPhotoFile(context, track.getId(), uriPhoto) != null; if (existsPhoto) { addImage(context, zipOutputStream, uriPhoto, marker); diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxTrackImporter.java index 91b8f6d33..5f980fea3 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxTrackImporter.java @@ -17,6 +17,7 @@ package de.dennisguse.opentracks.io.file.importer; import android.content.Context; +import android.net.Uri; import android.util.Log; import org.xml.sax.Attributes; @@ -109,7 +110,7 @@ public class GpxTrackImporter extends DefaultHandler implements XMLImporter.Trac private String cadence; private String power; private String markerType; - private String photoUrl; + private Uri photoUrl; private String uuid; private String gain; private String loss; diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlTrackImporter.java index 31895ffbf..a1e710ea8 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlTrackImporter.java @@ -18,6 +18,7 @@ package de.dennisguse.opentracks.io.file.importer; import android.content.Context; import android.location.Location; +import android.net.Uri; import android.util.Log; import org.xml.sax.Attributes; @@ -121,7 +122,7 @@ public class KmlTrackImporter extends DefaultHandler implements XMLImporter.Trac private String longitude; private String altitude; private String markerType; - private String photoUrl; + private Uri photoUrl; private String uuid; private final TrackImporter trackImporter; @@ -223,7 +224,7 @@ public class KmlTrackImporter extends DefaultHandler implements XMLImporter.Trac } case TAG_HREF -> { if (content != null) { - photoUrl = content.trim(); + photoUrl = Uri.parse(content.trim()); } } } diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java index 00eff7b9e..88c24f574 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java @@ -191,7 +191,7 @@ public class KmzTrackImporter { List photosName = new ArrayList<>(); for (Marker marker : markers) { if (marker.hasPhoto()) { - String photoUrl = Uri.decode(marker.getPhotoUrl()); + String photoUrl = Uri.decode(marker.getPhotoUrl().toString()); //TODO Why Uri.decode()? photosName.add(photoUrl.substring(photoUrl.lastIndexOf(File.separatorChar) + 1)); } } diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/TrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/TrackImporter.java index e2fd7f10e..d5df4ff69 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/TrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/TrackImporter.java @@ -233,12 +233,11 @@ public class TrackImporter { * * @param externalPhotoUrl the file name */ - private String getInternalPhotoUrl(@NonNull Track.Id trackId, @NonNull String externalPhotoUrl) { - String importFileName = KmzTrackImporter.importNameForFilename(externalPhotoUrl); + private Uri getInternalPhotoUrl(@NonNull Track.Id trackId, @NonNull Uri externalPhotoUrl) { + String importFileName = KmzTrackImporter.importNameForFilename(externalPhotoUrl.toString()); File file = MarkerUtils.buildInternalPhotoFile(context, trackId, Uri.parse(importFileName)); if (file != null) { - Uri photoUri = FileUtils.getUriForFile(context, file); - return "" + photoUri; + return FileUtils.getUriForFile(context, file); } return null; diff --git a/src/main/java/de/dennisguse/opentracks/share/ShareUtils.java b/src/main/java/de/dennisguse/opentracks/share/ShareUtils.java index 7a87241aa..df803a18e 100644 --- a/src/main/java/de/dennisguse/opentracks/share/ShareUtils.java +++ b/src/main/java/de/dennisguse/opentracks/share/ShareUtils.java @@ -93,14 +93,14 @@ public class ShareUtils { Log.e(TAG, "MarkerId " + markerId.id() + " could not be resolved."); continue; } - if (marker.getPhotoURI() == null) { + if (marker.getPhotoUrl() == null) { Log.e(TAG, "MarkerId " + markerId.id() + " has no picture."); continue; } - mime = context.getContentResolver().getType(marker.getPhotoURI()); + mime = context.getContentResolver().getType(marker.getPhotoUrl()); - uris.add(marker.getPhotoURI()); + uris.add(marker.getPhotoUrl()); } if (uris.isEmpty()) { diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerDetailFragment.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerDetailFragment.java index 9467aa9d5..a350d4af6 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerDetailFragment.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerDetailFragment.java @@ -235,7 +235,7 @@ public class MarkerDetailFragment extends Fragment { boolean hasPhoto = marker.hasPhoto(); if (hasPhoto) { handler.removeCallbacks(hideText); - viewBinding.markerDetailMarkerPhoto.setImageURI(marker.getPhotoURI()); + viewBinding.markerDetailMarkerPhoto.setImageURI(marker.getPhotoUrl()); handler.postDelayed(hideText, HIDE_TEXT_DELAY.toMillis()); } else { viewBinding.markerDetailMarkerPhoto.setImageResource(MarkerUtils.ICON_ID); diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditActivity.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditActivity.java index 40a272982..7b544490a 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditActivity.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditActivity.java @@ -183,7 +183,7 @@ public class MarkerEditActivity extends AbstractActivity { viewBinding.markerEditMarkerType.setText(marker.getCategory()); viewBinding.markerEditDescription.setText(marker.getDescription()); if (marker.hasPhoto()) { - setMarkerImageView(marker.getPhotoURI()); + setMarkerImageView(marker.getPhotoUrl()); } else { viewBinding.markerEditPhoto.setImageDrawable(null); } diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java index 3bf9b2b04..5d2c8529d 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java @@ -41,7 +41,7 @@ public class MarkerEditViewModel extends AndroidViewModel { Marker marker = new ContentProviderUtils(getApplication()).getMarker(markerId); if (marker.hasPhoto()) { - photoOriginalUri = marker.getPhotoURI(); + photoOriginalUri = marker.getPhotoUrl(); } markerData.postValue(marker); @@ -71,15 +71,15 @@ public class MarkerEditViewModel extends AndroidViewModel { private void deletePhoto(Marker marker) { if (marker.hasPhoto()) { - deletePhoto(marker.getPhotoURI()); + deletePhoto(marker.getPhotoUrl()); } } public void onPhotoDelete(String name, String category, String description) { Marker marker = getMarker(); if (marker.hasPhoto()) { - if (!marker.getPhotoURI().equals(photoOriginalUri)) { - deletePhoto(marker.getPhotoURI()); + if (!marker.getPhotoUrl().equals(photoOriginalUri)) { + deletePhoto(marker.getPhotoUrl()); } marker.setPhotoUrl(null); marker.setName(name); @@ -91,7 +91,7 @@ public class MarkerEditViewModel extends AndroidViewModel { public void onNewCameraPhoto(@NonNull Uri photoUri, String name, String category, String description) { Marker marker = getMarker(); - marker.setPhotoUrl(photoUri.toString()); + marker.setPhotoUrl(photoUri); marker.setName(name); marker.setCategory(category); marker.setDescription(description); @@ -107,7 +107,7 @@ public class MarkerEditViewModel extends AndroidViewModel { FileUtils.copy(srcFd, dstFile); Uri photoUri = FileUtils.getUriForFile(getApplication(), dstFile); - marker.setPhotoUrl(photoUri.toString()); + marker.setPhotoUrl(photoUri); marker.setName(name); marker.setCategory(category); marker.setDescription(description); @@ -130,7 +130,7 @@ public class MarkerEditViewModel extends AndroidViewModel { new ContentProviderUtils(getApplication()).updateMarker(getApplication(), marker); } - if (photoOriginalUri != null && (!marker.hasPhoto() || !photoOriginalUri.equals(marker.getPhotoURI()))) { + if (photoOriginalUri != null && (!marker.hasPhoto() || !photoOriginalUri.equals(marker.getPhotoUrl()))) { deletePhoto(photoOriginalUri); } } @@ -143,7 +143,7 @@ public class MarkerEditViewModel extends AndroidViewModel { String name = getApplication().getString(R.string.marker_name_format, nextMarkerNumber + 1); String icon = getApplication().getString(R.string.marker_icon_url); - Marker marker = new Marker(name, "", "", icon, trackId, trackPoint, ""); + Marker marker = new Marker(name, "", "", icon, trackId, trackPoint, null); if (markerData == null) { markerData = new MutableLiveData<>(); @@ -158,7 +158,7 @@ public class MarkerEditViewModel extends AndroidViewModel { // it's new marker -> clean all photos. deletePhoto(marker); deletePhoto(photoOriginalUri); - } else if (photoOriginalUri == null || (marker.hasPhoto() && !marker.getPhotoURI().equals(photoOriginalUri))) { + } else if (photoOriginalUri == null || (marker.hasPhoto() && !marker.getPhotoUrl().equals(photoOriginalUri))) { // it's an edit marker -> delete photo if it was empty or it was changed (leaving the original in that case). deletePhoto(marker); } diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListAdapter.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListAdapter.java index a88292b01..9a5aeb248 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListAdapter.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListAdapter.java @@ -190,7 +190,7 @@ public class MarkerListAdapter extends RecyclerView.Adapter