Benoit Review: cleanup 2

This commit is contained in:
Benoit Marty 2021-03-26 19:23:13 +01:00 committed by Valere
parent 2cccbb3fce
commit 66ac298e95
29 changed files with 80 additions and 65 deletions

View file

@ -75,9 +75,9 @@ class RxSession(private val session: Session) {
}
fun liveFlattenRoomSummaryChildOf(spaceId: String?): Observable<List<RoomSummary>> {
return session.getFlattenRoomSummaryChildOfLive(spaceId).asObservable()
return session.getFlattenRoomSummaryChildrenOfLive(spaceId).asObservable()
.startWithCallable {
session.getFlattenRoomSummaryChildOf(spaceId)
session.getFlattenRoomSummaryChildrenOf(spaceId)
}
}

View file

@ -180,13 +180,13 @@ class SpaceHierarchyTest : InstrumentedTest {
assertEquals(9, allRooms.size, "Unexpected number of rooms")
val orphans = session.getFlattenRoomSummaryChildOf(null)
val orphans = session.getFlattenRoomSummaryChildrenOf(null)
assertEquals(2, orphans.size, "Unexpected number of orphan rooms")
assertTrue(orphans.indexOfFirst { it.roomId == orphan1 } != -1, "O1 should be an orphan")
assertTrue(orphans.indexOfFirst { it.roomId == orphan2 } != -1, "O2 should be an orphan ${orphans.map { it.name }}")
val aChildren = session.getFlattenRoomSummaryChildOf(spaceAInfo.spaceId)
val aChildren = session.getFlattenRoomSummaryChildrenOf(spaceAInfo.spaceId)
assertEquals(4, aChildren.size, "Unexpected number of flatten child rooms")
assertTrue(aChildren.indexOfFirst { it.name == "A1" } != -1, "A1 should be a child of A")
@ -204,7 +204,7 @@ class SpaceHierarchyTest : InstrumentedTest {
// here we do not set the parent!!
}
val orphansUpdate = session.getFlattenRoomSummaryChildOf(null)
val orphansUpdate = session.getFlattenRoomSummaryChildrenOf(null)
assertEquals(2, orphansUpdate.size, "Unexpected number of orphan rooms ${orphansUpdate.map { it.name }}")
}
@ -240,7 +240,7 @@ class SpaceHierarchyTest : InstrumentedTest {
// A -> C -> A
val aChildren = session.getFlattenRoomSummaryChildOf(spaceAInfo.spaceId)
val aChildren = session.getFlattenRoomSummaryChildrenOf(spaceAInfo.spaceId)
assertEquals(4, aChildren.size, "Unexpected number of flatten child rooms ${aChildren.map { it.name }}")
assertTrue(aChildren.indexOfFirst { it.name == "A1" } != -1, "A1 should be a child of A")
@ -273,7 +273,7 @@ class SpaceHierarchyTest : InstrumentedTest {
}
val flatAChildren = runBlocking(Dispatchers.Main) {
session.getFlattenRoomSummaryChildOfLive(spaceAInfo.spaceId)
session.getFlattenRoomSummaryChildrenOfLive(spaceAInfo.spaceId)
}
commonTestHelper.waitWithLatch { latch ->

View file

@ -201,5 +201,9 @@ interface RoomService {
fun getFlattenRoomSummaryChildOf(spaceId: String?, memberships: List<Membership> = Membership.activeMemberships()) : List<RoomSummary>
fun getFlattenRoomSummaryChildOfLive(spaceId: String?, memberships: List<Membership> = Membership.activeMemberships()): LiveData<List<RoomSummary>>
/**
* Returns all the children of this space, as LiveData
*/
fun getFlattenRoomSummaryChildrenOfLive(spaceId: String?,
memberships: List<Membership> = Membership.activeMemberships()): LiveData<List<RoomSummary>>
}

View file

@ -47,7 +47,7 @@ data class RoomThirdPartyInviteContent(
/**
* Keys with which the token may be signed.
*/
@Json(name = "public_keys") val publicKeys: List<PublicKeys>? = emptyList()
@Json(name = "public_keys") val publicKeys: List<PublicKeys>?
)
@JsonClass(generateAdapter = true)

View file

@ -69,6 +69,9 @@ open class CreateRoomParams {
*/
val invite3pids = mutableListOf<ThreePid>()
/**
* Initial Guest Access
*/
var guestAccess: GuestAccess? = null
/**

View file

@ -35,12 +35,13 @@ interface Space {
*/
fun spaceSummary(): RoomSummary?
suspend fun addChildren(roomId: String, viaServers: List<String>,
suspend fun addChildren(roomId: String,
viaServers: List<String>,
order: String?,
autoJoin: Boolean = false,
suggested: Boolean? = false)
suspend fun removeRoom(roomId: String)
suspend fun removeChildren(roomId: String)
@Throws
suspend fun setChildrenOrder(roomId: String, order: String?)

View file

@ -28,7 +28,8 @@ typealias SpaceSummaryQueryParams = RoomSummaryQueryParams
interface SpaceService {
/**
* Create a room asynchronously
* Create a space asynchronously
* @return the spaceId of the created space
*/
suspend fun createSpace(params: CreateSpaceParams): String
@ -39,8 +40,8 @@ interface SpaceService {
/**
* Get a space from a roomId
* @param roomId the roomId to look for.
* @return a room with roomId or null if room type is not space
* @param spaceId the roomId to look for.
* @return a space with spaceId or null if room type is not space
*/
fun getSpace(spaceId: String): Space?
@ -54,8 +55,9 @@ interface SpaceService {
/**
* Get's information of a space by querying the server
*/
suspend fun querySpaceChildren(spaceId: String, suggestedOnly: Boolean? = null, autoJoinedOnly: Boolean? = null)
: Pair<RoomSummary, List<SpaceChildInfo>>
suspend fun querySpaceChildren(spaceId: String,
suggestedOnly: Boolean? = null,
autoJoinedOnly: Boolean? = null): Pair<RoomSummary, List<SpaceChildInfo>>
/**
* Get a live list of space summaries. This list is refreshed as soon as the data changes.

View file

@ -60,11 +60,9 @@ data class SpaceChildContent(
* or consist of more than 50 characters, are forbidden and should be ignored if received.)
*/
fun validOrder(): String? {
order?.let {
if (order.length > 50) return null
if (!ORDER_VALID_CHAR_REGEX.matches(it)) return null
}
return order
?.takeIf { it.length <= 50 }
?.takeIf { ORDER_VALID_CHAR_REGEX.matches(it) }
}
companion object {

View file

@ -894,7 +894,7 @@ internal class DefaultCryptoService @Inject constructor(
}
}
private fun onRoomHistoryVisibilityEvent(roomId: String, event: Event) {
private fun onRoomHistoryVisibilityEvent(roomId: String, event: Event) {
val eventContent = event.content.toModel<RoomHistoryVisibilityContent>()
eventContent?.safeHistoryVisibility()?.let {
cryptoStore.setShouldEncryptForInvitedMembers(roomId, it != RoomHistoryVisibility.JOINED)

View file

@ -26,7 +26,6 @@ import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeout
import timber.log.Timber
internal suspend fun <T> awaitNotEmptyResult(realmConfiguration: RealmConfiguration,
timeoutMillis: Long,
@ -41,7 +40,6 @@ internal suspend fun <T> awaitNotEmptyResult(realmConfiguration: RealmConfigurat
val listener = object : RealmChangeListener<RealmResults<T>> {
override fun onChange(it: RealmResults<T>) {
Timber.v("## Space: $it")
if (it.isNotEmpty()) {
result.removeChangeListener(this)
latch.complete(Unit)

View file

@ -121,9 +121,9 @@ internal class DefaultSession @Inject constructor(
private val integrationManagerService: IntegrationManagerService,
private val thirdPartyService: Lazy<ThirdPartyService>,
private val callSignalingService: Lazy<CallSignalingService>,
private val spaceService: Lazy<SpaceService>,
@UnauthenticatedWithCertificate
private val unauthenticatedWithCertificateOkHttpClient: Lazy<OkHttpClient>,
private val spaceService: Lazy<SpaceService>
private val unauthenticatedWithCertificateOkHttpClient: Lazy<OkHttpClient>
) : Session,
RoomService by roomService.get(),
RoomDirectoryService by roomDirectoryService.get(),

View file

@ -173,14 +173,14 @@ internal class DefaultRoomService @Inject constructor(
return peekRoomTask.execute(PeekRoomTask.Params(roomIdOrAlias))
}
override fun getFlattenRoomSummaryChildOf(spaceId: String?, memberships: List<Membership>): List<RoomSummary> {
override fun getFlattenRoomSummaryChildrenOf(spaceId: String?, memberships: List<Membership>): List<RoomSummary> {
if (spaceId == null) {
return roomSummaryDataSource.getFlattenOrphanRooms()
}
return roomSummaryDataSource.getAllRoomSummaryChildOf(spaceId, memberships)
}
override fun getFlattenRoomSummaryChildOfLive(spaceId: String?, memberships: List<Membership>): LiveData<List<RoomSummary>> {
override fun getFlattenRoomSummaryChildrenOfLive(spaceId: String?, memberships: List<Membership>): LiveData<List<RoomSummary>> {
if (spaceId == null) {
return roomSummaryDataSource.getFlattenOrphanRoomsLive()
}

View file

@ -145,7 +145,9 @@ internal class DefaultPeekRoomTask @Inject constructor(
val roomType = stateEvents
.lastOrNull { it.type == EventType.STATE_ROOM_CREATE }
?.let { it.content?.toModel<RoomCreateContent>()?.type }
?.content
?.toModel<RoomCreateContent>()
?.type
return PeekResult.Success(
roomId = roomId,

View file

@ -34,10 +34,11 @@ import timber.log.Timber
* The state_key is the ID of a child room or space, and the content should contain a via key which gives
* a list of candidate servers that can be used to join the room. present: true key is included to distinguish from a deleted state event.
*
* - Separately, rooms can claim parents via the m.room.parent state event:
* - Separately, rooms can claim parents via the m.room.parent state event.
*/
internal class RoomChildRelationInfo(private val realm: Realm,
private val roomId: String
internal class RoomChildRelationInfo(
private val realm: Realm,
private val roomId: String
) {
data class SpaceChildInfo(

View file

@ -29,7 +29,7 @@ internal class HierarchyLiveDataHelper(
private val sources = HashMap<String, LiveData<Optional<RoomSummary>>>()
private val mediatorLiveData = MediatorLiveData<List<String>>()
fun liveData() = mediatorLiveData
fun liveData(): LiveData<List<String>> = mediatorLiveData
init {
onChange()

View file

@ -251,14 +251,10 @@ internal class RoomSummaryDataSource @Inject constructor(@SessionDatabase privat
queryParams.includeType?.forEach {
query.equalTo(RoomSummaryEntityFields.ROOM_TYPE, it)
}
queryParams.roomCategoryFilter?.let {
when (it) {
RoomCategoryFilter.ONLY_DM -> query.equalTo(RoomSummaryEntityFields.IS_DIRECT, true)
RoomCategoryFilter.ONLY_ROOMS -> query.equalTo(RoomSummaryEntityFields.IS_DIRECT, false)
RoomCategoryFilter.ALL -> {
// nop
}
}
when (queryParams.roomCategoryFilter) {
RoomCategoryFilter.ONLY_DM -> query.equalTo(RoomSummaryEntityFields.IS_DIRECT, true)
RoomCategoryFilter.ONLY_ROOMS -> query.equalTo(RoomSummaryEntityFields.IS_DIRECT, false)
RoomCategoryFilter.ALL -> Unit // nop
}
return query
}
@ -272,7 +268,7 @@ internal class RoomSummaryDataSource @Inject constructor(@SessionDatabase privat
fun getAllRoomSummaryChildOfLive(spaceId: String, memberShips: List<Membership>): LiveData<List<RoomSummary>> {
// we want to listen to all spaces in hierarchy and on change compute back all childs
// and switch map to listen thoose?
// and switch map to listen those?
val mediatorLiveData = HierarchyLiveDataHelper(spaceId, memberShips, this).liveData()
return Transformations.switchMap(mediatorLiveData) { allIds ->

View file

@ -47,7 +47,8 @@ internal class DefaultSpace(
return spaceSummaryDataSource.getSpaceSummary(room.roomId)
}
override suspend fun addChildren(roomId: String, viaServers: List<String>,
override suspend fun addChildren(roomId: String,
viaServers: List<String>,
order: String?,
autoJoin: Boolean,
suggested: Boolean?) {
@ -63,7 +64,7 @@ internal class DefaultSpace(
)
}
override suspend fun removeRoom(roomId: String) {
override suspend fun removeChildren(roomId: String) {
val existing = room.getStateEvents(setOf(EventType.STATE_SPACE_CHILD), QueryStringValue.Equals(roomId))
.firstOrNull()
?.content.toModel<SpaceChildContent>()

View file

@ -18,7 +18,6 @@ package org.matrix.android.sdk.internal.session.space
import android.net.Uri
import androidx.lifecycle.LiveData
import com.zhuinden.monarchy.Monarchy
import org.matrix.android.sdk.api.query.QueryStringValue
import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.events.model.toContent
@ -37,7 +36,6 @@ import org.matrix.android.sdk.api.session.space.SpaceService
import org.matrix.android.sdk.api.session.space.SpaceSummaryQueryParams
import org.matrix.android.sdk.api.session.space.model.SpaceChildContent
import org.matrix.android.sdk.api.session.space.model.SpaceParentContent
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.di.UserId
import org.matrix.android.sdk.internal.session.room.RoomGetter
import org.matrix.android.sdk.internal.session.room.SpaceGetter
@ -49,7 +47,6 @@ import org.matrix.android.sdk.internal.session.space.peeking.SpacePeekResult
import javax.inject.Inject
internal class DefaultSpaceService @Inject constructor(
@SessionDatabase private val monarchy: Monarchy,
@UserId private val userId: String,
private val createSpaceTask: CreateSpaceTask,
private val joinSpaceTask: JoinSpaceTask,
@ -97,11 +94,14 @@ internal class DefaultSpaceService @Inject constructor(
override fun getRootSpaceSummaries(): List<RoomSummary> {
return roomSummaryDataSource.getRootSpaceSummaries()
}
override suspend fun peekSpace(spaceId: String): SpacePeekResult {
return peekSpaceTask.execute(PeekSpaceTask.Params(spaceId))
}
override suspend fun querySpaceChildren(spaceId: String, suggestedOnly: Boolean?, autoJoinedOnly: Boolean?): Pair<RoomSummary, List<SpaceChildInfo>> {
override suspend fun querySpaceChildren(spaceId: String,
suggestedOnly: Boolean?,
autoJoinedOnly: Boolean?): Pair<RoomSummary, List<SpaceChildInfo>> {
return resolveSpaceInfoTask.execute(ResolveSpaceInfoTask.Params.withId(spaceId, suggestedOnly, autoJoinedOnly)).let { response ->
val spaceDesc = response.rooms?.firstOrNull { it.roomId == spaceId }
Pair(
@ -136,7 +136,7 @@ internal class DefaultSpaceService @Inject constructor(
activeMemberCount = childSummary.numJoinedMembers,
parentRoomId = childStateEv?.roomId
)
} ?: emptyList()
}.orEmpty()
)
}
}

View file

@ -25,7 +25,6 @@ import org.matrix.android.sdk.internal.database.awaitNotEmptyResult
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.session.room.RoomAPI
import org.matrix.android.sdk.internal.session.room.membership.joining.JoinRoomTask
import org.matrix.android.sdk.internal.session.room.summary.RoomSummaryDataSource
import org.matrix.android.sdk.internal.task.Task
@ -42,7 +41,6 @@ internal interface JoinSpaceTask : Task<JoinSpaceTask.Params, SpaceService.JoinS
}
internal class DefaultJoinSpaceTask @Inject constructor(
private val roomAPI: RoomAPI,
private val joinRoomTask: JoinRoomTask,
@SessionDatabase
private val realmConfiguration: RealmConfiguration,
@ -81,7 +79,7 @@ internal class DefaultJoinSpaceTask @Inject constructor(
return SpaceService.JoinSpaceResult.PartialSuccess(emptyMap())
}
val errors = HashMap<String, Throwable>()
val errors = mutableMapOf<String, Throwable>()
Timber.v("## Space: > Sync done ...")
// after that i should have the children (? do I need to paginate to get state)
val summary = roomSummaryDataSource.getSpaceSummary(params.roomIdOrAlias)
@ -93,7 +91,7 @@ internal class DefaultJoinSpaceTask @Inject constructor(
// I should try to join as well
if (it.roomType == RoomType.SPACE) {
// recursively join auto-joined child of this space?
when (val subspaceJoinResult = this.execute(JoinSpaceTask.Params(it.childRoomId, null, it.viaServers))) {
when (val subspaceJoinResult = execute(JoinSpaceTask.Params(it.childRoomId, null, it.viaServers))) {
SpaceService.JoinSpaceResult.Success -> {
// nop
}

View file

@ -16,6 +16,7 @@
package org.matrix.android.sdk.internal.session.space
import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
import org.matrix.android.sdk.internal.network.executeRequest
import org.matrix.android.sdk.internal.task.Task
import javax.inject.Inject
@ -31,13 +32,21 @@ internal interface ResolveSpaceInfoTask : Task<ResolveSpaceInfoTask.Params, Spac
) {
companion object {
fun withId(spaceId: String, suggestedOnly: Boolean?, autoJoinOnly: Boolean?) =
Params(spaceId, 10, 20, null, suggestedOnly, autoJoinOnly)
Params(
spaceId = spaceId,
maxRoomPerSpace = 10,
limit = 20,
batchToken = null,
suggestedOnly = suggestedOnly,
autoJoinOnly = autoJoinOnly
)
}
}
}
internal class DefaultResolveSpaceInfoTask @Inject constructor(
private val spaceApi: SpaceApi
private val spaceApi: SpaceApi,
private val globalErrorReceiver: GlobalErrorReceiver
) : ResolveSpaceInfoTask {
override suspend fun execute(params: ResolveSpaceInfoTask.Params): SpacesResponse {
val body = SpaceSummaryParams(
@ -47,7 +56,7 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor(
autoJoinedOnly = params.autoJoinOnly,
suggestedOnly = params.suggestedOnly
)
return executeRequest<SpacesResponse>(null) {
return executeRequest(globalErrorReceiver) {
apiCall = spaceApi.getSpaces(params.spaceId, body)
}
}

View file

@ -38,6 +38,5 @@ internal interface SpaceApi {
*/
@POST(NetworkConstants.URI_API_PREFIX_PATH_UNSTABLE + "org.matrix.msc2946/rooms/{roomId}/spaces")
fun getSpaces(@Path("roomId") spaceId: String,
@Body params: SpaceSummaryParams
): Call<SpacesResponse>
@Body params: SpaceSummaryParams): Call<SpacesResponse>
}

View file

@ -49,8 +49,8 @@ internal abstract class SpaceModule {
abstract fun bindJoinSpaceTask(task: DefaultJoinSpaceTask): JoinSpaceTask
@Binds
abstract fun bindSpaceGetter(getter: DefaultSpaceGetter): SpaceGetter
abstract fun bindCreateSpaceTask(task: DefaultCreateSpaceTask): CreateSpaceTask
@Binds
abstract fun bindCreateSpaceTask(getter: DefaultCreateSpaceTask): CreateSpaceTask
abstract fun bindSpaceGetter(getter: DefaultSpaceGetter): SpaceGetter
}

View file

@ -55,7 +55,9 @@ internal class DefaultPeekSpaceTask @Inject constructor(
}
val isSpace = stateEvents
.lastOrNull { it.type == EventType.STATE_ROOM_CREATE && it.stateKey == "" }
?.let { it.content?.toModel<RoomCreateContent>()?.type } == RoomType.SPACE
?.content
?.toModel<RoomCreateContent>()
?.type == RoomType.SPACE
if (!isSpace) return SpacePeekResult.NotSpaceType(params.roomIdOrAlias)

View file

@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.session.space.peeking
import org.matrix.android.sdk.api.session.room.peeking.PeekResult
// TODO Move to api package
data class SpacePeekSummary(
val idOrAlias: String,
val roomPeekResult: PeekResult.Success,

View file

@ -215,7 +215,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, insertType)
CurrentStateEventEntity.getOrCreate(realm, roomId, event.stateKey, event.type).apply {
Timber.v("## Space state event: $eventEntity")
// Timber.v("## Space state event: $eventEntity")
eventId = event.eventId
root = eventEntity
}

View file

@ -25,5 +25,5 @@ internal data class RoomSyncAccountData(
/**
* List of account data events (array of Event).
*/
@Json(name = "events") val events: List<Event>? = emptyList()
@Json(name = "events") val events: List<Event>? = null
)

View file

@ -26,5 +26,5 @@ internal data class RoomSyncEphemeral(
/**
* List of ephemeral events (array of Event).
*/
@Json(name = "events") val events: List<Event>? = emptyList()
@Json(name = "events") val events: List<Event>? = null
)

View file

@ -27,5 +27,5 @@ internal data class RoomSyncState(
/**
* List of state events (array of Event). The resulting state corresponds to the *start* of the timeline.
*/
@Json(name = "events") val events: List<Event>? = emptyList()
@Json(name = "events") val events: List<Event>? = null
)

View file

@ -27,7 +27,7 @@ internal data class RoomSyncTimeline(
/**
* List of events (array of Event).
*/
@Json(name = "events") val events: List<Event>? = emptyList(),
@Json(name = "events") val events: List<Event>? = null,
/**
* Boolean which tells whether there are more events on the server