From f6e4043e87d2155b0fb081f115ceff50cf804b39 Mon Sep 17 00:00:00 2001
From: bontric <benjohnwie@gmail.com>
Date: Thu, 6 Sep 2018 14:13:56 +0200
Subject: [PATCH] Renaming/refactoring MailboxProtocol responses

---
 .../mailbox/protocol/MailboxProtocol.java     |  4 +--
 .../mailbox/protocol/MailboxRequest.java      | 36 +++++++++++--------
 .../mailbox/protocol/MailboxRequestEnd.java   |  2 +-
 .../mailbox/protocol/MailboxRequestStore.java |  2 +-
 .../mailbox/protocol/MailboxRequestSync.java  |  2 +-
 .../mailbox/protocol/MailboxRequestTake.java  |  2 +-
 .../MailboxProtocolIntegrationTest.java       | 14 ++++----
 7 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java
index 201cf29c9..0f1558710 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java
@@ -66,7 +66,7 @@ public class MailboxProtocol implements Runnable {
 
 		if (aborted.get())
 			throw new IOException("Connection closed");
-		outQueue.put(request.makeSuccessResponse());
+		outQueue.put(request.createSuccessResponse());
 	}
 
 	public void writeErrorResponse(MailboxRequest request, String error)
@@ -78,7 +78,7 @@ public class MailboxProtocol implements Runnable {
 		if (aborted.get())
 			throw new IOException("Connection closed");
 
-		outQueue.put(request.makeErrorResponse(error));
+		outQueue.put(request.createErrorResponse(error));
 	}
 
 	@Override
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java
index a68618369..425d97c66 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java
@@ -2,7 +2,6 @@ package org.briarproject.bramble.mailbox.protocol;
 
 import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.data.BdfList;
-import org.briarproject.bramble.api.mailbox.MailboxConstants;
 
 import java.util.Arrays;
 import java.util.Random;
@@ -15,7 +14,7 @@ public abstract class MailboxRequest implements MailboxMessage {
 	private TYPE type;
 	private String error;
 	private AtomicBoolean hasResponse = new AtomicBoolean(false);
-	private boolean wasSuccessfull = false;
+	private boolean wasSuccessful = false;
 
 	public MailboxRequest(TYPE type) {
 		this.type = type;
@@ -32,7 +31,8 @@ public abstract class MailboxRequest implements MailboxMessage {
 
 	@Override
 	public BdfList toBdfList() {
-		return new BdfList(Arrays.asList(type.getValue(), getId(), makeRequestBody()));
+		return new BdfList(
+				Arrays.asList(type.getValue(), getId(), getRequestBody()));
 	}
 
 	@Override
@@ -49,43 +49,51 @@ public abstract class MailboxRequest implements MailboxMessage {
 	public void signalSucess() {
 		synchronized (hasResponse) {
 			hasResponse.set(true);
-			wasSuccessfull = true;
+			wasSuccessful = true;
 			hasResponse.notifyAll();
 		}
 	}
-	public void signalError(String error){
+
+	public void signalError(String error) {
 		synchronized (hasResponse) {
 			hasResponse.set(true);
-			wasSuccessfull = false;
+			wasSuccessful = false;
 			this.error = error;
 			hasResponse.notifyAll();
 		}
 	}
 
-	public void awaitResponse() throws InterruptedException {
+	/**
+	 * Blocks until a response for this request has been received
+	 *
+	 * @return true if response indicates success, false if not
+	 * @throws InterruptedException
+	 */
+	public boolean awaitAndGetResponse() throws InterruptedException {
 		synchronized (hasResponse) {
 			while (!hasResponse.get()) {
 				hasResponse.wait();
 			}
 		}
-	}
 
-	public boolean wasSuccessfull() {
-		return wasSuccessfull;
+		return wasSuccessful;
 	}
 
-	public String getResponseError() {
+	public String getError() {
+		if (!hasResponse.get() || (hasResponse.get() && !wasSuccessful))
+			throw new RuntimeException(
+					"Trying to get Error from unfinished or successful request");
 		return error;
 	}
 
-	public MailboxResponse makeSuccessResponse() {
+	public MailboxResponse createSuccessResponse() {
 		return new MailboxResponse(msgId, true, null);
 	}
 
-	public MailboxResponse makeErrorResponse(String error) {
+	public MailboxResponse createErrorResponse(String error) {
 		return new MailboxResponse(msgId, false, error);
 	}
 
-	protected abstract BdfList makeRequestBody();
+	protected abstract BdfList getRequestBody();
 }
 
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java
index dde179f1e..3add90694 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java
@@ -16,7 +16,7 @@ public class MailboxRequestEnd extends MailboxRequest {
 	}
 
 	@Override
-	protected BdfList makeRequestBody() {
+	protected BdfList getRequestBody() {
 		return new BdfList();
 	}
 
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java
index fd0087484..f94f6e494 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java
@@ -24,7 +24,7 @@ public class MailboxRequestStore extends MailboxRequest {
 		super(msg);
 	}
 
-	public BdfList makeRequestBody() {
+	public BdfList getRequestBody() {
 		return new BdfList(Arrays.asList(contactId.getInt(),
 				encryptedSyncStream));
 	}
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java
index 9aceea5a7..7a69e135a 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java
@@ -19,7 +19,7 @@ public class MailboxRequestSync extends MailboxRequest {
 	}
 
 	@Override
-	protected BdfList makeRequestBody() {
+	protected BdfList getRequestBody() {
 		return new BdfList(Arrays.asList(encryptedStream));
 	}
 
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java
index 5fae3ae92..a1ccd2ecb 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java
@@ -23,7 +23,7 @@ public class MailboxRequestTake extends MailboxRequest {
 
 
 	@Override
-	protected BdfList makeRequestBody() {
+	protected BdfList getRequestBody() {
 		return new BdfList(Arrays.asList(contactId.getInt(), encryptedSyncStream));
 	}
 
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
index 702179b95..d7eeecdf2 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
@@ -79,6 +79,7 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
 			endRequest();
 		} finally {
 			mailboxProtocol.stop();
+			pipedOS.close();
 		}
 	}
 
@@ -90,9 +91,9 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
 		MailboxRequestTake recvReq =
 				(MailboxRequestTake) mailboxProtocol.getNextRequest();
 		mailboxProtocol.writeErrorResponse(recvReq, "Error");
-		req.awaitResponse();
-		assertEquals(false, req.wasSuccessfull());
-		assertEquals("Error", req.getResponseError());
+
+		assertEquals(false, req.awaitAndGetResponse());
+		assertEquals("Error", req.getError());
 	}
 
 	private void takeRequest() throws IOException, InterruptedException {
@@ -111,8 +112,8 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
 
 		// Write Success Response
 		mailboxProtocol.writeSucessResponse(recvReq);
-		req.awaitResponse();
-		assertEquals(true, req.wasSuccessfull());
+
+		assertEquals(true, req.awaitAndGetResponse());
 	}
 
 	private void endRequest() throws IOException, InterruptedException {
@@ -140,8 +141,7 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
 
 		// Write Success Response
 		mailboxProtocol.writeSucessResponse(recvReq);
-		req.awaitResponse();
-		assertEquals(true, req.wasSuccessfull());
+		assertEquals(true, req.awaitAndGetResponse());
 	}
 
 	private void syncRequest() throws IOException, InterruptedException {
-- 
GitLab