MIM-2749 muc_light: add listRooms admin GraphQL query#4744
Conversation
…on and filtering
Add a backend operation for listing the rooms of a MUC Light domain,
paginated with a keyset cursor so that each page costs O(page) instead of
O(domain): the backend returns at most Limit rooms ordered by the room
localpart, positioned strictly after the After cursor, together with the
total count of rooms matching the filter. The optional filter is a
case-insensitive substring match against the room localpart or the room
name, applied in the backend so that callers can search without loading
the whole domain.
The RDBMS backend pages room ids with a keyset select on the
(lserver, luser) primary key and fetches config and affiliations per page
room. A config that no longer decodes against the current config_schema
(e.g. created before a schema change) is logged and degraded to an empty
config instead of failing the whole listing, and a room destroyed between
the page select and the per-room fetch is skipped. Affiliation rows are
decoded with the existing decode_affs/1, keeping the sorted aff_users
invariant.
The mnesia backend filters and slices its table scan in place; the match
spec is built with ets:fun2ms, as the typed #muc_light_room{} record does
not admit '_' wildcards under dialyzer.
Expose the domain-wide room listing through the API layer: get_rooms(MUCServer, Filter, After, Limit) validates the MUC domain, namepreps the input so backends always receive the normalized lserver, and returns a page of room descriptions together with the total matching count and the next-page cursor. The API fetches one extra room from the backend to tell whether a next page exists, so an exactly-full last page reports no cursor instead of making the client fetch an empty page. Each room description carries the room JID, name, subject, member count and the owner (undefined when no owner affiliation is resolvable, so a legacy room cannot fail the listing). Name and subject are resolved through the config schema, as the stored config is keyed by the internal schema key, which may differ from the field name when config_schema customizes internal_key. Room JIDs are built with jid:make_noprep, following the convention for data already prepped at write time.
…nd filter Add muc_light.listRooms(mucDomain, filter, after, limit = 50) to the admin schema, filling the gap next to the classic muc.listRooms: until now MUC Light rooms could only be discovered per user (listUserRooms) or per room JID (getRoomConfig), with no domain-wide enumeration. The listing is paginated with a keyset cursor rather than an index offset: the payload returns rooms ordered by room JID plus a nextCursor to pass back as 'after', null when there are no more rooms. This keeps every page O(page) on the server and stable under concurrent room creation and deletion, whereas an offset would require fetching and sorting the whole domain per page; classic MUC can afford offset semantics because it lists online rooms from memory, while MUC Light rooms live in the database (the same trade-off that gives MAM cursor-based paging). The limit defaults to 50, so omitting it does not serialize an entire domain into one response. The optional filter selects rooms whose JID localpart or name contains the given string, case-insensitively, and is reflected in the returned count. Each room description carries jid, name, subject, usersNumber and a nullable ownerJid, so callers get the member count and owner without a follow-up query per room.
|
CircleCI results for 0eb4897 small_tests_latest / small_tests / 414f0ac small_tests_legacy / small_tests / 414f0ac small_tests_latest_arm64 / small_tests / 414f0ac ldap_mnesia_latest / ldap_mnesia / 414f0ac internal_mnesia_latest / internal_mnesia / 414f0ac dynamic_domains_pgsql_cets_legacy / pgsql_cets / 414f0ac dynamic_domains_pgsql_cets_latest / pgsql_cets / 414f0ac pgsql_cets_legacy / pgsql_cets / 414f0ac pgsql_redis_latest / pgsql_redis / 414f0ac pgsql_cets_latest / pgsql_cets / 414f0ac mysql_cets_latest / mysql_cets / 414f0ac elasticsearch_and_cassandra_mnesia_latest / elasticsearch_and_cassandra_mnesia / 414f0ac mongoose_elasticsearch_SUITE:end_per_suite{error,
{{badrpc,
{'EXIT',
{#{what => event_already_registered,
event_name => wpool_global_queue_lengths,
labels => #{pool_type => elastic,pool_tag => default}},
[{mongoose_instrument,set_up,3,
[{file,
"/home/circleci/project/src/instrument/mongoose_instrument.erl"},
{line,114}]},
{lists,foreach_1,2,[{file,"lists.erl"},{line,2641}]},
{mongoose_wpool,start,5,
[{file,
"/home/circleci/project/src/wpool/mongoose_wpool.erl"},
{line,162}]},
{mongoose_wpool,'-start_configured_pools/3-lc$^1/1-1-',1,
[{file,
"/home/circleci/project/src/wpool/mongoose_wpool.erl"},
{line,129}]},
{mongoose_wpool,start_configured_pools,1,[]}]}}},
[{distributed_helper,rpc,
[#{node => mongooseim@localhost},
mongoose_wpool,start_configured_pools,
[[#{scope => global,tag => default,type => elastic,
opts =>
#{strategy => best_worker,workers => 10,
call_timeout => 5000},
conn_opts => #{port => 9200,host => <<"localhost">>}}]]],
[{file,
"/home/circleci/project/big_tests/../test/common/distributed_helper.erl"},
{line,143}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1796}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1393}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1237}]}]}...cockroachdb_cets_latest / cockroachdb_cets / 414f0ac pgsql_mnesia_latest / pgsql_mnesia / 414f0ac |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4744 +/- ##
==========================================
+ Coverage 86.89% 87.08% +0.19%
==========================================
Files 554 554
Lines 33385 33471 +86
==========================================
+ Hits 29009 29148 +139
+ Misses 4376 4323 -53 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Cover the admin listRooms query end to end: domain listing with names, subjects, member counts and owners; cursor pagination including the exactly-full last page and paging past the last room; filtering by JID localpart and by room name (case-insensitively), composed with the cursor and reflected in the count; ownerJid = null for a room without an owner affiliation (written straight to the backend, as the regular API cannot create one); and the error for a non-existent MUC Light domain. Degraded configurations are covered too: an empty filter behaves like no filter, a room stored without any config is matched by JID but never by name, a config_schema without the roomname field yields name = null and name filters that cannot match, a stored config option that no longer decodes against the current schema (corrupted via raw SQL, as the regular API cannot produce one) degrades that room instead of failing the listing, and a domain failing nameprep is rejected by the Erlang API directly, as the GraphQL DomainName scalar rejects such input before the resolver. The corrupted-config case is skipped on mnesia, which does not re-decode stored configs, and on the CLI, where the decode warning is forwarded into the command output and breaks its JSON. The listing tests clear the database first, as domain-wide counts and page boundaries would otherwise depend on rooms left over by earlier test cases, and the tests using fixed room names must not fail on leftovers of an aborted previous run.
0eb4897 to
6d801d6
Compare
There was a problem hiding this comment.
The functionality looks OK in general, but I am not convinced about the many places where it tries to be "smarter" that the existing code, breaking consistency and introducing complexity.
A lot of new concepts, defensive checks and naming schemes are introduced only for this particular API call, breaking consistency with existing operations while making the code and tests bloated and difficult to reason about.
A suggestion to the implementer: whenever you introduce a new convention just for one operation, ask yourself the question: is my operation special or unique? Because if it's not, it's implementation shouldn't be unique either.
| MS = ets:fun2ms(fun(#muc_light_room{room = {_, RoomS}} = Room) | ||
| when RoomS =:= MUCServer -> Room end), |
There was a problem hiding this comment.
I think you should be able to encode the After condition in this MS as well to avoid fetching all rooms at once.
There was a problem hiding this comment.
Did the change, but we still need to do the second dirty_select for the count of all rooms, not sure if this version is better.
There was a problem hiding this comment.
Let's keep it as it is, especially since RDBMS is the recommended backen. As a note, mod_muc takes a similar approach to the first implementation and fetches everything.
Decode stored configs strictly when listing, exactly as get_info/2 does: an inconsistent database is an error worth surfacing, and one operation should not be lenient where its siblings are not. Split the muc_server validation into its own fold step instead of gluing a second clause onto check_muc_domain, and replace the nextCursor payload field with a plain nextPage boolean; the caller can take the JID of the last listed room itself. Drop the per-argument schema descriptions, which no other operation uses, folding the semantics into the operation description. In the mnesia backend, move the page cursor into the match specification, so rooms before the cursor are not selected at all, and count matching rooms with a separate select, mirroring the RDBMS backend split between the page and count queries.
Drop the corner-case tests exercising fabricated data: the member-only room without an owner, the config row corrupted via raw SQL, the raw backend room without a config, and the nameprep failure on the Erlang API. Rooms cannot reach these states through normal operation and such generic corner cases are not MUC Light's job to cover. Keep the custom-schema test, but set the schema up in init_per_testcase and restore it in end_per_testcase, as other suites do, instead of reconfiguring modules inside the test body. Adjust the remaining assertions to the nextPage boolean; the next page is requested by passing the JID of the last listed room as the cursor.
|
Thanks for the thorough review - all the points mentioned are valid. |
Note in the schema that ownerJid returns one of the owners, as several are possible with allow_multiple_owners. Fold the null handling into after_to_luser/1, so a single conversion suffices in the resolver, and drop redundant comments.
| @@ -171,7 +171,7 @@ get_user_rooms(UserJID) -> | |||
| get_rooms(MUCServer, Filter, After, Limit) -> | |||
There was a problem hiding this comment.
I'm not sure about changing the name from MUC(Light)Domain to MUCServer. If you look at the analogous argument in create_room/3, the name remains the same.
| MS = ets:fun2ms(fun(#muc_light_room{room = {_, RoomS}} = Room) | ||
| when RoomS =:= MUCServer -> Room end), |
There was a problem hiding this comment.
Let's keep it as it is, especially since RDBMS is the recommended backen. As a note, mod_muc takes a similar approach to the first implementation and fetches everything.
4 participants
MIM-2749
muc_light: add
listRoomsadmin GraphQL queryThe admin GraphQL API had no way to enumerate the MUC Light rooms of a domain — unlike classic MUC (
muc.listRooms), rooms could only be discovered per user (listUserRooms) or per room JID (getRoomConfig). This PR adds a domain-wide, paginated room listing to themuc_lightadmin category.API
To iterate: call without
after, then repeat passing the returnednextCursoruntil it isnull. Each room description carries the member count and owner, so callers don't need an N+1 follow-up query per room.Design notes
(lserver, luser)primary key, and stays stable under concurrent room creation/deletion (no skipped or duplicated rooms between pages). Classic MUC can afford offset semantics because it lists online rooms from memory; MUC Light rooms live in the database — the same trade-off that gives MAM cursor-based paging.filter— case-insensitive substring match against the room JID localpart or the room name, applied in the backend (LIKEwith proper escaping in RDBMS) so clients can search without loading the whole domain. It composes with pagination and is reflected incount.limitdefaults to 50, so omitting it does not serialize an entire domain into one response.ownerJidis null when no owner affiliation is resolvable, and a stored config that no longer decodes against the currentconfig_schemais logged and degraded to an empty config. Customconfig_schemaentries with a distinctinternal_keyare handled by resolving name/subject through the schema.Implementation
get_room_descs/5(filter, cursor, limit → page + total count) with RDBMS and mnesia implementations. RDBMS pages room ids with a keyset select, then fetches config/affiliations per page room, reusingdecode_affs/1(keeps the sortedaff_usersinvariant); rooms destroyed between the selects are skipped. Mnesia filters and slices its table scan in place (ets:fun2ms, as the typed record does not admit'_'wildcards under dialyzer).mod_muc_light_api:get_rooms/4validates and namepreps the domain, fetcheslimit + 1rows to computenextCursoraccurately (an exactly-full last page reportsnull), and builds the room descriptions.