On master, we changed the Store::update() function to do nothing more
than calling Store::_update() and wrapping an error.
On the libimagstore/store-tests branch, we modified the
Store::_update() function to contain an additional flag in the
parameters, whether the File-Lock-Entry internal `precense`-value should
be updated.
This merge commit resolves that conflict.
This is a bugfix for an very particular issue.
Here's what happens:
If we create() an FileLockEntry and then update() it, we are running
into a problem because update() calls _update() which changes the
precense status of a FileLockEntry.
Because update() is _consuming_, the FileLockEntry gets drop()ed
afterwards. This _again_ causes _update() to be called, but with a new
presence status, which is not true in this moment (as the FileLockEntry
is still borrowed, but we already marked it as present).
This patch is a short-term fix. The real problem is, that our Store
interface is consuming. If the Store interface would be non-consuming,
this issue wouldn't happen, as the drop() call would not happen.
I'm rather sure that this patch will not be reverted in the process of
rewriting the Store interface to be non-consuming. But we never know.
This is a bugfix for an very particular issue.
Here's what happens:
If we create() an FileLockEntry and then update() it, we are running
into a problem because update() calls _update() which changes the
precense status of a FileLockEntry.
Because update() is _consuming_, the FileLockEntry gets drop()ed
afterwards. This _again_ causes _update() to be called, but with a new
presence status, which is not true in this moment (as the FileLockEntry
is still borrowed, but we already marked it as present).
This patch is a short-term fix. The real problem is, that our Store
interface is consuming. If the Store interface would be non-consuming,
this issue wouldn't happen, as the drop() call would not happen.
I'm rather sure that this patch will not be reverted in the process of
rewriting the Store interface to be non-consuming. But we never know.
to actually test:
1. get -> Should return Ok(None)
2. create -> Should return Ok(())
3. get -> Should return Ok(Some(_))
4. delete -> Should return Ok(())
5. get -> Should return Ok(None)
This is a integration-merge, because the libimagstorestdhook/git branch
got really long and rebasing is not an option, as it would mainly
destroy history.
Using an integration branch is less hassle, really.
We must return an error if the file is not found here.
This is because if we unwrap() here, we panic if the store action was
Store::retrieve() and there wasn't a file there. We then unwrap() on
None and panic because of this, causing all other tests to panic as
well because the Mutex gets corrupted.
The store handles FileNotFound errors on its own, so it is safe to
return the error here.
