Fix NATS store creation concurrency

[burl21]

This PR addresses the case where multiple requests may occur concurrently. It adds a check to see if the key already exists, if it does, it updates it.
Closes #27

[arun0009]

LGTM

[arun0009]

Hi @burl21,

Thank you for the recent improvements to the NATS idempotency logic! I merged the PR based on the clean implementation and passing tests.

However, after a deeper review, I realized that the change to NatsIdempotentStore introduces a race condition that breaks idempotency under high load? Correct me if I'm wrong in my understanding.

The Issue: The IdempotentService relies on the store.store() operation being atomic—specifically, it expects it to fail if the key already exists (acting as a distributed lock).

The new createOrUpdate method catches the "Key Exists" error (10071) and proceeds to put (overwrite) the value:

// Current Implementation
catch (JetStreamApiException e) {
    if (e.getApiErrorCode() == 10071) {
        kv.put(key, value); // <--- This overwrites the lock!
        return;
    }
    throw e;
}

This means if two requests arrive simultaneously:
Request A creates the key (success).
Request B tries to create -> fails -> overwrites Request A's entry -> success.
Both requests execute the business logic.

Proposed Fix: We should revert createOrUpdate to only use kv.create(...) for the initial store() operation. Updates should only happen via update() method when the operation is completed.

We should also update NatsIdempotentServiceTest to specifically test for this race condition (perhaps by ensuring store() throws when key exists).

[burl21]

Hi @arun0009 yes you’re right.I had actually commented, but apparently I didn’t click Comment. My bad 😄
Comment:

I’m not entirely convinced yet. I see two possible approaches.
1. Change the interface to return a boolean. Introduce a boolean store() method in the interface. If it returns false, the core can treat the request as “already exists”. We would deprecate void store(…) and keep it as a default method.
Pros
- Explicit and simple contract: the outcome is clear from the return value
- No need to rely on exceptions for expected flow
- Easier to handle in the core without try/catch
Cons
- Requires a change to the interface (evolving the API)
- Implementations should be updated

2. Throw an exception (KeyAlreadyExistsException) Keep the current interface unchanged and throw a KeyAlreadyExistsException when the key already exists.
Pros
- No interface change required
- Clear error signaling when the operation cannot be completed
Cons
- Uses exceptions for control flow
- Requires try/catch handling in the core, which may be less clean for an expected scenario

I think the second solution is preferable, even though it may not be immediately clear for those implementing it (it would be worth adding this clarification to the javadoc of `void store()`).
At the moment it gets overwritten and race condition scenarios may occur. Even now, two requests are treated as two independent business operations.

I think this applies to all stores, in the sense that it’s not handled at the core level or am I misunderstanding?

At the moment, at least for NATS, if an update isn’t performed the request fails, which is correct because they’re treated as two separate business operations.

[burl21]

@arun0009 let’s continue with PR #29