Use JDK 11.0.3
. I have the following code snippet:
Set<String> allNumbersSet = customerInfoService.getCustomerPhoneNumbers(bankCustomerId);
additionalInformation
.map(info -> info.get(BANK_PE_CUSTOMER_ID_KEY))
.filter(StringUtils::isNotEmpty)
.ifPresent(id -> allNumbersSet.addAll(customerInfoService.getCustomerPhoneNumbers(id))); // fails here
Where get phone numbers is just Collectors.toSet()
:
@Override
public Set<String> getCustomerPhoneNumbers(String customerId) {
return backOfficeInfoClient.getCustByHashNo(customerId).getPropertyLOVs()
.flatMap(property -> property.getValues().values().stream())
.collect(Collectors.toSet());
}
However, it fails with:
java.lang.UnsupportedOperationException
at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java:71)
at java.base/java.util.ImmutableCollections$AbstractImmutableCollection.addAll(ImmutableCollections.java:76)
at service.impl.UserManagementServiceImpl.lambda$validateNewLogin$3(UserManagementServiceImpl.java:69)
If I update like the following:
var allNumbersSet = new HashSet<>(customerInfoService.getCustomerPhoneNumbers(bankCustomerId));
It works fine now.
What is wrong with the above code usage? Could you explain why this exactly appears?
This method call is surrounded by calling Hazelcast cache - before and after. As mentioned at comments it could be a reason for such behaviour:
The cached values are represented using immutable collections, which makes sense, as that allows sharing without the need for defensive copies
SOLUTION:
Found the way how to rewrite this logic and do that stuff without merging two sets:
var numbersSet = customerInfoService.getCustomerPhoneNumbers(id);
if (!numbersSet.contains(newLogin)) {
var peNumbersSet = additionalInformation
.map(info -> info.get(BANK_PE_CUSTOMER_ID_KEY))
.filter(StringUtils::isNotEmpty)
.map(customerInfoService::getCustomerPhoneNumbers)
.orElseGet(Collections::emptySet);
if (!peNumbersSet.contains(newLogin)) {
throw new ProcessException(ServerError.WRONG_LOGIN_PROVIDED.errorDTO());
}
}
Rethink this logic a little bit:
var additionalInformation = Optional.ofNullable(user.getAdditionalInformation());
var phoneNumbers = new HashSet<String>();
additionalInformation
.map(i -> i.get(BANK_CUSTOMER_ID_KEY))
.filter(StringUtils::isNotEmpty)
.map(customerInfoService::getCustomerPhoneNumbers)
.ifPresent(phoneNumbers::addAll);
additionalInformation
.map(i -> i.get(BANK_PE_CUSTOMER_ID_KEY))
.filter(StringUtils::isNotEmpty)
.map(customerInfoService::getCustomerPhoneNumbers)
.ifPresent(phoneNumbers::addAll);
if (!phoneNumbers.contains(newLogin)) {
throw new MetryusProcessException(AuthServerError.WRONG_LOGIN_PROVIDED.errorDTO());
}
However, understanding how exactly Collectors.toSet()
could work under different conditions is really very useful.
According to the Javadoc of Collectors.toSet()
:
There are no guarantees on the type, mutability, serializability, or thread-safety of the Set returned.
So, if you need a mutable set, you should create one yourself, to be sure that it is mutable.
You can either do that with the copy constructor that you have (new HashSet<>(...)
- don't forget the <>
), or you could use:
Collectors.toCollection(HashSet::new)
as the collector, as described in the linked Javadoc.
However, note that a more Stream-y way of doing this would be to concat two streams:
Set<String> someNumbersSet = customerInfoService.getCustomerPhoneNumbers(bankCustomerId);
Set<String> allNumbersSet =
Stream.concat(
someNumbersSet.stream(),
additionalInformation
.map(info -> info.get(BANK_PE_CUSTOMER_ID_KEY))
.filter(StringUtils::isNotEmpty)
.map(customerInfoService::getCustomerPhoneNumbers)
.stream()
.flatMap(Collection::stream))
.collect(Collectors.toSet());