After moving an entry, the entry should _not_ be in the cache. This is
just a decision that has to be made, whether we cache the moved entry or
not. I decided to not cache because it is results in less code.
After that check, we get the entry from the backend and then we can
check whether the entry is in the cache, which is then should be.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
When moving an entry, what we did is copying the entry inside the
backend abstraction (the hashmap) from one key to another.
But as the entry itself does also encode its location, we actually have
to rewrite the entire entry. This patch fixes this.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This is a bugfix for a bug in Store::move_entry_by_id():
The old entry was not removed after the new one was created.
The bug is a bit subtle. The issue was, that the internal cache held
open a reference to the old entry (as StoreEntry object) and when that
object got drop()ed, the contents of the entry were written to disk,
which resulted in the old entry being recreated.
This patch rewrites the function to behave well. The most critical part
is that we do not check whether the old entry is borrowed with
`hsmap.get()` but rather `hsmap.remove()`. The trick here is that the
`StoreEntry` object is dropped in the moment the check is done, clearing
the cached object and flushing it to the backend (the disk).
After that, we continue doing the filesystem operation and the cache is
clean.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This patch adds functions to the Entries type which can be used for
filtering by id, either with `contains()` or `starts_with()`.
This is useful for the end-user functionality where the user specifies
the ID of an entry only partially.
The implementation still iterates over all entries. This could be
improved, of course, by implementing a `find`-like function on `Store`
directly. But for now, this is good enough.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This patch changes the Entries::in_collection() interface to return a
Result<()>. This is needed because the fs backend implementation should
be able to check whether a directory actually exists whenever we change
the iterator.
If the implementation detects that the directory does not exist, we can
fail early and error out.
All usages of the interface are adapted by the patch as well.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
With this patch we move the codebase to Rust-2018.
The diff was generated by executing
cargo fix --all --all-features --edition
on the codebase.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
Because the code was so complex before, we had to create an object and
then cast that object into a `StoreId` rather than just creating a
`StoreId` object right away.
With this patch, we're using the code-generation approach to generate a
function that creates a `StoreId` object based on the name of the
current module. That's way easier and error handling was also improved
by the switch to the new implementation.
The patch also includes a rewrite of all usages of ModuleEntryPath and
changes them to `module_path::new_id()` calls.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
We iterate over full pathes to store entries here, which causes
`StoreId::new()` to error. But if we use the internal parsing helper
`StoreIdWithBase::from_full_path()` and then call `::into_storeid()` on
the resulting object, everything is fine.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This is a more complicated functionality (implementation-wise) of the
`Entries` iterator, which allows a callee to transform it into a
`StoreIdIterator`.
It uses the crate internal `PathIterator` type and a function to extract
the internals of that iterator which is then turned into an iterator
over `Result<StoreId>`, which can be used to build a `StoreIdIterator`.
The implementation is ugly, but we need to be able to transform a
`Entries` iterator into a `StoreIdIterator` in the
`libimagentryannotation`, and possibly in more cases where we want to be
agnostic over iterators.
Maybe there is a cleaner solution to this, hence the comment about
whether this should be removed.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
With this change applied, libimagstore does not export backend
representing types anymore.
This change is necessar because when we want to switch the `StoreId`
implementation from "complex and stateful" to "Stateless and Cow<'_,
&str>", we need to be able to have a type which represents a "`StoreId`
plus the path of the Store itself".
This "the path of the Store itself" is passed around as reference, to
minimize runtime impact.
Because such a type should not be exported by the libimagstore crate, we
make it `pub(crate)` internally. But because the backend APIs also have
to use this type, we would export the (private) type in the APIs of the
backend.
Because of that we make the backend API also non-visible to crate users,
which also decreases the surface of the libimagstore API itself.
Besides:
Remove function `Link::with_base()` which is not needed anymore (was
used in tests only).
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This changes the implementation of the StoreId type to be less complex.
Before we always had to re-attach the path of the store itself each time
a StoreId object was passed to the store. Now we do not do this anymore,
hopefully we can move to a implementation of the Store API which takes
references of StoreId objects in the future.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This patch fixes a bug where the following code (here pseudocode) did
the wrong thing:
store.entries().in_collection("foo").for_each(||...)
because the `.in_collection()` call for the underlying PathIterator
object did not actually re-build the iterator. It only changed the
contained `PathIterBuilder`, but did not call `.build_iter()` on it to
rebuild the iterator object.
The test added with this patch checks whether the iterator does the
right thing.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
::stf::fs::create_dir_all() takes any ref to `Path`, which is what we
have here, so we can leave out the allocation of a new PathBuf object
here.
Also remove the match by a `if let Some(_)`, which increases the
readability a bit.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
With this change, the cache is tested before accessing the filesystem,
which probably increases the speed if the cache has the entry, because
we avoid the slow IO operation.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This fixes a really ugly bug where the in-memory backend for the store
did not remove the entry from the in-memory hashmap on "move", but
simply copied it from the old location to the new one.
That caused tests to fail after the fixes introduced for the
Store::get() function which checked the filesystem and the internal
cache whether an entry exists before the actual "get" operation, because
an old entry would still exist after a move (only in the testcases).
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
Fixes: 09e8619cf5 ("libimagstore: Move from error-chain to failure")
This changes the implementation of Debug for the FileLockEntry to be
more explanatory of how the entry actually looks like. It does not only
print the path of the store anymore, but also the location of the Entry.
Printing header and content would be still too much, tho.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
for safely creating a `String` object from `StoreId` which can be shown
to the user.
Mainly introduced because this is useful for error handling (when
putting a `StoreId` into an error kind, the compiler complains that
`StoreId` does not meet the required trait bounds. But `String` does and
we do not process the ID any further anyways).
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
The implementation of the in-memory filesystem for testing imag code did
not actually use `HashMap::remove()` when an entry was moved, but
`HashMap::get().cloned()`, which caused the original entry to exist
_after_ the move.
I'm not sure why this did not fail much earlier, but it was clearly
wrong. This commit adjust the test to check the "filesystem" before
checking the store and fixes the bug.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This actually caused tests to fail if there was indeed a file at
/tmp/store/test and the test tried to create a "test" entry in the store.
Do use backend instead to check whether entry actually exists.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
