feat: `impl SearchableEntity for PersistedMlsGroup` [WPB-22945] by coriolinus · Pull Request #1778 · wireapp/core-crypto

coriolinus · 2026-01-23T08:51:01Z

What's new in this PR

Adds efficient search methods for groups by `parent_id`.

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

github-actions · 2026-01-23T08:55:55Z

Bencher Report

Branch	prgn/feat/22945-searchable-entity-persistedmlsgroup
Testbed	ubuntu-latest

⚠️ WARNING: No Threshold found!
Without a Threshold, no Alerts will ever be generated.
Latency (nanoseconds (ns))
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results

Benchmark	Latency	microseconds (µs)
Commit add f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	15,393.00 µs
Commit add f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	673.24 µs
Commit add f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	3,812.00 µs
Commit add f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	6,683.90 µs
Commit add f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	10,277.00 µs
Commit add f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	12,828.00 µs
Commit add f(number clients)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	983,100.00 µs
Commit add f(number clients)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	663.10 µs
Commit add f(number clients)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	78,839.00 µs
Commit add f(number clients)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	215,990.00 µs
Commit add f(number clients)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	426,100.00 µs
Commit add f(number clients)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	671,190.00 µs
Commit pending proposals f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	113,720.00 µs
Commit pending proposals f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	22,531.00 µs
Commit pending proposals f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	40,741.00 µs
Commit pending proposals f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	56,311.00 µs
Commit pending proposals f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	75,377.00 µs
Commit pending proposals f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	91,074.00 µs
Commit pending proposals f(pending size)/cs1/mem/1	📈 view plot ⚠️ NO THRESHOLD	15,144.00 µs
Commit pending proposals f(pending size)/cs1/mem/101	📈 view plot ⚠️ NO THRESHOLD	112,960.00 µs
Commit pending proposals f(pending size)/cs1/mem/21	📈 view plot ⚠️ NO THRESHOLD	32,298.00 µs
Commit pending proposals f(pending size)/cs1/mem/41	📈 view plot ⚠️ NO THRESHOLD	53,561.00 µs
Commit pending proposals f(pending size)/cs1/mem/61	📈 view plot ⚠️ NO THRESHOLD	72,495.00 µs
Commit pending proposals f(pending size)/cs1/mem/81	📈 view plot ⚠️ NO THRESHOLD	92,272.00 µs
Commit remove f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	10,051.00 µs
Commit remove f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	503.41 µs
Commit remove f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	1,881.10 µs
Commit remove f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	3,485.70 µs
Commit remove f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	5,562.30 µs
Commit remove f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	7,401.60 µs
Commit remove f(number clients)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	12,771.00 µs
Commit remove f(number clients)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	133,550.00 µs
Commit remove f(number clients)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	110,100.00 µs
Commit remove f(number clients)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	85,359.00 µs
Commit remove f(number clients)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	60,755.00 µs
Commit remove f(number clients)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	36,543.00 µs
Commit update f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	132,890.00 µs
Commit update f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	680.44 µs
Commit update f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	27,504.00 µs
Commit update f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	54,089.00 µs
Commit update f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	81,333.00 µs
Commit update f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	108,010.00 µs

🐰 View full continuous benchmarking report in Bencher

SimonThormeyer · 2026-01-27T09:24:09Z

Is there a test which tests the usage of the SearchableEntity trait yet?

SimonThormeyer

Great work!

…tedMlsGroup` Additionally, this cleans up the implementation a little, fixes a bug where the id wasn't de-hexed, and udpates the `child_groups` impl to be in terms of this implementation.

…tedMlsGroup`

Life is actually easier if we just implement the relevant methods directly on `PersistedMlsGroup`, which is perfectly legal. I suspect that someone was overthinking things when they introduced the trait.

This means that we use a database-first interface to search for entities as we do for all other accesses, instead of the entity-first view which requires us to extract the connection from the DB.

…ance This is necessary so that the transaction implementation can work properly.

…oup`

…in the tx buffer

…the transaction

This causes the field to bypass encryption and decryption in the `Encrypting` and `Decrypting` impls. This is necessary for a field to be used as an index in wasm; for obvious reasons, looking up a field by an encrypted index always fails.

This is necessary so that we can use the `parent_id` field as an index.

…st store name We adjust the JsValue serializer to ensure that `Vec<u8>` gets mapped to `Uint8Array`. This is necessary in order to ensure that those idb items can be index keys: `Uint8Array` is a valid indexable type, but a generic `Array` (even if it contains only `Number`) is not. We also set things up to optionally use a constant test store name instead of generating one because that's much easier to follow along with in a test browser. This depends on a constant instead of being a function parameter because the interaction between fixtures and rstest_reuse applications is confusing, and adding a parameter there is more confusing than not.

coriolinus changed the title ~~feat; impl SearchableEntity for PersistedMlsGroup [WPB-22945]~~ feat: impl SearchableEntity for PersistedMlsGroup [WPB-22945] Jan 23, 2026

coriolinus force-pushed the prgn/feat/22945-searchable-entity-persistedmlsgroup branch from 89bd13c to f9cdecb Compare January 23, 2026 16:21

coriolinus marked this pull request as ready for review January 23, 2026 16:22

coriolinus requested a review from a team January 23, 2026 16:22

coriolinus force-pushed the prgn/feat/22945-searchable-entity-persistedmlsgroup branch 2 times, most recently from 2853689 to 2ddca34 Compare January 26, 2026 12:19

SimonThormeyer reviewed Jan 27, 2026

View reviewed changes

Comment thread keystore/src/connection/platform/wasm/migrations/mod.rs

SimonThormeyer reviewed Jan 27, 2026

View reviewed changes

Comment thread keystore/src/entities/platform/generic/mls/group.rs Outdated

coriolinus force-pushed the prgn/feat/22945-searchable-entity-persistedmlsgroup branch 2 times, most recently from d52bb34 to 41e5a6e Compare January 29, 2026 11:47

SimonThormeyer approved these changes Jan 29, 2026

View reviewed changes

coriolinus added 17 commits January 29, 2026 14:50

chore(keystore): add struct ParentGroupId

0bc025e

chore(keystore): sql: add index on parent_id for mls_groups

72c168d

feat(keystore): sql: `impl SearchableEntity<ParentGroupid> for Persis…

171e530

…tedMlsGroup` Additionally, this cleans up the implementation a little, fixes a bug where the id wasn't de-hexed, and udpates the `child_groups` impl to be in terms of this implementation.

chore(keystore): idb: add index on parent_id for mls_groups

49e923b

feat(keystore): idb: `impl SearchableEntity<ParentGroupId> for Persis…

bd6cca5

…tedMlsGroup`

chore(keystore): rm pointless, inefficient default child_groups impl

53fa6e9

chore(keystore): rm trait PersistedMlsGroupExt

d7041a5

Life is actually easier if we just implement the relevant methods directly on `PersistedMlsGroup`, which is perfectly legal. I suspect that someone was overthinking things when they introduced the trait.

fix(keystore): replace buggy test with a simpler one which works

d0b2f9c

chore: fix ordering of wasm migrations

75fa4d9

feat(keystore): impl searching for searchable entities on the database

1d5e9f9

This means that we use a database-first interface to search for entities as we do for all other accesses, instead of the entity-first view which requires us to extract the connection from the DB.

test(keystore): add tests for searchable entity for persistedmlsgroup

a0ee52d

feat(keystore): SearchableEntitys know if they match an entity inst…

48fb5aa

…ance This is necessary so that the transaction implementation can work properly.

chore(keystore): impl SearchableEntity::matches for `PersistedMlsGr…

a61aec2

…oup`

fix(keystore): searching for entities considers uncommitted entities …

9ce9e91

…in the tx buffer

test(keystore): add tests for searching with uncommitted entities in …

285ca8a

…the transaction

chore(keystore): fix fmt, import errors

6324f4a

coriolinus added 3 commits January 29, 2026 14:50

refactor(keystore): do not encrypt PersistedMlsGroup::parent_id

b93e69b

This is necessary so that we can use the `parent_id` field as an index.

feat(keystore): add wasm-only migration to store parent ids as decrypted

1fadc35

coriolinus force-pushed the prgn/feat/22945-searchable-entity-persistedmlsgroup branch from a2f5ab8 to d8ba061 Compare January 29, 2026 13:50

coriolinus merged commit d8ba061 into main Jan 29, 2026
8 checks passed

coriolinus deleted the prgn/feat/22945-searchable-entity-persistedmlsgroup branch January 29, 2026 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `impl SearchableEntity for PersistedMlsGroup` [WPB-22945]#1778

feat: `impl SearchableEntity for PersistedMlsGroup` [WPB-22945]#1778
coriolinus merged 20 commits intomainfrom
prgn/feat/22945-searchable-entity-persistedmlsgroup

coriolinus commented Jan 23, 2026

Uh oh!

github-actions Bot commented Jan 23, 2026 •

edited

Loading

⚠️ WARNING: No Threshold found!

Uh oh!

Uh oh!

Uh oh!

SimonThormeyer commented Jan 27, 2026

Uh oh!

SimonThormeyer left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

coriolinus commented Jan 23, 2026

What's new in this PR

Adds efficient search methods for groups by parent_id.

PR Submission Checklist for internal contributors

Uh oh!

github-actions Bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bencher Report

⚠️ WARNING: No Threshold found!

Uh oh!

Uh oh!

Uh oh!

SimonThormeyer commented Jan 27, 2026

Uh oh!

SimonThormeyer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Adds efficient search methods for groups by `parent_id`.

github-actions Bot commented Jan 23, 2026 •

edited

Loading