From 93cd31fa2d9b4b002af3785b874dbfd15d900d66 Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Wed, 19 Oct 2011 15:54:56 +0100 Subject: [PATCH] Simplify Database methods, move logic to DatabaseComponent. --- .../sf/briar/api/db/DatabaseComponent.java | 3 + components/net/sf/briar/db/Database.java | 55 ++-- .../sf/briar/db/DatabaseComponentImpl.java | 57 +++- .../net/sf/briar/db/DatabaseConstants.java | 36 ++- components/net/sf/briar/db/JdbcDatabase.java | 248 +++++++++--------- .../sf/briar/plugins/PluginManagerImpl.java | 2 +- .../sf/briar/db/DatabaseComponentTest.java | 16 +- test/net/sf/briar/db/H2DatabaseTest.java | 8 +- .../briar/plugins/PluginManagerImplTest.java | 6 +- 9 files changed, 256 insertions(+), 175 deletions(-) diff --git a/api/net/sf/briar/api/db/DatabaseComponent.java b/api/net/sf/briar/api/db/DatabaseComponent.java index 047ee2a0d4..e8b1d51fd2 100644 --- a/api/net/sf/briar/api/db/DatabaseComponent.java +++ b/api/net/sf/briar/api/db/DatabaseComponent.java @@ -121,6 +121,9 @@ public interface DatabaseComponent { /** Returns the IDs of all contacts. */ Collection<ContactId> getContacts() throws DbException; + /** Returns the local transport properties for the given transport. */ + TransportProperties getLocalProperties(TransportId t) throws DbException; + /** Returns all local transport properties. */ Map<TransportId, TransportProperties> getLocalTransports() throws DbException; diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java index 7f5e16fcbd..90ec752112 100644 --- a/components/net/sf/briar/db/Database.java +++ b/components/net/sf/briar/db/Database.java @@ -40,12 +40,6 @@ import net.sf.briar.api.transport.ConnectionWindow; */ interface Database<T> { - /** - * A batch sent to a contact is considered lost when this many more - * recently sent batches have been acknowledged. - */ - static final int RETRANSMIT_THRESHOLD = 5; - /** * Opens the database. * @param resume True to reopen an existing database, false to create a @@ -220,6 +214,14 @@ interface Database<T> { */ MessageId getGroupMessageParent(T txn, MessageId m) throws DbException; + /** + * Returns the local transport properties for the given transport. + * <p> + * Locking: transports read. + */ + TransportProperties getLocalProperties(T txn, TransportId t) + throws DbException; + /** * Returns all local transport properties. * <p> @@ -406,15 +408,13 @@ interface Database<T> { void removeMessage(T txn, MessageId m) throws DbException; /** - * Unsubscribes from the given group and returns the IDs of any contacts - * affected by the change. Any messages belonging to the group are deleted - * from the database. + * Unsubscribes from the given group. Any messages belonging to the group + * are deleted from the database. * <p> * Locking: contacts read, messages write, messageStatuses write, * subscriptions write. */ - Collection<ContactId> removeSubscription(T txn, GroupId g) - throws DbException; + void removeSubscription(T txn, GroupId g) throws DbException; /** * Sets the configuration for the given transport, replacing any existing @@ -436,12 +436,11 @@ interface Database<T> { /** * Sets the local transport properties for the given transport, replacing - * any existing properties for that transport. Returns true if the - * properties have changed. + * any existing properties for that transport. * <p> * Locking: transports write. */ - boolean setLocalProperties(T txn, TransportId t, TransportProperties p) + void setLocalProperties(T txn, TransportId t, TransportProperties p) throws DbException; /** @@ -487,13 +486,22 @@ interface Database<T> { void setSubscriptions(T txn, ContactId c, Map<Group, Long> subs, long timestamp) throws DbException; + /** + * Records the time at which the subscriptions visible to the given contacts + * were last modified. + * <p> + * Locking: contacts read, subscriptions write. + */ + void setSubscriptionsModifiedTimestamp(T txn, + Collection<ContactId> contacts, long timestamp) throws DbException; + /** * Records the time at which a subscription update was last sent to the * given contact. * <p> * Locking: contacts read, subscriptions write. */ - void setSubscriptionTimestamp(T txn, ContactId c, long timestamp) + void setSubscriptionsSentTimestamp(T txn, ContactId c, long timestamp) throws DbException; /** @@ -507,22 +515,29 @@ interface Database<T> { Map<TransportId, TransportProperties> transports, long timestamp) throws DbException; + /** + * Records the time at which the local transports were last modified. + * <p> + * Locking: contacts read, transports write. + */ + void setTransportsModifiedTimestamp(T txn, long timestamp) + throws DbException; + /** * Records the time at which a transport update was last sent to the given * contact. * <p> * Locking: contacts read, transports write. */ - void setTransportTimestamp(T txn, ContactId c, long timestamp) + void setTransportsSentTimestamp(T txn, ContactId c, long timestamp) throws DbException; /** * Makes the given group visible to the given set of contacts and invisible - * to any other contacts. Returns the IDs of any contacts affected by the - * change. + * to any other contacts. * <p> * Locking: contacts read, subscriptions write. */ - Collection<ContactId> setVisibility(T txn, GroupId g, - Collection<ContactId> visible) throws DbException; + void setVisibility(T txn, GroupId g, Collection<ContactId> visible) + throws DbException; } diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java index ab0a56ab63..124974d91e 100644 --- a/components/net/sf/briar/db/DatabaseComponentImpl.java +++ b/components/net/sf/briar/db/DatabaseComponentImpl.java @@ -11,6 +11,7 @@ import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -595,7 +596,7 @@ DatabaseCleaner.Callback { try { subs = db.getVisibleSubscriptions(txn, c); timestamp = System.currentTimeMillis(); - db.setSubscriptionTimestamp(txn, c, timestamp); + db.setSubscriptionsSentTimestamp(txn, c, timestamp); db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -625,7 +626,7 @@ DatabaseCleaner.Callback { try { transports = db.getLocalTransports(txn); timestamp = System.currentTimeMillis(); - db.setTransportTimestamp(txn, c, timestamp); + db.setTransportsSentTimestamp(txn, c, timestamp); db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -724,6 +725,24 @@ DatabaseCleaner.Callback { } } + public TransportProperties getLocalProperties(TransportId t) + throws DbException { + transportLock.readLock().lock(); + try { + T txn = db.startTransaction(); + try { + TransportProperties p = db.getLocalProperties(txn, t); + db.commitTransaction(txn); + return p; + } catch(DbException e) { + db.abortTransaction(txn); + throw e; + } + } finally { + transportLock.readLock().unlock(); + } + } + public Map<TransportId, TransportProperties> getLocalTransports() throws DbException { transportLock.readLock().lock(); @@ -1111,13 +1130,13 @@ DatabaseCleaner.Callback { callListeners(new ContactRemovedEvent(c)); } - public void setConfig(TransportId t, TransportConfig config) + public void setConfig(TransportId t, TransportConfig c) throws DbException { transportLock.writeLock().lock(); try { T txn = db.startTransaction(); try { - db.setConfig(txn, t, config); + db.setConfig(txn, t, c); db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -1150,14 +1169,19 @@ DatabaseCleaner.Callback { } } - public void setLocalProperties(TransportId t, - TransportProperties properties) throws DbException { - boolean changed; + public void setLocalProperties(TransportId t, TransportProperties p) + throws DbException { + boolean changed = false; transportLock.writeLock().lock(); try { T txn = db.startTransaction(); try { - changed = db.setLocalProperties(txn, t, properties); + if(!p.equals(db.getLocalProperties(txn, t))) { + db.setLocalProperties(txn, t, p); + db.setTransportsModifiedTimestamp(txn, + System.currentTimeMillis()); + changed = true; + } db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -1266,16 +1290,20 @@ DatabaseCleaner.Callback { public void setVisibility(GroupId g, Collection<ContactId> visible) throws DbException { - Collection<ContactId> affected; + // Use HashSets for O(1) lookups, giving O(n) overall running time + HashSet<ContactId> then, now; contactLock.readLock().lock(); try { subscriptionLock.writeLock().lock(); try { T txn = db.startTransaction(); try { + // Retrieve the group's current visibility + then = new HashSet<ContactId>(db.getVisibility(txn, g)); // Don't try to make the group visible to ex-contacts - visible.retainAll((db.getContacts(txn))); - affected = db.setVisibility(txn, g, visible); + now = new HashSet<ContactId>(visible); + now.retainAll(new HashSet<ContactId>(db.getContacts(txn))); + db.setVisibility(txn, g, now); db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -1287,6 +1315,10 @@ DatabaseCleaner.Callback { } finally { contactLock.readLock().unlock(); } + // Work out which contacts were affected by the change + Collection<ContactId> affected = new ArrayList<ContactId>(); + for(ContactId c : then) if(!now.contains(c)) affected.add(c); + for(ContactId c : now) if(!then.contains(c)) affected.add(c); // Call the listeners outside the lock if(!affected.isEmpty()) callListeners(new SubscriptionsUpdatedEvent(affected)); @@ -1325,7 +1357,8 @@ DatabaseCleaner.Callback { T txn = db.startTransaction(); try { if(db.containsSubscription(txn, g)) { - affected = db.removeSubscription(txn, g); + affected = db.getVisibility(txn, g); + db.removeSubscription(txn, g); removed = true; } db.commitTransaction(txn); diff --git a/components/net/sf/briar/db/DatabaseConstants.java b/components/net/sf/briar/db/DatabaseConstants.java index e2c2991062..737e97054f 100644 --- a/components/net/sf/briar/db/DatabaseConstants.java +++ b/components/net/sf/briar/db/DatabaseConstants.java @@ -3,14 +3,48 @@ package net.sf.briar.db; interface DatabaseConstants { // FIXME: These should be configurable + + /** + * The minimum amount of space in bytes that should be kept free on the + * device where the database is stored. Whenever less than this much space + * is free, old messages will be expired from the database. + */ static final long MIN_FREE_SPACE = 300 * 1024 * 1024; // 300 MiB + + /** + * The minimum amount of space in bytes that must be kept free on the device + * where the database is stored. If less than this much space is free and + * there are no more messages to expire, the program will shut down. + */ static final long CRITICAL_FREE_SPACE = 100 * 1024 * 1024; // 100 MiB + /** + * The amount of free space will be checked whenever this many bytes of + * messages have been added to the database since the last check. + */ static final int MAX_BYTES_BETWEEN_SPACE_CHECKS = 5 * 1024 * 1024; // 5 MiB + + /** + * The amount of free space will be checked whenever this many milliseconds + * have passed since the last check. + */ static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min - static final long MS_PER_SWEEP = 10L * 1000L; // 10 sec + /** + * Up to this many bytes of messages will be expired from the database each + * time it is necessary to expire messages. + */ static final int BYTES_PER_SWEEP = 5 * 1024 * 1024; // 5 MiB + /** + * The timestamp of the oldest message in the database is rounded using + * this modulus to avoid revealing the presence of any particular message. + */ static final long EXPIRY_MODULUS = 60L * 60L * 1000L; // 1 hour + + /** + * A batch sent to a contact is considered lost when this many more + * recently sent batches have been acknowledged. + */ + static final int RETRANSMIT_THRESHOLD = 5; } diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java index a9f9440b4f..9565b54184 100644 --- a/components/net/sf/briar/db/JdbcDatabase.java +++ b/components/net/sf/briar/db/JdbcDatabase.java @@ -3,7 +3,6 @@ package net.sf.briar.db; import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; -import java.sql.Blob; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -12,9 +11,7 @@ import java.sql.Statement; import java.sql.Types; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedList; import java.util.Map; import java.util.Map.Entry; @@ -272,8 +269,7 @@ abstract class JdbcDatabase implements Database<Connection> { if(resume) { if(LOG.isLoggable(Level.FINE)) LOG.fine(getNumberOfMessages(txn) + " messages"); - } - else { + } else { if(LOG.isLoggable(Level.FINE)) LOG.fine("Creating database tables"); createTables(txn); @@ -384,13 +380,11 @@ abstract class JdbcDatabase implements Database<Connection> { } } catch(SQLException e) { // Try to close the connection - if(LOG.isLoggable(Level.WARNING)) - LOG.warning(e.getMessage()); + if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage()); try { txn.close(); } catch(SQLException e1) { - if(LOG.isLoggable(Level.WARNING)) - LOG.warning(e1.getMessage()); + if(LOG.isLoggable(Level.WARNING)) LOG.warning(e1.getMessage()); } // Whatever happens, allow the database to close synchronized(connections) { @@ -823,11 +817,11 @@ abstract class JdbcDatabase implements Database<Connection> { ps = txn.prepareStatement(sql); ps.setInt(1, t.getInt()); rs = ps.executeQuery(); - TransportConfig config = new TransportConfig(); - while(rs.next()) config.put(rs.getString(1), rs.getString(2)); + TransportConfig c = new TransportConfig(); + while(rs.next()) c.put(rs.getString(1), rs.getString(2)); rs.close(); ps.close(); - return config; + return c; } catch(SQLException e) { tryToClose(rs); tryToClose(ps); @@ -847,6 +841,7 @@ abstract class JdbcDatabase implements Database<Connection> { ps.setInt(2, t.getInt()); rs = ps.executeQuery(); if(rs.next()) { + // A connection window row exists - update it long outgoing = rs.getLong(1); if(rs.next()) throw new DbStateException(); rs.close(); @@ -862,6 +857,7 @@ abstract class JdbcDatabase implements Database<Connection> { ps.close(); return outgoing; } else { + // No connection window row exists - create one rs.close(); ps.close(); sql = "INSERT INTO connectionWindows" @@ -967,6 +963,28 @@ abstract class JdbcDatabase implements Database<Connection> { } } + public TransportProperties getLocalProperties(Connection txn, TransportId t) + throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = "SELECT key, value FROM transports" + + " WHERE transportId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, t.getInt()); + rs = ps.executeQuery(); + TransportProperties p = new TransportProperties(); + while(rs.next()) p.put(rs.getString(1), rs.getString(2)); + rs.close(); + ps.close(); + return p; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + public Map<TransportId, TransportProperties> getLocalTransports( Connection txn) throws DbException { PreparedStatement ps = null; @@ -978,15 +996,15 @@ abstract class JdbcDatabase implements Database<Connection> { rs = ps.executeQuery(); Map<TransportId, TransportProperties> transports = new HashMap<TransportId, TransportProperties>(); - TransportProperties properties = null; + TransportProperties p = null; TransportId lastId = null; while(rs.next()) { TransportId id = new TransportId(rs.getInt(1)); if(!id.equals(lastId)) { - properties = new TransportProperties(); - transports.put(id, properties); + p = new TransportProperties(); + transports.put(id, p); } - properties.put(rs.getString(2), rs.getString(3)); + p.put(rs.getString(2), rs.getString(3)); } rs.close(); ps.close(); @@ -1007,7 +1025,7 @@ abstract class JdbcDatabase implements Database<Connection> { + " WHERE contactId = ? AND passover >= ?"; ps = txn.prepareStatement(sql); ps.setInt(1, c.getInt()); - ps.setInt(2, RETRANSMIT_THRESHOLD); + ps.setInt(2, DatabaseConstants.RETRANSMIT_THRESHOLD); rs = ps.executeQuery(); Collection<BatchId> ids = new ArrayList<BatchId>(); while(rs.next()) ids.add(new BatchId(rs.getBytes(1))); @@ -1031,8 +1049,7 @@ abstract class JdbcDatabase implements Database<Connection> { rs = ps.executeQuery(); if(!rs.next()) throw new DbStateException(); int size = rs.getInt(1); - Blob b = rs.getBlob(2); - byte[] raw = b.getBytes(1, size); + byte[] raw = rs.getBlob(2).getBytes(1, size); if(raw.length != size) throw new DbStateException(); if(rs.next()) throw new DbStateException(); rs.close(); @@ -1063,8 +1080,7 @@ abstract class JdbcDatabase implements Database<Connection> { byte[] raw = null; if(rs.next()) { int size = rs.getInt(1); - Blob b = rs.getBlob(2); - raw = b.getBytes(1, size); + raw = rs.getBlob(2).getBytes(1, size); if(raw.length != size) throw new DbStateException(); } if(rs.next()) throw new DbStateException(); @@ -1093,8 +1109,7 @@ abstract class JdbcDatabase implements Database<Connection> { rs = ps.executeQuery(); if(rs.next()) { int size = rs.getInt(1); - Blob b = rs.getBlob(2); - raw = b.getBytes(1, size); + raw = rs.getBlob(2).getBytes(1, size); if(raw.length != size) throw new DbStateException(); } if(rs.next()) throw new DbStateException(); @@ -1160,7 +1175,7 @@ abstract class JdbcDatabase implements Database<Connection> { ps.setBytes(1, m.getBytes()); rs = ps.executeQuery(); if(!rs.next()) throw new DbStateException(); - byte[] group = rs.getBytes(1); + byte[] groupId = rs.getBytes(1); if(rs.next()) throw new DbStateException(); rs.close(); ps.close(); @@ -1169,7 +1184,7 @@ abstract class JdbcDatabase implements Database<Connection> { + " AND sendability > ZERO()"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getBytes()); - ps.setBytes(2, group); + ps.setBytes(2, groupId); rs = ps.executeQuery(); if(!rs.next()) throw new DbStateException(); int count = rs.getInt(1); @@ -1249,12 +1264,12 @@ abstract class JdbcDatabase implements Database<Connection> { Map<ContactId, TransportProperties> properties = new HashMap<ContactId, TransportProperties>(); TransportProperties p = null; - ContactId lastContactId = null; + ContactId lastId = null; while(rs.next()) { - ContactId contactId = new ContactId(rs.getInt(1)); - if(!contactId.equals(lastContactId)) { + ContactId id = new ContactId(rs.getInt(1)); + if(!id.equals(lastId)) { p = new TransportProperties(); - properties.put(contactId, p); + properties.put(id, p); } p.put(rs.getString(2), rs.getString(3)); } @@ -1504,8 +1519,8 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public Map<Group, Long> getVisibleSubscriptions(Connection txn, - ContactId c) throws DbException { + public Map<Group, Long> getVisibleSubscriptions(Connection txn, ContactId c) + throws DbException { long expiry = getApproximateExpiryTime(txn); PreparedStatement ps = null; ResultSet rs = null; @@ -1754,36 +1769,23 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public Collection<ContactId> removeSubscription(Connection txn, GroupId g) + public void removeSubscription(Connection txn, GroupId g) throws DbException { PreparedStatement ps = null; - ResultSet rs = null; try { - // Retrieve the contacts to which the subscription is visible - String sql = "SELECT contactId FROM visibilities WHERE groupId = ?"; - ps = txn.prepareStatement(sql); - ps.setBytes(1, g.getBytes()); - rs = ps.executeQuery(); - Collection<ContactId> visible = new ArrayList<ContactId>(); - while(rs.next()) visible.add(new ContactId(rs.getInt(1))); - rs.close(); - ps.close(); - // Delete the subscription - sql = "DELETE FROM subscriptions WHERE groupId = ?"; + String sql = "DELETE FROM subscriptions WHERE groupId = ?"; ps = txn.prepareStatement(sql); ps.setBytes(1, g.getBytes()); int affected = ps.executeUpdate(); if(affected != 1) throw new DbStateException(); ps.close(); - return visible; } catch(SQLException e) { - tryToClose(rs); tryToClose(ps); throw new DbException(e); } } - public void setConfig(Connection txn, TransportId t, TransportConfig config) + public void setConfig(Connection txn, TransportId t, TransportConfig c) throws DbException { PreparedStatement ps = null; try { @@ -1798,13 +1800,13 @@ abstract class JdbcDatabase implements Database<Connection> { + " VALUES (?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setInt(1, t.getInt()); - for(Entry<String, String> e : config.entrySet()) { + for(Entry<String, String> e : c.entrySet()) { ps.setString(2, e.getKey()); ps.setString(3, e.getValue()); ps.addBatch(); } int[] batchAffected = ps.executeBatch(); - if(batchAffected.length != config.size()) + if(batchAffected.length != c.size()) throw new DbStateException(); for(int i = 0; i < batchAffected.length; i++) { if(batchAffected[i] != 1) throw new DbStateException(); @@ -1827,10 +1829,12 @@ abstract class JdbcDatabase implements Database<Connection> { ps.setInt(1, c.getInt()); ps.setInt(2, t.getInt()); rs = ps.executeQuery(); - if(rs.next()) { - if(rs.next()) throw new DbStateException(); - rs.close(); - ps.close(); + boolean found = rs.next(); + if(rs.next()) throw new DbStateException(); + rs.close(); + ps.close(); + if(found) { + // A connection window row exists - update it sql = "UPDATE connectionWindows SET centre = ?, bitmap = ?" + " WHERE contactId = ? AND transportId = ?"; ps = txn.prepareStatement(sql); @@ -1842,8 +1846,7 @@ abstract class JdbcDatabase implements Database<Connection> { if(affected != 1) throw new DbStateException(); ps.close(); } else { - rs.close(); - ps.close(); + // No connection window row exists - create one sql = "INSERT INTO connectionWindows" + " (contactId, transportId, centre, bitmap, outgoing)" + " VALUES(?, ?, ?, ?, ZERO())"; @@ -1863,25 +1866,12 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public boolean setLocalProperties(Connection txn, TransportId t, - TransportProperties properties) throws DbException { + public void setLocalProperties(Connection txn, TransportId t, + TransportProperties p) throws DbException { PreparedStatement ps = null; - ResultSet rs = null; try { - // Retrieve any existing properties for the given transport - String sql = "SELECT key, value FROM transports" - + " WHERE transportId = ?"; - ps = txn.prepareStatement(sql); - ps.setInt(1, t.getInt()); - rs = ps.executeQuery(); - TransportProperties old = new TransportProperties(); - while(rs.next()) old.put(rs.getString(1), rs.getString(2)); - rs.close(); - ps.close(); - // If the properties haven't changed, return - if(old.equals(properties)) return false; // Delete any existing properties for the given transport - sql = "DELETE FROM transports WHERE transportId = ?"; + String sql = "DELETE FROM transports WHERE transportId = ?"; ps = txn.prepareStatement(sql); ps.setInt(1, t.getInt()); ps.executeUpdate(); @@ -1891,27 +1881,19 @@ abstract class JdbcDatabase implements Database<Connection> { + " VALUES (?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setInt(1, t.getInt()); - for(Entry<String, String> e : properties.entrySet()) { + for(Entry<String, String> e : p.entrySet()) { ps.setString(2, e.getKey()); ps.setString(3, e.getValue()); ps.addBatch(); } int[] batchAffected = ps.executeBatch(); - if(batchAffected.length != properties.size()) + if(batchAffected.length != p.size()) throw new DbStateException(); for(int i = 0; i < batchAffected.length; i++) { if(batchAffected[i] != 1) throw new DbStateException(); } ps.close(); - // Update the transport timestamps of all contacts - sql = "UPDATE transportTimestamps set modified = ?"; - ps = txn.prepareStatement(sql); - ps.setLong(1, System.currentTimeMillis()); - ps.executeUpdate(); - ps.close(); - return true; } catch(SQLException e) { - tryToClose(rs); tryToClose(ps); throw new DbException(e); } @@ -1928,18 +1910,22 @@ abstract class JdbcDatabase implements Database<Connection> { rs = ps.executeQuery(); Rating old; if(rs.next()) { + // A rating row exists - update it old = Rating.values()[rs.getByte(1)]; if(rs.next()) throw new DbStateException(); rs.close(); ps.close(); - sql = "UPDATE ratings SET rating = ? WHERE authorId = ?"; - ps = txn.prepareStatement(sql); - ps.setShort(1, (short) r.ordinal()); - ps.setBytes(2, a.getBytes()); - int affected = ps.executeUpdate(); - if(affected != 1) throw new DbStateException(); - ps.close(); + if(!old.equals(r)) { + sql = "UPDATE ratings SET rating = ? WHERE authorId = ?"; + ps = txn.prepareStatement(sql); + ps.setShort(1, (short) r.ordinal()); + ps.setBytes(2, a.getBytes()); + int affected = ps.executeUpdate(); + if(affected != 1) throw new DbStateException(); + ps.close(); + } } else { + // No rating row exists - create one rs.close(); ps.close(); old = Rating.UNRATED; @@ -1989,6 +1975,7 @@ abstract class JdbcDatabase implements Database<Connection> { ps.setInt(2, c.getInt()); rs = ps.executeQuery(); if(rs.next()) { + // A status row exists - update it Status old = Status.values()[rs.getByte(1)]; if(rs.next()) throw new DbStateException(); rs.close(); @@ -2005,6 +1992,7 @@ abstract class JdbcDatabase implements Database<Connection> { ps.close(); } } else { + // No status row exists - create one rs.close(); ps.close(); sql = "INSERT INTO statuses (messageId, contactId, status)" @@ -2117,13 +2105,39 @@ abstract class JdbcDatabase implements Database<Connection> { int affected = ps.executeUpdate(); if(affected != 1) throw new DbStateException(); ps.close(); + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + + public void setSubscriptionsModifiedTimestamp(Connection txn, + Collection<ContactId> contacts, long timestamp) throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE subscriptionTimestamps SET modified = ?" + + " WHERE contactId = ?"; + ps = txn.prepareStatement(sql); + ps.setLong(1, timestamp); + for(ContactId c : contacts) { + ps.setInt(2, c.getInt()); + ps.addBatch(); + } + int[] batchAffected = ps.executeBatch(); + if(batchAffected.length != contacts.size()) + throw new DbStateException(); + for(int i = 0; i < batchAffected.length; i++) { + if(batchAffected[i] > 1) throw new DbStateException(); + } + ps.close(); } catch(SQLException e) { tryToClose(ps); throw new DbException(e); } } - public void setSubscriptionTimestamp(Connection txn, ContactId c, + public void setSubscriptionsSentTimestamp(Connection txn, ContactId c, long timestamp) throws DbException { PreparedStatement ps = null; try { @@ -2199,12 +2213,28 @@ abstract class JdbcDatabase implements Database<Connection> { if(affected != 1) throw new DbStateException(); ps.close(); } catch(SQLException e) { + tryToClose(rs); tryToClose(ps); throw new DbException(e); } } - public void setTransportTimestamp(Connection txn, ContactId c, + public void setTransportsModifiedTimestamp(Connection txn, long timestamp) + throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE transportTimestamps set modified = ?"; + ps = txn.prepareStatement(sql); + ps.setLong(1, timestamp); + ps.executeUpdate(); + ps.close(); + } catch(SQLException e) { + tryToClose(ps); + throw new DbException(e); + } + } + + public void setTransportsSentTimestamp(Connection txn, ContactId c, long timestamp) throws DbException { PreparedStatement ps = null; try { @@ -2223,25 +2253,12 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public Collection<ContactId> setVisibility(Connection txn, GroupId g, + public void setVisibility(Connection txn, GroupId g, Collection<ContactId> visible) throws DbException { PreparedStatement ps = null; - ResultSet rs = null; try { - // Retrieve any existing visibilities - String sql = "SELECT contactId FROM visibilities WHERE groupId = ?"; - ps = txn.prepareStatement(sql); - ps.setBytes(1, g.getBytes()); - rs = ps.executeQuery(); - Collection<ContactId> then = new HashSet<ContactId>(); - while(rs.next()) then.add(new ContactId(rs.getInt(1))); - rs.close(); - ps.close(); - // If the visibilities haven't changed, return - Collection<ContactId> now = new HashSet<ContactId>(visible); - if(then.equals(now)) return Collections.emptyList(); // Delete any existing visibilities - sql = "DELETE FROM visibilities where groupId = ?"; + String sql = "DELETE FROM visibilities where groupId = ?"; ps = txn.prepareStatement(sql); ps.setBytes(1, g.getBytes()); ps.executeUpdate(); @@ -2262,28 +2279,7 @@ abstract class JdbcDatabase implements Database<Connection> { if(batchAffected[i] != 1) throw new DbStateException(); } ps.close(); - // Update the subscription timestamps of any affected contacts - Collection<ContactId> affected = new ArrayList<ContactId>(); - for(ContactId c : then) if(!now.contains(c)) affected.add(c); - for(ContactId c : now) if(!then.contains(c)) affected.add(c); - sql = "UPDATE subscriptionTimestamps SET modified = ?" - + " WHERE contactId = ?"; - ps = txn.prepareStatement(sql); - ps.setLong(1, System.currentTimeMillis()); - for(ContactId c : affected) { - ps.setInt(2, c.getInt()); - ps.addBatch(); - } - batchAffected = ps.executeBatch(); - if(batchAffected.length != affected.size()) - throw new DbStateException(); - for(int i = 0; i < batchAffected.length; i++) { - if(batchAffected[i] > 1) throw new DbStateException(); - } - ps.close(); - return affected; } catch(SQLException e) { - tryToClose(rs); tryToClose(ps); throw new DbException(e); } diff --git a/components/net/sf/briar/plugins/PluginManagerImpl.java b/components/net/sf/briar/plugins/PluginManagerImpl.java index adb10ed2c3..beb11d06e6 100644 --- a/components/net/sf/briar/plugins/PluginManagerImpl.java +++ b/components/net/sf/briar/plugins/PluginManagerImpl.java @@ -190,7 +190,7 @@ class PluginManagerImpl implements PluginManager { public TransportProperties getLocalProperties() { assert id != null; try { - TransportProperties p = db.getLocalTransports().get(id); + TransportProperties p = db.getLocalProperties(id); return p == null ? new TransportProperties() : p; } catch(DbException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage()); diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java index bb8cfa73ac..65c0acea60 100644 --- a/test/net/sf/briar/db/DatabaseComponentTest.java +++ b/test/net/sf/briar/db/DatabaseComponentTest.java @@ -148,8 +148,9 @@ public abstract class DatabaseComponentTest extends TestCase { // unsubscribe(groupId) oneOf(database).containsSubscription(txn, groupId); will(returnValue(true)); - oneOf(database).removeSubscription(txn, groupId); + oneOf(database).getVisibility(txn, groupId); will(returnValue(Collections.<ContactId>emptyList())); + oneOf(database).removeSubscription(txn, groupId); // unsubscribe(groupId) again oneOf(database).containsSubscription(txn, groupId); will(returnValue(false)); @@ -755,7 +756,7 @@ public abstract class DatabaseComponentTest extends TestCase { // Get the visible subscriptions oneOf(database).getVisibleSubscriptions(txn, contactId); will(returnValue(Collections.singletonMap(group, 0L))); - oneOf(database).setSubscriptionTimestamp(with(txn), with(contactId), + oneOf(database).setSubscriptionsSentTimestamp(with(txn), with(contactId), with(any(long.class))); // Add the subscriptions to the writer oneOf(subscriptionWriter).writeSubscriptions( @@ -790,7 +791,7 @@ public abstract class DatabaseComponentTest extends TestCase { // Get the local transport properties oneOf(database).getLocalTransports(txn); will(returnValue(transports)); - oneOf(database).setTransportTimestamp(with(txn), with(contactId), + oneOf(database).setTransportsSentTimestamp(with(txn), with(contactId), with(any(long.class))); // Add the properties to the writer oneOf(transportWriter).writeTransports(with(transports), @@ -1283,8 +1284,11 @@ public abstract class DatabaseComponentTest extends TestCase { context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); + oneOf(database).getLocalProperties(txn, transportId); + will(returnValue(new TransportProperties())); oneOf(database).setLocalProperties(txn, transportId, properties); - will(returnValue(true)); + oneOf(database).setTransportsModifiedTimestamp(with(txn), + with(any(long.class))); oneOf(database).commitTransaction(txn); oneOf(listener).eventOccurred(with(any( TransportsUpdatedEvent.class))); @@ -1310,8 +1314,8 @@ public abstract class DatabaseComponentTest extends TestCase { context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).setLocalProperties(txn, transportId, properties); - will(returnValue(false)); + oneOf(database).getLocalProperties(txn, transportId); + will(returnValue(properties)); oneOf(database).commitTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner); diff --git a/test/net/sf/briar/db/H2DatabaseTest.java b/test/net/sf/briar/db/H2DatabaseTest.java index fbbbbd3c60..64b2bce7b0 100644 --- a/test/net/sf/briar/db/H2DatabaseTest.java +++ b/test/net/sf/briar/db/H2DatabaseTest.java @@ -719,7 +719,7 @@ public class H2DatabaseTest extends TestCase { @Test public void testRetransmission() throws Exception { - BatchId[] ids = new BatchId[Database.RETRANSMIT_THRESHOLD + 5]; + BatchId[] ids = new BatchId[DatabaseConstants.RETRANSMIT_THRESHOLD + 5]; for(int i = 0; i < ids.length; i++) { ids[i] = new BatchId(TestUtils.getRandomId()); } @@ -740,7 +740,7 @@ public class H2DatabaseTest extends TestCase { // The contact acks the batches in reverse order. The first // RETRANSMIT_THRESHOLD - 1 acks should not trigger any retransmissions - for(int i = 0; i < Database.RETRANSMIT_THRESHOLD - 1; i++) { + for(int i = 0; i < DatabaseConstants.RETRANSMIT_THRESHOLD - 1; i++) { db.removeAckedBatch(txn, contactId, ids[ids.length - i - 1]); Collection<BatchId> lost = db.getLostBatches(txn, contactId); assertEquals(Collections.emptyList(), lost); @@ -748,7 +748,7 @@ public class H2DatabaseTest extends TestCase { // The next ack should trigger the retransmission of the remaining // five outstanding batches - int index = ids.length - Database.RETRANSMIT_THRESHOLD; + int index = ids.length - DatabaseConstants.RETRANSMIT_THRESHOLD; db.removeAckedBatch(txn, contactId, ids[index]); Collection<BatchId> lost = db.getLostBatches(txn, contactId); for(int i = 0; i < index; i++) { @@ -761,7 +761,7 @@ public class H2DatabaseTest extends TestCase { @Test public void testNoRetransmission() throws Exception { - BatchId[] ids = new BatchId[Database.RETRANSMIT_THRESHOLD * 2]; + BatchId[] ids = new BatchId[DatabaseConstants.RETRANSMIT_THRESHOLD * 2]; for(int i = 0; i < ids.length; i++) { ids[i] = new BatchId(TestUtils.getRandomId()); } diff --git a/test/net/sf/briar/plugins/PluginManagerImplTest.java b/test/net/sf/briar/plugins/PluginManagerImplTest.java index f8260fa81a..3d00aa5706 100644 --- a/test/net/sf/briar/plugins/PluginManagerImplTest.java +++ b/test/net/sf/briar/plugins/PluginManagerImplTest.java @@ -1,6 +1,5 @@ package net.sf.briar.plugins; -import java.util.Map; import java.util.concurrent.Executor; import junit.framework.TestCase; @@ -20,14 +19,11 @@ public class PluginManagerImplTest extends TestCase { public void testStartAndStop() throws Exception { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); - final Map<?, ?> localTransports = context.mock(Map.class); final ConnectionDispatcher dispatcher = context.mock(ConnectionDispatcher.class); final UiCallback uiCallback = context.mock(UiCallback.class); context.checking(new Expectations() {{ - allowing(db).getLocalTransports(); - will(returnValue(localTransports)); - allowing(localTransports).get(with(any(TransportId.class))); + allowing(db).getLocalProperties(with(any(TransportId.class))); will(returnValue(new TransportProperties())); allowing(db).getRemoteProperties(with(any(TransportId.class))); will(returnValue(new TransportProperties())); -- GitLab