fix(core)!: improved min chats handling

breaking:
- `IPeersRepository.PeerInfo` has new field `isMin`
- `getById` has a new argument `allowMin` describing whether it's allowed to return peers where `.isMin == true`
- `getByUsername`, `getByPhone` changed logic: they should *never* return peers where `.isMin == true`
This commit is contained in:
alina 🌸 2024-09-28 21:40:57 +03:00
parent c92292249b
commit 4952d33261
Signed by: teidesu
SSH key fingerprint: SHA256:uNeCpw6aTSU4aIObXLvHfLkDa82HWH9EiOj9AXOIRpI
8 changed files with 116 additions and 29 deletions

View file

@ -137,6 +137,7 @@ export async function start(
return me return me
} catch (e) { } catch (e) {
console.log(e)
if (tl.RpcError.is(e)) { if (tl.RpcError.is(e)) {
if (e.text === 'SESSION_PASSWORD_NEEDED') has2fa = true if (e.text === 'SESSION_PASSWORD_NEEDED') has2fa = true
else if (e.text !== 'AUTH_KEY_UNREGISTERED') throw e else if (e.text !== 'AUTH_KEY_UNREGISTERED') throw e

View file

@ -6,7 +6,7 @@ import { getMarkedPeerId, parseMarkedPeerId, toggleChannelIdMark } from '../../.
import type { ITelegramClient } from '../../client.types.js' import type { ITelegramClient } from '../../client.types.js'
import { MtPeerNotFoundError } from '../../types/errors.js' import { MtPeerNotFoundError } from '../../types/errors.js'
import type { InputPeerLike } from '../../types/peers/index.js' import type { InputPeerLike } from '../../types/peers/index.js'
import { toInputChannel, toInputPeer, toInputUser } from '../../utils/peer-utils.js' import { extractUsernames, toInputChannel, toInputPeer, toInputUser } from '../../utils/peer-utils.js'
export function _normalizePeerId(peerId: InputPeerLike): number | string | tl.TypeInputPeer { export function _normalizePeerId(peerId: InputPeerLike): number | string | tl.TypeInputPeer {
// for convenience we also accept tl and User/Chat objects directly // for convenience we also accept tl and User/Chat objects directly
@ -161,6 +161,32 @@ export async function resolvePeer(
const [peerType, bareId] = parseMarkedPeerId(peerId) const [peerType, bareId] = parseMarkedPeerId(peerId)
if (!(peerType === 'chat' || client.storage.self.getCached(true)?.isBot)) { if (!(peerType === 'chat' || client.storage.self.getCached(true)?.isBot)) {
// we might have a min peer in cache, which we can try to resolve by its username/phone
const cached = await client.storage.peers.getCompleteById(peerId, true)
if (cached && (cached._ === 'channel' || cached._ === 'user')) {
// do we have a username?
const [username] = extractUsernames(cached)
if (username) {
const resolved = await resolvePeer(client, username, true)
// username might already be taken by someone else, so we need to check it
if (getMarkedPeerId(resolved) === peerId) {
return resolved
}
}
if (cached._ === 'user' && cached.phone) {
// try resolving by phone
const resolved = await resolvePeer(client, cached.phone, true)
if (getMarkedPeerId(resolved) === peerId) {
return resolved
}
}
}
throw new MtPeerNotFoundError(`Peer ${peerId} is not found in local cache`) throw new MtPeerNotFoundError(`Peer ${peerId} is not found in local cache`)
} }

View file

@ -8,6 +8,8 @@ export namespace IPeersRepository {
id: number id: number
/** Peer access hash, as a fast string representation */ /** Peer access hash, as a fast string representation */
accessHash: string accessHash: string
/** Whether the peer is a "min" peer */
isMin: boolean
/** Peer usernames, if any */ /** Peer usernames, if any */
usernames: string[] usernames: string[]
/** Timestamp (in seconds) when the peer was last updated */ /** Timestamp (in seconds) when the peer was last updated */
@ -26,11 +28,21 @@ export namespace IPeersRepository {
export interface IPeersRepository { export interface IPeersRepository {
/** Store the given peer */ /** Store the given peer */
store: (peer: IPeersRepository.PeerInfo) => MaybePromise<void> store: (peer: IPeersRepository.PeerInfo) => MaybePromise<void>
/** Find a peer by their `id` */ /**
getById: (id: number) => MaybePromise<IPeersRepository.PeerInfo | null> * Find a peer by their `id`.
/** Find a peer by their username (where `usernames` includes `username`) */ *
* @param allowMin Whether to allow "min" peers to be returned
*/
getById: (id: number, allowMin: boolean) => MaybePromise<IPeersRepository.PeerInfo | null>
/**
* Find a peer by their username (where `usernames` includes `username`).
* Should never return "min" peers
*/
getByUsername: (username: string) => MaybePromise<IPeersRepository.PeerInfo | null> getByUsername: (username: string) => MaybePromise<IPeersRepository.PeerInfo | null>
/** Find a peer by their `phone` */ /**
* Find a peer by their `phone`.
* Should never return "min" peers
*/
getByPhone: (phone: string) => MaybePromise<IPeersRepository.PeerInfo | null> getByPhone: (phone: string) => MaybePromise<IPeersRepository.PeerInfo | null>
deleteAll: () => MaybePromise<void> deleteAll: () => MaybePromise<void>

View file

@ -64,10 +64,12 @@ export class PeersService extends BaseService {
async updatePeersFrom(obj: tl.TlObject | tl.TlObject[]): Promise<boolean> { async updatePeersFrom(obj: tl.TlObject | tl.TlObject[]): Promise<boolean> {
let count = 0 let count = 0
let minCount = 0
for (const peer of getAllPeersFrom(obj)) { for (const peer of getAllPeersFrom(obj)) {
// no point in caching min peers as we can't use them if ((peer as Extract<typeof peer, { min?: unknown }>).min) {
if ((peer as Extract<typeof peer, { min?: unknown }>).min) continue minCount += 1
}
count += 1 count += 1
@ -76,7 +78,7 @@ export class PeersService extends BaseService {
if (count > 0) { if (count > 0) {
await this._driver.save?.() await this._driver.save?.()
this._log.debug('cached %d peers', count) this._log.debug('cached %d peers (%d min)', count, minCount)
return true return true
} }
@ -99,6 +101,7 @@ export class PeersService extends BaseService {
dto = { dto = {
id: peer.id, id: peer.id,
accessHash: longToFastString(peer.accessHash), accessHash: longToFastString(peer.accessHash),
isMin: peer.min! && !(peer.phone !== undefined && peer.phone.length === 0),
phone: peer.phone, phone: peer.phone,
usernames: extractUsernames(peer), usernames: extractUsernames(peer),
updated: Date.now(), updated: Date.now(),
@ -112,6 +115,7 @@ export class PeersService extends BaseService {
dto = { dto = {
id: -peer.id, id: -peer.id,
accessHash: '', accessHash: '',
isMin: false, // chats can't be "min"
updated: Date.now(), updated: Date.now(),
complete: this._serializeTl(peer), complete: this._serializeTl(peer),
usernames: [], usernames: [],
@ -130,6 +134,7 @@ export class PeersService extends BaseService {
dto = { dto = {
id: toggleChannelIdMark(peer.id), id: toggleChannelIdMark(peer.id),
accessHash: longToFastString(peer.accessHash), accessHash: longToFastString(peer.accessHash),
isMin: peer._ === 'channel' ? peer.min! : false,
usernames: extractUsernames(peer as tl.RawChannel), usernames: extractUsernames(peer as tl.RawChannel),
updated: Date.now(), updated: Date.now(),
complete: this._serializeTl(peer), complete: this._serializeTl(peer),
@ -193,7 +198,7 @@ export class PeersService extends BaseService {
const cached = this._cache.get(id) const cached = this._cache.get(id)
if (cached) return cached.peer if (cached) return cached.peer
const dto = await this._peers.getById(id) const dto = await this._peers.getById(id, false)
if (dto) { if (dto) {
return this._returnCaching(id, dto) return this._returnCaching(id, dto)
@ -248,11 +253,11 @@ export class PeersService extends BaseService {
return this._returnCaching(dto.id, dto) return this._returnCaching(dto.id, dto)
} }
async getCompleteById(id: number): Promise<tl.TypeUser | tl.TypeChat | null> { async getCompleteById(id: number, allowMin = false): Promise<tl.TypeUser | tl.TypeChat | null> {
const cached = this._cache.get(id) const cached = this._cache.get(id)
if (cached) return cached.complete if (cached) return cached.complete
const dto = await this._peers.getById(id) const dto = await this._peers.getById(id, allowMin)
if (!dto) return null if (!dto) return null
const cacheItem: CacheItem = { const cacheItem: CacheItem = {

View file

@ -42,22 +42,31 @@ export class MemoryPeersRepository implements IPeersRepository {
this.state.entities.set(peer.id, peer) this.state.entities.set(peer.id, peer)
} }
getById(id: number): IPeersRepository.PeerInfo | null { getById(id: number, allowMin: boolean): IPeersRepository.PeerInfo | null {
return this.state.entities.get(id) ?? null const ent = this.state.entities.get(id)
if (!ent || (ent.isMin && !allowMin)) return null
return ent
} }
getByUsername(username: string): IPeersRepository.PeerInfo | null { getByUsername(username: string): IPeersRepository.PeerInfo | null {
const id = this.state.usernameIndex.get(username.toLowerCase()) const id = this.state.usernameIndex.get(username.toLowerCase())
if (!id) return null if (!id) return null
return this.state.entities.get(id) ?? null const ent = this.state.entities.get(id)
if (!ent || ent.isMin) return null
return ent
} }
getByPhone(phone: string): IPeersRepository.PeerInfo | null { getByPhone(phone: string): IPeersRepository.PeerInfo | null {
const id = this.state.phoneIndex.get(phone) const id = this.state.phoneIndex.get(phone)
if (!id) return null if (!id) return null
return this.state.entities.get(id) ?? null const ent = this.state.entities.get(id)
if (!ent || ent.isMin) return null
return ent
} }
deleteAll(): void { deleteAll(): void {

View file

@ -6,6 +6,7 @@ import type { ISqliteStatement } from '../types.js'
interface PeerDto { interface PeerDto {
id: number id: number
hash: string hash: string
isMin: 1 | 0
usernames: string usernames: string
updated: number updated: number
phone: string | null phone: string | null
@ -16,6 +17,7 @@ function mapPeerDto(dto: PeerDto): IPeersRepository.PeerInfo {
return { return {
id: dto.id, id: dto.id,
accessHash: dto.hash, accessHash: dto.hash,
isMin: dto.isMin === 1,
usernames: JSON.parse(dto.usernames) as string[], usernames: JSON.parse(dto.usernames) as string[],
updated: dto.updated, updated: dto.updated,
phone: dto.phone || undefined, phone: dto.phone || undefined,
@ -41,18 +43,22 @@ export class SqlitePeersRepository implements IPeersRepository {
create index idx_peers_phone on peers (phone); create index idx_peers_phone on peers (phone);
`) `)
}) })
_driver.registerMigration('peers', 2, (db) => {
db.exec('alter table peers add column isMin integer not null default false;')
})
_driver.onLoad((db) => { _driver.onLoad((db) => {
this._loaded = true this._loaded = true
this._store = db.prepare( this._store = db.prepare(
'insert or replace into peers (id, hash, usernames, updated, phone, complete) values (?, ?, ?, ?, ?, ?)', 'insert or replace into peers (id, hash, isMin, usernames, updated, phone, complete) values (?, ?, ?, ?, ?, ?, ?)',
) )
this._getById = db.prepare('select * from peers where id = ?') this._getById = db.prepare('select * from peers where id = ? and isMin = false')
this._getByIdAllowMin = db.prepare('select * from peers where id = ?')
this._getByUsername = db.prepare( this._getByUsername = db.prepare(
'select * from peers where exists (select 1 from json_each(usernames) where value = ?)', 'select * from peers where exists (select 1 from json_each(usernames) where value = ?) and isMin = false',
) )
this._getByPhone = db.prepare('select * from peers where phone = ?') this._getByPhone = db.prepare('select * from peers where phone = ? and isMin = false')
this._delAll = db.prepare('delete from peers') this._delAll = db.prepare('delete from peers')
}) })
@ -77,6 +83,7 @@ export class SqlitePeersRepository implements IPeersRepository {
this._driver._writeLater(this._store, [ this._driver._writeLater(this._store, [
peer.id, peer.id,
peer.accessHash, peer.accessHash,
peer.isMin ? 1 : 0,
JSON.stringify(peer.usernames), JSON.stringify(peer.usernames),
peer.updated, peer.updated,
peer.phone ?? null, peer.phone ?? null,
@ -85,9 +92,10 @@ export class SqlitePeersRepository implements IPeersRepository {
} }
private _getById!: ISqliteStatement private _getById!: ISqliteStatement
getById(id: number): IPeersRepository.PeerInfo | null { private _getByIdAllowMin!: ISqliteStatement
getById(id: number, allowMin: boolean): IPeersRepository.PeerInfo | null {
this._ensureLoaded() this._ensureLoaded()
const row = this._getById.get(id) const row = (allowMin ? this._getByIdAllowMin : this._getById).get(id)
if (!row) return null if (!row) return null
return mapPeerDto(row as PeerDto) return mapPeerDto(row as PeerDto)

View file

@ -32,6 +32,7 @@ export function testPeersRepository(repo: IPeersRepository, driver: IStorageDriv
const stubPeerUser: IPeersRepository.PeerInfo = { const stubPeerUser: IPeersRepository.PeerInfo = {
id: 123123, id: 123123,
accessHash: '123|456', accessHash: '123|456',
isMin: false,
usernames: ['some_user'], usernames: ['some_user'],
phone: '78005553535', phone: '78005553535',
updated: 666, updated: 666,
@ -41,14 +42,17 @@ export function testPeersRepository(repo: IPeersRepository, driver: IStorageDriv
const stubPeerChannel: IPeersRepository.PeerInfo = { const stubPeerChannel: IPeersRepository.PeerInfo = {
id: -1001183945448, id: -1001183945448,
accessHash: '666|555', accessHash: '666|555',
isMin: false,
usernames: ['some_channel'], usernames: ['some_channel'],
updated: 777, updated: 777,
complete: TlBinaryWriter.serializeObject(__tlWriterMap, createStub('channel', { id: 123123 })), complete: TlBinaryWriter.serializeObject(__tlWriterMap, createStub('channel', { id: 123123 })),
} }
const stupPeerMinUser: IPeersRepository.PeerInfo = { ...stubPeerUser, isMin: true }
describe('peers', () => { describe('peers', () => {
it('should be empty by default', async () => { it('should be empty by default', async () => {
expect(await repo.getById(123123)).toEqual(null) expect(await repo.getById(123123, false)).toEqual(null)
expect(await repo.getByUsername('some_user')).toEqual(null) expect(await repo.getByUsername('some_user')).toEqual(null)
expect(await repo.getByPhone('phone')).toEqual(null) expect(await repo.getByPhone('phone')).toEqual(null)
}) })
@ -58,11 +62,11 @@ export function testPeersRepository(repo: IPeersRepository, driver: IStorageDriv
await repo.store(stubPeerChannel) await repo.store(stubPeerChannel)
await driver.save?.() await driver.save?.()
expect(fixPeerInfo(await repo.getById(123123))).toEqual(stubPeerUser) expect(fixPeerInfo(await repo.getById(123123, false))).toEqual(stubPeerUser)
expect(fixPeerInfo(await repo.getByUsername('some_user'))).toEqual(stubPeerUser) expect(fixPeerInfo(await repo.getByUsername('some_user'))).toEqual(stubPeerUser)
expect(fixPeerInfo(await repo.getByPhone('78005553535'))).toEqual(stubPeerUser) expect(fixPeerInfo(await repo.getByPhone('78005553535'))).toEqual(stubPeerUser)
expect(fixPeerInfo(await repo.getById(-1001183945448))).toEqual(stubPeerChannel) expect(fixPeerInfo(await repo.getById(-1001183945448, false))).toEqual(stubPeerChannel)
expect(fixPeerInfo(await repo.getByUsername('some_channel'))).toEqual(stubPeerChannel) expect(fixPeerInfo(await repo.getByUsername('some_channel'))).toEqual(stubPeerChannel)
}) })
@ -74,9 +78,21 @@ export function testPeersRepository(repo: IPeersRepository, driver: IStorageDriv
await repo.store(modUser) await repo.store(modUser)
await driver.save?.() await driver.save?.()
expect(fixPeerInfo(await repo.getById(123123))).toEqual(modUser) expect(fixPeerInfo(await repo.getById(123123, false))).toEqual(modUser)
expect(await repo.getByUsername('some_user')).toEqual(null) expect(await repo.getByUsername('some_user')).toEqual(null)
expect(fixPeerInfo(await repo.getByUsername('some_user2'))).toEqual(modUser) expect(fixPeerInfo(await repo.getByUsername('some_user2'))).toEqual(modUser)
}) })
it('should not return min peers by default', async () => {
await repo.deleteAll()
await repo.store(stupPeerMinUser)
await driver.save?.()
expect(await repo.getById(123123, false)).toEqual(null)
expect(await repo.getByUsername('some_user')).toEqual(null)
expect(await repo.getByPhone('78005553535')).toEqual(null)
expect(fixPeerInfo(await repo.getById(123123, true))).toEqual(stupPeerMinUser)
})
}) })
} }

View file

@ -22,10 +22,14 @@ export class IdbPeersRepository implements IPeersRepository {
return this._driver.db.transaction(TABLE, mode).objectStore(TABLE) return this._driver.db.transaction(TABLE, mode).objectStore(TABLE)
} }
async getById(id: number): Promise<IPeersRepository.PeerInfo | null> { async getById(id: number, allowMin: boolean): Promise<IPeersRepository.PeerInfo | null> {
const it = await reqToPromise(this.os().get(id) as IDBRequest<IPeersRepository.PeerInfo>) const it = await reqToPromise(this.os().get(id) as IDBRequest<IPeersRepository.PeerInfo>)
return it ?? null if (!it) return null
// NB: older objects might not have isMin field
if (it.isMin && !allowMin) return null
return it
} }
async getByUsername(username: string): Promise<IPeersRepository.PeerInfo | null> { async getByUsername(username: string): Promise<IPeersRepository.PeerInfo | null> {
@ -33,13 +37,19 @@ export class IdbPeersRepository implements IPeersRepository {
this.os().index('by_username').get(username) as IDBRequest<IPeersRepository.PeerInfo>, this.os().index('by_username').get(username) as IDBRequest<IPeersRepository.PeerInfo>,
) )
return it ?? null // NB: older objects might not have isMin field
if (!it || it.isMin) return null
return it
} }
async getByPhone(phone: string): Promise<IPeersRepository.PeerInfo | null> { async getByPhone(phone: string): Promise<IPeersRepository.PeerInfo | null> {
const it = await reqToPromise(this.os().index('by_phone').get(phone) as IDBRequest<IPeersRepository.PeerInfo>) const it = await reqToPromise(this.os().index('by_phone').get(phone) as IDBRequest<IPeersRepository.PeerInfo>)
return it ?? null // NB: older objects might not have isMin field
if (!it || it.isMin) return null
return it
} }
deleteAll(): Promise<void> { deleteAll(): Promise<void> {