From 483736637e93bf1a4e4e4e44f2d696b8f5c7e2b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rom=C3=A1n=20Mart=C3=ADnez?= Date: Sun, 22 Nov 2020 17:33:46 +0100 Subject: [PATCH] Refactoring MarkerEditActivity. Logic has been moved to a ViewModel. --- .../opentracks/MarkerEditActivity.java | 169 +++-------------- .../opentracks/content/data/Marker.java | 5 + .../viewmodels/MarkerEditViewModel.java | 173 ++++++++++++++++++ 3 files changed, 208 insertions(+), 139 deletions(-) create mode 100644 src/main/java/de/dennisguse/opentracks/viewmodels/MarkerEditViewModel.java diff --git a/src/main/java/de/dennisguse/opentracks/MarkerEditActivity.java b/src/main/java/de/dennisguse/opentracks/MarkerEditActivity.java index b2698102a..a8f970a58 100644 --- a/src/main/java/de/dennisguse/opentracks/MarkerEditActivity.java +++ b/src/main/java/de/dennisguse/opentracks/MarkerEditActivity.java @@ -33,20 +33,16 @@ import android.widget.ArrayAdapter; import android.widget.Toast; import androidx.annotation.NonNull; +import androidx.lifecycle.ViewModelProvider; -import java.io.File; import java.io.FileDescriptor; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import de.dennisguse.opentracks.content.data.Marker; import de.dennisguse.opentracks.content.data.Track; -import de.dennisguse.opentracks.content.provider.ContentProviderUtils; import de.dennisguse.opentracks.databinding.MarkerEditBinding; -import de.dennisguse.opentracks.services.TrackRecordingServiceConnection; -import de.dennisguse.opentracks.util.FileUtils; import de.dennisguse.opentracks.util.IntentUtils; +import de.dennisguse.opentracks.viewmodels.MarkerEditViewModel; /** * An activity to add/edit a marker. @@ -58,25 +54,20 @@ public class MarkerEditActivity extends AbstractActivity { public static final String EXTRA_TRACK_ID = "track_id"; public static final String EXTRA_MARKER_ID = "marker_id"; - private static final String BUNDLE_PHOTO_URI = "photo_uri"; - private static final int CAMERA_REQUEST_CODE = 5; private static final int GALLERY_IMG_REQUEST_CODE = 7; private static final String TAG = MarkerEditActivity.class.getSimpleName(); private Track.Id trackId; - private final TrackRecordingServiceConnection trackRecordingServiceConnection = new TrackRecordingServiceConnection(); private Marker marker; private MenuItem insertPhotoMenuItem; private MenuItem insertGalleryImgMenuItem; - private Uri photoUri; - private Uri photoUriOriginal; - private final List photoUriDeleteList = new ArrayList<>(); private boolean hasCamera; + private Uri cameraPhotoUri; - private boolean isNewMarker; + private MarkerEditViewModel viewModel; // UI elements private MarkerEditBinding viewBinding; @@ -94,117 +85,40 @@ public class MarkerEditActivity extends AbstractActivity { ArrayAdapter adapter = ArrayAdapter.createFromResource(this, R.array.marker_types, android.R.layout.simple_dropdown_item_1line); viewBinding.markerEditMarkerType.setAdapter(adapter); viewBinding.markerEditPhotoDelete.setOnClickListener(v -> { - if (marker != null && marker.hasPhoto()) { - marker.setPhotoUrl(null); - } - viewBinding.markerEditPhoto.setImageBitmap(null); - photoUriDeleteList.add(photoUri); - photoUriOriginal = photoUriOriginal == null ? photoUri : photoUriOriginal; - photoUri = null; - hideAndShowOptions(); + viewModel.onPhotoDelete(viewBinding.markerEditName.getText().toString(), + viewBinding.markerEditMarkerType.getText().toString(), + viewBinding.markerEditDescription.getText().toString()); }); viewBinding.markerEditCancel.setOnClickListener(v -> { - Track.Id trackId = getTrackId(); - if (isNewMarker && trackId != null) { - // new marker and user cancel -> delete all photos from track directory - for (Uri photoUriDelete : photoUriDeleteList) { - File photoFile = FileUtils.getPhotoFileIfExists(this, trackId, photoUriDelete); - FileUtils.deleteDirectoryRecurse(photoFile); - } - if (photoUri != null) { - File photoFile = FileUtils.getPhotoFileIfExists(this, trackId, photoUri); - FileUtils.deleteDirectoryRecurse(photoFile); - } - } else if (!isNewMarker && trackId != null) { - // no new marker, user cancel and photo was changed -> delete all photos but original one - for (Uri photoUri : photoUriDeleteList) { - if (photoUriOriginal != photoUri) { - File photoFile = FileUtils.getPhotoFileIfExists(this, trackId, photoUri); - FileUtils.deleteDirectoryRecurse(photoFile); - } - } - if (photoUri != null && photoUri != photoUriOriginal) { - File photoFile = FileUtils.getPhotoFileIfExists(this, trackId, photoUri); - FileUtils.deleteDirectoryRecurse(photoFile); - } - } + viewModel.onCancel(); finish(); }); - isNewMarker = markerId == null; - + boolean isNewMarker = markerId == null; setTitle(isNewMarker ? R.string.menu_insert_marker : R.string.menu_edit); viewBinding.markerEditDone.setText(isNewMarker ? R.string.generic_add : R.string.generic_save); viewBinding.markerEditDone.setOnClickListener(v -> { - if (isNewMarker) { - addMarker(); - } else { - saveMarker(); - } - Track.Id trackId = getTrackId(); - if (trackId == null) { - for (Uri photoUri : photoUriDeleteList) { - File photoFile = FileUtils.getPhotoFileIfExists(this, trackId, photoUri); - FileUtils.deleteDirectoryRecurse(photoFile); - } - } + viewModel.onDone(viewBinding.markerEditName.getText().toString(), + viewBinding.markerEditMarkerType.getText().toString(), + viewBinding.markerEditDescription.getText().toString()); finish(); }); - if (isNewMarker) { - int nextMarkerNumber = trackId == null ? -1 : new ContentProviderUtils(this).getNextMarkerNumber(trackId); - if (nextMarkerNumber == -1) { - nextMarkerNumber = 0; - } - viewBinding.markerEditName.setText(getString(R.string.marker_name_format, nextMarkerNumber)); - viewBinding.markerEditName.selectAll(); - viewBinding.markerEditMarkerType.setText(""); - viewBinding.markerEditDescription.setText(""); - } else { - marker = new ContentProviderUtils(this).getMarker(markerId); - if (marker == null) { - Log.d(TAG, "marker is null"); - finish(); - return; - } + viewModel = new ViewModelProvider(this).get(MarkerEditViewModel.class); + viewModel.getMarkerData(trackId, markerId).observe(this, data -> { + marker = data; viewBinding.markerEditName.setText(marker.getName()); viewBinding.markerEditMarkerType.setText(marker.getCategory()); viewBinding.markerEditDescription.setText(marker.getDescription()); if (marker.hasPhoto()) { - photoUri = marker.getPhotoURI(); + setMarkerImageView(marker.getPhotoURI()); + } else { + viewBinding.markerEditPhoto.setImageDrawable(null); } - } - if (savedInstanceState != null) { - photoUri = savedInstanceState.getParcelable(BUNDLE_PHOTO_URI); - if (marker != null) { - marker.setPhotoUrl(photoUri != null ? photoUri.toString() : null); - } - } - if (photoUri != null) { - setMarkerImageView(photoUri); - } - - hideAndShowOptions(); - } - - @Override - protected void onStart() { - super.onStart(); - trackRecordingServiceConnection.startConnection(this); - } - - @Override - protected void onSaveInstanceState(@NonNull Bundle outState) { - super.onSaveInstanceState(outState); - outState.putParcelable(BUNDLE_PHOTO_URI, photoUri); - } - - @Override - protected void onStop() { - super.onStop(); - trackRecordingServiceConnection.unbind(this); + hideAndShowOptions(); + }); } @Override @@ -247,26 +161,20 @@ public class MarkerEditActivity extends AbstractActivity { Toast.makeText(this, R.string.marker_add_photo_canceled, Toast.LENGTH_LONG).show(); return; } else if (resultCode == RESULT_OK) { - setMarkerImageView(photoUri); + viewModel.onNewCameraPhoto(cameraPhotoUri, + viewBinding.markerEditName.getText().toString(), + viewBinding.markerEditMarkerType.getText().toString(), + viewBinding.markerEditDescription.getText().toString()); } } else if (requestCode == GALLERY_IMG_REQUEST_CODE) { if (resultCode == RESULT_CANCELED) { Toast.makeText(this, R.string.marker_add_photo_canceled, Toast.LENGTH_LONG).show(); return; } else if (resultCode == RESULT_OK) { - Uri srcUri = data.getData(); - try (ParcelFileDescriptor parcelFd = getContentResolver().openFileDescriptor(srcUri, "r")) { - FileDescriptor srcFd = parcelFd.getFileDescriptor(); - File dstFile = new File(FileUtils.getImageUrl(this, getTrackId())); - FileUtils.copy(srcFd, dstFile); - - photoUri = FileUtils.getUriForFile(this, dstFile); - setMarkerImageView(photoUri); - } catch (Exception e) { - Log.e(TAG, e.getMessage()); - Toast.makeText(this, R.string.marker_add_canceled, Toast.LENGTH_LONG).show(); - return; - } + viewModel.onNewGalleryPhoto(data.getData(), + viewBinding.markerEditName.getText().toString(), + viewBinding.markerEditMarkerType.getText().toString(), + viewBinding.markerEditDescription.getText().toString()); } } super.onActivityResult(requestCode, resultCode, data); @@ -278,7 +186,7 @@ public class MarkerEditActivity extends AbstractActivity { * If a photo is set then one's options are shown, otherwise another ones are shown. */ private void hideAndShowOptions() { - boolean isPhotoSet = (marker != null && marker.hasPhoto()) || photoUri != null; + boolean isPhotoSet = (marker != null && marker.hasPhoto()); if (insertPhotoMenuItem != null && insertGalleryImgMenuItem != null) { insertPhotoMenuItem.setVisible(!isPhotoSet); insertGalleryImgMenuItem.setVisible(!isPhotoSet); @@ -307,7 +215,7 @@ public class MarkerEditActivity extends AbstractActivity { private void createMarkerWithPicture() { Pair intentAndPhotoUri = IntentUtils.createTakePictureIntent(this, getTrackId()); - photoUri = intentAndPhotoUri.second; + cameraPhotoUri = intentAndPhotoUri.second; startActivityForResult(intentAndPhotoUri.first, CAMERA_REQUEST_CODE); } @@ -315,21 +223,4 @@ public class MarkerEditActivity extends AbstractActivity { Intent intent = new Intent(Intent.ACTION_PICK, MediaStore.Images.Media.EXTERNAL_CONTENT_URI); startActivityForResult(intent, GALLERY_IMG_REQUEST_CODE); } - - private void addMarker() { - trackRecordingServiceConnection.addMarker(this, - viewBinding.markerEditName.getText().toString(), - viewBinding.markerEditMarkerType.getText().toString(), - viewBinding.markerEditDescription.getText().toString(), - photoUri != null ? photoUri.toString() : null); - } - - private void saveMarker() { - marker.setName(viewBinding.markerEditName.getText().toString()); - marker.setCategory(viewBinding.markerEditMarkerType.getText().toString()); - marker.setDescription(viewBinding.markerEditDescription.getText().toString()); - marker.setPhotoUrl(photoUri != null ? photoUri.toString() : null); - - new ContentProviderUtils(this).updateMarker(this, marker); - } } diff --git a/src/main/java/de/dennisguse/opentracks/content/data/Marker.java b/src/main/java/de/dennisguse/opentracks/content/data/Marker.java index 62356650c..909245336 100644 --- a/src/main/java/de/dennisguse/opentracks/content/data/Marker.java +++ b/src/main/java/de/dennisguse/opentracks/content/data/Marker.java @@ -48,6 +48,11 @@ public final class Marker { @Deprecated //TODO Make an URI instead of String private String photoUrl = ""; + public Marker(@Nullable Track.Id trackId) { + this.trackId = trackId; + location = null; + } + @VisibleForTesting public Marker(@NonNull Track.Id trackId, @NonNull TrackPoint trackPoint) { this(trackId, trackPoint.getLocation()); diff --git a/src/main/java/de/dennisguse/opentracks/viewmodels/MarkerEditViewModel.java b/src/main/java/de/dennisguse/opentracks/viewmodels/MarkerEditViewModel.java new file mode 100644 index 000000000..fb9b69a26 --- /dev/null +++ b/src/main/java/de/dennisguse/opentracks/viewmodels/MarkerEditViewModel.java @@ -0,0 +1,173 @@ +package de.dennisguse.opentracks.viewmodels; + +import android.app.Application; +import android.net.Uri; +import android.os.ParcelFileDescriptor; +import android.util.Log; +import android.widget.Toast; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.lifecycle.AndroidViewModel; +import androidx.lifecycle.LiveData; +import androidx.lifecycle.MutableLiveData; + +import java.io.File; +import java.io.FileDescriptor; +import java.util.NoSuchElementException; + +import de.dennisguse.opentracks.R; +import de.dennisguse.opentracks.content.data.Marker; +import de.dennisguse.opentracks.content.data.Track; +import de.dennisguse.opentracks.content.provider.ContentProviderUtils; +import de.dennisguse.opentracks.services.TrackRecordingServiceConnection; +import de.dennisguse.opentracks.util.FileUtils; + +public class MarkerEditViewModel extends AndroidViewModel { + + private static final String TAG = MarkerEditViewModel.class.getSimpleName(); + + private MutableLiveData markerData; + private boolean isNewMarker; + private Uri photoOriginalUri; + private final TrackRecordingServiceConnection trackRecordingServiceConnection = new TrackRecordingServiceConnection(); + + public MarkerEditViewModel(@NonNull Application application) { + super(application); + } + + public LiveData getMarkerData(@NonNull Track.Id trackId, @Nullable Marker.Id markerId) { + if (markerData == null) { + markerData = new MutableLiveData<>(); + trackRecordingServiceConnection.startConnection(getApplication()); + loadData(trackId, markerId); + } + return markerData; + } + + @Override + protected void onCleared() { + super.onCleared(); + trackRecordingServiceConnection.unbind(getApplication()); + } + + private void loadData(Track.Id trackId, Marker.Id markerId) { + Marker marker; + isNewMarker = markerId == null; + if (isNewMarker) { + int nextMarkerNumber = trackId == null ? -1 : new ContentProviderUtils(getApplication()).getNextMarkerNumber(trackId); + if (nextMarkerNumber == -1) { + nextMarkerNumber = 0; + } + marker = new Marker(trackId); + marker.setId(markerId); + marker.setName(getApplication().getString(R.string.marker_name_format, nextMarkerNumber)); + } else { + marker = new ContentProviderUtils(getApplication()).getMarker(markerId); + if (marker.hasPhoto()) { + photoOriginalUri = marker.getPhotoURI(); + } + } + markerData.postValue(marker); + } + + private Marker getMarkerOrThrowException() { + Marker marker = markerData != null ? markerData.getValue() : null; + if (marker == null) { + Log.d(TAG, "Marker data shouldn't be null. Call getMarkerData before."); + throw new NoSuchElementException("Marker data shouldn't be null. Call getMarkerData before."); + } + + return marker; + } + + private void deletePhoto(Uri photoUri) { + File photoFile = FileUtils.getPhotoFileIfExists(getApplication(), markerData.getValue().getTrackId(), photoUri); + FileUtils.deleteDirectoryRecurse(photoFile); + } + + public void onPhotoDelete(String name, String category, String description) { + Marker marker = getMarkerOrThrowException(); + if (marker.hasPhoto()) { + if (!marker.getPhotoURI().equals(photoOriginalUri)) { + deletePhoto(marker.getPhotoURI()); + } + marker.setPhotoUrl(null); + marker.setName(name); + marker.setCategory(category); + marker.setDescription(description); + markerData.postValue(marker); + } + } + + public void onNewCameraPhoto(@NonNull Uri photoUri, String name, String category, String description) { + Marker marker = getMarkerOrThrowException(); + marker.setPhotoUrl(photoUri.toString()); + marker.setName(name); + marker.setCategory(category); + marker.setDescription(description); + markerData.postValue(marker); + } + + public void onNewGalleryPhoto(@NonNull Uri srcUri, String name, String category, String description) { + Marker marker = getMarkerOrThrowException(); + + try (ParcelFileDescriptor parcelFd = getApplication().getContentResolver().openFileDescriptor(srcUri, "r")) { + FileDescriptor srcFd = parcelFd.getFileDescriptor(); + File dstFile = new File(FileUtils.getImageUrl(getApplication(), marker.getTrackId())); + FileUtils.copy(srcFd, dstFile); + + Uri photoUri = FileUtils.getUriForFile(getApplication(), dstFile); + marker.setPhotoUrl(photoUri.toString()); + marker.setName(name); + marker.setCategory(category); + marker.setDescription(description); + markerData.postValue(marker); + } catch(Exception e) { + Log.e(TAG, e.getMessage()); + Toast.makeText(getApplication(), R.string.marker_add_canceled, Toast.LENGTH_LONG).show(); + } + } + + private void onAddDone(@NonNull Marker marker, String name, String category, String description) { + trackRecordingServiceConnection.addMarker(getApplication(), name, category, description, marker.hasPhoto() ? marker.getPhotoURI().toString() : null); + } + + private void onSaveDone(@NonNull Marker marker, String name, String category, String description) { + marker.setName(name); + marker.setCategory(category); + marker.setDescription(description); + new ContentProviderUtils(getApplication()).updateMarker(getApplication(), marker); + + if (photoOriginalUri != null && (!marker.hasPhoto() || !photoOriginalUri.equals(marker.getPhotoURI()))) { + deletePhoto(photoOriginalUri); + } + } + + public void onDone(String name, String category, String description) { + Marker marker = getMarkerOrThrowException(); + if (isNewMarker) { + onAddDone(marker, name, category, description); + } else { + onSaveDone(marker, name, category, description); + } + } + + public void onCancel() { + Marker marker = getMarkerOrThrowException(); + if (isNewMarker) { + if (marker.hasPhoto()) { + deletePhoto(marker.getPhotoURI()); + } + if (photoOriginalUri != null && !photoOriginalUri.equals(marker.getPhotoURI())) { + deletePhoto(photoOriginalUri); + } + } else if (photoOriginalUri != null) { + if (marker.hasPhoto() && !marker.getPhotoURI().equals(photoOriginalUri)) { + deletePhoto(marker.getPhotoURI()); + } + } else if (marker.hasPhoto()) { + deletePhoto(marker.getPhotoURI()); + } + } +}