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

Draft : add volatile storage when moving to edit mode. #3132

Merged
merged 4 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft

interface ComposerDraftService {
suspend fun loadDraft(roomId: RoomId): ComposerDraft?
suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?)
suspend fun loadDraft(roomId: RoomId, isVolatile: Boolean): ComposerDraft?
suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?, isVolatile: Boolean)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.element.android.features.messages.impl.draft

import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft

interface ComposerDraftStore {
suspend fun loadDraft(roomId: RoomId): ComposerDraft?
suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,44 +18,28 @@

import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
import timber.log.Timber
import javax.inject.Inject

@ContributesBinding(RoomScope::class)
class DefaultComposerDraftService @Inject constructor(
private val client: MatrixClient,
private val volatileComposerDraftStore: VolatileComposerDraftStore,
private val matrixComposerDraftStore: MatrixComposerDraftStore,

Check warning on line 28 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt#L27-L28

Added lines #L27 - L28 were not covered by tests
) : ComposerDraftService {
override suspend fun loadDraft(roomId: RoomId): ComposerDraft? {
return client.getRoom(roomId)?.use { room ->
room.loadComposerDraft()
.onFailure {
Timber.e(it, "Failed to load composer draft for room $roomId")
}
.onSuccess { draft ->
room.clearComposerDraft()
Timber.d("Loaded composer draft for room $roomId : $draft")
}
.getOrNull()
override suspend fun loadDraft(roomId: RoomId, isVolatile: Boolean): ComposerDraft? {
return if (isVolatile) {
volatileComposerDraftStore.loadDraft(roomId)

Check warning on line 32 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt#L32

Added line #L32 was not covered by tests
} else {
matrixComposerDraftStore.loadDraft(roomId)

Check warning on line 34 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt#L34

Added line #L34 was not covered by tests
}
}

override suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?) {
client.getRoom(roomId)?.use { room ->
val updateDraftResult = if (draft == null) {
room.clearComposerDraft()
} else {
room.saveComposerDraft(draft)
}
updateDraftResult
.onFailure {
Timber.e(it, "Failed to update composer draft for room $roomId")
}
.onSuccess {
Timber.d("Updated composer draft for room $roomId")
}
override suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?, isVolatile: Boolean) {
if (isVolatile) {
volatileComposerDraftStore.updateDraft(roomId, draft)

Check warning on line 40 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt#L40

Added line #L40 was not covered by tests
} else {
matrixComposerDraftStore.updateDraft(roomId, draft)

Check warning on line 42 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/DefaultComposerDraftService.kt#L42

Added line #L42 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe introduce a getStore fun to be able to write:

    override suspend fun loadDraft(roomId: RoomId, isVolatile: Boolean): ComposerDraft? {
        return getStore(isVolatile).loadDraft(roomId)
    }

    override suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?, isVolatile: Boolean) {
        getStore(isVolatile).updateDraft(roomId, draft)
    }

    private fun getStore(isVolatile: Boolean): ComposerDraftStore {
        return if (isVolatile) {
            volatileComposerDraftStore
        } else {
            matrixComposerDraftStore
        }
    }

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.element.android.features.messages.impl.draft

import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
import timber.log.Timber
import javax.inject.Inject

class MatrixComposerDraftStore @Inject constructor(
private val client: MatrixClient,

Check warning on line 26 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L25-L26

Added lines #L25 - L26 were not covered by tests
) : ComposerDraftStore {
override suspend fun loadDraft(roomId: RoomId): ComposerDraft? {
return client.getRoom(roomId)?.use { room ->
room.loadComposerDraft()

Check warning on line 30 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L30

Added line #L30 was not covered by tests
.onFailure {
Timber.e(it, "Failed to load composer draft for room $roomId")

Check warning on line 32 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L32

Added line #L32 was not covered by tests
}
.onSuccess { draft ->
room.clearComposerDraft()
Timber.d("Loaded composer draft for room $roomId : $draft")

Check warning on line 36 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L35-L36

Added lines #L35 - L36 were not covered by tests
}
.getOrNull()
}
}

override suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?) {
client.getRoom(roomId)?.use { room ->
val updateDraftResult = if (draft == null) {
room.clearComposerDraft()

Check warning on line 45 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L45

Added line #L45 was not covered by tests
} else {
room.saveComposerDraft(draft)

Check warning on line 47 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L47

Added line #L47 was not covered by tests
}
updateDraftResult

Check warning on line 49 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L49

Added line #L49 was not covered by tests
.onFailure {
Timber.e(it, "Failed to update composer draft for room $roomId")

Check warning on line 51 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L51

Added line #L51 was not covered by tests
}
.onSuccess {
Timber.d("Updated composer draft for room $roomId")

Check warning on line 54 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/draft/MatrixComposerDraftStore.kt#L54

Added line #L54 was not covered by tests
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.element.android.features.messages.impl.draft

import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
import javax.inject.Inject

class VolatileComposerDraftStore @Inject constructor() : ComposerDraftStore {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add docs on the 2 draft stores?

private val drafts: MutableMap<RoomId, ComposerDraft> = mutableMapOf()
Copy link
Member

@bmarty bmarty Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to store the draft in a map since a new VolatileComposerDraftStore will be created for each room I think.


override suspend fun loadDraft(roomId: RoomId): ComposerDraft? {
// Remove the draft from the map when it is loaded
return drafts.remove(roomId)
}

override suspend fun updateDraft(roomId: RoomId, draft: ComposerDraft?) {
if (draft == null) {
drafts.remove(roomId)
} else {
drafts[roomId] = draft
}
}
}
Loading
Loading