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

Feature/get space hierarchy cache #1885

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Feature/get space hierarchy cache #1885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 34 additions & 0 deletions lib/src/client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2194,6 +2194,22 @@ class Client extends MatrixApi {
EventUpdateType.accountData,
);
}

// invalidate _getSpaceHierarchyResponseCache
if (room.spaceParents.isNotEmpty) {
for (final space in room.spaceParents) {
final String? spaceId = space.roomId;
if (spaceId == null) continue;
if (spaceId == room.id) continue;
final Room? spaceAsRoom = getRoomById(spaceId);
if (spaceAsRoom == null) continue;
if (spaceAsRoom.spaceChildren
.map((child) => child.roomId)
.contains(room.id)) {
await database?.removeSpaceHierarchy(spaceId);
}
}
}
}

if (syncRoomUpdate is LeftRoomUpdate) {
Expand Down Expand Up @@ -3323,6 +3339,24 @@ class Client extends MatrixApi {
waitUntilLoadCompletedLoaded: false,
);
}

@override
Future<GetSpaceHierarchyResponse> getSpaceHierarchy(String roomId,
Copy link
Member

Choose a reason for hiding this comment

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

You seem to just be caching the whole response. This can introduce some weirdness if you mix cached and uncached responses. Additionally such responses shouldn't be cached indefinitely, I would say.

You also aren't indexing your cache entries by the other parameters for the request, which means for spaces with more than "limit" entries in the response, you will be returning the wrong response. You will also return the wrong response if any of the parameters (apart from the roomid) changes. Additionally you aren't invalidating the cache if a subspace member changes, which means for any depth > 1 this will yield wrong results, if the subspace changes.

I think this needs a lot more tests and quite a bit of work on the cache invalidation logic. While it would certainly be useful to have more efficient access to the space hierarchy, we don't want to return wrong results.

{bool? suggestedOnly, int? limit, int? maxDepth, String? from}) async {
final cachedResponse = await database?.getSpaceHierarchy(roomId);
if (cachedResponse == null) {
final response = await super.getSpaceHierarchy(
roomId,
suggestedOnly: suggestedOnly,
limit: limit,
maxDepth: maxDepth,
from: from,
);
await database?.storeSpaceHierarchy(roomId, response);
return response;
}
return cachedResponse;
}
}

class SdkError {
Expand Down
7 changes: 7 additions & 0 deletions lib/src/database/database_api.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,13 @@ abstract class DatabaseApi {

Future<CachedPresence?> getPresence(String userId);

Future<GetSpaceHierarchyResponse?> getSpaceHierarchy(String spaceId);

Future<void> storeSpaceHierarchy(
String spaceId, GetSpaceHierarchyResponse hierarchy);

Future<void> removeSpaceHierarchy(String spaceId);

/// Deletes the whole database. The database needs to be created again after
/// this. Used for migration only.
Future<void> delete();
Expand Down
32 changes: 32 additions & 0 deletions lib/src/database/hive_collections_database.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ class HiveCollectionsDatabase extends DatabaseApi {

late CollectionBox<String> _seenDeviceKeysBox;

late CollectionBox<GetSpaceHierarchyResponse> _spacesHierarchyBox;

String get _clientBoxName => 'box_client';

String get _accountDataBoxName => 'box_account_data';
Expand Down Expand Up @@ -127,6 +129,8 @@ class HiveCollectionsDatabase extends DatabaseApi {

String get _seenDeviceKeysBoxName => 'box_seen_device_keys';

static const String _spacesHierarchyBoxName = 'box_spaces_hierarchy';

HiveCollectionsDatabase(
this.name,
this.path, {
Expand Down Expand Up @@ -160,6 +164,7 @@ class HiveCollectionsDatabase extends DatabaseApi {
_eventsBoxName,
_seenDeviceIdsBoxName,
_seenDeviceKeysBoxName,
_spacesHierarchyBoxName,
},
key: key,
path: path,
Expand Down Expand Up @@ -226,6 +231,9 @@ class HiveCollectionsDatabase extends DatabaseApi {
_seenDeviceKeysBox = await _collection.openBox(
_seenDeviceKeysBoxName,
);
_spacesHierarchyBox = await _collection.openBox(
_spacesHierarchyBoxName,
);

// Check version and check if we need a migration
final currentVersion = int.tryParse(await _clientBox.get('version') ?? '');
Expand Down Expand Up @@ -280,6 +288,7 @@ class HiveCollectionsDatabase extends DatabaseApi {
await _outboundGroupSessionsBox.clear();
await _presencesBox.clear();
await _clientBox.delete('prev_batch');
await _spacesHierarchyBox.clear();
});

@override
Expand Down Expand Up @@ -326,6 +335,11 @@ class HiveCollectionsDatabase extends DatabaseApi {
if (multiKey.parts.first != roomId) continue;
await _roomAccountDataBox.delete(key);
}
final spaceHierarchyKeys = await _spacesHierarchyBox.getAllKeys();
for (final key in spaceHierarchyKeys) {
if (key != roomId) continue;
await _spacesHierarchyBox.delete(key);
}
await _roomsBox.delete(roomId);
});

Expand Down Expand Up @@ -1517,6 +1531,7 @@ class HiveCollectionsDatabase extends DatabaseApi {
_eventsBoxName: await _eventsBox.getAllValues(),
_seenDeviceIdsBoxName: await _seenDeviceIdsBox.getAllValues(),
_seenDeviceKeysBoxName: await _seenDeviceKeysBox.getAllValues(),
_spacesHierarchyBoxName: await _spacesHierarchyBox.getAllValues(),
};
final json = jsonEncode(dataMap);
await clear();
Expand Down Expand Up @@ -1588,13 +1603,30 @@ class HiveCollectionsDatabase extends DatabaseApi {
for (final key in json[_seenDeviceKeysBoxName]!.keys) {
await _seenDeviceKeysBox.put(key, json[_seenDeviceKeysBoxName]![key]);
}
for (final key in json[_spacesHierarchyBoxName]!.keys) {
await _spacesHierarchyBox.put(key, json[_spacesHierarchyBoxName]![key]);
}
return true;
} catch (e, s) {
Logs().e('Database import error: ', e, s);
return false;
}
}

@override
Future<GetSpaceHierarchyResponse?> getSpaceHierarchy(String spaceId) async {
return await _spacesHierarchyBox.get(spaceId);
}

@override
Future<void> storeSpaceHierarchy(
String spaceId, GetSpaceHierarchyResponse hierarchy) =>
_spacesHierarchyBox.put(spaceId, hierarchy);

@override
Future<void> removeSpaceHierarchy(String spaceId) =>
_spacesHierarchyBox.delete(spaceId);

@override
Future<void> delete() => _collection.deleteFromDisk();
}
Expand Down
29 changes: 29 additions & 0 deletions lib/src/database/hive_database.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {

late LazyBox _seenDeviceKeysBox;

late LazyBox<GetSpaceHierarchyResponse> _spacesHierarchyBox;

String get _clientBoxName => '$name.box.client';

String get _accountDataBoxName => '$name.box.account_data';
Expand Down Expand Up @@ -124,6 +126,8 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {

String get _seenDeviceKeysBoxName => '$name.box.seen_device_keys';

static const String _spacesHierarchyBoxName = 'box_spaces_hierarchy';

final HiveCipher? encryptionCipher;

FamedlySdkHiveDatabase(this.name, {this.encryptionCipher});
Expand Down Expand Up @@ -152,6 +156,7 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {
action(_eventsBox),
action(_seenDeviceIdsBox),
action(_seenDeviceKeysBox),
action(_spacesHierarchyBox)
]);

Future<void> open() async {
Expand Down Expand Up @@ -232,6 +237,11 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {
encryptionCipher: encryptionCipher,
);

_spacesHierarchyBox = await Hive.openLazyBox(
_spacesHierarchyBoxName,
encryptionCipher: encryptionCipher,
);

// Check version and check if we need a migration
final currentVersion = (await _clientBox.get('version') as int?);
if (currentVersion == null) {
Expand Down Expand Up @@ -295,6 +305,7 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {
await _outboundGroupSessionsBox.deleteAll(_outboundGroupSessionsBox.keys);
await _presencesBox.deleteAll(_presencesBox.keys);
await _clientBox.delete('prev_batch');
await _spacesHierarchyBox.clear();
}

@override
Expand Down Expand Up @@ -339,6 +350,10 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {
if (multiKey.parts.first != roomId) continue;
await _roomAccountDataBox.delete(key);
}
for (final key in _spacesHierarchyBox.keys) {
if (key != roomId) continue;
await _spacesHierarchyBox.delete(key);
}
await _roomsBox.delete(roomId.toHiveKey);
}

Expand Down Expand Up @@ -1401,6 +1416,20 @@ class FamedlySdkHiveDatabase extends DatabaseApi with ZoneTransactionMixin {
throw UnimplementedError();
}

@override
Future<GetSpaceHierarchyResponse?> getSpaceHierarchy(String spaceId) async {
return await _spacesHierarchyBox.get(spaceId);
}

@override
Future<void> storeSpaceHierarchy(
String spaceId, GetSpaceHierarchyResponse hierarchy) =>
_spacesHierarchyBox.put(spaceId, hierarchy);

@override
Future<void> removeSpaceHierarchy(String spaceId) =>
_spacesHierarchyBox.delete(spaceId);

@override
Future<void> delete() => Hive.deleteFromDisk();
}
Expand Down
34 changes: 34 additions & 0 deletions lib/src/database/matrix_sdk_database.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {

late Box<String> _seenDeviceKeysBox;

late Box<Map> _spacesHierarchyBox;

@override
final int maxFileSize;

Expand Down Expand Up @@ -159,6 +161,8 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {

static const String _seenDeviceKeysBoxName = 'box_seen_device_keys';

static const String _spacesHierarchyBoxName = 'box_spaces_hierarchy';

Database? database;

/// Custom IdbFactory used to create the indexedDB. On IO platforms it would
Expand Down Expand Up @@ -214,6 +218,7 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
_eventsBoxName,
_seenDeviceIdsBoxName,
_seenDeviceKeysBoxName,
_spacesHierarchyBoxName,
},
sqfliteDatabase: database,
sqfliteFactory: sqfliteFactory,
Expand Down Expand Up @@ -283,6 +288,9 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
_seenDeviceKeysBox = _collection.openBox(
_seenDeviceKeysBoxName,
);
_spacesHierarchyBox = _collection.openBox(
_spacesHierarchyBoxName,
);

// Check version and check if we need a migration
final currentVersion = int.tryParse(await _clientBox.get('version') ?? '');
Expand Down Expand Up @@ -341,6 +349,7 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
await _outboundGroupSessionsBox.clear();
await _presencesBox.clear();
await _clientBox.delete('prev_batch');
await _spacesHierarchyBox.clear();
});

@override
Expand Down Expand Up @@ -389,6 +398,11 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
if (multiKey.parts.first != roomId) continue;
await _roomAccountDataBox.delete(key);
}
final spaceHierarchyKeys = await _spacesHierarchyBox.getAllKeys();
for (final key in spaceHierarchyKeys) {
if (key != roomId) continue;
await _spacesHierarchyBox.delete(key);
}
await _roomsBox.delete(roomId);
}

Expand Down Expand Up @@ -1477,6 +1491,7 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
_eventsBoxName: await _eventsBox.getAllValues(),
_seenDeviceIdsBoxName: await _seenDeviceIdsBox.getAllValues(),
_seenDeviceKeysBoxName: await _seenDeviceKeysBox.getAllValues(),
_spacesHierarchyBoxName: await _spacesHierarchyBox.getAllValues(),
};
final json = jsonEncode(dataMap);
await clear();
Expand Down Expand Up @@ -1557,6 +1572,9 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
for (final key in json[_seenDeviceKeysBoxName]!.keys) {
await _seenDeviceKeysBox.put(key, json[_seenDeviceKeysBoxName]![key]);
}
for (final key in json[_spacesHierarchyBoxName]!.keys) {
await _spacesHierarchyBox.put(key, json[_spacesHierarchyBoxName]![key]);
}
return true;
} catch (e, s) {
Logs().e('Database import error: ', e, s);
Expand Down Expand Up @@ -1613,6 +1631,22 @@ class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage {
return CachedPresence.fromJson(copyMap(rawPresence));
}

@override
Future<GetSpaceHierarchyResponse?> getSpaceHierarchy(String spaceId) async {
final raw_space_hierarchy = await _spacesHierarchyBox.get(spaceId);
if (raw_space_hierarchy == null) return null;
return GetSpaceHierarchyResponse.fromJson(copyMap(raw_space_hierarchy));
}

@override
Future<void> storeSpaceHierarchy(
String spaceId, GetSpaceHierarchyResponse hierarchy) =>
_spacesHierarchyBox.put(spaceId, hierarchy.toJson());

@override
Future<void> removeSpaceHierarchy(String spaceId) =>
_spacesHierarchyBox.delete(spaceId);

@override
Future<void> delete() => BoxCollection.delete(
name,
Expand Down