Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Int vs Bool confusion for the fields returned by the users admin API #16733

Open
spantaleev opened this issue Dec 7, 2023 · 5 comments
Open

Comments

@spantaleev
Copy link
Contributor

Description

The List Accounts User Admin API (GET /_synapse/admin/v2/users) used to be returning certain boolean-like fields as integers (e.g. "is_guest": 0).

The v1.97.0 documentation page shows examples that use integers (e.g. "is_guest": 0) while actually saying something like:

The following fields are returned in the JSON response body:
...
is_guest - bool - Status if that user is a guest account.

This is confusing - is it an integer or a boolean that we're receiving?

However, going back to the documentation for an ancient version (like v1.35.0), one can read the same exact thing.

On that old version, I believe the API was definitely returning integers, not booleans.

However, on v1.97.0 (at least.. possibly even earlier), the API seems to have started returning booleans. It may be an accidental change. It may have been announced in some release notes and I may have missed it.

Nevertheless, I believe the documentation page is confusing (is it a bool or an integer?) and potentially out of date now - it needs fixes.

Related to: devture/matrix-corporal#30

Steps to reproduce

  1. Run Synapse v1.97.0
  2. Use the GET /_synapse/admin/v2/users API with an admin token
  3. Observe that various fields (is_guest, admin, deactivated, shadow_banned, approved, erased, locked) all a boolean type of value (false or true). On previous versions, integers (0, 1) were used.

Homeserver

another homeserver

Synapse Version

1.97.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL (irrelevant)

Workers

Multiple workers

Platform

VM + containers

Configuration

No response

Relevant log output

None

Anything else that would be useful to know?

No response

@DMRobertson
Copy link
Contributor

In the past we've seen this sort of int-bool confusion happen on SQLite, because:

  • There were columns created as BOOLEAN in the schema
  • SQLite stores booleans as integers (0 and 1), whereas postgres stored booleans as booleans.
  • The database drivers would deserialise these to Python integers and Python booleans , respectively.
  • The deserialised objects get serialised to json.

I think what's happened here is different. Take is_guest as an example: it's stored as a smallint:

is_guest smallint DEFAULT 0 NOT NULL,
so was presumably going to be returned as an integer... until recently. Let me see if I can dig up what changed...

@dklimpel
Copy link
Contributor

dklimpel commented Dec 7, 2023

Duplicate to #13519

@hermanniscoding
Copy link

hermanniscoding commented Dec 7, 2023

Duplicate to #13519

Maybe related, but not a duplicate I think, since this behavior is new: GET /_synapse/admin/v2/users is in fact sending bool values for is_guest and admin in the response instead of int, which is causing bugs in software expecting int values as documented.

@clokep
Copy link
Contributor

clokep commented Dec 7, 2023

It was probably one of the removal of cursor_to_dict PRs. bool is more appropriate though and I'd recommend updating the docs.

@hermanniscoding
Copy link

It was probably one of the removal of cursor_to_dict PRs. bool is more appropriate though and I'd recommend updating the docs.

Those values being bool absolutely makes sense. Can we assume this change won't be reverted and updating the docs is the way to go?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants