mirror of
https://github.com/signalapp/Signal-Desktop.git
synced 2026-02-15 07:28:59 +00:00
Deduplicate usernames
This commit is contained in:
@@ -57,6 +57,7 @@ import type {
|
||||
ConversationAttributesType,
|
||||
ConversationAttributesTypeType,
|
||||
ConversationRenderInfoType,
|
||||
SettableConversationAttributesType,
|
||||
} from './model-types.d.ts';
|
||||
import type {
|
||||
ServiceIdString,
|
||||
@@ -483,7 +484,7 @@ export class ConversationController {
|
||||
getOrCreate(
|
||||
identifier: string | null,
|
||||
type: ConversationAttributesTypeType,
|
||||
additionalInitialProps: Partial<ConversationAttributesType> = {}
|
||||
additionalInitialProps: Partial<SettableConversationAttributesType> = {}
|
||||
): ConversationModel {
|
||||
if (typeof identifier !== 'string') {
|
||||
throw new TypeError("'id' must be a string");
|
||||
@@ -613,7 +614,7 @@ export class ConversationController {
|
||||
async getOrCreateAndWait(
|
||||
id: string | null,
|
||||
type: ConversationAttributesTypeType,
|
||||
additionalInitialProps: Partial<ConversationAttributesType> = {}
|
||||
additionalInitialProps: Partial<SettableConversationAttributesType> = {}
|
||||
): Promise<ConversationModel> {
|
||||
await this.load();
|
||||
const conversation = this.getOrCreate(id, type, additionalInitialProps);
|
||||
@@ -1922,6 +1923,33 @@ export class ConversationController {
|
||||
}
|
||||
}
|
||||
|
||||
async usernameUpdated(conversationModel: ConversationModel): Promise<void> {
|
||||
const username = conversationModel.get('username');
|
||||
if (!username) {
|
||||
return;
|
||||
}
|
||||
|
||||
const otherConversationsWithThisUsername = this.getAll().filter(
|
||||
conversation =>
|
||||
conversation !== conversationModel &&
|
||||
conversation.get('username') === username
|
||||
);
|
||||
|
||||
if (otherConversationsWithThisUsername.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
log.warn(
|
||||
`usernameUpdated(${conversationModel.idForLogging()}): detected ${otherConversationsWithThisUsername.length}` +
|
||||
' with the same username, clearing'
|
||||
);
|
||||
await Promise.all(
|
||||
otherConversationsWithThisUsername.map(conversation =>
|
||||
conversation.updateUsername(undefined)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#resetLookups(): void {
|
||||
this.#eraseLookups();
|
||||
this.#generateLookups();
|
||||
|
||||
@@ -3280,8 +3280,10 @@ export async function startApp(): Promise<void> {
|
||||
const ourConversation =
|
||||
window.ConversationController.getOurConversation();
|
||||
if (ourConversation) {
|
||||
ourConversation.set({ username: undefined });
|
||||
await DataWriter.updateConversation(ourConversation.attributes);
|
||||
await ourConversation.updateUsername(undefined, {
|
||||
shouldSave: true,
|
||||
fromStorageService: false,
|
||||
});
|
||||
}
|
||||
|
||||
// Then make sure outstanding conversation saves are flushed
|
||||
|
||||
7
ts/model-types.d.ts
vendored
7
ts/model-types.d.ts
vendored
@@ -527,6 +527,13 @@ export type ConversationAttributesType = {
|
||||
test_chatFrameImportedFromBackup?: boolean;
|
||||
};
|
||||
|
||||
// Fields omitted from SettableConversationAttributesType must be set via their
|
||||
// appropriate setter functions (e.g. ConverationModel.updateUsername)
|
||||
export type SettableConversationAttributesType = Omit<
|
||||
ConversationAttributesType,
|
||||
'username'
|
||||
>;
|
||||
|
||||
export type ConversationRenderInfoType = Pick<
|
||||
ConversationAttributesType,
|
||||
| 'e164'
|
||||
|
||||
@@ -14,6 +14,7 @@ import type {
|
||||
MessageAttributesType,
|
||||
QuotedMessageType,
|
||||
SenderKeyInfoType,
|
||||
SettableConversationAttributesType,
|
||||
} from '../model-types.d.ts';
|
||||
import { DataReader, DataWriter } from '../sql/Client.preload.js';
|
||||
import { getConversation } from '../util/getConversation.preload.js';
|
||||
@@ -369,7 +370,15 @@ export class ConversationModel {
|
||||
): ConversationAttributesType[keyName] {
|
||||
return this.attributes[key];
|
||||
}
|
||||
|
||||
public set(
|
||||
attributes: Partial<SettableConversationAttributesType>,
|
||||
{ noTrigger }: { noTrigger?: boolean } = {}
|
||||
): void {
|
||||
this.#doSet(attributes, { noTrigger });
|
||||
}
|
||||
|
||||
#doSet(
|
||||
attributes: Partial<ConversationAttributesType>,
|
||||
{ noTrigger }: { noTrigger?: boolean } = {}
|
||||
): void {
|
||||
@@ -4327,7 +4336,7 @@ export class ConversationModel {
|
||||
|
||||
log.info(`maybeClearUsername(${this.idForLogging()}): clearing username`);
|
||||
|
||||
this.set({ username: undefined });
|
||||
this.#doSet({ username: undefined });
|
||||
|
||||
if (this.get('needsTitleTransition') && getProfileName(this.attributes)) {
|
||||
log.info(
|
||||
@@ -4356,7 +4365,13 @@ export class ConversationModel {
|
||||
|
||||
async updateUsername(
|
||||
username: string | undefined,
|
||||
{ shouldSave = true }: { shouldSave?: boolean } = {}
|
||||
{
|
||||
shouldSave = true,
|
||||
fromStorageService = false,
|
||||
}: {
|
||||
shouldSave?: boolean;
|
||||
fromStorageService?: boolean;
|
||||
} = {}
|
||||
): Promise<void> {
|
||||
const ourConversationId =
|
||||
window.ConversationController.getOurConversationId();
|
||||
@@ -4371,8 +4386,12 @@ export class ConversationModel {
|
||||
|
||||
log.info(`updateUsername(${this.idForLogging()}): updating username`);
|
||||
|
||||
this.set({ username });
|
||||
this.captureChange('updateUsername');
|
||||
this.#doSet({ username });
|
||||
await window.ConversationController.usernameUpdated(this);
|
||||
|
||||
if (!fromStorageService) {
|
||||
this.captureChange('updateUsername');
|
||||
}
|
||||
|
||||
if (shouldSave) {
|
||||
await DataWriter.updateConversation(this.attributes);
|
||||
|
||||
@@ -1459,6 +1459,7 @@ export async function mergeContactRecord(
|
||||
|
||||
await conversation.updateUsername(dropNull(contactRecord.username), {
|
||||
shouldSave: false,
|
||||
fromStorageService: true,
|
||||
});
|
||||
|
||||
let needsProfileFetch = false;
|
||||
@@ -2013,12 +2014,16 @@ export async function mergeAccountRecord(
|
||||
conversation.set({
|
||||
isArchived: Boolean(noteToSelfArchived),
|
||||
markedUnread: Boolean(noteToSelfMarkedUnread),
|
||||
username: dropNull(username),
|
||||
storageID,
|
||||
storageVersion,
|
||||
needsStorageServiceSync: false,
|
||||
});
|
||||
|
||||
await conversation.updateUsername(dropNull(username), {
|
||||
shouldSave: false,
|
||||
fromStorageService: true,
|
||||
});
|
||||
|
||||
let needsProfileFetch = false;
|
||||
if (profileKey && profileKey.byteLength > 0) {
|
||||
needsProfileFetch = await conversation.setProfileKey(
|
||||
|
||||
38
ts/sql/migrations/1600-deduplicate-usernames.std.ts
Normal file
38
ts/sql/migrations/1600-deduplicate-usernames.std.ts
Normal file
@@ -0,0 +1,38 @@
|
||||
// Copyright 2025 Signal Messenger, LLC
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
import type { LoggerType } from '../../types/Logging.std.js';
|
||||
import type { WritableDB } from '../Interface.std.js';
|
||||
import { sql } from '../util.std.js';
|
||||
|
||||
export default function updateToSchemaVersion1600(
|
||||
db: WritableDB,
|
||||
logger: LoggerType
|
||||
): void {
|
||||
const jsonPatch = JSON.stringify({
|
||||
username: null,
|
||||
needsStorageServiceSync: true,
|
||||
});
|
||||
const [query, params] = sql`
|
||||
WITH rowsKeepingUsername AS (
|
||||
SELECT
|
||||
rowId,
|
||||
json ->> '$.username' AS username,
|
||||
MAX(active_at)
|
||||
FROM conversations
|
||||
WHERE username IS NOT NULL
|
||||
GROUP BY username
|
||||
)
|
||||
UPDATE conversations AS c
|
||||
SET json = json_patch(json, ${jsonPatch})
|
||||
WHERE json ->> '$.username' IS NOT NULL
|
||||
AND c.rowId NOT IN (
|
||||
SELECT rowId from rowsKeepingUsername
|
||||
);
|
||||
`;
|
||||
const result = db.prepare(query).run(params);
|
||||
if (result.changes > 0) {
|
||||
logger.warn(
|
||||
`Removed duplicate usernames from ${result.changes} conversations`
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -136,6 +136,7 @@ import updateToSchemaVersion1561 from './1561-cleanup-polls.std.js';
|
||||
import updateToSchemaVersion1570 from './1570-pinned-messages-updates.std.js';
|
||||
import updateToSchemaVersion1580 from './1580-expired-group-replies.std.js';
|
||||
import updateToSchemaVersion1590 from './1590-megaphones.std.js';
|
||||
import updateToSchemaVersion1600 from './1600-deduplicate-usernames.std.js';
|
||||
|
||||
import { DataWriter } from '../Server.node.js';
|
||||
|
||||
@@ -1631,6 +1632,7 @@ export const SCHEMA_VERSIONS: ReadonlyArray<SchemaUpdateType> = [
|
||||
{ version: 1570, update: updateToSchemaVersion1570 },
|
||||
{ version: 1580, update: updateToSchemaVersion1580 },
|
||||
{ version: 1590, update: updateToSchemaVersion1590 },
|
||||
{ version: 1600, update: updateToSchemaVersion1600 },
|
||||
];
|
||||
|
||||
export class DBVersionFromFutureError extends Error {
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
import { assert } from 'chai';
|
||||
import { v7 as generateUuid } from 'uuid';
|
||||
|
||||
import { DataWriter } from '../../sql/Client.preload.js';
|
||||
import { DataReader, DataWriter } from '../../sql/Client.preload.js';
|
||||
import { SendStatus } from '../../messages/MessageSendState.std.js';
|
||||
import { IMAGE_PNG } from '../../types/MIME.std.js';
|
||||
import { generateAci, generatePni } from '../../types/ServiceId.std.js';
|
||||
@@ -12,6 +12,7 @@ import { MessageModel } from '../../models/messages.preload.js';
|
||||
import { DurationInSeconds } from '../../util/durations/index.std.js';
|
||||
import { ConversationModel } from '../../models/conversations.preload.js';
|
||||
import { itemStorage } from '../../textsecure/Storage.preload.js';
|
||||
import { strictAssert } from '../../util/assert.std.js';
|
||||
|
||||
describe('Conversations', () => {
|
||||
async function resetConversationController(): Promise<void> {
|
||||
@@ -200,4 +201,34 @@ describe('Conversations', () => {
|
||||
assert.equal(conversation.get('expireTimer'), DurationInSeconds.DAY);
|
||||
});
|
||||
});
|
||||
|
||||
it('setting a duplicate username clears it from existing conversations', async () => {
|
||||
const alice = await window.ConversationController.getOrCreateAndWait(
|
||||
generateAci(),
|
||||
'private',
|
||||
{ active_at: 10 }
|
||||
);
|
||||
const bob = await window.ConversationController.getOrCreateAndWait(
|
||||
generateAci(),
|
||||
'private',
|
||||
{ active_at: 1 }
|
||||
);
|
||||
|
||||
await alice.updateUsername('username.12');
|
||||
await bob.updateUsername('username.12');
|
||||
|
||||
assert.strictEqual(alice.get('username'), undefined);
|
||||
assert.strictEqual(bob.get('username'), 'username.12');
|
||||
|
||||
await DataWriter.flushUpdateConversationBatcher();
|
||||
|
||||
const aliceInDb = await DataReader.getConversationById(alice.attributes.id);
|
||||
const bobInDb = await DataReader.getConversationById(bob.attributes.id);
|
||||
|
||||
strictAssert(aliceInDb, 'must exist');
|
||||
strictAssert(bobInDb, 'must exist');
|
||||
|
||||
assert.strictEqual(aliceInDb.username, undefined);
|
||||
assert.strictEqual(bobInDb.username, 'username.12');
|
||||
});
|
||||
});
|
||||
|
||||
138
ts/test-node/sql/migration_1600_test.node.ts
Normal file
138
ts/test-node/sql/migration_1600_test.node.ts
Normal file
@@ -0,0 +1,138 @@
|
||||
// Copyright 2025 Signal Messenger, LLC
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
import { assert } from 'chai';
|
||||
|
||||
import { type WritableDB } from '../../sql/Interface.std.js';
|
||||
import {
|
||||
createDB,
|
||||
getTableData,
|
||||
insertData,
|
||||
updateToVersion,
|
||||
} from './helpers.node.js';
|
||||
|
||||
type ConversationRow = {
|
||||
id: string;
|
||||
json: {
|
||||
username?: string;
|
||||
needsStorageServiceSync?: boolean;
|
||||
};
|
||||
};
|
||||
describe('SQL/updateToSchemaVersion1600', () => {
|
||||
let db: WritableDB;
|
||||
|
||||
beforeEach(() => {
|
||||
db = createDB();
|
||||
updateToVersion(db, 1590);
|
||||
});
|
||||
afterEach(() => {
|
||||
db.close();
|
||||
});
|
||||
|
||||
it('deduplicates usernames based on most recent active_at', () => {
|
||||
insertData(db, 'conversations', [
|
||||
{
|
||||
id: 'convo1',
|
||||
type: 'private',
|
||||
active_at: 1,
|
||||
json: {
|
||||
username: 'username1',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'convo2',
|
||||
type: 'private',
|
||||
active_at: 2,
|
||||
json: {
|
||||
username: 'username1',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'convo3',
|
||||
type: 'private',
|
||||
active_at: null,
|
||||
json: {
|
||||
username: 'username1',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'convo4',
|
||||
type: 'private',
|
||||
active_at: 4,
|
||||
json: {
|
||||
username: 'username2',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'convo5',
|
||||
type: 'private',
|
||||
active_at: 5,
|
||||
json: {
|
||||
username: 'username2',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'convo6',
|
||||
type: 'private',
|
||||
active_at: 6,
|
||||
json: {
|
||||
username: 'username3',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'convo7',
|
||||
type: 'private',
|
||||
active_at: 7,
|
||||
json: {},
|
||||
},
|
||||
]);
|
||||
|
||||
updateToVersion(db, 1600);
|
||||
|
||||
assert.deepStrictEqual(
|
||||
(getTableData(db, 'conversations') as Array<ConversationRow>)
|
||||
.sort((a, b) => a.id.localeCompare(b.id))
|
||||
.map(({ id, json }) => ({
|
||||
id,
|
||||
username: json.username,
|
||||
needsStorageServiceSync: json.needsStorageServiceSync,
|
||||
})),
|
||||
[
|
||||
{
|
||||
id: 'convo1',
|
||||
username: undefined,
|
||||
needsStorageServiceSync: true,
|
||||
},
|
||||
{
|
||||
id: 'convo2',
|
||||
username: 'username1',
|
||||
needsStorageServiceSync: undefined,
|
||||
},
|
||||
{
|
||||
id: 'convo3',
|
||||
username: undefined,
|
||||
needsStorageServiceSync: true,
|
||||
},
|
||||
{
|
||||
id: 'convo4',
|
||||
username: undefined,
|
||||
needsStorageServiceSync: true,
|
||||
},
|
||||
{
|
||||
id: 'convo5',
|
||||
username: 'username2',
|
||||
needsStorageServiceSync: undefined,
|
||||
},
|
||||
{
|
||||
id: 'convo6',
|
||||
username: 'username3',
|
||||
needsStorageServiceSync: undefined,
|
||||
},
|
||||
{
|
||||
id: 'convo7',
|
||||
username: undefined,
|
||||
needsStorageServiceSync: undefined,
|
||||
},
|
||||
]
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user