javaconcurrencyhashmap

Concurrency - Inserting to a map only if record has a certain status


I have the below code snippet that I want to change. Currently, it reads from a list and creates a map keyed on transactionId and it overrides the element stored in the map irrespective of the status of the object. PaymentDetailsModel contains as attributes the transaction id and the status.

        Map<String, PaymentDetailsModel> transactions = new HashMap<>();
        allPaymentActivityRecords.stream()
                .forEach(paymentDetail -> transactions.put(paymentDetail.getTransactionId(), paymentDetail));
        return transactions;

I want to change it so that the override doesn't occur if the stored value has STATUS2 and the coming record has STATUS1. In the code, it would look something like this:

private Map<String, PaymentDetailsModel> process(List<PaymentDetailsModel> allPaymentActivityRecords){
        Map<String, PaymentDetailsModel> transactions = new HashMap<>();

        for (PaymentDetailsModel paymentDetail : allPaymentActivityRecords) {
            PaymentDetailsModel paymentDetailsModel = transactions(paymentDetail.getTransactionId());
            if (paymentDetailsModel == null || !shouldNotOverrideStates(paymentDetail, paymentDetailsModel)) {
                transactions.put(paymentDetail.getTransactionId(), paymentDetail);
            }
        }
        return transactions;

   private static boolean shouldNotOverrideStates(PaymentDetailsModel paymentDetail, PaymentDetailsModel paymentDetailsModel) {
        return STATUS2.equals(paymentDetailsModel.getStatus()) && STATUS1.equals(paymentDetail.getStatus());
    }

My concern is that this solution could suffer from concurrency issues if this code is run in parallel on multiple threads. Among the ideas to ensure correctness, I have the synchronisation on the condition or using ConcurrentHashMap. Please let me know your thoughts.


Solution

  • You should use ConcurrentHashMap and its merge method, which is processed atomically:

        Map<String, PaymentDetailsModel> transactions = new ConcurrentHashMap<>();
    
        for (PaymentDetailsModel paymentDetail : allPaymentActivityRecords) {
            transactions.merge(
                    paymentDetail.getTransactionId(),
                    paymentDetail,
                    (existingValue, newValue) -> {
                        if (shouldNotOverrideStates(newValue, existingValue)) {
                            return existingValue;
                        } else {
                            return newValue;
                        }
                    }
            );
        }