How we radically simplified new feature
Problem
In our application we have some reports that users can edit.
Trouble is, multiple users could open same report in edit mode and overwrite their work.
This problem got even more relevant as we introduced autosaving the report in background every 30s.
(We actually noticed this problem when one user opened the report in two tabs which kept rewriting each other).
So, we sat down and thought about possible solutions.
Looking for solutions
Solutions considered, and not chosen
We could solve the problem using some sort of collaboration mechanism, backed by CRDT.
But that would be very complex solution for problem we don’t have - we don’t need to support collaborative editing in our product, we just want to prevent collisions by mistake.
We could also save history of changes, allowing to rollback overwrite in case it happens.
But this would cause serious strain on database space for few cases, and finding good UX for this would also be difficult.
We were also thinking about using broadcasting/websockets to keep multiple clients up-to-date.
But it again introduces new complexity, it’s difficult to debug, and so we don’t want to have that as a core feature, more like progressive enhancement.
No, we wanted something simple, without much overhead, and easily understandable to our users.
First attempt - Resource locking
As we are building tool for architects, we looked for inspiration to other tools they use.
And we found it’s not uncommon (in 3D modelling tools, for example) to lock/claim resource, so only one user can edit it.
We liked that, so I started to write implementation for that (actually, first the specs).
Basic idea went like this:
- each report can have
lockattached - this
lockis valid for 5 minutes, prolonged by autosave, and released by manual save/cancel - lock is created by opening
editpage
Actually, in the middle of this I found there’s already
lockmethod on ActiveRecord objects, so I didn’t want to make confusion, and renamed it toWard, which has nice magical feeling. But I’ll keep referring to it as lock for this article, as it’s more readable.
Technically, it meant:
- creating
Lockmodel, with polymorphiclockableand generated uniquelock_id - adding
Lockablemodel concern for interactions - sending
lock_idwith update request for validation - recognizing autosave/manual save and have separate logic for them
- 9 request specs (for report
show,editandupdate), 10Lockablespecs (asRspec.shared_example, adding bit obscurity) - and some other stuff
All in all, we got to +461 -8 line changes across 16 files.
Even thought it was complex, it seemed working, I liked how the code was structured, and would merge it.
But then I did some additional testing, which showed me small problem.
As we have Turbo link prefetching turned on (meaning Turbo sends GET request when user hovers over link to make navigation faster), even just hovering over Edit button locked the resource.
This could be solved by turning of prefetching of course, but 1) it could be easy oversight on future reports and 2) it showed deeper problem:
GET request shouldn’t have side effects.
I went into thinking about how to get around this, and all solutions did make the whole thing much messier. I scratched my head about it for some time.
Thenk I discussed this problem with friend from outside the project, and they asked:
“Why don’t you just compare document versions?”
Second attempt - version control using updated_at
Then I realized I got lost in elegance of the engineering of locking mechanism, I didn’t stopped to look for simpler solutions again during the process.
We don’t need locking mechanism, we only want to prevent editing report that was changed after edit page was rendered.
This can be managed much more easily - just render form with version identifier (I used updated_at), and check if it still matches one in database on update.
During implementing this, I was looking for appropriate HTTP response code for unsuccessful update, and I noticed 412 Precondition Failed.
This lead me to small rabbit hole.
Small detour - HTTP Conditional requests
From this response code I found out about If-Unmodified-Since header, and went on to read about conditional requests.
Idea about it is very much what we are trying to solve - one of the mentioned usecases is conflict prevention!
For some time, I thought this will be the best way to manage it, with minimum application logic.
However, I didn’t realize one important thing for some time - I cannot send custom headers with POST request.
As we allow users to send data manually using standard HTML form (with Rails adding _method="put", so it gets routed to update), there’s no way to set Last-Modified header.
I could turn every form submission to PUT request by calling same javascript I use for autosaving, but I didn’t like it - I want to keep my app based on basic HTML/HTTP (as is a good recommendation for Rails apps in general).
So, back to version control, but at least I learned something new.
Final solution - Rails Optimistic Locking
During my research I mostly accidentally stumbled upon one Rails feature I didn’t know about until now - Optimistic Locking.
Looking back, it’s not a surprise that Rails already has solution for this pretty common problem.
It’s very similar to our updated_at solution, we just switch few things:
- add
lock_versioncolumn to our object (which gets incremented with every update) - use normal
updatemethod - rescue for
ActiveRecord::StaleObjectError
I only had to add feedback for users:
- on manual save, it rerenders form and shows warning infobox that they should copy their data outside not to lose them, and refresh page
- on autosave, we show alert with the same, and stop autosaving.
Mind, this is good solution thanks to our autosaving - so user gets the collision feedback in 30s or less. If we didn’t have that, this solution might not be sufficient, as users would have much more difficulty salvaging their draft and managing conflicts.
This solution got us to:
- ~70 lines of actual new/changed logic + 60 lines of tests (4 requests specs)
+324, -189line changes, withrescuebeing duplicated counting for big part of it (this can in future be moved to concern/parent class)- other part was moving most of form render to parent class not to duplicate rendering warning infobox and hidden input
Conclusion
1️⃣ Best solution depends on a lot of things
We had to consider our unique situation to find the best solution - how probable the conflict is (low, as reports are edited rarely, and they are mostly managed by one person), how big conflict can get (small, thanks to 30s autosave mechanism), and what will probably change in future for us (we will probably never need nor real-time collaboration neither complete change history).
2️⃣ Don’t get too attached to one solution
Goal is not to have perfected locking mechanism, when you really don’t need one.
3️⃣ Look into standards and already existing solutions
Even though I didn’t get to use If-Unmodified-When, I got confidence that this mechanism makes sense. Also it forced me to think about difference between using timestamp (Last-Modified) and resource identification etag.
Then it lead me to see how Rails does this in standard way, very similar to what I came up with.
Overall, I really enjoyed this, as there was a lot of engineering, thinking about the problem and experimenting with solutions.
I was proud of my first PR - it was mostly TDD (which was very reassuring for thing like this), cleanly structured
I also had a real pleasure when I was closing the first PR with locking mechanism, as I felt relief we found something simpler, more reliable and maintainable, backed by Rails mechanisms.
Now back to some coding.