Addressing review comments, Deleting unintentional file and used FAILURE constant in ClientTest as well
This commit is contained in:
parent
ac3cdfbf35
commit
339dacc01c
205
ambassador.patch
205
ambassador.patch
@ -1,205 +0,0 @@
|
|||||||
diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java
|
|
||||||
index 225e033..52b792b 100644
|
|
||||||
--- a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java
|
|
||||||
+++ b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java
|
|
||||||
@@ -22,6 +22,7 @@
|
|
||||||
*/
|
|
||||||
package com.iluwatar.ambassador;
|
|
||||||
|
|
||||||
+import com.iluwatar.ambassador.util.RandomProvider;
|
|
||||||
import org.slf4j.Logger;
|
|
||||||
import org.slf4j.LoggerFactory;
|
|
||||||
|
|
||||||
@@ -31,9 +32,10 @@ import static java.lang.Thread.sleep;
|
|
||||||
* A remote legacy application represented by a Singleton implementation.
|
|
||||||
*/
|
|
||||||
public class RemoteService implements RemoteServiceInterface {
|
|
||||||
-
|
|
||||||
+ static final int THRESHOLD = 200;
|
|
||||||
private static final Logger LOGGER = LoggerFactory.getLogger(RemoteService.class);
|
|
||||||
private static RemoteService service = null;
|
|
||||||
+ private final RandomProvider randomProvider;
|
|
||||||
|
|
||||||
static synchronized RemoteService getRemoteService() {
|
|
||||||
if (service == null) {
|
|
||||||
@@ -42,24 +44,33 @@ public class RemoteService implements RemoteServiceInterface {
|
|
||||||
return service;
|
|
||||||
}
|
|
||||||
|
|
||||||
- private RemoteService() {}
|
|
||||||
+ private RemoteService() {
|
|
||||||
+ this(Math::random);
|
|
||||||
+ }
|
|
||||||
|
|
||||||
/**
|
|
||||||
+ * This constuctor is used for testing purposes only.
|
|
||||||
+ */
|
|
||||||
+ RemoteService(RandomProvider randomProvider) {
|
|
||||||
+ this.randomProvider = randomProvider;
|
|
||||||
+ }
|
|
||||||
+ /**
|
|
||||||
* Remote function takes a value and multiplies it by 10 taking a random amount of time.
|
|
||||||
* Will sometimes return -1. This imitates connectivity issues a client might have to account for.
|
|
||||||
* @param value integer value to be multiplied.
|
|
||||||
- * @return if waitTime is more than 200ms, it returns value * 10, otherwise -1.
|
|
||||||
+ * @return if waitTime is less than {@link RemoteService#THRESHOLD}, it returns value * 10,
|
|
||||||
+ * otherwise {@link RemoteServiceInterface#FAILURE}.
|
|
||||||
*/
|
|
||||||
@Override
|
|
||||||
public long doRemoteFunction(int value) {
|
|
||||||
|
|
||||||
- long waitTime = (long) Math.floor(Math.random() * 1000);
|
|
||||||
+ long waitTime = (long) Math.floor(randomProvider.random() * 1000);
|
|
||||||
|
|
||||||
try {
|
|
||||||
sleep(waitTime);
|
|
||||||
} catch (InterruptedException e) {
|
|
||||||
LOGGER.error("Thread sleep state interrupted", e);
|
|
||||||
}
|
|
||||||
- return waitTime >= 200 ? value * 10 : -1;
|
|
||||||
+ return waitTime <= THRESHOLD ? value * 10 : FAILURE;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java
|
|
||||||
index 940313d..e228271 100644
|
|
||||||
--- a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java
|
|
||||||
+++ b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java
|
|
||||||
@@ -26,6 +26,7 @@ package com.iluwatar.ambassador;
|
|
||||||
* Interface shared by ({@link RemoteService}) and ({@link ServiceAmbassador}).
|
|
||||||
*/
|
|
||||||
interface RemoteServiceInterface {
|
|
||||||
-
|
|
||||||
+ int FAILURE = -1;
|
|
||||||
+
|
|
||||||
long doRemoteFunction(int value) throws Exception;
|
|
||||||
}
|
|
||||||
diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java b/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java
|
|
||||||
index 4be1dfc..52cd7df 100644
|
|
||||||
--- a/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java
|
|
||||||
+++ b/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java
|
|
||||||
@@ -59,15 +59,15 @@ public class ServiceAmbassador implements RemoteServiceInterface {
|
|
||||||
private long safeCall(int value) {
|
|
||||||
|
|
||||||
int retries = 0;
|
|
||||||
- long result = -1;
|
|
||||||
+ long result = FAILURE;
|
|
||||||
|
|
||||||
for (int i = 0; i < RETRIES; i++) {
|
|
||||||
|
|
||||||
if (retries >= RETRIES) {
|
|
||||||
- return -1;
|
|
||||||
+ return FAILURE;
|
|
||||||
}
|
|
||||||
|
|
||||||
- if ((result = checkLatency(value)) == -1) {
|
|
||||||
+ if ((result = checkLatency(value)) == FAILURE) {
|
|
||||||
LOGGER.info("Failed to reach remote: (" + (i + 1) + ")");
|
|
||||||
retries++;
|
|
||||||
try {
|
|
||||||
diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/util/RandomProvider.java b/ambassador/src/main/java/com/iluwatar/ambassador/util/RandomProvider.java
|
|
||||||
new file mode 100644
|
|
||||||
index 0000000..3afa3a6
|
|
||||||
--- /dev/null
|
|
||||||
+++ b/ambassador/src/main/java/com/iluwatar/ambassador/util/RandomProvider.java
|
|
||||||
@@ -0,0 +1,8 @@
|
|
||||||
+package com.iluwatar.ambassador.util;
|
|
||||||
+
|
|
||||||
+/**
|
|
||||||
+ * An interface for randomness. Useful for testing purposes.
|
|
||||||
+ */
|
|
||||||
+public interface RandomProvider {
|
|
||||||
+ double random();
|
|
||||||
+}
|
|
||||||
diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java
|
|
||||||
index 9354c4f..1d69a6a 100644
|
|
||||||
--- a/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java
|
|
||||||
+++ b/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java
|
|
||||||
@@ -24,6 +24,8 @@ package com.iluwatar.ambassador;
|
|
||||||
|
|
||||||
import org.junit.jupiter.api.Test;
|
|
||||||
|
|
||||||
+import static org.junit.jupiter.api.Assertions.assertTrue;
|
|
||||||
+
|
|
||||||
/**
|
|
||||||
* Test for {@link Client}
|
|
||||||
*/
|
|
||||||
@@ -35,6 +37,6 @@ public class ClientTest {
|
|
||||||
Client client = new Client();
|
|
||||||
long result = client.useService(10);
|
|
||||||
|
|
||||||
- assert result == 100 || result == -1;
|
|
||||||
+ assertTrue(result == 100 || result == -1);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java
|
|
||||||
index e0b297a..7d01469 100644
|
|
||||||
--- a/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java
|
|
||||||
+++ b/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java
|
|
||||||
@@ -22,16 +22,43 @@
|
|
||||||
*/
|
|
||||||
package com.iluwatar.ambassador;
|
|
||||||
|
|
||||||
+import com.iluwatar.ambassador.util.RandomProvider;
|
|
||||||
import org.junit.jupiter.api.Test;
|
|
||||||
|
|
||||||
+import static org.junit.jupiter.api.Assertions.assertEquals;
|
|
||||||
+import static org.junit.jupiter.api.Assertions.assertTrue;
|
|
||||||
+
|
|
||||||
/**
|
|
||||||
* Test for {@link RemoteService}
|
|
||||||
*/
|
|
||||||
public class RemoteServiceTest {
|
|
||||||
|
|
||||||
@Test
|
|
||||||
- public void test() {
|
|
||||||
- long result = RemoteService.getRemoteService().doRemoteFunction(10);
|
|
||||||
- assert result == 100 || result == -1;
|
|
||||||
+ public void testFailedCall() {
|
|
||||||
+ RemoteService remoteService = new RemoteService(
|
|
||||||
+ new StaticRandomProvider(0.21));
|
|
||||||
+ long result = remoteService.doRemoteFunction(10);
|
|
||||||
+ assertEquals(RemoteServiceInterface.FAILURE, result);
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ @Test
|
|
||||||
+ public void testSuccessfulCall() {
|
|
||||||
+ RemoteService remoteService = new RemoteService(
|
|
||||||
+ new StaticRandomProvider(0.2));
|
|
||||||
+ long result = remoteService.doRemoteFunction(10);
|
|
||||||
+ assertEquals(100, result);
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ private class StaticRandomProvider implements RandomProvider {
|
|
||||||
+ private double value;
|
|
||||||
+
|
|
||||||
+ StaticRandomProvider(double value) {
|
|
||||||
+ this.value = value;
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ @Override
|
|
||||||
+ public double random() {
|
|
||||||
+ return value;
|
|
||||||
+ }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java
|
|
||||||
index 432df9f..72e7106 100644
|
|
||||||
--- a/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java
|
|
||||||
+++ b/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java
|
|
||||||
@@ -24,6 +24,8 @@ package com.iluwatar.ambassador;
|
|
||||||
|
|
||||||
import org.junit.jupiter.api.Test;
|
|
||||||
|
|
||||||
+import static org.junit.jupiter.api.Assertions.assertTrue;
|
|
||||||
+
|
|
||||||
/**
|
|
||||||
* Test for {@link ServiceAmbassador}
|
|
||||||
*/
|
|
||||||
@@ -32,6 +34,6 @@ public class ServiceAmbassadorTest {
|
|
||||||
@Test
|
|
||||||
public void test() {
|
|
||||||
long result = new ServiceAmbassador().doRemoteFunction(10);
|
|
||||||
- assert result == 100 || result == -1;
|
|
||||||
+ assertTrue(result == 100 || result == RemoteServiceInterface.FAILURE);
|
|
||||||
}
|
|
||||||
}
|
|
@ -37,6 +37,6 @@ public class ClientTest {
|
|||||||
Client client = new Client();
|
Client client = new Client();
|
||||||
long result = client.useService(10);
|
long result = client.useService(10);
|
||||||
|
|
||||||
assertTrue(result == 100 || result == -1);
|
assertTrue(result == 100 || result == RemoteService.FAILURE);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user