From 8d20c5d8b8cb1df654e75253340d45aa2e51d3af Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 10 Mar 2023 15:15:29 +0000 Subject: [PATCH] Reify RecordPredicate for easier testing. --- .../bramble/api/record/RecordReader.java | 9 ++++++- .../contact/ContactExchangeManagerImpl.java | 6 ++--- .../bramble/contact/HandshakeManagerImpl.java | 11 ++++---- .../keyagreement/KeyAgreementTransport.java | 6 ++--- .../bramble/record/RecordReaderImpl.java | 3 +-- .../bramble/sync/SyncRecordReaderImpl.java | 6 ++--- .../KeyAgreementTransportTest.java | 27 ++++++++++++------- .../bramble/record/RecordReaderImplTest.java | 10 +++---- .../sync/SyncRecordReaderImplTest.java | 24 +++++++++++------ 9 files changed, 62 insertions(+), 40 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/record/RecordReader.java b/bramble-api/src/main/java/org/briarproject/bramble/api/record/RecordReader.java index ada8f1940..3076068c7 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/record/RecordReader.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/record/RecordReader.java @@ -32,8 +32,15 @@ public interface RecordReader { * 'accept' or 'ignore' predicates */ @Nullable - Record readRecord(Predicate accept, Predicate ignore) + Record readRecord(RecordPredicate accept, RecordPredicate ignore) throws IOException; void close() throws IOException; + + /** + * Interface that reifies the generic interface {@code Predicate} + * for easier testing. + */ + interface RecordPredicate extends Predicate { + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index 5040cf959..2875ad566 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -1,7 +1,6 @@ package org.briarproject.bramble.contact; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactExchangeManager; @@ -24,6 +23,7 @@ import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.api.record.RecordReaderFactory; import org.briarproject.bramble.api.record.RecordWriter; import org.briarproject.bramble.api.record.RecordWriterFactory; @@ -61,12 +61,12 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { getLogger(ContactExchangeManagerImpl.class.getName()); // Accept records with current protocol version, known record type - private static final Predicate ACCEPT = r -> + private static final RecordPredicate ACCEPT = r -> r.getProtocolVersion() == PROTOCOL_VERSION && isKnownRecordType(r.getRecordType()); // Ignore records with current protocol version, unknown record type - private static final Predicate IGNORE = r -> + private static final RecordPredicate IGNORE = r -> r.getProtocolVersion() == PROTOCOL_VERSION && !isKnownRecordType(r.getRecordType()); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java index acd1c80e7..6ad2ee9c0 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java @@ -2,7 +2,6 @@ package org.briarproject.bramble.contact; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.Pair; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.contact.HandshakeManager; import org.briarproject.bramble.api.contact.PendingContact; @@ -12,12 +11,12 @@ import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; -import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.api.record.RecordReaderFactory; import org.briarproject.bramble.api.record.RecordWriter; import org.briarproject.bramble.api.record.RecordWriterFactory; @@ -44,7 +43,7 @@ import static org.briarproject.bramble.util.ValidationUtils.checkLength; class HandshakeManagerImpl implements HandshakeManager { // Ignore records with current protocol version, unknown record type - private static final Predicate IGNORE = r -> + private static final RecordPredicate IGNORE = r -> r.getProtocolVersion() == PROTOCOL_VERSION && !isKnownRecordType(r.getRecordType()); @@ -61,7 +60,7 @@ class HandshakeManagerImpl implements HandshakeManager { private final RecordWriterFactory recordWriterFactory; @Inject - HandshakeManagerImpl(DatabaseComponent db, + HandshakeManagerImpl(TransactionManager db, IdentityManager identityManager, ContactManager contactManager, TransportCrypto transportCrypto, @@ -152,8 +151,8 @@ class HandshakeManagerImpl implements HandshakeManager { private Record readRecord(RecordReader r, byte expectedType) throws IOException { - // Accept records with current protocol version, expected type only - Predicate accept = rec -> + // Accept records with current protocol version, expected types only + RecordPredicate accept = rec -> rec.getProtocolVersion() == PROTOCOL_VERSION && rec.getRecordType() == expectedType; Record rec = r.readRecord(accept, IGNORE); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTransport.java b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTransport.java index e5e1e2ca3..bd09ceb09 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTransport.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTransport.java @@ -1,11 +1,11 @@ package org.briarproject.bramble.keyagreement; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.keyagreement.KeyAgreementConnection; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.api.record.RecordReaderFactory; import org.briarproject.bramble.api.record.RecordWriter; import org.briarproject.bramble.api.record.RecordWriterFactory; @@ -34,12 +34,12 @@ class KeyAgreementTransport { Logger.getLogger(KeyAgreementTransport.class.getName()); // Accept records with current protocol version, known record type - private static final Predicate ACCEPT = r -> + private static final RecordPredicate ACCEPT = r -> r.getProtocolVersion() == PROTOCOL_VERSION && isKnownRecordType(r.getRecordType()); // Ignore records with current protocol version, unknown record type - private static final Predicate IGNORE = r -> + private static final RecordPredicate IGNORE = r -> r.getProtocolVersion() == PROTOCOL_VERSION && !isKnownRecordType(r.getRecordType()); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/record/RecordReaderImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/record/RecordReaderImpl.java index fb6abb151..872c937fe 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/record/RecordReaderImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/record/RecordReaderImpl.java @@ -1,7 +1,6 @@ package org.briarproject.bramble.record; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; import org.briarproject.bramble.util.ByteUtils; @@ -45,7 +44,7 @@ class RecordReaderImpl implements RecordReader { @Nullable @Override - public Record readRecord(Predicate accept, Predicate ignore) + public Record readRecord(RecordPredicate accept, RecordPredicate ignore) throws IOException { while (true) { if (eof()) return null; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncRecordReaderImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncRecordReaderImpl.java index 250f72953..ee247e4b6 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncRecordReaderImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncRecordReaderImpl.java @@ -1,10 +1,10 @@ package org.briarproject.bramble.sync; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.UniqueId; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.api.sync.Ack; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageFactory; @@ -41,12 +41,12 @@ import static org.briarproject.bramble.api.sync.SyncConstants.PROTOCOL_VERSION; class SyncRecordReaderImpl implements SyncRecordReader { // Accept records with current protocol version, known record type - private static final Predicate ACCEPT = r -> + private static final RecordPredicate ACCEPT = r -> r.getProtocolVersion() == PROTOCOL_VERSION && isKnownRecordType(r.getRecordType()); // Ignore records with current protocol version, unknown record type - private static final Predicate IGNORE = r -> + private static final RecordPredicate IGNORE = r -> r.getProtocolVersion() == PROTOCOL_VERSION && !isKnownRecordType(r.getRecordType()); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementTransportTest.java b/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementTransportTest.java index d75bf40f4..b1708e859 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementTransportTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementTransportTest.java @@ -1,6 +1,5 @@ package org.briarproject.bramble.keyagreement; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.keyagreement.KeyAgreementConnection; import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; @@ -8,11 +7,13 @@ import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.api.record.RecordReaderFactory; import org.briarproject.bramble.api.record.RecordWriter; import org.briarproject.bramble.api.record.RecordWriterFactory; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.CaptureArgumentAction; +import org.briarproject.bramble.test.PredicateMatcher; import org.jmock.Expectations; import org.jmock.imposters.ByteBuddyClassImposteriser; import org.junit.Test; @@ -21,8 +22,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.concurrent.atomic.AtomicReference; -import javax.annotation.Nullable; - import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.PROTOCOL_VERSION; import static org.briarproject.bramble.api.keyagreement.RecordTypes.ABORT; import static org.briarproject.bramble.api.keyagreement.RecordTypes.CONFIRM; @@ -119,7 +118,7 @@ public class KeyAgreementTransportTest extends BrambleMockTestCase { public void testReceiveKeyThrowsExceptionIfAtEndOfStream() throws Exception { setup(); - expectReadRecord(null); + expectReadEof(); kat.receiveKey(); } @@ -148,7 +147,7 @@ public class KeyAgreementTransportTest extends BrambleMockTestCase { public void testReceiveConfirmThrowsExceptionIfAtEndOfStream() throws Exception { setup(); - expectReadRecord(null); + expectReadEof(); kat.receiveConfirm(); } @@ -209,12 +208,22 @@ public class KeyAgreementTransportTest extends BrambleMockTestCase { assertArrayEquals(expectedPayload, actual.getPayload()); } - private void expectReadRecord(@Nullable Record record) throws Exception { + private void expectReadRecord(Record record) throws Exception { context.checking(new Expectations() {{ - //noinspection unchecked - oneOf(recordReader).readRecord(with(any(Predicate.class)), - with(any(Predicate.class))); + // Test that the `accept` predicate passed to the reader would + // accept the expected record + oneOf(recordReader).readRecord(with(new PredicateMatcher<>( + RecordPredicate.class, rp -> rp.test(record))), + with(any(RecordPredicate.class))); will(returnValue(record)); }}); } + + private void expectReadEof() throws Exception { + context.checking(new Expectations() {{ + oneOf(recordReader).readRecord(with(any(RecordPredicate.class)), + with(any(RecordPredicate.class))); + will(returnValue(null)); + }}); + } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/record/RecordReaderImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/record/RecordReaderImplTest.java index c2cc4c55f..ed6f412ca 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/record/RecordReaderImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/record/RecordReaderImplTest.java @@ -1,9 +1,9 @@ package org.briarproject.bramble.record; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.test.BrambleTestCase; import org.briarproject.bramble.util.ByteUtils; import org.junit.Test; @@ -128,12 +128,12 @@ public class RecordReaderImplTest extends BrambleTestCase { RecordReader reader = new RecordReaderImpl(in); // Accept records with version 0, type 0 or 1 - Predicate accept = r -> { + RecordPredicate accept = r -> { byte version = r.getProtocolVersion(), type = r.getRecordType(); return version == 0 && (type == 0 || type == 1); }; // Ignore records with version 0, any other type - Predicate ignore = r -> { + RecordPredicate ignore = r -> { byte version = r.getProtocolVersion(), type = r.getRecordType(); return version == 0 && !(type == 0 || type == 1); }; @@ -183,12 +183,12 @@ public class RecordReaderImplTest extends BrambleTestCase { RecordReader reader = new RecordReaderImpl(in); // Accept records with version 0, type 0 or 1 - Predicate accept = r -> { + RecordPredicate accept = r -> { byte version = r.getProtocolVersion(), type = r.getRecordType(); return version == 0 && (type == 0 || type == 1); }; // Ignore records with version 0, any other type - Predicate ignore = r -> { + RecordPredicate ignore = r -> { byte version = r.getProtocolVersion(), type = r.getRecordType(); return version == 0 && !(type == 0 || type == 1); }; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java index d4eb61ff5..d9400cbd5 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java @@ -1,10 +1,10 @@ package org.briarproject.bramble.sync; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.UniqueId; import org.briarproject.bramble.api.record.Record; import org.briarproject.bramble.api.record.RecordReader; +import org.briarproject.bramble.api.record.RecordReader.RecordPredicate; import org.briarproject.bramble.api.sync.Ack; import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; @@ -24,8 +24,6 @@ import org.junit.Test; import java.io.ByteArrayOutputStream; import java.util.List; -import javax.annotation.Nullable; - import static org.briarproject.bramble.api.record.Record.MAX_RECORD_PAYLOAD_BYTES; import static org.briarproject.bramble.api.sync.RecordTypes.ACK; import static org.briarproject.bramble.api.sync.RecordTypes.MESSAGE; @@ -186,7 +184,7 @@ public class SyncRecordReaderImplTest extends BrambleMockTestCase { @Test public void testEofReturnsTrueWhenAtEndOfStream() throws Exception { expectReadRecord(createAck()); - expectReadRecord(null); + expectReadEof(); SyncRecordReader reader = new SyncRecordReaderImpl(messageFactory, recordReader); @@ -212,15 +210,25 @@ public class SyncRecordReaderImplTest extends BrambleMockTestCase { }}); } - private void expectReadRecord(@Nullable Record record) throws Exception { + private void expectReadRecord(Record record) throws Exception { context.checking(new Expectations() {{ - //noinspection unchecked - oneOf(recordReader).readRecord(with(any(Predicate.class)), - with(any(Predicate.class))); + // Test that the `accept` predicate passed to the reader would + // accept the expected record + oneOf(recordReader).readRecord(with(new PredicateMatcher<>( + RecordPredicate.class, rp -> rp.test(record))), + with(any(RecordPredicate.class))); will(returnValue(record)); }}); } + private void expectReadEof() throws Exception { + context.checking(new Expectations() {{ + oneOf(recordReader).readRecord(with(any(RecordPredicate.class)), + with(any(RecordPredicate.class))); + will(returnValue(null)); + }}); + } + private Record createMessage(int payloadLength) { return new Record(PROTOCOL_VERSION, MESSAGE, new byte[payloadLength]); }