From 359ef330dfae8b378d0e3607673f33305ea7758c Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 20 Jan 2023 14:39:43 +0100 Subject: [PATCH] get rid of upsert in favor of update and insert upsert seems to only work with primary keys and not other unique constraints. --- .../1.json | 15 +++-- .../android/database/dao/AccountDao.java | 11 ++- .../android/database/dao/DiscoDao.java | 67 ++++++------------- .../database/entity/AccountEntity.java | 3 +- .../database/entity/DiscoItemEntity.java | 31 +++++++-- .../android/xmpp/ConnectionPool.java | 56 +++++++++++++--- .../android/xmpp/XmppConnection.java | 5 +- 7 files changed, 113 insertions(+), 75 deletions(-) diff --git a/schemas/im.conversations.android.database.ConversationsDatabase/1.json b/schemas/im.conversations.android.database.ConversationsDatabase/1.json index f2eb613f1..e0e04eece 100644 --- a/schemas/im.conversations.android.database.ConversationsDatabase/1.json +++ b/schemas/im.conversations.android.database.ConversationsDatabase/1.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 1, - "identityHash": "b28e01dcbb5d9774a4b36783d0db6c73", + "identityHash": "8f1d4d8d2bdb8b2358132202037aba7a", "entities": [ { "tableName": "account", @@ -601,7 +601,7 @@ }, { "tableName": "disco_item", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT, `accountId` INTEGER NOT NULL, `address` TEXT NOT NULL, `node` TEXT NOT NULL, `parent` TEXT, `discoId` INTEGER, FOREIGN KEY(`accountId`) REFERENCES `account`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`discoId`) REFERENCES `disco`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT, `accountId` INTEGER NOT NULL, `address` TEXT NOT NULL, `node` TEXT NOT NULL, `parent` TEXT NOT NULL, `discoId` INTEGER, FOREIGN KEY(`accountId`) REFERENCES `account`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`discoId`) REFERENCES `disco`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", "fields": [ { "fieldPath": "id", @@ -631,7 +631,7 @@ "fieldPath": "parent", "columnName": "parent", "affinity": "TEXT", - "notNull": false + "notNull": true }, { "fieldPath": "discoId", @@ -648,15 +648,16 @@ }, "indices": [ { - "name": "index_disco_item_accountId_address_node", + "name": "index_disco_item_accountId_address_node_parent", "unique": true, "columnNames": [ "accountId", "address", - "node" + "node", + "parent" ], "orders": [], - "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_disco_item_accountId_address_node` ON `${TABLE_NAME}` (`accountId`, `address`, `node`)" + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_disco_item_accountId_address_node_parent` ON `${TABLE_NAME}` (`accountId`, `address`, `node`, `parent`)" }, { "name": "index_disco_item_accountId_parent", @@ -1336,7 +1337,7 @@ "views": [], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'b28e01dcbb5d9774a4b36783d0db6c73')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '8f1d4d8d2bdb8b2358132202037aba7a')" ] } } \ No newline at end of file diff --git a/src/main/java/im/conversations/android/database/dao/AccountDao.java b/src/main/java/im/conversations/android/database/dao/AccountDao.java index 094a47f26..4cfc8c8c5 100644 --- a/src/main/java/im/conversations/android/database/dao/AccountDao.java +++ b/src/main/java/im/conversations/android/database/dao/AccountDao.java @@ -5,6 +5,7 @@ import androidx.room.Insert; import androidx.room.OnConflictStrategy; import androidx.room.Query; import com.google.common.util.concurrent.ListenableFuture; +import eu.siacs.conversations.xmpp.Jid; import im.conversations.android.database.entity.AccountEntity; import im.conversations.android.database.model.Account; import im.conversations.android.database.model.Connection; @@ -13,12 +14,18 @@ import java.util.List; @Dao public interface AccountDao { - @Insert(onConflict = OnConflictStrategy.ABORT) - void insert(final AccountEntity account); + @Insert(onConflict = OnConflictStrategy.REPLACE) + long insert(final AccountEntity account); @Query("SELECT id,address,randomSeed FROM account WHERE enabled = 1") ListenableFuture> getEnabledAccounts(); + @Query("SELECT id,address,randomSeed FROM account WHERE address=:address AND enabled=1") + ListenableFuture getEnabledAccount(Jid address); + + @Query("SELECT id,address,randomSeed FROM account WHERE id=:id AND enabled=1") + ListenableFuture getEnabledAccount(long id); + @Query("SELECT hostname,port,directTls FROM account WHERE id=:id AND hostname != null") Connection getConnectionSettings(long id); diff --git a/src/main/java/im/conversations/android/database/dao/DiscoDao.java b/src/main/java/im/conversations/android/database/dao/DiscoDao.java index 0a61d5374..d8f6aa727 100644 --- a/src/main/java/im/conversations/android/database/dao/DiscoDao.java +++ b/src/main/java/im/conversations/android/database/dao/DiscoDao.java @@ -1,12 +1,11 @@ package im.conversations.android.database.dao; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.room.Dao; import androidx.room.Insert; +import androidx.room.OnConflictStrategy; import androidx.room.Query; import androidx.room.Transaction; -import androidx.room.Upsert; import com.google.common.base.Strings; import com.google.common.collect.Collections2; import eu.siacs.conversations.xmpp.Jid; @@ -32,8 +31,11 @@ import java.util.Collection; @Dao public abstract class DiscoDao { - @Upsert(entity = DiscoItemEntity.class) - protected abstract void insertDiscoItems(Collection items); + @Insert(onConflict = OnConflictStrategy.IGNORE) + protected abstract void insertDiscoItems(Collection items); + + @Insert + protected abstract void insert(DiscoItemEntity item); @Insert protected abstract void insertDiscoIdentities(Collection identities); @@ -53,13 +55,16 @@ public abstract class DiscoDao { protected abstract void updateDiscoIdInPresence( long account, Jid address, String resource, long discoId); + @Query( + "UPDATE disco_item SET discoId=:discoId WHERE accountId=:account AND address=:address" + + " AND node=:node") + protected abstract int updateDiscoIdInDiscoItem( + long account, Jid address, String node, long discoId); + @Insert protected abstract void insertDiscoFieldValues( Collection value); - @Upsert(entity = DiscoItemEntity.class) - protected abstract void updateDiscoIdInDiscoItem(DiscoItemWithDiscoId item); - @Insert protected abstract long insert(DiscoEntity entity); @@ -73,7 +78,8 @@ public abstract class DiscoDao { public void set( final Account account, final Entity.DiscoItem parent, final Collection items) { final var entities = - Collections2.transform(items, i -> DiscoItemWithParent.of(account.id, parent, i)); + Collections2.transform( + items, i -> DiscoItemEntity.of(account.id, parent.address, i)); insertDiscoItems(entities); deleteNonExistentDiscoItems( account.id, parent.address, Collections2.transform(items, Item::getJid)); @@ -106,8 +112,12 @@ public abstract class DiscoDao { @Nullable final String node, final long discoId) { if (entity instanceof Entity.DiscoItem) { - updateDiscoIdInDiscoItem( - DiscoItemWithDiscoId.of(account, (Entity.DiscoItem) entity, node, discoId)); + if (updateDiscoIdInDiscoItem( + account, entity.address, Strings.nullToEmpty(node), discoId) + > 0) { + return; + } + insert(DiscoItemEntity.of(account, entity.address, node, discoId)); } else if (entity instanceof Entity.Presence) { updateDiscoIdInPresence( account, @@ -167,41 +177,4 @@ public abstract class DiscoDao { + " disco_item.discoId=disco_feature.discoId WHERE accountId=:account AND" + " address=:entity AND feature=:feature)") public abstract boolean hasFeature(final long account, final Jid entity, final String feature); - - public static class DiscoItemWithParent { - public long accountId; - public @NonNull Jid address; - public @NonNull String node; - public @Nullable Jid parent; - - public static DiscoItemWithParent of( - final long account, Entity.DiscoItem parent, final Item item) { - final var entity = new DiscoItemWithParent(); - entity.accountId = account; - entity.address = item.getJid(); - entity.node = Strings.nullToEmpty(item.getNode()); - entity.parent = parent.address; - return entity; - } - } - - public static class DiscoItemWithDiscoId { - public long accountId; - public @NonNull Jid address; - public @NonNull String node; - public long discoId; - - public static DiscoItemWithDiscoId of( - final long account, - final Entity.DiscoItem discoItem, - @NonNull final String node, - final long discoId) { - final var entity = new DiscoItemWithDiscoId(); - entity.accountId = account; - entity.address = discoItem.address; - entity.node = node; - entity.discoId = discoId; - return entity; - } - } } diff --git a/src/main/java/im/conversations/android/database/entity/AccountEntity.java b/src/main/java/im/conversations/android/database/entity/AccountEntity.java index 0942623c4..f23a183d2 100644 --- a/src/main/java/im/conversations/android/database/entity/AccountEntity.java +++ b/src/main/java/im/conversations/android/database/entity/AccountEntity.java @@ -5,6 +5,7 @@ import androidx.room.Embedded; import androidx.room.Entity; import androidx.room.Index; import androidx.room.PrimaryKey; +import eu.siacs.conversations.xmpp.Jid; import im.conversations.android.database.model.Connection; import im.conversations.android.database.model.Proxy; @@ -20,7 +21,7 @@ public class AccountEntity { @PrimaryKey(autoGenerate = true) public Long id; - @NonNull public String address; + @NonNull public Jid address; public String resource; public byte[] randomSeed; diff --git a/src/main/java/im/conversations/android/database/entity/DiscoItemEntity.java b/src/main/java/im/conversations/android/database/entity/DiscoItemEntity.java index ada7d2606..23302d5a2 100644 --- a/src/main/java/im/conversations/android/database/entity/DiscoItemEntity.java +++ b/src/main/java/im/conversations/android/database/entity/DiscoItemEntity.java @@ -1,12 +1,13 @@ package im.conversations.android.database.entity; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.room.Entity; import androidx.room.ForeignKey; import androidx.room.Index; import androidx.room.PrimaryKey; +import com.google.common.base.Strings; import eu.siacs.conversations.xmpp.Jid; +import im.conversations.android.xmpp.model.disco.items.Item; @Entity( tableName = "disco_item", @@ -24,7 +25,7 @@ import eu.siacs.conversations.xmpp.Jid; }, indices = { @Index( - value = {"accountId", "address", "node"}, + value = {"accountId", "address", "node", "parent"}, unique = true), @Index( value = {"accountId", "parent"}, @@ -36,13 +37,33 @@ public class DiscoItemEntity { @PrimaryKey(autoGenerate = true) public Long id; - @NonNull Long accountId; + @NonNull public Long accountId; - @NonNull Jid address; + @NonNull public Jid address; @NonNull public String node; - @Nullable public Jid parent; + @NonNull public String parent; public Long discoId; + + public static DiscoItemEntity of(long accountId, final Jid parent, Item item) { + final var entity = new DiscoItemEntity(); + entity.accountId = accountId; + entity.address = item.getJid(); + entity.node = Strings.nullToEmpty(item.getNode()); + entity.parent = parent.toEscapedString(); + return entity; + } + + public static DiscoItemEntity of( + long accountId, final Jid address, final String node, final long discoId) { + final var entity = new DiscoItemEntity(); + entity.accountId = accountId; + entity.address = address; + entity.node = Strings.nullToEmpty(node); + entity.parent = ""; + entity.discoId = discoId; + return entity; + } } diff --git a/src/main/java/im/conversations/android/xmpp/ConnectionPool.java b/src/main/java/im/conversations/android/xmpp/ConnectionPool.java index 84bd4ecf0..fba51e0e9 100644 --- a/src/main/java/im/conversations/android/xmpp/ConnectionPool.java +++ b/src/main/java/im/conversations/android/xmpp/ConnectionPool.java @@ -55,15 +55,53 @@ public class ConnectionPool { reconfigurationExecutor); } - public synchronized XmppConnection get(final Jid address) { - return Iterables.find(this.connections, c -> address.equals(c.getAccount().address)); + public synchronized XmppConnection reconfigure(final Account account) { + final Optional xmppConnectionOptional = + Iterables.tryFind(this.connections, c -> c.getAccount().equals(account)); + if (xmppConnectionOptional.isPresent()) { + return xmppConnectionOptional.get(); + } + return setupXmppConnection(context, account); } - public synchronized XmppConnection get(final long id) { - return Iterables.find(this.connections, c -> id == c.getAccount().id); + public synchronized ListenableFuture get(final Jid address) { + final var configured = + Iterables.tryFind(this.connections, c -> address.equals(c.getAccount().address)); + if (configured.isPresent()) { + return Futures.immediateFuture(configured.get()); + } + return Futures.transform( + ConversationsDatabase.getInstance(context).accountDao().getEnabledAccount(address), + account -> { + if (account == null) { + throw new IllegalStateException( + String.format( + "No enabled account with address %s", + address.toEscapedString())); + } + return reconfigure(account); + }, + reconfigurationExecutor); } - public synchronized boolean isEnabled(final long id) { + public synchronized ListenableFuture get(final long id) { + final var configured = Iterables.tryFind(this.connections, c -> id == c.getAccount().id); + if (configured.isPresent()) { + return Futures.immediateFuture(configured.get()); + } + return Futures.transform( + ConversationsDatabase.getInstance(context).accountDao().getEnabledAccount(id), + account -> { + if (account == null) { + throw new IllegalStateException( + String.format("No enabled account with id %d", id)); + } + return reconfigure(account); + }, + reconfigurationExecutor); + } + + private synchronized boolean isEnabled(final long id) { return Iterables.any(this.connections, c -> id == c.getAccount().id); } @@ -76,9 +114,7 @@ public class ConnectionPool { final Set removed = Sets.difference(current, accounts); final Set added = Sets.difference(accounts, current); for (final Account account : added) { - final XmppConnection connection = this.instantiate(context, account); - connection.setOnStatusChangedListener(this::onStatusChanged); - reconnectAccount(connection); + this.setupXmppConnection(context, account); } for (final Account account : removed) { final Optional connectionOptional = @@ -324,9 +360,11 @@ public class ConnectionPool { } } - private XmppConnection instantiate(final Context context, final Account account) { + private XmppConnection setupXmppConnection(final Context context, final Account account) { final XmppConnection xmppConnection = new XmppConnection(context, account); this.connections.add(xmppConnection); + xmppConnection.setOnStatusChangedListener(this::onStatusChanged); + reconnectAccount(xmppConnection); return xmppConnection; } diff --git a/src/main/java/im/conversations/android/xmpp/XmppConnection.java b/src/main/java/im/conversations/android/xmpp/XmppConnection.java index 5a64a3eb0..19fb9a4c5 100644 --- a/src/main/java/im/conversations/android/xmpp/XmppConnection.java +++ b/src/main/java/im/conversations/android/xmpp/XmppConnection.java @@ -1979,6 +1979,7 @@ public class XmppConnection implements Runnable { } private void finalizeBind() { + this.enableAdvancedStreamFeatures(); this.bindConsumer.accept(this.connectionAddress); this.changeStatusToOnline(); } @@ -1989,10 +1990,6 @@ public class XmppConnection implements Runnable { && !this.carbonsEnabled) { sendEnableCarbons(); } - // TODO discover commands - /*if (getFeatures().commands()) { - discoverCommands(); - }*/ } private void sendEnableCarbons() {