Resolves #643, test cases failed due to global state in CallsCount (#803)

* Resolves #643, test cases failed due to presence of global state in CallsCount. Because AppTest was executed before B2BServiceTest, it scheduled 1 sec timer using ThrottleTimerImpl class. While resetting it used that global CallCount class reset() method, which reset all counters. So that causes thread safety issue because of unintended sharing of application state between test cases, which is not a good practice.

* Updated class diagram png and added UCLS file
This commit is contained in:
Narendra Pathai 2018-10-21 23:44:07 +05:30 committed by Ilkka Seppälä
parent 9e7a500743
commit 922fd62da6
9 changed files with 120 additions and 27 deletions

Binary file not shown.

Before

Width:  |  Height:  |  Size: 59 KiB

After

Width:  |  Height:  |  Size: 49 KiB

View File

@ -0,0 +1,88 @@
<?xml version="1.0" encoding="UTF-8"?>
<class-diagram version="1.2.2" icons="true" always-add-relationships="false" generalizations="true" realizations="true"
associations="true" dependencies="false" nesting-relationships="true" router="FAN">
<class id="1" language="java" name="com.iluwatar.throttling.CallsCount" project="throttling"
file="/throttling/src/main/java/com/iluwatar/throttling/CallsCount.java" binary="false" corner="BOTTOM_RIGHT">
<position height="211" width="256" x="656" y="228"/>
<display autosize="false" stereotype="true" package="true" initial-value="false" signature="true"
sort-features="false" accessors="true" visibility="true">
<attributes public="true" package="true" protected="true" private="true" static="true"/>
<operations public="true" package="true" protected="true" private="true" static="true"/>
</display>
</class>
<class id="2" language="java" name="com.iluwatar.throttling.Tenant" project="throttling"
file="/throttling/src/main/java/com/iluwatar/throttling/Tenant.java" binary="false" corner="BOTTOM_RIGHT">
<position height="-1" width="-1" x="465" y="524"/>
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
sort-features="false" accessors="true" visibility="true">
<attributes public="true" package="true" protected="true" private="true" static="true"/>
<operations public="true" package="true" protected="true" private="true" static="true"/>
</display>
</class>
<class id="3" language="java" name="com.iluwatar.throttling.B2BService" project="throttling"
file="/throttling/src/main/java/com/iluwatar/throttling/B2BService.java" binary="false" corner="BOTTOM_RIGHT">
<position height="-1" width="-1" x="464" y="192"/>
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
sort-features="false" accessors="true" visibility="true">
<attributes public="true" package="true" protected="true" private="true" static="true"/>
<operations public="true" package="true" protected="true" private="true" static="true"/>
</display>
</class>
<interface id="4" language="java" name="com.iluwatar.throttling.timer.Throttler" project="throttling"
file="/throttling/src/main/java/com/iluwatar/throttling/timer/Throttler.java" binary="false" corner="BOTTOM_RIGHT">
<position height="-1" width="-1" x="167" y="174"/>
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
sort-features="false" accessors="true" visibility="true">
<attributes public="true" package="true" protected="true" private="true" static="true"/>
<operations public="true" package="true" protected="true" private="true" static="true"/>
</display>
</interface>
<class id="5" language="java" name="com.iluwatar.throttling.timer.ThrottleTimerImpl" project="throttling"
file="/throttling/src/main/java/com/iluwatar/throttling/timer/ThrottleTimerImpl.java" binary="false"
corner="BOTTOM_RIGHT">
<position height="-1" width="-1" x="166" y="396"/>
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
sort-features="false" accessors="true" visibility="true">
<attributes public="true" package="true" protected="true" private="true" static="true"/>
<operations public="true" package="true" protected="true" private="true" static="true"/>
</display>
</class>
<association id="6">
<end type="SOURCE" refId="3" navigable="false">
<attribute id="7" name="callsCount"/>
<multiplicity id="8" minimum="0" maximum="1"/>
</end>
<end type="TARGET" refId="1" navigable="true"/>
<display labels="true" multiplicity="true"/>
</association>
<dependency id="9">
<end type="SOURCE" refId="3"/>
<end type="TARGET" refId="4"/>
</dependency>
<dependency id="10">
<end type="SOURCE" refId="3"/>
<end type="TARGET" refId="2"/>
</dependency>
<association id="11">
<end type="SOURCE" refId="5" navigable="false">
<attribute id="12" name="callsCount"/>
<multiplicity id="13" minimum="0" maximum="1"/>
</end>
<end type="TARGET" refId="1" navigable="true"/>
<display labels="true" multiplicity="true"/>
</association>
<dependency id="14">
<end type="SOURCE" refId="2"/>
<end type="TARGET" refId="1"/>
</dependency>
<realization id="15">
<end type="SOURCE" refId="5"/>
<end type="TARGET" refId="4"/>
</realization>
<classifier-display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
sort-features="false" accessors="true" visibility="true">
<attributes public="true" package="true" protected="true" private="true" static="true"/>
<operations public="true" package="true" protected="true" private="true" static="true"/>
</classifier-display>
<association-display labels="true" multiplicity="true"/>
</class-diagram>

View File

@ -53,14 +53,14 @@ public class App {
* @param args main arguments
*/
public static void main(String[] args) {
Tenant adidas = new Tenant("Adidas", 5);
Tenant nike = new Tenant("Nike", 6);
CallsCount callsCount = new CallsCount();
Tenant adidas = new Tenant("Adidas", 5, callsCount);
Tenant nike = new Tenant("Nike", 6, callsCount);
ExecutorService executorService = Executors.newFixedThreadPool(2);
executorService.execute(() -> makeServiceCalls(adidas));
executorService.execute(() -> makeServiceCalls(nike));
executorService.execute(() -> makeServiceCalls(adidas, callsCount));
executorService.execute(() -> makeServiceCalls(nike, callsCount));
executorService.shutdown();
try {
@ -73,9 +73,9 @@ public class App {
/**
* Make calls to the B2BService dummy API
*/
private static void makeServiceCalls(Tenant tenant) {
Throttler timer = new ThrottleTimerImpl(10);
B2BService service = new B2BService(timer);
private static void makeServiceCalls(Tenant tenant, CallsCount callsCount) {
Throttler timer = new ThrottleTimerImpl(10, callsCount);
B2BService service = new B2BService(timer, callsCount);
for (int i = 0; i < 20; i++) {
service.dummyCustomerApi(tenant);
// Sleep is introduced to keep the output in check and easy to view and analyze the results.

View File

@ -35,8 +35,10 @@ import java.util.concurrent.ThreadLocalRandom;
class B2BService {
private static final Logger LOGGER = LoggerFactory.getLogger(B2BService.class);
private final CallsCount callsCount;
public B2BService(Throttler timer) {
public B2BService(Throttler timer, CallsCount callsCount) {
this.callsCount = callsCount;
timer.start();
}
@ -46,13 +48,13 @@ class B2BService {
*/
public int dummyCustomerApi(Tenant tenant) {
String tenantName = tenant.getName();
long count = CallsCount.getCount(tenantName);
long count = callsCount.getCount(tenantName);
LOGGER.debug("Counter for {} : {} ", tenant.getName(), count);
if (count >= tenant.getAllowedCallsPerSecond()) {
LOGGER.error("API access per second limit reached for: {}", tenantName);
return -1;
}
CallsCount.incrementCount(tenantName);
callsCount.incrementCount(tenantName);
return getRandomCustomerId();
}

View File

@ -38,13 +38,13 @@ import java.util.concurrent.atomic.AtomicLong;
public final class CallsCount {
private static final Logger LOGGER = LoggerFactory.getLogger(CallsCount.class);
private static Map<String, AtomicLong> tenantCallsCount = new ConcurrentHashMap<>();
private Map<String, AtomicLong> tenantCallsCount = new ConcurrentHashMap<>();
/**
* Add a new tenant to the map.
* @param tenantName name of the tenant.
*/
public static void addTenant(String tenantName) {
public void addTenant(String tenantName) {
tenantCallsCount.putIfAbsent(tenantName, new AtomicLong(0));
}
@ -52,7 +52,7 @@ public final class CallsCount {
* Increment the count of the specified tenant.
* @param tenantName name of the tenant.
*/
public static void incrementCount(String tenantName) {
public void incrementCount(String tenantName) {
tenantCallsCount.get(tenantName).incrementAndGet();
}
@ -61,14 +61,14 @@ public final class CallsCount {
* @param tenantName name of the tenant.
* @return the count of the tenant.
*/
public static long getCount(String tenantName) {
public long getCount(String tenantName) {
return tenantCallsCount.get(tenantName).get();
}
/**
* Resets the count of all the tenants in the map.
*/
public static void reset() {
public void reset() {
LOGGER.debug("Resetting the map.");
for (Entry<String, AtomicLong> e : tenantCallsCount.entrySet()) {
tenantCallsCount.put(e.getKey(), new AtomicLong(0));

View File

@ -38,13 +38,13 @@ public class Tenant {
* @param allowedCallsPerSecond The number of calls allowed for a particular tenant.
* @throws InvalidParameterException If number of calls is less than 0, throws exception.
*/
public Tenant(String name, int allowedCallsPerSecond) {
public Tenant(String name, int allowedCallsPerSecond, CallsCount callsCount) {
if (allowedCallsPerSecond < 0) {
throw new InvalidParameterException("Number of calls less than 0 not allowed");
}
this.name = name;
this.allowedCallsPerSecond = allowedCallsPerSecond;
CallsCount.addTenant(name);
callsCount.addTenant(name);
}
public String getName() {

View File

@ -37,10 +37,12 @@ import com.iluwatar.throttling.CallsCount;
*/
public class ThrottleTimerImpl implements Throttler {
private int throttlePeriod;
public ThrottleTimerImpl(int throttlePeriod) {
private final int throttlePeriod;
private final CallsCount callsCount;
public ThrottleTimerImpl(int throttlePeriod, CallsCount callsCount) {
this.throttlePeriod = throttlePeriod;
this.callsCount = callsCount;
}
/**
@ -51,7 +53,7 @@ public class ThrottleTimerImpl implements Throttler {
new Timer(true).schedule(new TimerTask() {
@Override
public void run() {
CallsCount.reset();
callsCount.reset();
}
}, 0, throttlePeriod);
}

View File

@ -33,18 +33,19 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
*/
public class B2BServiceTest {
@Disabled
private CallsCount callsCount = new CallsCount();
@Test
public void dummyCustomerApiTest() {
Tenant tenant = new Tenant("testTenant", 2);
Tenant tenant = new Tenant("testTenant", 2, callsCount);
// In order to assure that throttling limits will not be reset, we use an empty throttling implementation
Throttler timer = () -> { };
B2BService service = new B2BService(timer);
B2BService service = new B2BService(timer, callsCount);
for (int i = 0; i < 5; i++) {
service.dummyCustomerApi(tenant);
}
long counter = CallsCount.getCount(tenant.getName());
long counter = callsCount.getCount(tenant.getName());
assertEquals(2, counter, "Counter limit must be reached");
}
}

View File

@ -36,7 +36,7 @@ public class TenantTest {
@Test
public void constructorTest() {
assertThrows(InvalidParameterException.class, () -> {
Tenant tenant = new Tenant("FailTenant", -1);
Tenant tenant = new Tenant("FailTenant", -1, new CallsCount());
});
}
}