Skip to content
This repository has been archived by the owner on Feb 21, 2024. It is now read-only.

Commit

Permalink
Merge pull request #24 from Contargo/fix-race-condition-in-validator
Browse files Browse the repository at this point in the history 
Fix race condition in validator
  • Loading branch information
@daschJulia
daschJulia authored Oct 8, 2020
2 parents 24a0823 + 561d685 commit 59ca382
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 126 deletions.
86 changes: 49 additions & 37 deletions src/main/java/net/contargo/types/contactinfo/validation/ReactiveUniquenessValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ public class ReactiveUniquenessValidator implements ContactInfoConsumer, Loggabl
private final ConcurrentMap<String, String> userUUIDToMail;
private final ConcurrentMap<String, String> userUUIDToMobile;
private final ConcurrentMap<String, Set<String>> mobileToUserUUIDs;

private final ConcurrentMap<String, Set<String>> mailToUserUUIDs;

private final PhoneNumberNormalizer phoneNumberNormalizer;
private final EmailAddressNormalizer emailAddressNormalizer;

private boolean isConsumingRegistrations = true;

public ReactiveUniquenessValidator(PhoneNumberNormalizer phoneNumberNormalizer,
Expand Down Expand Up @@ -105,9 +106,15 @@ private void extractMobileAndMailFromProfile(final ContactInformation contactInf
private void handleChangedMobile(final String newMobile, final String userUUID, final String oldMobile) {

userUUIDToMobile.put(userUUID, newMobile);
mobileToUserUUIDs.getOrDefault(oldMobile, Collections.emptySet()).remove(userUUID);
mobileToUserUUIDs.putIfAbsent(newMobile, new HashSet<>());
mobileToUserUUIDs.get(newMobile).add(userUUID);

mobileToUserUUIDs.computeIfPresent(oldMobile,
(k, userUUIDs) -> {
userUUIDs.remove(userUUID);

return userUUIDs;
});

mobileToUserUUIDs.computeIfAbsent(newMobile, k -> new HashSet<>()).add(userUUID);
}


Expand All @@ -126,9 +133,14 @@ public void reset() {
private void handleChangedMailAddress(final String newMail, final String userUUID, final String oldMail) {

userUUIDToMail.put(userUUID, newMail);
mailToUserUUIDs.getOrDefault(oldMail, Collections.emptySet()).remove(userUUID);
mailToUserUUIDs.putIfAbsent(newMail, new HashSet<>());
mailToUserUUIDs.get(newMail).add(userUUID);

mailToUserUUIDs.computeIfPresent(oldMail, (k, v) -> {
v.remove(userUUID);

return v;
});

mailToUserUUIDs.computeIfAbsent(newMail, k -> new HashSet<>()).add(userUUID);
}


Expand All @@ -137,7 +149,12 @@ private void handleRemovedMobile(final String userUUID, final String oldMobile)
userUUIDToMobile.remove(userUUID);

if (StringUtils.isNotBlank(oldMobile)) {
mobileToUserUUIDs.getOrDefault(oldMobile, Collections.emptySet()).remove(userUUID);
mobileToUserUUIDs.computeIfPresent(oldMobile,
(k, userUUIDs) -> {
userUUIDs.remove(userUUID);

return userUUIDs;
});
}
}

Expand All @@ -147,11 +164,7 @@ private void handleNewMobile(final String newMobile, final String userUUID) {
userUUIDToMobile.put(userUUID, newMobile);

// add reverse mapping
if (!mobileToUserUUIDs.containsKey(newMobile)) {
mobileToUserUUIDs.putIfAbsent(newMobile, new HashSet<>());
}

mobileToUserUUIDs.get(newMobile).add(userUUID);
mobileToUserUUIDs.computeIfAbsent(newMobile, k -> new HashSet<>()).add(userUUID);
}


Expand All @@ -160,7 +173,12 @@ private void handleRemovedMailAddress(final String userUUID, final String oldMai
userUUIDToMail.remove(userUUID);

if (StringUtils.isNotBlank(oldMail)) {
mailToUserUUIDs.getOrDefault(oldMail, Collections.emptySet()).remove(userUUID);
mailToUserUUIDs.computeIfPresent(oldMail,
(k, userUUIDs) -> {
userUUIDs.remove(userUUID);

return userUUIDs;
});
}
}

Expand All @@ -170,11 +188,7 @@ private void handleNewMailAddress(final String newMail, String userUUID) {
userUUIDToMail.put(userUUID, newMail);

// add reverse mapping
if (!mailToUserUUIDs.containsKey(newMail)) {
mailToUserUUIDs.putIfAbsent(newMail, new HashSet<>());
}

mailToUserUUIDs.get(newMail).add(userUUID);
mailToUserUUIDs.computeIfAbsent(newMail, k -> new HashSet<>()).add(userUUID);
}


Expand Down Expand Up @@ -233,28 +247,26 @@ public boolean isMobileUnique(final String userUUID, final String mobile) {
}


private boolean isValueUniqueForKey(final String key, final String value,
final Map<String, Set<String>> dataToCheck) {

if (dataToCheck.containsKey(key)) {
Set<String> values = dataToCheck.get(key);
protected boolean isValueUniqueForKey(final String key, final String value,
final ConcurrentMap<String, Set<String>> dataToCheck) {

// only one presence and mapped to the requester's userUUID
if (!values.isEmpty()) { // there are values for this key
// Use local copy of values, since modifications on the mapped set may occur
HashSet<String> values = new HashSet<>(dataToCheck.getOrDefault(key, Collections.emptySet()));

if (values.size() == 1 && values.contains(value)) { // only one value equals to the requested value: unique
return true;
} else { // more values or one not equal to the requested one: not unique
logger().info("detected non-unique value {}. claimed by {} but already taken by {}.", key, value,
String.join(",", values));
// No values for key: implied unique (we don't know)
if (values.isEmpty()) {
return true;
}

return false;
}
} else {
return true;
}
} else { // no mappings yet -> unique
// Exactly one value equal to the requested value: distinct unique, for key.
if (values.size() == 1 && values.contains(value)) {
return true;
}

// More values or one that is not equal to the requested: distinct not unique, for key.
logger().info("detected non-unique value {}. claimed by {} but already taken by {}.", key, value,
String.join(",", values));

return false;
}
}
121 changes: 32 additions & 89 deletions src/test/java/net/contargo/types/contactinfo/validation/ReactiveUniquenessValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,92 +17,31 @@

public class ReactiveUniquenessValidatorTest {

private void consumeColaProfiles(ContactInfoConsumer colaProfileConsumer) {
private void consumeColaProfilesFixtures(ContactInfoConsumer colaProfileConsumer) {

List<ContactInformation> allProfiles = new ArrayList<>();

ContactInformation contactInformation1 = new ContactInformation("uuid1", "01234", "04567", "foo1@bar");
allProfiles.add(contactInformation1);

ContactInformation contactInformation2 = new ContactInformation("uuid2", "01233", "04567", "foo@bar");
allProfiles.add(contactInformation2);

ContactInformation contactInformation3 = new ContactInformation("uuid3", "0171789987", "04567", "foo@baz");
allProfiles.add(contactInformation3);

ContactInformation contactInformation4 = new ContactInformation("uuid4", "0171789987", "04567", "foo@bazl");
allProfiles.add(contactInformation4);
allProfiles.add(new ContactInformation("uuid1", "01234", "04567", "foo1@bar"));
allProfiles.add(new ContactInformation("uuid2", "01233", "04567", "foo@bar"));
allProfiles.add(new ContactInformation("uuid3", "0171789987", "04567", "foo@baz"));
allProfiles.add(new ContactInformation("uuid4", "0171789987", "04567", "foo@bazl"));

colaProfileConsumer.consume(allProfiles);
}


private ContactInformation contactInformationWithAllData() {

return new ContactInformation("uuid1", "0171789987", "0721321451", "foo@bar");
}


private ContactInformation contactInformationWithAllDataAndDuplicateMailOfOtherUser() {

return new ContactInformation("uuid1", "", "0721321451", "foo@baz");
}


private ContactInformation contactInformationWithDuplicateMailAfterNormalization() {

return new ContactInformation("uuid1-1", "", "0721321451", "foo1@BAR");
}


private ContactInformation contactInformationWithAllDataAndDistinctMail() {

return new ContactInformation("uuid3", "", "0721321451", "foo@baz");
}


private ContactInformation contactInformationWithAllDataAndDistinctMobile() {

return new ContactInformation("uuid4", "0171789988", "0721321451", "foo@barz");
}


private ContactInformation contactInformationWithDuplicateMobileAfterNormalization() {

return new ContactInformation("uuid5", "+49171789987", "0721321451", null);
}


private ContactInformation contactInformationWithDuplicateMobile() {

return new ContactInformation("uuid5", "0171789987", "0721321451", null);
}


private ContactInformation contactInformationWithoutPhoneNumbers() {

return new ContactInformation("uuid1", null, null, "foo@bar");
}


private ContactInformation contactInformationWithoutEmailAndMobile() {

return new ContactInformation("uuid1", null, "01728092174", null);
}


@Test
public void ensureThatResetClearsAllData() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());

consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

reactiveUniquenessValidator.reset();

ContactInformation contactInformationWithDuplicateMobile =
contactInformationWithDuplicateMobileAfterNormalization();
ContactInformation contactInformationWithDuplicateMobile = new ContactInformation("uuid5", "+49171789987",
"0721321451", null);
List<ValidationResult> validationResults = reactiveUniquenessValidator.checkUniqueness(
contactInformationWithDuplicateMobile);

Expand All @@ -117,10 +56,10 @@ public void ensureThatDuplicateMobileIsDetectedAfterNormalization() {
ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());

consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

ContactInformation contactInformationWithDuplicateMobile =
contactInformationWithDuplicateMobileAfterNormalization();
ContactInformation contactInformationWithDuplicateMobile = new ContactInformation("uuid5", "+49171789987",
"0721321451", null);
List<ValidationResult> validationResults = reactiveUniquenessValidator.checkUniqueness(
contactInformationWithDuplicateMobile);

Expand Down Expand Up @@ -163,9 +102,10 @@ public void ensureThatDuplicateMobileIsDetected() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

assertDetectionOfDuplicateMobile(reactiveUniquenessValidator, contactInformationWithDuplicateMobile());
assertDetectionOfDuplicateMobile(reactiveUniquenessValidator,
new ContactInformation("uuid5", "0171789987", "0721321451", null));
}


Expand Down Expand Up @@ -198,9 +138,10 @@ private void ensureDuplicateMobileIsDetectedAndNotDetected(ContactInformation co

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

assertDetectionOfDuplicateMobile(reactiveUniquenessValidator, contactInformationWithDuplicateMobile());
assertDetectionOfDuplicateMobile(reactiveUniquenessValidator,
new ContactInformation("uuid5", "0171789987", "0721321451", null));

reactiveUniquenessValidator.consume(contactInfoToBeChanged);
assertThatNoValidationErrorIsReported(reactiveUniquenessValidator, contactInfoToBeValidated);
Expand Down Expand Up @@ -234,7 +175,7 @@ private void ensureDuplicateMailIsDetectedAndNotDected(ContactInformation contac
ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());

consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

assertDetectionOfDuplicateMail(reactiveUniquenessValidator, contactInfoToBeValidated);

Expand All @@ -248,9 +189,10 @@ public void ensureThatDuplicateMobileIsDetectedAndNotDetectedAfterRemoval() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

assertDetectionOfDuplicateMobile(reactiveUniquenessValidator, contactInformationWithDuplicateMobile());
assertDetectionOfDuplicateMobile(reactiveUniquenessValidator,
new ContactInformation("uuid5", "0171789987", "0721321451", null));

ContactInformation contactInfoToBeRemoved = new ContactInformation("uuid2", "01233", "04567", "foo@bar");
ContactInformation contactInfoToBeValidated = new ContactInformation("uuid8", "01233", "04567", "foo@bar");
Expand All @@ -264,10 +206,10 @@ public void ensureThatDuplicateMailIsDetectedAndNotDetectedAfterRemoval() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

assertDetectionOfDuplicateMail(reactiveUniquenessValidator,
contactInformationWithDuplicateMailAfterNormalization());
new ContactInformation("uuid1-1", "", "0721321451", "foo1@BAR"));

ContactInformation contactInfoToBeRemoved = new ContactInformation("uuid3", "01233", "04567", "foo@baz");
ContactInformation contactInfoToBeValidated = new ContactInformation("uuid8", "01233123", "04567", "foo@baz");
Expand All @@ -281,9 +223,9 @@ public void ensureThatDuplicateMailIsDetected() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

ContactInformation contactInfoWithAllData = contactInformationWithAllDataAndDuplicateMailOfOtherUser();
ContactInformation contactInfoWithAllData = new ContactInformation("uuid1", "", "0721321451", "foo@baz");
List<ValidationResult> validationResults = reactiveUniquenessValidator.checkUniqueness(contactInfoWithAllData);
assertEquals(1, validationResults.size());
assertEquals(ValidationResult.NON_UNIQUE_EMAIL, validationResults.get(0));
Expand All @@ -295,9 +237,10 @@ public void ensureThatUserOwnMobileIsNotHandledAsDuplicate() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

ContactInformation contactInfoWithAllData = contactInformationWithAllDataAndDistinctMobile();
ContactInformation contactInfoWithAllData = new ContactInformation("uuid4", "0171789988", "0721321451",
"foo@barz");
List<ValidationResult> validationResults = reactiveUniquenessValidator.checkUniqueness(contactInfoWithAllData);
assertEquals(0, validationResults.size());
}
Expand All @@ -308,9 +251,9 @@ public void ensureThatUserOwnMailIsNotHandledAsDuplicate() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

ContactInformation contactInfoWithAllData = contactInformationWithAllDataAndDistinctMail();
ContactInformation contactInfoWithAllData = new ContactInformation("uuid3", "", "0721321451", "foo@baz");
List<ValidationResult> validationResults = reactiveUniquenessValidator.checkUniqueness(contactInfoWithAllData);
assertTrue(validationResults.isEmpty());
}
Expand Down Expand Up @@ -364,9 +307,9 @@ public void ensureThatDuplicateMailIsDetectedAfterNormalization() {

ReactiveUniquenessValidator reactiveUniquenessValidator = new ReactiveUniquenessValidator(
new PhoneNumberNormalizer(), new EmailAddressNormalizer());
consumeColaProfiles(reactiveUniquenessValidator);
consumeColaProfilesFixtures(reactiveUniquenessValidator);

ContactInformation contactInfoWithAllData = contactInformationWithDuplicateMailAfterNormalization();
ContactInformation contactInfoWithAllData = new ContactInformation("uuid1-1", "", "0721321451", "foo1@BAR");
List<ValidationResult> validationResults = reactiveUniquenessValidator.checkUniqueness(contactInfoWithAllData);
assertEquals(1, validationResults.size());
assertEquals(ValidationResult.NON_UNIQUE_EMAIL, validationResults.get(0));
Expand Down

0 comments on commit 59ca382

Please sign in to comment.