Skip to content

Commit 922fd62

Browse files
npathaiiluwatar
npathai
authored and
iluwatar
committed
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
1 parent 9e7a500 commit 922fd62

File tree

9 files changed

+120
-27
lines changed

9 files changed

+120
-27
lines changed

‎throttling/etc/throttling-pattern.png

-9.95 KB
Loading

‎throttling/etc/throttling-pattern.ucls

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<class-diagram version="1.2.2" icons="true" always-add-relationships="false" generalizations="true" realizations="true"
3+
associations="true" dependencies="false" nesting-relationships="true" router="FAN">
4+
<class id="1" language="java" name="com.iluwatar.throttling.CallsCount" project="throttling"
5+
file="/throttling/src/main/java/com/iluwatar/throttling/CallsCount.java" binary="false" corner="BOTTOM_RIGHT">
6+
<position height="211" width="256" x="656" y="228"/>
7+
<display autosize="false" stereotype="true" package="true" initial-value="false" signature="true"
8+
sort-features="false" accessors="true" visibility="true">
9+
<attributes public="true" package="true" protected="true" private="true" static="true"/>
10+
<operations public="true" package="true" protected="true" private="true" static="true"/>
11+
</display>
12+
</class>
13+
<class id="2" language="java" name="com.iluwatar.throttling.Tenant" project="throttling"
14+
file="/throttling/src/main/java/com/iluwatar/throttling/Tenant.java" binary="false" corner="BOTTOM_RIGHT">
15+
<position height="-1" width="-1" x="465" y="524"/>
16+
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
17+
sort-features="false" accessors="true" visibility="true">
18+
<attributes public="true" package="true" protected="true" private="true" static="true"/>
19+
<operations public="true" package="true" protected="true" private="true" static="true"/>
20+
</display>
21+
</class>
22+
<class id="3" language="java" name="com.iluwatar.throttling.B2BService" project="throttling"
23+
file="/throttling/src/main/java/com/iluwatar/throttling/B2BService.java" binary="false" corner="BOTTOM_RIGHT">
24+
<position height="-1" width="-1" x="464" y="192"/>
25+
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
26+
sort-features="false" accessors="true" visibility="true">
27+
<attributes public="true" package="true" protected="true" private="true" static="true"/>
28+
<operations public="true" package="true" protected="true" private="true" static="true"/>
29+
</display>
30+
</class>
31+
<interface id="4" language="java" name="com.iluwatar.throttling.timer.Throttler" project="throttling"
32+
file="/throttling/src/main/java/com/iluwatar/throttling/timer/Throttler.java" binary="false" corner="BOTTOM_RIGHT">
33+
<position height="-1" width="-1" x="167" y="174"/>
34+
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
35+
sort-features="false" accessors="true" visibility="true">
36+
<attributes public="true" package="true" protected="true" private="true" static="true"/>
37+
<operations public="true" package="true" protected="true" private="true" static="true"/>
38+
</display>
39+
</interface>
40+
<class id="5" language="java" name="com.iluwatar.throttling.timer.ThrottleTimerImpl" project="throttling"
41+
file="/throttling/src/main/java/com/iluwatar/throttling/timer/ThrottleTimerImpl.java" binary="false"
42+
corner="BOTTOM_RIGHT">
43+
<position height="-1" width="-1" x="166" y="396"/>
44+
<display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
45+
sort-features="false" accessors="true" visibility="true">
46+
<attributes public="true" package="true" protected="true" private="true" static="true"/>
47+
<operations public="true" package="true" protected="true" private="true" static="true"/>
48+
</display>
49+
</class>
50+
<association id="6">
51+
<end type="SOURCE" refId="3" navigable="false">
52+
<attribute id="7" name="callsCount"/>
53+
<multiplicity id="8" minimum="0" maximum="1"/>
54+
</end>
55+
<end type="TARGET" refId="1" navigable="true"/>
56+
<display labels="true" multiplicity="true"/>
57+
</association>
58+
<dependency id="9">
59+
<end type="SOURCE" refId="3"/>
60+
<end type="TARGET" refId="4"/>
61+
</dependency>
62+
<dependency id="10">
63+
<end type="SOURCE" refId="3"/>
64+
<end type="TARGET" refId="2"/>
65+
</dependency>
66+
<association id="11">
67+
<end type="SOURCE" refId="5" navigable="false">
68+
<attribute id="12" name="callsCount"/>
69+
<multiplicity id="13" minimum="0" maximum="1"/>
70+
</end>
71+
<end type="TARGET" refId="1" navigable="true"/>
72+
<display labels="true" multiplicity="true"/>
73+
</association>
74+
<dependency id="14">
75+
<end type="SOURCE" refId="2"/>
76+
<end type="TARGET" refId="1"/>
77+
</dependency>
78+
<realization id="15">
79+
<end type="SOURCE" refId="5"/>
80+
<end type="TARGET" refId="4"/>
81+
</realization>
82+
<classifier-display autosize="true" stereotype="true" package="true" initial-value="false" signature="true"
83+
sort-features="false" accessors="true" visibility="true">
84+
<attributes public="true" package="true" protected="true" private="true" static="true"/>
85+
<operations public="true" package="true" protected="true" private="true" static="true"/>
86+
</classifier-display>
87+
<association-display labels="true" multiplicity="true"/>
88+
</class-diagram>

‎throttling/src/main/java/com/iluwatar/throttling/App.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,14 @@ public class App {
5353
* @param args main arguments
5454
*/
5555
public static void main(String[] args) {
56-
57-
Tenant adidas = new Tenant("Adidas", 5);
58-
Tenant nike = new Tenant("Nike", 6);
56+
CallsCount callsCount = new CallsCount();
57+
Tenant adidas = new Tenant("Adidas", 5, callsCount);
58+
Tenant nike = new Tenant("Nike", 6, callsCount);
5959

6060
ExecutorService executorService = Executors.newFixedThreadPool(2);
6161

62-
executorService.execute(() -> makeServiceCalls(adidas));
63-
executorService.execute(() -> makeServiceCalls(nike));
62+
executorService.execute(() -> makeServiceCalls(adidas, callsCount));
63+
executorService.execute(() -> makeServiceCalls(nike, callsCount));
6464

6565
executorService.shutdown();
6666
try {
@@ -73,9 +73,9 @@ public static void main(String[] args) {
7373
/**
7474
* Make calls to the B2BService dummy API
7575
*/
76-
private static void makeServiceCalls(Tenant tenant) {
77-
Throttler timer = new ThrottleTimerImpl(10);
78-
B2BService service = new B2BService(timer);
76+
private static void makeServiceCalls(Tenant tenant, CallsCount callsCount) {
77+
Throttler timer = new ThrottleTimerImpl(10, callsCount);
78+
B2BService service = new B2BService(timer, callsCount);
7979
for (int i = 0; i < 20; i++) {
8080
service.dummyCustomerApi(tenant);
8181
// Sleep is introduced to keep the output in check and easy to view and analyze the results.

‎throttling/src/main/java/com/iluwatar/throttling/B2BService.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@
3535
class B2BService {
3636

3737
private static final Logger LOGGER = LoggerFactory.getLogger(B2BService.class);
38+
private final CallsCount callsCount;
3839

39-
public B2BService(Throttler timer) {
40+
public B2BService(Throttler timer, CallsCount callsCount) {
41+
this.callsCount = callsCount;
4042
timer.start();
4143
}
4244

@@ -46,13 +48,13 @@ public B2BService(Throttler timer) {
4648
*/
4749
public int dummyCustomerApi(Tenant tenant) {
4850
String tenantName = tenant.getName();
49-
long count = CallsCount.getCount(tenantName);
51+
long count = callsCount.getCount(tenantName);
5052
LOGGER.debug("Counter for {} : {} ", tenant.getName(), count);
5153
if (count >= tenant.getAllowedCallsPerSecond()) {
5254
LOGGER.error("API access per second limit reached for: {}", tenantName);
5355
return -1;
5456
}
55-
CallsCount.incrementCount(tenantName);
57+
callsCount.incrementCount(tenantName);
5658
return getRandomCustomerId();
5759
}
5860

‎throttling/src/main/java/com/iluwatar/throttling/CallsCount.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,21 @@
3838
public final class CallsCount {
3939

4040
private static final Logger LOGGER = LoggerFactory.getLogger(CallsCount.class);
41-
private static Map<String, AtomicLong> tenantCallsCount = new ConcurrentHashMap<>();
41+
private Map<String, AtomicLong> tenantCallsCount = new ConcurrentHashMap<>();
4242

4343
/**
4444
* Add a new tenant to the map.
4545
* @param tenantName name of the tenant.
4646
*/
47-
public static void addTenant(String tenantName) {
47+
public void addTenant(String tenantName) {
4848
tenantCallsCount.putIfAbsent(tenantName, new AtomicLong(0));
4949
}
5050

5151
/**
5252
* Increment the count of the specified tenant.
5353
* @param tenantName name of the tenant.
5454
*/
55-
public static void incrementCount(String tenantName) {
55+
public void incrementCount(String tenantName) {
5656
tenantCallsCount.get(tenantName).incrementAndGet();
5757
}
5858

@@ -61,14 +61,14 @@ public static void incrementCount(String tenantName) {
6161
* @param tenantName name of the tenant.
6262
* @return the count of the tenant.
6363
*/
64-
public static long getCount(String tenantName) {
64+
public long getCount(String tenantName) {
6565
return tenantCallsCount.get(tenantName).get();
6666
}
6767

6868
/**
6969
* Resets the count of all the tenants in the map.
7070
*/
71-
public static void reset() {
71+
public void reset() {
7272
LOGGER.debug("Resetting the map.");
7373
for (Entry<String, AtomicLong> e : tenantCallsCount.entrySet()) {
7474
tenantCallsCount.put(e.getKey(), new AtomicLong(0));

‎throttling/src/main/java/com/iluwatar/throttling/Tenant.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ public class Tenant {
3838
* @param allowedCallsPerSecond The number of calls allowed for a particular tenant.
3939
* @throws InvalidParameterException If number of calls is less than 0, throws exception.
4040
*/
41-
public Tenant(String name, int allowedCallsPerSecond) {
41+
public Tenant(String name, int allowedCallsPerSecond, CallsCount callsCount) {
4242
if (allowedCallsPerSecond < 0) {
4343
throw new InvalidParameterException("Number of calls less than 0 not allowed");
4444
}
4545
this.name = name;
4646
this.allowedCallsPerSecond = allowedCallsPerSecond;
47-
CallsCount.addTenant(name);
47+
callsCount.addTenant(name);
4848
}
4949

5050
public String getName() {

‎throttling/src/main/java/com/iluwatar/throttling/timer/ThrottleTimerImpl.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,12 @@
3737
*/
3838
public class ThrottleTimerImpl implements Throttler {
3939

40-
private int throttlePeriod;
41-
42-
public ThrottleTimerImpl(int throttlePeriod) {
40+
private final int throttlePeriod;
41+
private final CallsCount callsCount;
42+
43+
public ThrottleTimerImpl(int throttlePeriod, CallsCount callsCount) {
4344
this.throttlePeriod = throttlePeriod;
45+
this.callsCount = callsCount;
4446
}
4547

4648
/**
@@ -51,7 +53,7 @@ public void start() {
5153
new Timer(true).schedule(new TimerTask() {
5254
@Override
5355
public void run() {
54-
CallsCount.reset();
56+
callsCount.reset();
5557
}
5658
}, 0, throttlePeriod);
5759
}

‎throttling/src/test/java/com/iluwatar/throttling/B2BServiceTest.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,19 @@
3333
*/
3434
public class B2BServiceTest {
3535

36-
@Disabled
36+
private CallsCount callsCount = new CallsCount();
37+
3738
@Test
3839
public void dummyCustomerApiTest() {
39-
Tenant tenant = new Tenant("testTenant", 2);
40+
Tenant tenant = new Tenant("testTenant", 2, callsCount);
4041
// In order to assure that throttling limits will not be reset, we use an empty throttling implementation
4142
Throttler timer = () -> { };
42-
B2BService service = new B2BService(timer);
43+
B2BService service = new B2BService(timer, callsCount);
4344

4445
for (int i = 0; i < 5; i++) {
4546
service.dummyCustomerApi(tenant);
4647
}
47-
long counter = CallsCount.getCount(tenant.getName());
48+
long counter = callsCount.getCount(tenant.getName());
4849
assertEquals(2, counter, "Counter limit must be reached");
4950
}
5051
}

‎throttling/src/test/java/com/iluwatar/throttling/TenantTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public class TenantTest {
3636
@Test
3737
public void constructorTest() {
3838
assertThrows(InvalidParameterException.class, () -> {
39-
Tenant tenant = new Tenant("FailTenant", -1);
39+
Tenant tenant = new Tenant("FailTenant", -1, new CallsCount());
4040
});
4141
}
4242
}

0 commit comments

Comments
 (0)