This fixes a bug in 25911b44a6 where the
check in the storage implementation was too strict (not allowing
`clients.latest_version_id == Uuid::nil()`), causing spurious failures.
Well, two bugs, one in each storage implementation.
The core crate calls `get_client` and verifies the `latest_version_id`
before it invokes `add_version`. With the SQLite backend, transactions
lock the entire database, so these two queries cannot be interleaved
with any changes to the `latest_version_id` and there's no possibility
of incorrect updates. With Postgres (#129) the effect is similar: the
read performed by `get_client` locks that row and prevents other
transactions from writing to it.
However, the storage trait should not rely on this behavior --
`add_version` should verify that it is adding a new version on top of
the correct parent version.
Improve error handling in the inmemory storage
This addresses a TODO, in a type that is really only used for testing.
This also adds a test for a similar circumstance -- adding the same
version twice -- in the SQLite storage, but it is already handled
correctly.
Transactions for different client_ids cannot interfere with one another,
so this provides an opportunity for the sort of concurrency that a
mult-client hosting solution might need. For example, a postgres backend
could lock the client row in each transaction.
This is a bit tricky because the `Storage` trait is `Send + Sync`, but
an SQLite connection is neither. Since this is not intended for
large-scale use, the simple solution is just to open a new SQLite
connection for each transaction. More complex alternatives include
thread-local connection pools or a "worker thread" that owns the
connection and communicates with other threads via channels.
This also updates the InMemoryStorage implementation to be a bit more
strict about transactional integrity, which led to a number of test
changes.
