perf: remove `contacts` from `FullChat`

[WofWca]

• Operating System (Linux/Mac/Windows/iOS/Android): All
• Core Version: 1.160.0
• Client Version: 1.59.2

Expected behavior

get_full_chat_by_id is fast-ish

Actual behavior

get_full_chat_by_id takes ~1 second to complete for a chat with 1000 members.

With DC desktop 1.59.2 the effect is that opening such a chat and loading its messages takes 1 second. May be even more on slower machines, or HDDs.

Steps to reproduce the problem

1. Call get_full_chat_by_id

---

Although I haven't measured this exactly, but I imagine this part is pretty slow:

core/deltachat-jsonrpc/src/api/types/chat.rs

Lines 68 to 80 in 8dcd8aa

	let mut contacts = Vec::with_capacity(contact_ids.len());
	for contact_id in &contact_ids {
	contacts.push(
	ContactObject::try_from_dc_contact(
	context,
	Contact::get_by_id(context, *contact_id)
	.await
	.context("failed to load contact")?,
	)
	.await?,
	)
	}

IMO providing contact_ids is enough, and loading the individual contacts can usually be performed separately.

I would have made an MR, but I wanted to discuss the fact that it's a breaking change. The most conservative course of action would be to introduce a new RPC method called e.g. get_full_chat_by_id_2 and a new type, e.g. FullChat2, and mark the old ones as deprecated, but I wanted to check in about whether this is too ugly of a name.
And even if we do introduce it, when would be the time to remove the deprecated functions and rename the new ones? Are we ever gonna have the courage to start using semver and release a major version?

But this is probably not that significant of an issue and e.g. solving deltachat/deltachat-desktop#5235 could mitigate the effects of it.

[iequidoo]

Indeed, i'd also remove it. It's not clear why it's needed because e.g. on my screen there can be a maximum of 13 contacts (in the group info). A new type isn't really needed, we can comment contacts as deprecated and add a new function that leaves it empty. Who uses this API besides DC Desktop?

Alternatively we can add smth like InnerContext::api_version (which is accessible from Context) instead of adding new functions. But if some functions don't take the Context parameter, that won't help for them, so this approach isn't generic.
... But i think adding Context to all functions eventually isn't that bad. It also may be used for logging.

[WofWca]

we can comment contacts as deprecated and add a new function that leaves it empty.

You mean turning it into an Option<...>? Or keeping as is, but returning an empty array / list?

[iequidoo]

I think neither one nor the other. Now i think we should use Cargo's "features" or some compile-time API version and only add contacts in the older version.

[WofWca]

WDYM? What older version? We publish the RPC to NPM pre-compiled, how would one configure this?

[iequidoo]

WDYM? What older version? We publish the RPC to NPM pre-compiled, how would one configure this?

Ok, then leaving contacts empty is fine. If it's not requested by the caller, that looks safe. As for adding a new function vs api_version, i'm not sure. Sometimes we still need to add new functions because of signature changes.

[WofWca]

api_version

How would that look from the JSON-RPC POV? You mean an extra function argument?

[iequidoo]

I thought of smth like CommandApi::require_version(usize), but such an approach isn't generic, it doesn't help with signature changes, so maybe just rename FullChat to FullChat2 so that all users are warned and add try_from_dc_chat_id_v1() with the old behavior?