diff --git a/api/net/sf/briar/api/db/DatabaseComponent.java b/api/net/sf/briar/api/db/DatabaseComponent.java index f8dda8592..2ef2b4d96 100644 --- a/api/net/sf/briar/api/db/DatabaseComponent.java +++ b/api/net/sf/briar/api/db/DatabaseComponent.java @@ -37,6 +37,7 @@ public interface DatabaseComponent { static final int MAX_BYTES_BETWEEN_SPACE_CHECKS = 5 * MEGABYTES; static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min static final int BYTES_PER_SWEEP = 5 * MEGABYTES; + static final long EXPIRY_MODULUS = 60L * 60L * 1000L; // 1 hour /** * Opens the database. diff --git a/api/net/sf/briar/api/protocol/SubscriptionUpdate.java b/api/net/sf/briar/api/protocol/SubscriptionUpdate.java index cdbd2fa20..4e6d025ad 100644 --- a/api/net/sf/briar/api/protocol/SubscriptionUpdate.java +++ b/api/net/sf/briar/api/protocol/SubscriptionUpdate.java @@ -1,6 +1,6 @@ package net.sf.briar.api.protocol; -import java.util.Collection; +import java.util.Map; /** A packet updating the sender's subscriptions. */ public interface SubscriptionUpdate { @@ -12,7 +12,7 @@ public interface SubscriptionUpdate { static final int MAX_SIZE = (1024 * 1024) - 100; /** Returns the subscriptions contained in the update. */ - Collection getSubscriptions(); + Map getSubscriptions(); /** * Returns the update's timestamp. Updates that are older than the newest diff --git a/api/net/sf/briar/api/protocol/writers/SubscriptionWriter.java b/api/net/sf/briar/api/protocol/writers/SubscriptionWriter.java index 5bbf4b506..e3f81b917 100644 --- a/api/net/sf/briar/api/protocol/writers/SubscriptionWriter.java +++ b/api/net/sf/briar/api/protocol/writers/SubscriptionWriter.java @@ -1,7 +1,7 @@ package net.sf.briar.api.protocol.writers; import java.io.IOException; -import java.util.Collection; +import java.util.Map; import net.sf.briar.api.protocol.Group; @@ -9,5 +9,5 @@ import net.sf.briar.api.protocol.Group; public interface SubscriptionWriter { /** Writes the contents of the update. */ - void writeSubscriptions(Collection subs) throws IOException; + void writeSubscriptions(Map subs) throws IOException; } diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java index f3f7a865a..8752bc977 100644 --- a/components/net/sf/briar/db/Database.java +++ b/components/net/sf/briar/db/Database.java @@ -123,20 +123,30 @@ interface Database { boolean containsMessage(T txn, MessageId m) throws DbException; /** - * Returns true if the user is subscribed to the given group. + * Returns true if the user subscribes to the given group. *

* Locking: subscriptions read. */ boolean containsSubscription(T txn, GroupId g) throws DbException; /** - * Returns true if the user is subscribed to the given group and the - * group is visible to the given contact. + * Returns true if the user has been subscribed to the given group since + * the given time. + *

+ * Locking: subscriptions read. + */ + boolean containsSubscription(T txn, GroupId g, long time) + throws DbException; + + /** + * Returns true if the user is subscribed to the given group, the group is + * visible to the given contact, and the subscription has existed since the + * given time. *

* Locking: contacts read, subscriptions read. */ - boolean containsVisibleSubscription(T txn, GroupId g, ContactId c) - throws DbException; + boolean containsVisibleSubscription(T txn, GroupId g, ContactId c, + long time) throws DbException; /** * Returns the IDs of any batches received from the given contact that need @@ -301,7 +311,7 @@ interface Database { * Returns the groups to which the user subscribes that are visible to the * given contact. */ - Collection getVisibleSubscriptions(T txn, ContactId c) + Map getVisibleSubscriptions(T txn, ContactId c) throws DbException; /** @@ -402,7 +412,7 @@ interface Database { *

* Locking: contacts write, subscriptions write. */ - void setSubscriptions(T txn, ContactId c, Collection subs, + void setSubscriptions(T txn, ContactId c, Map subs, long timestamp) throws DbException; /** diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java index 6a4df751b..716146fd6 100644 --- a/components/net/sf/briar/db/JdbcDatabase.java +++ b/components/net/sf/briar/db/JdbcDatabase.java @@ -10,6 +10,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.LinkedList; import java.util.Map; import java.util.Map.Entry; @@ -19,6 +20,7 @@ import java.util.logging.Logger; import net.sf.briar.api.ContactId; import net.sf.briar.api.Rating; +import net.sf.briar.api.db.DatabaseComponent; import net.sf.briar.api.db.DbException; import net.sf.briar.api.db.Status; import net.sf.briar.api.protocol.AuthorId; @@ -41,6 +43,7 @@ abstract class JdbcDatabase implements Database { + " (groupId HASH NOT NULL," + " groupName VARCHAR NOT NULL," + " groupKey BINARY," + + " start TIMESTAMP NOT NULL," + " PRIMARY KEY (groupId))"; private static final String CREATE_MESSAGES = @@ -103,6 +106,7 @@ abstract class JdbcDatabase implements Database { + " groupId HASH NOT NULL," + " groupName VARCHAR NOT NULL," + " groupKey BINARY," + + " start TIMESTAMP NOT NULL," + " PRIMARY KEY (contactId, groupId)," + " FOREIGN KEY (contactId) REFERENCES contacts (contactId)" + " ON DELETE CASCADE)"; @@ -564,12 +568,13 @@ abstract class JdbcDatabase implements Database { PreparedStatement ps = null; try { String sql = "INSERT INTO subscriptions" - + " (groupId, groupName, groupKey)" - + " VALUES (?, ?, ?)"; + + " (groupId, groupName, groupKey, start)" + + " VALUES (?, ?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setBytes(1, g.getId().getBytes()); ps.setString(2, g.getName()); ps.setBytes(3, g.getPublicKey()); + ps.setLong(4, System.currentTimeMillis()); int affected = ps.executeUpdate(); if(affected != 1) throw new DbStateException(); ps.close(); @@ -651,26 +656,52 @@ abstract class JdbcDatabase implements Database { } } - public boolean containsVisibleSubscription(Connection txn, GroupId g, - ContactId c) throws DbException { + public boolean containsSubscription(Connection txn, GroupId g, long time) + throws DbException { + boolean found = false; PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT COUNT(subscriptions.groupId)" - + " FROM subscriptions JOIN visibilities" + String sql = "SELECT start FROM subscriptions WHERE groupId = ?"; + ps = txn.prepareStatement(sql); + ps.setBytes(1, g.getBytes()); + rs = ps.executeQuery(); + if(rs.next()) { + long start = rs.getLong(1); + if(start <= time) found = true; + if(rs.next()) throw new DbStateException(); + } + rs.close(); + ps.close(); + return found; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + + public boolean containsVisibleSubscription(Connection txn, GroupId g, + ContactId c, long time) throws DbException { + boolean found = false; + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = "SELECT start FROM subscriptions JOIN visibilities" + " ON subscriptions.groupId = visibilities.groupId" + " WHERE subscriptions.groupId = ? AND contactId = ?"; ps = txn.prepareStatement(sql); ps.setBytes(1, g.getBytes()); ps.setInt(2, c.getInt()); rs = ps.executeQuery(); - if(!rs.next()) throw new DbStateException(); - int count = rs.getInt(1); - if(count > 1) throw new DbStateException(); - if(rs.next()) throw new DbStateException(); + if(rs.next()) { + long start = rs.getLong(1); + if(start <= time) found = true; + if(rs.next()) throw new DbStateException(); + } rs.close(); ps.close(); - return count > 0; + return found; } catch(SQLException e) { tryToClose(rs); tryToClose(ps); @@ -810,6 +841,7 @@ abstract class JdbcDatabase implements Database { + " AND contactSubscriptions.contactId = ?" + " AND visibilities.contactId = ?" + " AND statuses.contactId = ?" + + " AND timestamp >= start" + " AND status = ? AND sendability > ZERO()"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getBytes()); @@ -1019,6 +1051,7 @@ abstract class JdbcDatabase implements Database { + " WHERE contactSubscriptions.contactId = ?" + " AND visibilities.contactId = ?" + " AND statuses.contactId = ?" + + " AND timestamp >= start" + " AND status = ? AND sendability > ZERO()"; // FIXME: Investigate the performance impact of "ORDER BY timestamp" ps = txn.prepareStatement(sql); @@ -1213,24 +1246,28 @@ abstract class JdbcDatabase implements Database { } } - public Collection getVisibleSubscriptions(Connection txn, + public Map getVisibleSubscriptions(Connection txn, ContactId c) throws DbException { + long expiry = getApproximateExpiryTime(txn); PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT subscriptions.groupId, groupName, groupKey" + String sql = + "SELECT subscriptions.groupId, groupName, groupKey, start" + " FROM subscriptions JOIN visibilities" + " ON subscriptions.groupId = visibilities.groupId" + " WHERE contactId = ?"; ps = txn.prepareStatement(sql); ps.setInt(1, c.getInt()); rs = ps.executeQuery(); - Collection subs = new ArrayList(); + Map subs = new HashMap(); while(rs.next()) { GroupId id = new GroupId(rs.getBytes(1)); String name = rs.getString(2); byte[] publicKey = rs.getBytes(3); - subs.add(groupFactory.createGroup(id, name, publicKey)); + Group g = groupFactory.createGroup(id, name, publicKey); + long start = Math.max(rs.getLong(4), expiry); + subs.put(g, start); } rs.close(); ps.close(); @@ -1242,6 +1279,31 @@ abstract class JdbcDatabase implements Database { } } + private long getApproximateExpiryTime(Connection txn) throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + long timestamp = 0L; + String sql = "SELECT timestamp FROM messages" + + " ORDER BY timestamp LIMIT ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, 1); + rs = ps.executeQuery(); + if(rs.next()) { + timestamp = rs.getLong(1); + timestamp -= timestamp % DatabaseComponent.EXPIRY_MODULUS; + } + if(rs.next()) throw new DbStateException(); + rs.close(); + ps.close(); + return timestamp; + } catch(SQLException e) { + tryToClose(rs); + tryToClose(ps); + throw new DbException(e); + } + } + public boolean hasSendableMessages(Connection txn, ContactId c) throws DbException { PreparedStatement ps = null; @@ -1256,6 +1318,7 @@ abstract class JdbcDatabase implements Database { + " WHERE contactSubscriptions.contactId = ?" + " AND visibilities.contactId = ?" + " AND statuses.contactId = ?" + + " AND timestamp >= start" + " AND status = ? AND sendability > ZERO()" + " LIMIT ?"; ps = txn.prepareStatement(sql); @@ -1583,7 +1646,7 @@ abstract class JdbcDatabase implements Database { } public void setSubscriptions(Connection txn, ContactId c, - Collection subs, long timestamp) throws DbException { + Map subs, long timestamp) throws DbException { PreparedStatement ps = null; ResultSet rs = null; try { @@ -1607,14 +1670,16 @@ abstract class JdbcDatabase implements Database { ps.close(); // Store the new subscriptions sql = "INSERT INTO contactSubscriptions" - + " (contactId, groupId, groupName, groupKey)" - + " VALUES (?, ?, ?, ?)"; + + " (contactId, groupId, groupName, groupKey, start)" + + " VALUES (?, ?, ?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setInt(1, c.getInt()); - for(Group g : subs) { + for(Entry e : subs.entrySet()) { + Group g = e.getKey(); ps.setBytes(2, g.getId().getBytes()); ps.setString(3, g.getName()); ps.setBytes(4, g.getPublicKey()); + ps.setLong(5, e.getValue()); ps.addBatch(); } int[] batchAffected = ps.executeBatch(); @@ -1656,7 +1721,7 @@ abstract class JdbcDatabase implements Database { ps.close(); // Store the new details sql = "INSERT INTO " + table + " (transportName, key, value)" - + " VALUES (?, ?, ?)"; + + " VALUES (?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setString(1, name); for(Entry e : details.entrySet()) { diff --git a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java index 2221390f4..8afa61d03 100644 --- a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java +++ b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java @@ -170,16 +170,11 @@ class ReadWriteLockDatabaseComponent extends DatabaseComponentImpl { Txn txn = db.startTransaction(); try { // Don't store the message if the user has - // unsubscribed from the group - if(db.containsSubscription(txn, m.getGroup())) { + // unsubscribed from the group or the message + // predates the subscription + if(db.containsSubscription(txn, m.getGroup(), + m.getTimestamp())) { added = storeMessage(txn, m, null); - if(!added) { - if(LOG.isLoggable(Level.FINE)) - LOG.fine("Duplicate local message"); - } - } else { - if(LOG.isLoggable(Level.FINE)) - LOG.fine("Not subscribed"); } db.commitTransaction(txn); } catch(DbException e) { @@ -473,7 +468,7 @@ class ReadWriteLockDatabaseComponent extends DatabaseComponentImpl { try { Txn txn = db.startTransaction(); try { - Collection subs = db.getVisibleSubscriptions(txn, c); + Map subs = db.getVisibleSubscriptions(txn, c); s.writeSubscriptions(subs); if(LOG.isLoggable(Level.FINE)) LOG.fine("Added " + subs.size() + " subscriptions"); @@ -740,8 +735,8 @@ class ReadWriteLockDatabaseComponent extends DatabaseComponentImpl { int received = 0, stored = 0; for(Message m : b.getMessages()) { received++; - GroupId g = m.getGroup(); - if(db.containsVisibleSubscription(txn, g, c)) { + if(db.containsVisibleSubscription(txn, + m.getGroup(), c, m.getTimestamp())) { if(storeMessage(txn, m, c)) { anyAdded = true; stored++; @@ -826,7 +821,7 @@ class ReadWriteLockDatabaseComponent extends DatabaseComponentImpl { try { Txn txn = db.startTransaction(); try { - Collection subs = s.getSubscriptions(); + Map subs = s.getSubscriptions(); db.setSubscriptions(txn, c, subs, s.getTimestamp()); if(LOG.isLoggable(Level.FINE)) LOG.fine("Received " + subs.size() + " subscriptions"); @@ -1013,7 +1008,7 @@ class ReadWriteLockDatabaseComponent extends DatabaseComponentImpl { try { Txn txn = db.startTransaction(); try { - if(!db.containsSubscription(txn, g.getId())) { + if(db.containsSubscription(txn, g.getId())) { db.addSubscription(txn, g); added = true; } diff --git a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java index 381dda60f..4db374998 100644 --- a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java +++ b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java @@ -126,8 +126,10 @@ class SynchronizedDatabaseComponent extends DatabaseComponentImpl { Txn txn = db.startTransaction(); try { // Don't store the message if the user has - // unsubscribed from the group - if(db.containsSubscription(txn, m.getGroup())) { + // unsubscribed from the group or the message + // predates the subscription + if(db.containsSubscription(txn, m.getGroup(), + m.getTimestamp())) { added = storeMessage(txn, m, null); if(!added) { if(LOG.isLoggable(Level.FINE)) @@ -343,7 +345,7 @@ class SynchronizedDatabaseComponent extends DatabaseComponentImpl { synchronized(subscriptionLock) { Txn txn = db.startTransaction(); try { - Collection subs = db.getVisibleSubscriptions(txn, c); + Map subs = db.getVisibleSubscriptions(txn, c); s.writeSubscriptions(subs); if(LOG.isLoggable(Level.FINE)) LOG.fine("Added " + subs.size() + " subscriptions"); @@ -549,7 +551,8 @@ class SynchronizedDatabaseComponent extends DatabaseComponentImpl { for(Message m : b.getMessages()) { received++; GroupId g = m.getGroup(); - if(db.containsVisibleSubscription(txn, g, c)) { + if(db.containsVisibleSubscription(txn, g, c, + m.getTimestamp())) { if(storeMessage(txn, m, c)) { anyAdded = true; stored++; @@ -612,7 +615,7 @@ class SynchronizedDatabaseComponent extends DatabaseComponentImpl { synchronized(subscriptionLock) { Txn txn = db.startTransaction(); try { - Collection subs = s.getSubscriptions(); + Map subs = s.getSubscriptions(); db.setSubscriptions(txn, c, subs, s.getTimestamp()); if(LOG.isLoggable(Level.FINE)) LOG.fine("Received " + subs.size() + " subscriptions"); @@ -758,7 +761,7 @@ class SynchronizedDatabaseComponent extends DatabaseComponentImpl { synchronized(subscriptionLock) { Txn txn = db.startTransaction(); try { - if(!db.containsSubscription(txn, g.getId())) { + if(db.containsSubscription(txn, g.getId())) { db.addSubscription(txn, g); added = true; } diff --git a/components/net/sf/briar/protocol/SubscriptionFactory.java b/components/net/sf/briar/protocol/SubscriptionFactory.java index 136bcffea..ee5982968 100644 --- a/components/net/sf/briar/protocol/SubscriptionFactory.java +++ b/components/net/sf/briar/protocol/SubscriptionFactory.java @@ -1,11 +1,12 @@ package net.sf.briar.protocol; -import java.util.Collection; +import java.util.Map; import net.sf.briar.api.protocol.Group; import net.sf.briar.api.protocol.SubscriptionUpdate; interface SubscriptionFactory { - SubscriptionUpdate createSubscriptions(Collection subs, long timestamp); + SubscriptionUpdate createSubscriptions(Map subs, + long timestamp); } diff --git a/components/net/sf/briar/protocol/SubscriptionFactoryImpl.java b/components/net/sf/briar/protocol/SubscriptionFactoryImpl.java index 0d3391ebe..b545e4e5a 100644 --- a/components/net/sf/briar/protocol/SubscriptionFactoryImpl.java +++ b/components/net/sf/briar/protocol/SubscriptionFactoryImpl.java @@ -1,13 +1,13 @@ package net.sf.briar.protocol; -import java.util.Collection; +import java.util.Map; import net.sf.briar.api.protocol.Group; import net.sf.briar.api.protocol.SubscriptionUpdate; class SubscriptionFactoryImpl implements SubscriptionFactory { - public SubscriptionUpdate createSubscriptions(Collection subs, + public SubscriptionUpdate createSubscriptions(Map subs, long timestamp) { return new SubscriptionsImpl(subs, timestamp); } diff --git a/components/net/sf/briar/protocol/SubscriptionReader.java b/components/net/sf/briar/protocol/SubscriptionReader.java index c13125d6a..6c31dad6f 100644 --- a/components/net/sf/briar/protocol/SubscriptionReader.java +++ b/components/net/sf/briar/protocol/SubscriptionReader.java @@ -1,7 +1,7 @@ package net.sf.briar.protocol; import java.io.IOException; -import java.util.Collection; +import java.util.Map; import net.sf.briar.api.protocol.Group; import net.sf.briar.api.protocol.SubscriptionUpdate; @@ -31,7 +31,7 @@ class SubscriptionReader implements ObjectReader { r.addConsumer(counting); r.readUserDefinedTag(Tags.SUBSCRIPTIONS); r.addObjectReader(Tags.GROUP, groupReader); - Collection subs = r.readList(Group.class); + Map subs = r.readMap(Group.class, Long.class); r.removeObjectReader(Tags.GROUP); long timestamp = r.readInt64(); r.removeConsumer(counting); diff --git a/components/net/sf/briar/protocol/SubscriptionsImpl.java b/components/net/sf/briar/protocol/SubscriptionsImpl.java index 43be09e70..0d15e0d07 100644 --- a/components/net/sf/briar/protocol/SubscriptionsImpl.java +++ b/components/net/sf/briar/protocol/SubscriptionsImpl.java @@ -1,21 +1,21 @@ package net.sf.briar.protocol; -import java.util.Collection; +import java.util.Map; import net.sf.briar.api.protocol.Group; import net.sf.briar.api.protocol.SubscriptionUpdate; class SubscriptionsImpl implements SubscriptionUpdate { - private final Collection subs; + private final Map subs; private final long timestamp; - SubscriptionsImpl(Collection subs, long timestamp) { + SubscriptionsImpl(Map subs, long timestamp) { this.subs = subs; this.timestamp = timestamp; } - public Collection getSubscriptions() { + public Map getSubscriptions() { return subs; } diff --git a/components/net/sf/briar/protocol/writers/SubscriptionWriterImpl.java b/components/net/sf/briar/protocol/writers/SubscriptionWriterImpl.java index 67e5736c2..3b591f408 100644 --- a/components/net/sf/briar/protocol/writers/SubscriptionWriterImpl.java +++ b/components/net/sf/briar/protocol/writers/SubscriptionWriterImpl.java @@ -2,7 +2,7 @@ package net.sf.briar.protocol.writers; import java.io.IOException; import java.io.OutputStream; -import java.util.Collection; +import java.util.Map; import net.sf.briar.api.protocol.Group; import net.sf.briar.api.protocol.Tags; @@ -20,9 +20,9 @@ class SubscriptionWriterImpl implements SubscriptionWriter { w = writerFactory.createWriter(out); } - public void writeSubscriptions(Collection subs) throws IOException { + public void writeSubscriptions(Map subs) throws IOException { w.writeUserDefinedTag(Tags.SUBSCRIPTIONS); - w.writeList(subs); + w.writeMap(subs); w.writeInt64(System.currentTimeMillis()); out.flush(); } diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java index 6ef02cdc9..f12acc766 100644 --- a/test/net/sf/briar/db/DatabaseComponentTest.java +++ b/test/net/sf/briar/db/DatabaseComponentTest.java @@ -112,6 +112,11 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).addSubscription(txn, group); oneOf(listener).eventOccurred( DatabaseListener.Event.SUBSCRIPTIONS_UPDATED); + // subscribe(group) again + oneOf(group).getId(); + will(returnValue(groupId)); + oneOf(database).containsSubscription(txn, groupId); + will(returnValue(true)); // getSubscriptions() oneOf(database).getSubscriptions(txn); will(returnValue(Collections.singletonList(groupId))); @@ -121,6 +126,9 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).removeSubscription(txn, groupId); oneOf(listener).eventOccurred( DatabaseListener.Event.SUBSCRIPTIONS_UPDATED); + // unsubscribe(groupId) again + oneOf(database).containsSubscription(txn, groupId); + will(returnValue(false)); // removeContact(contactId) oneOf(database).removeContact(txn, contactId); // close() @@ -136,8 +144,10 @@ public abstract class DatabaseComponentTest extends TestCase { assertEquals(Collections.singletonList(contactId), db.getContacts()); assertEquals(transports, db.getTransports(contactId)); db.subscribe(group); + db.subscribe(group); // Again - check listeners aren't called assertEquals(Collections.singletonList(groupId), db.getSubscriptions()); db.unsubscribe(groupId); + db.unsubscribe(groupId); // Again - check listeners aren't called db.removeContact(contactId); db.removeListener(listener); db.close(); @@ -336,7 +346,7 @@ public abstract class DatabaseComponentTest extends TestCase { // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsSubscription(txn, groupId); + oneOf(database).containsSubscription(txn, groupId, timestamp); will(returnValue(false)); oneOf(database).commitTransaction(txn); }}); @@ -357,7 +367,7 @@ public abstract class DatabaseComponentTest extends TestCase { // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsSubscription(txn, groupId); + oneOf(database).containsSubscription(txn, groupId, timestamp); will(returnValue(true)); oneOf(database).addMessage(txn, message); will(returnValue(false)); @@ -380,7 +390,7 @@ public abstract class DatabaseComponentTest extends TestCase { // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsSubscription(txn, groupId); + oneOf(database).containsSubscription(txn, groupId, timestamp); will(returnValue(true)); oneOf(database).addMessage(txn, message); will(returnValue(true)); @@ -413,7 +423,7 @@ public abstract class DatabaseComponentTest extends TestCase { // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsSubscription(txn, groupId); + oneOf(database).containsSubscription(txn, groupId, timestamp); will(returnValue(true)); oneOf(database).addMessage(txn, message); will(returnValue(true)); @@ -460,15 +470,21 @@ public abstract class DatabaseComponentTest extends TestCase { context.mock(SubscriptionUpdate.class); final TransportUpdate transportsUpdate = context.mock(TransportUpdate.class); context.checking(new Expectations() {{ - // Check whether the contact is still in the DB - which it's not - exactly(12).of(database).startTransaction(); + // Check whether the contact is still in the DB (which it's not) + // once for each method + exactly(14).of(database).startTransaction(); will(returnValue(txn)); - exactly(12).of(database).containsContact(txn, contactId); + exactly(14).of(database).containsContact(txn, contactId); will(returnValue(false)); - exactly(12).of(database).commitTransaction(txn); + exactly(14).of(database).commitTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner); + try { + db.findLostBatches(contactId); + fail(); + } catch(NoSuchContactException expected) {} + try { db.generateAck(contactId, ackWriter); fail(); @@ -500,6 +516,11 @@ public abstract class DatabaseComponentTest extends TestCase { fail(); } catch(NoSuchContactException expected) {} + try { + db.getTransports(contactId); + fail(); + } catch(NoSuchContactException expected) {} + try { db.hasSendableMessages(contactId); fail(); @@ -715,10 +736,10 @@ public abstract class DatabaseComponentTest extends TestCase { will(returnValue(true)); // Get the visible subscriptions oneOf(database).getVisibleSubscriptions(txn, contactId); - will(returnValue(Collections.singletonList(group))); + will(returnValue(Collections.singletonMap(group, 0L))); // Add the subscriptions to the writer oneOf(subscriptionWriter).writeSubscriptions( - Collections.singletonList(group)); + Collections.singletonMap(group, 0L)); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner); @@ -800,7 +821,7 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(batch).getMessages(); will(returnValue(Collections.singletonList(message))); oneOf(database).containsVisibleSubscription(txn, groupId, - contactId); + contactId, timestamp); will(returnValue(false)); // The message is not stored but the batch must still be acked oneOf(batch).getId(); @@ -832,7 +853,7 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(batch).getMessages(); will(returnValue(Collections.singletonList(message))); oneOf(database).containsVisibleSubscription(txn, groupId, - contactId); + contactId, timestamp); will(returnValue(true)); // The message is stored, but it's a duplicate oneOf(database).addMessage(txn, message); @@ -867,7 +888,7 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(batch).getMessages(); will(returnValue(Collections.singletonList(message))); oneOf(database).containsVisibleSubscription(txn, groupId, - contactId); + contactId, timestamp); will(returnValue(true)); // The message is stored, and it's not a duplicate oneOf(database).addMessage(txn, message); @@ -911,7 +932,7 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(batch).getMessages(); will(returnValue(Collections.singletonList(message))); oneOf(database).containsVisibleSubscription(txn, groupId, - contactId); + contactId, timestamp); will(returnValue(true)); // The message is stored, and it's not a duplicate oneOf(database).addMessage(txn, message); @@ -998,11 +1019,11 @@ public abstract class DatabaseComponentTest extends TestCase { will(returnValue(true)); // Get the contents of the update oneOf(subscriptionUpdate).getSubscriptions(); - will(returnValue(Collections.singletonList(group))); + will(returnValue(Collections.singletonMap(group, 0L))); oneOf(subscriptionUpdate).getTimestamp(); will(returnValue(timestamp)); oneOf(database).setSubscriptions(txn, contactId, - Collections.singletonList(group), timestamp); + Collections.singletonMap(group, 0L), timestamp); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner); @@ -1052,7 +1073,7 @@ public abstract class DatabaseComponentTest extends TestCase { // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsSubscription(txn, groupId); + oneOf(database).containsSubscription(txn, groupId, timestamp); will(returnValue(true)); oneOf(database).addMessage(txn, message); will(returnValue(true)); @@ -1088,7 +1109,7 @@ public abstract class DatabaseComponentTest extends TestCase { // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsSubscription(txn, groupId); + oneOf(database).containsSubscription(txn, groupId, timestamp); will(returnValue(true)); oneOf(database).addMessage(txn, message); will(returnValue(false)); diff --git a/test/net/sf/briar/db/H2DatabaseTest.java b/test/net/sf/briar/db/H2DatabaseTest.java index ef82cfc49..1514006f0 100644 --- a/test/net/sf/briar/db/H2DatabaseTest.java +++ b/test/net/sf/briar/db/H2DatabaseTest.java @@ -61,6 +61,7 @@ public class H2DatabaseTest extends TestCase { private final Message message; private final Group group; private final Map> transports; + private final Map subscriptions; public H2DatabaseTest() throws Exception { super(); @@ -81,6 +82,7 @@ public class H2DatabaseTest extends TestCase { group = groupFactory.createGroup(groupId, "Group name", null); transports = Collections.singletonMap("foo", Collections.singletonMap("bar", "baz")); + subscriptions = Collections.singletonMap(group, 0L); } @Before @@ -205,7 +207,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); db.setStatus(txn, contactId, messageId, Status.NEW); @@ -235,7 +237,7 @@ public class H2DatabaseTest extends TestCase { } @Test - public void testSendableMessagesMustBeNew() throws DbException { + public void testSendableMessagesMustHaveStatusNew() throws DbException { Database db = open(false); Connection txn = db.startTransaction(); @@ -243,7 +245,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); db.setSendability(txn, messageId, 1); @@ -296,7 +298,7 @@ public class H2DatabaseTest extends TestCase { assertFalse(it.hasNext()); // The contact subscribing should make the message sendable - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); assertTrue(db.hasSendableMessages(txn, contactId)); it = db.getSendableMessages(txn, contactId, ONE_MEGABYTE).iterator(); assertTrue(it.hasNext()); @@ -304,7 +306,8 @@ public class H2DatabaseTest extends TestCase { assertFalse(it.hasNext()); // The contact unsubscribing should make the message unsendable - db.setSubscriptions(txn, contactId, Collections.emptySet(), 2); + db.setSubscriptions(txn, contactId, + Collections.emptyMap(), 2); assertFalse(db.hasSendableMessages(txn, contactId)); it = db.getSendableMessages(txn, contactId, ONE_MEGABYTE).iterator(); assertFalse(it.hasNext()); @@ -313,6 +316,42 @@ public class H2DatabaseTest extends TestCase { db.close(); } + @Test + public void testSendableMessagesMustBeNewerThanSubscriptions() + throws DbException { + Database db = open(false); + Connection txn = db.startTransaction(); + + // Add a contact, subscribe to a group and store a message + assertEquals(contactId, db.addContact(txn, transports)); + db.addSubscription(txn, group); + db.setVisibility(txn, groupId, Collections.singleton(contactId)); + db.addMessage(txn, message); + db.setSendability(txn, messageId, 1); + db.setStatus(txn, contactId, messageId, Status.NEW); + + // The message is older than the contact's subscription, so it should + // not be sendable + db.setSubscriptions(txn, contactId, + Collections.singletonMap(group, timestamp + 1), 1); + assertFalse(db.hasSendableMessages(txn, contactId)); + Iterator it = + db.getSendableMessages(txn, contactId, ONE_MEGABYTE).iterator(); + assertFalse(it.hasNext()); + + // Changing the contact's subscription should make the message sendable + db.setSubscriptions(txn, contactId, + Collections.singletonMap(group, timestamp), 2); + assertTrue(db.hasSendableMessages(txn, contactId)); + it = db.getSendableMessages(txn, contactId, ONE_MEGABYTE).iterator(); + assertTrue(it.hasNext()); + assertEquals(messageId, it.next()); + assertFalse(it.hasNext()); + + db.commitTransaction(txn); + db.close(); + } + @Test public void testSendableMessagesMustFitCapacity() throws DbException { Database db = open(false); @@ -322,7 +361,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); db.setSendability(txn, messageId, 1); db.setStatus(txn, contactId, messageId, Status.NEW); @@ -352,7 +391,7 @@ public class H2DatabaseTest extends TestCase { // Add a contact, subscribe to a group and store a message assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); db.setSendability(txn, messageId, 1); db.setStatus(txn, contactId, messageId, Status.NEW); @@ -441,7 +480,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); db.setSendability(txn, messageId, 1); db.setStatus(txn, contactId, messageId, Status.NEW); @@ -478,7 +517,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); db.setSendability(txn, messageId, 1); db.setStatus(txn, contactId, messageId, Status.NEW); @@ -876,13 +915,14 @@ public class H2DatabaseTest extends TestCase { // Add a contact assertEquals(contactId, db.addContact(txn, transports)); // Add some subscriptions - Collection subs = Collections.singletonList(group); - db.setSubscriptions(txn, contactId, subs, 1); - assertEquals(subs, db.getSubscriptions(txn, contactId)); + db.setSubscriptions(txn, contactId, subscriptions, 1); + assertEquals(Collections.singletonList(group), + db.getSubscriptions(txn, contactId)); // Update the subscriptions - Collection subs1 = Collections.singletonList(group1); - db.setSubscriptions(txn, contactId, subs1, 2); - assertEquals(subs1, db.getSubscriptions(txn, contactId)); + Map subscriptions1 = Collections.singletonMap(group1, 0L); + db.setSubscriptions(txn, contactId, subscriptions1, 2); + assertEquals(Collections.singletonList(group1), + db.getSubscriptions(txn, contactId)); db.commitTransaction(txn); db.close(); @@ -900,14 +940,15 @@ public class H2DatabaseTest extends TestCase { // Add a contact assertEquals(contactId, db.addContact(txn, transports)); // Add some subscriptions - Collection subs = Collections.singletonList(group); - db.setSubscriptions(txn, contactId, subs, 2); - assertEquals(subs, db.getSubscriptions(txn, contactId)); + db.setSubscriptions(txn, contactId, subscriptions, 2); + assertEquals(Collections.singletonList(group), + db.getSubscriptions(txn, contactId)); // Try to update the subscriptions using a timestamp of 1 - Collection subs1 = Collections.singletonList(group1); - db.setSubscriptions(txn, contactId, subs1, 1); + Map subscriptions1 = Collections.singletonMap(group1, 0L); + db.setSubscriptions(txn, contactId, subscriptions1, 1); // The old subscriptions should still be there - assertEquals(subs, db.getSubscriptions(txn, contactId)); + assertEquals(Collections.singletonList(group), + db.getSubscriptions(txn, contactId)); db.commitTransaction(txn); db.close(); @@ -922,7 +963,7 @@ public class H2DatabaseTest extends TestCase { // Add a contact and subscribe to a group assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); // The message is not in the database assertNull(db.getMessageIfSendable(txn, contactId, messageId)); @@ -940,7 +981,7 @@ public class H2DatabaseTest extends TestCase { // Add a contact, subscribe to a group and store a message assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); // Set the sendability to > 0 db.setSendability(txn, messageId, 1); @@ -963,7 +1004,7 @@ public class H2DatabaseTest extends TestCase { // Add a contact, subscribe to a group and store a message assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); // Set the sendability to 0 db.setSendability(txn, messageId, 0); @@ -977,6 +1018,31 @@ public class H2DatabaseTest extends TestCase { db.close(); } + @Test + public void testGetMessageIfSendableReturnsNullIfOld() throws DbException { + Database db = open(false); + Connection txn = db.startTransaction(); + + // Add a contact, subscribe to a group and store a message + assertEquals(contactId, db.addContact(txn, transports)); + db.addSubscription(txn, group); + db.setVisibility(txn, groupId, Collections.singleton(contactId)); + // The message is older than the contact's subscription + Map subs = Collections.singletonMap(group, timestamp + 1); + db.setSubscriptions(txn, contactId, subs, 1); + db.addMessage(txn, message); + // Set the sendability to > 0 + db.setSendability(txn, messageId, 1); + // Set the status to NEW + db.setStatus(txn, contactId, messageId, Status.NEW); + + // The message is not sendable because it's too old + assertNull(db.getMessageIfSendable(txn, contactId, messageId)); + + db.commitTransaction(txn); + db.close(); + } + @Test public void testGetMessageIfSendableReturnsMessage() throws DbException { Database db = open(false); @@ -986,7 +1052,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); // Set the sendability to > 0 db.setSendability(txn, messageId, 1); @@ -1011,7 +1077,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); // The message is not in the database assertFalse(db.setStatusSeenIfVisible(txn, contactId, messageId)); @@ -1026,9 +1092,9 @@ public class H2DatabaseTest extends TestCase { Database db = open(false); Connection txn = db.startTransaction(); - // Add a contact and a neighbour subscription + // Add a contact with a subscription assertEquals(contactId, db.addContact(txn, transports)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); // There's no local subscription for the group assertFalse(db.setStatusSeenIfVisible(txn, contactId, messageId)); @@ -1066,7 +1132,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.addMessage(txn, message); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.setStatus(txn, contactId, messageId, Status.NEW); // The subscription is not visible @@ -1086,7 +1152,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); // The message has already been seen by the contact db.setStatus(txn, contactId, messageId, Status.SEEN); @@ -1107,7 +1173,7 @@ public class H2DatabaseTest extends TestCase { assertEquals(contactId, db.addContact(txn, transports)); db.addSubscription(txn, group); db.setVisibility(txn, groupId, Collections.singleton(contactId)); - db.setSubscriptions(txn, contactId, Collections.singleton(group), 1); + db.setSubscriptions(txn, contactId, subscriptions, 1); db.addMessage(txn, message); // The message has not been seen by the contact db.setStatus(txn, contactId, messageId, Status.NEW); diff --git a/test/net/sf/briar/protocol/FileReadWriteTest.java b/test/net/sf/briar/protocol/FileReadWriteTest.java index 808a630e3..a0ec39fd3 100644 --- a/test/net/sf/briar/protocol/FileReadWriteTest.java +++ b/test/net/sf/briar/protocol/FileReadWriteTest.java @@ -4,11 +4,11 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.security.KeyPair; -import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Iterator; import java.util.Map; @@ -152,9 +152,10 @@ public class FileReadWriteTest extends TestCase { SubscriptionWriter s = packetWriterFactory.createSubscriptionWriter(out); - Collection subs = new ArrayList(); - subs.add(group); - subs.add(group1); + // Use a LinkedHashMap for predictable iteration order + Map subs = new LinkedHashMap(); + subs.put(group, 0L); + subs.put(group1, 0L); s.writeSubscriptions(subs); TransportWriter t = packetWriterFactory.createTransportWriter(out); @@ -221,11 +222,10 @@ public class FileReadWriteTest extends TestCase { assertTrue(reader.hasUserDefined(Tags.SUBSCRIPTIONS)); SubscriptionUpdate s = reader.readUserDefined(Tags.SUBSCRIPTIONS, SubscriptionUpdate.class); - Collection subs = s.getSubscriptions(); + Map subs = s.getSubscriptions(); assertEquals(2, subs.size()); - Iterator it2 = subs.iterator(); - checkGroupEquality(group, it2.next()); - checkGroupEquality(group1, it2.next()); + assertEquals(Long.valueOf(0L), subs.get(group)); + assertEquals(Long.valueOf(0L), subs.get(group1)); assertTrue(s.getTimestamp() > start); assertTrue(s.getTimestamp() <= System.currentTimeMillis()); @@ -253,13 +253,4 @@ public class FileReadWriteTest extends TestCase { assertEquals(m1.getTimestamp(), m2.getTimestamp()); assertTrue(Arrays.equals(m1.getBytes(), m2.getBytes())); } - - private void checkGroupEquality(Group g1, Group g2) { - assertEquals(g1.getId(), g2.getId()); - assertEquals(g1.getName(), g2.getName()); - byte[] k1 = g1.getPublicKey(); - byte[] k2 = g2.getPublicKey(); - if(k1 == null) assertNull(k2); - else assertTrue(Arrays.equals(k1, k2)); - } }