From 3d7109eb013230438a2e05eec79fac25cda6bbd4 Mon Sep 17 00:00:00 2001 From: AsamK Date: Sun, 20 Sep 2026 18:03:20 +0200 Subject: [PATCH] Remove numberless flag in accounts file This is an edge case and keeping the ACI only account discoverable is actually the better way to handle it. --- .../storage/accounts/AccountsStorage.java | 3 +- .../storage/accounts/AccountsStore.java | 47 ++----- .../storage/accounts/AccountsStoreTest.java | 133 +----------------- .../signal-cli/reachability-metadata.json | 7 +- 4 files changed, 15 insertions(+), 175 deletions(-) diff --git a/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStorage.java b/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStorage.java index ca202b67..40107986 100644 --- a/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStorage.java +++ b/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStorage.java @@ -4,6 +4,5 @@ import java.util.List; public record AccountsStorage(List accounts, Integer version) { - // Older indexes omit this flag; entries with only a UUID must then be checked against their account state. - public record Account(String path, String environment, String number, String uuid, Boolean numberless) {} + public record Account(String path, String environment, String number, String uuid) {} } diff --git a/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStore.java b/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStore.java index 8d3ade20..4ce4b783 100644 --- a/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStore.java +++ b/lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStore.java @@ -63,7 +63,7 @@ public class AccountsStore { public synchronized Set getAllAccounts() throws IOException { return readAccounts().stream() .filter(a -> a.environment() == null || serviceEnvironment.equals(a.environment())) - .filter(a -> a.number() != null || (a.uuid() != null && Boolean.TRUE.equals(a.numberless()))) + .filter(a -> a.number() != null || a.uuid() != null) .collect(Collectors.toSet()); } @@ -98,15 +98,14 @@ public class AccountsStore { return new AccountsStorage.Account(a.path(), serviceEnvironment, number, - aci == null ? null : aci.toString(), - number == null && aci != null); + aci == null ? null : aci.toString()); } if (number != null && number.equals(a.number())) { - return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid(), false); + return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid()); } if (aci != null && aci.toString().equals(a.uuid())) { - return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null, false); + return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null); } return a; @@ -118,8 +117,7 @@ public class AccountsStore { final var account = new AccountsStorage.Account(accountPath, serviceEnvironment, number, - aci == null ? null : aci.toString(), - number == null && aci != null); + aci == null ? null : aci.toString()); updateAccounts(accounts -> { final var existingAccounts = accounts.stream().map(a -> { if (a.environment() != null && !serviceEnvironment.equals(a.environment())) { @@ -127,10 +125,10 @@ public class AccountsStore { } if (number != null && number.equals(a.number())) { - return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid(), false); + return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid()); } if (aci != null && aci.toString().equals(a.uuid())) { - return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null, false); + return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null); } return a; @@ -161,7 +159,7 @@ public class AccountsStore { private void createInitialAccounts() throws IOException { final var legacyAccountPaths = getLegacyAccountPaths(); final var accountsStorage = new AccountsStorage(legacyAccountPaths.stream() - .map(number -> new AccountsStorage.Account(number, null, number, null, false)) + .map(number -> new AccountsStorage.Account(number, null, number, null)) .toList(), CURRENT_STORAGE_VERSION); IOUtils.createPrivateDirectories(dataPath); @@ -204,34 +202,10 @@ public class AccountsStore { } else if (accountsVersion < CURRENT_STORAGE_VERSION) { storage = upgradeAccountsFile(fileChannel, storage, accountsVersion); } - final var accounts = storage.accounts().stream().map(this::resolveNumberlessAccount).toList(); - if (!accounts.equals(storage.accounts())) { - saveAccountsLocked(fileChannel, new AccountsStorage(accounts, storage.version())); - } - return accounts; + return storage.accounts(); } } - private AccountsStorage.Account resolveNumberlessAccount(final AccountsStorage.Account entry) { - if (entry.numberless() != null) { - return entry; - } - - var numberless = false; - if (entry.number() == null && entry.uuid() != null) { - // Older entries also lose their number when another account takes it over. - try (final var account = accountLoader.loadAccountOrNull(entry.path())) { - if (account == null) { - // Keep the entry unresolved so a locked or unavailable account can be checked later. - return entry; - } - numberless = account.getNumber() == null && account.getAci() != null && entry.uuid() - .equals(account.getAci().toString()); - } - } - return new AccountsStorage.Account(entry.path(), entry.environment(), entry.number(), entry.uuid(), numberless); - } - private AccountsStorage upgradeAccountsFile( final FileChannel fileChannel, final AccountsStorage storage, @@ -252,8 +226,7 @@ public class AccountsStore { return new AccountsStorage.Account(a.path(), getServiceEnvironmentString(account.getServiceEnvironment()), a.number(), - a.uuid(), - a.numberless()); + a.uuid()); } }).toList(); } diff --git a/lib/src/test/java/org/asamk/signal/manager/storage/accounts/AccountsStoreTest.java b/lib/src/test/java/org/asamk/signal/manager/storage/accounts/AccountsStoreTest.java index 5f30dd64..8299be1c 100644 --- a/lib/src/test/java/org/asamk/signal/manager/storage/accounts/AccountsStoreTest.java +++ b/lib/src/test/java/org/asamk/signal/manager/storage/accounts/AccountsStoreTest.java @@ -1,10 +1,6 @@ package org.asamk.signal.manager.storage.accounts; -import org.asamk.signal.manager.Settings; import org.asamk.signal.manager.api.ServiceEnvironment; -import org.asamk.signal.manager.storage.SignalAccount; -import org.asamk.signal.manager.storage.Utils; -import org.asamk.signal.manager.util.KeyUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; @@ -12,7 +8,6 @@ import org.junit.jupiter.params.provider.ValueSource; import org.signal.core.models.ServiceId.ACI; import java.io.IOException; -import java.nio.channels.OverlappingFileLockException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Set; @@ -61,7 +56,7 @@ class AccountsStoreTest { @ParameterizedTest @ValueSource(booleans = {false, true}) - void replacingANumberDoesNotRediscoverTheOldAccountAsNumberless(final boolean update) throws Exception { + void replacingANumberKeepsTheOldAccountAvailableByAci(final boolean update) throws Exception { final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> null); final var oldPath = store.addAccount("+12025550123", OLD_ACI); Files.createFile(directory.resolve(oldPath)); @@ -73,136 +68,14 @@ class AccountsStoreTest { newPath = store.addAccount("+12025550123", NEW_ACI); } - final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> { - throw new AssertionError("New entries should not require loading account state for discovery"); - }); - assertEquals(Set.of(newPath), getAccountPaths(reopened)); + final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> null); + assertEquals(Set.of(oldPath, newPath), getAccountPaths(reopened)); assertEquals(newPath, reopened.getPathByNumber("+12025550123")); assertEquals(oldPath, reopened.getPathByAci(OLD_ACI)); - - // Only an explicit account update should turn the old entry into a numberless account. - reopened.updateAccount(oldPath, null, OLD_ACI); - assertEquals(Set.of(oldPath, newPath), getAccountPaths(reopened)); - } - - @ParameterizedTest - @ValueSource(ints = {1, 2}) - void legacyIndexesDistinguishNumberlessAccountsFromSupersededNumberedAccounts(final int version) throws Exception { - createSavedAccount("old", "+12025550123", OLD_ACI); - createSavedAccount("numberless", null, NEW_ACI); - final var legacyIndex = """ - { - "version": %d, - "accounts": [ - {"path": "old", "environment": %s, "number": null, "uuid": "%s"}, - {"path": "numberless", "environment": %s, "number": null, "uuid": "%s"}, - {"path": "current", "environment": "STAGING", "number": "+12025550123", "uuid": null} - ] - } - """.formatted(version, - version == 1 ? "null" : "\"STAGING\"", - OLD_ACI, - version == 1 ? "null" : "\"STAGING\"", - NEW_ACI); - Files.writeString(directory.resolve("accounts.json"), legacyIndex); - - final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull); - assertEquals(Set.of("numberless", "current"), getAccountPaths(store)); - assertEquals("old", store.getPathByAci(OLD_ACI)); - assertEquals("numberless", store.getPathByAci(NEW_ACI)); - assertEquals(Set.of("+12025550123"), store.getAllNumbers()); - - final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> { - throw new AssertionError("Resolved entries should not require loading account state again"); - }); - assertEquals(Set.of("numberless", "current"), getAccountPaths(reopened)); - assertTrue(new AccountsStore(directory.toFile(), - ServiceEnvironment.LIVE, - this::loadAccountOrNull).getAllAccounts().isEmpty()); - } - - @Test - void retriesLockedLegacyAccountsAndPersistsSuccessfulClassification() throws Exception { - createSavedAccount("numberless", null, NEW_ACI); - writeLegacyNumberlessEntry(NEW_ACI); - final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull); - - try (final var account = SignalAccount.load(directory.toFile(), "numberless", false, Settings.DEFAULT)) { - assertTrue(store.getAllAccounts().isEmpty()); - assertEquals("numberless", store.getPathByAci(NEW_ACI)); - final var storage = Utils.createStorageObjectMapper() - .readValue(directory.resolve("accounts.json").toFile(), AccountsStorage.class); - assertNull(storage.accounts().getFirst().numberless()); - } - - assertEquals(Set.of("numberless"), getAccountPaths(store)); - try (final var account = SignalAccount.load(directory.toFile(), "numberless", false, Settings.DEFAULT)) { - final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> { - throw new AssertionError("A classified account should remain discoverable while locked"); - }); - assertEquals(Set.of("numberless"), getAccountPaths(reopened)); - } - } - - @Test - void retriesLegacyAccountsAfterMissingStateIsRestored() throws Exception { - writeLegacyNumberlessEntry(NEW_ACI); - final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull); - assertTrue(store.getAllAccounts().isEmpty()); - assertEquals("numberless", store.getPathByAci(NEW_ACI)); - - createSavedAccount("numberless", null, NEW_ACI); - assertEquals(Set.of("numberless"), getAccountPaths(store)); - } - - @Test - void doesNotDiscoverLegacyEntriesWithAMismatchedAci() throws Exception { - createSavedAccount("numberless", null, NEW_ACI); - writeLegacyNumberlessEntry(OLD_ACI); - final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull); - assertTrue(store.getAllAccounts().isEmpty()); - assertEquals("numberless", store.getPathByAci(OLD_ACI)); } private Set getAccountPaths(final AccountsStore store) throws IOException { return store.getAllAccounts().stream().map(AccountsStorage.Account::path).collect(Collectors.toSet()); } - private void writeLegacyNumberlessEntry(final ACI aci) throws IOException { - final var legacyIndex = """ - { - "version": 2, - "accounts": [ - {"path": "numberless", "environment": "STAGING", "number": null, "uuid": "%s"} - ] - } - """.formatted(aci); - Files.writeString(directory.resolve("accounts.json"), legacyIndex); - } - - private void createSavedAccount(final String path, final String number, final ACI aci) throws IOException { - // Account creation needs a number until the ACI has been assigned. - try (final var account = SignalAccount.create(directory.toFile(), - path, - "+12025550123", - ServiceEnvironment.STAGING, - KeyUtils.generateIdentityKeyPair(), - KeyUtils.generateIdentityKeyPair(), - KeyUtils.createProfileKey(), - Settings.DEFAULT)) { - account.setAci(aci); - account.setNumber(number); - } - } - - private SignalAccount loadAccountOrNull(final String path) { - if (!SignalAccount.accountFileExists(directory.toFile(), path)) { - return null; - } - try { - return SignalAccount.load(directory.toFile(), path, false, Settings.DEFAULT); - } catch (IOException | OverlappingFileLockException e) { - return null; - } - } } diff --git a/src/main/resources/META-INF/native-image/org.asamk/signal-cli/reachability-metadata.json b/src/main/resources/META-INF/native-image/org.asamk/signal-cli/reachability-metadata.json index 2c4f242d..60ff4c40 100644 --- a/src/main/resources/META-INF/native-image/org.asamk/signal-cli/reachability-metadata.json +++ b/src/main/resources/META-INF/native-image/org.asamk/signal-cli/reachability-metadata.json @@ -4012,8 +4012,7 @@ "java.lang.String", "java.lang.String", "java.lang.String", - "java.lang.String", - "java.lang.Boolean" + "java.lang.String" ] }, { @@ -4024,10 +4023,6 @@ "name": "number", "parameterTypes": [] }, - { - "name": "numberless", - "parameterTypes": [] - }, { "name": "path", "parameterTypes": []