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.

Sign up for GitHub

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

Jump to bottom

Update MSC3912 implementation: Redaction of related events #8532

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Update MSC3912 implementation: Redaction of related events #8532

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b61b9d
Update MSC3912 implementation: Redaction of related events
yostyle Jun 13, 2023
148030c
Refactoring
yostyle Jun 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8481.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update MSC3912 implementation: Redaction of related events
4 changes: 1 addition & 3 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/version/Versions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ private const val FEATURE_THREADS_MSC3771 = "org.matrix.msc3771"
private const val FEATURE_THREADS_MSC3773 = "org.matrix.msc3773"
private const val FEATURE_REMOTE_TOGGLE_PUSH_NOTIFICATIONS_MSC3881 = "org.matrix.msc3881"
private const val FEATURE_EVENT_REDACTION_WITH_RELATIONS = "org.matrix.msc3912"
yostyle marked this conversation as resolved.
Show resolved Hide resolved
private const val FEATURE_EVENT_REDACTION_WITH_RELATIONS_STABLE = "org.matrix.msc3912.stable"

/**
* Return true if the SDK supports this homeserver version.
Expand Down Expand Up @@ -165,6 +164,5 @@ internal fun Versions.doesServerSupportRemoteToggleOfPushNotifications(): Boolea
* @return true if event redaction with relations is supported
*/
internal fun Versions.doesServerSupportRedactEventWithRelations(): Boolean {
Copy link
Member

Choose a reason for hiding this comment

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

This method just let you know if the feature is supported or not. There is no way to know if this feature is unstable or not. This is a potential pb, because the name of the attribute depends on the stability of the feature.

This remark is not limited to this feature. This concerns all the features handled here.
I don't know if @bmarty wants to improve this here. This remark has to be taken into account at least for EX.

Here I would add a second method
doesServerSupportStableRedactionOfRelatedEvents
and rename the current one: doesServerSupportUnstableRedactionOfRelatedEvents

return unstableFeatures?.get(FEATURE_EVENT_REDACTION_WITH_RELATIONS).orFalse() ||
unstableFeatures?.get(FEATURE_EVENT_REDACTION_WITH_RELATIONS_STABLE).orFalse()
return unstableFeatures?.get(FEATURE_EVENT_REDACTION_WITH_RELATIONS).orFalse()
}
2 changes: 1 addition & 1 deletion ...sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/RedactEventTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal class DefaultRedactEventTask @Inject constructor(
eventId = params.eventId,
body = EventRedactBody(
reason = params.reason,
withRelations = withRelations,
unstableWithRelations = withRelations,
yostyle marked this conversation as resolved.
Show resolved Hide resolved
)
)
}
Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ internal class LocalEchoEventFactory @Inject constructor(
val content = if (reason != null || withRelations != null) {
EventRedactBody(
reason = reason,
withRelations = withRelations,
unstableWithRelations = withRelations,
yostyle marked this conversation as resolved.
Show resolved Hide resolved
).toContent().plus(additionalContent.orEmpty())
} else {
additionalContent
Expand Down
7 changes: 6 additions & 1 deletion .../src/main/java/org/matrix/android/sdk/internal/session/room/send/model/EventRedactBody.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,10 @@ internal data class EventRedactBody(
val reason: String? = null,

@Json(name = "org.matrix.msc3912.with_relations")
val unstableWithRelations: List<String>? = null,

@Json(name = "with_rel_types")
val withRelations: List<String>? = null,
)
) {
fun getBestWithRelations() = withRelations ?: unstableWithRelations
Copy link
Member

Choose a reason for hiding this comment

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

This function should select the right option according to the stability of the feature

}
2 changes: 1 addition & 1 deletion ...oid/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueueMemento.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ internal class QueueMemento @Inject constructor(
eventId = it.redacts,
roomId = it.roomId,
reason = body?.reason,
withRelations = body?.withRelations,
withRelations = body?.getBestWithRelations(),
)
)
}
Expand Down