Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MOB-21074] Fix threading issues during map updates in updateProposition #87

Merged
merged 7 commits into from
Aug 6, 2024
Prev Previous commit
Next Next commit
[MOB-21074] Removing the tests and updating the comments and data str…
…ucture of cachedPropositions to ConcurrentHashMap from hashmap
  • Loading branch information
siddique-adobe committed Jul 31, 2024
commit 692eff691ea53f77f354a566f3c6b6ec9766f412
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,18 @@ public boolean doWork(final Event event) {

// Map containing the update event IDs (and corresponding requested scopes) for Edge events that
// haven't yet received an Edge completion response.
// Updating the map type from Hashmap to ConcurrentHashMap to avoid the race condition to
// restrict it getting accessed by multiple threads
// Also declaring its reference as static and final because we have to make sure there is only
// one instance of the map for the singleton optimize extension class.
private static final Map<String, List<DecisionScope>> updateRequestEventIdsInProgress =
// Concurrent Map containing the update event IDs (and corresponding requested scopes) for Edge
// events that haven't yet received an Edge completion response.
// This is accessed from multiple threads.
private final Map<String, List<DecisionScope>> updateRequestEventIdsInProgress =
new ConcurrentHashMap<>();

// a dictionary to accumulate propositions returned in various personalization:decisions events
// for the same Edge personalization request.
siddique-adobe marked this conversation as resolved.
Show resolved Hide resolved
// Updating the map type from Hashmap to ConcurrentHashMap to avoid the race condition to
// restrict it getting accessed by multiple threads.
// Also declaring its reference as static and final because we have to make sure there is only
// one instance of the map for the singleton optimize extension class.
private static final Map<DecisionScope, OptimizeProposition> propositionsInProgress =
// Concurrent Map containing the get propositions (and corresponding requested scopes) for Edge
// events that are going to be pushed to the Edge network.
// This is accessed from multiple threads.
private final Map<DecisionScope, OptimizeProposition> propositionsInProgress =
new ConcurrentHashMap<>();

// List containing the schema strings for the proposition items supported by the SDK, sent in
Expand Down Expand Up @@ -118,8 +116,10 @@ public boolean doWork(final Event event) {
*/
protected OptimizeExtension(final ExtensionApi extensionApi) {
super(extensionApi);

cachedPropositions = new HashMap<>();
/// Concurrent Map containing the get propositions (and corresponding requested scopes) for
// Edge events that are yet to be pushed to the Edge network.
// This is accessed from multiple threads.
cachedPropositions = new ConcurrentHashMap<>();
siddique-adobe marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down Expand Up @@ -879,8 +879,8 @@ Map<DecisionScope, OptimizeProposition> getPropositionsInProgress() {
@VisibleForTesting
void setPropositionsInProgress(
final Map<DecisionScope, OptimizeProposition> propositionsInProgress) {
OptimizeExtension.propositionsInProgress.clear();
OptimizeExtension.propositionsInProgress.putAll(propositionsInProgress);
this.propositionsInProgress.clear();
this.propositionsInProgress.putAll(propositionsInProgress);
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -2153,15 +2152,4 @@ private void setConfigurationSharedState(
ArgumentMatchers.eq(SharedStateResolution.ANY)))
.thenReturn(new SharedStateResult(status, data));
}

@Test
public void testUpdateRequestEventIdsInProgressIsConcurrentHashMap() {
Assert.assertTrue(
(extension.getUpdateRequestEventIdsInProgress()) instanceof ConcurrentHashMap);
}

@Test
public void testPropositionsInProgressIsConcurrentHashMap() {
Assert.assertTrue(extension.getPropositionsInProgress() instanceof ConcurrentHashMap);
}
}