Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GSoC'24] Refactor: Consolidate media paths into a single variable #16750

Merged
merged 1 commit into from
Jul 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AnkiDroid/src/main/java/com/ichi2/anki/NoteEditor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class NoteEditor : AnkiFragment(R.layout.note_editor), DeckSelectionListener, Su

val index = extras.getInt(MultimediaEditFieldActivity.EXTRA_RESULT_FIELD_INDEX)
val field = extras.getSerializableCompat<IField>(MultimediaEditFieldActivity.EXTRA_RESULT_FIELD) ?: return@NoteEditorActivityResultCallback
if (field.type != EFieldType.TEXT && (field.imagePath == null && field.audioPath == null)) {
if (field.type != EFieldType.TEXT && (field.mediaPath == null)) {
Timber.i("field imagePath and audioPath are both null")
return@NoteEditorActivityResultCallback
}
Expand Down Expand Up @@ -1788,7 +1788,7 @@ class NoteEditor : AnkiFragment(R.layout.note_editor), DeckSelectionListener, Su
?: return

// Process successful result only if field has data
if (field.type != EFieldType.TEXT || field.imagePath != null || field.audioPath != null) {
if (field.type != EFieldType.TEXT || field.mediaPath != null) {
addMediaFileToField(index, field)
} else {
Timber.i("field imagePath and audioPath are both null")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class MultimediaImageFragment : MultimediaFragment(R.layout.fragment_multimedia_
return@setOnClickListener
}

field.imagePath = viewModel.currentImagePath
field.mediaPath = viewModel.currentImagePath
field.hasTemporaryMedia = true

val resultData = Intent().apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,32 +249,22 @@ class MultimediaEditFieldActivity :
@KotlinCleanup("rename: bChangeToText")
private fun done() {
var bChangeToText = false
if (field.type === EFieldType.IMAGE) {
if (field.imagePath == null) {
if (field.type === EFieldType.IMAGE || (field.type === EFieldType.AUDIO_RECORDING && isAudioRecordingSaved)) {
if (field.mediaPath == null) {
bChangeToText = true
}
if (!bChangeToText) {
val f = File(field.imagePath!!)
val f = File(field.mediaPath!!)
if (!f.exists()) {
bChangeToText = true
} else {
} else if (field.type === EFieldType.IMAGE) {
val length = f.length()
if (length > IMAGE_LIMIT) {
showLargeFileCropDialog((1.0 * length / IMAGE_LIMIT).toFloat())
return
}
}
}
} else if (field.type === EFieldType.AUDIO_RECORDING && isAudioRecordingSaved) {
if (field.audioPath == null) {
bChangeToText = true
}
if (!bChangeToText) {
val f = File(field.audioPath!!)
if (!f.exists()) {
bChangeToText = true
}
}
}
fieldController!!.onDone()
saveAndExit(bChangeToText)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ import java.util.regex.Pattern
abstract class AudioField : FieldBase(), IField {
private var _audioPath: String? = null

override var imagePath: String? = null

override var audioPath: String?
override var mediaPath: String?
get() = _audioPath
set(value) {
_audioPath = value
Expand All @@ -45,7 +43,7 @@ abstract class AudioField : FieldBase(), IField {
override var hasTemporaryMedia: Boolean = false

override val formattedValue: String
get() = audioPath?.let { path ->
get() = mediaPath?.let { path ->
val file = File(path)
if (file.exists()) "[sound:${file.name}]" else ""
} ?: ""
Expand All @@ -58,7 +56,7 @@ abstract class AudioField : FieldBase(), IField {
res = m.group(1)!!
}
val mediaDir = col.media.dir + "/"
audioPath = mediaDir + res
mediaPath = mediaDir + res
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ class BasicImageFieldController : FieldControllerBase(), IFieldController {
private fun revertToPreviousImage() {
viewModel.deleteImagePath()
viewModel = ImageViewModel(previousImagePath, previousImageUri)
_field.imagePath = previousImagePath
_field.mediaPath = previousImagePath
previousImagePath = null
previousImageUri = null
}
Expand Down Expand Up @@ -575,7 +575,7 @@ class BasicImageFieldController : FieldControllerBase(), IFieldController {
Timber.w("rotateAndCompress() delete of pre-compressed image failed %s", imagePath)
}
viewModel = imageViewModel.rotateAndCompressTo(outFile.absolutePath, getUriForFile(outFile))
_field.imagePath = outFile.absolutePath
_field.mediaPath = outFile.absolutePath
Timber.d("rotateAndCompress out path %s has size %d", outFile.absolutePath, outFile.length())
} catch (e: FileNotFoundException) {
Timber.w(e, "rotateAndCompress() File not found for image compression %s", imagePath)
Expand Down Expand Up @@ -700,7 +700,7 @@ class BasicImageFieldController : FieldControllerBase(), IFieldController {

private fun setTemporaryMedia(imagePath: String) {
_field.apply {
this.imagePath = imagePath
this.mediaPath = imagePath
hasTemporaryMedia = true
}
}
Expand Down Expand Up @@ -730,7 +730,7 @@ class BasicImageFieldController : FieldControllerBase(), IFieldController {
Timber.i("handleCropResult() appears to have an invalid file, reverting")
return
}
Timber.d("handleCropResult() = image path now %s", _field.imagePath)
Timber.d("handleCropResult() = image path now %s", _field.mediaPath)
}

private fun rotateAndCompress(): Boolean {
Expand Down Expand Up @@ -864,7 +864,7 @@ class BasicImageFieldController : FieldControllerBase(), IFieldController {
var newImagePath = imagePath
var newImageUri = imageUri
if (newImagePath == null) {
newImagePath = field.imagePath
newImagePath = field.mediaPath
}
if (newImageUri == null && newImagePath != null) {
newImageUri = getUriForFile(File(newImagePath), context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ class BasicMediaClipFieldController : FieldControllerBase(), IFieldController {
}
layout.addView(btnVideo, ViewGroup.LayoutParams.MATCH_PARENT)
tvAudioClip = FixedTextView(_activity)
if (_field.audioPath == null) {
if (_field.mediaPath == null) {
tvAudioClip.visibility = View.GONE
} else {
tvAudioClip.text = _field.audioPath
tvAudioClip.text = _field.mediaPath
tvAudioClip.visibility = View.VISIBLE
}
layout.addView(tvAudioClip, ViewGroup.LayoutParams.MATCH_PARENT)
Expand Down Expand Up @@ -181,7 +181,7 @@ class BasicMediaClipFieldController : FieldControllerBase(), IFieldController {

// If everything worked, hand off the information
_field.hasTemporaryMedia = true
_field.audioPath = clipCopy.absolutePath
_field.mediaPath = clipCopy.absolutePath
tvAudioClip.text = clipCopy.name
tvAudioClip.visibility = View.VISIBLE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,8 @@ interface IField : Serializable {

val isModified: Boolean

// For image type. Resets type.
// Makes no sense to call when type is not image.
// the same for other groups below.
var imagePath: String?

// For Audio type
var audioPath: String?
// Path of the folder containing media used by AnkiDroid.
var mediaPath: String?

// For Text type
var text: String?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,13 @@ class ImageField : FieldBase(), IField {
override val isModified: Boolean
get() = thisModified

override var imagePath: String?
override var mediaPath: String?
get() = extraImagePathRef
set(value) {
extraImagePathRef = value
setThisModified()
}

override var audioPath: String? = null

override var text: String? = null

override var hasTemporaryMedia: Boolean = false
Expand All @@ -64,7 +62,7 @@ class ImageField : FieldBase(), IField {

override val formattedValue: String
get() {
val file = File(imagePath!!)
val file = File(mediaPath!!)
return formatImageFileName(file)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ class TextField : FieldBase(), IField {
override val isModified: Boolean
get() = thisModified

override var imagePath: String? = null

override var audioPath: String? = null
override var mediaPath: String? = null

override var text: String?
get() = _text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ object NoteService {
fun importMediaToDirectory(col: Collection, field: IField?) {
var tmpMediaPath: String? = null
when (field!!.type) {
EFieldType.AUDIO_RECORDING, EFieldType.MEDIA_CLIP -> tmpMediaPath = field.audioPath
EFieldType.IMAGE -> tmpMediaPath = field.imagePath
EFieldType.AUDIO_RECORDING, EFieldType.MEDIA_CLIP, EFieldType.IMAGE -> tmpMediaPath = field.mediaPath
EFieldType.TEXT -> {
}
}
Expand All @@ -147,8 +146,7 @@ object NoteService {
inFile.delete()
}
when (field.type) {
EFieldType.AUDIO_RECORDING, EFieldType.MEDIA_CLIP -> field.audioPath = outFile.absolutePath
EFieldType.IMAGE -> field.imagePath = outFile.absolutePath
EFieldType.AUDIO_RECORDING, EFieldType.MEDIA_CLIP, EFieldType.IMAGE -> field.mediaPath = outFile.absolutePath
else -> {
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class AudioRecordingController :
this.state = initialState
audioRecorder = AudioRecorder()
if (inEditField) {
val origAudioPath = this._field.audioPath
val origAudioPath = this._field.mediaPath
var bExist = false
if (origAudioPath != null) {
val f = File(origAudioPath)
Expand Down Expand Up @@ -583,7 +583,7 @@ class AudioRecordingController :
}

private fun saveRecording() {
_field.audioPath = tempAudioPath
_field.mediaPath = tempAudioPath
_field.hasTemporaryMedia = true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ open class BasicImageFieldControllerTest : MultimediaEditFieldActivityTestBase()

private fun imageFieldWithData(): IField {
val field = emptyImageField()
field.imagePath = targetContext.cacheDir.toString() + "/temp-photos/test"
field.mediaPath = targetContext.cacheDir.toString() + "/temp-photos/test"
return field
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class MediaClipTest {
it.createNewFile()
it.deleteOnExit()
}
mediaClipField.audioPath = mediaFile.path
mediaClipField.mediaPath = mediaFile.path

assertThat(mediaClipField.formattedValue, equalTo("[sound:foo]"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ class NoteServiceTest : RobolectricTest() {
FileWriter(fileAudio).use { fileWriter -> fileWriter.write("line1") }

val audioField = MediaClipField()
audioField.audioPath = fileAudio.absolutePath
audioField.mediaPath = fileAudio.absolutePath

NoteService.importMediaToDirectory(col, audioField)

val outFile = File(col.media.dir, fileAudio.name)

assertThat("path should be equal to new file made in NoteService.importMediaToDirectory", outFile, aFileWithAbsolutePath(equalTo(audioField.audioPath)))
assertThat("path should be equal to new file made in NoteService.importMediaToDirectory", outFile, aFileWithAbsolutePath(equalTo(audioField.mediaPath)))
}

// Similar test like above, but with an ImageField instead of a MediaClipField
Expand Down Expand Up @@ -144,14 +144,14 @@ class NoteServiceTest : RobolectricTest() {
FileWriter(f2).use { fileWriter -> fileWriter.write("2") }

val fld1 = MediaClipField()
fld1.audioPath = f1.absolutePath
fld1.mediaPath = f1.absolutePath

val fld2 = MediaClipField()
fld2.audioPath = f2.absolutePath
fld2.mediaPath = f2.absolutePath

// third field to test if name is kept after reimporting the same file
val fld3 = MediaClipField()
fld3.audioPath = f1.absolutePath
fld3.mediaPath = f1.absolutePath

Timber.e("media folder is %s %b", col.media.dir, File(col.media.dir).exists())
NoteService.importMediaToDirectory(col, fld1)
Expand All @@ -163,9 +163,9 @@ class NoteServiceTest : RobolectricTest() {
NoteService.importMediaToDirectory(col, fld3)
// creating a third outfile isn't necessary because it should be equal to the first one

assertThat("path should be equal to new file made in NoteService.importMediaToDirectory", o1, aFileWithAbsolutePath(equalTo(fld1.audioPath)))
assertThat("path should be different to new file made in NoteService.importMediaToDirectory", o2, aFileWithAbsolutePath(not(fld2.audioPath)))
assertThat("path should be equal to new file made in NoteService.importMediaToDirectory", o1, aFileWithAbsolutePath(equalTo(fld3.audioPath)))
assertThat("path should be equal to new file made in NoteService.importMediaToDirectory", o1, aFileWithAbsolutePath(equalTo(fld1.mediaPath)))
assertThat("path should be different to new file made in NoteService.importMediaToDirectory", o2, aFileWithAbsolutePath(not(fld2.mediaPath)))
assertThat("path should be equal to new file made in NoteService.importMediaToDirectory", o1, aFileWithAbsolutePath(equalTo(fld3.mediaPath)))
}

// Similar test like above, but with an ImageField instead of a MediaClipField
Expand Down Expand Up @@ -214,7 +214,7 @@ class NoteServiceTest : RobolectricTest() {
val file = createTransientFile("foo")

val field = MediaClipField().apply {
audioPath = file.absolutePath
mediaPath = file.absolutePath
hasTemporaryMedia = true
}

Expand Down