7

Considering that:

  • when using the repository pattern you deal with entities - those are the atomic units you persist (regardless of Hibernate or "manually")
  • when changing an entity you save it as a whole (there's no set or increment field)
  • multiple application instances might be running
  • the same entity might be fetched/changed/saved concurrently by different application instances

Won't this lead to bad data in the end? Consider the use case of "let me save the amount of password retries in the user entity". Won't that be problematic if an attacker launches many multiple client requests to login? Instance A fetches entity, instance B fetches same entity, instance A changes/saves it, instance B changes/saves the original entity unknowingly of A's change/save.

Christophe
  • 74,672
  • 10
  • 115
  • 187
Luís Soares
  • 271
  • 1
  • 7
  • 2
    I know that this is avoiding the specific problem of updating a count field in an entity, but for your particular problem of password retries, could you simply insert each retry attempt in the database, and do a `COUNT(...)` on that table? – Matthew Jan 09 '20 at 19:01
  • 1
    Good idea! That respects the repository pattern. Creating the entity UserRetries so it can be managed. Let's see if there are more answers but it makes sense. – Luís Soares Jan 09 '20 at 19:06
  • 2
    No problem, I'm genuinely curious about your particular question too. I've seen examples of setting the transaction isolation to serializable, or having a last-updated field on the entity to ensure you're always dealing with an unchanged entity when saving, though a practical example would be best. – Matthew Jan 09 '20 at 19:09
  • The weird part is that this applies to any field and you can't be creating tables per every field. – Luís Soares Jan 09 '20 at 19:20
  • 2
    I don't know if that's weird necessarily, different applications have different requirements in terms of concurrency. For example, if you have a CMS application you probably want to make sure no two people edit a page at the same time, though on StackOverflow for example, editing your own personal profile doesn't have the same concurrency requirements as only a single user should be editing your profile at a time. – Matthew Jan 09 '20 at 19:24
  • 1
    I agree with Matthew here: The case you discribed have a really specific requirement to concurrency. In most cases, you're usually totally fine with having the last arrived data "win" and getting saved. I would, btw., also go with one of the solutions Matthew mentioned, however, the first one sounds interesting, I never thought about that really and it sounds promising :) – Florian Jan 09 '20 at 19:55

1 Answers1

6

The repository pattern does not intend to solve concurrency issues. It only provides a convenient way to work with persistent entities.

In principle you'd use your repository pattern in combination with:

  • Unit of work: it keeps track of changed objects that need to be updated consistently in the database, and manages potential concurrency issues if they happen;
  • Identity map: it ensures that the same entity is not loaded into distinct objects in the same client session, which could lead to inconsistencies.
  • Concurrency patterns: these manage the consistency when several clients or services are involved.

And concurrency patterns are a complex topic. Martin Fowler, in his excellent book Patterns of Enterprise Architecture identifies for example the following alternatives:

  • optimistic lock: you acquire the lock when data needs to be written, hoping everything will go fine;
  • pessimistic lock: you try to gain exclusive access before the data is changes.

But microservices architectures having data of different services in different databases, use more creative alternatives. Chris Richardson for example, proposes in his excellent Microservices Patterns the saga pattern: instead of locking data in a two phase commit scheme, it uses an orchestration of steps that follow the do/undo logic with status management.

Christophe
  • 74,672
  • 10
  • 115
  • 187
  • Thanks for the insightful answer! What's your opinion on the comments above? Would you see them as ways to avoid the problem? – Luís Soares Jan 10 '20 at 20:46
  • 1
    I'm not a fan of adding counters in the database to manage consistency. Because concurrent sessions could die leaving the counter unupdated. If you want to avoid the problem and if a lot of people might work on the same data, go for the pessimistic lock. If the risk of conflict is low, go for the optimistic lock and a retry strategy. Caution: before you're doing the retry, you must assess if it is desirable. THis can be done by looking at what changed in DB compared to what was originally loaded in the session and involve the user to see if new version can be merged with intended changes. – Christophe Jan 10 '20 at 20:58
  • and what about the use case I presented in the original question? ("number of password retries"). what pattern would you use in relational? what about non-relational? – Luís Soares Jan 10 '20 at 21:13
  • 1
    @LuísSoares Sorry, I misunderstood the use case. This is very specific. First, you could consider a pessimistic locking, since the user is supposed to try only one password at a time. Optimistic lock is there to maximise throughput, but for passwords, you do not want to facilitate the work of the attackers, do you ? Now even if you go for an optimistic lock, the change is about how many more attempts were done (i.e. new value-old value). You can do a retry very easily withoutinvolving user: just re-readig the latest status from db and add the delta. – Christophe Jan 10 '20 at 21:41
  • 1
    With pessimistic lock, you are sure that only one instance can update the user, and you are sure that your comparison to the max attempt is always correct. With an optimistic lock, and retry, you may exceed the number of max attempts in the retry. So you'd bloch the user account after 15 tries instead of 10 ? comapred to the hundreds of thousands more attempts needed to crack the password, it's negletible IMHO. – Christophe Jan 10 '20 at 21:44