diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java index 90ec75211256207962eb0f92d2ba33549e11cc27..f5dd6e39ae874826c29277a7aea34024615134e2 100644 --- a/components/net/sf/briar/db/Database.java +++ b/components/net/sf/briar/db/Database.java @@ -344,6 +344,37 @@ interface Database<T> { */ Collection<Group> getSubscriptions(T txn, ContactId c) throws DbException; + /** + * Returns the time at which the subscriptions visible to the given contact + * were last modified. + * <p> + * Locking: contacts read, subscriptions read. + */ + long getSubscriptionsModified(T txn, ContactId c) throws DbException; + + /** + * Returns the time at which a subscription update was last sent to the + * given contact. + * <p> + * Locking: contacts read, subscriptions read. + */ + long getSubscriptionsSent(T txn, ContactId c) throws DbException; + + /** + * Returns the time at which the local transports were last modified. + * <p> + * Locking: transports read. + */ + long getTransportsModified(T txn) throws DbException; + + /** + * Returns the time at which a transport update was last sent to the given + * contact. + * <p> + * Locking: contacts read, transports read. + */ + long getTransportsSent(T txn, ContactId c) throws DbException; + /** * Returns the contacts to which the given group is visible. * <p> @@ -492,8 +523,8 @@ interface Database<T> { * <p> * Locking: contacts read, subscriptions write. */ - void setSubscriptionsModifiedTimestamp(T txn, - Collection<ContactId> contacts, long timestamp) throws DbException; + void setSubscriptionsModified(T txn, Collection<ContactId> contacts, + long timestamp) throws DbException; /** * Records the time at which a subscription update was last sent to the @@ -501,7 +532,7 @@ interface Database<T> { * <p> * Locking: contacts read, subscriptions write. */ - void setSubscriptionsSentTimestamp(T txn, ContactId c, long timestamp) + void setSubscriptionsSent(T txn, ContactId c, long timestamp) throws DbException; /** @@ -520,8 +551,7 @@ interface Database<T> { * <p> * Locking: contacts read, transports write. */ - void setTransportsModifiedTimestamp(T txn, long timestamp) - throws DbException; + void setTransportsModified(T txn, long timestamp) throws DbException; /** * Records the time at which a transport update was last sent to the given @@ -529,7 +559,7 @@ interface Database<T> { * <p> * Locking: contacts read, transports write. */ - void setTransportsSentTimestamp(T txn, ContactId c, long timestamp) + void setTransportsSent(T txn, ContactId c, long timestamp) throws DbException; /** diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java index 124974d91e20c66e1761ab6503a871ca72acc6ae..c89f788915e2392e352f2597858181526f087296 100644 --- a/components/net/sf/briar/db/DatabaseComponentImpl.java +++ b/components/net/sf/briar/db/DatabaseComponentImpl.java @@ -585,8 +585,8 @@ DatabaseCleaner.Callback { public void generateSubscriptionUpdate(ContactId c, SubscriptionWriter s) throws DbException, IOException { - Map<Group, Long> subs; - long timestamp; + Map<Group, Long> subs = null; + long timestamp = 0L; contactLock.readLock().lock(); try { if(!containsContact(c)) throw new NoSuchContactException(); @@ -594,9 +594,14 @@ DatabaseCleaner.Callback { try { T txn = db.startTransaction(); try { - subs = db.getVisibleSubscriptions(txn, c); - timestamp = System.currentTimeMillis(); - db.setSubscriptionsSentTimestamp(txn, c, timestamp); + // Work out whether an update is due + long modified = db.getSubscriptionsModified(txn, c); + long sent = db.getSubscriptionsSent(txn, c); + if(modified >= sent || updateIsDue(sent)) { + subs = db.getVisibleSubscriptions(txn, c); + timestamp = System.currentTimeMillis(); + db.setSubscriptionsSent(txn, c, timestamp); + } db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -608,15 +613,22 @@ DatabaseCleaner.Callback { } finally { contactLock.readLock().unlock(); } - s.writeSubscriptions(subs, timestamp); - if(LOG.isLoggable(Level.FINE)) - LOG.fine("Added " + subs.size() + " subscriptions to update"); + if(subs != null) { + s.writeSubscriptions(subs, timestamp); + if(LOG.isLoggable(Level.FINE)) + LOG.fine("Added " + subs.size() + " subscriptions to update"); + } + } + + private boolean updateIsDue(long sent) { + long now = System.currentTimeMillis(); + return now - sent >= DatabaseConstants.MAX_UPDATE_INTERVAL; } public void generateTransportUpdate(ContactId c, TransportWriter t) throws DbException, IOException { - Map<TransportId, TransportProperties> transports; - long timestamp; + Map<TransportId, TransportProperties> transports = null; + long timestamp = 0L; contactLock.readLock().lock(); try { if(!containsContact(c)) throw new NoSuchContactException(); @@ -624,9 +636,14 @@ DatabaseCleaner.Callback { try { T txn = db.startTransaction(); try { - transports = db.getLocalTransports(txn); - timestamp = System.currentTimeMillis(); - db.setTransportsSentTimestamp(txn, c, timestamp); + // Work out whether an update is due + long modified = db.getTransportsModified(txn); + long sent = db.getTransportsSent(txn, c); + if(modified >= sent || updateIsDue(sent)) { + transports = db.getLocalTransports(txn); + timestamp = System.currentTimeMillis(); + db.setTransportsSent(txn, c, timestamp); + } db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -638,9 +655,12 @@ DatabaseCleaner.Callback { } finally { contactLock.readLock().unlock(); } - t.writeTransports(transports, timestamp); - if(LOG.isLoggable(Level.FINE)) - LOG.fine("Added " + transports.size() + " transports to update"); + if(transports != null) { + t.writeTransports(transports, timestamp); + if(LOG.isLoggable(Level.FINE)) + LOG.fine("Added " + transports.size() + + " transports to update"); + } } public TransportConfig getConfig(TransportId t) throws DbException { @@ -1178,8 +1198,7 @@ DatabaseCleaner.Callback { try { if(!p.equals(db.getLocalProperties(txn, t))) { db.setLocalProperties(txn, t, p); - db.setTransportsModifiedTimestamp(txn, - System.currentTimeMillis()); + db.setTransportsModified(txn, System.currentTimeMillis()); changed = true; } db.commitTransaction(txn); @@ -1290,20 +1309,31 @@ DatabaseCleaner.Callback { public void setVisibility(GroupId g, Collection<ContactId> visible) throws DbException { - // Use HashSets for O(1) lookups, giving O(n) overall running time - HashSet<ContactId> then, now; + Collection<ContactId> affected; contactLock.readLock().lock(); try { subscriptionLock.writeLock().lock(); try { T txn = db.startTransaction(); try { + // Use HashSets for O(1) lookups, O(n) overall running time + HashSet<ContactId> then, now; // 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 now = new HashSet<ContactId>(visible); now.retainAll(new HashSet<ContactId>(db.getContacts(txn))); db.setVisibility(txn, g, now); + // Work out which contacts were affected by the change + 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); + } + db.setSubscriptionsModified(txn, affected, + System.currentTimeMillis()); db.commitTransaction(txn); } catch(DbException e) { db.abortTransaction(txn); @@ -1315,10 +1345,6 @@ 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)); @@ -1344,7 +1370,6 @@ DatabaseCleaner.Callback { public void unsubscribe(GroupId g) throws DbException { if(LOG.isLoggable(Level.FINE)) LOG.fine("Unsubscribing from " + g); - boolean removed = false; Collection<ContactId> affected = null; contactLock.readLock().lock(); try { @@ -1359,7 +1384,6 @@ DatabaseCleaner.Callback { if(db.containsSubscription(txn, g)) { affected = db.getVisibility(txn, g); db.removeSubscription(txn, g); - removed = true; } db.commitTransaction(txn); } catch(DbException e) { @@ -1379,7 +1403,7 @@ DatabaseCleaner.Callback { contactLock.readLock().unlock(); } // Call the listeners outside the lock - if(removed && !affected.isEmpty()) + if(affected != null && !affected.isEmpty()) callListeners(new SubscriptionsUpdatedEvent(affected)); } diff --git a/components/net/sf/briar/db/DatabaseConstants.java b/components/net/sf/briar/db/DatabaseConstants.java index 737e97054fc11aea936d52c81f30a4fcfef27d84..fa013a562d114d3d4657d57bf9150c5d968b1e5a 100644 --- a/components/net/sf/briar/db/DatabaseConstants.java +++ b/components/net/sf/briar/db/DatabaseConstants.java @@ -47,4 +47,10 @@ interface DatabaseConstants { * recently sent batches have been acknowledged. */ static final int RETRANSMIT_THRESHOLD = 5; + + /** + * The time in milliseconds after which a subscription or transport update + * should be sent to a contact even if no changes have occurred. + */ + static final long MAX_UPDATE_INTERVAL = 12L * 60L * 60L * 1000L; // 12 hours } diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java index 9565b54184becc977e704e58aae821f5e51ae8e6..a6d6adc66da6f54af6cf7bcdce4d9d099f512b32 100644 --- a/components/net/sf/briar/db/JdbcDatabase.java +++ b/components/net/sf/briar/db/JdbcDatabase.java @@ -1498,6 +1498,95 @@ abstract class JdbcDatabase implements Database<Connection> { } } + public long getSubscriptionsModified(Connection txn, ContactId c) + throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = "SELECT modified FROM subscriptionTimestamps" + + " WHERE contactId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + rs = ps.executeQuery(); + if(!rs.next()) throw new DbException(); + long modified = rs.getLong(1); + if(rs.next()) throw new DbException(); + rs.close(); + ps.close(); + return modified; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + + public long getSubscriptionsSent(Connection txn, ContactId c) + throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = "SELECT sent FROM subscriptionTimestamps" + + " WHERE contactId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + rs = ps.executeQuery(); + if(!rs.next()) throw new DbException(); + long sent = rs.getLong(1); + if(rs.next()) throw new DbException(); + rs.close(); + ps.close(); + return sent; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + + public long getTransportsModified(Connection txn) throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = "SELECT DISTINCT modified FROM transportTimestamps"; + ps = txn.prepareStatement(sql); + rs = ps.executeQuery(); + if(!rs.next()) throw new DbException(); + long modified = rs.getLong(1); + if(rs.next()) throw new DbException(); + rs.close(); + ps.close(); + return modified; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + + public long getTransportsSent(Connection txn, ContactId c) + throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = "SELECT sent FROM transportTimestamps" + + " WHERE contactId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + rs = ps.executeQuery(); + if(!rs.next()) throw new DbException(); + long sent = rs.getLong(1); + if(rs.next()) throw new DbException(); + rs.close(); + ps.close(); + return sent; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + public Collection<ContactId> getVisibility(Connection txn, GroupId g) throws DbException { PreparedStatement ps = null; @@ -2112,7 +2201,7 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public void setSubscriptionsModifiedTimestamp(Connection txn, + public void setSubscriptionsModified(Connection txn, Collection<ContactId> contacts, long timestamp) throws DbException { PreparedStatement ps = null; try { @@ -2137,7 +2226,7 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public void setSubscriptionsSentTimestamp(Connection txn, ContactId c, + public void setSubscriptionsSent(Connection txn, ContactId c, long timestamp) throws DbException { PreparedStatement ps = null; try { @@ -2219,7 +2308,7 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public void setTransportsModifiedTimestamp(Connection txn, long timestamp) + public void setTransportsModified(Connection txn, long timestamp) throws DbException { PreparedStatement ps = null; try { @@ -2234,8 +2323,8 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public void setTransportsSentTimestamp(Connection txn, ContactId c, - long timestamp) throws DbException { + public void setTransportsSent(Connection txn, ContactId c, long timestamp) + throws DbException { PreparedStatement ps = null; try { String sql = "UPDATE transportTimestamps SET sent = ?" diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java index 65c0acea600c24a7d465968bb67e5eab01afb7e2..fecd3f41830730210299ab7b4192e80a0032058f 100644 --- a/test/net/sf/briar/db/DatabaseComponentTest.java +++ b/test/net/sf/briar/db/DatabaseComponentTest.java @@ -735,6 +735,34 @@ public abstract class DatabaseComponentTest extends TestCase { context.assertIsSatisfied(); } + @Test + public void testSubscriptionUpdateNotSentUnlessDue() throws Exception { + final long now = System.currentTimeMillis(); + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + final SubscriptionWriter subscriptionWriter = + context.mock(SubscriptionWriter.class); + context.checking(new Expectations() {{ + allowing(database).startTransaction(); + will(returnValue(txn)); + allowing(database).commitTransaction(txn); + allowing(database).containsContact(txn, contactId); + will(returnValue(true)); + // Check whether an update is due + oneOf(database).getSubscriptionsModified(txn, contactId); + will(returnValue(now - 1L)); + oneOf(database).getSubscriptionsSent(txn, contactId); + will(returnValue(now)); + }}); + DatabaseComponent db = createDatabaseComponent(database, cleaner); + + db.generateSubscriptionUpdate(contactId, subscriptionWriter); + + context.assertIsSatisfied(); + } + @Test public void testGenerateSubscriptionUpdate() throws Exception { final MessageId messageId1 = new MessageId(TestUtils.getRandomId()); @@ -753,10 +781,15 @@ public abstract class DatabaseComponentTest extends TestCase { allowing(database).commitTransaction(txn); allowing(database).containsContact(txn, contactId); will(returnValue(true)); + // Check whether an update is due + oneOf(database).getSubscriptionsModified(txn, contactId); + will(returnValue(0L)); + oneOf(database).getSubscriptionsSent(txn, contactId); + will(returnValue(0L)); // Get the visible subscriptions oneOf(database).getVisibleSubscriptions(txn, contactId); will(returnValue(Collections.singletonMap(group, 0L))); - oneOf(database).setSubscriptionsSentTimestamp(with(txn), with(contactId), + oneOf(database).setSubscriptionsSent(with(txn), with(contactId), with(any(long.class))); // Add the subscriptions to the writer oneOf(subscriptionWriter).writeSubscriptions( @@ -770,6 +803,34 @@ public abstract class DatabaseComponentTest extends TestCase { context.assertIsSatisfied(); } + @Test + public void testTransportUpdateNotSentUnlessDue() throws Exception { + final long now = System.currentTimeMillis(); + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + final TransportWriter transportWriter = + context.mock(TransportWriter.class); + context.checking(new Expectations() {{ + allowing(database).startTransaction(); + will(returnValue(txn)); + allowing(database).commitTransaction(txn); + allowing(database).containsContact(txn, contactId); + will(returnValue(true)); + // Check whether an update is due + oneOf(database).getTransportsModified(txn); + will(returnValue(now - 1L)); + oneOf(database).getTransportsSent(txn, contactId); + will(returnValue(now)); + }}); + DatabaseComponent db = createDatabaseComponent(database, cleaner); + + db.generateTransportUpdate(contactId, transportWriter); + + context.assertIsSatisfied(); + } + @Test public void testGenerateTransportUpdate() throws Exception { final MessageId messageId1 = new MessageId(TestUtils.getRandomId()); @@ -788,10 +849,15 @@ public abstract class DatabaseComponentTest extends TestCase { allowing(database).commitTransaction(txn); allowing(database).containsContact(txn, contactId); will(returnValue(true)); + // Check whether an update is due + oneOf(database).getTransportsModified(txn); + will(returnValue(0L)); + oneOf(database).getTransportsSent(txn, contactId); + will(returnValue(0L)); // Get the local transport properties oneOf(database).getLocalTransports(txn); will(returnValue(transports)); - oneOf(database).setTransportsSentTimestamp(with(txn), with(contactId), + oneOf(database).setTransportsSent(with(txn), with(contactId), with(any(long.class))); // Add the properties to the writer oneOf(transportWriter).writeTransports(with(transports), @@ -1287,7 +1353,7 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).getLocalProperties(txn, transportId); will(returnValue(new TransportProperties())); oneOf(database).setLocalProperties(txn, transportId, properties); - oneOf(database).setTransportsModifiedTimestamp(with(txn), + oneOf(database).setTransportsModified(with(txn), with(any(long.class))); oneOf(database).commitTransaction(txn); oneOf(listener).eventOccurred(with(any(